1243174 - Review proposed FxA integration for leaderboard

Status: RESOLVED FIXED

(Cloud Services :: Server: Firefox Accounts, defect)

Description

[Ryan Kelly [:rfkelly]]

I'm spinning off a new bug from Bug 1225620 Comment 6, where :jkerim wrote:

The stumbler client and Leaderboard server will be using the FXA oauth redirect flow for sign in and will not be generating their own tokens or receiving FXA credentials directly from the user.

The client constructs a login URL using a similar method to this:

https://github.com/mozilla-services/location-leaderboard/blob/master/leaderboard/fxa/client.py#L10-L31

(Note: this is used for debug purposes and is not the actual code used to build the login url in the stumbler client)

The leaderboard server requests the following permissions

- 'profile'
- 'leaderboard'

See here:

https://github.com/mozilla-services/location-leaderboard/blob/master/leaderboard/settings.py#L141

We use the profile:uid to identify the user and profile:displayName to set their name which will publicly appear on the website.  We could potentially request these two individually rather than in aggregate as 'profile' if that will simplify and/or improve security.  

We also request the custom 'leaderboard' permission so that an access token granted for the leaderboard is not valid to request profile data from another application.  Originally, I was going to validate that an access token passed by the client is authorized against the set of scopes we've defined, however I can not find an API endpoint which, given an access token, returns the list of scopes for which it is authorized.  Perhaps I misunderstood the proposal of how to ensure that an access token is valid for my application.

Requesting the 'profile' and 'leaderboard' permissions together requires that the client id in use by a leaderboard deployment be flagged as a 'trusted client' in the FXA database.  If we do not want to flag the leaderboard as a 'trusted client' then we will need to change the set of scopes that are being requested.

The android client goes to the FXA login pages, logs in directly, authorizes, gets redirected back to the leaderboard server which validates and exchanges the code for access and refresh tokens and then passes them back to the client.  Only the client stores the access and refresh tokens.

The redirect handler is here:

https://github.com/mozilla-services/location-leaderboard/blob/master/leaderboard/fxa/views.py#L51-L95

We return the access token and the refresh token to the client.  The server stores neither, it only stores the FXA uid.  When the client makes an authenticated call to the server, it passes the access token in through an auth header, which the server unpacks and validates against FXA here:

https://github.com/mozilla-services/location-leaderboard/blob/master/leaderboard/fxa/authenticator.py#L28-L72

It takes the uid that was returned by the FXA profile call and looks that up in the local user table.  If the FXA call is unsuccessful, we fail to identify and authenticate the user and the call fails.  At the same time, we attempt to unpack the 'displayName' field from the profile call and store it against our local user object.

The client exchanges its refresh token for a new access token at client init time, and this is facilitated by the server through the refresh endpoint here:

https://github.com/mozilla-services/location-leaderboard/blob/master/leaderboard/fxa/views.py#L101-L114

I believe that covers the significant portion of the FXA integration for the geolocation leaderboard.

You can test a working version of it by going here and logging in:

http://leaderboard-dev.jaredkerim.com/api/v1/fxa/login/

Please advise as to whether we should use the set of scopes described here and have the leaderboard client id flagged as 'trusted client', or if we should restrict the set of scopes to something allowed by an untrusted client.

Comment 1

[Ryan Kelly [:rfkelly]]

My initial comments from the previous bug, with some additions:

> I can not find an API endpoint which, given an access token,
> returns the list of scopes for which it is authorized. 

The `/v1/verify` endpoint returns the list of scopes associated with a token:

  https://github.com/mozilla/fxa-oauth-server/blob/master/docs/api.md#post-v1verify

I'm thinking about an addition to this API that lets you pass *in* the expected scopes, rather than having to go through the returned list of scopes and look for the ones you expect.

> We also request the custom 'leaderboard' permission so that an access token granted
> for the leaderboard is not valid to request profile data from another application.

OAuth scopes are additive, so IIUC this won't have the desired effect.  A token with scope "profile leaderboard" would be valid to access both the profile data and the leaderboard service.  So if some other application can e.g. steal the access token out of the client, then it could us it to read the user's FxA profile data.

It sounds like your ideal security situation would be for the client code to hold a token with only "leaderboard" scope..?

> When the client makes an authenticated call to the server, it passes the access token
> in through an auth header, which the server unpacks and validates against FXA here

So, it fetches the profile data and potentially updates the displayName on every request?  This seems nice for keeping the data up to date, but potentially expensive.  How frequently does the client make authenticated requests to this API?

Comment 2

[Jared Lockhart [:jlockhart]]

After discussing with Ryan Kelly, we came to a few proposed changes to the current FXA integration story for the Location Leaderboard.

When the client makes an authenticated call to the leaderboard, the leaderboard must do four things:

1) authenticate the user with fxa
2) identify the user with the leaderboard
3) validate that the user is allowed to interact with the leaderboard
4) update the local cache of the displayName

We were solving #1 and #2 by 

- receiving the access token from the client
- passing that to FXA through a /profile call which returns the users uid and displayName
- looking that user up in the leaderboard database through their FXA uid

The user is now authenticated (FXA has accepted the access token) and identified (we have the users uid which we can use to identify the user in our system)

This does not solve #3, as someone can use a valid FXA token granted by another service to identify and authenticate themselves to our system, at least partially.  Because we generate a row in our user table during the FXA oauth redirect flow, if someone did not walk through that flow with the leaderboard and later came and attempted to make an authenticated call with an access token granted by FXA via another relier, FXA would validly return their profile data (assuming that token was valid to query profile data) but we would fail to find a corresponding FXA uid in our database if they had not gone through the initial FXA oauth redirect flow with the leaderboard.  But we do not explicitly validate that an access token provided is granted specifically for use with the leaderboard, so we are accidentally solving #3 but not explicitly solving it, which is bad because if we change something about our database storage rules we might inadvertently break #3.

There are several ways to solve #3.

One is to pass the access token to the verify endpoint here:

https://github.com/mozilla/fxa-oauth-server/blob/master/docs/api.md#post-v1verify

Which will return the client id of the relier (the leaderboard in this case) as well as the scopes for which that access token is authorized.  This can be used to determine whether an access token presently valid with FXA, is granted for the leaderboard, and will also identify the user to the leaderboard.

So calling the /verify endpoint will accomplish #1 #2 #3, but not #4.  To keep the structure of the leaderboard server authenticated calls the same, we would also need to call the /profile endpoint to find the displayName and accomplish #4 during an authentication flow.

This leads us to the point where during an authentication flow, we make two outgoing calls to FXA, one to /verify to accomplish #1 #2 #3, and one to /profile to accomplish #4.

We folded #4 into the authentication flow because we can not make calls to FXA to query the user's profile data without a valid access token, and the server does not store the access token, only the client does.

Ryan Kelly suggested we grant a separate token which allows the leaderboard server to make authenticated calls on behalf of the user directly to the FXA profile server to query the displayName to satisfy #4, which would allow us to remove the requirement of #4 from the authentication flow, and so we could satisfy #1 #2 #3 using only the /verify endpoint.

However adding this additional token would require building additional overhead for granting and storing and managing this token as well as asynchronously scheduling batch jobs to use it to query the displayNames, which is a lot of additional code.

For the time being I think the simplest approach that requires no changes to the clients and the smallest change to the server is just to go with the approach that involves calling both the /profile and /verify endpoints at token authentication time during an authenticated call, then we satisfy all of #1 #2 #3 #4.  Yes we make two calls, but the client will only make authenticated calls during data submission time which happens not often, maybe once per day.

But this requires some further discussion and analysis to figure out a work flow which satisfies all of requirements, clearly distinguishes the roles and responsibilities of the client, the leaderboard server, and FXA.  We should schedule a post mortem once we've finished implementing a first pass on this to discuss the implementation, it's technical and security considerations.  Perhaps we could include Julien as well.

Comment 3

[Ryan Kelly [:rfkelly]]

> Only the client stores the access and refresh tokens.

I'd like to understand the security properties of this client-side token storage, :vng maybe you can comment?  My particular concern is whether other apps running on the device might be able to access them, and what powers that would grant such malicious apps.

Concretely, if the token has "profile" scope, then any other client-side code that's able to get the token is able to use it to read the user's profile data out of FxA.

> However adding this additional token would require building additional overhead for granting and storing
> and managing this token as well as asynchronously scheduling batch jobs to use it to query
> the displayNames

FWIW, you would not have to store this additional token anywhere, you could generate-use-and-discard it in a completely transient manner.

Comment 4

[Victor Ng [:vng] [:crankycoder]]

We store the token in the "Internal Storage" on the device - this data is accessible only to the application, unless the device is rooted.

I've filed a bug https://github.com/mozilla/MozStumbler/issues/1747 to disallow backups of the stumbler so that internal storage doesn't leak out.

The risk of another application reading that data is contained to rooted devices and some unforeseen security problem in Android's security sandboxing.

I'm fairly sure that this isn't a problem worth considering, but maybe we should have a security person chime in.

Comment 5

[Ryan Kelly [:rfkelly]]

Thanks Vic, this sounds OK to me, approximately equivalent to storing the tokens in localStorage in the browser (which we already do).

So concretely, the proposal is as follows:

* Request tokens with just the scopes you need, which IIUC is "profile:uid profile:display_name".  Don't request a custom "leaderboard" scope.  This helps limit any fallout from token leakage on the client.
* When you receive a token in the Authorization header, send it to the oauth-server's /v1/verify endpoint, and check that the returned client_id matches that of the leaderboard application.  This is your authorization check.
* Concurrently, and entirely at your option, use this token request the user's profile data from the profile-server and update your local cache of it.

It's a bit of an abuse of oauth, but it sounds like the simplest option given the current state of things with this project.

:jkerim can you confirm whether this matches your understanding of what we discussed?

Comment 6

[Jared Lockhart [:jlockhart]]

:rfkelly Yes, this matches my understanding of our discussion and I am in the process of implementing these changes now.  After we get all this launched and running we should sit down and evaluate what a more idiomatic solution would look like for future reference and future FXA relier integrations.

Comment 7

[Ryan Kelly [:rfkelly]]

OK, thanks guys, I'm going to close this out.