Remove ability to fetch email address from the oauth-server API

[rfk]

When mozilla/fxa-auth-server#1053 lands, tokens with "profile:email" scope can be used to fetch the user's email address directly from the auth-server. We should do so rather than keeping a copy of it in our own db. This will allow us to drop the "email" column from the "tokens" table, unblocking the final landing of service tokens.

[seanmonstar]

Is anyone getting the user's email from the /v1/verify request besides the profile server? If not, that's an easy deletion. If so, they shouldn't be! Shame on them!

Also if so, then I imagine the verify route will need to be able to detect recursion, since when verify is called, it would have to ask the auth-server, passing the token, which would turn around and ask the oauth-server to verify the token (wanting to fetch the email again)...

[rfk]

IIRC the basket proxy does, but that's an easy fix on our side. Sadly, it is part of our documented API, so others could legitimately be looking for it and we'll have to move carefully if we want to remove it.

[seanmonstar]

Hm, I don't see how to stop the recursion without the auth-server sending an additional parameter when it wants to verify a token...

[rfk]

Agree. We should try to remove it, since I really don't think anyone should be using this endpoint except for our own services.

[seanmonstar]

@vladikoff I tried just chopping the email out, and found out that developers use the email address everywhere... Any change would require the console to update as well.

It looks like the developer routes require oauth authorization, and the developer's email is used from the verified token. What if instead we just keyed everything regarding developers with their FxA uid?

[vladikoff]

What if instead we just keyed everything regarding developers with their FxA uid?

@seanmonstar we need to validate that the user is @mozilla.com email

[seanmonstar]

@vladikoff oh right. Hmm, but that validation only needs to happen once, when logging in, right?

[vladikoff]

@seanmonstar Yeap!

[rfk]

Having mulled this over a little, I feel like we should try to set a good habit here of moving cautiously with changes to our public-facing APIs, even if it means eating a bit of short-term complexity/ugliness internally. I'm pretty sure that nothing outside our own systems will break if we yank email from the /v1/verify response, but let's do better than "pretty sure".

Strawman:

1. Add some (hacky, awful, internal-only) query parameter that opts out of receiving the email, as suggested in Remove ability to fetch email address from the oauth-server API #352 (comment)
2. Update auth-server, profile-server, basket-proxy etc to all use it, temporarily
3. Add logging for any requests to /v1/verify that do not include the flag, so that we can detect whatever strange system it is that we've inevitably forgotten about...
4. Watch it in production for a train, fixup other consumers as we find them, and remove the hackery once we're confident nobody is using it.

That's more work for us, but not too much more, and it keeps us moving forward. @seanmonstar what do you think, worth it, or no?

[seanmonstar]

That seems fine to me. I'll create an issue and bump this one back to next.

[rfk]

I think the title of this issue now out of date, and we plan to just completely drop the ability to retrieve an email from the oauth-server API. I'm changing it, but @seanmonstar please change it back if that doesn't reflect your current understanding of the situation.

[vladikoff]

@rfk https://kibana-fxa-us-west-2.prod.mozaws.net/#dashboard/temp/AWK1heimTlXM2FwOOFau

i think there is a single client that asks for this atm

[rfk]

Looks like this is kintowe:

client_id: 5882386c6d801776
scope: ["sync:addon_storage"]

@glasserc could you please point us to the code kintowe uses for oauth token verification? Is it using https://github.com/Kinto/kinto-fxa/ or something else?

[glasserc]

Yes, that's it!

[rfk]

Hrm. As far as I can tell, kinto-fxa just calls OAuthClient.verify_token here:

https://github.com/mozilla/PyFxA/blob/master/fxa/oauth.py#L176

Which doesn't explicitly request the "email" field. So at first glance I'm not sure what's actually causing it to request this.

[rfk]

On reflection, the client_id being reported here is the cliend-id to which the token was issued, but it doesn't necessarily indicate what service is calling the /verify endpoint. It might be something other than the kintowe server stack.

[rfk]

On further reflection, I think I'm misinterpreting things here. The linked kibana dashboard shows a sudden drop in these events at around 04:00 04/11, which I believe corresponds pretty closely to train-109 going to production.

If you filter out events for the client_id of "5882386c6d801776", there are in fact a much smaller number of events in there from other clients.

So what I think this indicates is, prior to train-109 we were logging the "routes.verify.email.requested" event for basically every request, since the email request parameter was defaulting to true. After deploying train-109 it is now defaulting to false, and we're not logging any of these events. So, I don't think any services are requesting this in production \o/

@vladikoff does that make sense?

[vladikoff]

@rfk ah that makes sense , i just thought kibana was behind, but it actually was empty.

[vladikoff]

@deeptibaghel wanna try closing this out?

Basically we need to remove support for this parameter: https://github.com/mozilla/fxa-oauth-server/blob/master/lib/routes/verify.js#L15

Some tests will probably start failing, for example this test could probably be removed or changed to assert a "unknown payload email": https://github.com/mozilla/fxa-oauth-server/pull/533/files#diff-9265a69d5e81bfaab4edbd417a33a626R2707

@rfk confirm?

[rfk]

Yep. We'll also need to stop auth-server from sending an explicit email: false by removing the "extras" config option here:

https://github.com/mozilla/fxa-auth-server/blob/master/config/index.js#L544

We'll probably need to continue to accept the "email: false" parameter for a train cycle until we can confirm that everything has stopped sending it, but we can certainly remove all the logic for "email: true" at this time.

[deeptibaghel]

@vladikoff sure , happy to work this :)

[shane-tomlinson]

Yep. We'll also need to stop auth-server from sending an explicit email: false by removing the "extras" config option here:

Has an issue on the auth-server been added for this?

[vladikoff]

@deeptibaghel the PR is a good start but we need to do 1-2 other PRs before merging it.

We'll also need to stop auth-server from sending an explicit email: false by removing the "extras"

So fxa-auth-server needs a change for that first

[deeptibaghel]

@vladikoff i will work on removing it from fxa-auth-server