diff --git a/.claude/skills/ci-pipeline/SKILL.md b/.claude/skills/ci-pipeline/SKILL.md index 20f07ced7..91d049ca3 100644 --- a/.claude/skills/ci-pipeline/SKILL.md +++ b/.claude/skills/ci-pipeline/SKILL.md @@ -156,7 +156,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 @@ -169,7 +169,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 a47420c59..d9aeecbb5 100644 --- a/.github/workflows/audit.yaml +++ b/.github/workflows/audit.yaml @@ -301,6 +301,7 @@ jobs: run: | npm run test:reviews npm run test:applications + npm run test:auth - name: Run Tests run: | diff --git a/claude.md b/claude.md index c95dcf895..6d1ad2912 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 staging before merging and to prod before the release. diff --git a/client/portal/package.json b/client/portal/package.json index 4d8ee1872..167c941d8 100644 --- a/client/portal/package.json +++ b/client/portal/package.json @@ -11,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/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..259ee0c0f --- /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{}, nil)) + + 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")) +}