From da97cea4f0944b3d74ec5f47ce302ba62d6d001d Mon Sep 17 00:00:00 2001 From: Ioan Moldovan Date: Wed, 24 Jul 2024 09:51:49 +0300 Subject: [PATCH 1/9] WIP: add custom idp implementation --- .../authentication/configured-idp-oauth.ts | 172 ++++++++++++++++-- .../api/authentication/generic/oauth.ts | 36 ++++ .../api/authentication/google/google-oauth.ts | 44 +---- extension/js/common/browser/browser-msg.ts | 2 +- extension/js/common/core/const.ts | 1 + extension/js/common/settings.ts | 2 - 6 files changed, 205 insertions(+), 52 deletions(-) diff --git a/extension/js/common/api/authentication/configured-idp-oauth.ts b/extension/js/common/api/authentication/configured-idp-oauth.ts index 6364ffb46dd..f1ce8192e04 100644 --- a/extension/js/common/api/authentication/configured-idp-oauth.ts +++ b/extension/js/common/api/authentication/configured-idp-oauth.ts @@ -2,21 +2,165 @@ 'use strict'; -import { GoogleOAuth } from './google/google-oauth.js'; import { Ui } from '../../browser/ui.js'; -import { AcctStore } from '../../platform/store/acct-store.js'; -import { OAuth } from './generic/oauth.js'; - +// import { AcctStore } from '../../platform/store/acct-store.js'; +import { AuthRes, OAuth, OAuthTokensResponse } from './generic/oauth.js'; +import { AuthenticationConfiguration } from '../../authentication-configuration.js'; +import { Url } from '../../core/common.js'; +import { OAuth2 } from '../../oauth2/oauth2.js'; +import { Bm } from '../../browser/browser-msg.js'; +import { Assert, AssertError } from '../../assert.js'; +import { Api } from '../shared/api.js'; +import { Catch } from '../../platform/catch.js'; +import { InMemoryStoreKeys } from '../../core/const.js'; +import { InMemoryStore } from '../../platform/store/in-memory-store.js'; +// import { GoogleOAuth } from './google/google-oauth.js'; export class ConfiguredIdpOAuth extends OAuth { - public static newAuthPopupForEnterpriseServerAuthenticationIfNeeded = async (acctEmail: string) => { - const storage = await AcctStore.get(acctEmail, ['authentication']); - if (storage?.authentication?.oauth?.clientId && storage.authentication.oauth.clientId !== GoogleOAuth.OAUTH.client_id) { - await Ui.modal.warning( - `Custom IdP is configured on this domain, but it is not supported on browser extension yet. - Authentication with Enterprise Server will continue using Google IdP until implemented in a future update.` - ); - } else { - return; - } + public static newAuthPopupForEnterpriseServerAuthenticationIfNeeded = async (authRes: AuthRes) => { + const authConf = { + clientId: 'ZFfdspDb9Oz1vnFdA4NrCyNaA9N3jYDl', + clientSecret: 'HnXSXW5x1ea0nKTIqxZGrtHf1hdzQ83DA5fY0_j2catzmA9TlXw7_XE6F_r0nblc', + redirectUrl: 'https://www.google.com/robots.txt', + authCodeUrl: 'https://dev-c6f0fhaxut6kyeif.us.auth0.com/authorize', + tokensUrl: 'https://dev-c6f0fhaxut6kyeif.us.auth0.com/oauth/token', + }; + // const storage = await AcctStore.get(acctEmail, ['authentication']); + // if (storage?.authentication?.oauth?.clientId && storage.authentication.oauth.clientId !== GoogleOAuth.OAUTH.client_id) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const res = await this.newAuthPopup(authRes.acctEmail!, { oauth: authConf }); + return res; + // } else { + // return authRes; + // } }; + + public static async newAuthPopup(acctEmail: string, authConf: AuthenticationConfiguration): Promise { + if (acctEmail) { + acctEmail = acctEmail.toLowerCase(); + } + const authRequest = this.newAuthRequest(acctEmail, ['offline_access', 'openid', 'profile', 'email']); + const authUrl = this.apiOAuthCodeUrl(authConf, authRequest.expectedState, acctEmail); + // Added below logic because in service worker, it's not possible to access window object. + // Therefore need to retrieve screenDimensions when calling service worker and pass it to OAuth2 + const screenDimensions = Ui.getScreenDimensions(); + const authWindowResult = await OAuth2.webAuthFlow(authUrl, screenDimensions); + console.log(authWindowResult); + const authRes = await this.getAuthRes({ + acctEmail, + expectedState: authRequest.expectedState, + authWindowResult, + authConf, + }); + console.log(authRes); + if (authRes.result === 'Success') { + if (!authRes.id_token) { + return { + result: 'Error', + error: 'Grant was successful but missing id_token', + acctEmail, + id_token: undefined, // eslint-disable-line @typescript-eslint/naming-convention + }; + } + if (!authRes.acctEmail) { + return { + result: 'Error', + error: 'Grant was successful but missing acctEmail', + acctEmail: authRes.acctEmail, + id_token: undefined, // eslint-disable-line @typescript-eslint/naming-convention + }; + } + } + return authRes; + } + + private static apiOAuthCodeUrl(authConf: AuthenticationConfiguration, state: string, acctEmail: string) { + /* eslint-disable @typescript-eslint/naming-convention */ + return Url.create(authConf.oauth.authCodeUrl, { + client_id: authConf.oauth.clientId, + response_type: 'code', + access_type: 'offline', + prompt: 'login', + state, + redirect_uri: authConf.oauth.redirectUrl, + scope: 'offline_access openid profile email', + login_hint: acctEmail, + }); + /* eslint-enable @typescript-eslint/naming-convention */ + } + + private static async getAuthRes({ + acctEmail, + expectedState, + authWindowResult, + authConf, + }: { + acctEmail: string; + expectedState: string; + authWindowResult: Bm.AuthWindowResult; + authConf: AuthenticationConfiguration; + }): Promise { + /* eslint-disable @typescript-eslint/naming-convention */ + try { + if (!authWindowResult.url) { + return { acctEmail, result: 'Denied', error: 'Invalid response url', id_token: undefined }; + } + if (authWindowResult.error) { + return { acctEmail, result: 'Denied', error: authWindowResult.error, id_token: undefined }; + } + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const uncheckedUrlParams = Url.parse(['scope', 'code', 'state'], authWindowResult.url); + const code = Assert.urlParamRequire.optionalString(uncheckedUrlParams, 'code'); + const receivedState = Assert.urlParamRequire.string(uncheckedUrlParams, 'state'); + if (!code) { + return { + acctEmail, + result: 'Denied', + error: "OAuth result was 'Success' but no auth code", + id_token: undefined, + }; + } + if (receivedState !== expectedState) { + return { acctEmail, result: 'Error', error: `Wrong oauth CSRF token. Please try again.`, id_token: undefined }; + } + const { id_token } = await this.authGetTokens(code, authConf); + const { email } = this.parseIdToken(id_token); + if (!email) { + throw new Error('Missing email address in id_token'); + } + if (acctEmail !== email) { + return { + acctEmail, + result: 'Error', + error: `Google account email and custom IDP email doesn't match. Please use the same email address.`, + id_token: undefined, + }; + } + await InMemoryStore.set(acctEmail, InMemoryStoreKeys.CUSTOM_IDP_ID_TOKEN, id_token); + return { acctEmail: email, result: 'Success', id_token }; + } catch (err) { + return { acctEmail, result: 'Error', error: err instanceof AssertError ? 'Could not parse URL returned from OAuth' : String(err), id_token: undefined }; + } + /* eslint-enable @typescript-eslint/naming-convention */ + } + + private static async authGetTokens(code: string, authConf: AuthenticationConfiguration): Promise { + return await Api.ajax( + { + /* eslint-disable @typescript-eslint/naming-convention */ + url: authConf.oauth.tokensUrl, + method: 'POST', + data: { + grant_type: 'authorization_code', + code, + client_id: authConf.oauth.clientId, + client_secret: authConf.oauth.clientSecret, + redirect_uri: authConf.oauth.redirectUrl, + }, + dataType: 'JSON', + /* eslint-enable @typescript-eslint/naming-convention */ + stack: Catch.stackTrace(), + }, + 'json' + ); + } } diff --git a/extension/js/common/api/authentication/generic/oauth.ts b/extension/js/common/api/authentication/generic/oauth.ts index b157b849ed2..a0946a35c07 100644 --- a/extension/js/common/api/authentication/generic/oauth.ts +++ b/extension/js/common/api/authentication/generic/oauth.ts @@ -2,9 +2,33 @@ 'use strict'; +import { GoogleAuthWindowResult$result } from '../../../browser/browser-msg.js'; import { Buf } from '../../../core/buf.js'; import { Str } from '../../../core/common.js'; import { GmailRes } from '../../email-provider/gmail/gmail-parser.js'; +import { Api } from '../../shared/api.js'; + +export type AuthReq = { acctEmail?: string; scopes: string[]; messageId?: string; expectedState: string }; +// eslint-disable-next-line @typescript-eslint/naming-convention +type AuthResultSuccess = { result: 'Success'; acctEmail: string; id_token: string; error?: undefined }; +type AuthResultError = { + result: GoogleAuthWindowResult$result; + acctEmail?: string; + error?: string; + // eslint-disable-next-line @typescript-eslint/naming-convention + id_token: undefined; +}; +export type AuthRes = AuthResultSuccess | AuthResultError; + +/* eslint-disable @typescript-eslint/naming-convention */ +export type OAuthTokensResponse = { + access_token: string; + expires_in: number; + refresh_token?: string; + id_token: string; + token_type: 'Bearer'; +}; +/* eslint-enable @typescript-eslint/naming-convention */ export class OAuth { /** @@ -32,4 +56,16 @@ export class OAuth { } return claims; }; + + public static newAuthRequest(acctEmail: string | undefined, scopes: string[]): AuthReq { + const authReq = { + acctEmail, + scopes, + csrfToken: `csrf-${Api.randomFortyHexChars()}`, + }; + return { + ...authReq, + expectedState: `CRYPTUP_STATE_${JSON.stringify(authReq)}`, + }; + } } diff --git a/extension/js/common/api/authentication/google/google-oauth.ts b/extension/js/common/api/authentication/google/google-oauth.ts index c9640aaa7f7..a226e714731 100644 --- a/extension/js/common/api/authentication/google/google-oauth.ts +++ b/extension/js/common/api/authentication/google/google-oauth.ts @@ -7,38 +7,23 @@ import { FLAVOR, GOOGLE_OAUTH_SCREEN_HOST, OAUTH_GOOGLE_API_HOST } from '../../. import { ApiErr } from '../../shared/api-error.js'; import { Ajax, Api } from '../../shared/api.js'; -import { Bm, GoogleAuthWindowResult$result, ScreenDimensions } from '../../../browser/browser-msg.js'; +import { Bm, ScreenDimensions } from '../../../browser/browser-msg.js'; import { InMemoryStoreKeys } from '../../../core/const.js'; import { OAuth2 } from '../../../oauth2/oauth2.js'; import { Catch } from '../../../platform/catch.js'; import { AcctStore, AcctStoreDict } from '../../../platform/store/acct-store.js'; import { InMemoryStore } from '../../../platform/store/in-memory-store.js'; import { AccountServer } from '../../account-server.js'; -import { OAuth } from '../generic/oauth.js'; +import { AuthReq, AuthRes, OAuth, OAuthTokensResponse } from '../generic/oauth.js'; import { ExternalService } from '../../account-servers/external-service.js'; import { GoogleAuthErr } from '../../shared/api-error.js'; import { Assert, AssertError } from '../../../assert.js'; import { Ui } from '../../../browser/ui.js'; +import { ConfiguredIdpOAuth } from '../configured-idp-oauth.js'; -/* eslint-disable @typescript-eslint/naming-convention */ -type GoogleAuthTokensResponse = { - access_token: string; - expires_in: number; - refresh_token?: string; - id_token: string; - token_type: 'Bearer'; -}; -type AuthResultSuccess = { result: 'Success'; acctEmail: string; id_token: string; error?: undefined }; -type AuthResultError = { - result: GoogleAuthWindowResult$result; - acctEmail?: string; - error?: string; - id_token: undefined; -}; - -type AuthReq = { acctEmail?: string; scopes: string[]; messageId?: string; expectedState: string }; +// eslint-disable-next-line @typescript-eslint/naming-convention type GoogleTokenInfo = { email: string; scope: string; expires_in: number; token_type: string }; -export type AuthRes = AuthResultSuccess | AuthResultError; + /* eslint-enable @typescript-eslint/naming-convention */ export class GoogleOAuth extends OAuth { @@ -209,6 +194,7 @@ export class GoogleOAuth extends OAuth { } // fetch and store ClientConfiguration (not authenticated) await (await AccountServer.init(authRes.acctEmail)).fetchAndSaveClientConfiguration(); + return await ConfiguredIdpOAuth.newAuthPopupForEnterpriseServerAuthenticationIfNeeded(authRes); } catch (e) { if (GoogleOAuth.isFesUnreachableErr(e, authRes.acctEmail)) { const error = `Cannot reach your company's FlowCrypt External Service (FES). Contact your Help Desk when unsure. (${String(e)})`; @@ -283,18 +269,6 @@ export class GoogleOAuth extends OAuth { /* eslint-enable @typescript-eslint/naming-convention */ } - private static newAuthRequest(acctEmail: string | undefined, scopes: string[]): AuthReq { - const authReq = { - acctEmail, - scopes, - csrfToken: `csrf-${Api.randomFortyHexChars()}`, - }; - return { - ...authReq, - expectedState: GoogleOAuth.OAUTH.state_header + JSON.stringify(authReq), - }; - } - private static apiGoogleAuthCodeUrl(authReq: AuthReq) { /* eslint-disable @typescript-eslint/naming-convention */ return Url.create(GoogleOAuth.OAUTH.url_code, { @@ -310,7 +284,7 @@ export class GoogleOAuth extends OAuth { /* eslint-enable @typescript-eslint/naming-convention */ } - private static async googleAuthSaveTokens(acctEmail: string, tokensObj: GoogleAuthTokensResponse) { + private static async googleAuthSaveTokens(acctEmail: string, tokensObj: OAuthTokensResponse) { const parsedOpenId = GoogleOAuth.parseIdToken(tokensObj.id_token); const { full_name, picture } = await AcctStore.get(acctEmail, ['full_name', 'picture']); // eslint-disable-line @typescript-eslint/naming-convention const googleTokenExpires = new Date().getTime() + (tokensObj.expires_in - 120) * 1000; // let our copy expire 2 minutes beforehand @@ -326,7 +300,7 @@ export class GoogleOAuth extends OAuth { await InMemoryStore.set(acctEmail, InMemoryStoreKeys.GOOGLE_TOKEN_ACCESS, tokensObj.access_token, googleTokenExpires); } - private static async googleAuthGetTokens(code: string): Promise { + private static async googleAuthGetTokens(code: string): Promise { return await Api.ajax( { /* eslint-disable @typescript-eslint/naming-convention */ @@ -345,7 +319,7 @@ export class GoogleOAuth extends OAuth { ); } - private static async googleAuthRefreshToken(refreshToken: string): Promise { + private static async googleAuthRefreshToken(refreshToken: string): Promise { const url = /* eslint-disable @typescript-eslint/naming-convention */ Url.create(GoogleOAuth.OAUTH.url_tokens, { diff --git a/extension/js/common/browser/browser-msg.ts b/extension/js/common/browser/browser-msg.ts index bc182269024..dfa5d7f11c7 100644 --- a/extension/js/common/browser/browser-msg.ts +++ b/extension/js/common/browser/browser-msg.ts @@ -2,7 +2,6 @@ 'use strict'; -import { AuthRes } from '../api/authentication/google/google-oauth.js'; import { AjaxErr } from '../api/shared/api-error.js'; import { Buf } from '../core/buf.js'; import { Dict, Str, UrlParams } from '../core/common.js'; @@ -15,6 +14,7 @@ import { RenderMessage } from '../render-message.js'; import { SymEncryptedMessage, SymmetricMessageEncryption } from '../symmetric-message-encryption.js'; import { Ajax as ApiAjax, ResFmt } from '../api/shared/api.js'; import { Ui } from './ui.js'; +import { AuthRes } from '../api/authentication/generic/oauth.js'; export type GoogleAuthWindowResult$result = 'Success' | 'Denied' | 'Error' | 'Closed'; export type ScreenDimensions = { width: number; height: number; availLeft: number; availTop: number }; diff --git a/extension/js/common/core/const.ts b/extension/js/common/core/const.ts index e0475aff902..018fb0dd6ed 100644 --- a/extension/js/common/core/const.ts +++ b/extension/js/common/core/const.ts @@ -38,5 +38,6 @@ export const gmailBackupSearchQuery = (acctEmail: string) => { export class InMemoryStoreKeys { public static readonly ID_TOKEN = 'idToken'; + public static readonly CUSTOM_IDP_ID_TOKEN = 'customIdpIdToken'; public static readonly GOOGLE_TOKEN_ACCESS = 'google_token_access'; } diff --git a/extension/js/common/settings.ts b/extension/js/common/settings.ts index 9474e7aa6e9..1e7d8b3d362 100644 --- a/extension/js/common/settings.ts +++ b/extension/js/common/settings.ts @@ -27,7 +27,6 @@ import { isCustomerUrlFesUsed } from './helpers.js'; import { Api } from './api/shared/api.js'; import { Time } from './browser/time.js'; import { Google } from './api/email-provider/gmail/google.js'; -import { ConfiguredIdpOAuth } from './api/authentication/configured-idp-oauth.js'; declare const zxcvbn: (password: string, userInputs: string[]) => { guesses: number }; @@ -344,7 +343,6 @@ export class Settings { const response = await GoogleOAuth.newAuthPopup({ acctEmail, scopes }); if (response.result === 'Success' && response.acctEmail) { await GlobalStore.acctEmailsAdd(response.acctEmail); - await ConfiguredIdpOAuth.newAuthPopupForEnterpriseServerAuthenticationIfNeeded(response.acctEmail); const storage = await AcctStore.get(response.acctEmail, ['setup_done']); if (storage.setup_done) { // this was just an additional permission From 3f10b9ef5595428a3d34d07b6f728e6109bb48e8 Mon Sep 17 00:00:00 2001 From: Ioan Moldovan Date: Thu, 25 Jul 2024 11:48:02 +0300 Subject: [PATCH 2/9] WIP: added ui test --- .../authentication/configured-idp-oauth.ts | 32 ++++----- .../api/authentication/generic/oauth.ts | 23 +++++++ .../api/authentication/google/google-oauth.ts | 66 +++++++------------ .../js/common/platform/store/acct-store.ts | 13 ++-- test/source/mock/google/google-endpoints.ts | 49 ++++++++++---- test/source/mock/lib/oauth.ts | 2 + test/source/tests/setup.ts | 14 ++-- test/source/tests/tooling/browser-recipe.ts | 10 ++- 8 files changed, 120 insertions(+), 89 deletions(-) diff --git a/extension/js/common/api/authentication/configured-idp-oauth.ts b/extension/js/common/api/authentication/configured-idp-oauth.ts index f1ce8192e04..dd41b10b116 100644 --- a/extension/js/common/api/authentication/configured-idp-oauth.ts +++ b/extension/js/common/api/authentication/configured-idp-oauth.ts @@ -14,24 +14,26 @@ import { Api } from '../shared/api.js'; import { Catch } from '../../platform/catch.js'; import { InMemoryStoreKeys } from '../../core/const.js'; import { InMemoryStore } from '../../platform/store/in-memory-store.js'; +import { AcctStore } from '../../platform/store/acct-store.js'; // import { GoogleOAuth } from './google/google-oauth.js'; export class ConfiguredIdpOAuth extends OAuth { public static newAuthPopupForEnterpriseServerAuthenticationIfNeeded = async (authRes: AuthRes) => { - const authConf = { - clientId: 'ZFfdspDb9Oz1vnFdA4NrCyNaA9N3jYDl', - clientSecret: 'HnXSXW5x1ea0nKTIqxZGrtHf1hdzQ83DA5fY0_j2catzmA9TlXw7_XE6F_r0nblc', - redirectUrl: 'https://www.google.com/robots.txt', - authCodeUrl: 'https://dev-c6f0fhaxut6kyeif.us.auth0.com/authorize', - tokensUrl: 'https://dev-c6f0fhaxut6kyeif.us.auth0.com/oauth/token', - }; - // const storage = await AcctStore.get(acctEmail, ['authentication']); - // if (storage?.authentication?.oauth?.clientId && storage.authentication.oauth.clientId !== GoogleOAuth.OAUTH.client_id) { + // const authConf = { + // clientId: 'ZFfdspDb9Oz1vnFdA4NrCyNaA9N3jYDl', + // clientSecret: 'HnXSXW5x1ea0nKTIqxZGrtHf1hdzQ83DA5fY0_j2catzmA9TlXw7_XE6F_r0nblc', + // redirectUrl: 'https://www.google.com/robots.txt', + // authCodeUrl: 'https://dev-c6f0fhaxut6kyeif.us.auth0.com/authorize', + // tokensUrl: 'https://dev-c6f0fhaxut6kyeif.us.auth0.com/oauth/token', + // }; // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const res = await this.newAuthPopup(authRes.acctEmail!, { oauth: authConf }); - return res; - // } else { - // return authRes; - // } + const acctEmail = authRes.acctEmail!; + const storage = await AcctStore.get(acctEmail, ['authentication']); + if (storage?.authentication?.oauth?.clientId && storage.authentication.oauth.clientId !== this.GOOGLE_OAUTH_CONFIG.client_id) { + const res = await this.newAuthPopup(acctEmail, { oauth: storage.authentication.oauth }); + return res; + } else { + return authRes; + } }; public static async newAuthPopup(acctEmail: string, authConf: AuthenticationConfiguration): Promise { @@ -44,14 +46,12 @@ export class ConfiguredIdpOAuth extends OAuth { // Therefore need to retrieve screenDimensions when calling service worker and pass it to OAuth2 const screenDimensions = Ui.getScreenDimensions(); const authWindowResult = await OAuth2.webAuthFlow(authUrl, screenDimensions); - console.log(authWindowResult); const authRes = await this.getAuthRes({ acctEmail, expectedState: authRequest.expectedState, authWindowResult, authConf, }); - console.log(authRes); if (authRes.result === 'Success') { if (!authRes.id_token) { return { diff --git a/extension/js/common/api/authentication/generic/oauth.ts b/extension/js/common/api/authentication/generic/oauth.ts index a0946a35c07..cdb58ee00a4 100644 --- a/extension/js/common/api/authentication/generic/oauth.ts +++ b/extension/js/common/api/authentication/generic/oauth.ts @@ -5,6 +5,7 @@ import { GoogleAuthWindowResult$result } from '../../../browser/browser-msg.js'; import { Buf } from '../../../core/buf.js'; import { Str } from '../../../core/common.js'; +import { GOOGLE_OAUTH_SCREEN_HOST, OAUTH_GOOGLE_API_HOST } from '../../../core/const.js'; import { GmailRes } from '../../email-provider/gmail/gmail-parser.js'; import { Api } from '../../shared/api.js'; @@ -31,6 +32,28 @@ export type OAuthTokensResponse = { /* eslint-enable @typescript-eslint/naming-convention */ export class OAuth { + /* eslint-disable @typescript-eslint/naming-convention */ + public static GOOGLE_OAUTH_CONFIG = { + client_id: '717284730244-5oejn54f10gnrektjdc4fv4rbic1bj1p.apps.googleusercontent.com', + client_secret: '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', + redirect_uri: 'https://www.google.com/robots.txt', + url_code: `${GOOGLE_OAUTH_SCREEN_HOST}/o/oauth2/auth`, + url_tokens: `${OAUTH_GOOGLE_API_HOST}/token`, + state_header: 'CRYPTUP_STATE_', + scopes: { + email: 'email', + openid: 'openid', + profile: 'https://www.googleapis.com/auth/userinfo.profile', // needed so that `name` is present in `id_token`, which is required for key-server auth when in use + compose: 'https://www.googleapis.com/auth/gmail.compose', + modify: 'https://www.googleapis.com/auth/gmail.modify', + readContacts: 'https://www.googleapis.com/auth/contacts.readonly', + readOtherContacts: 'https://www.googleapis.com/auth/contacts.other.readonly', + }, + legacy_scopes: { + gmail: 'https://mail.google.com/', // causes a freakish oauth warn: "can permannently delete all your email" ... + }, + }; + /* eslint-enable @typescript-eslint/naming-convention */ /** * Happens on enterprise builds */ diff --git a/extension/js/common/api/authentication/google/google-oauth.ts b/extension/js/common/api/authentication/google/google-oauth.ts index a226e714731..e526a47ffd9 100644 --- a/extension/js/common/api/authentication/google/google-oauth.ts +++ b/extension/js/common/api/authentication/google/google-oauth.ts @@ -3,7 +3,7 @@ 'use strict'; import { Url } from '../../../core/common.js'; -import { FLAVOR, GOOGLE_OAUTH_SCREEN_HOST, OAUTH_GOOGLE_API_HOST } from '../../../core/const.js'; +import { FLAVOR, OAUTH_GOOGLE_API_HOST } from '../../../core/const.js'; import { ApiErr } from '../../shared/api-error.js'; import { Ajax, Api } from '../../shared/api.js'; @@ -27,31 +27,8 @@ type GoogleTokenInfo = { email: string; scope: string; expires_in: number; token /* eslint-enable @typescript-eslint/naming-convention */ export class GoogleOAuth extends OAuth { - /* eslint-disable @typescript-eslint/naming-convention */ - public static OAUTH = { - client_id: '717284730244-5oejn54f10gnrektjdc4fv4rbic1bj1p.apps.googleusercontent.com', - client_secret: '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', - redirect_uri: 'https://www.google.com/robots.txt', - url_code: `${GOOGLE_OAUTH_SCREEN_HOST}/o/oauth2/auth`, - url_tokens: `${OAUTH_GOOGLE_API_HOST}/token`, - state_header: 'CRYPTUP_STATE_', - scopes: { - email: 'email', - openid: 'openid', - profile: 'https://www.googleapis.com/auth/userinfo.profile', // needed so that `name` is present in `id_token`, which is required for key-server auth when in use - compose: 'https://www.googleapis.com/auth/gmail.compose', - modify: 'https://www.googleapis.com/auth/gmail.modify', - readContacts: 'https://www.googleapis.com/auth/contacts.readonly', - readOtherContacts: 'https://www.googleapis.com/auth/contacts.other.readonly', - }, - legacy_scopes: { - gmail: 'https://mail.google.com/', // causes a freakish oauth warn: "can permannently delete all your email" ... - }, - }; - /* eslint-enable @typescript-eslint/naming-convention */ - public static defaultScopes(group: 'default' | 'contacts' = 'default') { - const { readContacts, readOtherContacts, compose, modify, openid, email, profile } = GoogleOAuth.OAUTH.scopes; + const { readContacts, readOtherContacts, compose, modify, openid, email, profile } = this.GOOGLE_OAUTH_CONFIG.scopes; if (group === 'default') { if (FLAVOR === 'consumer') { return [openid, email, profile, compose, modify]; // consumer may freak out that extension asks for their contacts early on @@ -237,7 +214,12 @@ export class GoogleOAuth extends OAuth { const allowedScopes = Assert.urlParamRequire.string(uncheckedUrlParams, 'scope'); const code = Assert.urlParamRequire.optionalString(uncheckedUrlParams, 'code'); const receivedState = Assert.urlParamRequire.string(uncheckedUrlParams, 'state'); - const scopesToCheck = [this.OAUTH.scopes.compose, this.OAUTH.scopes.modify, this.OAUTH.scopes.readContacts, this.OAUTH.scopes.readOtherContacts]; + 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 }; @@ -271,13 +253,13 @@ export class GoogleOAuth extends OAuth { private static apiGoogleAuthCodeUrl(authReq: AuthReq) { /* eslint-disable @typescript-eslint/naming-convention */ - return Url.create(GoogleOAuth.OAUTH.url_code, { - client_id: GoogleOAuth.OAUTH.client_id, + return Url.create(this.GOOGLE_OAUTH_CONFIG.url_code, { + client_id: this.GOOGLE_OAUTH_CONFIG.client_id, response_type: 'code', access_type: 'offline', prompt: 'consent', state: authReq.expectedState, - redirect_uri: GoogleOAuth.OAUTH.redirect_uri, + redirect_uri: this.GOOGLE_OAUTH_CONFIG.redirect_uri, scope: (authReq.scopes || []).join(' '), login_hint: authReq.acctEmail, }); @@ -304,12 +286,12 @@ export class GoogleOAuth extends OAuth { return await Api.ajax( { /* eslint-disable @typescript-eslint/naming-convention */ - url: Url.create(GoogleOAuth.OAUTH.url_tokens, { + url: Url.create(this.GOOGLE_OAUTH_CONFIG.url_tokens, { grant_type: 'authorization_code', code, - client_id: GoogleOAuth.OAUTH.client_id, - client_secret: GoogleOAuth.OAUTH.client_secret, - redirect_uri: GoogleOAuth.OAUTH.redirect_uri, + client_id: this.GOOGLE_OAUTH_CONFIG.client_id, + client_secret: this.GOOGLE_OAUTH_CONFIG.client_secret, + redirect_uri: this.GOOGLE_OAUTH_CONFIG.redirect_uri, }), /* eslint-enable @typescript-eslint/naming-convention */ method: 'POST', @@ -322,11 +304,11 @@ export class GoogleOAuth extends OAuth { private static async googleAuthRefreshToken(refreshToken: string): Promise { const url = /* eslint-disable @typescript-eslint/naming-convention */ - Url.create(GoogleOAuth.OAUTH.url_tokens, { + Url.create(this.GOOGLE_OAUTH_CONFIG.url_tokens, { grant_type: 'refresh_token', refreshToken, - client_id: GoogleOAuth.OAUTH.client_id, - client_secret: GoogleOAuth.OAUTH.client_secret, + client_id: this.GOOGLE_OAUTH_CONFIG.client_id, + client_secret: this.GOOGLE_OAUTH_CONFIG.client_secret, }); /* eslint-enable @typescript-eslint/naming-convention */ const req: Ajax = { @@ -350,14 +332,14 @@ export class GoogleOAuth extends OAuth { } private static apiGoogleAuthPopupPrepareAuthReqScopes(addScopes: string[]): string[] { - if (!addScopes.includes(GoogleOAuth.OAUTH.scopes.email)) { - addScopes.push(GoogleOAuth.OAUTH.scopes.email); + if (!addScopes.includes(this.GOOGLE_OAUTH_CONFIG.scopes.email)) { + addScopes.push(this.GOOGLE_OAUTH_CONFIG.scopes.email); } - if (!addScopes.includes(GoogleOAuth.OAUTH.scopes.openid)) { - addScopes.push(GoogleOAuth.OAUTH.scopes.openid); + if (!addScopes.includes(this.GOOGLE_OAUTH_CONFIG.scopes.openid)) { + addScopes.push(this.GOOGLE_OAUTH_CONFIG.scopes.openid); } - if (!addScopes.includes(GoogleOAuth.OAUTH.scopes.profile)) { - addScopes.push(GoogleOAuth.OAUTH.scopes.profile); + if (!addScopes.includes(this.GOOGLE_OAUTH_CONFIG.scopes.profile)) { + addScopes.push(this.GOOGLE_OAUTH_CONFIG.scopes.profile); } return addScopes; } diff --git a/extension/js/common/platform/store/acct-store.ts b/extension/js/common/platform/store/acct-store.ts index 75d45c77b46..943c508e11a 100644 --- a/extension/js/common/platform/store/acct-store.ts +++ b/extension/js/common/platform/store/acct-store.ts @@ -1,5 +1,6 @@ /* ©️ 2016 - present FlowCrypt a.s. Limitations apply. Contact human@flowcrypt.com */ +import { OAuth } from '../../api/authentication/generic/oauth.js'; import { GoogleOAuth } from '../../api/authentication/google/google-oauth.js'; import { ApiErr } from '../../api/shared/api-error.js'; import { AuthenticationConfiguration } from '../../authentication-configuration.js'; @@ -13,7 +14,7 @@ import { AbstractStore, RawStore } from './abstract-store.js'; import { InMemoryStore } from './in-memory-store.js'; export type EmailProvider = 'gmail'; -type GoogleAuthScopesNames = [keyof typeof GoogleOAuth.OAUTH.scopes, keyof typeof GoogleOAuth.OAUTH.legacy_scopes][number]; +type GoogleAuthScopesNames = [keyof typeof OAuth.GOOGLE_OAUTH_CONFIG.scopes, keyof typeof OAuth.GOOGLE_OAUTH_CONFIG.legacy_scopes][number]; export type Scopes = { openid: boolean; @@ -148,12 +149,12 @@ export class AcctStore extends AbstractStore { BrowserMsg.send.notificationShowAuthPopupNeeded('broadcast', { acctEmail }); } } - for (const key of Object.keys({ ...GoogleOAuth.OAUTH.scopes, ...GoogleOAuth.OAUTH.legacy_scopes })) { + for (const key of Object.keys({ ...OAuth.GOOGLE_OAUTH_CONFIG.scopes, ...OAuth.GOOGLE_OAUTH_CONFIG.legacy_scopes })) { const scopeName = key as GoogleAuthScopesNames; - if (scopeName in GoogleOAuth.OAUTH.scopes) { - result[scopeName] = allowedScopes.includes(GoogleOAuth.OAUTH.scopes[scopeName as keyof typeof GoogleOAuth.OAUTH.scopes]); - } else if (scopeName in GoogleOAuth.OAUTH.legacy_scopes) { - result[scopeName] = allowedScopes.includes(GoogleOAuth.OAUTH.legacy_scopes[scopeName as keyof typeof GoogleOAuth.OAUTH.legacy_scopes]); + if (scopeName in OAuth.GOOGLE_OAUTH_CONFIG.scopes) { + result[scopeName] = allowedScopes.includes(OAuth.GOOGLE_OAUTH_CONFIG.scopes[scopeName as keyof typeof OAuth.GOOGLE_OAUTH_CONFIG.scopes]); + } else if (scopeName in OAuth.GOOGLE_OAUTH_CONFIG.legacy_scopes) { + result[scopeName] = allowedScopes.includes(OAuth.GOOGLE_OAUTH_CONFIG.legacy_scopes[scopeName as keyof typeof OAuth.GOOGLE_OAUTH_CONFIG.legacy_scopes]); } } return result; diff --git a/test/source/mock/google/google-endpoints.ts b/test/source/mock/google/google-endpoints.ts index 106e08fa9d0..ff7987c022c 100644 --- a/test/source/mock/google/google-endpoints.ts +++ b/test/source/mock/google/google-endpoints.ts @@ -14,6 +14,16 @@ import { Dict } from '../../core/common'; type DraftSaveModel = { message: { raw: string; threadId: string } }; +/* eslint-disable @typescript-eslint/naming-convention */ +type OAuthTokenRequestModel = { + grant_type: string; + code: string; + client_id: string; + client_secret: string; + redirect_uri: string; +}; +/* eslint-enable @typescript-eslint/naming-convention */ + const allowedRecipients: string[] = [ 'flowcrypt.compatibility@gmail.com', 'manualcopypgp@flowcrypt.com', @@ -114,26 +124,37 @@ export const getMockGoogleEndpoints = (oauth: OauthMock, config: GoogleConfig | { query: { client_id, response_type, access_type, state, scope, login_hint, proceed } }, req ) => { - if (isGet(req) && client_id === oauth.clientId && response_type === 'code' && access_type === 'offline' && state && scope) { - // auth screen - if (!login_hint) { - return oauth.renderText('choose account with login_hint'); - } else if (!proceed) { - return oauth.renderText('redirect with proceed=true to continue'); - } else { + if (isGet(req) && response_type === 'code' && access_type === 'offline' && state && scope) { + if (client_id === oauth.clientId) { + // auth screen + if (!login_hint) { + return oauth.renderText('choose account with login_hint'); + } else if (!proceed) { + return oauth.renderText('redirect with proceed=true to continue'); + } else { + return oauth.successResult(parsePort(req), login_hint, state, scope); + } + } else if (client_id === OauthMock.customIDPClientId) { return oauth.successResult(parsePort(req), login_hint, state, scope); } } throw new HttpClientErr(`Method not implemented for ${req.url}: ${req.method}`); }, // eslint-disable-next-line @typescript-eslint/naming-convention - '/token': async ({ query: { grant_type, refreshToken, client_id, code } }, req) => { - if (isPost(req) && grant_type === 'authorization_code' && code && client_id === oauth.clientId) { - // auth code from auth screen gets exchanged for access and refresh tokens - return oauth.getRefreshTokenResponse(code); - } else if (isPost(req) && grant_type === 'refresh_token' && refreshToken && client_id === oauth.clientId) { - // here also later refresh token gets exchanged for access token - return oauth.getTokenResponse(refreshToken); + '/token': async ({ query: { grant_type, refreshToken, client_id, code }, body }, req) => { + if (isPost(req)) { + if (grant_type === 'authorization_code' && code && client_id === oauth.clientId) { + // auth code from auth screen gets exchanged for access and refresh tokens + return oauth.getRefreshTokenResponse(code); + } else if (grant_type === 'refresh_token' && refreshToken && client_id === oauth.clientId) { + // here also later refresh token gets exchanged for access token + return oauth.getTokenResponse(refreshToken); + } + const parsedBody = body as OAuthTokenRequestModel; + // Above is for Google OAuth and this is for normal OAuth + if (parsedBody.grant_type === 'authorization_code' && parsedBody.code && parsedBody.client_id === OauthMock.customIDPClientId) { + return oauth.getRefreshTokenResponse(parsedBody.code); + } } throw new Error(`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 418744c924c..07457f137b3 100644 --- a/test/source/mock/lib/oauth.ts +++ b/test/source/mock/lib/oauth.ts @@ -8,6 +8,8 @@ import { Str } from '../../core/common'; const authURL = 'https://localhost:8001'; export class OauthMock { + public static customIDPClientId = 'ZFfdspDb9Oz1vnFdA4NrCyNaA9N3jYDl'; + public static customIDPClientSecret = 'test-secret'; public clientId = '717284730244-5oejn54f10gnrektjdc4fv4rbic1bj1p.apps.googleusercontent.com'; public expiresIn = 2 * 60 * 60; // 2hrs in seconds diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 4b4d0a0a89a..528986c1b89 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -34,6 +34,7 @@ import { FesAuthenticationConfiguration, FesClientConfiguration } from '../mock/ import { GoogleData } from '../mock/google/google-data'; import Parse from '../util/parse'; import { MsgUtil } from '../core/crypto/pgp/msg-util'; +import { OauthMock } from '../mock/lib/oauth'; const getAuthorizationHeader = async (t: AvaContext, browser: BrowserHandle, acctEmail: string) => { const settingsPage = await browser.newExtensionSettingsPage(t, acctEmail); @@ -2525,12 +2526,13 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== test( 'setup - check custom authentication config from the local store (customer url fes)', testWithBrowser(async (t, browser) => { + const port = t.context.urls?.port ?? ''; const oauthConfig = { - clientId: 'your-client-id', - clientSecret: 'your-client-secret', - redirectUrl: 'https://example.com/redirect', - authCodeUrl: 'https://example.com/auth', - tokensUrl: 'https://example.com/tokens', + clientId: OauthMock.customIDPClientId, + clientSecret: OauthMock.customIDPClientId, + redirectUrl: `https://google.localhost:${port}/robots.txt`, + authCodeUrl: `https://localhost:${port}/o/oauth2/auth`, + tokensUrl: `https://localhost:${port}/token`, }; t.context.mockApi!.configProvider = new ConfigurationProvider({ fes: { @@ -2548,6 +2550,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== const auth = (await settingsPage.getFromLocalStorage([key]))[key]; const { oauth } = auth as FesAuthenticationConfiguration; expect(oauth).to.deep.equal(oauthConfig); + const savedCustomIDPIDToken = await BrowserRecipe.getCustomIDPIdTokenFromInMemoryStore(settingsPage, acctEmail); + expect(savedCustomIDPIDToken).to.not.be.an.undefined; }) ); } diff --git a/test/source/tests/tooling/browser-recipe.ts b/test/source/tests/tooling/browser-recipe.ts index 5bf916b0624..d82da84e2b7 100644 --- a/test/source/tests/tooling/browser-recipe.ts +++ b/test/source/tests/tooling/browser-recipe.ts @@ -46,12 +46,7 @@ export class BrowserRecipe { const settingsPage = await browser.newExtensionSettingsPage(t, acctEmail); const oauthPopup = await browser.newPageTriggeredBy(t, () => settingsPage.waitAndClick('@action-connect-to-gmail')); await OauthPageRecipe.google(t, oauthPopup, acctEmail, 'approve'); - if (checkForConfiguredIdPOAuth) - await settingsPage.waitAndRespondToModal( - 'warning', - 'confirm', - 'Custom IdP is configured on this domain, but it is not supported on browser extension yet.' - ); + if (checkForConfiguredIdPOAuth) await Util.sleep(20); // Wait until custom IDP authentication finished return settingsPage; }; @@ -215,6 +210,9 @@ export class BrowserRecipe { public static getPassphraseFromInMemoryStore = (controllable: Controllable, acctEmail: string, longid: string): Promise => BrowserRecipe.getFromInMemoryStore(controllable, acctEmail, `passphrase_${longid}`); + public static getCustomIDPIdTokenFromInMemoryStore = (controllable: Controllable, acctEmail: string): Promise => + BrowserRecipe.getFromInMemoryStore(controllable, acctEmail, 'customIdpIdToken'); + public static deleteAllDraftsInGmailAccount = async (accessToken: string): Promise => { const gmail = google.gmail({ version: 'v1' }); // eslint-disable-next-line @typescript-eslint/naming-convention From a642da78c277ab54e8406456d6fd443c0a66b04e Mon Sep 17 00:00:00 2001 From: Ioan Moldovan Date: Fri, 26 Jul 2024 03:22:52 +0300 Subject: [PATCH 3/9] fix: ui test --- test/source/tests/setup.ts | 6 ++++++ test/source/tests/tooling/browser-recipe.ts | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 528986c1b89..05d13101cd1 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -2535,10 +2535,16 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== tokensUrl: `https://localhost:${port}/token`, }; t.context.mockApi!.configProvider = new ConfigurationProvider({ + attester: { + pubkeyLookup: {}, + }, fes: { authenticationConfiguration: { oauth: oauthConfig, }, + clientConfiguration: { + flags: ['NO_ATTESTER_SUBMIT'], + }, }, }); const acctEmail = 'user@authentication-config-test.flowcrypt.test'; diff --git a/test/source/tests/tooling/browser-recipe.ts b/test/source/tests/tooling/browser-recipe.ts index d82da84e2b7..9c624512c22 100644 --- a/test/source/tests/tooling/browser-recipe.ts +++ b/test/source/tests/tooling/browser-recipe.ts @@ -46,7 +46,8 @@ export class BrowserRecipe { const settingsPage = await browser.newExtensionSettingsPage(t, acctEmail); const oauthPopup = await browser.newPageTriggeredBy(t, () => settingsPage.waitAndClick('@action-connect-to-gmail')); await OauthPageRecipe.google(t, oauthPopup, acctEmail, 'approve'); - if (checkForConfiguredIdPOAuth) await Util.sleep(20); // Wait until custom IDP authentication finished + if (checkForConfiguredIdPOAuth) await settingsPage.page.waitForNavigation({ waitUntil: 'networkidle2' }); + // Wait until custom IDP authentication finished return settingsPage; }; From d969c1b9b464af47a03fafd4c976884de0512d0f Mon Sep 17 00:00:00 2001 From: Ioan Moldovan Date: Mon, 29 Jul 2024 05:01:32 +0300 Subject: [PATCH 4/9] fix: refactor --- .../api/authentication/configured-idp-oauth.ts | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/extension/js/common/api/authentication/configured-idp-oauth.ts b/extension/js/common/api/authentication/configured-idp-oauth.ts index dd41b10b116..1c55b4d739b 100644 --- a/extension/js/common/api/authentication/configured-idp-oauth.ts +++ b/extension/js/common/api/authentication/configured-idp-oauth.ts @@ -18,28 +18,17 @@ import { AcctStore } from '../../platform/store/acct-store.js'; // import { GoogleOAuth } from './google/google-oauth.js'; export class ConfiguredIdpOAuth extends OAuth { public static newAuthPopupForEnterpriseServerAuthenticationIfNeeded = async (authRes: AuthRes) => { - // const authConf = { - // clientId: 'ZFfdspDb9Oz1vnFdA4NrCyNaA9N3jYDl', - // clientSecret: 'HnXSXW5x1ea0nKTIqxZGrtHf1hdzQ83DA5fY0_j2catzmA9TlXw7_XE6F_r0nblc', - // redirectUrl: 'https://www.google.com/robots.txt', - // authCodeUrl: 'https://dev-c6f0fhaxut6kyeif.us.auth0.com/authorize', - // tokensUrl: 'https://dev-c6f0fhaxut6kyeif.us.auth0.com/oauth/token', - // }; // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const acctEmail = authRes.acctEmail!; const storage = await AcctStore.get(acctEmail, ['authentication']); if (storage?.authentication?.oauth?.clientId && storage.authentication.oauth.clientId !== this.GOOGLE_OAUTH_CONFIG.client_id) { - const res = await this.newAuthPopup(acctEmail, { oauth: storage.authentication.oauth }); - return res; - } else { - return authRes; + return await this.newAuthPopup(acctEmail, { oauth: storage.authentication.oauth }); } + return authRes; }; public static async newAuthPopup(acctEmail: string, authConf: AuthenticationConfiguration): Promise { - if (acctEmail) { - acctEmail = acctEmail.toLowerCase(); - } + acctEmail = acctEmail?.toLowerCase(); const authRequest = this.newAuthRequest(acctEmail, ['offline_access', 'openid', 'profile', 'email']); const authUrl = this.apiOAuthCodeUrl(authConf, authRequest.expectedState, acctEmail); // Added below logic because in service worker, it's not possible to access window object. From 6ad02f1bdba568398a28b4992f625fdced39e741 Mon Sep 17 00:00:00 2001 From: Ioan Moldovan Date: Mon, 29 Jul 2024 05:08:28 +0300 Subject: [PATCH 5/9] fix: typo --- .../js/common/api/authentication/configured-idp-oauth.ts | 6 +++--- extension/js/common/api/authentication/generic/oauth.ts | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/extension/js/common/api/authentication/configured-idp-oauth.ts b/extension/js/common/api/authentication/configured-idp-oauth.ts index 1c55b4d739b..005cb864e2e 100644 --- a/extension/js/common/api/authentication/configured-idp-oauth.ts +++ b/extension/js/common/api/authentication/configured-idp-oauth.ts @@ -29,7 +29,7 @@ export class ConfiguredIdpOAuth extends OAuth { public static async newAuthPopup(acctEmail: string, authConf: AuthenticationConfiguration): Promise { acctEmail = acctEmail?.toLowerCase(); - const authRequest = this.newAuthRequest(acctEmail, ['offline_access', 'openid', 'profile', 'email']); + const authRequest = this.newAuthRequest(acctEmail, this.OAUTH_REQUEST_SCOPES); const authUrl = this.apiOAuthCodeUrl(authConf, authRequest.expectedState, acctEmail); // Added below logic because in service worker, it's not possible to access window object. // Therefore need to retrieve screenDimensions when calling service worker and pass it to OAuth2 @@ -71,7 +71,7 @@ export class ConfiguredIdpOAuth extends OAuth { prompt: 'login', state, redirect_uri: authConf.oauth.redirectUrl, - scope: 'offline_access openid profile email', + scope: this.OAUTH_REQUEST_SCOPES.join(' '), login_hint: acctEmail, }); /* eslint-enable @typescript-eslint/naming-convention */ @@ -120,7 +120,7 @@ export class ConfiguredIdpOAuth extends OAuth { return { acctEmail, result: 'Error', - error: `Google account email and custom IDP email doesn't match. Please use the same email address.`, + error: `Google account email and custom IDP email do not match. Please use the same email address..`, id_token: undefined, }; } diff --git a/extension/js/common/api/authentication/generic/oauth.ts b/extension/js/common/api/authentication/generic/oauth.ts index cdb58ee00a4..75a7f2e26eb 100644 --- a/extension/js/common/api/authentication/generic/oauth.ts +++ b/extension/js/common/api/authentication/generic/oauth.ts @@ -53,6 +53,7 @@ export class OAuth { gmail: 'https://mail.google.com/', // causes a freakish oauth warn: "can permannently delete all your email" ... }, }; + public static OAUTH_REQUEST_SCOPES = ['offline_access', 'openid', 'profile', 'email']; /* eslint-enable @typescript-eslint/naming-convention */ /** * Happens on enterprise builds From 87c2aba3460a06b4f4fc9bc0bb3a2beadf3f2888 Mon Sep 17 00:00:00 2001 From: Ioan Moldovan Date: Tue, 30 Jul 2024 02:42:18 +0300 Subject: [PATCH 6/9] fix: test client id --- test/source/mock/lib/oauth.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/source/mock/lib/oauth.ts b/test/source/mock/lib/oauth.ts index 07457f137b3..7010f52b370 100644 --- a/test/source/mock/lib/oauth.ts +++ b/test/source/mock/lib/oauth.ts @@ -8,7 +8,7 @@ import { Str } from '../../core/common'; const authURL = 'https://localhost:8001'; export class OauthMock { - public static customIDPClientId = 'ZFfdspDb9Oz1vnFdA4NrCyNaA9N3jYDl'; + public static customIDPClientId = 'custom-test-idp-client-id'; public static customIDPClientSecret = 'test-secret'; public clientId = '717284730244-5oejn54f10gnrektjdc4fv4rbic1bj1p.apps.googleusercontent.com'; public expiresIn = 2 * 60 * 60; // 2hrs in seconds From 7326b5d2d00676dd3441b189edb3822d0b2c747b Mon Sep 17 00:00:00 2001 From: Ioan Moldovan Date: Tue, 30 Jul 2024 14:49:05 +0300 Subject: [PATCH 7/9] fix: pr reviews --- .../api/authentication/configured-idp-oauth.ts | 1 + test/source/mock/google/google-endpoints.ts | 3 +++ test/source/tests/page-recipe/oauth-page-recipe.ts | 5 +++++ test/source/tests/setup.ts | 2 +- test/source/tests/tooling/browser-recipe.ts | 13 +++++++++---- 5 files changed, 19 insertions(+), 5 deletions(-) diff --git a/extension/js/common/api/authentication/configured-idp-oauth.ts b/extension/js/common/api/authentication/configured-idp-oauth.ts index 005cb864e2e..62647c97624 100644 --- a/extension/js/common/api/authentication/configured-idp-oauth.ts +++ b/extension/js/common/api/authentication/configured-idp-oauth.ts @@ -22,6 +22,7 @@ export class ConfiguredIdpOAuth extends OAuth { const acctEmail = authRes.acctEmail!; const storage = await AcctStore.get(acctEmail, ['authentication']); if (storage?.authentication?.oauth?.clientId && storage.authentication.oauth.clientId !== this.GOOGLE_OAUTH_CONFIG.client_id) { + await Ui.modal.info('Google login succeeded. Now, please log in with your company credentials as well.'); return await this.newAuthPopup(acctEmail, { oauth: storage.authentication.oauth }); } return authRes; diff --git a/test/source/mock/google/google-endpoints.ts b/test/source/mock/google/google-endpoints.ts index ff7987c022c..84e417bba5a 100644 --- a/test/source/mock/google/google-endpoints.ts +++ b/test/source/mock/google/google-endpoints.ts @@ -135,6 +135,9 @@ export const getMockGoogleEndpoints = (oauth: OauthMock, config: GoogleConfig | return oauth.successResult(parsePort(req), login_hint, state, scope); } } else if (client_id === OauthMock.customIDPClientId) { + if (!proceed) { + return oauth.renderText('redirect with proceed=true to continue'); + } return oauth.successResult(parsePort(req), login_hint, state, scope); } } diff --git a/test/source/tests/page-recipe/oauth-page-recipe.ts b/test/source/tests/page-recipe/oauth-page-recipe.ts index ce92e394558..c8c3cef3bac 100644 --- a/test/source/tests/page-recipe/oauth-page-recipe.ts +++ b/test/source/tests/page-recipe/oauth-page-recipe.ts @@ -39,6 +39,11 @@ export class OauthPageRecipe extends PageRecipe { } }; + public static customIdp = async (t: AvaContext, oauthPage: ControllablePage): Promise => { + const mockOauthUrl = oauthPage.target.url(); + await oauthPage.target.goto(mockOauthUrl + '&proceed=true'); + }; + public static google = async ( t: AvaContext, oauthPage: ControllablePage, diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 05d13101cd1..9ee048efb90 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -2529,7 +2529,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== const port = t.context.urls?.port ?? ''; const oauthConfig = { clientId: OauthMock.customIDPClientId, - clientSecret: OauthMock.customIDPClientId, + clientSecret: OauthMock.customIDPClientSecret, redirectUrl: `https://google.localhost:${port}/robots.txt`, authCodeUrl: `https://localhost:${port}/o/oauth2/auth`, tokensUrl: `https://localhost:${port}/token`, diff --git a/test/source/tests/tooling/browser-recipe.ts b/test/source/tests/tooling/browser-recipe.ts index 9c624512c22..a75e3a0cb92 100644 --- a/test/source/tests/tooling/browser-recipe.ts +++ b/test/source/tests/tooling/browser-recipe.ts @@ -42,11 +42,16 @@ export class BrowserRecipe { return settingsPage; }; - public static openSettingsLoginApprove = async (t: AvaContext, browser: BrowserHandle, acctEmail: string, checkForConfiguredIdPOAuth?: boolean) => { + public static openSettingsLoginApprove = async (t: AvaContext, browser: BrowserHandle, acctEmail: string, expectCustomIdp?: boolean) => { const settingsPage = await browser.newExtensionSettingsPage(t, acctEmail); - const oauthPopup = await browser.newPageTriggeredBy(t, () => settingsPage.waitAndClick('@action-connect-to-gmail')); - await OauthPageRecipe.google(t, oauthPopup, acctEmail, 'approve'); - if (checkForConfiguredIdPOAuth) await settingsPage.page.waitForNavigation({ waitUntil: 'networkidle2' }); + const googleOAuthPopup = await browser.newPageTriggeredBy(t, () => settingsPage.waitAndClick('@action-connect-to-gmail')); + await OauthPageRecipe.google(t, googleOAuthPopup, acctEmail, 'approve'); + if (expectCustomIdp) { + const customOAuthPopup = await browser.newPageTriggeredBy(t, () => + settingsPage.waitAndRespondToModal('info', 'confirm', 'Google login succeeded. Now, please log in with your company credentials as well.') + ); + await OauthPageRecipe.customIdp(t, customOAuthPopup); + } // Wait until custom IDP authentication finished return settingsPage; }; From ddae27be37000ce23a02f4d95533c052f6f2df69 Mon Sep 17 00:00:00 2001 From: Ioan Moldovan Date: Wed, 31 Jul 2024 12:37:30 +0300 Subject: [PATCH 8/9] fix: pr reviews --- .../js/common/api/authentication/configured-idp-oauth.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/extension/js/common/api/authentication/configured-idp-oauth.ts b/extension/js/common/api/authentication/configured-idp-oauth.ts index 62647c97624..d37317fcc66 100644 --- a/extension/js/common/api/authentication/configured-idp-oauth.ts +++ b/extension/js/common/api/authentication/configured-idp-oauth.ts @@ -3,7 +3,6 @@ 'use strict'; import { Ui } from '../../browser/ui.js'; -// import { AcctStore } from '../../platform/store/acct-store.js'; import { AuthRes, OAuth, OAuthTokensResponse } from './generic/oauth.js'; import { AuthenticationConfiguration } from '../../authentication-configuration.js'; import { Url } from '../../core/common.js'; @@ -15,7 +14,6 @@ import { Catch } from '../../platform/catch.js'; import { InMemoryStoreKeys } from '../../core/const.js'; import { InMemoryStore } from '../../platform/store/in-memory-store.js'; import { AcctStore } from '../../platform/store/acct-store.js'; -// import { GoogleOAuth } from './google/google-oauth.js'; export class ConfiguredIdpOAuth extends OAuth { public static newAuthPopupForEnterpriseServerAuthenticationIfNeeded = async (authRes: AuthRes) => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion @@ -29,7 +27,7 @@ export class ConfiguredIdpOAuth extends OAuth { }; public static async newAuthPopup(acctEmail: string, authConf: AuthenticationConfiguration): Promise { - acctEmail = acctEmail?.toLowerCase(); + acctEmail = acctEmail.toLowerCase(); const authRequest = this.newAuthRequest(acctEmail, this.OAUTH_REQUEST_SCOPES); const authUrl = this.apiOAuthCodeUrl(authConf, authRequest.expectedState, acctEmail); // Added below logic because in service worker, it's not possible to access window object. From 468bfbaf595258bcabe1b48fa1de0161b43166ee Mon Sep 17 00:00:00 2001 From: Ioan Moldovan Date: Thu, 1 Aug 2024 14:51:13 +0300 Subject: [PATCH 9/9] feat: use chrome.identity.launchWebAuthFlow and updated UI test --- .../authentication/configured-idp-oauth.ts | 30 ++++++++----------- extension/manifest.json | 2 +- test/source/browser/browser-handle.ts | 3 +- test/source/mock/google/google-endpoints.ts | 4 +-- test/source/mock/lib/api.ts | 4 +-- test/source/mock/lib/oauth.ts | 5 ++-- .../tests/page-recipe/oauth-page-recipe.ts | 8 ++++- test/source/tests/setup.ts | 2 +- test/source/tests/tooling/browser-recipe.ts | 2 +- tooling/build-types-and-manifests.ts | 2 +- 10 files changed, 32 insertions(+), 30 deletions(-) diff --git a/extension/js/common/api/authentication/configured-idp-oauth.ts b/extension/js/common/api/authentication/configured-idp-oauth.ts index d37317fcc66..df7611885aa 100644 --- a/extension/js/common/api/authentication/configured-idp-oauth.ts +++ b/extension/js/common/api/authentication/configured-idp-oauth.ts @@ -6,8 +6,6 @@ import { Ui } from '../../browser/ui.js'; import { AuthRes, OAuth, OAuthTokensResponse } from './generic/oauth.js'; import { AuthenticationConfiguration } from '../../authentication-configuration.js'; import { Url } from '../../core/common.js'; -import { OAuth2 } from '../../oauth2/oauth2.js'; -import { Bm } from '../../browser/browser-msg.js'; import { Assert, AssertError } from '../../assert.js'; import { Api } from '../shared/api.js'; import { Catch } from '../../platform/catch.js'; @@ -30,14 +28,10 @@ export class ConfiguredIdpOAuth extends OAuth { acctEmail = acctEmail.toLowerCase(); const authRequest = this.newAuthRequest(acctEmail, this.OAUTH_REQUEST_SCOPES); const authUrl = this.apiOAuthCodeUrl(authConf, authRequest.expectedState, acctEmail); - // Added below logic because in service worker, it's not possible to access window object. - // Therefore need to retrieve screenDimensions when calling service worker and pass it to OAuth2 - const screenDimensions = Ui.getScreenDimensions(); - const authWindowResult = await OAuth2.webAuthFlow(authUrl, screenDimensions); const authRes = await this.getAuthRes({ acctEmail, expectedState: authRequest.expectedState, - authWindowResult, + authUrl, authConf, }); if (authRes.result === 'Success') { @@ -69,7 +63,7 @@ export class ConfiguredIdpOAuth extends OAuth { access_type: 'offline', prompt: 'login', state, - redirect_uri: authConf.oauth.redirectUrl, + redirect_uri: chrome.identity.getRedirectURL('oauth'), scope: this.OAUTH_REQUEST_SCOPES.join(' '), login_hint: acctEmail, }); @@ -79,24 +73,25 @@ export class ConfiguredIdpOAuth extends OAuth { private static async getAuthRes({ acctEmail, expectedState, - authWindowResult, + authUrl, authConf, }: { acctEmail: string; expectedState: string; - authWindowResult: Bm.AuthWindowResult; + authUrl: string; authConf: AuthenticationConfiguration; }): Promise { /* eslint-disable @typescript-eslint/naming-convention */ try { - if (!authWindowResult.url) { - return { acctEmail, result: 'Denied', error: 'Invalid response url', id_token: undefined }; + const redirectUri = await chrome.identity.launchWebAuthFlow({ url: authUrl, interactive: true }); + if (chrome.runtime.lastError || !redirectUri || redirectUri?.includes('access_denied')) { + return { acctEmail, result: 'Denied', error: `Failed to launch web auth flow`, id_token: undefined }; } - if (authWindowResult.error) { - return { acctEmail, result: 'Denied', error: authWindowResult.error, id_token: undefined }; + + if (!redirectUri) { + return { acctEmail, result: 'Denied', error: 'Invalid response url', id_token: undefined }; } - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const uncheckedUrlParams = Url.parse(['scope', 'code', 'state'], authWindowResult.url); + const uncheckedUrlParams = Url.parse(['scope', 'code', 'state'], redirectUri); const code = Assert.urlParamRequire.optionalString(uncheckedUrlParams, 'code'); const receivedState = Assert.urlParamRequire.string(uncheckedUrlParams, 'state'); if (!code) { @@ -141,8 +136,7 @@ export class ConfiguredIdpOAuth extends OAuth { grant_type: 'authorization_code', code, client_id: authConf.oauth.clientId, - client_secret: authConf.oauth.clientSecret, - redirect_uri: authConf.oauth.redirectUrl, + redirect_uri: chrome.identity.getRedirectURL('oauth'), }, dataType: 'JSON', /* eslint-enable @typescript-eslint/naming-convention */ diff --git a/extension/manifest.json b/extension/manifest.json index 910c2bfe4f0..34c0e03f66c 100644 --- a/extension/manifest.json +++ b/extension/manifest.json @@ -25,7 +25,7 @@ "64": "/img/logo/flowcrypt-logo-64-64.png", "128": "/img/logo/flowcrypt-logo-128-128.png" }, - "permissions": ["alarms", "storage", "tabs", "scripting", "unlimitedStorage"], + "permissions": ["alarms", "storage", "tabs", "scripting", "unlimitedStorage", "identity"], "host_permissions": [ "https://flowcrypt.com/*", "https://*.google.com/*", diff --git a/test/source/browser/browser-handle.ts b/test/source/browser/browser-handle.ts index 7effc970a79..fbb496f4cde 100644 --- a/test/source/browser/browser-handle.ts +++ b/test/source/browser/browser-handle.ts @@ -73,12 +73,13 @@ export class BrowserHandle { public newPageTriggeredBy = async (t: AvaContext, triggeringAction: () => Promise): Promise => { const page = (await this.doAwaitTriggeredPage(triggeringAction))!; const url = page.url(); + const parsedUrl = new URL(url); const controllablePage = new ControllablePage(t, page); try { await page.setViewport(this.viewport); this.pages.push(controllablePage); - if (url.includes(t.context.urls!.extensionId)) { + if (parsedUrl.pathname.includes(t.context.urls!.extensionId)) { await controllablePage.waitUntilViewLoaded(); } return controllablePage; diff --git a/test/source/mock/google/google-endpoints.ts b/test/source/mock/google/google-endpoints.ts index 84e417bba5a..86f4d9958f4 100644 --- a/test/source/mock/google/google-endpoints.ts +++ b/test/source/mock/google/google-endpoints.ts @@ -121,7 +121,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 } }, + { query: { client_id, response_type, access_type, state, scope, login_hint, proceed, redirect_uri } }, req ) => { if (isGet(req) && response_type === 'code' && access_type === 'offline' && state && scope) { @@ -138,7 +138,7 @@ export const getMockGoogleEndpoints = (oauth: OauthMock, config: GoogleConfig | if (!proceed) { return oauth.renderText('redirect with proceed=true to continue'); } - return oauth.successResult(parsePort(req), login_hint, state, scope); + return oauth.successResult(parsePort(req), login_hint, state, scope, redirect_uri); } } throw new HttpClientErr(`Method not implemented for ${req.url}: ${req.method}`); diff --git a/test/source/mock/lib/api.ts b/test/source/mock/lib/api.ts index f5292d9ba5e..c830bc20e63 100644 --- a/test/source/mock/lib/api.ts +++ b/test/source/mock/lib/api.ts @@ -300,8 +300,8 @@ export class Api { }; private throttledResponse = async (response: http2.Http2ServerResponse, data: Buffer) => { - // If google oauth2 login, then redirect to url - if (/^https:\/\/google\.localhost:[0-9]+\/robots\.txt/.test(data.toString())) { + // If google oauth2 or custom oauth login, then redirect to url + if (/^https:\/\/(google\.localhost:[0-9]+\/robots\.txt|[a-zA-Z0-9]+\.chromiumapp\.org)/.test(data.toString())) { response.writeHead(302, { Location: data.toString() }); // eslint-disable-line @typescript-eslint/naming-convention } else { const chunkSize = 100 * 1024; diff --git a/test/source/mock/lib/oauth.ts b/test/source/mock/lib/oauth.ts index 7010f52b370..a5f1103295f 100644 --- a/test/source/mock/lib/oauth.ts +++ b/test/source/mock/lib/oauth.ts @@ -25,7 +25,8 @@ export class OauthMock { return this.htmlPage(text, text); }; - public successResult = (port: string, acct: string, state: string, scope: string) => { + // eslint-disable-next-line @typescript-eslint/naming-convention + public successResult = (port: string, acct: string, state: string, scope: string, redirect_uri?: 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, '')}`; @@ -34,7 +35,7 @@ export class OauthMock { this.accessTokenByRefreshToken[refreshToken] = accessToken; this.acctByAccessToken[accessToken] = acct; this.scopesByAccessToken[accessToken] = `${this.scopesByAccessToken[accessToken] ?? ''} ${scope}`; - const url = new URL(`https://google.localhost:${port}/robots.txt`); + const url = new URL(redirect_uri ?? `https://google.localhost:${port}/robots.txt`); url.searchParams.set('code', authCode); url.searchParams.set('scope', scope); // return invalid state for test.invalid.csrf@gmail.com to check invalid csrf login diff --git a/test/source/tests/page-recipe/oauth-page-recipe.ts b/test/source/tests/page-recipe/oauth-page-recipe.ts index c8c3cef3bac..37c0227d5d9 100644 --- a/test/source/tests/page-recipe/oauth-page-recipe.ts +++ b/test/source/tests/page-recipe/oauth-page-recipe.ts @@ -41,7 +41,13 @@ export class OauthPageRecipe extends PageRecipe { public static customIdp = async (t: AvaContext, oauthPage: ControllablePage): Promise => { const mockOauthUrl = oauthPage.target.url(); - await oauthPage.target.goto(mockOauthUrl + '&proceed=true'); + try { + await oauthPage.target.goto(mockOauthUrl + '&proceed=true'); + } catch (e) { + if (!e.message.includes('Navigating frame was detached')) { + throw e; + } + } }; public static google = async ( diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 9ee048efb90..adb7f8688e0 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -2530,7 +2530,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== const oauthConfig = { clientId: OauthMock.customIDPClientId, clientSecret: OauthMock.customIDPClientSecret, - redirectUrl: `https://google.localhost:${port}/robots.txt`, + redirectUrl: `custom-redirect-url`, // This won't be used as we use our https://{id}.chromiumapp.org with chrome.identity.getRedirectURL authCodeUrl: `https://localhost:${port}/o/oauth2/auth`, tokensUrl: `https://localhost:${port}/token`, }; diff --git a/test/source/tests/tooling/browser-recipe.ts b/test/source/tests/tooling/browser-recipe.ts index a75e3a0cb92..1034e545fd3 100644 --- a/test/source/tests/tooling/browser-recipe.ts +++ b/test/source/tests/tooling/browser-recipe.ts @@ -50,9 +50,9 @@ export class BrowserRecipe { const customOAuthPopup = await browser.newPageTriggeredBy(t, () => settingsPage.waitAndRespondToModal('info', 'confirm', 'Google login succeeded. Now, please log in with your company credentials as well.') ); + // Wait until custom IDP authentication finished await OauthPageRecipe.customIdp(t, customOAuthPopup); } - // Wait until custom IDP authentication finished return settingsPage; }; diff --git a/tooling/build-types-and-manifests.ts b/tooling/build-types-and-manifests.ts index d7809d09b42..34acf9813da 100644 --- a/tooling/build-types-and-manifests.ts +++ b/tooling/build-types-and-manifests.ts @@ -86,7 +86,7 @@ addManifest('chrome-enterprise', manifest => { manifest.name = 'FlowCrypt for Enterprise'; manifest.description = 'FlowCrypt Chrome Extension for Enterprise clients (stable)'; // careful - changing this will likely cause all extensions to be disabled in their user's browsers - manifest.permissions = ['alarms', 'scripting', 'storage', 'tabs', 'unlimitedStorage']; + manifest.permissions = ['alarms', 'scripting', 'storage', 'tabs', 'unlimitedStorage', 'identity']; manifest.host_permissions = [ 'https://*.google.com/*', // customer enterprise environments use people,gmail,oauth2 subdomains of googleapis.com