From 8da7583ec622b1d4a0115142f3ebdc0d2cbeea81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 26 Sep 2026 16:10:32 +0200 Subject: [PATCH] fix(auth): link single sign-on to an existing account only after its own sign-in The SSO callback bound a verified provider email to the local account with the same address on the spot. The local address is not verified: anyone could register with it, or type it into their profile, and the SSO user then landed in an account the other person still held the password to. The callback now links nothing. It records the link it would make in a sealed, ten-minute, HttpOnly cookie and sends the browser to the sign-in page, and the link is made only when that browser then signs in to that same account with its password (and second factor) or passkey. Under an SSO-only policy the password form opens for exactly that account. The native sign-in answers oidc_link_required; the link is confirmed once on the web. Moving an account to a new email address now also needs a fresh proof: a sign-in or second factor in the last five minutes, or the current password sent alongside the change. Without it the address is left as it was and reported in rejectedFields; the rest of the save still lands. --- docs/api/openapi.yaml | 25 ++- messages/de.json | 14 +- messages/en.json | 14 +- messages/es.json | 14 +- messages/fr.json | 14 +- messages/it.json | 14 +- messages/ko.json | 14 +- messages/pl.json | 14 +- src/app/api/auth/login/route.ts | 15 +- .../oidc/callback/__tests__/route.test.ts | 38 ++-- src/app/api/auth/oidc/callback/route.ts | 28 ++- .../api/auth/passkey/login-verify/route.ts | 5 + src/app/api/auth/profile/route.ts | 18 +- src/app/api/user/profile/route.ts | 18 +- src/app/auth/login/page.tsx | 72 ++++--- .../password-card-placement.test.tsx | 6 +- .../settings/account-section/index.tsx | 31 +++ .../profile-update-email-limit.test.ts | 82 +++++++- src/lib/auth/__tests__/profile-update.test.ts | 13 +- src/lib/auth/existing-factor-proof.ts | 73 +++++++ src/lib/auth/login-response.ts | 8 + src/lib/auth/oidc-pending-link.ts | 143 +++++++++++++ src/lib/auth/profile-update.ts | 78 +++++++- src/lib/openapi/routes/auth.ts | 3 +- src/lib/openapi/routes/profile.ts | 7 + src/lib/profile/rejected-fields.ts | 4 + tests/integration/oidc-pending-link.test.ts | 189 ++++++++++++++++++ 27 files changed, 878 insertions(+), 76 deletions(-) create mode 100644 src/lib/auth/oidc-pending-link.ts create mode 100644 tests/integration/oidc-pending-link.test.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index e612d34e0..44f261259 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1853,8 +1853,9 @@ paths: - $ref: "#/components/schemas/MfaRequiredResponse" "401": *a1 "403": - description: Password login is disabled — the operator runs `OIDC_ONLY=true`. `meta.errorCode` = `oidc_only`; sign in - through SSO instead. + description: "Password login is disabled — the operator runs `OIDC_ONLY=true`. `meta.errorCode` = `oidc_only`; sign in + through SSO instead. The one exception is a browser the SSO callback sent back with a pending link to + confirm an existing account: it may sign in with that one account's password, which links the account." content: application/json: schema: @@ -3294,6 +3295,16 @@ paths: configure: the scheme is a compile-time constant, so the open-redirect class is removed by construction. + **An account that exists but is not linked yet.** When the provider's verified email matches a local account + with no SSO identity, nothing is linked and nobody is signed in: the local address was never verified, so + matching it proves nothing about who owns the account. The browser arm redirects to + `/auth/login?error=oidc_link_required` with a sealed, ten-minute, HttpOnly pending-link cookie; the person then + signs in to that account once with its own password (and second factor) or passkey, and that sign-in makes the + link. Under `OIDC_ONLY=true` the password sign-in is admitted for exactly the account the pending link names. + The native arm redirects to `healthlog://oidc-callback?error=oidc_link_required`; the link is confirmed once on + the web, after which the app's SSO sign-in works. + + Two PKCE exchanges run here and must not be confused: the server↔provider verifier the server generates for itself, and the app↔server challenge supplied above. The native flag and the app's challenge travel only inside an AES-256-GCM state cookie, so the callback branches on tamper-authenticated state rather than on anything the @@ -22435,6 +22446,16 @@ components: anyOf: - type: string - type: "null" + currentPassword: + description: "The account password, sent only alongside a NEW `email`. Moving the account to a new address needs a fresh + proof because single sign-on links an existing account by it: a browser session that signed in or completed + a second factor in the last five minutes needs nothing more, otherwise this field must carry the current + password. Without an accepted proof the address is not changed and comes back in `rejectedFields` as + `reproof_required` or `reproof_failed` (or, when the address was the only field sent, 401 + `auth.reproof.required` / `auth.reproof.failed`). Attempts draw on the account's shared re-proof budget of + five per fifteen minutes. Never stored." + type: string + maxLength: 512 description: Partial profile update. Every field is optional and an omitted field is left untouched. An explicit null clears any nullable field; an empty string also clears `displayName`, `dateOfBirth`, `gender`, `fullName`, `insurerName`, `insuranceNumber` and `insurerIkNumber`. `email` is not among them — it is validated as an diff --git a/messages/de.json b/messages/de.json index bfdb8da39..de3131ffc 100644 --- a/messages/de.json +++ b/messages/de.json @@ -128,6 +128,12 @@ "emailInUse": "Diese E-Mail-Adresse wird bereits verwendet.", "emailRateLimited": "Zu viele Änderungen der E-Mail-Adresse. Versuche es später erneut." } + }, + "auth": { + "reproof": { + "required": "Bestätige dein aktuelles Passwort, um das zu ändern.", + "failed": "Die Bestätigung ist fehlgeschlagen. Es wurde nichts geändert." + } } }, "doctorReport": { @@ -439,6 +445,7 @@ "errorNoEmail": "Dein Identitätsanbieter hat keine bestätigte E-Mail-Adresse übermittelt.", "errorEmailUnverified": "Dein Identitätsanbieter hat deine E-Mail-Adresse nicht als bestätigt markiert.", "errorIdentityConflict": "Diese E-Mail-Adresse gehört zu einem Konto, das bereits mit einer anderen SSO-Identität verknüpft ist.", + "errorLinkRequired": "Hier gibt es bereits ein Konto mit dieser E-Mail-Adresse. Melde dich einmal mit seinem Passwort oder Passkey an, um es mit Single Sign-on zu verbinden.", "errorRegistrationDisabled": "Kein Konto passt zu dieser Identität, und neue Registrierungen sind derzeit geschlossen.", "errorFailed": "Single Sign-On ist fehlgeschlagen. Bitte versuche es erneut." }, @@ -5553,7 +5560,9 @@ "invalidFormat": "Das ist nicht in einer Form geschrieben, die dieses Feld lesen kann.", "invalidType": "Der Wert kam nicht in einer Form an, die dieses Feld speichern kann.", "rateLimited": "Zu oft geändert. Warte kurz und versuche es dann erneut.", - "other": "Dieser Wert wurde nicht angenommen. Bitte prüfen und erneut versuchen." + "other": "Dieser Wert wurde nicht angenommen. Bitte prüfen und erneut versuchen.", + "reproofRequired": "Gib dein aktuelles Passwort ein, um die E-Mail-Adresse zu ändern.", + "reproofFailed": "Das Passwort stimmt nicht, die E-Mail-Adresse wurde nicht geändert." }, "avatar": { "title": "Profilfoto", @@ -6700,7 +6709,8 @@ "showChecklist": "Checkliste wieder anzeigen", "checklistShown": "Die Checkliste ist wieder auf dem Dashboard.", "checklistNothingHidden": "Die Checkliste war nicht ausgeblendet; sie öffnet sich auf dem Dashboard." - } + }, + "emailChangePassword": "Aktuelles Passwort (nötig, um die E-Mail-Adresse zu ändern)" }, "admin": { "title": "Administration", diff --git a/messages/en.json b/messages/en.json index 8abfade86..66715d4b5 100644 --- a/messages/en.json +++ b/messages/en.json @@ -128,6 +128,12 @@ "emailInUse": "This email address is already in use.", "emailRateLimited": "Too many email-address changes. Try again later." } + }, + "auth": { + "reproof": { + "required": "Confirm your current password to change this.", + "failed": "That did not verify. Nothing was changed." + } } }, "doctorReport": { @@ -439,6 +445,7 @@ "errorNoEmail": "Your identity provider did not share a verified email address.", "errorEmailUnverified": "Your identity provider did not mark your email address as verified.", "errorIdentityConflict": "That email address belongs to an account that is already linked to a different SSO identity.", + "errorLinkRequired": "An account with this email address already exists here. Sign in to it once with its password or passkey to connect it to single sign-on.", "errorRegistrationDisabled": "No account matches that identity, and new sign-ups are currently closed.", "errorFailed": "Single sign-on failed. Please try again." }, @@ -5553,7 +5560,9 @@ "invalidFormat": "That is not written in a form this field can read.", "invalidType": "That value did not arrive in a form this field can store.", "rateLimited": "Changed too often. Wait a little and try again.", - "other": "This value was not accepted. Check it and try again." + "other": "This value was not accepted. Check it and try again.", + "reproofRequired": "Enter your current password to change the email address.", + "reproofFailed": "That password is not correct, so the email address was not changed." }, "avatar": { "title": "Profile photo", @@ -6700,7 +6709,8 @@ "showChecklist": "Show the checklist again", "checklistShown": "The checklist is back on the dashboard.", "checklistNothingHidden": "The checklist was not hidden; it opens on the dashboard." - } + }, + "emailChangePassword": "Current password (needed to change the email address)" }, "admin": { "title": "Administration", diff --git a/messages/es.json b/messages/es.json index ca8347a0e..a442d7db2 100644 --- a/messages/es.json +++ b/messages/es.json @@ -128,6 +128,12 @@ "emailInUse": "Esta dirección de correo ya está en uso.", "emailRateLimited": "Demasiados cambios de dirección de correo. Inténtalo más tarde." } + }, + "auth": { + "reproof": { + "required": "Confirma tu contraseña actual para cambiar esto.", + "failed": "La verificación ha fallado. No se ha cambiado nada." + } } }, "doctorReport": { @@ -439,6 +445,7 @@ "errorNoEmail": "Tu proveedor de identidad no compartió un correo electrónico verificado.", "errorEmailUnverified": "Tu proveedor de identidad no marcó tu correo electrónico como verificado.", "errorIdentityConflict": "Ese correo electrónico pertenece a una cuenta que ya está vinculada a otra identidad SSO.", + "errorLinkRequired": "Ya existe aquí una cuenta con esta dirección de correo. Inicia sesión en ella una vez con su contraseña o llave de acceso para conectarla al inicio de sesión único.", "errorRegistrationDisabled": "Ninguna cuenta coincide con esa identidad y los nuevos registros están cerrados actualmente.", "errorFailed": "El inicio de sesión único falló. Inténtalo de nuevo." }, @@ -5553,7 +5560,9 @@ "invalidFormat": "Eso no está escrito en un formato que este campo pueda leer.", "invalidType": "El valor no llegó en un formato que este campo pueda guardar.", "rateLimited": "Se ha cambiado demasiadas veces. Espera un poco e inténtalo de nuevo.", - "other": "No se aceptó este valor. Revísalo e inténtalo de nuevo." + "other": "No se aceptó este valor. Revísalo e inténtalo de nuevo.", + "reproofRequired": "Introduce tu contraseña actual para cambiar la dirección de correo.", + "reproofFailed": "La contraseña no es correcta, así que la dirección de correo no se ha cambiado." }, "avatar": { "title": "Foto de perfil", @@ -6700,7 +6709,8 @@ "showChecklist": "Mostrar la lista de nuevo", "checklistShown": "La lista vuelve a estar en el panel.", "checklistNothingHidden": "La lista no estaba oculta; se abre en el panel." - } + }, + "emailChangePassword": "Contraseña actual (necesaria para cambiar la dirección de correo)" }, "admin": { "title": "Administration", diff --git a/messages/fr.json b/messages/fr.json index 0600cc41c..16b933bae 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -128,6 +128,12 @@ "emailInUse": "Cette adresse e-mail est déjà utilisée.", "emailRateLimited": "Trop de changements d'adresse e-mail. Réessayez plus tard." } + }, + "auth": { + "reproof": { + "required": "Confirmez votre mot de passe actuel pour modifier ceci.", + "failed": "La vérification a échoué. Rien n'a été modifié." + } } }, "doctorReport": { @@ -439,6 +445,7 @@ "errorNoEmail": "Votre fournisseur d'identité n'a pas transmis d'adresse e-mail vérifiée.", "errorEmailUnverified": "Votre fournisseur d'identité n'a pas marqué votre adresse e-mail comme vérifiée.", "errorIdentityConflict": "Cette adresse e-mail appartient à un compte déjà lié à une autre identité SSO.", + "errorLinkRequired": "Un compte avec cette adresse e-mail existe déjà ici. Connectez-vous-y une fois avec son mot de passe ou sa clé d'accès pour le relier à l'authentification unique.", "errorRegistrationDisabled": "Aucun compte ne correspond à cette identité et les nouvelles inscriptions sont actuellement fermées.", "errorFailed": "L'authentification unique a échoué. Veuillez réessayer." }, @@ -5553,7 +5560,9 @@ "invalidFormat": "Ce n'est pas écrit dans un format que ce champ peut lire.", "invalidType": "La valeur n'est pas arrivée dans un format que ce champ peut enregistrer.", "rateLimited": "Modifié trop souvent. Patientez un peu et réessayez.", - "other": "Cette valeur n'a pas été acceptée. Vérifiez-la et réessayez." + "other": "Cette valeur n'a pas été acceptée. Vérifiez-la et réessayez.", + "reproofRequired": "Saisissez votre mot de passe actuel pour changer l'adresse e-mail.", + "reproofFailed": "Ce mot de passe est incorrect, l'adresse e-mail n'a donc pas été modifiée." }, "avatar": { "title": "Photo de profil", @@ -6700,7 +6709,8 @@ "showChecklist": "Réafficher la liste de départ", "checklistShown": "La liste de départ est de retour sur le tableau de bord.", "checklistNothingHidden": "La liste n'était pas masquée ; elle s'ouvre sur le tableau de bord." - } + }, + "emailChangePassword": "Mot de passe actuel (nécessaire pour changer l'adresse e-mail)" }, "admin": { "title": "Administration", diff --git a/messages/it.json b/messages/it.json index 27d24cb9b..7f501602f 100644 --- a/messages/it.json +++ b/messages/it.json @@ -128,6 +128,12 @@ "emailInUse": "Questo indirizzo email è già in uso.", "emailRateLimited": "Troppi cambi di indirizzo email. Riprova più tardi." } + }, + "auth": { + "reproof": { + "required": "Conferma la password attuale per modificarlo.", + "failed": "La verifica non è riuscita. Non è stato modificato nulla." + } } }, "doctorReport": { @@ -439,6 +445,7 @@ "errorNoEmail": "Il tuo provider di identità non ha condiviso un indirizzo email verificato.", "errorEmailUnverified": "Il tuo provider di identità non ha contrassegnato il tuo indirizzo email come verificato.", "errorIdentityConflict": "Questo indirizzo email appartiene a un account già collegato a un'altra identità SSO.", + "errorLinkRequired": "Esiste già un account con questo indirizzo email. Accedi una volta con la sua password o passkey per collegarlo al single sign-on.", "errorRegistrationDisabled": "Nessun account corrisponde a questa identità e le nuove registrazioni sono attualmente chiuse.", "errorFailed": "L'accesso Single Sign-On non è riuscito. Riprova." }, @@ -5553,7 +5560,9 @@ "invalidFormat": "Non è scritto in un formato che questo campo possa leggere.", "invalidType": "Il valore non è arrivato in un formato che questo campo possa salvare.", "rateLimited": "Modificato troppe volte. Aspetta un momento e riprova.", - "other": "Questo valore non è stato accettato. Controllalo e riprova." + "other": "Questo valore non è stato accettato. Controllalo e riprova.", + "reproofRequired": "Inserisci la password attuale per cambiare l'indirizzo email.", + "reproofFailed": "La password non è corretta, quindi l'indirizzo email non è stato cambiato." }, "avatar": { "title": "Foto del profilo", @@ -6700,7 +6709,8 @@ "showChecklist": "Mostra di nuovo la lista", "checklistShown": "La lista è di nuovo nella dashboard.", "checklistNothingHidden": "La lista non era nascosta; si apre nella dashboard." - } + }, + "emailChangePassword": "Password attuale (necessaria per cambiare l'indirizzo email)" }, "admin": { "title": "Administration", diff --git a/messages/ko.json b/messages/ko.json index e5837c96f..ce93f5291 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -128,6 +128,12 @@ "emailInUse": "이미 사용 중인 이메일 주소예요.", "emailRateLimited": "이메일 주소를 너무 자주 변경했어요. 잠시 후 다시 시도해 주세요." } + }, + "auth": { + "reproof": { + "required": "변경하려면 현재 비밀번호를 확인하세요.", + "failed": "확인에 실패했습니다. 아무것도 변경되지 않았습니다." + } } }, "doctorReport": { @@ -439,6 +445,7 @@ "errorNoEmail": "제공자가 인증된 이메일 주소를 전달하지 않았어요.", "errorEmailUnverified": "제공자가 이메일 주소를 인증됨으로 표시하지 않았어요.", "errorIdentityConflict": "그 이메일 주소는 다른 SSO 계정에 이미 연결돼 있어요.", + "errorLinkRequired": "이 이메일 주소를 사용하는 계정이 이미 있습니다. 싱글 사인온에 연결하려면 해당 계정의 비밀번호나 패스키로 한 번 로그인하세요.", "errorRegistrationDisabled": "일치하는 계정이 없고, 지금은 회원가입이 닫혀 있어요.", "errorFailed": "싱글 사인온에 실패했어요. 다시 시도해 주세요." }, @@ -5553,7 +5560,9 @@ "invalidFormat": "이 항목이 읽을 수 있는 형식이 아니에요.", "invalidType": "이 항목이 저장할 수 있는 형식으로 오지 않았어요.", "rateLimited": "너무 자주 바꿨어요. 잠시 후 다시 시도해 주세요.", - "other": "이 값은 저장하지 못했어요. 확인한 뒤 다시 시도해 주세요." + "other": "이 값은 저장하지 못했어요. 확인한 뒤 다시 시도해 주세요.", + "reproofRequired": "이메일 주소를 변경하려면 현재 비밀번호를 입력하세요.", + "reproofFailed": "비밀번호가 올바르지 않아 이메일 주소가 변경되지 않았습니다." }, "avatar": { "title": "프로필 사진", @@ -6700,7 +6709,8 @@ "showChecklist": "체크리스트 다시 표시", "checklistShown": "체크리스트가 대시보드에 다시 표시됩니다.", "checklistNothingHidden": "체크리스트는 숨겨져 있지 않았습니다. 대시보드에서 열립니다." - } + }, + "emailChangePassword": "현재 비밀번호 (이메일 주소를 변경하려면 필요)" }, "admin": { "title": "관리자", diff --git a/messages/pl.json b/messages/pl.json index 30f7639aa..30f4d34e6 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -128,6 +128,12 @@ "emailInUse": "Ten adres e-mail jest już używany.", "emailRateLimited": "Zbyt wiele zmian adresu e-mail. Spróbuj ponownie później." } + }, + "auth": { + "reproof": { + "required": "Potwierdź obecne hasło, aby to zmienić.", + "failed": "Weryfikacja nie powiodła się. Nic nie zostało zmienione." + } } }, "doctorReport": { @@ -439,6 +445,7 @@ "errorNoEmail": "Twój dostawca tożsamości nie przekazał zweryfikowanego adresu e-mail.", "errorEmailUnverified": "Twój dostawca tożsamości nie oznaczył Twojego adresu e-mail jako zweryfikowanego.", "errorIdentityConflict": "Ten adres e-mail należy do konta powiązanego już z inną tożsamością SSO.", + "errorLinkRequired": "Konto z tym adresem e-mail już tu istnieje. Zaloguj się do niego raz hasłem lub kluczem dostępu, aby połączyć je z logowaniem jednokrotnym.", "errorRegistrationDisabled": "Żadne konto nie pasuje do tej tożsamości, a nowe rejestracje są obecnie zamknięte.", "errorFailed": "Logowanie jednokrotne nie powiodło się. Spróbuj ponownie." }, @@ -5553,7 +5560,9 @@ "invalidFormat": "To nie jest zapisane w formacie, który to pole może odczytać.", "invalidType": "Wartość nie dotarła w formacie, który to pole może zapisać.", "rateLimited": "Zmieniane zbyt często. Odczekaj chwilę i spróbuj ponownie.", - "other": "Ta wartość nie została przyjęta. Sprawdź ją i spróbuj ponownie." + "other": "Ta wartość nie została przyjęta. Sprawdź ją i spróbuj ponownie.", + "reproofRequired": "Wpisz obecne hasło, aby zmienić adres e-mail.", + "reproofFailed": "Hasło jest nieprawidłowe, więc adres e-mail nie został zmieniony." }, "avatar": { "title": "Zdjęcie profilowe", @@ -6700,7 +6709,8 @@ "showChecklist": "Pokaż listę ponownie", "checklistShown": "Lista wróciła na panel.", "checklistNothingHidden": "Lista nie była ukryta; otwiera się na panelu." - } + }, + "emailChangePassword": "Obecne hasło (potrzebne do zmiany adresu e-mail)" }, "admin": { "title": "Administracja", diff --git a/src/app/api/auth/login/route.ts b/src/app/api/auth/login/route.ts index 5c163c0dd..63e285c6a 100644 --- a/src/app/api/auth/login/route.ts +++ b/src/app/api/auth/login/route.ts @@ -19,12 +19,19 @@ import { createMfaChallenge } from "@/lib/auth/mfa/challenge"; import { consumeTrustedDevice } from "@/lib/auth/trusted-device"; import { syncMfaEnrollCookie } from "@/lib/auth/mfa-enrollment"; import { isOidcOnly } from "@/lib/auth/oidc"; +import { readPendingLink } from "@/lib/auth/oidc-pending-link"; export const POST = apiHandler(async (request: NextRequest) => { // OIDC_ONLY means password login must be a dead end, not just a hidden // button — otherwise a pre-existing password (or a leaked/reused one) // stays a live bypass of the operator's SSO-only policy. - if (isOidcOnly()) { + // + // One exception, and only for one account: a browser the SSO callback sent + // here to confirm an existing account carries a sealed pending link naming + // that account. It may prove that account's password (checked again once + // the account is resolved) so the link can be made; nothing else opens. + const pendingLink = isOidcOnly() ? await readPendingLink() : null; + if (isOidcOnly() && !pendingLink) { return apiError("Password login is disabled. Sign in with SSO.", 403, { errorCode: "oidc_only", }); @@ -119,6 +126,12 @@ export const POST = apiHandler(async (request: NextRequest) => { return apiError("Invalid credentials", 401); } + if (pendingLink && pendingLink.userId !== user.id) { + return apiError("Password login is disabled. Sign in with SSO.", 403, { + errorCode: "oidc_only", + }); + } + const ua = request.headers.get("user-agent"); await auditLog("auth.login.password", { diff --git a/src/app/api/auth/oidc/callback/__tests__/route.test.ts b/src/app/api/auth/oidc/callback/__tests__/route.test.ts index 769eeff35..370884fba 100644 --- a/src/app/api/auth/oidc/callback/__tests__/route.test.ts +++ b/src/app/api/auth/oidc/callback/__tests__/route.test.ts @@ -16,6 +16,7 @@ vi.mock("@/lib/rate-limit", () => ({ vi.mock("@/lib/crypto", () => ({ decrypt: vi.fn((s: string) => s.replace(/^enc\(/, "").replace(/\)$/, "")), + encrypt: vi.fn((s: string) => `enc(${s})`), })); vi.mock("@/lib/db", () => ({ @@ -321,26 +322,34 @@ describe("GET /api/auth/oidc/callback", () => { expect(res.status).toBe(307); }); - it("links an existing email-matched account ONCE, stamping (issuer, sub)", async () => { + // Inverted in v1.39.3. This used to pin the auto-link: an existing account + // with the same (unverified) local email was bound on the spot and signed + // in. Whoever set that address first owned the account the SSO user landed + // in, so the callback now links nothing and signs nobody in; it leaves a + // sealed pending link and sends the browser to confirm the account with its + // own credential (`oidc-pending-link.test.ts` covers the completion). + it("does not link an email-matched account on the email alone", async () => { mockIdentity(); mockUserLookups({ byIdentity: null, byEmail: userRow() }); - vi.mocked(prisma.user.update).mockResolvedValue( - userRow({ oidcIssuer: METADATA.issuer, oidcSub: "sub-1" }) as never, - ); const res = await GET(validRequest("/dashboard")); expect(prisma.user.create).not.toHaveBeenCalled(); - expect(prisma.user.update).toHaveBeenCalledWith({ - where: { id: "user-1" }, - data: { oidcIssuer: METADATA.issuer, oidcSub: "sub-1" }, - }); - expect(auditLog).toHaveBeenCalledWith( + expect(prisma.user.update).not.toHaveBeenCalled(); + expect(createSession).not.toHaveBeenCalled(); + expect(auditLog).not.toHaveBeenCalledWith( "auth.oidc.linked", + expect.anything(), + ); + expect(auditLog).toHaveBeenCalledWith( + "auth.oidc.link_pending", expect.objectContaining({ userId: "user-1" }), ); - expect(createSession).toHaveBeenCalled(); - expect(res.headers.get("location")).toContain("/dashboard"); + expect(res.headers.get("location")).toContain("error=oidc_link_required"); + const setCookie = res.headers.get("set-cookie") ?? ""; + expect(setCookie).toContain("hl_oidc_link="); + expect(setCookie.toLowerCase()).toContain("httponly"); + expect(setCookie).toContain("Path=/api/auth"); }); it("rejects a login whose email matches an account bound to a DIFFERENT identity", async () => { @@ -505,10 +514,9 @@ describe("GET /api/auth/oidc/callback", () => { it("never stamps the session step-up fresh — the 5th createSession arg is null", async () => { mockIdentity(); - mockUserLookups({ byIdentity: null, byEmail: userRow() }); - vi.mocked(prisma.user.update).mockResolvedValue( - userRow({ oidcIssuer: METADATA.issuer, oidcSub: "sub-1" }) as never, - ); + mockUserLookups({ + byIdentity: userRow({ oidcIssuer: METADATA.issuer, oidcSub: "sub-1" }), + }); await GET(validRequest()); diff --git a/src/app/api/auth/oidc/callback/route.ts b/src/app/api/auth/oidc/callback/route.ts index efa1221b5..0c090f4a0 100644 --- a/src/app/api/auth/oidc/callback/route.ts +++ b/src/app/api/auth/oidc/callback/route.ts @@ -37,6 +37,7 @@ import { buildNativeCallbackUrl, mintNativeHandoff, } from "@/lib/auth/oidc-native-handoff"; +import { setPendingLinkCookie } from "@/lib/auth/oidc-pending-link"; const LOGIN_ERROR_URL = "/auth/login"; @@ -340,14 +341,29 @@ export const GET = apiHandler(async (req: NextRequest) => { annotate({ meta: { reason: "identity_conflict" } }); return failRedirect("oidc_identity_conflict"); } - user = await prisma.user.update({ - where: { id: byEmail.id }, - data: { oidcIssuer: metadata.issuer, oidcSub: identity.sub }, - }); - await auditLog("auth.oidc.linked", { - userId: user.id, + // (2b) Never link on the email alone. The local address was never + // verified — anyone can register with it or type it into a profile — + // so matching it proves nothing about who owns the account. Record + // the link this login would make and send the browser to sign in to + // the account with its own credential; the link is made there + // (`completePendingOidcLink`), and only for this account. + await auditLog("auth.oidc.link_pending", { + userId: byEmail.id, ipAddress: ip, }); + annotate({ meta: { reason: "link_confirmation_required" } }); + if (isNative) { + // The native sign-in sheet cannot run the confirmation; the person + // signs in on the web once, after which the app's SSO works. + return failRedirect("oidc_link_required"); + } + const response = errorRedirect("oidc_link_required"); + setPendingLinkCookie(response, { + userId: byEmail.id, + issuer: metadata.issuer, + sub: identity.sub, + }); + return response; } else { // (3) Provision. const userCount = await prisma.user.count(); diff --git a/src/app/api/auth/passkey/login-verify/route.ts b/src/app/api/auth/passkey/login-verify/route.ts index 9b2b36878..4032fdb15 100644 --- a/src/app/api/auth/passkey/login-verify/route.ts +++ b/src/app/api/auth/passkey/login-verify/route.ts @@ -18,6 +18,7 @@ import { } from "@/lib/auth/native-client"; import { issueAccessAndRefresh } from "@/lib/auth/refresh-token"; import { isOidcOnly } from "@/lib/auth/oidc"; +import { completePendingOidcLink } from "@/lib/auth/oidc-pending-link"; export const POST = apiHandler(async (request: NextRequest) => { // OIDC_ONLY must block passkey login too, not just password login — a @@ -87,6 +88,10 @@ export const POST = apiHandler(async (request: NextRequest) => { // meets an MFA-enforcement policy AND passes `requireFreshMfa` step-up, // exactly like a completed password+TOTP login. (The password+TOTP path is // unchanged — it stamps the session at `/api/auth/mfa/verify`.) + // The passkey proved this account's own credential, so a single sign-on + // identity this browser was sent here to confirm may now be linked to it. + await completePendingOidcLink(user.id, ip ?? null); + const ua = request.headers.get("user-agent"); await createSession( user.id, diff --git a/src/app/api/auth/profile/route.ts b/src/app/api/auth/profile/route.ts index 515a3e272..3497f6ba8 100644 --- a/src/app/api/auth/profile/route.ts +++ b/src/app/api/auth/profile/route.ts @@ -9,16 +9,30 @@ import { NextRequest } from "next/server"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; import { applyProfileUpdate } from "@/lib/auth/profile-update"; +import { authorizeSensitiveChange } from "@/lib/auth/existing-factor-proof"; export const PUT = apiHandler(async (request: NextRequest) => { - const { user } = await requireAuth(); + const auth = await requireAuth(); + const { user } = auth; const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 64 * 1024, }); if (jsonError) return jsonError; - const result = await applyProfileUpdate(user.id, body, getClientIp(request)); + const ip = getClientIp(request); + const result = await applyProfileUpdate(user.id, body, ip, { + // A new email address needs a fresh proof: the address is what single + // sign-on links an existing account by. + authorizeEmailChange: (currentPassword) => + authorizeSensitiveChange({ + user, + cookieSessionId: auth.authMethod === "cookie" ? auth.session.id : null, + currentPassword, + ipAddress: ip, + stage: "email_change", + }), + }); if (!result.ok) { const meta = result.errorCode ? { errorCode: result.errorCode } : undefined; return result.issues diff --git a/src/app/api/user/profile/route.ts b/src/app/api/user/profile/route.ts index 38e3dd77f..efa35a471 100644 --- a/src/app/api/user/profile/route.ts +++ b/src/app/api/user/profile/route.ts @@ -17,6 +17,7 @@ import { } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; import { applyProfileUpdate } from "@/lib/auth/profile-update"; +import { authorizeSensitiveChange } from "@/lib/auth/existing-factor-proof"; import { prisma } from "@/lib/db"; import { decrypt } from "@/lib/crypto"; import { resolveModuleMap } from "@/lib/modules/gate"; @@ -97,14 +98,27 @@ export const GET = apiHandler(async () => { }); export const PATCH = apiHandler(async (request: NextRequest) => { - const { user } = await requireAuth(); + const auth = await requireAuth(); + const { user } = auth; const { data: body, error } = await safeJson(request, { maxBytes: 64 * 1024, }); if (error) return error; - const result = await applyProfileUpdate(user.id, body, getClientIp(request)); + const ip = getClientIp(request); + const result = await applyProfileUpdate(user.id, body, ip, { + // A new email address needs a fresh proof: the address is what single + // sign-on links an existing account by. + authorizeEmailChange: (currentPassword) => + authorizeSensitiveChange({ + user, + cookieSessionId: auth.authMethod === "cookie" ? auth.session.id : null, + currentPassword, + ipAddress: ip, + stage: "email_change", + }), + }); if (!result.ok) { const meta = result.errorCode ? { errorCode: result.errorCode } : undefined; return result.issues diff --git a/src/app/auth/login/page.tsx b/src/app/auth/login/page.tsx index b6f739670..571c9aeb7 100644 --- a/src/app/auth/login/page.tsx +++ b/src/app/auth/login/page.tsx @@ -32,7 +32,14 @@ export default function LoginPage() { const searchParams = useSearchParams(); const queryClient = useQueryClient(); const { t } = useTranslations(); - const [mode, setMode] = useState<"passkey" | "password">("passkey"); + // The SSO callback sends a browser here when its email matches an account + // that exists but is not linked yet: the person signs in to that account + // once with its own credential, which links it. Under an SSO-only policy + // this is the one case the password form is shown for. + const linkPending = searchParams.get("error") === "oidc_link_required"; + const [mode, setMode] = useState<"passkey" | "password">( + linkPending ? "password" : "passkey", + ); const [email, setEmail] = useState(""); const [password, setPassword] = useState(""); const [error, setError] = useState(() => { @@ -43,6 +50,7 @@ export default function LoginPage() { oidc_no_email: "auth.oidc.errorNoEmail", oidc_email_unverified: "auth.oidc.errorEmailUnverified", oidc_identity_conflict: "auth.oidc.errorIdentityConflict", + oidc_link_required: "auth.oidc.errorLinkRequired", oidc_registration_disabled: "auth.oidc.errorRegistrationDisabled", }; return t(key[oidcError] ?? "auth.oidc.errorFailed"); @@ -394,33 +402,37 @@ export default function LoginPage() { )} - {!oidcStatus?.only && ( + {(!oidcStatus?.only || linkPending) && ( <> {/* Phase A5 / B-mobile: bumped from default size (h-9, 36px) to size="lg" so the login CTAs meet WCAG 2.5.5 (44px minimum) on mobile. Login is the most-tapped flow on a fresh install. */} - + {!oidcStatus?.only && ( + <> + -
- - - {t("common.or")} - - -
+
+ + + {t("common.or")} + + +
+ + )} {mode === "passkey" ? ( + {!oidcStatus?.only && ( + + )} )} diff --git a/src/components/settings/__tests__/password-card-placement.test.tsx b/src/components/settings/__tests__/password-card-placement.test.tsx index de120af1e..e08ced3b9 100644 --- a/src/components/settings/__tests__/password-card-placement.test.tsx +++ b/src/components/settings/__tests__/password-card-placement.test.tsx @@ -116,7 +116,11 @@ describe("change-password card placement", () => { "settings.passwordReset", "settings.changePassword", "/api/auth/password", - "current-password", + // A change-password form is the pair current + new. The Account form + // asks for the CURRENT password alone, beside a new email address, + // which is a proof and not a password change; the new-password half is + // what would mean the old form came back. + "new-password", ]) { expect(account, `Account still carries ${marker}`).not.toContain(marker); } diff --git a/src/components/settings/account-section/index.tsx b/src/components/settings/account-section/index.tsx index e07b9c7fa..3228186b5 100644 --- a/src/components/settings/account-section/index.tsx +++ b/src/components/settings/account-section/index.tsx @@ -71,6 +71,10 @@ export function AccountSection() { ); const [email, setEmail] = useState(""); + // A new email address needs the current password unless the session signed + // in within the last few minutes; the server decides and names the field if + // it wanted one. Never stored beyond this form. + const [emailPassword, setEmailPassword] = useState(""); const [height, setHeight] = useState(EMPTY_HEIGHT_DRAFT); const [dateOfBirth, setDateOfBirth] = useState(""); const [gender, setGender] = useState(""); @@ -185,6 +189,10 @@ export function AccountSection() { navigate, }); + const emailChanged = + email.trim() !== "" && + email.trim().toLowerCase() !== profileSeed.email.trim().toLowerCase(); + async function handleSaveProfile(e: React.FormEvent) { e.preventDefault(); setSaving(true); @@ -212,6 +220,9 @@ export function AccountSection() { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ email: savedProfile.email || null, + ...(emailChanged && emailPassword + ? { currentPassword: emailPassword } + : {}), heightCm: heightAdapter.toCanonicalCm(savedProfile.height), dateOfBirth: savedProfile.dateOfBirth || null, gender: savedProfile.gender || null, @@ -229,6 +240,8 @@ export function AccountSection() { : Promise.resolve({ ok: true } as Response), ]); + setEmailPassword(""); + if (profileRes.ok && tzRes.ok) { // Instant same-tab flip of every rendered timestamp (issue #490): // mirror the accepted zone before the `/me` refetch confirms it — @@ -373,6 +386,24 @@ export function AccountSection() { aria-describedby={fieldErrors.email ? "email-error" : undefined} /> + {emailChanged && ( +
+ + { + setEmailPassword(e.target.value); + clearFieldError("email"); + }} + autoComplete="current-password" + maxLength={512} + /> +
+ )} diff --git a/src/lib/auth/__tests__/profile-update-email-limit.test.ts b/src/lib/auth/__tests__/profile-update-email-limit.test.ts index 7693c5ca3..c8806ac1f 100644 --- a/src/lib/auth/__tests__/profile-update-email-limit.test.ts +++ b/src/lib/auth/__tests__/profile-update-email-limit.test.ts @@ -33,7 +33,18 @@ vi.mock("@/lib/rate-limit", () => ({ checkProfileEmailRateLimit: vi.fn(), })); -import { applyProfileUpdate } from "../profile-update"; +import { applyProfileUpdate as applyWithProof } from "../profile-update"; + +/** These suites are about validation and the email budget, not the proof: + * every call is made as an owner who has just proved themselves. */ +const applyProfileUpdate = ( + userId: string, + body: unknown, + ipAddress?: string | null, +) => + applyWithProof(userId, body, ipAddress, { + authorizeEmailChange: async () => "ok", + }); import { prisma } from "@/lib/db"; import { auditLog } from "@/lib/auth/audit"; import { checkProfileEmailRateLimit } from "@/lib/rate-limit"; @@ -238,3 +249,72 @@ describe("applyProfileUpdate — the email conflict is metered", () => { expect(checkProfileEmailRateLimit).toHaveBeenCalledTimes(1); }); }); + +describe("applyProfileUpdate — a new address needs a fresh proof", () => { + it("refuses an address-only change without asking about the address", async () => { + const authorizeEmailChange = vi.fn(async () => "required" as const); + const result = await applyWithProof( + USER_ID, + { email: "someone@example.test" }, + null, + { authorizeEmailChange }, + ); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.status).toBe(401); + expect(result.errorCode).toBe("auth.reproof.required"); + } + expect(emailProbes()).toHaveLength(0); + expect(checkProfileEmailRateLimit).not.toHaveBeenCalled(); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it("drops only the address when the save carries other fields", async () => { + const result = await applyWithProof( + USER_ID, + { email: "someone@example.test", heightCm: 180 }, + null, + { authorizeEmailChange: async () => "failed" }, + ); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.rejectedFields).toEqual([ + expect.objectContaining({ path: "email", code: "reproof_failed" }), + ]); + } + const written = vi.mocked(prisma.user.update).mock.calls[0]?.[0] as { + data: Record; + }; + expect(written.data).not.toHaveProperty("email"); + expect(written.data).toHaveProperty("heightCm", 180); + }); + + it("hands the body's current password to the proof and never writes it", async () => { + const authorizeEmailChange = vi.fn(async () => "ok" as const); + await applyWithProof( + USER_ID, + { email: "someone@example.test", currentPassword: "hunter2hunter2" }, + null, + { authorizeEmailChange }, + ); + expect(authorizeEmailChange).toHaveBeenCalledWith("hunter2hunter2"); + const written = vi.mocked(prisma.user.update).mock.calls[0]?.[0] as { + data: Record; + }; + expect(JSON.stringify(written.data)).not.toContain("hunter2"); + }); + + it("does not ask when the address on file is re-posted", async () => { + const authorizeEmailChange = vi.fn(async () => "required" as const); + const result = await applyWithProof( + USER_ID, + { email: ON_FILE, heightCm: 170 }, + null, + { authorizeEmailChange }, + ); + expect(result.ok).toBe(true); + expect(authorizeEmailChange).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/auth/__tests__/profile-update.test.ts b/src/lib/auth/__tests__/profile-update.test.ts index eb89c0ff4..f12f45e91 100644 --- a/src/lib/auth/__tests__/profile-update.test.ts +++ b/src/lib/auth/__tests__/profile-update.test.ts @@ -16,7 +16,18 @@ vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn().mockResolvedValue(undefined), })); -import { applyProfileUpdate } from "../profile-update"; +import { applyProfileUpdate as applyWithProof } from "../profile-update"; + +/** These suites are about validation and the email budget, not the proof: + * every call is made as an owner who has just proved themselves. */ +const applyProfileUpdate = ( + userId: string, + body: unknown, + ipAddress?: string | null, +) => + applyWithProof(userId, body, ipAddress, { + authorizeEmailChange: async () => "ok", + }); import { prisma } from "@/lib/db"; const USER_ID = "user-1"; diff --git a/src/lib/auth/existing-factor-proof.ts b/src/lib/auth/existing-factor-proof.ts index baef55342..6740aab81 100644 --- a/src/lib/auth/existing-factor-proof.ts +++ b/src/lib/auth/existing-factor-proof.ts @@ -282,3 +282,76 @@ async function readProof( const parsed = stepUpMintSchema.safeParse(body); return parsed.success ? parsed.data : "invalid"; } + +export type SensitiveChangeProof = "ok" | "required" | "failed" | "rate_limited"; + +/** + * The proof in front of changing the account's email address. + * + * The address is what single sign-on matches an existing account by, so it is + * part of how the account is reached and a stolen session must not rewrite it. + * Accepted: on a cookie session, a sign-in or second factor inside the last + * five minutes; on either transport, the current password alongside the + * change. The password draws on the shared re-proof budget and a wrong one is + * audited like every other refused proof. + */ +export async function authorizeSensitiveChange(args: { + user: Pick< + User, + | "id" + | "passwordHash" + | "totpSecretEncrypted" + | "totpLastStep" + | "totpConfirmedAt" + >; + /** The session row id on a cookie request; null on a Bearer request. */ + cookieSessionId: string | null; + currentPassword: string | null; + ipAddress: string | null; + stage: string; +}): Promise { + const { user, cookieSessionId, currentPassword, ipAddress, stage } = args; + + if (cookieSessionId) { + const row = await prisma.session.findUnique({ + where: { id: cookieSessionId }, + select: { mfaVerifiedAt: true, createdAt: true }, + }); + const now = Date.now(); + const recent = (d: Date | null | undefined) => + d != null && now - d.getTime() <= RECENT_PROOF_MAX_AGE_MS; + if (recent(row?.mfaVerifiedAt) || recent(row?.createdAt)) return "ok"; + } + + if (!currentPassword) return "required"; + + const rl = await checkRateLimit( + reproofBucket(user.id), + REPROOF_LIMIT, + REPROOF_WINDOW_MS, + ); + if (!rl.allowed) { + await auditLog("auth.reproof.rate_limited", { + userId: user.id, + ipAddress, + details: { stage }, + }); + return "rate_limited"; + } + + const outcome = await verifyExistingFactorProof(user, { + method: "password", + password: currentPassword, + }); + if (!outcome.ok) { + await recordReproofFailure( + user.id, + ipAddress, + stage, + "password", + outcome.reason, + ); + return "failed"; + } + return "ok"; +} diff --git a/src/lib/auth/login-response.ts b/src/lib/auth/login-response.ts index d9330dd83..6a0c71ff4 100644 --- a/src/lib/auth/login-response.ts +++ b/src/lib/auth/login-response.ts @@ -29,6 +29,7 @@ import { issueAccessAndRefresh } from "@/lib/auth/refresh-token"; import { recordSignInDevice } from "@/lib/auth/login-alert"; import { detectInsecureCookieTransport } from "@/lib/auth/secure-cookie"; import { annotate } from "@/lib/logging/context"; +import { completePendingOidcLink } from "@/lib/auth/oidc-pending-link"; import type { User } from "@/generated/prisma/client"; export interface FinishLoginParams { @@ -130,6 +131,13 @@ export async function finishLogin( annotate({ meta: { session_cookie_secure_transport_mismatch: true } }); } + // A browser sign-in that proved the account's own credential confirms a + // single sign-on link the callback left pending for this very account. An + // SSO sign-in is not such a proof, so it never completes one. + if (!source.startsWith("login.oidc") && source !== "login.web_handoff") { + await completePendingOidcLink(user.id, ip); + } + // `createSession` anchors the onboarding cookie itself; thread the user's // onboarding state through. await createSession( diff --git a/src/lib/auth/oidc-pending-link.ts b/src/lib/auth/oidc-pending-link.ts new file mode 100644 index 000000000..bc826de92 --- /dev/null +++ b/src/lib/auth/oidc-pending-link.ts @@ -0,0 +1,143 @@ +/** + * Linking a single sign-on identity to an account that already exists here. + * + * The SSO callback used to bind a verified IdP email to the local account with + * the same address on the spot. The local address is NOT verified — anyone can + * register with any address, and an account holder can type any address into + * their profile — so whoever set that address first owned the account the + * other person's SSO login landed in: they kept the password, and the SSO user + * filled their health record into an account the other person could read. + * + * So the callback no longer links. It records what it WOULD link in a sealed, + * short-lived, HttpOnly cookie and sends the browser to the sign-in page, and + * the link is made only when that same browser then signs in to that same + * account with the account's own credential — password (plus its second + * factor, if it has one) or passkey. The IdP proved who the person is; the + * local sign-in proves the account is theirs. Neither alone links anything. + * + * The cookie is AES-256-GCM sealed with the at-rest key (`encrypt`), so it can + * be neither read nor forged, and it carries its own expiry. It is scoped to + * `/api/auth`, the only place that reads it. + */ +import type { NextResponse } from "next/server"; +import { cookies } from "next/headers"; +import { prisma } from "@/lib/db"; +import { decrypt, encrypt } from "@/lib/crypto"; +import { auditLog } from "@/lib/auth/audit"; +import { annotate } from "@/lib/logging/context"; +import { shouldEmitSecureCookie } from "@/lib/auth/secure-cookie"; + +export const OIDC_LINK_COOKIE = "hl_oidc_link"; +export const OIDC_LINK_COOKIE_PATH = "/api/auth"; +export const OIDC_LINK_TTL_MS = 10 * 60 * 1000; + +export interface PendingOidcLink { + userId: string; + issuer: string; + sub: string; +} + +/** Attach the sealed pending link to a callback redirect. */ +export function setPendingLinkCookie( + response: NextResponse, + link: PendingOidcLink, +): void { + const sealed = encrypt( + JSON.stringify({ + u: link.userId, + i: link.issuer, + s: link.sub, + e: Date.now() + OIDC_LINK_TTL_MS, + }), + ); + response.cookies.set(OIDC_LINK_COOKIE, sealed, { + httpOnly: true, + secure: shouldEmitSecureCookie(), + // Lax, not Strict: the browser arrives here from the IdP's redirect and + // must carry it on the sign-in that follows. A cross-site POST cannot + // carry it, and every reader is a JSON POST besides. + sameSite: "lax", + maxAge: Math.floor(OIDC_LINK_TTL_MS / 1000), + path: OIDC_LINK_COOKIE_PATH, + }); +} + +/** The pending link this browser carries, or null when absent, forged or expired. */ +export async function readPendingLink(): Promise { + let raw: string | undefined; + try { + raw = (await cookies()).get(OIDC_LINK_COOKIE)?.value; + } catch { + return null; + } + if (!raw) return null; + try { + const parsed = JSON.parse(decrypt(raw)) as Record; + if ( + typeof parsed.u !== "string" || + typeof parsed.i !== "string" || + typeof parsed.s !== "string" || + typeof parsed.e !== "number" || + parsed.e <= Date.now() + ) { + return null; + } + return { userId: parsed.u, issuer: parsed.i, sub: parsed.s }; + } catch { + return null; + } +} + +async function clearPendingLinkCookie(): Promise { + try { + (await cookies()).set(OIDC_LINK_COOKIE, "", { + httpOnly: true, + secure: shouldEmitSecureCookie(), + sameSite: "lax", + maxAge: 0, + path: OIDC_LINK_COOKIE_PATH, + }); + } catch { + // Outside a request scope there is no cookie to clear. + } +} + +/** + * Called once a browser sign-in has proved the account's OWN credential. + * Binds the pending SSO identity when the cookie names this very account and + * the account is still unlinked. Never throws into the sign-in: a link that + * cannot be made leaves the login intact and the account as it was. + */ +export async function completePendingOidcLink( + userId: string, + ipAddress: string | null, +): Promise { + const link = await readPendingLink(); + if (!link) return false; + await clearPendingLinkCookie(); + if (link.userId !== userId) { + annotate({ meta: { oidc_link: "other_account" } }); + return false; + } + try { + const linked = await prisma.user.updateMany({ + where: { id: userId, oidcIssuer: null, oidcSub: null }, + data: { oidcIssuer: link.issuer, oidcSub: link.sub }, + }); + if (linked.count !== 1) { + annotate({ meta: { oidc_link: "already_linked" } }); + return false; + } + } catch { + // The (issuer, sub) pair became bound to another account in between. + annotate({ meta: { oidc_link: "identity_taken" } }); + return false; + } + await auditLog("auth.oidc.linked", { + userId, + ipAddress, + details: { confirmedBy: "local_sign_in" }, + }); + annotate({ action: { name: "auth.oidc.linked" } }); + return true; +} diff --git a/src/lib/auth/profile-update.ts b/src/lib/auth/profile-update.ts index 1891c1575..bf57b99a7 100644 --- a/src/lib/auth/profile-update.ts +++ b/src/lib/auth/profile-update.ts @@ -16,6 +16,11 @@ import { sanitiseZodIssues, type SanitisedZodIssue } from "@/lib/api-response"; import { checkProfileEmailRateLimit } from "@/lib/rate-limit"; import { annotate } from "@/lib/logging/context"; import { z } from "zod/v4"; +import { + REPROOF_FAILED_CODE, + REPROOF_REQUIRED_CODE, + type SensitiveChangeProof, +} from "@/lib/auth/existing-factor-proof"; const extendedProfileSchema = profileSchema.extend({ displayName: z.string().min(1).max(80).nullable().optional(), @@ -185,7 +190,19 @@ export interface ApplyProfileError { export async function applyProfileUpdate( userId: string, body: unknown, - ipAddress?: string | null, + ipAddress: string | null | undefined, + opts: { + /** + * Decides whether this request may move the account to a NEW email + * address. The address is what single sign-on links an existing account + * by, so changing it asks for a fresh proof (`authorizeSensitiveChange`). + * Receives the `currentPassword` the body carried, if any; the password is + * never written anywhere. + */ + authorizeEmailChange: ( + currentPassword: string | null, + ) => Promise; + }, ): Promise { const parsed = extendedProfileSchema.safeParse(body); @@ -258,6 +275,65 @@ export async function applyProfileUpdate( const addressOnFile = current?.email?.trim().toLowerCase() ?? null; if (addressOnFile !== normalizedEmail) { + // Asked before the uniqueness probe and before the budget: a caller who + // cannot show it is the account owner learns nothing about which + // addresses exist here and spends nothing. + const rawPassword = + typeof body === "object" && body !== null && !Array.isArray(body) + ? (body as Record).currentPassword + : undefined; + const currentPassword = + typeof rawPassword === "string" && + rawPassword.length > 0 && + rawPassword.length <= 512 + ? rawPassword + : null; + const proof = await opts.authorizeEmailChange(currentPassword); + if (proof !== "ok") { + annotate({ + action: { name: "profile.email.reproof_refused" }, + meta: { proof }, + }); + const code = + proof === "rate_limited" + ? "rate_limited" + : proof === "failed" + ? "reproof_failed" + : "reproof_required"; + // Same narrowing as the email budget below: the address is dropped, + // the rest of the save lands, and `rejectedFields` says why. + const { email: _unproven, ...keepable } = data; + if (Object.keys(keepable).length === 0) { + return { + ok: false, + status: proof === "rate_limited" ? 429 : 401, + message: + proof === "failed" + ? "Verification failed" + : proof === "rate_limited" + ? "Too many attempts. Please wait 15 minutes." + : "Confirm your current password to change the email address.", + errorCode: + proof === "failed" + ? REPROOF_FAILED_CODE + : proof === "rate_limited" + ? "profile.update.emailRateLimited" + : REPROOF_REQUIRED_CODE, + }; + } + data = keepable; + rejectedFields = [ + ...rejectedFields, + { + path: "email", + code, + message: "Confirm your current password to change the email address.", + }, + ]; + } + } + + if (data.email !== undefined && addressOnFile !== normalizedEmail) { // Charged BEFORE the probe, not after: a refused request must not // learn the answer it was refused for. The 409 below tells any // signed-in caller whether an address is registered here, and until diff --git a/src/lib/openapi/routes/auth.ts b/src/lib/openapi/routes/auth.ts index a294e6eed..7e3885c78 100644 --- a/src/lib/openapi/routes/auth.ts +++ b/src/lib/openapi/routes/auth.ts @@ -652,7 +652,7 @@ export const authPaths: NonNullable = { }, "403": { description: - "Password login is disabled — the operator runs `OIDC_ONLY=true`. `meta.errorCode` = `oidc_only`; sign in through SSO instead.", + "Password login is disabled — the operator runs `OIDC_ONLY=true`. `meta.errorCode` = `oidc_only`; sign in through SSO instead. The one exception is a browser the SSO callback sent back with a pending link to confirm an existing account: it may sign in with that one account's password, which links the account.", content: { "application/json": { schema: errorEnvelope } }, }, ...stdResponses, @@ -1903,6 +1903,7 @@ export const authPaths: NonNullable = { "**Browser arm.** No parameters, or `next=` to return somewhere specific afterwards (sanitised to an in-app path — an absolute URL is discarded, not honoured). Redirects to the provider's authorization endpoint. Every failure redirects to `/auth/login?error=`.\n\n" + "**Native arm.** The iOS app opens this URL inside an `ASWebAuthenticationSession` with `client=native` and its own PKCE `code_challenge`. The challenge is mandatory on this arm and must be S256, 43–128 characters; `plain` is structurally unsupported because the exchange only ever verifies S256. `next` is ignored and pinned to `/` — a post-login web path means nothing to the app. Every failure redirects to `healthlog://oidc-callback?error=`, and an error redirect carries no code, no ticket and no session, which is why it is safe that `client` is caller-supplied.\n\n" + "**What comes back.** The provider returns to the server's own callback, not to the app. On success the callback redirects to the compiled-in `healthlog://oidc-callback` with a one-time `code=hlh_…`, which the app exchanges at POST /api/auth/oidc/native/token. When the account has a second factor the callback instead returns `mfa_ticket=…`, which the app completes at /api/auth/mfa/verify to obtain the same bundle. The token pair NEVER rides a URL — only the opaque code or ticket does. There is no `redirect_uri` parameter and no allowlist to configure: the scheme is a compile-time constant, so the open-redirect class is removed by construction.\n\n" + + "**An account that exists but is not linked yet.** When the provider's verified email matches a local account with no SSO identity, nothing is linked and nobody is signed in: the local address was never verified, so matching it proves nothing about who owns the account. The browser arm redirects to `/auth/login?error=oidc_link_required` with a sealed, ten-minute, HttpOnly pending-link cookie; the person then signs in to that account once with its own password (and second factor) or passkey, and that sign-in makes the link. Under `OIDC_ONLY=true` the password sign-in is admitted for exactly the account the pending link names. The native arm redirects to `healthlog://oidc-callback?error=oidc_link_required`; the link is confirmed once on the web, after which the app's SSO sign-in works.\n\n" + "Two PKCE exchanges run here and must not be confused: the server↔provider verifier the server generates for itself, and the app↔server challenge supplied above. The native flag and the app's challenge travel only inside an AES-256-GCM state cookie, so the callback branches on tamper-authenticated state rather than on anything the provider or a network attacker can flip.", requestParams: { query: z.object({ diff --git a/src/lib/openapi/routes/profile.ts b/src/lib/openapi/routes/profile.ts index b1fa0100d..12154f4cd 100644 --- a/src/lib/openapi/routes/profile.ts +++ b/src/lib/openapi/routes/profile.ts @@ -482,6 +482,13 @@ export const profileUpdateRequest = z .describe( "German insurer institution number (IKNR). Optional; empty/null clears it. A non-empty value must be exactly 9 digits (no checksum enforced).", ), + currentPassword: z + .string() + .max(512) + .optional() + .describe( + "The account password, sent only alongside a NEW `email`. Moving the account to a new address needs a fresh proof because single sign-on links an existing account by it: a browser session that signed in or completed a second factor in the last five minutes needs nothing more, otherwise this field must carry the current password. Without an accepted proof the address is not changed and comes back in `rejectedFields` as `reproof_required` or `reproof_failed` (or, when the address was the only field sent, 401 `auth.reproof.required` / `auth.reproof.failed`). Attempts draw on the account's shared re-proof budget of five per fifteen minutes. Never stored.", + ), }) .meta({ id: "ProfileUpdateRequest", diff --git a/src/lib/profile/rejected-fields.ts b/src/lib/profile/rejected-fields.ts index 87225925e..03d0be90f 100644 --- a/src/lib/profile/rejected-fields.ts +++ b/src/lib/profile/rejected-fields.ts @@ -78,6 +78,10 @@ const REJECTION_REASON_KEYS: Record = { // still lands. Without an entry it would fall back to "check it and try // again", which is wrong — the value was fine, the timing was not. rate_limited: "settings.profileRejection.rateLimited", + // Not Zod codes either: a new email address asks for a fresh proof, and + // these say whether none was given or the one given was wrong. + reproof_required: "settings.profileRejection.reproofRequired", + reproof_failed: "settings.profileRejection.reproofFailed", too_big: "settings.profileRejection.tooBig", too_small: "settings.profileRejection.tooSmall", invalid_value: "settings.profileRejection.invalidValue", diff --git a/tests/integration/oidc-pending-link.test.ts b/tests/integration/oidc-pending-link.test.ts new file mode 100644 index 000000000..10a17d90d --- /dev/null +++ b/tests/integration/oidc-pending-link.test.ts @@ -0,0 +1,189 @@ +/** + * A single sign-on identity is linked to an existing account only after the + * account's own credential is proved in the same browser. + * + * The callback leaves a sealed pending link (unit-tested in the callback's own + * suite); this pins the other half against real Postgres: which sign-ins + * complete it, which do not, and that an SSO-only policy opens the password + * form for exactly the account the IdP vouched for and nothing else. + */ +import { NextRequest } from "next/server"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +process.env.API_TOKEN_HMAC_KEY ??= + "test-hmac-key-oidc-pending-link-32-bytes-minimum-1234567890"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => cookieJar.set(name, value), + delete: (name: string) => cookieJar.delete(name), + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +const { hashPassword } = await import("@/lib/auth/password"); +const { encrypt } = await import("@/lib/crypto"); + +const PASSWORD = "Correct horse battery staple!42"; +const ISSUER = "https://idp.example.test"; +const OIDC_ENV = { + OIDC_ISSUER_URL: ISSUER, + OIDC_CLIENT_ID: "client", + OIDC_CLIENT_SECRET: "secret", +}; + +function pendingLinkFor(userId: string, expiresInMs = 60_000): void { + cookieJar.set( + "hl_oidc_link", + encrypt( + JSON.stringify({ + u: userId, + i: ISSUER, + s: "idp-sub-1", + e: Date.now() + expiresInMs, + }), + ), + ); +} + +async function passwordLogin(identifier: string, password = PASSWORD) { + const { POST } = await import("@/app/api/auth/login/route"); + return POST( + new NextRequest("http://localhost/api/auth/login", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ email: identifier, password }), + }), + ); +} + +async function linkOf(id: string) { + return getPrismaClient().user.findUniqueOrThrow({ + where: { id }, + select: { oidcIssuer: true, oidcSub: true }, + }); +} + +beforeEach(async () => { + vi.clearAllMocks(); + await truncateAllTables(getPrismaClient()); + cookieJar.clear(); + headerJar.clear(); + process.env.APP_URL = "http://localhost:3000"; + process.env.NEXT_PUBLIC_APP_URL = "http://localhost:3000"; + const hash = await hashPassword(PASSWORD); + await getPrismaClient().user.createMany({ + data: [ + { + id: "acct-a", + username: "acct-a", + email: "a@example.test", + passwordHash: hash, + onboardingCompletedAt: new Date(), + }, + { + id: "acct-b", + username: "acct-b", + email: "b@example.test", + passwordHash: hash, + onboardingCompletedAt: new Date(), + }, + ], + }); +}); + +afterEach(() => { + for (const key of [...Object.keys(OIDC_ENV), "OIDC_ONLY"]) { + delete process.env[key]; + } +}); + +describe("pending single sign-on link", () => { + it("is completed by a password sign-in to the named account", async () => { + pendingLinkFor("acct-a"); + const res = await passwordLogin("acct-a"); + expect(res.status).toBe(200); + expect(await linkOf("acct-a")).toEqual({ + oidcIssuer: ISSUER, + oidcSub: "idp-sub-1", + }); + expect( + await getPrismaClient().auditLog.count({ + where: { userId: "acct-a", action: "auth.oidc.linked" }, + }), + ).toBe(1); + }); + + it("is not completed by a sign-in to a different account", async () => { + pendingLinkFor("acct-a"); + const res = await passwordLogin("acct-b"); + expect(res.status).toBe(200); + expect(await linkOf("acct-a")).toEqual({ oidcIssuer: null, oidcSub: null }); + expect(await linkOf("acct-b")).toEqual({ oidcIssuer: null, oidcSub: null }); + }); + + it("is not completed by a wrong password", async () => { + pendingLinkFor("acct-a"); + const res = await passwordLogin("acct-a", "not the password"); + expect(res.status).toBe(401); + expect(await linkOf("acct-a")).toEqual({ oidcIssuer: null, oidcSub: null }); + }); + + it("expires", async () => { + pendingLinkFor("acct-a", -1); + await passwordLogin("acct-a"); + expect(await linkOf("acct-a")).toEqual({ oidcIssuer: null, oidcSub: null }); + }); + + it("refuses a forged cookie", async () => { + cookieJar.set( + "hl_oidc_link", + JSON.stringify({ u: "acct-a", i: ISSUER, s: "x", e: Date.now() + 1e6 }), + ); + await passwordLogin("acct-a"); + expect(await linkOf("acct-a")).toEqual({ oidcIssuer: null, oidcSub: null }); + }); +}); + +describe("under an SSO-only policy", () => { + beforeEach(() => { + Object.assign(process.env, OIDC_ENV, { OIDC_ONLY: "true" }); + }); + + it("keeps password sign-in closed without a pending link", async () => { + const res = await passwordLogin("acct-a"); + expect(res.status).toBe(403); + }); + + it("opens it for the account the pending link names, and links it", async () => { + pendingLinkFor("acct-a"); + const res = await passwordLogin("acct-a"); + expect(res.status).toBe(200); + expect((await linkOf("acct-a")).oidcSub).toBe("idp-sub-1"); + }); + + it("keeps it closed for any other account", async () => { + pendingLinkFor("acct-a"); + const res = await passwordLogin("acct-b"); + expect(res.status).toBe(403); + expect( + await getPrismaClient().session.count({ where: { userId: "acct-b" } }), + ).toBe(0); + }); +});