mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-20 18:08:55 +00:00
fix(oauth): disable insecure Google Calendar web flow
This commit is contained in:
parent
60c0ba4e42
commit
cdff2b3907
11 changed files with 200 additions and 41 deletions
|
|
@ -347,21 +347,12 @@ export interface OAuthFlowConfig {
|
|||
*/
|
||||
iosClientId?: string;
|
||||
/**
|
||||
* Client ID for the web build (Google "Web application" client).
|
||||
* Overrides `clientId` in the browser (non-Electron, non-native).
|
||||
* The deployed origin's `/assets/oauth-callback.html` must be registered as an
|
||||
* Authorized redirect URI on the Google Cloud Console OAuth client, and the
|
||||
* deployed origin must be listed in Authorized JavaScript origins.
|
||||
* Client ID for the web build, for providers that support public browser
|
||||
* clients via Authorization Code + PKCE without a client secret.
|
||||
* Overrides `clientId` in the browser (non-Electron, non-native) and omits
|
||||
* `clientSecret`.
|
||||
*/
|
||||
webClientId?: string;
|
||||
/**
|
||||
* Client secret paired with `webClientId`, sent on the token exchange.
|
||||
* Required for IdPs (e.g. Google "Web application" clients) that classify
|
||||
* web clients as confidential and reject PKCE-only exchanges. Ships in
|
||||
* client-side JS like the auth code itself — only set this if you accept
|
||||
* that the secret is visible to anyone running the web build.
|
||||
*/
|
||||
webClientSecret?: string;
|
||||
scopes: string[];
|
||||
/** Additional query parameters to append to the authorization URL (e.g. access_type, prompt). */
|
||||
extraAuthParams?: Record<string, string>;
|
||||
|
|
|
|||
|
|
@ -16,6 +16,16 @@ beforeAll(async () => {
|
|||
});
|
||||
|
||||
describe('Google Calendar Plugin', () => {
|
||||
describe('OAuth config', () => {
|
||||
it('does not expose a browser web OAuth client', () => {
|
||||
const oauthField = definition.configFields.find((field) => field.key === 'oauth');
|
||||
const oauthConfig = oauthField?.oauthConfig as Record<string, unknown>;
|
||||
|
||||
expect(oauthConfig.webClientId).toBeUndefined();
|
||||
expect(oauthConfig.webClientSecret).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('fieldMappings', () => {
|
||||
const ctx = { issueId: 'event-1' };
|
||||
|
||||
|
|
@ -309,7 +319,7 @@ describe('Google Calendar Plugin', () => {
|
|||
expect(body.start.date).toBeDefined();
|
||||
expect(body.end.date).toBeDefined();
|
||||
expect(body.start.dateTime).toBeUndefined();
|
||||
expect(result.issueId).toBe('new-event-1');
|
||||
expect(result.issueId).toBe('test-cal::new-event-1');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -37,20 +37,6 @@ const MOBILE_CLIENT_ID =
|
|||
// Requires "Custom URI scheme" to be enabled in Google Cloud Console.
|
||||
const IOS_CLIENT_ID =
|
||||
'637968426975-ka1muro7mee1go0m7hhog49fm7svr4os.apps.googleusercontent.com';
|
||||
// Web OAuth client — used by the browser build (app.super-productivity.com).
|
||||
// Google classifies "Web application" clients as confidential, so the token
|
||||
// endpoint rejects PKCE-only exchanges and we must ship the client_secret
|
||||
// alongside the auth code. Unlike CLIENT_SECRET above (which is RFC 8252-style
|
||||
// public for desktop apps), Google's policy formally requires this secret to
|
||||
// be kept confidential — embedding it in client-side JS deviates from policy
|
||||
// and the credential MAY be revoked by Google if abused.
|
||||
// In practice the actual security boundary is PKCE + the registered redirect
|
||||
// URI: a stolen secret cannot be used to redeem codes for a different origin.
|
||||
// The deployed origin's /assets/oauth-callback.html must be registered as an
|
||||
// Authorized redirect URI, and the origin itself in Authorized JavaScript origins.
|
||||
const WEB_CLIENT_ID =
|
||||
'637968426975-6t6q988dk0417re8pcl06vq75obqq944.apps.googleusercontent.com';
|
||||
const WEB_CLIENT_SECRET = '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';
|
||||
|
||||
// --- Config ---
|
||||
|
||||
|
|
@ -286,8 +272,8 @@ PluginAPI.registerIssueProvider({
|
|||
clientSecret: CLIENT_SECRET,
|
||||
mobileClientId: MOBILE_CLIENT_ID,
|
||||
iosClientId: IOS_CLIENT_ID,
|
||||
webClientId: WEB_CLIENT_ID,
|
||||
webClientSecret: WEB_CLIENT_SECRET,
|
||||
// Intentionally no webClientId: durable Google Calendar web OAuth
|
||||
// requires a confidential client secret, which browser JS cannot keep.
|
||||
scopes: [CALENDAR_EVENTS_SCOPE, CALENDAR_READONLY_SCOPE],
|
||||
extraAuthParams: { access_type: 'offline', prompt: 'consent' },
|
||||
},
|
||||
|
|
|
|||
|
|
@ -95,7 +95,20 @@
|
|||
}
|
||||
@for (btn of oauthButtons; track btn.label) {
|
||||
<div class="action-row">
|
||||
@if (isOAuthConnected()) {
|
||||
@if (isOAuthUnavailableInWeb(btn.oauthConfig)) {
|
||||
<button
|
||||
mat-stroked-button
|
||||
type="button"
|
||||
disabled
|
||||
>
|
||||
<mat-icon>block</mat-icon>
|
||||
{{ btn.label }}
|
||||
</button>
|
||||
<span class="error-status">
|
||||
<mat-icon class="error-status-icon">info</mat-icon>
|
||||
{{ T.F.ISSUE.DIALOG.OAUTH_UNAVAILABLE_WEB | translate }}
|
||||
</span>
|
||||
} @else if (isOAuthConnected()) {
|
||||
<button
|
||||
mat-stroked-button
|
||||
type="button"
|
||||
|
|
|
|||
|
|
@ -60,6 +60,7 @@ import { PluginIssueProviderRegistryService } from '../../../plugins/issue-provi
|
|||
import { PluginBridgeService } from '../../../plugins/plugin-bridge.service';
|
||||
import { PluginHttpService } from '../../../plugins/issue-provider/plugin-http.service';
|
||||
import { OAuthFlowConfig } from '@super-productivity/plugin-api';
|
||||
import { IS_NATIVE_PLATFORM } from '../../../util/is-native-platform';
|
||||
import { TrelloAdditionalCfgComponent } from '../providers/trello/trello-view-components/trello_cfg/trello_additional_cfg.component';
|
||||
// ClickUp is now a plugin — no built-in config component needed
|
||||
import { NextcloudDeckAdditionalCfgComponent } from '../providers/nextcloud-deck/nextcloud-deck-additional-cfg.component';
|
||||
|
|
@ -350,6 +351,9 @@ export class DialogEditIssueProviderComponent {
|
|||
}
|
||||
|
||||
async connectOAuth(oauthConfig: OAuthFlowConfig): Promise<void> {
|
||||
if (this.isOAuthUnavailableInWeb(oauthConfig)) {
|
||||
return;
|
||||
}
|
||||
const pluginId = this._pluginRegistry.getProvider(this.issueProviderKey)?.pluginId;
|
||||
if (!pluginId) {
|
||||
return;
|
||||
|
|
@ -395,6 +399,10 @@ export class DialogEditIssueProviderComponent {
|
|||
protected readonly IS_ELECTRON = IS_ELECTRON;
|
||||
protected readonly IS_WEB_EXTENSION_REQUIRED_FOR_JIRA = IS_WEB_BROWSER;
|
||||
|
||||
protected isOAuthUnavailableInWeb(oauthConfig: OAuthFlowConfig): boolean {
|
||||
return !IS_ELECTRON && !IS_NATIVE_PLATFORM && !oauthConfig.webClientId;
|
||||
}
|
||||
|
||||
/**
|
||||
* @returns true if all fields loaded successfully, false if any failed
|
||||
*/
|
||||
|
|
|
|||
114
src/app/plugins/oauth/plugin-oauth-bridge.service.spec.ts
Normal file
114
src/app/plugins/oauth/plugin-oauth-bridge.service.spec.ts
Normal file
|
|
@ -0,0 +1,114 @@
|
|||
import { TestBed } from '@angular/core/testing';
|
||||
import { Subject } from 'rxjs';
|
||||
import type { OAuthFlowConfig } from '@super-productivity/plugin-api';
|
||||
import { PluginOAuthBridgeService } from './plugin-oauth-bridge.service';
|
||||
import { PluginOAuthService } from './plugin-oauth.service';
|
||||
|
||||
describe('PluginOAuthBridgeService', () => {
|
||||
let service: PluginOAuthBridgeService;
|
||||
let oauthService: jasmine.SpyObj<PluginOAuthService>;
|
||||
|
||||
const baseConfig: OAuthFlowConfig = {
|
||||
authUrl: 'https://accounts.google.com/o/oauth2/v2/auth',
|
||||
tokenUrl: 'https://oauth2.googleapis.com/token',
|
||||
clientId: 'desktop-client-id',
|
||||
clientSecret: 'desktop-client-secret',
|
||||
scopes: ['calendar.readonly'],
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
oauthService = jasmine.createSpyObj<PluginOAuthService>(
|
||||
'PluginOAuthService',
|
||||
[
|
||||
'validateOAuthConfig',
|
||||
'getRedirectUri',
|
||||
'buildAuthUrl',
|
||||
'waitForRedirectCode',
|
||||
'exchangeCodeForTokens',
|
||||
'storeTokens',
|
||||
'serializeTokens',
|
||||
'clearTokens',
|
||||
'hasTokens',
|
||||
'restoreTokens',
|
||||
'getValidToken',
|
||||
],
|
||||
{ tokenInvalidated$: new Subject<string>() },
|
||||
);
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
PluginOAuthBridgeService,
|
||||
{ provide: PluginOAuthService, useValue: oauthService },
|
||||
],
|
||||
});
|
||||
|
||||
service = TestBed.inject(PluginOAuthBridgeService);
|
||||
});
|
||||
|
||||
it('rejects browser OAuth when a plugin has no web client id', async () => {
|
||||
await expectAsync(
|
||||
service.startOAuthFlow('google-calendar', baseConfig),
|
||||
).toBeRejectedWithError(/not available in the web build/);
|
||||
|
||||
expect(oauthService.validateOAuthConfig).toHaveBeenCalledWith(baseConfig);
|
||||
expect(oauthService.getRedirectUri).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('uses a public web client id without carrying the desktop client secret', async () => {
|
||||
spyOn(window, 'open').and.returnValue({} as Window);
|
||||
oauthService.getRedirectUri.and.resolveTo(
|
||||
'https://app.super-productivity.com/assets/oauth-callback.html',
|
||||
);
|
||||
oauthService.buildAuthUrl.and.resolveTo({
|
||||
url: 'https://accounts.google.com/o/oauth2/v2/auth',
|
||||
codeVerifier: 'verifier',
|
||||
state: 'state',
|
||||
});
|
||||
oauthService.waitForRedirectCode.and.resolveTo('auth-code');
|
||||
oauthService.exchangeCodeForTokens.and.resolveTo({
|
||||
accessToken: 'access-token',
|
||||
refreshToken: 'refresh-token',
|
||||
expiresAt: Date.now() + 3600000,
|
||||
});
|
||||
oauthService.serializeTokens.and.returnValue(null);
|
||||
|
||||
await service.startOAuthFlow('pkce-web-provider', {
|
||||
...baseConfig,
|
||||
webClientId: 'web-client-id',
|
||||
});
|
||||
|
||||
const effectiveConfig = oauthService.buildAuthUrl.calls.mostRecent()
|
||||
.args[0] as OAuthFlowConfig;
|
||||
expect(effectiveConfig.clientId).toBe('web-client-id');
|
||||
expect(effectiveConfig.clientSecret).toBeUndefined();
|
||||
expect(oauthService.exchangeCodeForTokens).toHaveBeenCalledWith(
|
||||
jasmine.objectContaining({
|
||||
clientId: 'web-client-id',
|
||||
clientSecret: undefined,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('clears stale browser tokens for providers that are unavailable on web', async () => {
|
||||
oauthService.hasTokens.and.returnValue(true);
|
||||
|
||||
const hasTokens = await service.restoreAndCheckOAuthTokens(
|
||||
'google-calendar',
|
||||
baseConfig,
|
||||
);
|
||||
|
||||
expect(hasTokens).toBeFalse();
|
||||
expect(oauthService.clearTokens).toHaveBeenCalledWith('google-calendar');
|
||||
expect(oauthService.getValidToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not return stale browser tokens for providers that are unavailable on web', async () => {
|
||||
oauthService.hasTokens.and.returnValue(true);
|
||||
|
||||
const token = await service.getOAuthToken('google-calendar', baseConfig);
|
||||
|
||||
expect(token).toBeNull();
|
||||
expect(oauthService.clearTokens).toHaveBeenCalledWith('google-calendar');
|
||||
expect(oauthService.getValidToken).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -47,7 +47,7 @@ export class PluginOAuthBridgeService {
|
|||
// Pick the platform-specific client. The default `clientId` is the desktop
|
||||
// client (loopback redirect, used by Electron); other platforms override it.
|
||||
// - Android/iOS authenticate via app signing → no client secret
|
||||
// - Web uses an IdP-specific "Web application" client → may ship a secret
|
||||
// - Web can only use providers that support public browser clients via PKCE
|
||||
// Order matters: Android-WebView sets both IS_ANDROID_NATIVE and IS_NATIVE_PLATFORM,
|
||||
// so it lands in the Android branch (correct) and never reaches the web branch.
|
||||
const effectiveConfig = ((): OAuthFlowConfig => {
|
||||
|
|
@ -58,15 +58,16 @@ export class PluginOAuthBridgeService {
|
|||
return { ...config, clientId: config.iosClientId, clientSecret: undefined };
|
||||
}
|
||||
if (!IS_ELECTRON && !IS_NATIVE_PLATFORM) {
|
||||
if (!config.webClientId) {
|
||||
const webClientId = config.webClientId;
|
||||
if (!webClientId) {
|
||||
throw new Error(
|
||||
'OAuth: this plugin does not support the web build (no webClientId configured). Connect from the desktop or mobile app instead.',
|
||||
'OAuth: this plugin is not available in the web build. Connect from the desktop or mobile app instead.',
|
||||
);
|
||||
}
|
||||
return {
|
||||
...config,
|
||||
clientId: config.webClientId,
|
||||
clientSecret: config.webClientSecret,
|
||||
clientId: webClientId,
|
||||
clientSecret: undefined,
|
||||
};
|
||||
}
|
||||
return config;
|
||||
|
|
@ -108,20 +109,38 @@ export class PluginOAuthBridgeService {
|
|||
await this._clearPersistedOAuthTokens(pluginId);
|
||||
}
|
||||
|
||||
async restoreAndCheckOAuthTokens(pluginId: string): Promise<boolean> {
|
||||
async restoreAndCheckOAuthTokens(
|
||||
pluginId: string,
|
||||
config?: OAuthFlowConfig,
|
||||
): Promise<boolean> {
|
||||
if (this._isUnavailableInWeb(config)) {
|
||||
await this.clearOAuthTokens(pluginId);
|
||||
return false;
|
||||
}
|
||||
if (!this._pluginOAuthService.hasTokens(pluginId)) {
|
||||
await this._restoreOAuthTokens(pluginId);
|
||||
}
|
||||
return this._pluginOAuthService.hasTokens(pluginId);
|
||||
}
|
||||
|
||||
async getOAuthToken(pluginId: string): Promise<string | null> {
|
||||
async getOAuthToken(
|
||||
pluginId: string,
|
||||
config?: OAuthFlowConfig,
|
||||
): Promise<string | null> {
|
||||
if (this._isUnavailableInWeb(config)) {
|
||||
await this.clearOAuthTokens(pluginId);
|
||||
return null;
|
||||
}
|
||||
if (!this._pluginOAuthService.hasTokens(pluginId)) {
|
||||
await this._restoreOAuthTokens(pluginId);
|
||||
}
|
||||
return this._pluginOAuthService.getValidToken(pluginId);
|
||||
}
|
||||
|
||||
private _isUnavailableInWeb(config?: OAuthFlowConfig): boolean {
|
||||
return !!config && !IS_ELECTRON && !IS_NATIVE_PLATFORM && !config.webClientId;
|
||||
}
|
||||
|
||||
private _openOAuthWindow(url: string): void {
|
||||
if (IS_ELECTRON) {
|
||||
window.ea.pluginOAuthStart(url);
|
||||
|
|
|
|||
|
|
@ -4,5 +4,5 @@ export interface PluginOAuthTokens {
|
|||
expiresAt: number; // unix ms
|
||||
tokenUrl: string; // needed for refresh
|
||||
clientId: string; // needed for refresh
|
||||
clientSecret?: string; // needed for refresh (Google requires it)
|
||||
clientSecret?: string; // optional non-confidential installed-app secret for refresh
|
||||
}
|
||||
|
|
|
|||
|
|
@ -260,7 +260,10 @@ export class PluginBridgeService implements OnDestroy {
|
|||
startOAuthFlow: (config: OAuthFlowConfig): Promise<OAuthTokenResult> =>
|
||||
this._pluginOAuthBridge.startOAuthFlow(pluginId, config),
|
||||
getOAuthToken: (): Promise<string | null> =>
|
||||
this._pluginOAuthBridge.getOAuthToken(pluginId),
|
||||
this._pluginOAuthBridge.getOAuthToken(
|
||||
pluginId,
|
||||
this._getOAuthConfigForPlugin(pluginId),
|
||||
),
|
||||
clearOAuthToken: (): Promise<void> =>
|
||||
this._pluginOAuthBridge.clearOAuthTokens(pluginId),
|
||||
|
||||
|
|
@ -370,7 +373,20 @@ export class PluginBridgeService implements OnDestroy {
|
|||
}
|
||||
|
||||
async restoreAndCheckOAuthTokens(pluginId: string): Promise<boolean> {
|
||||
return this._pluginOAuthBridge.restoreAndCheckOAuthTokens(pluginId);
|
||||
return this._pluginOAuthBridge.restoreAndCheckOAuthTokens(
|
||||
pluginId,
|
||||
this._getOAuthConfigForPlugin(pluginId),
|
||||
);
|
||||
}
|
||||
|
||||
private _getOAuthConfigForPlugin(pluginId: string): OAuthFlowConfig | undefined {
|
||||
const registeredKey = this._pluginIssueProviderRegistry.getRegisteredKey(pluginId);
|
||||
if (!registeredKey) {
|
||||
return undefined;
|
||||
}
|
||||
return this._pluginIssueProviderRegistry
|
||||
.getConfigFields(registeredKey)
|
||||
.find((f) => f.type === 'oauthButton' && f.oauthConfig)?.oauthConfig;
|
||||
}
|
||||
|
||||
private async _downloadFile(filename: string, data: string): Promise<void> {
|
||||
|
|
|
|||
|
|
@ -488,6 +488,7 @@ const T = {
|
|||
LOAD_OPTIONS: 'F.ISSUE.DIALOG.LOAD_OPTIONS',
|
||||
LOAD_OPTIONS_FAILED: 'F.ISSUE.DIALOG.LOAD_OPTIONS_FAILED',
|
||||
LOADING_OPTIONS: 'F.ISSUE.DIALOG.LOADING_OPTIONS',
|
||||
OAUTH_UNAVAILABLE_WEB: 'F.ISSUE.DIALOG.OAUTH_UNAVAILABLE_WEB',
|
||||
OPTIONS_LOADED: 'F.ISSUE.DIALOG.OPTIONS_LOADED',
|
||||
RELOAD_OPTIONS: 'F.ISSUE.DIALOG.RELOAD_OPTIONS',
|
||||
SETUP_TITLE: 'F.ISSUE.DIALOG.SETUP_TITLE',
|
||||
|
|
|
|||
|
|
@ -488,6 +488,7 @@
|
|||
"LOAD_OPTIONS": "Load Options",
|
||||
"LOAD_OPTIONS_FAILED": "Failed to load options",
|
||||
"LOADING_OPTIONS": "Loading options...",
|
||||
"OAUTH_UNAVAILABLE_WEB": "Available in the desktop and mobile apps. Synced settings stay saved, but live data cannot be loaded in the web app.",
|
||||
"OPTIONS_LOADED": "Options loaded",
|
||||
"RELOAD_OPTIONS": "Reload Options",
|
||||
"SETUP_TITLE": "Setup {{title}} Config",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue