Skip to content

Commit 6ec943b

Browse files
committed
feat(signin): Add conditional keys fetch, scopes in fxaOAuthLogin, "Authorization" in Strapi
Because: * Non-Sync browser services (VPN, Relay, SmartWindow) should not force password entry just to fetch keys, but should fetch them opportunistically if a password is entered for another reason * The browser needs to know which scopes were granted after the authorization flow completes * The isSignedIntoFirefoxDesktop state was too narrow for the scope authorization flow which applies to all Firefox platforms, and we want this editable in Strapi This commit: * Splits wantsKeys into requiresKeys (Sync only, forces password) and wantsKeysIfPasswordEntered (non-Sync, opportunistic), with wantsKeys, to allow a "cached login" render without the "keys optional" capability, which is a capability intended for passwordless non-sync browser logins * Adds scopes field to fxaOAuthLogin WebChannel message at all call sites, because the browser needs to know which scopes were actually granted — with ADR 0049, FxA may deny requested scopes or grant additional ones, and the browser may not request scope at all * Renames isSignedIntoFirefoxDesktop to isSignedIntoFirefox and removes the Desktop-only check, adds a new page in Strapi for this state so the copy can be updated (e.g. "Authorize") closes FXA-12939
1 parent 8aea17f commit 6ec943b

34 files changed

Lines changed: 446 additions & 118 deletions

File tree

libs/shared/cms/src/__generated__/gql.ts

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

libs/shared/cms/src/__generated__/graphql.ts

Lines changed: 68 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

libs/shared/cms/src/lib/queries/relying-party/factories.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,13 @@ export const RelyingPartyResultFactory = (
175175
pageTitle: faker.string.sample(),
176176
splitLayout: faker.datatype.boolean(),
177177
},
178+
AuthorizePage: {
179+
headline: faker.string.sample(),
180+
description: faker.string.sample(),
181+
primaryButtonText: faker.string.sample(),
182+
pageTitle: faker.string.sample(),
183+
splitLayout: faker.datatype.boolean(),
184+
},
178185
SigninPasswordlessCodePage: {
179186
logoUrl: faker.internet.url(),
180187
logoAltText: faker.internet.url(),

libs/shared/cms/src/lib/queries/relying-party/query.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,13 @@ export const relyingPartyQuery = graphql(`
112112
pageTitle
113113
splitLayout
114114
}
115+
AuthorizePage {
116+
headline
117+
description
118+
primaryButtonText
119+
pageTitle
120+
splitLayout
121+
}
115122
SigninPasswordlessCodePage {
116123
headline
117124
description

libs/shared/cms/src/lib/queries/relying-party/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ export interface RelyingPartyResult {
7373
SignupConfirmedSyncPage: Page;
7474
SigninPage: Page;
7575
SigninCachedPage?: Page;
76+
AuthorizePage?: Page;
7677
SigninTokenCodePage?: Page;
7778
SigninUnblockCodePage?: Page;
7879
SigninTotpCodePage?: Page;

packages/fxa-settings/src/components/App/index.tsx

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,10 @@ export const App = ({
219219
// Determine if user is actually signed in
220220
const [isSignedIn, setIsSignedIn] = useState<boolean | undefined>(undefined);
221221

222-
// Track whether the user is signed into Firefox Desktop via WebChannel
223-
const [isSignedIntoFirefoxDesktop, setIsSignedIntoFirefoxDesktop] =
224-
useState(false);
222+
// Determine whether the user is signed into Firefox via WebChannel.
223+
// This also tells us if the user is in a Firefox "authorization" flow, where
224+
// they are already signed in but need to consent to a new scope.
225+
const [isSignedIntoFirefox, setIsSignedIntoFirefox] = useState(false);
225226

226227
// Track current page's split layout state to prevent visual flashing during navigation.
227228
// This state is updated by AppLayout and read by the Suspense fallback to preserve
@@ -256,10 +257,7 @@ export const App = ({
256257
userFromBrowser.sessionToken
257258
);
258259
if (isValidSession) {
259-
setIsSignedIntoFirefoxDesktop(
260-
!!userFromBrowser?.sessionToken &&
261-
integration.isFirefoxDesktopClient()
262-
);
260+
setIsSignedIntoFirefox(true);
263261
const cachedUser = getAccountByUid(userFromBrowser.uid);
264262
// Refresh the token without switching the "current" account.
265263
persistAccount(
@@ -421,7 +419,7 @@ export const App = ({
421419
isSignedIn,
422420
integration,
423421
flowQueryParams: updatedFlowQueryParams,
424-
isSignedIntoFirefoxDesktop,
422+
isSignedIntoFirefox,
425423
setCurrentSplitLayout,
426424
}}
427425
path="/*"
@@ -496,13 +494,13 @@ const AuthAndAccountSetupRoutes = ({
496494
isSignedIn,
497495
integration,
498496
flowQueryParams,
499-
isSignedIntoFirefoxDesktop,
497+
isSignedIntoFirefox,
500498
setCurrentSplitLayout,
501499
}: {
502500
isSignedIn: boolean;
503501
integration: Integration;
504502
flowQueryParams: QueryParams;
505-
isSignedIntoFirefoxDesktop: boolean;
503+
isSignedIntoFirefox: boolean;
506504
setCurrentSplitLayout: (value: boolean) => void;
507505
} & RouteComponentProps) => {
508506
const localAccount = currentAccount();
@@ -639,7 +637,7 @@ const AuthAndAccountSetupRoutes = ({
639637
serviceName,
640638
flowQueryParams,
641639
useFxAStatusResult,
642-
isSignedIntoFirefoxDesktop,
640+
isSignedIntoFirefox,
643641
setCurrentSplitLayout,
644642
}}
645643
/>
@@ -650,7 +648,7 @@ const AuthAndAccountSetupRoutes = ({
650648
serviceName,
651649
flowQueryParams,
652650
useFxAStatusResult,
653-
isSignedIntoFirefoxDesktop,
651+
isSignedIntoFirefox,
654652
setCurrentSplitLayout,
655653
}}
656654
/>
@@ -679,7 +677,7 @@ const AuthAndAccountSetupRoutes = ({
679677
serviceName,
680678
flowQueryParams,
681679
useFxAStatusResult,
682-
isSignedIntoFirefoxDesktop,
680+
isSignedIntoFirefox,
683681
setCurrentSplitLayout,
684682
}}
685683
/>
@@ -690,7 +688,7 @@ const AuthAndAccountSetupRoutes = ({
690688
serviceName,
691689
flowQueryParams,
692690
useFxAStatusResult,
693-
isSignedIntoFirefoxDesktop,
691+
isSignedIntoFirefox,
694692
setCurrentSplitLayout,
695693
}}
696694
/>

packages/fxa-settings/src/lib/channels/firefox.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@ export type FxAOAuthLogin = {
146146
// eventually move to look at fxaLogin as well to prevent FXA-10596.
147147
declinedSyncEngines?: string[];
148148
offeredSyncEngines?: string[];
149+
// Space-separated list of granted scopes, sent so the browser knows
150+
// which scopes were authorized in this flow.
151+
scopes?: string;
149152
};
150153

151154
// ref: https://searchfox.org/mozilla-central/rev/82828dba9e290914eddd294a0871533875b3a0b5/services/fxaccounts/FxAccountsWebChannel.sys.mjs#230

packages/fxa-settings/src/lib/oauth/hooks.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,15 +305,15 @@ export function useFinishOAuthFlowHandler(
305305
* to /signin instead of showing an error component? FXA-10889
306306
*/
307307
export function useOAuthKeysCheck(
308-
integration: Pick<OAuthIntegration, 'type' | 'wantsKeys'>,
308+
integration: Pick<OAuthIntegration, 'type' | 'requiresKeys'>,
309309
keyFetchToken?: hexstring,
310310
unwrapBKey?: hexstring,
311311
isSignInWithThirdPartyAuth?: boolean
312312
) {
313313
if (
314314
(isOAuthIntegration(integration) ||
315315
isSyncDesktopV3Integration(integration)) &&
316-
integration.wantsKeys() &&
316+
integration.requiresKeys() &&
317317
// If the user has 2FA enabled but chose to login to the browser via third party
318318
// auth, keys are not fetched because the user didn't enter a password.
319319
// For this case, skip the keys check, the browser expects them to be undefined.

packages/fxa-settings/src/models/integrations/data/data.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@ import {
2222
KeyTransforms as T,
2323
ModelDataProvider,
2424
} from '../../../lib/model-data';
25-
import { IsEmailOrEmpty, IsFxaRedirectToUrl, IsFxaRedirectUri } from '../../../lib/validation';
25+
import {
26+
IsEmailOrEmpty,
27+
IsFxaRedirectToUrl,
28+
IsFxaRedirectUri,
29+
} from '../../../lib/validation';
2630

2731
/**
2832
* Base integration class. Fields in this class represents data commonly accessed across many pages and is useful for various flows.

packages/fxa-settings/src/models/integrations/integration.ts

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,10 @@ export class GenericIntegration<
106106
return undefined;
107107
}
108108

109+
isFirefoxClient() {
110+
return this.isFirefoxDesktopClient() || this.isFirefoxMobileClient();
111+
}
112+
109113
isFirefoxMobileClient() {
110114
return false;
111115
}
@@ -151,23 +155,50 @@ export class GenericIntegration<
151155
}
152156

153157
/**
154-
* Currently only Sync _requires_ keys (entering a password). Other
155-
* services may see cached signin or choose to sign in with third party auth.
156-
* However, for non-Sync browser service signins, if the password is entered,
157-
* we go ahead and get Sync keys so users can turn Sync on in the browser
158-
* without being bounced back to FxA.
158+
* Whether this integration strictly requires keys, forcing password entry.
159+
* Currently only Sync requires keys.
159160
*
160161
* Note, the Relay browser service login launched in Firefox desktop 135, and
161162
* the "keys optional" capability launched in Fx desktop 147, meaning all Relay
162163
* service users in those Fx versions require a password.
163164
*
164165
* Desktop OAuth launched with Fx 134. SyncDesktopV3 users don't have non-Sync
165166
* browser support.
166-
* */
167-
wantsKeys(): boolean {
167+
*/
168+
requiresKeys(): boolean {
169+
return false;
170+
}
171+
172+
/**
173+
* Whether this integration wants keys opportunistically — if the user
174+
* enters a password for another reason, we should also fetch keys.
175+
* This applies to non-Sync browser services (Relay, VPN, SmartWindow) that
176+
* request the Sync scope so users can turn Sync on in the browser without
177+
* being bounced back to FxA.
178+
*/
179+
wantsKeysIfPasswordEntered(): boolean {
168180
return false;
169181
}
170182

183+
/**
184+
* Combined check: whether this integration wants keys in any scenario.
185+
* Use `requiresKeys()` when you need to know if keys are mandatory (e.g.,
186+
* deciding whether to force a password). Use `wantsKeys()` when you need
187+
* to know if keys should be fetched when available (e.g., setting the
188+
* `keys` param on the auth server request).
189+
*/
190+
wantsKeys(): boolean {
191+
return this.requiresKeys() || this.wantsKeysIfPasswordEntered();
192+
}
193+
194+
/**
195+
* Returns the scopes that were granted for this integration.
196+
* Overridden in OAuthNativeIntegration; returns undefined for all other types.
197+
*/
198+
getGrantedScopes(): string | undefined {
199+
return undefined;
200+
}
201+
171202
wantsLogin(): boolean {
172203
return false;
173204
}

0 commit comments

Comments
 (0)