From 92c9933cb56dbd0af3fafbeaba8d282034d12a51 Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Fri, 1 May 2026 17:44:58 +1000 Subject: [PATCH 1/7] feat: apply hannes' poc for oauth authorization code flow based on poc patches in databiosphere/data-browser#4793. url hardcoded as in the original; subsequent commit makes it configurable. Co-authored-by: hannes-ucsc Co-Authored-By: Claude Opus 4.7 (1M context) --- src/google/service.ts | 67 ++++++++++++++++++++++++++----------------- src/google/types.ts | 12 ++++++++ 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/google/service.ts b/src/google/service.ts index 3aaa07e2..38d99d5d 100644 --- a/src/google/service.ts +++ b/src/google/service.ts @@ -8,7 +8,12 @@ import { resetCredentialsState } from "../auth/dispatch/credentials"; import { resetTokenState, updateToken } from "../auth/dispatch/token"; import { AUTHENTICATION_STATUS } from "../auth/types/authentication"; import { OAuthProvider } from "../config/entities"; -import { GoogleProfile, SessionDispatch, TokenSetParameters } from "./types"; +import type { + CodeResponse, + GoogleProfile, + SessionDispatch, + TokenSetParameters, +} from "./types"; import { fetchProfile, getAuthenticationRequestOptions } from "./utils/auth"; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- TODO see https://github.com/clevercanary/data-browser/issues/544. @@ -30,37 +35,45 @@ export const service = { "authDispatch" | "authenticationDispatch" | "tokenDispatch" >, ): void => { - const client = google.accounts.oauth2.initTokenClient({ - callback: (response: TokenSetParameters) => { + const client = google.accounts.oauth2.initCodeClient({ + callback: (response: CodeResponse) => { const { id, profile, userinfo } = provider; - const { access_token: token } = response; - dispatch.authDispatch?.(requestAuth()); - dispatch.authenticationDispatch?.(requestAuthentication()); - dispatch.tokenDispatch?.(updateToken({ providerId: id, token })); - fetchProfile(userinfo, getAuthenticationRequestOptions(token), { - onError: () => { - dispatch.authDispatch?.(resetAuthState()); - dispatch.authenticationDispatch?.( - updateAuthentication({ - profile: undefined, - status: AUTHENTICATION_STATUS.SETTLED, - }), - ); - dispatch.tokenDispatch?.(resetTokenState()); - }, - onSuccess: (r: GoogleProfile) => - dispatch.authenticationDispatch?.( - updateAuthentication({ - profile: profile(r), - status: AUTHENTICATION_STATUS.PENDING, // Authentication is pending until session controller is resolved. - }), - ), - }); + fetch("https://service.hannes2.anvil.gi.ucsc.edu/user/authorize", { + body: JSON.stringify(response), + headers: { "Content-Type": "application/json" }, + method: "POST", + }) + .then((r) => r.json()) + .then((tokens: TokenSetParameters) => { + const { access_token: token } = tokens; + dispatch.authDispatch?.(requestAuth()); + dispatch.authenticationDispatch?.(requestAuthentication()); + dispatch.tokenDispatch?.(updateToken({ providerId: id, token })); + fetchProfile(userinfo, getAuthenticationRequestOptions(token), { + onError: () => { + dispatch.authDispatch?.(resetAuthState()); + dispatch.authenticationDispatch?.( + updateAuthentication({ + profile: undefined, + status: AUTHENTICATION_STATUS.SETTLED, + }), + ); + dispatch.tokenDispatch?.(resetTokenState()); + }, + onSuccess: (r: GoogleProfile) => + dispatch.authenticationDispatch?.( + updateAuthentication({ + profile: profile(r), + status: AUTHENTICATION_STATUS.PENDING, // Authentication is pending until session controller is resolved. + }), + ), + }); + }); }, client_id: provider.clientId, scope: provider.authorization.params.scope, }); - client.requestAccessToken(); + client.requestCode(); }, /** * Logout and clear all auth state. diff --git a/src/google/types.ts b/src/google/types.ts index 8aedc611..dba81b68 100644 --- a/src/google/types.ts +++ b/src/google/types.ts @@ -14,6 +14,18 @@ import { TokenState, } from "../auth/types/token"; +/** + * Authorization code response from Google OAuth. + */ +export interface CodeResponse { + code: string; + error: string; + error_description: string; + error_uri: string; + scope: string; + state: string; +} + /** * Google Sign-In authentication provider props. */ From d7866d20a7e675d58a728e0af079403ba5187bb8 Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Fri, 1 May 2026 17:56:16 +1000 Subject: [PATCH 2/7] feat: make oauth authorize url configurable on oauthprovider introduce optional `authorize` field on oauthprovider; service.login uses initcodeclient + requestcode when set and falls back to the existing inittokenclient + requestaccesstoken implicit flow when unset. add unit tests covering both branches. refs databiosphere/data-browser#4793 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/config/entities.ts | 1 + src/google/service.ts | 90 +++++++++++++++++++++---------------- tests/googleService.test.ts | 74 ++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 39 deletions(-) create mode 100644 tests/googleService.test.ts diff --git a/src/config/entities.ts b/src/config/entities.ts index efab4e06..e13f0687 100644 --- a/src/config/entities.ts +++ b/src/config/entities.ts @@ -285,6 +285,7 @@ export interface ListViewConfig { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Use of `any` is intentional to allow for flexibility in the model. export interface OAuthProvider

{ authorization: { params: { scope: string } }; + authorize?: string; // Backend authorize endpoint that exchanges an OAuth authorization code for a token set. When set, the authorization code flow is used; otherwise the implicit token flow is used. clientId: string; icon: ReactNode; id: ProviderId; diff --git a/src/google/service.ts b/src/google/service.ts index 38d99d5d..baf4770e 100644 --- a/src/google/service.ts +++ b/src/google/service.ts @@ -35,45 +35,57 @@ export const service = { "authDispatch" | "authenticationDispatch" | "tokenDispatch" >, ): void => { - const client = google.accounts.oauth2.initCodeClient({ - callback: (response: CodeResponse) => { - const { id, profile, userinfo } = provider; - fetch("https://service.hannes2.anvil.gi.ucsc.edu/user/authorize", { - body: JSON.stringify(response), - headers: { "Content-Type": "application/json" }, - method: "POST", - }) - .then((r) => r.json()) - .then((tokens: TokenSetParameters) => { - const { access_token: token } = tokens; - dispatch.authDispatch?.(requestAuth()); - dispatch.authenticationDispatch?.(requestAuthentication()); - dispatch.tokenDispatch?.(updateToken({ providerId: id, token })); - fetchProfile(userinfo, getAuthenticationRequestOptions(token), { - onError: () => { - dispatch.authDispatch?.(resetAuthState()); - dispatch.authenticationDispatch?.( - updateAuthentication({ - profile: undefined, - status: AUTHENTICATION_STATUS.SETTLED, - }), - ); - dispatch.tokenDispatch?.(resetTokenState()); - }, - onSuccess: (r: GoogleProfile) => - dispatch.authenticationDispatch?.( - updateAuthentication({ - profile: profile(r), - status: AUTHENTICATION_STATUS.PENDING, // Authentication is pending until session controller is resolved. - }), - ), - }); - }); - }, - client_id: provider.clientId, - scope: provider.authorization.params.scope, - }); - client.requestCode(); + const { authorize, id, profile, userinfo } = provider; + const onAccessToken = (token: string): void => { + dispatch.authDispatch?.(requestAuth()); + dispatch.authenticationDispatch?.(requestAuthentication()); + dispatch.tokenDispatch?.(updateToken({ providerId: id, token })); + fetchProfile(userinfo, getAuthenticationRequestOptions(token), { + onError: () => { + dispatch.authDispatch?.(resetAuthState()); + dispatch.authenticationDispatch?.( + updateAuthentication({ + profile: undefined, + status: AUTHENTICATION_STATUS.SETTLED, + }), + ); + dispatch.tokenDispatch?.(resetTokenState()); + }, + onSuccess: (r: GoogleProfile) => + dispatch.authenticationDispatch?.( + updateAuthentication({ + profile: profile(r), + status: AUTHENTICATION_STATUS.PENDING, // Authentication is pending until session controller is resolved. + }), + ), + }); + }; + if (authorize) { + const client = google.accounts.oauth2.initCodeClient({ + callback: (response: CodeResponse) => { + fetch(authorize, { + body: JSON.stringify(response), + headers: { "Content-Type": "application/json" }, + method: "POST", + }) + .then((r) => r.json()) + .then((tokens: TokenSetParameters) => + onAccessToken(tokens.access_token), + ); + }, + client_id: provider.clientId, + scope: provider.authorization.params.scope, + }); + client.requestCode(); + } else { + const client = google.accounts.oauth2.initTokenClient({ + callback: (response: TokenSetParameters) => + onAccessToken(response.access_token), + client_id: provider.clientId, + scope: provider.authorization.params.scope, + }); + client.requestAccessToken(); + } }, /** * Logout and clear all auth state. diff --git a/tests/googleService.test.ts b/tests/googleService.test.ts new file mode 100644 index 00000000..456ca321 --- /dev/null +++ b/tests/googleService.test.ts @@ -0,0 +1,74 @@ +import { jest } from "@jest/globals"; +import type { OAuthProvider } from "../src/config/entities"; +import { service } from "../src/google/service"; +import type { SessionDispatch } from "../src/google/types"; + +const PROVIDER_BASE: OAuthProvider = { + authorization: { params: { scope: "openid email profile" } }, + clientId: "test-client-id", + icon: null, + id: "google", + name: "Google", + profile: (p) => p, + userinfo: "https://example.com/userinfo", +}; + +const DISPATCH_NO_OPS: Pick< + SessionDispatch, + "authDispatch" | "authenticationDispatch" | "tokenDispatch" +> = { + authDispatch: jest.fn(), + authenticationDispatch: jest.fn(), + tokenDispatch: jest.fn(), +}; + +describe("service.login (Google)", () => { + let initCodeClient: jest.Mock; + let initTokenClient: jest.Mock; + let requestCode: jest.Mock; + let requestAccessToken: jest.Mock; + + beforeEach(() => { + requestCode = jest.fn(); + requestAccessToken = jest.fn(); + initCodeClient = jest.fn(() => ({ requestCode })); + initTokenClient = jest.fn(() => ({ requestAccessToken })); + (globalThis as unknown as { google: unknown }).google = { + accounts: { oauth2: { initCodeClient, initTokenClient } }, + }; + }); + + afterEach(() => { + delete (globalThis as unknown as { google?: unknown }).google; + }); + + it("uses the authorization code flow when provider.authorize is set", () => { + const provider: OAuthProvider = { + ...PROVIDER_BASE, + authorize: "https://service.example.com/user/authorize", + }; + + service.login(provider, DISPATCH_NO_OPS); + + expect(initCodeClient).toHaveBeenCalledTimes(1); + expect(requestCode).toHaveBeenCalledTimes(1); + expect(initTokenClient).not.toHaveBeenCalled(); + expect(requestAccessToken).not.toHaveBeenCalled(); + + const config = initCodeClient.mock.calls[0]?.[0] as { + client_id: string; + scope: string; + }; + expect(config.client_id).toBe(PROVIDER_BASE.clientId); + expect(config.scope).toBe(PROVIDER_BASE.authorization.params.scope); + }); + + it("falls back to the implicit token flow when provider.authorize is unset", () => { + service.login(PROVIDER_BASE, DISPATCH_NO_OPS); + + expect(initTokenClient).toHaveBeenCalledTimes(1); + expect(requestAccessToken).toHaveBeenCalledTimes(1); + expect(initCodeClient).not.toHaveBeenCalled(); + expect(requestCode).not.toHaveBeenCalled(); + }); +}); From dad07c150a72ceed443ed6e12e054dceb61d11c3 Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Fri, 1 May 2026 20:27:18 +1000 Subject: [PATCH 3/7] fix: handle authorize fetch failures and missing access tokens (#906) per copilot review on #905: - catch network errors, non-ok responses, and missing access_token in the authorize exchange; reset auth/token state on failure so login no longer hangs - extract resetSession helper now that the reset is reused by both the fetchprofile error path and the new authorize catch - add tests covering successful exchange (POST + updateToken dispatch), non-ok response, and missing access_token Co-Authored-By: Claude Opus 4.7 (1M context) --- src/google/service.ts | 38 +++++++---- tests/googleService.test.ts | 127 +++++++++++++++++++++++++++++++++--- 2 files changed, 142 insertions(+), 23 deletions(-) diff --git a/src/google/service.ts b/src/google/service.ts index baf4770e..5de5613d 100644 --- a/src/google/service.ts +++ b/src/google/service.ts @@ -36,21 +36,22 @@ export const service = { >, ): void => { const { authorize, id, profile, userinfo } = provider; + const resetSession = (): void => { + dispatch.authDispatch?.(resetAuthState()); + dispatch.authenticationDispatch?.( + updateAuthentication({ + profile: undefined, + status: AUTHENTICATION_STATUS.SETTLED, + }), + ); + dispatch.tokenDispatch?.(resetTokenState()); + }; const onAccessToken = (token: string): void => { dispatch.authDispatch?.(requestAuth()); dispatch.authenticationDispatch?.(requestAuthentication()); dispatch.tokenDispatch?.(updateToken({ providerId: id, token })); fetchProfile(userinfo, getAuthenticationRequestOptions(token), { - onError: () => { - dispatch.authDispatch?.(resetAuthState()); - dispatch.authenticationDispatch?.( - updateAuthentication({ - profile: undefined, - status: AUTHENTICATION_STATUS.SETTLED, - }), - ); - dispatch.tokenDispatch?.(resetTokenState()); - }, + onError: resetSession, onSuccess: (r: GoogleProfile) => dispatch.authenticationDispatch?.( updateAuthentication({ @@ -68,10 +69,19 @@ export const service = { headers: { "Content-Type": "application/json" }, method: "POST", }) - .then((r) => r.json()) - .then((tokens: TokenSetParameters) => - onAccessToken(tokens.access_token), - ); + .then((r) => { + if (!r.ok) { + throw new Error(`authorize request failed (${r.status})`); + } + return r.json(); + }) + .then((tokens: TokenSetParameters) => { + if (!tokens?.access_token) { + throw new Error("authorize response missing access_token"); + } + onAccessToken(tokens.access_token); + }) + .catch(resetSession); }, client_id: provider.clientId, scope: provider.authorization.params.scope, diff --git a/tests/googleService.test.ts b/tests/googleService.test.ts index 456ca321..67905599 100644 --- a/tests/googleService.test.ts +++ b/tests/googleService.test.ts @@ -1,7 +1,8 @@ import { jest } from "@jest/globals"; +import { resetTokenState, updateToken } from "../src/auth/dispatch/token"; import type { OAuthProvider } from "../src/config/entities"; import { service } from "../src/google/service"; -import type { SessionDispatch } from "../src/google/types"; +import type { CodeResponse, SessionDispatch } from "../src/google/types"; const PROVIDER_BASE: OAuthProvider = { authorization: { params: { scope: "openid email profile" } }, @@ -13,13 +14,26 @@ const PROVIDER_BASE: OAuthProvider = { userinfo: "https://example.com/userinfo", }; -const DISPATCH_NO_OPS: Pick< +const AUTHORIZE_URL = "https://service.example.com/user/authorize"; + +const CODE_RESPONSE: CodeResponse = { + code: "test-code", + error: "", + error_description: "", + error_uri: "", + scope: "", + state: "", +}; + +type LoginDispatch = Pick< SessionDispatch, "authDispatch" | "authenticationDispatch" | "tokenDispatch" -> = { - authDispatch: jest.fn(), - authenticationDispatch: jest.fn(), - tokenDispatch: jest.fn(), +>; + +const flushPromises = async (): Promise => { + for (let i = 0; i < 5; i++) { + await Promise.resolve(); + } }; describe("service.login (Google)", () => { @@ -27,12 +41,18 @@ describe("service.login (Google)", () => { let initTokenClient: jest.Mock; let requestCode: jest.Mock; let requestAccessToken: jest.Mock; + let dispatch: LoginDispatch; beforeEach(() => { requestCode = jest.fn(); requestAccessToken = jest.fn(); initCodeClient = jest.fn(() => ({ requestCode })); initTokenClient = jest.fn(() => ({ requestAccessToken })); + dispatch = { + authDispatch: jest.fn(), + authenticationDispatch: jest.fn(), + tokenDispatch: jest.fn(), + }; (globalThis as unknown as { google: unknown }).google = { accounts: { oauth2: { initCodeClient, initTokenClient } }, }; @@ -40,15 +60,16 @@ describe("service.login (Google)", () => { afterEach(() => { delete (globalThis as unknown as { google?: unknown }).google; + delete (globalThis as unknown as { fetch?: unknown }).fetch; }); it("uses the authorization code flow when provider.authorize is set", () => { const provider: OAuthProvider = { ...PROVIDER_BASE, - authorize: "https://service.example.com/user/authorize", + authorize: AUTHORIZE_URL, }; - service.login(provider, DISPATCH_NO_OPS); + service.login(provider, dispatch); expect(initCodeClient).toHaveBeenCalledTimes(1); expect(requestCode).toHaveBeenCalledTimes(1); @@ -64,11 +85,99 @@ describe("service.login (Google)", () => { }); it("falls back to the implicit token flow when provider.authorize is unset", () => { - service.login(PROVIDER_BASE, DISPATCH_NO_OPS); + service.login(PROVIDER_BASE, dispatch); expect(initTokenClient).toHaveBeenCalledTimes(1); expect(requestAccessToken).toHaveBeenCalledTimes(1); expect(initCodeClient).not.toHaveBeenCalled(); expect(requestCode).not.toHaveBeenCalled(); }); + + it("exchanges the authorization code at provider.authorize and dispatches updateToken", async () => { + const fetchMock = jest.fn(() => + Promise.resolve({ + json: () => Promise.resolve({ access_token: "fake-access-token" }), + ok: true, + } as Response), + ); + (globalThis as unknown as { fetch: typeof fetch }).fetch = + fetchMock as unknown as typeof fetch; + + const provider: OAuthProvider = { + ...PROVIDER_BASE, + authorize: AUTHORIZE_URL, + }; + service.login(provider, dispatch); + + const config = initCodeClient.mock.calls[0]?.[0] as { + callback: (response: CodeResponse) => void; + }; + config.callback(CODE_RESPONSE); + await flushPromises(); + + expect(fetchMock).toHaveBeenCalledWith(AUTHORIZE_URL, { + body: JSON.stringify(CODE_RESPONSE), + headers: { "Content-Type": "application/json" }, + method: "POST", + }); + expect(dispatch.tokenDispatch).toHaveBeenCalledWith( + updateToken({ providerId: "google", token: "fake-access-token" }), + ); + }); + + it("resets session state when the authorize request returns a non-ok response", async () => { + const fetchMock = jest.fn(() => + Promise.resolve({ + json: () => Promise.resolve({}), + ok: false, + status: 500, + } as Response), + ); + (globalThis as unknown as { fetch: typeof fetch }).fetch = + fetchMock as unknown as typeof fetch; + + const provider: OAuthProvider = { + ...PROVIDER_BASE, + authorize: AUTHORIZE_URL, + }; + service.login(provider, dispatch); + + const config = initCodeClient.mock.calls[0]?.[0] as { + callback: (response: CodeResponse) => void; + }; + config.callback(CODE_RESPONSE); + await flushPromises(); + + expect(dispatch.tokenDispatch).toHaveBeenCalledWith(resetTokenState()); + expect(dispatch.tokenDispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ + payload: expect.objectContaining({ token: expect.any(String) }), + }), + ); + }); + + it("resets session state when the authorize response is missing access_token", async () => { + const fetchMock = jest.fn(() => + Promise.resolve({ + json: () => Promise.resolve({}), + ok: true, + } as Response), + ); + (globalThis as unknown as { fetch: typeof fetch }).fetch = + fetchMock as unknown as typeof fetch; + + const provider: OAuthProvider = { + ...PROVIDER_BASE, + authorize: AUTHORIZE_URL, + }; + service.login(provider, dispatch); + + const config = initCodeClient.mock.calls[0]?.[0] as { + callback: (response: CodeResponse) => void; + }; + config.callback(CODE_RESPONSE); + await flushPromises(); + + expect(dispatch.tokenDispatch).toHaveBeenCalledWith(resetTokenState()); + }); }); From 7957b1d54cf18f8fefc4c3c2efc8e9421ab0e8a0 Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Fri, 1 May 2026 20:48:33 +1000 Subject: [PATCH 4/7] refactor: make codeResponse fields optional (#907) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit google's oauth code-flow callback returns either a success payload (code, scope, state) or an error payload (error, error_description, error_uri, state) — required strings on every field misrepresent the contract. trim the test fixture down to the only field we actually populate. Closes #907 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/google/types.ts | 12 ++++++------ tests/googleService.test.ts | 9 +-------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/google/types.ts b/src/google/types.ts index dba81b68..a9ee312b 100644 --- a/src/google/types.ts +++ b/src/google/types.ts @@ -18,12 +18,12 @@ import { * Authorization code response from Google OAuth. */ export interface CodeResponse { - code: string; - error: string; - error_description: string; - error_uri: string; - scope: string; - state: string; + code?: string; + error?: string; + error_description?: string; + error_uri?: string; + scope?: string; + state?: string; } /** diff --git a/tests/googleService.test.ts b/tests/googleService.test.ts index 67905599..3cf6f4a5 100644 --- a/tests/googleService.test.ts +++ b/tests/googleService.test.ts @@ -16,14 +16,7 @@ const PROVIDER_BASE: OAuthProvider = { const AUTHORIZE_URL = "https://service.example.com/user/authorize"; -const CODE_RESPONSE: CodeResponse = { - code: "test-code", - error: "", - error_description: "", - error_uri: "", - scope: "", - state: "", -}; +const CODE_RESPONSE: CodeResponse = { code: "test-code" }; type LoginDispatch = Pick< SessionDispatch, From ace57fa01ab868a80633038e60156e949017878d Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Tue, 5 May 2026 13:25:49 +1000 Subject: [PATCH 5/7] refactor: split google service into implicit flow and authorization code flow (#906) Co-Authored-By: Claude Opus 4.6 (1M context) --- src/google/service.ts | 101 ++----------------- src/google/services/authorizationCodeFlow.ts | 48 +++++++++ src/google/services/common.ts | 77 ++++++++++++++ src/google/services/implicitFlow.ts | 28 +++++ 4 files changed, 164 insertions(+), 90 deletions(-) create mode 100644 src/google/services/authorizationCodeFlow.ts create mode 100644 src/google/services/common.ts create mode 100644 src/google/services/implicitFlow.ts diff --git a/src/google/service.ts b/src/google/service.ts index 5de5613d..f305b107 100644 --- a/src/google/service.ts +++ b/src/google/service.ts @@ -1,23 +1,8 @@ -import { requestAuth, resetAuthState } from "../auth/dispatch/auth"; -import { - requestAuthentication, - resetAuthenticationState, - updateAuthentication, -} from "../auth/dispatch/authentication"; -import { resetCredentialsState } from "../auth/dispatch/credentials"; -import { resetTokenState, updateToken } from "../auth/dispatch/token"; -import { AUTHENTICATION_STATUS } from "../auth/types/authentication"; -import { OAuthProvider } from "../config/entities"; -import type { - CodeResponse, - GoogleProfile, - SessionDispatch, - TokenSetParameters, -} from "./types"; -import { fetchProfile, getAuthenticationRequestOptions } from "./utils/auth"; - -// eslint-disable-next-line @typescript-eslint/no-explicit-any -- TODO see https://github.com/clevercanary/data-browser/issues/544. -declare const google: any; +import type { OAuthProvider } from "../config/entities"; +import { login as authorizationCodeFlowLogin } from "./services/authorizationCodeFlow"; +import { logout, type LoginDispatch } from "./services/common"; +import { login as implicitFlowLogin } from "./services/implicitFlow"; +import type { SessionDispatch } from "./types"; /** * Google Sign-In service. @@ -25,76 +10,15 @@ declare const google: any; export const service = { /** * Login with Google OAuth. + * Selects the authorization code flow or implicit flow based on provider configuration. * @param provider - OAuth provider configuration. * @param dispatch - Dispatch functions for auth state. */ - login: ( - provider: OAuthProvider, - dispatch: Pick< - SessionDispatch, - "authDispatch" | "authenticationDispatch" | "tokenDispatch" - >, - ): void => { - const { authorize, id, profile, userinfo } = provider; - const resetSession = (): void => { - dispatch.authDispatch?.(resetAuthState()); - dispatch.authenticationDispatch?.( - updateAuthentication({ - profile: undefined, - status: AUTHENTICATION_STATUS.SETTLED, - }), - ); - dispatch.tokenDispatch?.(resetTokenState()); - }; - const onAccessToken = (token: string): void => { - dispatch.authDispatch?.(requestAuth()); - dispatch.authenticationDispatch?.(requestAuthentication()); - dispatch.tokenDispatch?.(updateToken({ providerId: id, token })); - fetchProfile(userinfo, getAuthenticationRequestOptions(token), { - onError: resetSession, - onSuccess: (r: GoogleProfile) => - dispatch.authenticationDispatch?.( - updateAuthentication({ - profile: profile(r), - status: AUTHENTICATION_STATUS.PENDING, // Authentication is pending until session controller is resolved. - }), - ), - }); - }; - if (authorize) { - const client = google.accounts.oauth2.initCodeClient({ - callback: (response: CodeResponse) => { - fetch(authorize, { - body: JSON.stringify(response), - headers: { "Content-Type": "application/json" }, - method: "POST", - }) - .then((r) => { - if (!r.ok) { - throw new Error(`authorize request failed (${r.status})`); - } - return r.json(); - }) - .then((tokens: TokenSetParameters) => { - if (!tokens?.access_token) { - throw new Error("authorize response missing access_token"); - } - onAccessToken(tokens.access_token); - }) - .catch(resetSession); - }, - client_id: provider.clientId, - scope: provider.authorization.params.scope, - }); - client.requestCode(); + login: (provider: OAuthProvider, dispatch: LoginDispatch): void => { + if (provider.authorize) { + authorizationCodeFlowLogin(provider, dispatch); } else { - const client = google.accounts.oauth2.initTokenClient({ - callback: (response: TokenSetParameters) => - onAccessToken(response.access_token), - client_id: provider.clientId, - scope: provider.authorization.params.scope, - }); - client.requestAccessToken(); + implicitFlowLogin(provider, dispatch); } }, /** @@ -102,9 +26,6 @@ export const service = { * @param dispatch - Dispatch functions for auth state. */ logout: (dispatch: SessionDispatch): void => { - dispatch.authDispatch?.(resetAuthState()); - dispatch.authenticationDispatch?.(resetAuthenticationState()); - dispatch.credentialsDispatch?.(resetCredentialsState()); - dispatch.tokenDispatch?.(resetTokenState()); + logout(dispatch); }, }; diff --git a/src/google/services/authorizationCodeFlow.ts b/src/google/services/authorizationCodeFlow.ts new file mode 100644 index 00000000..9c3df3e2 --- /dev/null +++ b/src/google/services/authorizationCodeFlow.ts @@ -0,0 +1,48 @@ +import type { OAuthProvider } from "../../config/entities"; +import type { CodeResponse, TokenSetParameters } from "../types"; +import { + createOnAccessToken, + createResetSession, + type LoginDispatch, +} from "./common"; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any -- TODO see https://github.com/clevercanary/data-browser/issues/544. +declare const google: any; + +/** + * Login using the OAuth 2.0 authorization code flow. + * Uses Google's initCodeClient to request an authorization code, + * then exchanges it for an access token via the configured authorize endpoint. + * @param provider - OAuth provider configuration (must have authorize set). + * @param dispatch - Dispatch functions for auth state. + */ +export function login(provider: OAuthProvider, dispatch: LoginDispatch): void { + const { authorize } = provider; + const resetSession = createResetSession(dispatch); + const onAccessToken = createOnAccessToken(provider, dispatch, resetSession); + const client = google.accounts.oauth2.initCodeClient({ + callback: (response: CodeResponse) => { + fetch(authorize as string, { + body: JSON.stringify(response), + headers: { "Content-Type": "application/json" }, + method: "POST", + }) + .then((r) => { + if (!r.ok) { + throw new Error(`authorize request failed (${r.status})`); + } + return r.json(); + }) + .then((tokens: TokenSetParameters) => { + if (!tokens?.access_token) { + throw new Error("authorize response missing access_token"); + } + onAccessToken(tokens.access_token); + }) + .catch(resetSession); + }, + client_id: provider.clientId, + scope: provider.authorization.params.scope, + }); + client.requestCode(); +} diff --git a/src/google/services/common.ts b/src/google/services/common.ts new file mode 100644 index 00000000..fde9f05f --- /dev/null +++ b/src/google/services/common.ts @@ -0,0 +1,77 @@ +import { requestAuth, resetAuthState } from "../../auth/dispatch/auth"; +import { + requestAuthentication, + resetAuthenticationState, + updateAuthentication, +} from "../../auth/dispatch/authentication"; +import { resetCredentialsState } from "../../auth/dispatch/credentials"; +import { resetTokenState, updateToken } from "../../auth/dispatch/token"; +import { AUTHENTICATION_STATUS } from "../../auth/types/authentication"; +import type { OAuthProvider } from "../../config/entities"; +import type { GoogleProfile, SessionDispatch } from "../types"; +import { fetchProfile, getAuthenticationRequestOptions } from "../utils/auth"; + +export type LoginDispatch = Pick< + SessionDispatch, + "authDispatch" | "authenticationDispatch" | "tokenDispatch" +>; + +/** + * Creates a function that resets the session state on auth failure. + * @param dispatch - Dispatch functions for auth state. + * @returns reset session function. + */ +export function createResetSession(dispatch: LoginDispatch): () => void { + return (): void => { + dispatch.authDispatch?.(resetAuthState()); + dispatch.authenticationDispatch?.( + updateAuthentication({ + profile: undefined, + status: AUTHENTICATION_STATUS.SETTLED, + }), + ); + dispatch.tokenDispatch?.(resetTokenState()); + }; +} + +/** + * Creates a function that handles a successful access token. + * Dispatches auth state updates and fetches the user profile. + * @param provider - OAuth provider configuration. + * @param dispatch - Dispatch functions for auth state. + * @param resetSession - Reset session function. + * @returns on access token function. + */ +export function createOnAccessToken( + provider: OAuthProvider, + dispatch: LoginDispatch, + resetSession: () => void, +): (token: string) => void { + const { id, profile, userinfo } = provider; + return (token: string): void => { + dispatch.authDispatch?.(requestAuth()); + dispatch.authenticationDispatch?.(requestAuthentication()); + dispatch.tokenDispatch?.(updateToken({ providerId: id, token })); + fetchProfile(userinfo, getAuthenticationRequestOptions(token), { + onError: resetSession, + onSuccess: (r: GoogleProfile) => + dispatch.authenticationDispatch?.( + updateAuthentication({ + profile: profile(r), + status: AUTHENTICATION_STATUS.PENDING, + }), + ), + }); + }; +} + +/** + * Logout and clear all auth state. + * @param dispatch - Dispatch functions for auth state. + */ +export function logout(dispatch: SessionDispatch): void { + dispatch.authDispatch?.(resetAuthState()); + dispatch.authenticationDispatch?.(resetAuthenticationState()); + dispatch.credentialsDispatch?.(resetCredentialsState()); + dispatch.tokenDispatch?.(resetTokenState()); +} diff --git a/src/google/services/implicitFlow.ts b/src/google/services/implicitFlow.ts new file mode 100644 index 00000000..96b17d23 --- /dev/null +++ b/src/google/services/implicitFlow.ts @@ -0,0 +1,28 @@ +import type { OAuthProvider } from "../../config/entities"; +import type { TokenSetParameters } from "../types"; +import { + createOnAccessToken, + createResetSession, + type LoginDispatch, +} from "./common"; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any -- TODO see https://github.com/clevercanary/data-browser/issues/544. +declare const google: any; + +/** + * Login using the OAuth 2.0 implicit flow. + * Uses Google's initTokenClient to request an access token directly. + * @param provider - OAuth provider configuration. + * @param dispatch - Dispatch functions for auth state. + */ +export function login(provider: OAuthProvider, dispatch: LoginDispatch): void { + const resetSession = createResetSession(dispatch); + const onAccessToken = createOnAccessToken(provider, dispatch, resetSession); + const client = google.accounts.oauth2.initTokenClient({ + callback: (response: TokenSetParameters) => + onAccessToken(response.access_token), + client_id: provider.clientId, + scope: provider.authorization.params.scope, + }); + client.requestAccessToken(); +} From 2236ef2c4ca7ebcc7cfea2ee1bb40123333f6d4e Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Tue, 5 May 2026 14:11:52 +1000 Subject: [PATCH 6/7] refactor: add flow discriminator to oauthprovider (#906) select the oauth flow via an explicit `flow` field on the provider config instead of inferring it from `authorize` truthiness. oauthprovider is now a discriminated union of implicitflowprovider | authorizationcodeflowprovider, so misconfiguration (auth code flow without an authorize url) is a type error rather than a silent fallback to implicit. drops the `as string` cast on authorize. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/config/entities.ts | 26 ++++++++++++-- src/google/service.ts | 6 ++-- src/google/services/authorizationCodeFlow.ts | 11 +++--- src/google/services/implicitFlow.ts | 7 ++-- tests/googleService.test.ts | 38 ++++++++------------ 5 files changed, 53 insertions(+), 35 deletions(-) diff --git a/src/config/entities.ts b/src/config/entities.ts index e13f0687..44534520 100644 --- a/src/config/entities.ts +++ b/src/config/entities.ts @@ -282,10 +282,14 @@ export interface ListViewConfig { rowSelectionView?: ComponentsConfig; } +export enum OAUTH_FLOW { + AUTHORIZATION_CODE = "AUTHORIZATION_CODE", + IMPLICIT = "IMPLICIT", +} + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Use of `any` is intentional to allow for flexibility in the model. -export interface OAuthProvider

{ +interface OAuthProviderBase

{ authorization: { params: { scope: string } }; - authorize?: string; // Backend authorize endpoint that exchanges an OAuth authorization code for a token set. When set, the authorization code flow is used; otherwise the implicit token flow is used. clientId: string; icon: ReactNode; id: ProviderId; @@ -294,6 +298,24 @@ export interface OAuthProvider

{ userinfo: string; } +// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Use of `any` is intentional to allow for flexibility in the model. +export interface ImplicitFlowProvider

extends OAuthProviderBase

{ + flow: OAUTH_FLOW.IMPLICIT; +} + +export interface AuthorizationCodeFlowProvider< + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Use of `any` is intentional to allow for flexibility in the model. + P = any, +> extends OAuthProviderBase

{ + authorize: string; // Backend endpoint that exchanges an OAuth authorization code for a token set. + flow: OAUTH_FLOW.AUTHORIZATION_CODE; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Use of `any` is intentional to allow for flexibility in the model. +export type OAuthProvider

= + | ImplicitFlowProvider

+ | AuthorizationCodeFlowProvider

; + /** * Option Method. */ diff --git a/src/google/service.ts b/src/google/service.ts index f305b107..4cc56605 100644 --- a/src/google/service.ts +++ b/src/google/service.ts @@ -1,4 +1,4 @@ -import type { OAuthProvider } from "../config/entities"; +import { OAUTH_FLOW, type OAuthProvider } from "../config/entities"; import { login as authorizationCodeFlowLogin } from "./services/authorizationCodeFlow"; import { logout, type LoginDispatch } from "./services/common"; import { login as implicitFlowLogin } from "./services/implicitFlow"; @@ -10,12 +10,12 @@ import type { SessionDispatch } from "./types"; export const service = { /** * Login with Google OAuth. - * Selects the authorization code flow or implicit flow based on provider configuration. + * Dispatches to the configured flow based on `provider.flow`. * @param provider - OAuth provider configuration. * @param dispatch - Dispatch functions for auth state. */ login: (provider: OAuthProvider, dispatch: LoginDispatch): void => { - if (provider.authorize) { + if (provider.flow === OAUTH_FLOW.AUTHORIZATION_CODE) { authorizationCodeFlowLogin(provider, dispatch); } else { implicitFlowLogin(provider, dispatch); diff --git a/src/google/services/authorizationCodeFlow.ts b/src/google/services/authorizationCodeFlow.ts index 9c3df3e2..853a940a 100644 --- a/src/google/services/authorizationCodeFlow.ts +++ b/src/google/services/authorizationCodeFlow.ts @@ -1,4 +1,4 @@ -import type { OAuthProvider } from "../../config/entities"; +import type { AuthorizationCodeFlowProvider } from "../../config/entities"; import type { CodeResponse, TokenSetParameters } from "../types"; import { createOnAccessToken, @@ -13,16 +13,19 @@ declare const google: any; * Login using the OAuth 2.0 authorization code flow. * Uses Google's initCodeClient to request an authorization code, * then exchanges it for an access token via the configured authorize endpoint. - * @param provider - OAuth provider configuration (must have authorize set). + * @param provider - OAuth provider configuration. * @param dispatch - Dispatch functions for auth state. */ -export function login(provider: OAuthProvider, dispatch: LoginDispatch): void { +export function login( + provider: AuthorizationCodeFlowProvider, + dispatch: LoginDispatch, +): void { const { authorize } = provider; const resetSession = createResetSession(dispatch); const onAccessToken = createOnAccessToken(provider, dispatch, resetSession); const client = google.accounts.oauth2.initCodeClient({ callback: (response: CodeResponse) => { - fetch(authorize as string, { + fetch(authorize, { body: JSON.stringify(response), headers: { "Content-Type": "application/json" }, method: "POST", diff --git a/src/google/services/implicitFlow.ts b/src/google/services/implicitFlow.ts index 96b17d23..b401494c 100644 --- a/src/google/services/implicitFlow.ts +++ b/src/google/services/implicitFlow.ts @@ -1,4 +1,4 @@ -import type { OAuthProvider } from "../../config/entities"; +import type { ImplicitFlowProvider } from "../../config/entities"; import type { TokenSetParameters } from "../types"; import { createOnAccessToken, @@ -15,7 +15,10 @@ declare const google: any; * @param provider - OAuth provider configuration. * @param dispatch - Dispatch functions for auth state. */ -export function login(provider: OAuthProvider, dispatch: LoginDispatch): void { +export function login( + provider: ImplicitFlowProvider, + dispatch: LoginDispatch, +): void { const resetSession = createResetSession(dispatch); const onAccessToken = createOnAccessToken(provider, dispatch, resetSession); const client = google.accounts.oauth2.initTokenClient({ diff --git a/tests/googleService.test.ts b/tests/googleService.test.ts index 3cf6f4a5..36900837 100644 --- a/tests/googleService.test.ts +++ b/tests/googleService.test.ts @@ -1,12 +1,13 @@ import { jest } from "@jest/globals"; import { resetTokenState, updateToken } from "../src/auth/dispatch/token"; -import type { OAuthProvider } from "../src/config/entities"; +import { OAUTH_FLOW, type OAuthProvider } from "../src/config/entities"; import { service } from "../src/google/service"; import type { CodeResponse, SessionDispatch } from "../src/google/types"; const PROVIDER_BASE: OAuthProvider = { authorization: { params: { scope: "openid email profile" } }, clientId: "test-client-id", + flow: OAUTH_FLOW.IMPLICIT, icon: null, id: "google", name: "Google", @@ -16,6 +17,12 @@ const PROVIDER_BASE: OAuthProvider = { const AUTHORIZE_URL = "https://service.example.com/user/authorize"; +const PROVIDER_AUTH_CODE: OAuthProvider = { + ...PROVIDER_BASE, + authorize: AUTHORIZE_URL, + flow: OAUTH_FLOW.AUTHORIZATION_CODE, +}; + const CODE_RESPONSE: CodeResponse = { code: "test-code" }; type LoginDispatch = Pick< @@ -56,13 +63,8 @@ describe("service.login (Google)", () => { delete (globalThis as unknown as { fetch?: unknown }).fetch; }); - it("uses the authorization code flow when provider.authorize is set", () => { - const provider: OAuthProvider = { - ...PROVIDER_BASE, - authorize: AUTHORIZE_URL, - }; - - service.login(provider, dispatch); + it("uses the authorization code flow when provider.flow is OAUTH_FLOW.AUTHORIZATION_CODE", () => { + service.login(PROVIDER_AUTH_CODE, dispatch); expect(initCodeClient).toHaveBeenCalledTimes(1); expect(requestCode).toHaveBeenCalledTimes(1); @@ -77,7 +79,7 @@ describe("service.login (Google)", () => { expect(config.scope).toBe(PROVIDER_BASE.authorization.params.scope); }); - it("falls back to the implicit token flow when provider.authorize is unset", () => { + it("uses the implicit token flow when provider.flow is OAUTH_FLOW.IMPLICIT", () => { service.login(PROVIDER_BASE, dispatch); expect(initTokenClient).toHaveBeenCalledTimes(1); @@ -96,11 +98,7 @@ describe("service.login (Google)", () => { (globalThis as unknown as { fetch: typeof fetch }).fetch = fetchMock as unknown as typeof fetch; - const provider: OAuthProvider = { - ...PROVIDER_BASE, - authorize: AUTHORIZE_URL, - }; - service.login(provider, dispatch); + service.login(PROVIDER_AUTH_CODE, dispatch); const config = initCodeClient.mock.calls[0]?.[0] as { callback: (response: CodeResponse) => void; @@ -129,11 +127,7 @@ describe("service.login (Google)", () => { (globalThis as unknown as { fetch: typeof fetch }).fetch = fetchMock as unknown as typeof fetch; - const provider: OAuthProvider = { - ...PROVIDER_BASE, - authorize: AUTHORIZE_URL, - }; - service.login(provider, dispatch); + service.login(PROVIDER_AUTH_CODE, dispatch); const config = initCodeClient.mock.calls[0]?.[0] as { callback: (response: CodeResponse) => void; @@ -159,11 +153,7 @@ describe("service.login (Google)", () => { (globalThis as unknown as { fetch: typeof fetch }).fetch = fetchMock as unknown as typeof fetch; - const provider: OAuthProvider = { - ...PROVIDER_BASE, - authorize: AUTHORIZE_URL, - }; - service.login(provider, dispatch); + service.login(PROVIDER_AUTH_CODE, dispatch); const config = initCodeClient.mock.calls[0]?.[0] as { callback: (response: CodeResponse) => void; From c9d8b281b5004a2e63defb4a474fcd8410f3d52d Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Tue, 5 May 2026 14:21:09 +1000 Subject: [PATCH 7/7] fix: restore globalThis.fetch instead of deleting it (#906) per copilot review on #905: deleting globalThis.fetch in afterEach permanently removes node 18+'s built-in fetch for any later tests in the same worker, risking order-dependent failures. snapshot the original in beforeEach and restore (or delete only if it wasn't there) in afterEach. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/googleService.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/googleService.test.ts b/tests/googleService.test.ts index 36900837..1703eaee 100644 --- a/tests/googleService.test.ts +++ b/tests/googleService.test.ts @@ -42,6 +42,7 @@ describe("service.login (Google)", () => { let requestCode: jest.Mock; let requestAccessToken: jest.Mock; let dispatch: LoginDispatch; + let originalFetch: typeof fetch | undefined; beforeEach(() => { requestCode = jest.fn(); @@ -56,11 +57,16 @@ describe("service.login (Google)", () => { (globalThis as unknown as { google: unknown }).google = { accounts: { oauth2: { initCodeClient, initTokenClient } }, }; + originalFetch = (globalThis as unknown as { fetch?: typeof fetch }).fetch; }); afterEach(() => { delete (globalThis as unknown as { google?: unknown }).google; - delete (globalThis as unknown as { fetch?: unknown }).fetch; + if (originalFetch === undefined) { + delete (globalThis as unknown as { fetch?: unknown }).fetch; + } else { + (globalThis as unknown as { fetch: typeof fetch }).fetch = originalFetch; + } }); it("uses the authorization code flow when provider.flow is OAUTH_FLOW.AUTHORIZATION_CODE", () => {