diff --git a/extension/js/common/api/authentication/configured-idp-oauth.ts b/extension/js/common/api/authentication/configured-idp-oauth.ts index 0913d2e5417..c6df3a58051 100644 --- a/extension/js/common/api/authentication/configured-idp-oauth.ts +++ b/extension/js/common/api/authentication/configured-idp-oauth.ts @@ -140,7 +140,7 @@ export class ConfiguredIdpOAuth extends OAuth { if (!redirectUri) { return { acctEmail, result: 'Denied', error: 'Invalid response url', id_token: undefined }; } - const uncheckedUrlParams = Url.parse(['scope', 'code', 'state'], redirectUri); + const uncheckedUrlParams = Url.parse(['code', 'state'], redirectUri); const code = Assert.urlParamRequire.optionalString(uncheckedUrlParams, 'code'); const receivedState = Assert.urlParamRequire.string(uncheckedUrlParams, 'state'); if (!code) { diff --git a/extension/js/common/api/authentication/generic/oauth.ts b/extension/js/common/api/authentication/generic/oauth.ts index 572567a406d..7d7e1bc4895 100644 --- a/extension/js/common/api/authentication/generic/oauth.ts +++ b/extension/js/common/api/authentication/generic/oauth.ts @@ -29,6 +29,7 @@ export type OAuthTokensResponse = { refresh_token?: string; id_token: string; token_type: 'Bearer'; + scope?: string; }; /* eslint-enable @typescript-eslint/naming-convention */ diff --git a/extension/js/common/api/authentication/google/google-oauth.ts b/extension/js/common/api/authentication/google/google-oauth.ts index 19d062d8d77..c3ce7ee4bff 100644 --- a/extension/js/common/api/authentication/google/google-oauth.ts +++ b/extension/js/common/api/authentication/google/google-oauth.ts @@ -211,21 +211,9 @@ export class GoogleOAuth extends OAuth { } return { acctEmail, result: 'Denied', error: `Failed to launch web auth flow: ${errorMsg}`, id_token: undefined }; } - const uncheckedUrlParams = Url.parse(['scope', 'code', 'state'], redirectUri); - const allowedScopes = Assert.urlParamRequire.string(uncheckedUrlParams, 'scope'); + const uncheckedUrlParams = Url.parse(['code', 'state'], redirectUri); const code = Assert.urlParamRequire.optionalString(uncheckedUrlParams, 'code'); const receivedState = Assert.urlParamRequire.string(uncheckedUrlParams, 'state'); - const scopesToCheck = [ - this.GOOGLE_OAUTH_CONFIG.scopes.compose, - this.GOOGLE_OAUTH_CONFIG.scopes.modify, - this.GOOGLE_OAUTH_CONFIG.scopes.readContacts, - this.GOOGLE_OAUTH_CONFIG.scopes.readOtherContacts, - ]; - for (const scopeToCheck of scopesToCheck) { - if (requestedScopes.includes(scopeToCheck) && !allowedScopes?.includes(scopeToCheck)) { - return { acctEmail, result: 'Denied', error: 'Missing permissions', id_token: undefined }; - } - } if (!code) { return { acctEmail, @@ -237,12 +225,29 @@ export class GoogleOAuth extends OAuth { if (receivedState !== expectedState) { return { acctEmail, result: 'Error', error: `Wrong oauth CSRF token. Please try again.`, id_token: undefined }; } - const { id_token } = save ? await GoogleOAuth.retrieveAndSaveAuthToken(code) : await GoogleOAuth.googleAuthGetTokens(code); - const { email } = GoogleOAuth.parseIdToken(id_token); + const tokensObj = await GoogleOAuth.googleAuthGetTokens(code); + const grantedScopes = tokensObj.scope + ? tokensObj.scope.split(/\s+/).filter(Boolean) + : (await GoogleOAuth.getTokenInfo(tokensObj.access_token)).scope.split(/\s+/).filter(Boolean); + const scopesToCheck = [ + this.GOOGLE_OAUTH_CONFIG.scopes.compose, + this.GOOGLE_OAUTH_CONFIG.scopes.modify, + this.GOOGLE_OAUTH_CONFIG.scopes.readContacts, + this.GOOGLE_OAUTH_CONFIG.scopes.readOtherContacts, + ]; + for (const scopeToCheck of scopesToCheck) { + if (requestedScopes.includes(scopeToCheck) && !grantedScopes.includes(scopeToCheck)) { + return { acctEmail, result: 'Denied', error: 'Missing permissions', id_token: undefined }; + } + } + const { email } = GoogleOAuth.parseIdToken(tokensObj.id_token); if (!email) { throw new Error('Missing email address in id_token'); } - return { acctEmail: email, result: 'Success', id_token }; + if (save) { + await GoogleOAuth.googleAuthSaveTokens(email, tokensObj); + } + return { acctEmail: email, result: 'Success', id_token: tokensObj.id_token }; } catch (err) { if (err instanceof AssertError) { return { acctEmail, result: 'Error', error: 'Could not parse URL returned from Google', id_token: undefined }; @@ -302,17 +307,6 @@ export class GoogleOAuth extends OAuth { ); } - // eslint-disable-next-line @typescript-eslint/naming-convention - private static async retrieveAndSaveAuthToken(authCode: string): Promise<{ id_token: string }> { - const tokensObj = await GoogleOAuth.googleAuthGetTokens(authCode); - const claims = GoogleOAuth.parseIdToken(tokensObj.id_token); - if (!claims.email) { - throw new Error('Missing email address in id_token'); - } - await GoogleOAuth.googleAuthSaveTokens(claims.email, tokensObj); - return { id_token: tokensObj.id_token }; // eslint-disable-line @typescript-eslint/naming-convention - } - private static apiGoogleAuthPopupPrepareAuthReqScopes(addScopes: string[]): string[] { if (!addScopes.includes(this.GOOGLE_OAUTH_CONFIG.scopes.email)) { addScopes.push(this.GOOGLE_OAUTH_CONFIG.scopes.email); diff --git a/extension/js/common/platform/store/acct-store.ts b/extension/js/common/platform/store/acct-store.ts index 668f8ba5e21..77beed1a582 100644 --- a/extension/js/common/platform/store/acct-store.ts +++ b/extension/js/common/platform/store/acct-store.ts @@ -145,7 +145,7 @@ export class AcctStore extends AbstractStore { let allowedScopes: string[] = []; try { const { scope } = await GoogleOAuth.getTokenInfo(accessToken); - allowedScopes = scope.split(' '); + allowedScopes = scope.split(/\s+/).filter(Boolean); } catch (e) { if (ApiErr.isAuthErr(e)) { BrowserMsg.send.notificationShowAuthPopupNeeded('broadcast', { acctEmail }); diff --git a/test/source/mock/google/google-endpoints.ts b/test/source/mock/google/google-endpoints.ts index 9493ba3b3ae..6d1f2fdd762 100644 --- a/test/source/mock/google/google-endpoints.ts +++ b/test/source/mock/google/google-endpoints.ts @@ -122,7 +122,7 @@ export const getMockGoogleEndpoints = (oauth: OauthMock, config: GoogleConfig | return { '/o/oauth2/auth': async ( // eslint-disable-next-line @typescript-eslint/naming-convention - { query: { client_id, response_type, access_type, state, scope, login_hint, proceed, redirect_uri } }, + { query: { client_id, response_type, access_type, state, scope, login_hint, proceed, redirect_uri, redirect_scope } }, req ) => { if (isGet(req) && response_type === 'code' && access_type === 'offline' && state && scope) { @@ -133,13 +133,13 @@ export const getMockGoogleEndpoints = (oauth: OauthMock, config: GoogleConfig | } else if (!proceed) { return oauth.renderText('redirect with proceed=true to continue'); } else { - return oauth.successResult(login_hint, state, scope, redirect_uri); + return oauth.successResult(login_hint, state, scope, redirect_uri, redirect_scope); } } else if (client_id === OauthMock.customIDPClientId) { if (!proceed) { return oauth.renderText('redirect with proceed=true to continue'); } - return oauth.successResult(login_hint, state, scope, redirect_uri); + return oauth.successResult(login_hint, state, scope, redirect_uri, redirect_scope); } } throw new HttpClientErr(`Method not implemented for ${req.url}: ${req.method}`); diff --git a/test/source/mock/lib/oauth.ts b/test/source/mock/lib/oauth.ts index 9430c89a41b..16149c12470 100644 --- a/test/source/mock/lib/oauth.ts +++ b/test/source/mock/lib/oauth.ts @@ -37,7 +37,7 @@ export class OauthMock { }; // eslint-disable-next-line @typescript-eslint/naming-convention - public successResult = (acct: string, state: string, scope: string, redirect_uri: string) => { + public successResult = (acct: string, state: string, scope: string, redirect_uri: string, redirect_scope?: string) => { const authCode = `mock-auth-code-${Str.sloppyRandom(4)}-${acct.replace(/[^a-z0-9]+/g, '')}`; const refreshToken = `mock-refresh-token-${Str.sloppyRandom(4)}-${acct.replace(/[^a-z0-9]+/g, '')}`; const accessToken = `mock-access-token-${Str.sloppyRandom(4)}-${acct.replace(/[^a-z0-9]+/g, '')}`; @@ -48,7 +48,13 @@ export class OauthMock { this.scopesByAccessToken[accessToken] = `${this.scopesByAccessToken[accessToken] ?? ''} ${scope}`; const url = new URL(redirect_uri); url.searchParams.set('code', authCode); - url.searchParams.set('scope', scope); + if (redirect_scope === '') { + url.searchParams.delete('scope'); + } else if (typeof redirect_scope !== 'undefined') { + url.searchParams.set('scope', redirect_scope); + } else { + url.searchParams.set('scope', scope); + } // return invalid state for test.invalid.csrf@gmail.com to check invalid csrf login url.searchParams.set('state', acct === 'test.invalid.csrf@gmail.com' ? 'invalid state' : state); return url.href; diff --git a/test/source/tests/page-recipe/oauth-page-recipe.ts b/test/source/tests/page-recipe/oauth-page-recipe.ts index 7bf6d2f8d6b..9930adccf8e 100644 --- a/test/source/tests/page-recipe/oauth-page-recipe.ts +++ b/test/source/tests/page-recipe/oauth-page-recipe.ts @@ -14,7 +14,7 @@ export class OauthPageRecipe extends PageRecipe { t: AvaContext, oauthPage: ControllablePage, acctEmail: string, - action: 'close' | 'deny' | 'approve' | 'login' | 'login_with_invalid_state' | 'override_acct' | 'missing_permission' + action: 'close' | 'deny' | 'approve' | 'login' | 'login_with_invalid_state' | 'override_acct' | 'missing_permission' | 'missing_redirect_scope' ): Promise => { let mockOauthUrl = oauthPage.target.url(); const { login_hint } = Url.parse(['login_hint'], mockOauthUrl); // eslint-disable-line @typescript-eslint/naming-convention @@ -30,6 +30,9 @@ export class OauthPageRecipe extends PageRecipe { mockOauthUrl = Url.removeParamsFromUrl(mockOauthUrl, ['scope']); mockOauthUrl += '&scope=missing_scope'; await OauthPageRecipe.safeGoto(oauthPage, mockOauthUrl + '&proceed=true'); + } else if (action === 'missing_redirect_scope') { + mockOauthUrl += '&redirect_scope=missing_scope'; + await OauthPageRecipe.safeGoto(oauthPage, mockOauthUrl + '&proceed=true'); } else if (!login_hint) { await OauthPageRecipe.safeGoto(oauthPage, mockOauthUrl + '&login_hint=' + encodeURIComponent(acctEmail) + '&proceed=true'); } else { diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 69e033726a8..790bd826493 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -734,6 +734,32 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== }) ); + test( + 'test re-auth after updating chrome extension - redirect scope omitted', + testWithBrowser(async (t, browser) => { + const acctEmail = 'flowcrypt.compatibility@gmail.com'; + await BrowserRecipe.setupCommonAcctWithAttester(t, browser, 'compatibility'); + // Wipe google tokens to test re-auth popup + await Util.wipeGoogleTokensUsingExperimentalSettingsPage(t, browser, acctEmail); + const gmailPage1 = await openMockGmailPage(t, browser, acctEmail); + await gmailPage1.waitAndClick('@action-secure-compose'); + // Check reconnect auth notification + await gmailPage1.waitForContent('@webmail-notification-setup', 'Please reconnect FlowCrypt to your Gmail Account.'); + const oauthPopup = await browser.newPageTriggeredBy(t, () => gmailPage1.waitAndClick('@action-reconnect-account')); + // mock api returns full token scopes but omits scope from the redirect URL (Firefox loopback style) + await OauthPageRecipe.mock(t, oauthPopup, acctEmail, 'missing_redirect_scope'); + // connection should succeed without a missing-permissions prompt + await gmailPage1.waitForContent('@webmail-notification-setup', 'Connected successfully. You may need to reload the tab.'); + await gmailPage1.close(); + // reload and test that it has no more notifications + const gmailPage2 = await openMockGmailPage(t, browser, acctEmail); + await gmailPage2.waitAndClick('@action-secure-compose'); + await Util.sleep(2); + await gmailPage2.notPresent(['@webmail-notification-setup']); + await gmailPage2.close(); + }) + ); + test( 'mail.google.com - success notif after setup, click hides it, does not re-appear + offers to reauth', testWithBrowser(async (t, browser) => {