Conversation
LZoog
force-pushed
the
FXA-12939
branch
12 times, most recently
from
40c93ae to
17e4a70
Compare
| - **Option I: Use the session token with full scopes via `fxa-credentials` grant**: Reuse the existing custom grant type, passing a session token and the complete desired scope set with `access_type=offline` to obtain a new refresh token. This is already implemented in application-services (`create_refresh_token_using_session_token`). The client would then revoke the old refresh token and manage the device record. | ||
| - **Option II: RFC 8693 token exchange with full desired scope set**: Use the standard token exchange grant type, passing the existing refresh token and the complete desired scope set per RFC 8693 semantics | ||
| - **Option III: RFC 8693 token exchange with additional scopes only**: Use the standard token exchange grant type, passing the existing refresh token and only the new scopes needed; the server unions them with the original token's scopes | ||
| - **Option IV: Authorization code exchange with broader scopes**: Use the session token to silently obtain an authorization code with the full desired scope set (`authorize_code_using_session_token` in application-services), then exchange the code for a new refresh token (`create_refresh_token_using_authorization_code`). This is a two-step flow but does not require a browser redirect — both calls are server-to-server. |
There was a problem hiding this comment.
For reviewers of this PR / these ADRs: in the original authorization doc, we didn't list option IV. Mark noted this as an option late last last week when he saw these functions on the Rust side, and I wanted to add it here to consider. While technically feasible, we think the right approach is probably still the token exchange endpoint.
LZoog
changed the title
| ### Consent model | ||
|
|
||
| - **Option A: Implicit consent by granted tokens**: Consent is implicit in the existence of a refresh token with the relevant scopes. No persistent authorization record beyond the tokens themselves. | ||
| - **Option B: Explicit account-level authorization**: Consent is recorded at the account level in a new table in the FxA database. When signing in on a new device, FxA checks existing authorizations and issues tokens without re-prompting. |
There was a problem hiding this comment.
So in the Connected Services list we don't show it per-device, only per-service? Is device stored in the new table? Why not record device also and have this grant per-device instead of per-account?
There was a problem hiding this comment.
Refresh tokens are per-device, and those show up under connected services. We have this showing for Relay like so:
and we have tickets filed to allow revocation, with this work: https://mozilla-hub.atlassian.net/browse/FXA-12541
I have a different epic filed for displaying account-level auth in Settings if we want to: https://mozilla-hub.atlassian.net/browse/FXA-12940
Do we need/want to record devices in the account-level auth table?
There was a problem hiding this comment.
I mostly just wanted to make sure it was a deliberate choice. We'd want to record devices if there was a scenario where someone wanted to revoke auth only for that one device. Maybe if a device is stolen?
There was a problem hiding this comment.
Currently we record at least something when the user chooses to disconnect from Sync. Do you know if this records anything about the device or how we look at that data? I can check our code otherwise. I know Ross wasn't even aware of this until recently, but we could file a ticket in case we want to capture this info somewhere we aren't.
There was a problem hiding this comment.
|
|
||
| ### Use RFC 8693 token exchange with additional scopes only (Option III) | ||
|
|
||
| When a signed-in user needs an additional scope (e.g., a Sync user enabling Relay), the browser sends a token exchange request containing the existing refresh token as the `subject_token` and only the additional scopes needed in the `scope` parameter. The server unions the original token's scopes with the requested scopes to produce the new token's scope set, then revokes the original refresh token. |
There was a problem hiding this comment.
What does this look like in the Connected Services table before and after this exchange? Both exactly the same? I think so, but wanted to think through it.... What if Relay has multiple scopes and doesn't always request all of them? Do we have a way of showing that or is that unsupported functionality?
There was a problem hiding this comment.
Before the exchange, since we do not currently display Sync (but we would like to!), it looks like this:
Then after, the same screenshot as before:
What if Relay has multiple scopes and doesn't always request all of them? Do we have a way of showing that or is that unsupported functionality?
IIUC, this is a good argument for 0049, where they don't have to worry about that. FxA can see service=vpn, and grant the appropriate scopes, and then we can choose what we display in connected services (like if we want to display profile or not). We currently only check the "Relay" scope case.
There was a problem hiding this comment.
I can't think of a reason why Relay would want one scope and not all scopes. If a product grew enough that it wanted different scopes I guess it would probably be a different service. If a product runs into this situation I think we can deal with it and call my scenario unsupported for now.
|
|
||
| Chosen option: "Option 3 -- Server resolves scopes from service", because it eliminates client-server coupling for product decisions, avoids the scoped keys problem of option 2, and aligns with FxA already being the authority on scope-to-service mappings. | ||
|
|
||
| When `service` is specified, FxA resolves the applicable scopes server-side — `service` is a more concise way of specifying scopes, and the `scope` parameter can be dropped. When `service` is not specified, `scope` continues to work as it does today. |
There was a problem hiding this comment.
Pedantic, but:
- What if they are both specified? and if they do/don't overlap (e.g. a scope for a different service?)
- What if one is invalid and one isn't?
There was a problem hiding this comment.
What if they are both specified? and if they do/don't overlap (e.g. a scope for a different service?)
If they are both specified, it'll behave the way it does now, where only the requested scope can be granted (unless it's Desktop using the session token, which can just get any scope it still wants). Good call out though, the "When service is not specified, scope continues to work as it does today." bit was meant to refer to normal RP redirects, and it should call that case out too.
What if one is invalid and one isn't?
I can make a note about this too. In this case, if one of those requested scopes is not in the alllowlist, then it will error out with "invalid scope" as it does today. If in the future we allow the user to deny certain scopes that are being requested but are in the allowlist, then the refresh token the auth server grants will only contain that subset of scopes, and Firefox will know what scopes were granted by us sending them in a new scopes option in the oauth web channel message.
|
|
||
| ### Option 3: Server resolves scopes from service | ||
|
|
||
| Firefox sends only the `service` parameter instead of `scope`. FxA resolves the required and optional scopes, renders the consent page, and grants only what the user approves. |
There was a problem hiding this comment.
Are we restricting (and verifying) that only Firefox can use this service parameter?
There was a problem hiding this comment.
Yes, that will be part of https://mozilla-hub.atlassian.net/browse/FXA-13495
|
|
||
| - Good, because product scope changes are server-side configuration, no client release needed | ||
| - Good, because the server already maintains these mappings and is the enforcement point for grants | ||
| - Good, because it only requires a password entry when the Sync scope is actually needed |
There was a problem hiding this comment.
When a new scope is added (re: ADR 0048) is that automatically granted? Or the RP will have to do a token exchange w/ a new consent screen?
There was a problem hiding this comment.
It's only automatically granted if the user has previously authorized that scope under that "project."
So if it's the user's first time ever using VPN, the browser tries to do the token exchange, and we reject it. In response, they take the user to FxA to authorize the scope. When the scope is authorized, we update our table. Then, the browser tries again to do the token exchange and it gets granted.
If the user then tries to use VPN on another device, then the first time the browser tries to do the token exchange, we grant it, and then the user doesn't have to be taken back to FxA to authorize it.
| - FxA already maintains client registrations with `allowedScopes`, scope-to-key mappings, and consent logic server-side | ||
| - Product requirements change (e.g., "offer Monitor during sign-up") and should not require Firefox desktop release cycles | ||
| - RFC 9700 Section 2.3 (OAuth Security Best Current Practice) recommends least-privilege tokens but focuses on what is _granted_, not what is _requested_ -- the server is the enforcement point | ||
| - Firefox is a trusted first-party client, not a third-party app -- the traditional OAuth scope request model was designed for untrusted clients |
There was a problem hiding this comment.
This line makes me hesitate a bit. I feel like this kind of thing can lead to cut corners later and reviewing the doc I realized we have a problem statement and these decision drivers but not really a goal. It sounds like we're optimizing for fewer required client-releases and simplifying the code in the client. I'm not saying that's wrong, but I don't see that stated here and without it it makes me wonder why we're treating Firefox as a special case.
There was a problem hiding this comment.
If other RPs need additional scopes, they can just update it in their URL for login/signup, or take users back to FxA with the additional scope requested if they're already signed in (though, see this Slack message).
Firefox is the special case because it's the only RP where Product can decide what (browser) services to offer during the flows, which doesn't feel ideal to tie to a long Firefox release cycle. Maybe this needs to be rephrased slightly, and I can add this (and yes, a note about simplification) as an explicit goal statement to the ADR.
| - Good, because the server already maintains these mappings and is the enforcement point for grants | ||
| - Good, because it only requires a password entry when the Sync scope is actually needed | ||
| - Good, because it aligns with RFC 9700 Section 2.3 which places privilege restriction responsibility on the authorization server | ||
| - Neutral, because the client loses granular control over scopes -- if two features both use the same `service` value but need different scopes, the client has no way to express that distinction, but this is mitigated by using different `service` values or by varying scope resolution server-side based on other flow context like `entrypoint` |
There was a problem hiding this comment.
I probably missed it but this is the first time I'm seeing mention of other context determining scopes. That feels important to specify here as that is essentially a contract we're making with our API consumer and if we're going off "standard" oauth we need to be really specific. What other contexts hints are we taking into account? What if they clash? Do we log the service or the scopes we determined? Do we log the context(s) we took into account when determining the scopes?
There was a problem hiding this comment.
I added this point after thinking about if, for example, we later want a different kind of relay integration that requires or can offer different scopes than the existing service=relay flow. Perhaps entrypoint complicates this too much, and if this happens, we would just require that they use a different service, or possibly specify the scope, which would take precedence.
There was a problem hiding this comment.
I think calling that a different service let's us stop thinking about a lot of edge cases here. If we want to use other contexts I think we need to get really specific as we're essentially designing a contract here.
| ### Consent model | ||
|
|
||
| - **Option A: Implicit consent by granted tokens**: Consent is implicit in the existence of a refresh token with the relevant scopes. No persistent authorization record beyond the tokens themselves. | ||
| - **Option B: Explicit account-level authorization**: Consent is recorded at the account level in a new table in the FxA database. When signing in on a new device, FxA checks existing authorizations and issues tokens without re-prompting. |
There was a problem hiding this comment.
I mostly just wanted to make sure it was a deliberate choice. We'd want to record devices if there was a scenario where someone wanted to revoke auth only for that one device. Maybe if a device is stolen?
|
|
||
| ### Use RFC 8693 token exchange with additional scopes only (Option III) | ||
|
|
||
| When a signed-in user needs an additional scope (e.g., a Sync user enabling Relay), the browser sends a token exchange request containing the existing refresh token as the `subject_token` and only the additional scopes needed in the `scope` parameter. The server unions the original token's scopes with the requested scopes to produce the new token's scope set, then revokes the original refresh token. |
There was a problem hiding this comment.
I can't think of a reason why Relay would want one scope and not all scopes. If a product grew enough that it wanted different scopes I guess it would probably be a different service. If a product runs into this situation I think we can deal with it and call my scenario unsupported for now.
| - Good, because the server already maintains these mappings and is the enforcement point for grants | ||
| - Good, because it only requires a password entry when the Sync scope is actually needed | ||
| - Good, because it aligns with RFC 9700 Section 2.3 which places privilege restriction responsibility on the authorization server | ||
| - Neutral, because the client loses granular control over scopes -- if two features both use the same `service` value but need different scopes, the client has no way to express that distinction, but this is mitigated by using different `service` values or by varying scope resolution server-side based on other flow context like `entrypoint` |
There was a problem hiding this comment.
I think calling that a different service let's us stop thinking about a lot of edge cases here. If we want to use other contexts I think we need to get really specific as we're essentially designing a contract here.
|
|
||
| Currently, all Firefox browser services (Relay, Smart Window, Sync) share Firefox's single OAuth client ID. A possible future direction is to assign each service its own client ID per platform (e.g., Relay-in-Desktop, Relay-in-iOS, Relay-in-Android). This maps directly to [Google's Cross-client Identity](https://developers.google.com/identity/protocols/oauth2/cross-client-identity) model, where multiple client IDs are grouped under a single project umbrella and share authorization at the account level. This would make browser service metrics consistent with how FxA already tracks third-party RPs by `client_id`, and scope allowlists could be managed per `client_id` using standard OAuth client registration rather than the novel `service`-based resolution described in ADR 0049. However, it's not necessarily better OAuth practice overall — Firefox is the actual client receiving the tokens, not the individual services. | ||
|
|
||
| The decisions in this ADR are compatible with that migration. Account-level authorization would function like Google's project umbrella — applying across client IDs for the same service. The token exchange mechanism and silent grant migration path would not need to change. See also ADR 0049, which notes this as a decision driver for server-side scope resolution. |
There was a problem hiding this comment.
This seems like the best long term solution
There was a problem hiding this comment.
In the authorization doc, I know Barry and Wil said this seemed best from their perspective as well. While in hindsight I agree it would have been best to start this way, I'm a little torn now given the level of effort, backwards compatibility support needed, and that it's not clear it's bad practice the way we're doing it now, but given your thoughts too, I'll file an issue for this. It's probably not something we'll tackle until later this year at the earliest, unless it makes sense to switch over when desktop moves away from a session token.
…ution Because: * Decisions around refresh tokens and account-level authorization for browser services were made but not formally documented as an ADR. * Server-side scope resolution would decouple Firefox from knowing which scopes to request, allowing product changes without client releases and eliminating the problem of Firefox either over-requesting scopes (triggering the scoped keys password path) or under-requesting them (missing scopes FxA wants to offer), and leveraging the scope-to-service mapping FxA already maintains for consent rendering and token granting This commit: * Adds ADR 0048 documenting accepted decisions for refresh tokens, silent grant migration, explicit account-level consent, and the RFC 8693 token exchange mechanism. * Adds ADR 0049 proposing server-side scope resolution via the service parameter for Firefox OAuth flows closes FXA-13492
3 participants
I've been working with Mark on FXA-12939 and we came to an agreement on what we've got proposed here in ADR 0049. This WIP branch needs some edits if we'll implement that here too.
For 0048, I'm recording decisions already decided on in a previous doc linked, because they relate to 0049 and they weren't captured in an ADR.
Edit: I've pulled out the code changes for this, so that this PR only holds the ADRs.
closes FXA-13492