mozilla / fxa-oauth-server

OAuth server for Firefox Accounts
49 stars 40 forks source link

Prepare a train-123 point-release that removes tokens.profileChangedAt #620

Closed rfk closed 6 years ago

rfk commented 6 years ago

As @jrgm noted in email, the migration from https://github.com/mozilla/fxa-oauth-server/pull/607 is running into some difficulty in production due to the enormous size of the tokens table. We may need to revert that part of the change in order to unblock the rest of train-123.

In preparation for that eventuality, let's:

If it turns out that we can't apply the migration in prod, we can merge the above PR in order to sync up the database state between prod and our dev environments, then deploy the point-release against the production database to unblock train-123.

We can then do another migration to bring back the profileChangedAt column in a future train, once we've successfully purged expired data from the tokens table in production.

@jrgm does this capture what we discussed earlier?

@vbudhram thoughts?

rfk commented 6 years ago

mozilla/fxa-auth-server#2638

I'm pretty confident that rolling back the change in oauth-server won't break anything in auth-server, since auth-server only records this information and pushes it out in the identity certificate, it doesn't expect to be able to receive any profile-changed-at information back from other services.

mozilla/fxa-profile-server#343

If we back out the profile-changed-at functionality in oauth-server, then in the code here:

https://github.com/mozilla/fxa-profile-server/pull/343/files#diff-741b62ef36092821244dbc9e55bc1af4R82

We will have creds.profileChangedAt as undefined while result.profileChangedAt will be a real timestamp. In javascript, nothing will evaluate less than undefined, e.g.:

> 12345 < undefined
false
> Date.now() < undefined
false

Thus the (result.profileChangedAt < creds.profileChangedAt) condition will never be true, and we will not attempt to purge the profile cache.

So I think the auth and profile changes should be safe in the face of rollback. We can test that out explicitly in stage if we need to go this route.

jrgm commented 6 years ago

@jrgm does this capture what we discussed earlier?

Yes it does.

vbudhram commented 6 years ago

Thus the (result.profileChangedAt < creds.profileChangedAt) condition will never be true, and we will not attempt to purge the profile cache.

That looks right to me. I'll prep an oauth migration for consideration.

vbudhram commented 6 years ago

Fixed in https://github.com/mozilla/fxa-oauth-server/pull/621