A number of places that were sending emails assumed they had access to
flowId and flowBeginTime on the request payload. However, we're in the
process of changing some of those endpoints so that they don't receive
metrics context data in the payload.
To fix this, all endpoints are changed here to read metrics context data
via a lazy getter that falls back to loading from memcached if there's
no flow data in the payload. This happened to be a nice refactor anyway,
fitting in with our broader adoption of lazy getters. It makes the code
more resilient to change and reduces the frequency that we hit the cache
with.
We are planning to deprecate metrics context payloads on endpoints where
it is not strictly required. This means only endpoints that start a flow
or endpoints that occur outside of any flow will accept metrics context
payloads in the future.
The motivation for making this change is to reduce the likelihood of
metrics weirdness arising from conflicting metrics context data being
set mid-flow.
The account reset flow includes three different token types. By stashing
metrics context as each new token is created, we can stop sending it to
the subsequent endpoints in some future content server change without
interrupting metrics.
The BrowserID verifier insists that we advertize "provisioning" and
"authentication" routes in /.well-known/browserid but nothing in the
current architecture requires these routes to actually exist. Let's
remove the unused HTML content to reduce future potential attack
surface.
Fixes#2106.
Prevents us from accidentally calling db.devices more than once per request. I saw one definite case of this in /recovery_email/verify_code and it's possible there were others. I'll also be making use of this property heavily for the amplitude events, so it will get further usage imminently.
Making the change necessitated pulling calls to db.devices out of lib/push, which triggered some refactoring that almost got away from me. I'll add inline commentary to call out why things have changed the way they have, but most push methods now take an extra devices argument and a few other methods became redundant so I deleted them. I don't think I've broken anything.
Session tokens that have no device record and are older than 4 weeks old
(by default) will now be rejected as expired by all auth server endpoints.
Additionally, the `/account/sessions` endpoint will filter out expired session
tokens on the same basis.
https://github.com/mozilla/fxa-auth-server/pull/1996
r=vbudhram
This settles our dance of `Buffer` vs `String` down to simply this:
> You have a `String`. You should (almost) never have a `Buffer`.
Buffers are useful for talking about a specific set of bytes, without an
encoding. In our app, the places where this is useful are:
- crypto
- mysql
We don't actually speak MySQL in this repo anywhere, so that leaves us
with only crypto. Instead of requiring the mental overhead of "Do I have
a buffer or a string?" throughout all our code base, we can just push
that completely into the crypto code.
This *should* reduce bugs where we aren't sure if we have a `Buffer` or
a `String`. If you're not in crypto, you should just have a `String`.
This refactors our remote test driver to stop spawning multiple
child processes to run our servers, and instead to run the servers
in the same process.
- By using the same process, we can pass configuration as a plain old
JavaScript object, and not have to be adjusting the `process.env`.
While writing this patch, `process.env` pollution was already found
to make some tests dependent on others running first. Now, we can
isolate the tests by starting a server with a private config object,
and the other tests are non the wiser.
- By not starting up and tear down child processes for each suite of
remote tests, the full set runs much faster. In my case, running the
remote tests went from ~4 minutes to ~1 minute.
* feat(hpkp): Add hpkp support
This adds support for sending HPKP headers in all requests. Feature is disabled by default and should be put in report only to avoid any footguns.