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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .claude/agent-memory/qa-integration-tester/environment-setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,15 @@ NODE_PATH=/path/to/cornerstone/server/node_modules:/path/to/cornerstone/client/n
→ After changing `shared/src/types/`, rebuild main project's dist OR copy worktree dist files:
`cp -r /worktree/shared/dist /path/to/cornerstone/shared/`
- Server tests (better-sqlite3 native binary) may SIGKILL on ARM64 sandbox — validate via CI if needed

## LoginPage.test.tsx — pre-existing local-only failures (verified not caused by new tests, 2026-07-21)

Running `npx jest client/src/pages/LoginPage/LoginPage.test.tsx --coverage --maxWorkers=1` locally on Node 24.18.0
in this worktree shows 9 of 18 tests failing with `expect(mockGetAuthMe).toHaveBeenCalled()` receiving 0 calls,
plus `console.error: An update to LoginPage/AuthProvider inside a test was not wrapped in act(...)`. Confirmed via
`git stash` that this reproduces identically on the pre-existing baseline (8/17 failing before any edit) — it is
NOT caused by adding a new test and is not a regression. Root cause looks like an async-timing/jsdom quirk with
the `getAuthMe` mock resolving after the effect's cleanup in this sandbox; all 7 `oidc*`/error-code tests
(the simple render + findByText ones) reliably PASS regardless. Coverage numbers from a run with these failures
are not representative of true coverage — treat CI's coverage-report artifact as authoritative for this file,
not a local run.
3 changes: 2 additions & 1 deletion client/src/i18n/de/auth.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"invalid_state": "Authentifizierungssitzung abgelaufen. Bitte versuchen Sie es erneut.",
"missing_email": "Ihr Identitätsanbieter hat keine E-Mail-Adresse bereitgestellt.",
"email_conflict": "Diese E-Mail-Adresse ist bereits mit einem anderen Konto verknüpft.",
"account_deactivated": "Ihr Konto wurde deaktiviert. Bitte wenden Sie sich an einen Administrator."
"account_deactivated": "Ihr Konto wurde deaktiviert. Bitte wenden Sie sich an einen Administrator.",
"oidc_no_matching_account": "Für Ihre E-Mail-Adresse wurde kein Konto gefunden. Bitte wenden Sie sich an einen Administrator, damit zunächst ein Konto für Sie angelegt wird."
},
"validation": {
"emailRequired": "E-Mail ist erforderlich",
Expand Down
3 changes: 2 additions & 1 deletion client/src/i18n/en/auth.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"invalid_state": "Authentication session expired. Please try again.",
"missing_email": "Your identity provider did not provide an email address.",
"email_conflict": "This email is already associated with a different account.",
"account_deactivated": "Your account has been deactivated. Please contact an administrator."
"account_deactivated": "Your account has been deactivated. Please contact an administrator.",
"oidc_no_matching_account": "No account was found for your email address. Ask an administrator to create an account for you first."
},
"validation": {
"emailRequired": "Email is required",
Expand Down
10 changes: 10 additions & 0 deletions client/src/pages/LoginPage/LoginPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,16 @@ describe('LoginPage', () => {
expect(await screen.findByText(/your account has been deactivated/i)).toBeInTheDocument();
});

it('shows OIDC error message from URL query parameter (oidc_no_matching_account)', async () => {
window.history.pushState({}, '', '/login?error=oidc_no_matching_account');

renderWithAuth(<LoginPage />);

expect(
await screen.findByText(/no account was found for your email address/i),
).toBeInTheDocument();
});

it('does not show error message when no error in URL', async () => {
renderWithAuth(<LoginPage />);

Expand Down
1 change: 1 addition & 0 deletions client/src/pages/LoginPage/LoginPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export function LoginPage() {
'missing_email',
'email_conflict',
'account_deactivated',
'oidc_no_matching_account',
];
if (knownCodes.includes(errorCode)) {
/* eslint-disable @eslint-react/set-state-in-effect -- initializing error state from url params */
Expand Down
7 changes: 6 additions & 1 deletion e2e/fixtures/testData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ export const TEST_ADMIN = {
export const TEST_MEMBER = {
email: 'member@e2e-test.local',
displayName: 'E2E Member',
// Created via OIDC flow, no local password
// Pre-created as a local account (via the admin-authenticated POST /api/users
// API — see the `beforeAll` seed step in oidc.spec.ts), then linked to the
// mock OIDC provider's fixed identity on first SSO login. OIDC is purely an
// alternate login method for an already-existing account: authProvider stays
// 'local' after linking, and this password keeps working alongside SSO.
localPassword: 'e2e-member-password-456!',
};

export const ROUTES = {
Expand Down
105 changes: 95 additions & 10 deletions e2e/tests/auth/oidc.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* E2E tests for OIDC SSO flow (Stories #34, #35)
* E2E tests for OIDC SSO flow (Stories #34, #35; account-linking fix #1865)
*
* These tests verify the full OIDC authentication flow using a mock OIDC provider.
* The Cornerstone server communicates with the OIDC provider via Docker network alias
Expand All @@ -9,20 +9,49 @@
* redirects from the Docker network alias (http://oidc-server:8080/...) to
* browser-accessible URLs (/oidc-proxy/...). The proxy forwards /oidc-proxy/* requests
* to the OIDC server with Host: oidc-server:8080 to ensure the issuer claim matches.
*
* As of #1865, OIDC login is purely an alternate login method for accounts that
* ALREADY EXIST — it never creates a new account. The mock OIDC provider
* (e2e/containers/oidcContainer.ts) is hardcoded to always return a fixed identity
* (member@e2e-test.local). For the happy path below, that identity must already
* exist as a local account (created via the admin API in `beforeAll`) so the first
* SSO login can link it; subsequent logins are resolved by the linked oidcSubject.
*/

import { test, expect } from '@playwright/test';
import { LoginPage } from '../../pages/LoginPage.js';
import { UserManagementPage } from '../../pages/UserManagementPage.js';
import { createLocalUserViaApi } from '../../fixtures/apiHelpers.js';
import { TEST_MEMBER, ROUTES, API } from '../../fixtures/testData.js';

// Serial mode: test 3 creates the OIDC user, tests 4 and 5 verify it
// Serial mode: beforeAll seeds the local account that gets linked on first SSO
// login (test 3); tests 4 and 5 verify the linked account's state.
test.describe.configure({ mode: 'serial' });

test.describe('OIDC SSO Flow', () => {
// Unauthenticated context for OIDC login tests
test.use({ storageState: { cookies: [], origins: [] } });

test.beforeAll(async ({ browser }) => {
// Seed TEST_MEMBER as a local account via the admin-authenticated API.
// The mock OIDC provider always returns this email/identity, and (post
// #1865) OIDC can only link/log in to an account that already exists —
// it never creates one. Use an explicit admin-authenticated context here
// (rather than the `authenticatedPage` fixture) since `beforeAll` hooks
// don't participate in the describe-level `storageState` override above.
const adminContext = await browser.newContext({
storageState: 'test-results/.auth/admin.json',
});
const adminPage = await adminContext.newPage();
await createLocalUserViaApi(adminPage, {
email: TEST_MEMBER.email,
displayName: TEST_MEMBER.displayName,
password: TEST_MEMBER.localPassword,
role: 'member',
});
await adminContext.close();
});

test('Login page shows SSO button when OIDC is enabled', async ({ page }) => {
const loginPage = new LoginPage(page);

Expand Down Expand Up @@ -55,10 +84,11 @@ test.describe('OIDC SSO Flow', () => {
test('Full OIDC flow creates session', async ({ page }) => {
const loginPage = new LoginPage(page);

// Given: User is on the login page
// Given: TEST_MEMBER already exists as a local account (seeded in beforeAll)
await loginPage.goto();

// When: User completes the OIDC flow
// When: User completes the OIDC flow — this links the local account to the
// OIDC identity (oidcSubject is set; authProvider stays 'local')
await loginPage.clickSSO();

// Then: User should be redirected to dashboard (session created)
Expand All @@ -73,30 +103,33 @@ test.describe('OIDC SSO Flow', () => {
expect(me.user.email).toBe(TEST_MEMBER.email);
});

test('Auto-provisioned OIDC user has correct attributes', async ({ page }) => {
test('Linked OIDC user retains local account attributes', async ({ page }) => {
const loginPage = new LoginPage(page);

// Given: OIDC user was created in the previous test
// When: User logs in via OIDC again
// Given: The local account was linked to the OIDC identity in the previous test
// When: User logs in via OIDC again — resolved directly by the linked oidcSubject
await loginPage.goto();
await loginPage.clickSSO();
await expect(page).toHaveURL(ROUTES.home, { timeout: 15000 });

// Then: /api/auth/me should return correct user details
// Then: /api/auth/me should return correct user details.
// Critically, authProvider stays 'local' — linking an existing account via
// OIDC never changes its auth provider or removes its password.
const meResponse = await page.request.get(API.authMe);
expect(meResponse.ok()).toBe(true);
const me = await meResponse.json();
expect(me.user).not.toBeNull();
expect(me.user.email).toBe(TEST_MEMBER.email);
expect(me.user.displayName).toBe(TEST_MEMBER.displayName);
expect(me.user.role).toBe('member');
expect(me.user.authProvider).toBe('oidc');
expect(me.user.authProvider).toBe('local');
});

test('OIDC user appears in admin user management', async ({ page, browser }) => {
const loginPage = new LoginPage(page);

// First: Complete OIDC login to ensure user exists
// First: Complete OIDC login (the account was already seeded + linked by
// earlier tests in this serial suite; this just re-confirms the session)
await loginPage.goto();
await loginPage.clickSSO();
await expect(page).toHaveURL(ROUTES.home, { timeout: 15000 });
Expand Down Expand Up @@ -127,4 +160,56 @@ test.describe('OIDC SSO Flow', () => {

await adminContext.close();
});

// ───────────────────────────────────────────────────────────────────────────
// Rejection path (#1865): OIDC email matches no existing account
//
// Coverage note: the mock OIDC provider (e2e/containers/oidcContainer.ts) is
// hardcoded to a single fixed identity (member@e2e-test.local), which by this
// point in the serial suite above is already a linked local account. Getting
// a genuinely SECOND, never-seeded identity out of the real container would
// require either an interactive-login form-fill step (changing the flow shape
// of every other test in this file, which all rely on non-interactive
// auto-grant) or an unverifiable per-request claim-mapping trick — and the
// token exchange happens server-to-server over the Docker network, so
// page.route() in the browser cannot intercept/vary it either. Given that
// constraint, this test exercises the same URL-param -> translated-banner
// code path a real rejection redirect produces (LoginPage.tsx reads
// `?error=` on mount), without depending on a second container identity.
// The real backend rejection path (403 OidcNoMatchingAccountError, no user
// row created) is covered by qa-integration-tester's route-level test.
test('Login page shows the rejection banner for oidc_no_matching_account and creates no account', async ({
page,
browser,
}) => {
const loginPage = new LoginPage(page);
const unmatchedEmail = 'no-such-oidc-user@e2e-test.local';

// When: Browser lands on /login with the rejection error code — exactly
// the redirect target the server uses for OidcNoMatchingAccountError
await page.goto(`${ROUTES.login}?error=oidc_no_matching_account`);

// Then: Browser stays on /login with the error code in the URL
await expect(page).toHaveURL(/\/login\?error=oidc_no_matching_account/);

// And: The translated rejection message is visible
await expect(loginPage.errorBanner).toBeVisible();
await expect(loginPage.errorBanner).toContainText(
'No account was found for your email address',
);

// And: No account exists for an email that was never provisioned — sanity
// check that rejection never has the side effect of creating a user
const adminContext = await browser.newContext({
storageState: 'test-results/.auth/admin.json',
});
const adminPage = await adminContext.newPage();
const usersResponse = await adminPage.request.get(
`${API.users}?q=${encodeURIComponent(unmatchedEmail)}`,
);
expect(usersResponse.ok()).toBe(true);
const { users } = (await usersResponse.json()) as { users: { email: string }[] };
expect(users.find((u) => u.email === unmatchedEmail)).toBeUndefined();
await adminContext.close();
});
});
7 changes: 7 additions & 0 deletions server/src/errors/AppError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,13 @@ export class AccountLockedError extends AppError {
}
}

export class OidcNoMatchingAccountError extends AppError {
constructor(message = 'No existing account matches this identity provider email address') {
super('OIDC_NO_MATCHING_ACCOUNT', 403, message);
this.name = 'OidcNoMatchingAccountError';
}
}

export class InvalidMetadataError extends AppError {
constructor(message = 'Metadata does not match schema for the entry type') {
super('INVALID_METADATA', 400, message);
Expand Down
Loading