Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,6 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
const authResult = await BrowserMsg.send.bg.await.reconnectAcctAuthPopup({
acctEmail: this.view.acctEmail,
scopes: GoogleOAuth.defaultScopes('contacts'),
screenDimensions: Ui.getScreenDimensions(),
});
if (authResult.result === 'Success') {
this.googleContactsSearchEnabled = true;
Expand Down
8 changes: 0 additions & 8 deletions extension/chrome/elements/oauth2.htm

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export class ConfiguredIdpOAuth extends OAuth {
refreshToken,
client_id: authConf.oauth.clientId,

redirect_uri: chrome.identity.getRedirectURL('oauth'),
redirect_uri: this.getRedirectUri(),
},
dataType: 'JSON',
/* eslint-enable @typescript-eslint/naming-convention */
Expand All @@ -121,7 +121,7 @@ export class ConfiguredIdpOAuth extends OAuth {
prompt: 'login',
state,

redirect_uri: chrome.identity.getRedirectURL('oauth'),
redirect_uri: this.getRedirectUri(),
scope: this.OAUTH_REQUEST_SCOPES.join(' '),
login_hint: acctEmail,
});
Expand Down Expand Up @@ -207,7 +207,7 @@ export class ConfiguredIdpOAuth extends OAuth {
code,
client_id: authConf.oauth.clientId,

redirect_uri: chrome.identity.getRedirectURL('oauth'),
redirect_uri: this.getRedirectUri(),
},
dataType: 'JSON',
/* eslint-enable @typescript-eslint/naming-convention */
Expand Down
55 changes: 35 additions & 20 deletions extension/js/common/api/authentication/generic/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,27 +37,42 @@ export type AuthorizationHeader = {

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" ...
},
};
public static OAUTH_REQUEST_SCOPES = ['offline_access', 'openid', 'profile', 'email'];

public static get GOOGLE_OAUTH_CONFIG() {
return {
client_id: '717284730244-5oejn54f10gnrektjdc4fv4rbic1bj1p.apps.googleusercontent.com',
client_secret: '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',
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" ...
},
};
}

public static getRedirectUri(): string {
if (!chrome?.identity?.getRedirectURL) {
throw new Error('chrome.identity.getRedirectURL is not available in this context');
}
const redirectUri = chrome.identity.getRedirectURL('oauth');
if (navigator.userAgent.includes('Firefox')) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use existing Catch.isFirefox() check

const url = new URL(redirectUri);
const subdomain = url.hostname.split('.')[0];
return `http://127.0.0.1/mozoauth2/${subdomain}`;
}
return redirectUri;
}
/* eslint-enable @typescript-eslint/naming-convention */
/**
* Happens on enterprise builds
Expand Down
49 changes: 16 additions & 33 deletions extension/js/common/api/authentication/google/google-oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ import { Url } from '../../../core/common.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';

import { Bm, ScreenDimensions } from '../../../browser/browser-msg.js';
import { InMemoryStoreKeys } from '../../../core/const.js';
import { OAuth2 } from '../../../oauth2/oauth2.js';
import { CatchHelper } from '../../../platform/catch-helper.js';
import { AcctStore, AcctStoreDict } from '../../../platform/store/acct-store.js';
import { InMemoryStore } from '../../../platform/store/in-memory-store.js';
Expand All @@ -18,7 +15,6 @@ import { AuthorizationHeader, AuthReq, AuthRes, OAuth, OAuthTokensResponse } fro
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-next-line @typescript-eslint/naming-convention
Expand Down Expand Up @@ -110,17 +106,7 @@ export class GoogleOAuth extends OAuth {
}
}

public static async newAuthPopup({
acctEmail,
scopes,
save,
screenDimensions,
}: {
acctEmail?: string;
scopes?: string[];
save?: boolean;
screenDimensions?: ScreenDimensions;
}): Promise<AuthRes> {
public static async newAuthPopup({ acctEmail, scopes, save }: { acctEmail?: string; scopes?: string[]; save?: boolean }): Promise<AuthRes> {
if (acctEmail) {
acctEmail = acctEmail.toLowerCase();
}
Expand All @@ -133,18 +119,12 @@ export class GoogleOAuth extends OAuth {
}
const authRequest = GoogleOAuth.newAuthRequest(acctEmail, scopes);
const authUrl = GoogleOAuth.apiGoogleAuthCodeUrl(authRequest);
// 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
if (!screenDimensions) {
screenDimensions = Ui.getScreenDimensions();
}
const authWindowResult = await OAuth2.webAuthFlow(authUrl, screenDimensions);
const authRes = await GoogleOAuth.getAuthRes({
acctEmail,
save,
requestedScopes: scopes,
expectedState: authRequest.expectedState,
authWindowResult,
authUrl,
});
if (authRes.result === 'Success') {
if (!authRes.id_token) {
Expand Down Expand Up @@ -211,24 +191,27 @@ export class GoogleOAuth extends OAuth {
save,
requestedScopes,
expectedState,
authWindowResult,
authUrl,
}: {
acctEmail?: string;
save: boolean;
requestedScopes: string[];
expectedState: string;
authWindowResult: Bm.AuthWindowResult;
authUrl: string;
}): Promise<AuthRes> {
/* 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 };
const redirectUri = await chrome.identity.launchWebAuthFlow({ url: authUrl, interactive: true });
Comment thread
martgil marked this conversation as resolved.
if (chrome.runtime.lastError || !redirectUri || redirectUri?.includes('access_denied')) {
const errorMsg = chrome.runtime.lastError?.message || 'access_denied';
const normalizedErrorMsg = errorMsg.toLowerCase();
const userCancelled = ['user', 'cancel', 'deny', 'denied', 'close'].some(keyword => normalizedErrorMsg.includes(keyword));
if (userCancelled) {
return { acctEmail, result: 'Closed', error: errorMsg, id_token: undefined };
}
return { acctEmail, result: 'Denied', error: `Failed to launch web auth flow: ${errorMsg}`, id_token: undefined };
Comment thread
martgil marked this conversation as resolved.
}

const uncheckedUrlParams = Url.parse(['scope', 'code', 'state'], authWindowResult.url);
const uncheckedUrlParams = Url.parse(['scope', 'code', 'state'], redirectUri);
const allowedScopes = Assert.urlParamRequire.string(uncheckedUrlParams, 'scope');
const code = Assert.urlParamRequire.optionalString(uncheckedUrlParams, 'code');
const receivedState = Assert.urlParamRequire.string(uncheckedUrlParams, 'state');
Expand Down Expand Up @@ -277,7 +260,7 @@ export class GoogleOAuth extends OAuth {
access_type: 'offline',
prompt: 'consent',
state: authReq.expectedState,
redirect_uri: this.GOOGLE_OAUTH_CONFIG.redirect_uri,
redirect_uri: this.getRedirectUri(),
scope: (authReq.scopes || []).join(' '),
login_hint: authReq.acctEmail,
});
Expand Down Expand Up @@ -309,7 +292,7 @@ export class GoogleOAuth extends OAuth {
code,
client_id: this.GOOGLE_OAUTH_CONFIG.client_id,
client_secret: this.GOOGLE_OAUTH_CONFIG.client_secret,
redirect_uri: this.GOOGLE_OAUTH_CONFIG.redirect_uri,
redirect_uri: this.getRedirectUri(),
}),
/* eslint-enable @typescript-eslint/naming-convention */
method: 'POST',
Expand Down
2 changes: 1 addition & 1 deletion extension/js/common/browser/browser-msg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export namespace Bm {
};
export type InMemoryStoreGet = { acctEmail: string; key: string };
export type GetApiAuthorization = { idToken: string };
export type ReconnectAcctAuthPopup = { acctEmail: string; scopes?: string[]; screenDimensions: ScreenDimensions };
export type ReconnectAcctAuthPopup = { acctEmail: string; scopes?: string[] };
export type ReconnectCustomIDPAcctAuthPopup = { acctEmail: string };
export type Ajax = { req: ApiAjax; resFmt: ResFmt };
export type AjaxProgress = { operationId: string; percent?: number; loaded: number; total: number; expectedTransferSize: number };
Expand Down
1 change: 0 additions & 1 deletion extension/js/common/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ export class Notifications {
private reconnectAcctAuthPopup = async (acctEmail: string) => {
const authRes = await BrowserMsg.send.bg.await.reconnectAcctAuthPopup({
acctEmail,
screenDimensions: Ui.getScreenDimensions(),
});
if (authRes.result === 'Success') {
this.show(`Connected successfully. You may need to reload the tab. <a href="#" class="close">Close</a>`);
Expand Down
53 changes: 0 additions & 53 deletions extension/js/common/oauth2/oauth2.ts

This file was deleted.

4 changes: 0 additions & 4 deletions extension/js/common/oauth2/oauth2_finish.ts

This file was deleted.

11 changes: 0 additions & 11 deletions extension/js/common/oauth2/oauth2_inject.ts

This file was deleted.

6 changes: 0 additions & 6 deletions extension/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@
"/lib/emailjs/emailjs-mime-parser.js",
"/js/content_scripts/webmail_bundle.js"
]
},
{
"matches": ["https://www.google.com/robots.txt*"],
"js": ["/js/common/oauth2/oauth2_inject.js"],
"run_at": "document_start"
}
],
"background": {
Expand Down Expand Up @@ -90,7 +85,6 @@
"/chrome/elements/add_pubkey.htm",
"/chrome/elements/pgp_pubkey.htm",
"/chrome/elements/backup.htm",
"/chrome/elements/oauth2.htm",
"/js/common/core/feature-config-injector.js"
],
"matches": ["https://mail.google.com/*", "https://accounts.google.com/*", "https://www.google.com/*"]
Expand Down
4 changes: 2 additions & 2 deletions test/source/mock/google/google-endpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(parsePort(req), login_hint, state, scope);
return oauth.successResult(login_hint, state, scope, redirect_uri);
}
} 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, redirect_uri);
return oauth.successResult(login_hint, state, scope, redirect_uri);
}
}
throw new HttpClientErr(`Method not implemented for ${req.url}: ${req.method}`);
Expand Down
2 changes: 1 addition & 1 deletion test/source/mock/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ export class Api<REQ, RES> {

private throttledResponse = async (response: http2.Http2ServerResponse, data: Buffer) => {
// 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())) {
if (/^(https:\/\/[a-zA-Z0-9-]+\.(chromiumapp\.org|extensions\.mozilla\.org)|http:\/\/127\.0\.0\.1\/mozoauth2\/)/.test(data.toString())) {
response.writeHead(302, { Location: data.toString() }); // eslint-disable-line @typescript-eslint/naming-convention
} else {
const chunkSize = 100 * 1024;
Expand Down
4 changes: 2 additions & 2 deletions test/source/mock/lib/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class OauthMock {
};

// eslint-disable-next-line @typescript-eslint/naming-convention
public successResult = (port: string, acct: string, state: string, scope: string, redirect_uri?: string) => {
public successResult = (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, '')}`;
Expand All @@ -46,7 +46,7 @@ export class OauthMock {
this.accessTokenByRefreshToken[refreshToken] = accessToken;
this.acctByAccessToken[accessToken] = acct;
this.scopesByAccessToken[accessToken] = `${this.scopesByAccessToken[accessToken] ?? ''} ${scope}`;
const url = new URL(redirect_uri ?? `https://google.localhost:${port}/robots.txt`);
const url = new URL(redirect_uri);
url.searchParams.set('code', authCode);
url.searchParams.set('scope', scope);
// return invalid state for test.invalid.csrf@gmail.com to check invalid csrf login
Expand Down
3 changes: 3 additions & 0 deletions test/source/tests/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ export const defineSetupTests = (testVariant: TestVariant, testWithBrowser: Test
test(
'settings > login > close oauth window > close popup',
testWithBrowser(async (t, browser) => {
t.context.mockApi!.configProvider = new ConfigurationProvider({
attester: { pubkeyLookup: {} },
});
const settingsPage = await BrowserRecipe.openSettingsLoginButCloseOauthWindowBeforeGrantingPermission(
t,
browser,
Expand Down
Loading
Loading