This was a surprisingly involved change. Some of the difficulties included
java.util.Optional purposely not being Serializable (so I had to move a
few Optionals in mapreduce classes to @Nullable) and having to add the Truth
Java8 extension library for assertion support.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=171863777
This CL adds transferredRegistrationExpirationTime as a TransferData field
persisted to Datastore. It's only relevant for domains, and it represents the
registration expiration time resulting from the approval of the most recent
transfer request. For pending transfers, we assume the transfer will be
server-approved, and thus in DomainTransferRequestFlow we set this field to the
existing computed value serverApproveNewExpirationTime, which is what we use
for setting up the server-approve autorenew billing event and poll message.
In DomainTransferApproveFlow we overwrite this field with the freshly computed
newExpirationTime, whereas in DomainTransferCancel/RejectFlow (and in the
implicit cancel of DomainDeleteFlow during a pending transfer) we null it out.
There are two key benefits to having this field, which are described in more
detail in b/36405140.
1) b/25084229 - it allows storage of a frozen value to back the "exDate" field
of DomainTransferResponse, which we can use to fix various errors with how
exDate display currently works.
2) b/36354434 - it allows DomainResource.cloneProjectedAtTime() to just directly
set the registrationExpirationTime to this value, without computing it de
novo, which reduces duplicated logic and ensures that the new expiration time
matches the autorenew child objects.
This CL only starts writing the field on TransferData as persisted directly on
the DomainResource itself. We'll then want to backfill the field for at
least pending transfers, whether expired or not (so we can do (2) above), but
I think we might as well backfill it for all pending and approved transfers
so that we also fix (1) even for historical transfers. And then we can start
actually reading the field for both purposes. (Note that for (1), this will
only fix synchronous transfer responses served via DomainTransferQueryFlow,
not async transfer responses served via poll messages, since these have already
been persisted with a potentially bad exDate, but I don't think it's worth a
backfill for those).
One last naming note: I chose the verbose transferredRegistrationExpirationTime
rather than the extendedRegistrationExpirationTime of DomainTransferResponse
because (as is the case in autorenew grace, or for a superuser transfer) the
new registration time isn't necessarily extended at all; it may be the same as
the pre-transfer expiration time. Also, including "registration" helps clarify
w.r.t. pendingTransferExpirationTime which refers confusingly to the expiry of
the transfer itself, rather than the domain registration.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=171858083
Also fixes the issue that dry run EPP commands were incorrectly being
reported on.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=171062984
This CL changes the domain and contact transfer flows to check the entire
TransferData on the post-transfer resource, rather than just spot-checking
certain fields. This approach provides much better code coverage - in
particular, it checks that the non-request flows (approve, cancel, reject)
don't modify the fields that they shouldn't be modifying, and that they do
actually clear out the transfer server-approve entities fields written by
the transfer request flow. It's slightly orthogonal, but I also added
testing that the server-approve entities fields are actually set in the
request flows, which was previously untested.
This is pre-work for introducing an exDate-storing field into TransferData,
by making it easier to test everywhere that exDate is set *and* unset only
in the correct places.
As part of this CL, I've introduced a TransferData.copyConstantFieldsToBuilder()
method that is like asBuilder() but instead of copying all the fields to the new
builder, it only copies the logically constant ones: losing/gaining client IDs,
the request time and TRID, and transferPeriod. This is useful both in tests but
is also used in the resolvingPendingTransfer() helper that centralizes the core
transfer resolution logic (as of [] That method has its own tests,
and in the process I removed a bunch of crufty defunct TransferData tests.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=171053454
In the great flow flattening, ResourceFlowUtils grew a couple nice helpers
for rebuilding transferrable resources (Domains and Contacts) upon the
resolution of a transfer - approvePendingTransfer() and denyPendingTransfer().
Most transfer-resolving callsites use one of these two helpers, but for legacy
reasons the deletion flows (DomainDeleteFlow and DeleteContactsAndHostsAction)
were instead using the "manual" resolvePendingTransfer() method or its even more
low-level createResolvedTransferData() helper instead of denyPendingTransfer().
It's simpler to just have two options - approve and deny - so this CL inlines
createResolvedTransferData() into resolvePendingTransfer() and makes the latter
a private helper for the approve/denyPendingTransfer() public helpers.
This CL also adds sanity checks that approve/denyPendingTransfer() are called
only with the logically appropriate values of TransferStatus.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=170819358
Strict autoescaping is the default so they serve no purpose.
Design doc: []
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=170725633
Normally, if a domain is in the auto-renew grace period, a transfer will cancel the auto-renew billing event. In the event of a transfer with no change to registration end date, the auto-renew billing event should not be cancelled and the gaining registrar should not be charged for the transfer.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=170576726
Specifically, this prevents suspended registrars from creating domains or applications. Pending registrars already can't perform these actions because they get an error message when attempting to log in.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=170481338
Void tags (e.g. <img>) couldn't have a closing tag (e.g. </img> is invalid). Non-void tags (e.g. <div>) couldn't be self closing (e.g. <div/> is invalid) and must be closed explicitly (e.g. with </div>). This CL fixes the tags which also prepares the templates for stricthtml which enforces it.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=168829028
The bad prober domain data has since been deleted, so we no longer need
to handle the case where these Keys point to entities that don't exist.
This mostly reverses []
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=168687701
Allow superusers to change the grace period and allow
superusers to change the pending delete length.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=168028545
Allow superusers to change the transfer period to zero years and allow
superusers to change the automatic transfer length.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167598314
Previously, I would cancel all the records associated with HistoryEntry that's
available for cancellation. This could cause unexpected behavior if we
cancelled a historyEntry which itself had cancelled records (in effect we would
negate the negation unintentionally). This is easily remedied by only
cancelling records which want to be cancelled.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167204383
The probers make a constant stream of create and delete calls, which we don't
want to account for when constructing transaction reports. This change will
cause only real TLDs to log create and delete transaction records.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=166737801
This is the last of many cls adding explicit logging in all our domain
mutation flows to facilitate transaction reporting.
The transfer process is as follows:
GAINING sends a TransferRequest to LOSING
LOSING either acks (TransferApprove), nacks (TransferReject) or does nothing
(auto approve). For acks and autoapproves, we produce a +1 counter for GAINING
and LOSING for domain-gaining/losing-successful for each registrar, to be
reported on the approve date + the transfer grace period. For nacks, we produce
a +1 counter for domain-gaining/losing-nacked for each registrar.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=166535579
This is the third of many cls adding explicit logging in all our domain
mutation flows to facilitate transaction reporting.
We add a +1 counter for either grace or nograce deletes, based on the grace period status of the domain. We then search back in time for DOMAIN_CREATE, DOMAIN_RENEW and DOMAIN_AUTORENEW HistoryEntries off the same resource that happened in their corresponding grace periods (5, 5 and 45 days respectively). All transaction records for these events are then given -1 counters to properly account for cancellations in the NET_CREATE and NET_RENEW fields.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=166506010
After working further with domain deletes, I realized we'll need to record multiple reportingTimes under a single historyEntry when issuing a -1 counter to cancel grace-period adds. Since the TLD would be the only shared component within a record, we'll just duplicate it across all records to save an unnecessary layer of hierarchy.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=166261413
This is the second of many cls adding explicit logging in all our domain
mutation flows to facilitate transaction reporting.
Adds and renews each result in a +1 counter for the NET_ADDS/RENEWS_#_YR field,
which I've added simple (# of years, add or renew) -> Enum functions to get.
Allocates are just a special case of adds, and are counted in a similar manner.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165963249
This is the first of many cls adding explicit logging in all our domain mutation flows to facilitate transaction reporting.
Restores are relatively simple- it happens immediately, so the reporting time is just the time of the HistoryEntry, and we add a single "RESTORED_DOMAINS" count of 1.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165639084
Currently we exact the client certificate hash from header X-GFE-SSL-Certificate. This works because the proxy running on [] sends the request directly to the AFE via HttpOverRpc, bypassing the frontline GFE, which would strip away this header.
[]
After the proxy moves to GCP we can no longer use that header. Instead we should use X-SSL-Certificate, which does not get stripped by the GFE. In fact the open source build should never have contained X-GFE-SSL-Certificate because obviously external nomulus users have to go through the GFE to reach the registry app and that header would never have survived.
Without changing how the [] proxy works, this CL makes the registry first try to extract the hash from X-GFE-SSL-Certificate, and fallback to X-SSL-Certificate if necessary. This allows the current setup to continue to work, while the new proxy is being tested.
This should not open us up to attacks because even if an attacker uses a proxy that uses X-SSL-Certificate, it still needs to pass OAuth in order to talk to /_dr/epp.
Similarly, we use X-Requested-Servername-SNI as fallback to X-GFE-Requested-Servername-SNI. This can be eliminated altogether when the [] proxy is retired, because the only reason we check if the client request is SNI enabled (by checking the existence of that header) is because the GFE only requests client certificate when SNI is enabled. The GCP proxy does not have that limitation, and also will be only serving one certificate with all SAN listed in it.
Some formatting change is also introduced by the formatter. They seem to be better conforming to the style guide, so I left them there.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165378083
Added validation on domain creation, preventing a domain from being created if
it equals an existing TLD. Added domain create tests for domains using
multi-part TLDs that shared suffixes and prefixes. Added host create tests for
hosts using multi-part TLDs that shared suffixes.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164297749
This makes the code more understandable from callsites, and also forces
users of this function to deal with the situation where the registrar
with a given client ID might not be present (it was previously silently
NPEing from some of the callsites).
This also adds a test helper method loadRegistrar(clientId) that retains
the old functionality for terseness in tests. It also fixes some instances
of using the load method with the wrong cachedness -- some uses in high-
traffic situations (WHOIS) that should have caching, but also low-traffic
reporting that don't benefit from caching so might as well always be
current.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=162990468
We want to be safer and more explicit about the authentication needed by the many actions that exist.
As such, we make the 'auth' parameter required in @Action (so it's always clear who can run a specific action) and we replace the @Auth with an enum so that only pre-approved configurations that are aptly named and documented can be used.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=162210306
We're already storing this as a metric, but on a registry of our
scale these operations tend to only happen on a daily-ish basis,
for which seeing results in logs is easier to deal with than metrics
(and also still very light-weight).
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=160552291
The --superuser command in the nomulus command-line tool should be
bypassing checks on whether the passed-in registrar client ID has access
to the TLD in question, but currently it is not.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=158974462
These shouldn't ever be null, but we have some bad data in production
for prober TLDs left over from the Registry 2.0 transition. Ignoring
null values here is required to finish cleanup for this old data, which
currently cannot even be deleted because it's throwing an NPE when
trying to update these values.
This commit will be reverted after the bad data is cleaned up, likely
sometime next week.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=158546840
Memcache is already off but now it's not in the code anymore.
This includes removing domain creation failfast, since that is actually
slower now than just running the flow - all you gain is a non-transactional
read over a transactional read, but the cost is that you always pay that
read, which is going to drive up latency.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=158183506
We want to know how long it's actually taking to process asynchronous
contact/host deletions and DNS refreshes on host renames. This adds
instrumentation. Five metrics are recorded as follows:
* An incrementable metric for each async task processed (split out by
type of task and result).
* Two event metrics for processing time between when a task is enqueued
and when it is processed -- tracked separately for contact/host
deletion and DNS refresh on host rename.
* Two event metrics for batch size every time the two mapreduces are
run (this is usually 0). Tracked separately for contact/host deletion
and DNS refresh on host rename.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=157001310
The StatusProhibitsOperationException superclass constructor just wants a raw message to return to the client, so we should pass it an explanation rather than just the superordinate domain name. In fact, I think the superordinate domain name isn't really necessary since it should be obvious from the hostname in the create/update request what the superordinate domain would be.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=156747185
This replaces the memcache caching, which we think is overall a bad idea.
We load all registrars at once instead of caching each as needed, so that
the loadAllCached() methods can be cached as well, and therefore will
always produce results consistent with loadByClientIdCached()'s view of the
registrar's values. All of our prod registrars together total 300k of data
right now, so this is hardly worth optimizing further, and in any case this
will likely reduce latency even further since most requests will be
served out of memory.
While I was in the Registrar file I standardized the error messages for incorrect
password and clientId length to be the same format, and cleaned up a few
random things I noticed in the code.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=156151828
The logging for exceptions in FlowRunner - always at WARNING - has long been sub-optimal. For EppExceptions it's too aggressive/spammy to log at WARNING because it's generally not actionable - EppException gets properly thrown for all kinds of ordinary reasons (trying to create a resource when one already exists with that foreign key) and/or for client misbehavior that we can't control (sending bad parameter values, etc.). For non-EppException RuntimeExceptions, it's redundant with existing logging in EppController.
This CL resolves this by removing that logging in FlowRunner entirely in favor of the EppController logging, where we're now logging EppExceptions at INFO in parallel with the existing logging of RuntimeExceptions at SEVERE. This has the benefit that we're now logging EppExceptions that come from FlowPicker (by way of EppExceptionInProviderException), which previously were unlogged.
Note however that this does mean that in places where we run FlowRunner without EppController - exclusively test code as it stands today - we'd no longer be logging EppExceptions. If that seems like a loss, we could either reinstate logging there (at INFO) and just deal with redundant messages for most EppExceptions, or we could add it manually to places where we call FlowRunner.run() in tests and avoid the redundancy that way.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154733365
TESTED=For all tests, I added @Cache to DomainBase because otherwise the tests will
fail. We aren't ready to do this in prod yet, which is why the tests have a TODO
in them. The new tests fail if you change line 134 in Ofy to not use memcache
and either use the unchanged original flow code, or use the new
inlined code and change loadWithMemcache() to load(). They pass with the new
inlined code that calls loadWithMemcache(), as long as the @Cache is added to
DomainResource.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154457655
TESTED=For all tests, I added @Cache to DomainBase because otherwise the tests will
fail. We aren't ready to do this in prod yet, which is why the tests are still
marked @Ignore. The new tests fail if you change line 134 in Ofy to not use memcache
and either use the unchanged original DomainCreateFlow code, or use the new
inlined code and change loadWithMemcache() to load(). They pass with the new
inlined code that calls loadWithMemcache(), as long as the @Cache is added to
DomainResource.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154224748
This avoids loading all contacts and hosts before the failfast runs
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154179321
As part of b/36599833, this makes FlowReporter log the tld(s) of every domain
flow it executes, so we can provide ICANN reporting totals on a per-TLD basis.
It also adds several other fields that we're computing anyway and which seem
useful, particularly for debugging any issues we see in production with the data
that we're attempting to record for ICANN reporting. The full set of fields is:
- commandType (e.g. "create", "info", "transfer")
- resourceType* (e.g. "domain", "contact", "host")
- flowClassName (e.g. "ContactCreateFlow", "DomainRestoreRequestFlow")
- targetId* (e.g. "ns1.foo.com", "bar.org", "contact-1234")
- targetIds* - plural of the above, for multi-resource checks
- tld** (e.g. "com", "co.uk") - extracted from targetId, lowercased
- tlds** - plural of the above, deduplicated, for multi-resource checks
* = only non-empty for resource flows (not e.g. login, logout, poll)
** = only non-empty for domain flows
Note that TLD extraction is deliberately very lenient to avoid the complexity
overhead of double-validation of the domain names in the common case.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154070794
This wasn't being recorded correctly because the clientId is only
set in LoginFlow after the flow succeeds, whereas we were
previously logging the clientId before executing the flow. This
adds special handling for LoginFlow. Note that we only set the
metric label to the clientId for valid registrar logins, to ensure
that metric cardinality doesn't grow unbounded (as it might if we
used every arbitrary string passed in as an attempted login).
This also refactors creation and handling of FakesAndMocksModule so
as to be able to make test assertions about EPP metrics from
integration flow tests.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154048280
Since this feeds into ICANN reporting, we don't want to muddy the data
there with dry-runs, which are always internal-only artifacts of tool usage
and shouldn't really count as real attempts to do SRS actions, since they
are always going to abort with no effect.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=153856915
This prevents a possible failure mode of the logging where the logged
EPP input XML is very large (which can happen e.g. for domain creates
with large SMD values). In those cases, the XML might cause the overall
JSON string to be too large to fit within a single log entry [1], in which
case it gets split over multiple lines and breaks automatic parsing.
This mitigates that case by logging the EPP input (raw and base64-encoded)
in a separate log statement so that the more compact metadata (like clientId)
and derived values (like ICANN reporting field) will still be in an intact
JSON string even in that case, and can still be readily parsed. It's okay
if the actual EPP XML is harder to parse, since once we're logging the right
metadata fields we shouldn't need to automatically parse the EPP XML in any
normal cases.
[1] I haven't found this exact limit or splitting algorithm, or whether it's
a property of java logging or GAE log ingestion. The GAE logs page does note
that a single application log entry (within a request, which can have up to
1000 such entries) maxes out at 8KB, so that might be it:
https://cloud.google.com/appengine/docs/standard/java/logs/#writing_application_logs
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=153771335
Since this reporting is getting more complicated (see b/36599833), it'll
be better to have a dedicated class to encapsulate it, which also lets us
keep the tests separate and focus FlowRunner more on its core purpose of
actually running the flow.
Note that this doesn't move the legacy log statement logging because that
specifically must be logged from the FlowRunner.run() method to preserve
the existing log signature matching in our ICANN activity reporting query.
(The new statement is designed to be robust to moves like this since it
doesn't use the logging callsite to match log lines, and it's not in use
yet anyway.)
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=153762008
