From 171765298b81dddc5e1b0cb366468fe10f29bba8 Mon Sep 17 00:00:00 2001 From: Agent Ralph <91435868+NoelVarghese2006@users.noreply.github.com> Date: Mon, 21 Sep 2026 14:10:58 -0500 Subject: [PATCH 1/2] fix(auth): request Google account selection after explicit logout --- .github/workflows/audit.yaml | 1 + client/portal/package.json | 1 + .../portal/scripts/auth-regressions.test.mjs | 124 ++++++++++++++++++ .../src/pages/admin/_shared/NavUser.tsx | 4 +- .../src/pages/hacker/profile/ProfilePage.tsx | 6 +- .../src/pages/public/AuthCallbackPage.tsx | 3 +- .../src/shared/auth/account-selection.ts | 52 ++++++++ client/portal/src/shared/auth/index.ts | 1 + client/portal/src/shared/auth/supertokens.ts | 3 + internal/auth/supertokens_test.go | 46 +++++++ 10 files changed, 235 insertions(+), 6 deletions(-) create mode 100644 client/portal/scripts/auth-regressions.test.mjs create mode 100644 client/portal/src/shared/auth/account-selection.ts create mode 100644 internal/auth/supertokens_test.go diff --git a/.github/workflows/audit.yaml b/.github/workflows/audit.yaml index 951d14865..a96a5834d 100644 --- a/.github/workflows/audit.yaml +++ b/.github/workflows/audit.yaml @@ -82,6 +82,7 @@ jobs: - name: Run Tests run: | + npm run test:auth if [ -d "__tests__" ] || find src -name '*.test.*' -o -name '*.spec.*' 2>/dev/null | grep -q .; then npx vitest run else diff --git a/client/portal/package.json b/client/portal/package.json index 4d8ee1872..dc2319184 100644 --- a/client/portal/package.json +++ b/client/portal/package.json @@ -4,6 +4,7 @@ "version": "0.0.0", "type": "module", "scripts": { + "test:auth": "node --test scripts/auth-regressions.test.mjs", "dev": "vite", "build": "tsc -b && vite build", "lint": "eslint .", diff --git a/client/portal/scripts/auth-regressions.test.mjs b/client/portal/scripts/auth-regressions.test.mjs new file mode 100644 index 000000000..991892a02 --- /dev/null +++ b/client/portal/scripts/auth-regressions.test.mjs @@ -0,0 +1,124 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { fileURLToPath } from "node:url"; + +import { build } from "esbuild"; + +const bundle = await build({ + absWorkingDir: fileURLToPath(new URL("../", import.meta.url)), + entryPoints: ["src/shared/auth/account-selection.ts"], + platform: "node", + format: "cjs", + bundle: true, + write: false, + external: ["supertokens-auth-react/recipe/session"], +}); + +function loadAuth(signOut = async () => {}) { + const module = { exports: {} }; + new Function("module", "exports", "require", bundle.outputFiles[0].text)( + module, + module.exports, + () => ({ signOut }), + ); + return module.exports; +} + +test("explicit logout persists through restart and OAuth cancellation until portal login", async () => { + const previousStorage = globalThis.localStorage; + const values = new Map(); + globalThis.localStorage = { + getItem: (key) => values.get(key) ?? null, + setItem: (key, value) => values.set(key, value), + removeItem: (key) => values.delete(key), + }; + try { + const input = { thirdPartyId: "google" }; + const originalURL = + "https://accounts.google.com/o/oauth2/v2/auth?state=saved-state&code_challenge=saved-challenge&redirect_uri=https%3A%2F%2Fportal.example%2Fauth%2Fcallback%2Fgoogle"; + const original = { + getAuthorisationURLWithQueryParamsAndSetState: async (received) => { + assert.equal(received, input); + return originalURL; + }, + }; + let auth = loadAuth(); + const redirect = () => + auth + .withAccountSelection(original) + .getAuthorisationURLWithQueryParamsAndSetState(input); + + // Fresh login and session expiration preserve the SDK URL unchanged. + assert.equal(await redirect(), originalURL); + const failingAuth = loadAuth(async () => { + throw new Error("logout failed"); + }); + await assert.rejects(failingAuth.signOutExplicitly(), /logout failed/); + assert.equal(await redirect(), originalURL); + + await auth.signOutExplicitly(); + auth = loadAuth(); // App restart: only persistent storage survives. + for (let attempt = 0; attempt < 2; attempt++) { + const url = new URL(await redirect()); + assert.equal(url.searchParams.get("prompt"), "select_account"); + for (const [key, value] of new URL(originalURL).searchParams) { + assert.equal(url.searchParams.get(key), value); + } + } + // Both Google and magic-link callbacks complete the same portal login. + auth.completePortalLogin(); + assert.equal(await redirect(), originalURL); + auth = loadAuth(); + assert.equal(await redirect(), originalURL); + } finally { + if (previousStorage === undefined) delete globalThis.localStorage; + else globalThis.localStorage = previousStorage; + } +}); + +test("storage denial does not prevent logout or login", async () => { + const descriptor = Object.getOwnPropertyDescriptor( + globalThis, + "localStorage", + ); + Object.defineProperty(globalThis, "localStorage", { + configurable: true, + get() { + throw new Error("storage denied"); + }, + }); + try { + const auth = loadAuth(); + const originalURL = + "https://accounts.google.com/o/oauth2/v2/auth?state=saved"; + const recipe = auth.withAccountSelection({ + getAuthorisationURLWithQueryParamsAndSetState: async () => originalURL, + }); + await auth.signOutExplicitly(); + assert.equal( + new URL( + await recipe.getAuthorisationURLWithQueryParamsAndSetState({ + thirdPartyId: "google", + }), + ).searchParams.get("prompt"), + "select_account", + ); + assert.equal( + await recipe.getAuthorisationURLWithQueryParamsAndSetState({ + thirdPartyId: "other", + }), + originalURL, + ); + auth.completePortalLogin(); + assert.equal( + await recipe.getAuthorisationURLWithQueryParamsAndSetState({ + thirdPartyId: "google", + }), + originalURL, + ); + } finally { + if (descriptor) + Object.defineProperty(globalThis, "localStorage", descriptor); + else delete globalThis.localStorage; + } +}); diff --git a/client/portal/src/pages/admin/_shared/NavUser.tsx b/client/portal/src/pages/admin/_shared/NavUser.tsx index a4f9cbc53..ba8295590 100644 --- a/client/portal/src/pages/admin/_shared/NavUser.tsx +++ b/client/portal/src/pages/admin/_shared/NavUser.tsx @@ -2,7 +2,6 @@ import { ChevronsUpDown, Eye, LogOut, ShieldCheck } from "lucide-react"; import { useLocation, useNavigate } from "react-router"; -import Session from "supertokens-auth-react/recipe/session"; import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; import { @@ -19,6 +18,7 @@ import { SidebarMenuItem, useSidebar, } from "@/components/ui/sidebar"; +import { signOutExplicitly } from "@/shared/auth"; import { useUserStore } from "@/shared/stores"; export function NavUser({ @@ -47,7 +47,7 @@ export function NavUser({ }; const handleLogout = async () => { - await Session.signOut(); + await signOutExplicitly(); clearUser(); navigate("/"); }; diff --git a/client/portal/src/pages/hacker/profile/ProfilePage.tsx b/client/portal/src/pages/hacker/profile/ProfilePage.tsx index 9f24b4e94..4a4c05c98 100644 --- a/client/portal/src/pages/hacker/profile/ProfilePage.tsx +++ b/client/portal/src/pages/hacker/profile/ProfilePage.tsx @@ -10,7 +10,6 @@ import { import { type ChangeEvent, useEffect, useRef, useState } from "react"; import { useNavigate } from "react-router"; import { toast } from "sonner"; -import { signOut } from "supertokens-auth-react/recipe/session"; import { AdminPortalButton } from "@/components/AdminPortalButton"; import { InstallGuideDialog } from "@/components/InstallGuideDialog"; @@ -27,6 +26,7 @@ import { } from "@/components/ui/alert-dialog"; import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; import { Switch } from "@/components/ui/switch"; +import { signOutExplicitly } from "@/shared/auth"; import { useInstallPrompt } from "@/shared/install"; import { errorAlert, getRequest } from "@/shared/lib/api"; import { usePushSubscription } from "@/shared/push/usePushSubscription"; @@ -108,7 +108,7 @@ export default function ProfilePage() { const hasResume = Boolean(application?.resume_path); const handleLogout = async () => { - await signOut(); + await signOutExplicitly(); clearUser(); navigate("/", { replace: true }); }; @@ -204,7 +204,7 @@ export default function ProfilePage() { setDeleting(true); const res = await deleteMyAccount(); if (res.status === 204 || res.status === 200) { - await signOut(); + await signOutExplicitly(); clearUser(); navigate("/", { replace: true }); } else { diff --git a/client/portal/src/pages/public/AuthCallbackPage.tsx b/client/portal/src/pages/public/AuthCallbackPage.tsx index d24b75595..3136c036b 100644 --- a/client/portal/src/pages/public/AuthCallbackPage.tsx +++ b/client/portal/src/pages/public/AuthCallbackPage.tsx @@ -13,7 +13,7 @@ import { CardHeader, CardTitle, } from "@/components/ui/card"; -import { isGoogleAuthEnabled } from "@/shared/auth"; +import { completePortalLogin, isGoogleAuthEnabled } from "@/shared/auth"; import { isMobileViewport } from "@/shared/hooks"; import { useUserStore } from "@/shared/stores"; @@ -34,6 +34,7 @@ export default function AuthCallback() { const { user, authError: error } = useUserStore.getState(); if (user) { + completePortalLogin(); // Redirect based on role. The admin portal is desktop-only, so // admins signing in on a small screen land in the hacker app. const isAdmin = user.role === "admin" || user.role === "super_admin"; diff --git a/client/portal/src/shared/auth/account-selection.ts b/client/portal/src/shared/auth/account-selection.ts new file mode 100644 index 000000000..c3ee52941 --- /dev/null +++ b/client/portal/src/shared/auth/account-selection.ts @@ -0,0 +1,52 @@ +import { signOut } from "supertokens-auth-react/recipe/session"; +import type { RecipeInterface } from "supertokens-web-js/recipe/thirdparty"; + +const storageKey = "harp:choose-google-account"; +let selectionRequired = false; + +function requiresAccountSelection() { + try { + return localStorage.getItem(storageKey) === "true" || selectionRequired; + } catch { + return selectionRequired; + } +} + +export async function signOutExplicitly() { + await signOut(); + selectionRequired = true; + try { + localStorage.setItem(storageKey, "true"); + selectionRequired = false; + } catch { + // Storage restrictions must not prevent logout; retain intent in memory. + } +} + +export function completePortalLogin() { + selectionRequired = false; + try { + localStorage.removeItem(storageKey); + } catch { + // Login still succeeds when persistent storage is unavailable. + } +} + +export function withAccountSelection( + original: RecipeInterface, +): RecipeInterface { + return { + ...original, + getAuthorisationURLWithQueryParamsAndSetState: async (input) => { + const authorizationURL = + await original.getAuthorisationURLWithQueryParamsAndSetState(input); + if (input.thirdPartyId !== "google" || !requiresAccountSelection()) { + return authorizationURL; + } + const url = new URL(authorizationURL); + url.searchParams.set("prompt", "select_account"); + // Keep intent through cancellation/errors, until portal login succeeds. + return url.toString(); + }, + }; +} diff --git a/client/portal/src/shared/auth/index.ts b/client/portal/src/shared/auth/index.ts index aa6e76a09..2334fcb39 100644 --- a/client/portal/src/shared/auth/index.ts +++ b/client/portal/src/shared/auth/index.ts @@ -1,5 +1,6 @@ // Auth - public API +export { completePortalLogin, signOutExplicitly } from "./account-selection"; export { default as RequireAdmin } from "./guards/RequireAdmin"; export { default as RequireAuth } from "./guards/RequireAuth"; export { default as RequireSuperAdmin } from "./guards/RequireSuperAdmin"; diff --git a/client/portal/src/shared/auth/supertokens.ts b/client/portal/src/shared/auth/supertokens.ts index ac3f710d8..57b4cd49f 100644 --- a/client/portal/src/shared/auth/supertokens.ts +++ b/client/portal/src/shared/auth/supertokens.ts @@ -5,6 +5,8 @@ import ThirdParty, { Google } from "supertokens-auth-react/recipe/thirdparty"; import { branding } from "@/branding"; +import { withAccountSelection } from "./account-selection"; + export const isGoogleAuthEnabled = import.meta.env.VITE_GOOGLE_AUTH_ENABLED === "true"; @@ -24,6 +26,7 @@ export function initSuperTokens() { ...(isGoogleAuthEnabled ? [ ThirdParty.init({ + override: { functions: withAccountSelection }, signInAndUpFeature: { providers: [Google.init()], }, diff --git a/internal/auth/supertokens_test.go b/internal/auth/supertokens_test.go new file mode 100644 index 000000000..772d14818 --- /dev/null +++ b/internal/auth/supertokens_test.go @@ -0,0 +1,46 @@ +package auth + +import ( + "net/url" + "testing" + + "github.com/hackutd/harp/internal/store" + "github.com/stretchr/testify/require" + "github.com/supertokens/supertokens-golang/recipe/thirdparty" + "github.com/supertokens/supertokens-golang/recipe/thirdparty/providers" + "github.com/supertokens/supertokens-golang/supertokens" +) + +func TestGoogleLoginUsesDefaultAccountSelection(t *testing.T) { + supertokens.ResetForTest() + t.Cleanup(supertokens.ResetForTest) + require.NoError(t, InitSuperTokens(Config{ + AppName: "test", ConnectionURI: "http://localhost:3567", + APIBasePath: "/auth", APIURL: "http://localhost:8080", + FrontendURL: "http://localhost:3000", + GoogleClientID: "test-client", GoogleClientSecret: "test-secret", + }, store.Storage{})) + + recipe, err := thirdparty.GetRecipeInstanceOrThrowError() + require.NoError(t, err) + require.Len(t, recipe.Providers, 1) + provider := providers.Google(recipe.Providers[0]) + userContext := &map[string]interface{}{} + provider.Config, err = provider.GetConfigForClientType(nil, userContext) + require.NoError(t, err) + // Supply the discovery result locally; URL generation needs no Google session + // or running SuperTokens core. All query parameters come from the real recipe. + provider.Config.AuthorizationEndpoint = "https://accounts.google.com/o/oauth2/v2/auth" + callback := "http://localhost:3000/auth/callback/google" + redirect, err := provider.GetAuthorisationRedirectURL(callback, userContext) + require.NoError(t, err) + authorizationURL, err := url.Parse(redirect.URLWithQueryParams) + require.NoError(t, err) + params := authorizationURL.Query() + require.Equal(t, callback, params.Get("redirect_uri")) + require.Equal(t, "test-client", params.Get("client_id")) + require.Equal(t, "code", params.Get("response_type")) + // The frontend requests account selection only after an explicit logout. + // Session expiration must retain Google's default sign-in behavior. + require.Empty(t, params.Get("prompt")) +} From cedba8d798baeaaa2ff9ba3a2cdfe714c0354546 Mon Sep 17 00:00:00 2001 From: Caleb Bae Date: Sun, 4 Oct 2026 12:31:26 -0500 Subject: [PATCH 2/2] fix(auth): update supertokens test for new init signature Pass the magic-link email sender added on main, and run the auth regression suite in the Regression Tests CI step. --- .claude/skills/ci-pipeline/SKILL.md | 4 ++-- .github/workflows/audit.yaml | 2 +- claude.md | 2 +- client/portal/package.json | 4 ++-- internal/auth/supertokens_test.go | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.claude/skills/ci-pipeline/SKILL.md b/.claude/skills/ci-pipeline/SKILL.md index f7c8de16e..00facc5f7 100644 --- a/.claude/skills/ci-pipeline/SKILL.md +++ b/.claude/skills/ci-pipeline/SKILL.md @@ -152,7 +152,7 @@ directory `client/portal`. Steps, in order: this is *both* the TypeScript type check and the production build). 5. **Dependency Audit** — `npm audit --audit-level=high --omit=dev` (prod deps only; high+ severity fails). -6. **Regression Tests** — `npm run test:reviews` and `npm run test:applications` +6. **Regression Tests** — `npm run test:reviews`, `npm run test:applications`, and `npm run test:auth` (`node --test` over `client/portal/scripts/*.test.mjs`). 7. **Tests** — runs `npx vitest run` **only if** test files exist (a `__tests__` dir or any `*.test.*` / `*.spec.*` under `src`); otherwise it prints "No test @@ -165,7 +165,7 @@ pushing (or use the `/ci-audit` command). The key mirrors are `task migrate-check`, `gofmt -l .`, `go vet ./...`, `staticcheck ./...`, `govulncheck ./...`, `task gen-docs && git status --porcelain`, `go test -race ./...` for the backend and -`npm run format:check && npm run lint && npm run build && npm run test:reviews && npm run test:applications` +`npm run format:check && npm run lint && npm run build && npm run test:reviews && npm run test:applications && npm run test:auth` in `client/portal` for the frontend. `db-integration` needs a scratch Postgres, and `docker-build` needs `docker build .`. diff --git a/.github/workflows/audit.yaml b/.github/workflows/audit.yaml index 0278101be..d9aeecbb5 100644 --- a/.github/workflows/audit.yaml +++ b/.github/workflows/audit.yaml @@ -301,10 +301,10 @@ jobs: run: | npm run test:reviews npm run test:applications + npm run test:auth - name: Run Tests run: | - npm run test:auth if [ -d "__tests__" ] || find src -name '*.test.*' -o -name '*.spec.*' 2>/dev/null | grep -q .; then npx vitest run else diff --git a/claude.md b/claude.md index 45ba023f8..a18a446d2 100644 --- a/claude.md +++ b/claude.md @@ -167,7 +167,7 @@ Runs on every push/PR to `main` (`.github/workflows/audit.yaml`): - **Go lint (`backend-lint`):** migration naming check, gofmt check, `go mod verify`, `go vet`, `staticcheck`, `govulncheck`, Swagger docs drift check (`task gen-docs` must leave no diff) - **DB (`db-integration`):** throwaway Postgres 16.3 service container; migrations `up` → `down -all` → `up`, then the store integration tests with `HARP_TEST_DSN` set - **Image (`docker-build`):** builds the production `Dockerfile` without pushing -- **Portal (`frontend-audit`):** `npm run format:check`, `npm run lint`, `npm run build`, `npm audit --audit-level=high`, `npm run test:reviews`, `npm run test:applications` +- **Portal (`frontend-audit`):** `npm run format:check`, `npm run lint`, `npm run build`, `npm audit --audit-level=high`, `npm run test:reviews`, `npm run test:applications`, `npm run test:auth` PRs that change `cmd/migrate/migrations/` also get a reminder comment (`.github/workflows/migration-reminder.yaml`) to apply the migration to prod before merging. diff --git a/client/portal/package.json b/client/portal/package.json index dc2319184..167c941d8 100644 --- a/client/portal/package.json +++ b/client/portal/package.json @@ -4,7 +4,6 @@ "version": "0.0.0", "type": "module", "scripts": { - "test:auth": "node --test scripts/auth-regressions.test.mjs", "dev": "vite", "build": "tsc -b && vite build", "lint": "eslint .", @@ -12,7 +11,8 @@ "format:check": "prettier --check \"{src,branding}/**/*.{ts,tsx,css,json}\"", "preview": "vite preview", "test:reviews": "node --test scripts/review-regressions.test.mjs", - "test:applications": "node --test scripts/application-regressions.test.mjs" + "test:applications": "node --test scripts/application-regressions.test.mjs", + "test:auth": "node --test scripts/auth-regressions.test.mjs" }, "dependencies": { "@hookform/resolvers": "^5.2.2", diff --git a/internal/auth/supertokens_test.go b/internal/auth/supertokens_test.go index 772d14818..259ee0c0f 100644 --- a/internal/auth/supertokens_test.go +++ b/internal/auth/supertokens_test.go @@ -19,7 +19,7 @@ func TestGoogleLoginUsesDefaultAccountSelection(t *testing.T) { APIBasePath: "/auth", APIURL: "http://localhost:8080", FrontendURL: "http://localhost:3000", GoogleClientID: "test-client", GoogleClientSecret: "test-secret", - }, store.Storage{})) + }, store.Storage{}, nil)) recipe, err := thirdparty.GetRecipeInstanceOrThrowError() require.NoError(t, err)