From faa2cf550b95220b44d81d868cb33577cb89d870 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 15:09:22 -0800 Subject: [PATCH 01/17] server: move Firebase auth to server-side endpoints Migrate Firebase authentication from client-side SDK to server-side endpoints that call the Firebase REST API. This reduces client bundle size by removing the Firebase SDK dependencies (~80-200KB) and enables future migration to alternative auth backends. The implementation includes: - Email/password auth endpoints (/auth/login, /auth/signup, etc.) - OAuth handlers for Google and Apple Sign-In with full server-side token exchange - Firestore-backed OAuth state storage for CSRF protection - Full cryptographic verification of Apple ID tokens using JWKS - Return URL validation to prevent open redirect attacks - User protobuf schema extended with providerUserId for stable OAuth provider identity Client code now uses simple redirects for OAuth and fetch calls for email/password auth, with all Firebase SDK code removed. --- config/default.json | 6 + eslint.config.shared.js | 1 + pnpm-lock.yaml | 8 + src/app/App.tsx | 96 +- src/app/Login.tsx | 111 ++- src/app/package.json | 2 - src/server/app.ts | 61 +- src/server/auth/auth-handlers.ts | 219 +++++ src/server/auth/auth-router.ts | 86 ++ src/server/auth/firebase-rest-client.ts | 141 +++ src/server/auth/oauth-handlers.ts | 333 +++++++ src/server/auth/oauth-state.ts | 87 ++ src/server/auth/oauth-token-exchange.ts | 183 ++++ src/server/auth/url-validation.ts | 49 + src/server/authn.ts | 107 ++- src/server/jest.config.js | 1 + src/server/models/db-firestore.ts | 6 +- src/server/models/db-interfaces.ts | 3 + src/server/package.json | 1 + src/server/schemas/user.proto | 1 + src/server/schemas/user_pb.d.ts | 4 + src/server/schemas/user_pb.js | 32 +- src/server/tests/auth-handlers.test.ts | 897 ++++++++++++++++++ src/server/tests/auth-integration.test.ts | 108 +++ src/server/tests/firebase-rest-client.test.ts | 281 ++++++ src/server/tests/oauth-handlers.test.ts | 520 ++++++++++ src/server/tests/oauth-state.test.ts | 145 +++ src/server/tests/url-validation.test.ts | 85 ++ src/simlin-engine/src/project_io.gen.rs | 2 +- 29 files changed, 3414 insertions(+), 162 deletions(-) create mode 100644 src/server/auth/auth-handlers.ts create mode 100644 src/server/auth/auth-router.ts create mode 100644 src/server/auth/firebase-rest-client.ts create mode 100644 src/server/auth/oauth-handlers.ts create mode 100644 src/server/auth/oauth-state.ts create mode 100644 src/server/auth/oauth-token-exchange.ts create mode 100644 src/server/auth/url-validation.ts create mode 100644 src/server/tests/auth-handlers.test.ts create mode 100644 src/server/tests/auth-integration.test.ts create mode 100644 src/server/tests/firebase-rest-client.test.ts create mode 100644 src/server/tests/oauth-handlers.test.ts create mode 100644 src/server/tests/oauth-state.test.ts create mode 100644 src/server/tests/url-validation.test.ts diff --git a/config/default.json b/config/default.json index 191d4407d..20bf25c7e 100644 --- a/config/default.json +++ b/config/default.json @@ -25,6 +25,12 @@ "clientID": "", "clientSecret": "" }, + "apple": { + "clientID": "", + "teamID": "", + "keyID": "", + "privateKey": "" + }, "path": "/authn", "service": "users" } diff --git a/eslint.config.shared.js b/eslint.config.shared.js index acbf52c10..044d50d40 100644 --- a/eslint.config.shared.js +++ b/eslint.config.shared.js @@ -23,6 +23,7 @@ const baseConfig = { require: 'readonly', global: 'readonly', URL: 'readonly', + URLSearchParams: 'readonly', // Browser globals window: 'readonly', document: 'readonly', diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 95c88163a..f62c76139 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -371,6 +371,9 @@ importers: immutable: specifier: ^5.0.3 version: 5.1.4 + jose: + specifier: ^6.1.3 + version: 6.1.3 node-fetch: specifier: ^3.0.0 version: 3.3.2 @@ -4930,6 +4933,9 @@ packages: jose@4.15.9: resolution: {integrity: sha512-1vUQX+IdDMVPj4k8kOxgUqlcK518yluMuGZwqlr44FS1ppZB/5GWh4rZG89erpOBOJjU/OBsnCVFfapsRz6nEA==} + jose@6.1.3: + resolution: {integrity: sha512-0TpaTfihd4QMNwrz/ob2Bp7X04yuxJkjRGi4aKmOqwhov54i6u79oCv7T+C7lo70MKH6BesI3vscD1yb/yzKXQ==} + js-base64@3.7.8: resolution: {integrity: sha512-hNngCeKxIUQiEUN3GPJOkz4wF/YvdUdbNL9hsBcMQTkKzboD7T/q3OYOuuPZLUE6dBxSGpwhk5mwuDud7JVAow==} @@ -13338,6 +13344,8 @@ snapshots: jose@4.15.9: {} + jose@6.1.3: {} + js-base64@3.7.8: {} js-tokens@4.0.0: {} diff --git a/src/app/App.tsx b/src/app/App.tsx index 5e9bc8463..dda714c6f 100644 --- a/src/app/App.tsx +++ b/src/app/App.tsx @@ -4,15 +4,6 @@ import * as React from 'react'; -import { initializeApp } from '@firebase/app'; -import { - getAuth, - connectAuthEmulator, - onAuthStateChanged, - Auth as FirebaseAuth, - User as FirebaseUser, -} from '@firebase/auth'; - import { useLocation, Route, RouteComponentProps, Switch, Redirect } from 'wouter'; import { defined } from '@simlin/core/common'; @@ -25,12 +16,6 @@ import { User } from './User'; import styles from './App.module.css'; -const config = { - apiKey: 'AIzaSyConH72HQl9xOtjmYJO9o2kQ9nZZzl96G8', - authDomain: 'auth.simlin.com', -}; -const firebaseApp = initializeApp(config); - interface EditorMatchParams { username: string; projectName: string; @@ -42,8 +27,6 @@ class UserInfoSingleton { private resultPromise?: Promise<[User | undefined, number]>; private result?: [User | undefined, number]; constructor() { - // store this promise; we might race calling get() below, but all racers will - // await this single fetch result. this.fetch(); } @@ -75,7 +58,6 @@ class UserInfoSingleton { this.fetch(); if (resultPromise) { - // don't leave the promise un-awaited await resultPromise; } } @@ -87,8 +69,6 @@ interface AppState { authUnknown: boolean; isNewUser?: boolean; user?: User; - auth: FirebaseAuth; - firebaseIdToken?: string | null; } class InnerApp extends React.PureComponent<{}, AppState> { @@ -97,91 +77,19 @@ class InnerApp extends React.PureComponent<{}, AppState> { constructor(props: {}) { super(props); - const isDevServer = process.env.NODE_ENV === 'development'; - const auth = getAuth(firebaseApp); - if (isDevServer) { - connectAuthEmulator(auth, 'http://localhost:9099', { disableWarnings: true }); - } - this.state = { authUnknown: true, - auth, }; - // notify our app when a user logs in - onAuthStateChanged(auth, this.authStateChanged); - setTimeout(this.getUserInfo); } - authStateChanged = (user: FirebaseUser | null) => { - setTimeout(this.asyncAuthStateChanged, undefined, user); - }; - - asyncAuthStateChanged = async (user: FirebaseUser | null) => { - if (!user) { - this.setState({ firebaseIdToken: null }); - return; - } - - const firebaseIdToken = await user.getIdToken(); - this.setState({ firebaseIdToken }); - await this.maybeLogin(undefined, firebaseIdToken); - }; - - async maybeLogin(authIsKnown = false, firebaseIdToken?: string): Promise { - authIsKnown = authIsKnown || !this.state.authUnknown; - if (!authIsKnown) { - return; - } - - // if we know the user, we don't need to log in - const [user] = await userInfo.get(); - if (user) { - return; - } - - const idToken = firebaseIdToken ?? this.state.firebaseIdToken; - if (idToken === null || idToken === undefined) { - return; - } - - const bodyContents = { - idToken, - }; - - const base = this.getBaseURL(); - const apiPath = `${base}/session`; - const response = await fetch(apiPath, { - credentials: 'same-origin', - method: 'POST', - cache: 'no-cache', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify(bodyContents), - }); - - const status = response.status; - if (!(status >= 200 && status < 400)) { - const body = await response.json(); - const errorMsg = - body && body.error ? (body.error as string) : `HTTP ${status}; maybe try a different username ¯\\_(ツ)_/¯`; - // this.appendModelError(errorMsg); - console.log(`session error: ${errorMsg}`); - return undefined; - } - - this.handleUsernameChanged(); - } - getUserInfo = async (): Promise => { const [user, status] = await userInfo.get(); if (!(status >= 200 && status < 400) || !user) { this.setState({ authUnknown: false, }); - await this.maybeLogin(true); return; } const isNewUser = user.id.startsWith(`temp-`); @@ -230,11 +138,9 @@ class InnerApp extends React.PureComponent<{}, AppState> { const projectParam = urlParams.get('project'); if (projectParam) return ; - // if a user is navigating to a project, - // skip the high level auth check, to enable public models if (!/\/.*\/.*/.test(window.location.pathname)) { if (!this.state.user) { - return ; + return ; } if (this.state.isNewUser) { diff --git a/src/app/Login.tsx b/src/app/Login.tsx index 424b89856..fcf452da1 100644 --- a/src/app/Login.tsx +++ b/src/app/Login.tsx @@ -4,17 +4,6 @@ import * as React from 'react'; -import { - signInWithRedirect, - GoogleAuthProvider, - OAuthProvider, - Auth as FirebaseAuth, - fetchSignInMethodsForEmail, - createUserWithEmailAndPassword, - updateProfile, - sendPasswordResetEmail, - signInWithEmailAndPassword, -} from '@firebase/auth'; import clsx from 'clsx'; import { @@ -38,7 +27,7 @@ type EmailLoginStates = 'showEmail' | 'showPassword' | 'showSignup' | 'showProvi export interface LoginProps { disabled: boolean; - auth: FirebaseAuth; + onLoginSuccess?: () => void; } interface LoginState { @@ -52,13 +41,6 @@ interface LoginState { provider: 'google.com' | 'apple.com' | undefined; } -function appleProvider(): OAuthProvider { - const provider = new OAuthProvider('apple.com'); - provider.addScope('email'); - provider.addScope('name'); - return provider; -} - export const GoogleIcon: React.FunctionComponent = (props) => { return ( @@ -86,17 +68,10 @@ export class Login extends React.Component { } appleLoginClick = () => { - const provider = appleProvider(); - setTimeout(async () => { - await signInWithRedirect(this.props.auth, provider); - }); + window.location.href = '/auth/apple'; }; googleLoginClick = () => { - const provider = new GoogleAuthProvider(); - provider.addScope('profile'); - setTimeout(async () => { - await signInWithRedirect(this.props.auth, provider); - }); + window.location.href = '/auth/google'; }; emailLoginClick = () => { this.setState({ emailLoginFlow: 'showEmail' }); @@ -120,24 +95,32 @@ export class Login extends React.Component { return; } - const methods = await fetchSignInMethodsForEmail(this.props.auth, email); - if (methods.includes('password')) { - this.setState({ emailLoginFlow: 'showPassword' }); - } else if (methods.length === 0) { - this.setState({ emailLoginFlow: 'showSignup' }); - } else { - // we only allow 1 method - const method = methods[0]; - if (method === 'google.com' || method === 'apple.com') { - this.setState({ - emailLoginFlow: 'showProviderRedirect', - provider: methods[0] as 'google.com' | 'apple.com', - }); + try { + const response = await fetch('/auth/providers', { + method: 'POST', + credentials: 'same-origin', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email }), + }); + + const { providers, registered } = await response.json(); + + if (providers.includes('password')) { + this.setState({ emailLoginFlow: 'showPassword' }); + } else if (!registered) { + this.setState({ emailLoginFlow: 'showSignup' }); + } else if (providers.includes('google.com')) { + this.setState({ emailLoginFlow: 'showProviderRedirect', provider: 'google.com' }); + } else if (providers.includes('apple.com')) { + this.setState({ emailLoginFlow: 'showProviderRedirect', provider: 'apple.com' }); } else { this.setState({ emailError: 'an unknown error occurred; try a different email address', }); } + } catch (err) { + console.log(err); + this.setState({ emailError: 'Failed to check email. Please try again.' }); } }; onSubmitRecovery = async () => { @@ -147,7 +130,16 @@ export class Login extends React.Component { return; } - await sendPasswordResetEmail(this.props.auth, email); + try { + await fetch('/auth/reset-password', { + method: 'POST', + credentials: 'same-origin', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email }), + }); + } catch (err) { + console.log(err); + } this.setState({ emailLoginFlow: 'showPassword', @@ -175,8 +167,23 @@ export class Login extends React.Component { } try { - const userCred = await createUserWithEmailAndPassword(this.props.auth, email, password); - await updateProfile(userCred.user, { displayName: fullName }); + const response = await fetch('/auth/signup', { + method: 'POST', + credentials: 'same-origin', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + email, + password, + displayName: fullName, + }), + }); + + if (response.ok) { + this.props.onLoginSuccess?.(); + } else { + const { error } = await response.json(); + this.setState({ passwordError: error || 'Something went wrong' }); + } } catch (err) { console.log(err); if (err instanceof Error) { @@ -202,12 +209,24 @@ export class Login extends React.Component { const password = this.state.password.trim(); if (!password) { - this.setState({ passwordError: 'Enter your email address to continue' }); + this.setState({ passwordError: 'Enter your password to continue' }); return; } try { - await signInWithEmailAndPassword(this.props.auth, email, password); + const response = await fetch('/auth/login', { + method: 'POST', + credentials: 'same-origin', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email, password }), + }); + + if (response.ok) { + this.props.onLoginSuccess?.(); + } else { + const { error } = await response.json(); + this.setState({ passwordError: error || 'Incorrect password' }); + } } catch (err) { console.log(err); if (err instanceof Error) { diff --git a/src/app/package.json b/src/app/package.json index bc9164d6c..203187749 100644 --- a/src/app/package.json +++ b/src/app/package.json @@ -37,8 +37,6 @@ "eslint-plugin-prettier": "^5.0.0", "eslint-plugin-react": "^7.37.2", "eslint-plugin-react-hooks": "^5.1.0", - "@firebase/app": "^0.13.2", - "@firebase/auth": "^1.10.8", "prettier": "^3.0.0", "typescript": "^5.7.2", "wouter": "^3.6.0" diff --git a/src/server/app.ts b/src/server/app.ts index f384fc1a1..4b57e433b 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -27,6 +27,8 @@ import { requestLogger } from './request-logger'; import { createProjectRouteHandler } from './route-handlers'; import { initializeServerDependencies } from './server-init'; import { getStaticDirectory, validateStaticDirectory } from './static-config'; +import { createAuthRouter } from './auth/auth-router'; +import { createFirebaseRestClient } from './auth/firebase-rest-client'; // redefinition from Helmet, as they don't export it interface ContentSecurityPolicyDirectiveValueFunction { @@ -161,12 +163,7 @@ class App { 'frame-src': ["'self'", 'https://simlin.firebaseapp.com', 'https://auth.simlin.com'], 'base-uri': ["'self'"], 'block-all-mixed-content': [], - 'connect-src': [ - "'self'", - 'https://www.googleapis.com', - 'https://securetoken.googleapis.com', - 'https://identitytoolkit.googleapis.com', - ], + 'connect-src': ["'self'"], 'font-src': ["'self'", 'data:', 'https://fonts.gstatic.com'], 'frame-ancestors': ["'self'"], 'img-src': ["'self'", 'data:', 'blob:', 'https://*.googleusercontent.com', 'https://www.gstatic.com'], @@ -215,6 +212,58 @@ class App { authn(this.app, this.authn); + // Server-side auth endpoints (email/password, OAuth) + const firebaseRestClient = createFirebaseRestClient({ + apiKey: 'AIzaSyConH72HQl9xOtjmYJO9o2kQ9nZZzl96G8', + emulatorHost: process.env.FIREBASE_AUTH_EMULATOR_HOST, + }); + + const host = this.app.get('host') as string; + const port = this.app.get('port') as number; + const baseUrl = host === 'localhost' ? `http://localhost:${port}` : `https://${host}`; + + const authConfig = this.app.get('authentication') as Record; + const googleAuthConfig = authConfig?.google as Record | undefined; + const appleAuthConfig = authConfig?.apple as Record | undefined; + + const authRouter = createAuthRouter({ + firebaseRestClient, + firebaseAdmin: this.authn, + users: this.app.db.user, + baseUrl, + firestore: this.app.db.firestore, + googleConfig: + googleAuthConfig?.clientID && googleAuthConfig?.clientSecret + ? { + clientId: googleAuthConfig.clientID, + clientSecret: googleAuthConfig.clientSecret, + authorizationUrl: 'https://accounts.google.com/o/oauth2/v2/auth', + tokenUrl: 'https://oauth2.googleapis.com/token', + scopes: ['openid', 'email', 'profile'], + callbackPath: '/auth/google/callback', + } + : undefined, + appleConfig: + appleAuthConfig?.clientID && + appleAuthConfig?.teamID && + appleAuthConfig?.keyID && + appleAuthConfig?.privateKey + ? { + clientId: appleAuthConfig.clientID, + clientSecret: '', // Generated dynamically + teamId: appleAuthConfig.teamID, + keyId: appleAuthConfig.keyID, + privateKey: appleAuthConfig.privateKey, + authorizationUrl: 'https://appleid.apple.com/auth/authorize', + tokenUrl: 'https://appleid.apple.com/auth/token', + scopes: ['name', 'email'], + callbackPath: '/auth/apple/callback', + } + : undefined, + }); + + this.app.use('/auth', authRouter); + // authenticated: // /api is for API requests // all others should serve index.js if user is authorized diff --git a/src/server/auth/auth-handlers.ts b/src/server/auth/auth-handlers.ts new file mode 100644 index 000000000..1cd62a5a7 --- /dev/null +++ b/src/server/auth/auth-handlers.ts @@ -0,0 +1,219 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { Request, Response, RequestHandler } from 'express'; +import * as admin from 'firebase-admin'; +import * as logger from 'winston'; + +import { FirebaseRestClient, FirebaseAuthError } from './firebase-rest-client'; +import { getOrCreateUserFromIdToken } from '../authn'; +import { Table } from '../models/table'; +import { User } from '../schemas/user_pb'; + +export interface AuthHandlerDeps { + firebaseRestClient: FirebaseRestClient; + firebaseAdmin: admin.auth.Auth; + users: Table; + baseUrl: string; +} + +function getHttpStatusForError(err: FirebaseAuthError): number { + switch (err.code) { + case 'INVALID_PASSWORD': + case 'EMAIL_NOT_FOUND': + return 401; + case 'USER_DISABLED': + return 403; + case 'EMAIL_EXISTS': + return 409; + case 'WEAK_PASSWORD': + case 'INVALID_EMAIL': + return 400; + case 'TOO_MANY_ATTEMPTS_TRY_LATER': + return 429; + default: + return 500; + } +} + +function loginUser(req: Request, user: User): Promise { + return new Promise((resolve, reject) => { + req.login(user, (err) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); +} + +export function createLoginHandler(deps: AuthHandlerDeps): RequestHandler { + return async (req: Request, res: Response): Promise => { + const email = typeof req.body?.email === 'string' ? req.body.email.trim() : ''; + const password = typeof req.body?.password === 'string' ? req.body.password.trim() : ''; + + if (!email) { + res.status(400).json({ error: 'Email is required' }); + return; + } + + if (!password) { + res.status(400).json({ error: 'Password is required' }); + return; + } + + try { + const signInResult = await deps.firebaseRestClient.signInWithPassword(email, password); + + const [user, err] = await getOrCreateUserFromIdToken(deps.users, deps.firebaseAdmin, signInResult.idToken); + + if (err) { + logger.error('Error getting or creating user:', err); + if (err.message === 'account is disabled') { + res.status(403).json({ error: 'This account has been disabled' }); + } else { + res.status(500).json({ error: 'An unexpected error occurred' }); + } + return; + } + + await loginUser(req, user); + + res.status(200).json({ + success: true, + user: { + id: user.getId(), + email: user.getEmail(), + displayName: user.getDisplayName(), + }, + }); + } catch (err) { + if (err instanceof FirebaseAuthError) { + const status = getHttpStatusForError(err); + res.status(status).json({ error: err.message }); + } else { + logger.error('Unexpected login error:', err); + res.status(500).json({ error: 'An unexpected error occurred' }); + } + } + }; +} + +export function createSignupHandler(deps: AuthHandlerDeps): RequestHandler { + return async (req: Request, res: Response): Promise => { + const email = typeof req.body?.email === 'string' ? req.body.email.trim() : ''; + const password = typeof req.body?.password === 'string' ? req.body.password.trim() : ''; + const displayName = typeof req.body?.displayName === 'string' ? req.body.displayName.trim() : ''; + + if (!email) { + res.status(400).json({ error: 'Email is required' }); + return; + } + + if (!password) { + res.status(400).json({ error: 'Password is required' }); + return; + } + + if (!displayName) { + res.status(400).json({ error: 'Display name is required' }); + return; + } + + try { + const signUpResult = await deps.firebaseRestClient.signUp(email, password, displayName); + + const decodedToken = await deps.firebaseAdmin.verifyIdToken(signUpResult.idToken); + await deps.firebaseAdmin.updateUser(decodedToken.uid, { displayName }); + + const [user, err] = await getOrCreateUserFromIdToken(deps.users, deps.firebaseAdmin, signUpResult.idToken); + + if (err) { + logger.error('Error creating user:', err); + res.status(500).json({ error: 'An unexpected error occurred' }); + return; + } + + await loginUser(req, user); + + res.status(201).json({ + success: true, + user: { + id: user.getId(), + email: user.getEmail(), + displayName: user.getDisplayName(), + }, + }); + } catch (err) { + if (err instanceof FirebaseAuthError) { + const status = getHttpStatusForError(err); + res.status(status).json({ error: err.message }); + } else { + logger.error('Unexpected signup error:', err); + res.status(500).json({ error: 'An unexpected error occurred' }); + } + } + }; +} + +export function createProvidersHandler(deps: AuthHandlerDeps): RequestHandler { + return async (req: Request, res: Response): Promise => { + const email = typeof req.body?.email === 'string' ? req.body.email.trim() : ''; + + if (!email) { + res.status(400).json({ error: 'Email is required' }); + return; + } + + try { + const continueUri = `${deps.baseUrl}/auth/callback`; + const result = await deps.firebaseRestClient.fetchProviders(email, continueUri); + + res.status(200).json({ + registered: result.registered, + providers: result.providers, + }); + } catch (err) { + logger.error('Error fetching providers:', err); + res.status(500).json({ error: 'An unexpected error occurred' }); + } + }; +} + +export function createResetPasswordHandler(deps: AuthHandlerDeps): RequestHandler { + return async (req: Request, res: Response): Promise => { + const email = typeof req.body?.email === 'string' ? req.body.email.trim() : ''; + + if (!email) { + res.status(400).json({ error: 'Email is required' }); + return; + } + + try { + await deps.firebaseRestClient.sendPasswordResetEmail(email); + res.status(200).json({ success: true }); + } catch (err) { + logger.error('Error sending password reset email:', err); + res.status(200).json({ success: true }); + } + }; +} + +export function createLogoutHandler(): RequestHandler { + return async (req: Request, res: Response): Promise => { + return new Promise((resolve) => { + req.logout((err) => { + if (err) { + logger.error('Error during logout:', err); + } + Object.keys(req.session as Record).forEach((key) => { + delete (req.session as Record)[key]; + }); + res.sendStatus(200); + resolve(); + }); + }); + }; +} diff --git a/src/server/auth/auth-router.ts b/src/server/auth/auth-router.ts new file mode 100644 index 000000000..6d428d0cb --- /dev/null +++ b/src/server/auth/auth-router.ts @@ -0,0 +1,86 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { Router } from 'express'; +import * as admin from 'firebase-admin'; +import { Firestore } from '@google-cloud/firestore'; + +import { FirebaseRestClient } from './firebase-rest-client'; +import { + createLoginHandler, + createSignupHandler, + createProvidersHandler, + createResetPasswordHandler, + createLogoutHandler, +} from './auth-handlers'; +import { + createGoogleOAuthInitiateHandler, + createGoogleOAuthCallbackHandler, + createAppleOAuthInitiateHandler, + createAppleOAuthCallbackHandler, + OAuthConfig, + AppleOAuthConfig, +} from './oauth-handlers'; +import { createFirestoreStateStore } from './oauth-state'; +import { Table } from '../models/table'; +import { User } from '../schemas/user_pb'; + +export interface AuthRouterDeps { + firebaseRestClient: FirebaseRestClient; + firebaseAdmin: admin.auth.Auth; + users: Table; + baseUrl: string; + firestore?: Firestore; + googleConfig?: OAuthConfig; + appleConfig?: AppleOAuthConfig; +} + +export function createAuthRouter(deps: AuthRouterDeps): Router { + const router = Router(); + + const handlerDeps = { + firebaseRestClient: deps.firebaseRestClient, + firebaseAdmin: deps.firebaseAdmin, + users: deps.users, + baseUrl: deps.baseUrl, + }; + + router.post('/login', createLoginHandler(handlerDeps)); + router.post('/signup', createSignupHandler(handlerDeps)); + router.post('/providers', createProvidersHandler(handlerDeps)); + router.post('/reset-password', createResetPasswordHandler(handlerDeps)); + router.post('/logout', createLogoutHandler()); + + if (deps.firestore && deps.googleConfig) { + const stateStore = createFirestoreStateStore(deps.firestore); + + const googleDeps = { + config: deps.googleConfig, + stateStore, + firebaseAdmin: deps.firebaseAdmin, + users: deps.users, + baseUrl: deps.baseUrl, + }; + + router.get('/google', createGoogleOAuthInitiateHandler(googleDeps)); + router.get('/google/callback', createGoogleOAuthCallbackHandler(googleDeps)); + } + + if (deps.firestore && deps.appleConfig) { + const stateStore = createFirestoreStateStore(deps.firestore); + + const appleDeps = { + config: deps.appleConfig, + stateStore, + firebaseAdmin: deps.firebaseAdmin, + users: deps.users, + baseUrl: deps.baseUrl, + }; + + router.get('/apple', createAppleOAuthInitiateHandler(appleDeps)); + router.post('/apple/callback', createAppleOAuthCallbackHandler(appleDeps)); + } + + return router; +} diff --git a/src/server/auth/firebase-rest-client.ts b/src/server/auth/firebase-rest-client.ts new file mode 100644 index 000000000..a4f0a7706 --- /dev/null +++ b/src/server/auth/firebase-rest-client.ts @@ -0,0 +1,141 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +export interface FirebaseAuthConfig { + apiKey: string; + emulatorHost?: string; +} + +export interface SignInResponse { + idToken: string; + email: string; + refreshToken: string; + expiresIn: string; + localId: string; + displayName?: string; +} + +export interface FetchProvidersResponse { + providers: string[]; + registered: boolean; +} + +export class FirebaseAuthError extends Error { + constructor( + public readonly code: string, + message: string, + ) { + super(message); + this.name = 'FirebaseAuthError'; + } +} + +const ERROR_MESSAGES: Record = { + EMAIL_NOT_FOUND: 'No account found with this email', + INVALID_PASSWORD: 'Incorrect password', + EMAIL_EXISTS: 'An account with this email already exists', + WEAK_PASSWORD: 'Password must be at least 6 characters', + USER_DISABLED: 'This account has been disabled', + TOO_MANY_ATTEMPTS_TRY_LATER: 'Too many attempts. Try again later.', + INVALID_EMAIL: 'Invalid email address', +}; + +function getErrorCode(rawMessage: string): string { + const colonIndex = rawMessage.indexOf(':'); + if (colonIndex !== -1) { + return rawMessage.substring(0, colonIndex).trim(); + } + return rawMessage; +} + +function parseErrorMessage(rawMessage: string): { code: string; message: string } { + const code = getErrorCode(rawMessage); + const message = ERROR_MESSAGES[code] ?? rawMessage; + return { code, message }; +} + +export interface FirebaseRestClient { + signInWithPassword(email: string, password: string): Promise; + signUp(email: string, password: string, displayName?: string): Promise; + fetchProviders(email: string, continueUri: string): Promise; + sendPasswordResetEmail(email: string): Promise; +} + +export function createFirebaseRestClient(config: FirebaseAuthConfig): FirebaseRestClient { + const baseUrl = config.emulatorHost + ? `http://${config.emulatorHost}/identitytoolkit.googleapis.com/v1` + : 'https://identitytoolkit.googleapis.com/v1'; + + async function request(endpoint: string, body: object): Promise { + const url = `${baseUrl}/${endpoint}?key=${config.apiKey}`; + const response = await fetch(url, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify(body), + }); + + const data = await response.json(); + + if (!response.ok) { + const rawMessage = data.error?.message ?? 'Unknown error'; + const { code, message } = parseErrorMessage(rawMessage); + throw new FirebaseAuthError(code, message); + } + + return data as T; + } + + return { + async signInWithPassword(email: string, password: string): Promise { + return request('accounts:signInWithPassword', { + email, + password, + returnSecureToken: true, + }); + }, + + async signUp(email: string, password: string, displayName?: string): Promise { + const body: Record = { + email, + password, + returnSecureToken: true, + }; + if (displayName) { + body.displayName = displayName; + } + return request('accounts:signUp', body); + }, + + async fetchProviders(email: string, continueUri: string): Promise { + interface RawResponse { + registered?: boolean; + allProviders?: string[]; + } + const data = await request('accounts:createAuthUri', { + identifier: email, + continueUri, + }); + return { + registered: data.registered ?? false, + providers: data.allProviders ?? [], + }; + }, + + async sendPasswordResetEmail(email: string): Promise { + try { + await request('accounts:sendOobCode', { + requestType: 'PASSWORD_RESET', + email, + }); + } catch (err) { + if (err instanceof FirebaseAuthError && err.code === 'EMAIL_NOT_FOUND') { + return; + } + throw err; + } + }, + }; +} diff --git a/src/server/auth/oauth-handlers.ts b/src/server/auth/oauth-handlers.ts new file mode 100644 index 000000000..af21cb5f9 --- /dev/null +++ b/src/server/auth/oauth-handlers.ts @@ -0,0 +1,333 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { Request, Response, RequestHandler } from 'express'; +import * as admin from 'firebase-admin'; +import * as logger from 'winston'; + +import { OAuthStateStore } from './oauth-state'; +import { validateReturnUrl } from './url-validation'; +import { + exchangeGoogleCode, + fetchGoogleUserInfo, + exchangeAppleCode, + verifyAppleIdToken, + generateAppleClientSecret, +} from './oauth-token-exchange'; +import { getOrCreateUserFromVerifiedInfo } from '../authn'; +import { Table } from '../models/table'; +import { User } from '../schemas/user_pb'; + +export interface OAuthConfig { + clientId: string; + clientSecret: string; + authorizationUrl: string; + tokenUrl: string; + scopes: string[]; + callbackPath: string; +} + +export interface AppleOAuthConfig extends OAuthConfig { + teamId: string; + keyId: string; + privateKey: string; +} + +export interface OAuthHandlerDeps { + stateStore: OAuthStateStore; + firebaseAdmin: admin.auth.Auth; + users: Table; + baseUrl: string; +} + +export interface GoogleOAuthHandlerDeps extends OAuthHandlerDeps { + config: OAuthConfig; +} + +export interface AppleOAuthHandlerDeps extends OAuthHandlerDeps { + config: AppleOAuthConfig; +} + +function loginUser(req: Request, user: User): Promise { + return new Promise((resolve, reject) => { + req.login(user, (err) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); +} + +export function createGoogleOAuthInitiateHandler(deps: GoogleOAuthHandlerDeps): RequestHandler { + return async (req: Request, res: Response): Promise => { + try { + const returnUrl = typeof req.query.returnUrl === 'string' ? req.query.returnUrl : undefined; + const state = await deps.stateStore.create(returnUrl); + + const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; + const params = new URLSearchParams({ + client_id: deps.config.clientId, + redirect_uri: redirectUri, + response_type: 'code', + scope: deps.config.scopes.join(' '), + state, + access_type: 'offline', + prompt: 'select_account', + }); + + res.redirect(`${deps.config.authorizationUrl}?${params.toString()}`); + } catch (err) { + logger.error('Error initiating Google OAuth:', err); + res.redirect('/?error=oauth_init_failed'); + } + }; +} + +export function createGoogleOAuthCallbackHandler(deps: GoogleOAuthHandlerDeps): RequestHandler { + return async (req: Request, res: Response): Promise => { + const { code, state, error } = req.query; + + if (error) { + logger.error('Google OAuth error:', error); + res.redirect('/?error=oauth_denied'); + return; + } + + if (typeof state !== 'string' || !state) { + res.status(400).json({ error: 'Missing state parameter' }); + return; + } + + if (typeof code !== 'string' || !code) { + res.status(400).json({ error: 'Missing code parameter' }); + return; + } + + try { + const stateResult = await deps.stateStore.validate(state); + if (!stateResult.valid) { + res.status(400).json({ error: 'Invalid or expired state' }); + return; + } + + const returnUrl = validateReturnUrl(stateResult.returnUrl, deps.baseUrl); + + await deps.stateStore.invalidate(state); + + const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; + const tokens = await exchangeGoogleCode( + deps.config.clientId, + deps.config.clientSecret, + code, + redirectUri, + ); + + const userInfo = await fetchGoogleUserInfo(tokens.access_token); + + try { + await deps.firebaseAdmin.getUserByEmail(userInfo.email); + } catch (err: unknown) { + const adminErr = err as { code?: string }; + if (adminErr.code === 'auth/user-not-found') { + await deps.firebaseAdmin.createUser({ + email: userInfo.email, + displayName: userInfo.name, + photoURL: userInfo.picture, + emailVerified: userInfo.email_verified, + }); + } else { + throw err; + } + } + + const [user, userErr] = await getOrCreateUserFromVerifiedInfo(deps.users, { + email: userInfo.email, + displayName: userInfo.name, + photoUrl: userInfo.picture, + provider: 'google', + providerUserId: userInfo.sub, + }); + + if (userErr) { + logger.error('Error creating user from Google info:', userErr); + res.redirect('/?error=user_creation_failed'); + return; + } + + await loginUser(req, user); + + res.redirect(returnUrl); + } catch (err) { + logger.error('Error in Google OAuth callback:', err); + await deps.stateStore.invalidate(state).catch(() => {}); + res.redirect('/?error=oauth_callback_failed'); + } + }; +} + +export function createAppleOAuthInitiateHandler(deps: AppleOAuthHandlerDeps): RequestHandler { + return async (req: Request, res: Response): Promise => { + try { + const returnUrl = typeof req.query.returnUrl === 'string' ? req.query.returnUrl : undefined; + const state = await deps.stateStore.create(returnUrl); + + const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; + const params = new URLSearchParams({ + client_id: deps.config.clientId, + redirect_uri: redirectUri, + response_type: 'code', + scope: deps.config.scopes.join(' '), + state, + response_mode: 'form_post', + }); + + res.redirect(`${deps.config.authorizationUrl}?${params.toString()}`); + } catch (err) { + logger.error('Error initiating Apple OAuth:', err); + res.redirect('/?error=oauth_init_failed'); + } + }; +} + +export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): RequestHandler { + return async (req: Request, res: Response): Promise => { + const { code, state, error, id_token: bodyIdToken, user: appleUserJson } = req.body; + + if (error) { + logger.error('Apple OAuth error:', error); + res.redirect('/?error=oauth_denied'); + return; + } + + if (typeof state !== 'string' || !state) { + res.status(400).json({ error: 'Missing state parameter' }); + return; + } + + if (typeof code !== 'string' || !code) { + res.status(400).json({ error: 'Missing code parameter' }); + return; + } + + try { + const stateResult = await deps.stateStore.validate(state); + if (!stateResult.valid) { + res.status(400).json({ error: 'Invalid or expired state' }); + return; + } + + const returnUrl = validateReturnUrl(stateResult.returnUrl, deps.baseUrl); + + await deps.stateStore.invalidate(state); + + const clientSecret = generateAppleClientSecret( + deps.config.teamId, + deps.config.clientId, + deps.config.keyId, + deps.config.privateKey, + ); + + const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; + const tokens = await exchangeAppleCode( + deps.config.clientId, + clientSecret, + code, + redirectUri, + ); + + const idToken = tokens.id_token || bodyIdToken; + if (!idToken) { + throw new Error('No ID token received from Apple'); + } + + const claims = await verifyAppleIdToken(idToken, { clientId: deps.config.clientId }); + + let appleUserName: string | undefined; + if (typeof appleUserJson === 'string') { + try { + const appleUser = JSON.parse(appleUserJson); + if (appleUser.name) { + const { firstName, lastName } = appleUser.name; + appleUserName = [firstName, lastName].filter(Boolean).join(' '); + } + } catch { + // Ignore JSON parse errors + } + } + + const displayName = appleUserName || claims.name || claims.email || 'Apple User'; + const email = claims.email; + + if (!email) { + let fbUser: admin.auth.UserRecord | undefined; + try { + const listResult = await deps.firebaseAdmin.listUsers(1000); + for (const user of listResult.users) { + const appleProviderData = user.providerData.find((p) => p.providerId === 'apple.com'); + if (appleProviderData && appleProviderData.uid === claims.sub) { + fbUser = user; + break; + } + } + } catch (err) { + logger.error('Error searching for Apple user by sub:', err); + } + + if (!fbUser) { + logger.error('Apple user has no email and could not be found by sub'); + res.redirect('/?error=apple_no_email'); + return; + } + + const existingUser = await deps.users.findOneByScan({ providerUserId: claims.sub }); + if (existingUser) { + await loginUser(req, existingUser); + res.redirect(returnUrl); + return; + } + + res.redirect('/?error=apple_user_not_found'); + return; + } + + try { + await deps.firebaseAdmin.getUserByEmail(email); + } catch (err: unknown) { + const adminErr = err as { code?: string }; + if (adminErr.code === 'auth/user-not-found') { + await deps.firebaseAdmin.createUser({ + email, + displayName, + emailVerified: claims.email_verified ?? false, + }); + } else { + throw err; + } + } + + const [user, userErr] = await getOrCreateUserFromVerifiedInfo(deps.users, { + email, + displayName, + provider: 'apple', + providerUserId: claims.sub, + }); + + if (userErr) { + logger.error('Error creating user from Apple info:', userErr); + res.redirect('/?error=user_creation_failed'); + return; + } + + await loginUser(req, user); + + res.redirect(returnUrl); + } catch (err) { + logger.error('Error in Apple OAuth callback:', err); + await deps.stateStore.invalidate(state).catch(() => {}); + res.redirect('/?error=oauth_callback_failed'); + } + }; +} diff --git a/src/server/auth/oauth-state.ts b/src/server/auth/oauth-state.ts new file mode 100644 index 000000000..9a5837477 --- /dev/null +++ b/src/server/auth/oauth-state.ts @@ -0,0 +1,87 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { Firestore } from '@google-cloud/firestore'; +import { randomBytes } from 'crypto'; + +export interface OAuthState { + state: string; + returnUrl?: string; + createdAt: Date; + expiresAt: Date; +} + +export interface OAuthStateStore { + create(returnUrl?: string): Promise; + validate(state: string): Promise<{ valid: boolean; returnUrl?: string }>; + invalidate(state: string): Promise; +} + +const DEFAULT_TTL_MS = 10 * 60 * 1000; // 10 minutes + +export function createFirestoreStateStore( + firestore: Firestore, + collectionName = 'oauth_state', + ttlMs = DEFAULT_TTL_MS, +): OAuthStateStore { + const collection = firestore.collection(collectionName); + + return { + async create(returnUrl?: string): Promise { + const state = randomBytes(32).toString('hex'); + const now = new Date(); + const expiresAt = new Date(now.getTime() + ttlMs); + + const data: Record = { + createdAt: now, + expiresAt, + }; + + if (returnUrl !== undefined) { + data.returnUrl = returnUrl; + } + + await collection.doc(state).set(data); + + return state; + }, + + async validate(state: string): Promise<{ valid: boolean; returnUrl?: string }> { + const doc = await collection.doc(state).get(); + + if (!doc.exists) { + return { valid: false }; + } + + const data = doc.data(); + if (!data) { + return { valid: false }; + } + + const expiresAt = data.expiresAt; + let expiresAtDate: Date; + + if (expiresAt && typeof expiresAt.toDate === 'function') { + expiresAtDate = expiresAt.toDate(); + } else if (expiresAt instanceof Date) { + expiresAtDate = expiresAt; + } else { + return { valid: false }; + } + + if (expiresAtDate < new Date()) { + return { valid: false }; + } + + return { + valid: true, + returnUrl: data.returnUrl as string | undefined, + }; + }, + + async invalidate(state: string): Promise { + await collection.doc(state).delete(); + }, + }; +} diff --git a/src/server/auth/oauth-token-exchange.ts b/src/server/auth/oauth-token-exchange.ts new file mode 100644 index 000000000..22b44a6dd --- /dev/null +++ b/src/server/auth/oauth-token-exchange.ts @@ -0,0 +1,183 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import * as jose from 'jose'; + +export interface TokenResponse { + access_token: string; + id_token?: string; + refresh_token?: string; + expires_in: number; + token_type: string; +} + +export interface GoogleUserInfo { + sub: string; + email: string; + email_verified: boolean; + name: string; + picture?: string; +} + +export interface AppleIdTokenClaims { + sub: string; + email?: string; + email_verified?: boolean; + name?: string; +} + +export async function exchangeGoogleCode( + clientId: string, + clientSecret: string, + code: string, + redirectUri: string, +): Promise { + const response = await fetch('https://oauth2.googleapis.com/token', { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + body: new URLSearchParams({ + client_id: clientId, + client_secret: clientSecret, + code, + redirect_uri: redirectUri, + grant_type: 'authorization_code', + }).toString(), + }); + + if (!response.ok) { + const error = await response.text(); + throw new Error(`Google token exchange failed: ${error}`); + } + + return (await response.json()) as TokenResponse; +} + +export async function fetchGoogleUserInfo(accessToken: string): Promise { + const response = await fetch('https://www.googleapis.com/oauth2/v3/userinfo', { + headers: { + Authorization: `Bearer ${accessToken}`, + }, + }); + + if (!response.ok) { + const error = await response.text(); + throw new Error(`Failed to fetch Google user info: ${error}`); + } + + return (await response.json()) as GoogleUserInfo; +} + +export function generateAppleClientSecret( + teamId: string, + clientId: string, + keyId: string, + privateKey: string, +): string { + const now = Math.floor(Date.now() / 1000); + const expiresIn = 15777000; // ~6 months + + const header = { + alg: 'ES256', + kid: keyId, + }; + + const payload = { + iss: teamId, + iat: now, + exp: now + expiresIn, + aud: 'https://appleid.apple.com', + sub: clientId, + }; + + const privateKeyObj = require('crypto').createPrivateKey(privateKey); + const sign = require('crypto').createSign('SHA256'); + + const headerB64 = Buffer.from(JSON.stringify(header)).toString('base64url'); + const payloadB64 = Buffer.from(JSON.stringify(payload)).toString('base64url'); + const signingInput = `${headerB64}.${payloadB64}`; + + sign.update(signingInput); + const signature = sign.sign(privateKeyObj); + + const r = signature.slice(0, 32); + const s = signature.slice(32, 64); + const signatureB64 = Buffer.concat([r, s]).toString('base64url'); + + return `${signingInput}.${signatureB64}`; +} + +export async function exchangeAppleCode( + clientId: string, + clientSecret: string, + code: string, + redirectUri: string, +): Promise { + const response = await fetch('https://appleid.apple.com/auth/token', { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + body: new URLSearchParams({ + client_id: clientId, + client_secret: clientSecret, + code, + redirect_uri: redirectUri, + grant_type: 'authorization_code', + }).toString(), + }); + + if (!response.ok) { + const error = await response.text(); + throw new Error(`Apple token exchange failed: ${error}`); + } + + return (await response.json()) as TokenResponse; +} + +let cachedJwks: jose.JSONWebKeySet | undefined; +let jwksCacheTime = 0; +const JWKS_CACHE_TTL_MS = 60 * 60 * 1000; // 1 hour + +async function fetchAppleJwks(): Promise { + const now = Date.now(); + if (cachedJwks && now - jwksCacheTime < JWKS_CACHE_TTL_MS) { + return cachedJwks; + } + + const response = await fetch('https://appleid.apple.com/auth/keys'); + if (!response.ok) { + throw new Error('Failed to fetch Apple JWKS'); + } + + cachedJwks = (await response.json()) as jose.JSONWebKeySet; + jwksCacheTime = now; + return cachedJwks; +} + +export async function verifyAppleIdToken( + idToken: string, + options: { clientId: string }, +): Promise { + const jwks = await fetchAppleJwks(); + const JWKS = jose.createLocalJWKSet(jwks); + + const { payload } = await jose.jwtVerify(idToken, JWKS, { + issuer: 'https://appleid.apple.com', + audience: options.clientId, + }); + + return { + sub: payload.sub as string, + email: payload.email as string | undefined, + email_verified: payload.email_verified as boolean | undefined, + name: (payload as { name?: string }).name, + }; +} + +export function clearJwksCache(): void { + cachedJwks = undefined; + jwksCacheTime = 0; +} diff --git a/src/server/auth/url-validation.ts b/src/server/auth/url-validation.ts new file mode 100644 index 000000000..6e0a425af --- /dev/null +++ b/src/server/auth/url-validation.ts @@ -0,0 +1,49 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +const DANGEROUS_PROTOCOLS = ['javascript:', 'data:', 'vbscript:', 'file:']; + +export function validateReturnUrl(returnUrl: string | undefined, baseUrl: string): string { + if (!returnUrl || returnUrl.trim() === '') { + return '/'; + } + + const trimmedUrl = returnUrl.trim(); + + if (DANGEROUS_PROTOCOLS.some((proto) => trimmedUrl.toLowerCase().startsWith(proto))) { + return '/'; + } + + if (trimmedUrl.startsWith('//')) { + return '/'; + } + + if (trimmedUrl.includes('\\')) { + return '/'; + } + + if (trimmedUrl.startsWith('/') && !trimmedUrl.startsWith('//')) { + if (!trimmedUrl.includes('\\') && !trimmedUrl.includes('\n') && !trimmedUrl.includes('\r')) { + return trimmedUrl; + } + return '/'; + } + + try { + const returnUrlObj = new URL(returnUrl); + const baseUrlObj = new URL(baseUrl); + + if (returnUrlObj.protocol !== baseUrlObj.protocol) { + return '/'; + } + + if (returnUrlObj.host !== baseUrlObj.host) { + return '/'; + } + + return returnUrl; + } catch { + return '/'; + } +} diff --git a/src/server/authn.ts b/src/server/authn.ts index 3f8e52336..d1275c178 100644 --- a/src/server/authn.ts +++ b/src/server/authn.ts @@ -14,6 +14,16 @@ import { Application } from './application'; import { Table } from './models/table'; import { User } from './schemas/user_pb'; +export type AuthProvider = 'google' | 'apple' | 'password'; + +export interface VerifiedUserInfo { + email: string; + displayName: string; + photoUrl?: string; + provider: AuthProvider; + providerUserId: string; +} + interface StrategyOptions {} interface VerifyFunction { @@ -56,7 +66,21 @@ class FirestoreAuthStrategy extends BaseStrategy implements passport.Strategy { } } -async function getOrCreateUserFromProfile( +function getProviderFromFirebaseUser(fbUser: admin.auth.UserRecord): AuthProvider { + if (!fbUser.providerData || fbUser.providerData.length === 0) { + return 'password'; + } + const providerIds = fbUser.providerData.map((p) => p.providerId); + if (providerIds.includes('google.com')) { + return 'google'; + } + if (providerIds.includes('apple.com')) { + return 'apple'; + } + return 'password'; +} + +export async function getOrCreateUserFromIdToken( users: Table, firebaseAuthn: admin.auth.Auth, firebaseIdToken: string, @@ -88,13 +112,10 @@ async function getOrCreateUserFromProfile( } const email = fbUser.email; - // TODO: should we verify the email? - const displayName = fbUser.displayName ?? email; const photoUrl = fbUser.photoURL; + const provider = getProviderFromFirebaseUser(fbUser); - // since a document with the email already exists, just get the - // document with it let user: User | undefined; try { user = await users.findOneByScan({ email }); @@ -106,7 +127,8 @@ async function getOrCreateUserFromProfile( user.setId(`temp-${uuidV4()}`); user.setEmail(email); user.setDisplayName(displayName); - user.setProvider('google'); + user.setProvider(provider); + user.setProviderUserId(decodedToken.uid); if (photoUrl) { user.setPhotoUrl(photoUrl); } @@ -139,7 +161,6 @@ async function getOrCreateUserFromProfile( } } } - // it should work now user = await users.findOneByScan({ email }); } } @@ -153,12 +174,82 @@ async function getOrCreateUserFromProfile( return [user, undefined]; } +export async function getOrCreateUserFromVerifiedInfo( + users: Table, + info: VerifiedUserInfo, +): Promise<[User, undefined] | [undefined, Error]> { + if (!info.email) { + return [undefined, new Error('expected user to have an email')]; + } + + let user: User | undefined; + try { + if (info.providerUserId) { + user = await users.findOneByScan({ providerUserId: info.providerUserId }); + } + if (!user && info.email) { + user = await users.findOneByScan({ email: info.email }); + } + if (!user) { + const created = new Timestamp(); + created.fromDate(new Date()); + + user = new User(); + user.setId(`temp-${uuidV4()}`); + user.setEmail(info.email); + user.setDisplayName(info.displayName); + user.setProvider(info.provider); + user.setProviderUserId(info.providerUserId); + if (info.photoUrl) { + user.setPhotoUrl(info.photoUrl); + } + user.setCreated(created); + user.setCanCreateProjects(false); + + await users.create(user.getId(), user); + } + } catch (err) { + if (err instanceof Error) { + if (err.message.includes('expected single result document')) { + const userDocs = await users.findByScan({ email: info.email }); + if (userDocs) { + let fullUserFound = false; + for (const user of userDocs) { + if (!user.getId().startsWith('temp-')) { + fullUserFound = true; + break; + } + } + if (fullUserFound) { + for (const user of userDocs) { + const userId = user.getId(); + if (userId.startsWith('temp-')) { + logger.info(`fixing inconsistency with ${info.email} -- deleting '${userId}' in DB`); + await users.deleteOne(userId); + } + } + } + user = await users.findOneByScan({ email: info.email }); + } + } + } + } + + if (!user) { + return [undefined, new Error(`unable to insert or find user ${info.email}`)]; + } + + return [user, undefined]; +} + export const authn = (app: Application, firebaseAuthn: admin.auth.Auth): void => { // const config = app.get('authentication'); + // DEPRECATED: Use /auth/login instead. This endpoint exists for backward + // compatibility with existing mobile apps and will be removed in a future release. passport.use( new FirestoreAuthStrategy({}, async (firestoreIdToken: string, done: (error: any, user?: any) => void) => { - const [user, err] = await getOrCreateUserFromProfile(app.db.user, firebaseAuthn, firestoreIdToken); + const [user, err] = await getOrCreateUserFromIdToken(app.db.user, firebaseAuthn, firestoreIdToken); if (err !== undefined) { logger.error(err); done(err); diff --git a/src/server/jest.config.js b/src/server/jest.config.js index fb67cd81a..755a3d47c 100644 --- a/src/server/jest.config.js +++ b/src/server/jest.config.js @@ -15,6 +15,7 @@ const config = { '^@simlin/core/(.*)$': '/../core/lib/$1.js', '^@simlin/core$': '/../core/lib/index.js', }, + transformIgnorePatterns: ['/node_modules/(?!(jose)/)'], }; module.exports = config; diff --git a/src/server/models/db-firestore.ts b/src/server/models/db-firestore.ts index fa8d61b9b..5bb308017 100644 --- a/src/server/models/db-firestore.ts +++ b/src/server/models/db-firestore.ts @@ -14,15 +14,15 @@ import { FirestoreTable } from './table-firestore'; import { Table } from './table'; export class FirestoreDatabase implements Database { - private readonly client: Firestore; + readonly firestore: Firestore; readonly file: Table; readonly project: Table; readonly preview: Table; readonly user: Table; constructor(client: Firestore) { - this.client = client; - const db = this.client; + this.firestore = client; + const db = this.firestore; this.file = new FirestoreTable(File, { db, name: 'files' }); this.project = new FirestoreTable(Project, { db, name: 'project', hoistColumns: { version: 7 } }); diff --git a/src/server/models/db-interfaces.ts b/src/server/models/db-interfaces.ts index ab72418e4..23d13e8e0 100644 --- a/src/server/models/db-interfaces.ts +++ b/src/server/models/db-interfaces.ts @@ -2,6 +2,8 @@ // Use of this source code is governed by the Apache License, // Version 2.0, that can be found in the LICENSE file. +import { Firestore } from '@google-cloud/firestore'; + import { File } from '../schemas/file_pb'; import { Preview } from '../schemas/preview_pb'; import { Project } from '../schemas/project_pb'; @@ -19,4 +21,5 @@ export interface Database { readonly project: Table; readonly preview: Table; readonly user: Table; + readonly firestore?: Firestore; } diff --git a/src/server/package.json b/src/server/package.json index 176a2f016..0d1d9c057 100644 --- a/src/server/package.json +++ b/src/server/package.json @@ -22,6 +22,7 @@ "google-protobuf": "^4.0.0", "helmet": "^8.0.0", "immutable": "^5.0.3", + "jose": "^6.1.3", "node-fetch": "^3.0.0", "passport": "^0.5.3", "passport-strategy": "^1.0.0", diff --git a/src/server/schemas/user.proto b/src/server/schemas/user.proto index 98ba13f48..90f0fdec9 100644 --- a/src/server/schemas/user.proto +++ b/src/server/schemas/user.proto @@ -16,4 +16,5 @@ message User { bool is_admin = 6; bool is_deactivated = 7; bool can_create_projects = 9; + string provider_user_id = 10; // Stable identifier from OAuth provider (sub claim) } diff --git a/src/server/schemas/user_pb.d.ts b/src/server/schemas/user_pb.d.ts index f8856b916..32a5fdb2c 100644 --- a/src/server/schemas/user_pb.d.ts +++ b/src/server/schemas/user_pb.d.ts @@ -34,6 +34,9 @@ export class User extends jspb.Message { getCanCreateProjects(): boolean; setCanCreateProjects(value: boolean): void; + getProviderUserId(): string; + setProviderUserId(value: string): void; + serializeBinary(): Uint8Array; toObject(includeInstance?: boolean): User.AsObject; static toObject(includeInstance: boolean, msg: User): User.AsObject; @@ -55,5 +58,6 @@ export namespace User { isAdmin: boolean; isDeactivated: boolean; canCreateProjects: boolean; + providerUserId: string; }; } diff --git a/src/server/schemas/user_pb.js b/src/server/schemas/user_pb.js index d1124af1b..dbd2c8b6d 100644 --- a/src/server/schemas/user_pb.js +++ b/src/server/schemas/user_pb.js @@ -79,7 +79,8 @@ provider: jspb.Message.getFieldWithDefault(msg, 5, ""), created: (f = msg.getCreated()) && google_protobuf_timestamp_pb.Timestamp.toObject(includeInstance, f), isAdmin: jspb.Message.getBooleanFieldWithDefault(msg, 6, false), isDeactivated: jspb.Message.getBooleanFieldWithDefault(msg, 7, false), -canCreateProjects: jspb.Message.getBooleanFieldWithDefault(msg, 9, false) +canCreateProjects: jspb.Message.getBooleanFieldWithDefault(msg, 9, false), +providerUserId: jspb.Message.getFieldWithDefault(msg, 10, "") }; if (includeInstance) { @@ -153,6 +154,10 @@ proto.User.deserializeBinaryFromReader = function(msg, reader) { var value = /** @type {boolean} */ (reader.readBool()); msg.setCanCreateProjects(value); break; + case 10: + var value = /** @type {string} */ (reader.readStringRequireUtf8()); + msg.setProviderUserId(value); + break; default: reader.skipField(); break; @@ -246,6 +251,13 @@ proto.User.serializeBinaryToWriter = function(message, writer) { f ); } + f = message.getProviderUserId(); + if (f.length > 0) { + writer.writeString( + 10, + f + ); + } }; @@ -430,4 +442,22 @@ proto.User.prototype.setCanCreateProjects = function(value) { }; +/** + * optional string provider_user_id = 10; + * @return {string} + */ +proto.User.prototype.getProviderUserId = function() { + return /** @type {string} */ (jspb.Message.getFieldWithDefault(this, 10, "")); +}; + + +/** + * @param {string} value + * @return {!proto.User} returns this + */ +proto.User.prototype.setProviderUserId = function(value) { + return jspb.Message.setProto3StringField(this, 10, value); +}; + + goog.object.extend(exports, proto); diff --git a/src/server/tests/auth-handlers.test.ts b/src/server/tests/auth-handlers.test.ts new file mode 100644 index 000000000..f85de4259 --- /dev/null +++ b/src/server/tests/auth-handlers.test.ts @@ -0,0 +1,897 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { Request, Response } from 'express'; +import * as admin from 'firebase-admin'; + +import { + createLoginHandler, + createSignupHandler, + createProvidersHandler, + createResetPasswordHandler, + createLogoutHandler, + AuthHandlerDeps, +} from '../auth/auth-handlers'; +import { FirebaseRestClient, FirebaseAuthError } from '../auth/firebase-rest-client'; +import { Table } from '../models/table'; +import { User } from '../schemas/user_pb'; + +function createMockFirebaseRestClient(): jest.Mocked { + return { + signInWithPassword: jest.fn(), + signUp: jest.fn(), + fetchProviders: jest.fn(), + sendPasswordResetEmail: jest.fn(), + }; +} + +function createMockFirebaseAdmin(): jest.Mocked { + return { + verifyIdToken: jest.fn(), + getUser: jest.fn(), + updateUser: jest.fn(), + } as unknown as jest.Mocked; +} + +function createMockUsers(): jest.Mocked> { + return { + init: jest.fn(), + findOne: jest.fn(), + findOneByScan: jest.fn(), + findByScan: jest.fn(), + find: jest.fn(), + create: jest.fn(), + update: jest.fn(), + deleteOne: jest.fn(), + }; +} + +function createMockUser(id: string, email: string, displayName: string): User { + const user = new User(); + user.setId(id); + user.setEmail(email); + user.setDisplayName(displayName); + return user; +} + +function createMockRequest(body: object = {}): Partial { + const loginFn = jest.fn((user: unknown, cb: (err?: Error) => void) => cb()); + const logoutFn = jest.fn((cb: (err?: Error) => void) => cb()); + return { + body, + session: {} as Request['session'], + login: loginFn as unknown as Request['login'], + logout: logoutFn as unknown as Request['logout'], + }; +} + +interface MockResponseResult { + res: Partial; + getStatus: () => number; + getBody: () => unknown; +} + +function createMockResponse(): MockResponseResult { + let status = 200; + let body: unknown; + const res: Partial = { + status: jest.fn((s: number) => { + status = s; + return res as Response; + }), + json: jest.fn((b: unknown) => { + body = b; + return res as Response; + }), + sendStatus: jest.fn((s: number) => { + status = s; + return res as Response; + }), + }; + return { res, getStatus: () => status, getBody: () => body }; +} + +function createMockDeps(): AuthHandlerDeps & { + firebaseRestClient: jest.Mocked; + firebaseAdmin: jest.Mocked; + users: jest.Mocked>; +} { + return { + firebaseRestClient: createMockFirebaseRestClient(), + firebaseAdmin: createMockFirebaseAdmin(), + users: createMockUsers(), + baseUrl: 'http://localhost:3030', + }; +} + +describe('createLoginHandler', () => { + describe('validation', () => { + it('should return 400 if email missing', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + const req = createMockRequest({ password: 'password123' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'Email is required' }); + }); + + it('should return 400 if password missing', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + const req = createMockRequest({ email: 'test@example.com' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'Password is required' }); + }); + + it('should return 400 if email is empty string', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + const req = createMockRequest({ email: ' ', password: 'password123' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'Email is required' }); + }); + + it('should trim whitespace from email and password', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + deps.firebaseRestClient.signInWithPassword.mockResolvedValue({ + idToken: 'token123', + email: 'test@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }); + deps.firebaseAdmin.verifyIdToken.mockResolvedValue({ + uid: 'uid123', + aud: '', + auth_time: 0, + exp: 0, + iat: 0, + iss: '', + sub: '', + firebase: { identities: {}, sign_in_provider: 'password' }, + }); + deps.firebaseAdmin.getUser.mockResolvedValue({ + uid: 'uid123', + email: 'test@example.com', + emailVerified: true, + disabled: false, + metadata: { creationTime: '', lastSignInTime: '' }, + providerData: [{ providerId: 'password', uid: 'uid123' }], + toJSON: () => ({}), + } as admin.auth.UserRecord); + deps.users.findOneByScan.mockResolvedValue(createMockUser('testuser', 'test@example.com', 'Test User')); + + const req = createMockRequest({ email: ' test@example.com ', password: ' password123 ' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(deps.firebaseRestClient.signInWithPassword).toHaveBeenCalledWith('test@example.com', 'password123'); + }); + }); + + describe('successful login', () => { + it('should call firebaseRestClient.signInWithPassword with credentials', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + deps.firebaseRestClient.signInWithPassword.mockResolvedValue({ + idToken: 'token123', + email: 'test@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }); + deps.firebaseAdmin.verifyIdToken.mockResolvedValue({ + uid: 'uid123', + aud: '', + auth_time: 0, + exp: 0, + iat: 0, + iss: '', + sub: '', + firebase: { identities: {}, sign_in_provider: 'password' }, + }); + deps.firebaseAdmin.getUser.mockResolvedValue({ + uid: 'uid123', + email: 'test@example.com', + emailVerified: true, + disabled: false, + metadata: { creationTime: '', lastSignInTime: '' }, + providerData: [{ providerId: 'password', uid: 'uid123' }], + toJSON: () => ({}), + } as admin.auth.UserRecord); + deps.users.findOneByScan.mockResolvedValue(createMockUser('testuser', 'test@example.com', 'Test User')); + + const req = createMockRequest({ email: 'test@example.com', password: 'password123' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(deps.firebaseRestClient.signInWithPassword).toHaveBeenCalledWith('test@example.com', 'password123'); + }); + + it('should verify idToken with admin SDK', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + deps.firebaseRestClient.signInWithPassword.mockResolvedValue({ + idToken: 'token123', + email: 'test@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }); + deps.firebaseAdmin.verifyIdToken.mockResolvedValue({ + uid: 'uid123', + aud: '', + auth_time: 0, + exp: 0, + iat: 0, + iss: '', + sub: '', + firebase: { identities: {}, sign_in_provider: 'password' }, + }); + deps.firebaseAdmin.getUser.mockResolvedValue({ + uid: 'uid123', + email: 'test@example.com', + emailVerified: true, + disabled: false, + metadata: { creationTime: '', lastSignInTime: '' }, + providerData: [{ providerId: 'password', uid: 'uid123' }], + toJSON: () => ({}), + } as admin.auth.UserRecord); + deps.users.findOneByScan.mockResolvedValue(createMockUser('testuser', 'test@example.com', 'Test User')); + + const req = createMockRequest({ email: 'test@example.com', password: 'password123' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(deps.firebaseAdmin.verifyIdToken).toHaveBeenCalledWith('token123'); + }); + + it('should call req.login to create session', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + deps.firebaseRestClient.signInWithPassword.mockResolvedValue({ + idToken: 'token123', + email: 'test@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }); + deps.firebaseAdmin.verifyIdToken.mockResolvedValue({ + uid: 'uid123', + aud: '', + auth_time: 0, + exp: 0, + iat: 0, + iss: '', + sub: '', + firebase: { identities: {}, sign_in_provider: 'password' }, + }); + deps.firebaseAdmin.getUser.mockResolvedValue({ + uid: 'uid123', + email: 'test@example.com', + emailVerified: true, + disabled: false, + metadata: { creationTime: '', lastSignInTime: '' }, + providerData: [{ providerId: 'password', uid: 'uid123' }], + toJSON: () => ({}), + } as admin.auth.UserRecord); + const mockUser = createMockUser('testuser', 'test@example.com', 'Test User'); + deps.users.findOneByScan.mockResolvedValue(mockUser); + + const req = createMockRequest({ email: 'test@example.com', password: 'password123' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(req.login).toHaveBeenCalledWith(mockUser, expect.any(Function)); + }); + + it('should return 200 with user data', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + deps.firebaseRestClient.signInWithPassword.mockResolvedValue({ + idToken: 'token123', + email: 'test@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }); + deps.firebaseAdmin.verifyIdToken.mockResolvedValue({ + uid: 'uid123', + aud: '', + auth_time: 0, + exp: 0, + iat: 0, + iss: '', + sub: '', + firebase: { identities: {}, sign_in_provider: 'password' }, + }); + deps.firebaseAdmin.getUser.mockResolvedValue({ + uid: 'uid123', + email: 'test@example.com', + emailVerified: true, + disabled: false, + metadata: { creationTime: '', lastSignInTime: '' }, + providerData: [{ providerId: 'password', uid: 'uid123' }], + toJSON: () => ({}), + } as admin.auth.UserRecord); + deps.users.findOneByScan.mockResolvedValue(createMockUser('testuser', 'test@example.com', 'Test User')); + + const req = createMockRequest({ email: 'test@example.com', password: 'password123' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(200); + expect(getBody()).toEqual({ + success: true, + user: { + id: 'testuser', + email: 'test@example.com', + displayName: 'Test User', + }, + }); + }); + }); + + describe('error handling', () => { + it('should return 401 for INVALID_PASSWORD error', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + deps.firebaseRestClient.signInWithPassword.mockRejectedValue( + new FirebaseAuthError('INVALID_PASSWORD', 'Incorrect password'), + ); + + const req = createMockRequest({ email: 'test@example.com', password: 'wrongpass' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(401); + expect(getBody()).toEqual({ error: 'Incorrect password' }); + }); + + it('should return 401 for EMAIL_NOT_FOUND error', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + deps.firebaseRestClient.signInWithPassword.mockRejectedValue( + new FirebaseAuthError('EMAIL_NOT_FOUND', 'No account found with this email'), + ); + + const req = createMockRequest({ email: 'unknown@example.com', password: 'password123' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(401); + expect(getBody()).toEqual({ error: 'No account found with this email' }); + }); + + it('should return 403 for USER_DISABLED error', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + deps.firebaseRestClient.signInWithPassword.mockRejectedValue( + new FirebaseAuthError('USER_DISABLED', 'This account has been disabled'), + ); + + const req = createMockRequest({ email: 'disabled@example.com', password: 'password123' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(403); + expect(getBody()).toEqual({ error: 'This account has been disabled' }); + }); + + it('should return 500 for unexpected errors', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + deps.firebaseRestClient.signInWithPassword.mockRejectedValue(new Error('Network error')); + + const req = createMockRequest({ email: 'test@example.com', password: 'password123' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(500); + expect(getBody()).toEqual({ error: 'An unexpected error occurred' }); + }); + + it('should not include stack traces in error responses', async () => { + const deps = createMockDeps(); + const handler = createLoginHandler(deps); + + const errorWithStack = new Error('Network error'); + errorWithStack.stack = 'Error: Network error\n at someFunction (file.js:123)'; + deps.firebaseRestClient.signInWithPassword.mockRejectedValue(errorWithStack); + + const req = createMockRequest({ email: 'test@example.com', password: 'password123' }); + const { res, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + const body = getBody() as { error: string; stack?: string }; + expect(body.stack).toBeUndefined(); + }); + }); +}); + +describe('createSignupHandler', () => { + describe('validation', () => { + it('should return 400 if displayName missing', async () => { + const deps = createMockDeps(); + const handler = createSignupHandler(deps); + + const req = createMockRequest({ email: 'test@example.com', password: 'password123' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'Display name is required' }); + }); + + it('should return 400 if password too short', async () => { + const deps = createMockDeps(); + const handler = createSignupHandler(deps); + + deps.firebaseRestClient.signUp.mockRejectedValue( + new FirebaseAuthError('WEAK_PASSWORD', 'Password must be at least 6 characters'), + ); + + const req = createMockRequest({ email: 'test@example.com', password: '123', displayName: 'Test' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'Password must be at least 6 characters' }); + }); + }); + + describe('successful signup', () => { + it('should create Firebase user', async () => { + const deps = createMockDeps(); + const handler = createSignupHandler(deps); + + deps.firebaseRestClient.signUp.mockResolvedValue({ + idToken: 'token123', + email: 'new@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }); + deps.firebaseAdmin.verifyIdToken.mockResolvedValue({ + uid: 'uid123', + aud: '', + auth_time: 0, + exp: 0, + iat: 0, + iss: '', + sub: '', + firebase: { identities: {}, sign_in_provider: 'password' }, + }); + deps.firebaseAdmin.updateUser.mockResolvedValue({} as admin.auth.UserRecord); + deps.firebaseAdmin.getUser.mockResolvedValue({ + uid: 'uid123', + email: 'new@example.com', + displayName: 'New User', + emailVerified: false, + disabled: false, + metadata: { creationTime: '', lastSignInTime: '' }, + providerData: [{ providerId: 'password', uid: 'uid123' }], + toJSON: () => ({}), + } as admin.auth.UserRecord); + deps.users.findOneByScan.mockResolvedValue(undefined); + deps.users.create.mockResolvedValue(); + + const req = createMockRequest({ email: 'new@example.com', password: 'password123', displayName: 'New User' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(deps.firebaseRestClient.signUp).toHaveBeenCalledWith('new@example.com', 'password123', 'New User'); + }); + + it('should set displayName via updateUser', async () => { + const deps = createMockDeps(); + const handler = createSignupHandler(deps); + + deps.firebaseRestClient.signUp.mockResolvedValue({ + idToken: 'token123', + email: 'new@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }); + deps.firebaseAdmin.verifyIdToken.mockResolvedValue({ + uid: 'uid123', + aud: '', + auth_time: 0, + exp: 0, + iat: 0, + iss: '', + sub: '', + firebase: { identities: {}, sign_in_provider: 'password' }, + }); + deps.firebaseAdmin.updateUser.mockResolvedValue({} as admin.auth.UserRecord); + deps.firebaseAdmin.getUser.mockResolvedValue({ + uid: 'uid123', + email: 'new@example.com', + displayName: 'New User', + emailVerified: false, + disabled: false, + metadata: { creationTime: '', lastSignInTime: '' }, + providerData: [{ providerId: 'password', uid: 'uid123' }], + toJSON: () => ({}), + } as admin.auth.UserRecord); + deps.users.findOneByScan.mockResolvedValue(undefined); + deps.users.create.mockResolvedValue(); + + const req = createMockRequest({ email: 'new@example.com', password: 'password123', displayName: 'New User' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(deps.firebaseAdmin.updateUser).toHaveBeenCalledWith('uid123', { displayName: 'New User' }); + }); + + it('should create local user record with temp- prefix', async () => { + const deps = createMockDeps(); + const handler = createSignupHandler(deps); + + deps.firebaseRestClient.signUp.mockResolvedValue({ + idToken: 'token123', + email: 'new@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }); + deps.firebaseAdmin.verifyIdToken.mockResolvedValue({ + uid: 'uid123', + aud: '', + auth_time: 0, + exp: 0, + iat: 0, + iss: '', + sub: '', + firebase: { identities: {}, sign_in_provider: 'password' }, + }); + deps.firebaseAdmin.updateUser.mockResolvedValue({} as admin.auth.UserRecord); + deps.firebaseAdmin.getUser.mockResolvedValue({ + uid: 'uid123', + email: 'new@example.com', + displayName: 'New User', + emailVerified: false, + disabled: false, + metadata: { creationTime: '', lastSignInTime: '' }, + providerData: [{ providerId: 'password', uid: 'uid123' }], + toJSON: () => ({}), + } as admin.auth.UserRecord); + deps.users.findOneByScan.mockResolvedValue(undefined); + deps.users.create.mockResolvedValue(); + + const req = createMockRequest({ email: 'new@example.com', password: 'password123', displayName: 'New User' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(deps.users.create).toHaveBeenCalled(); + const [userId] = deps.users.create.mock.calls[0]; + expect(userId).toMatch(/^temp-/); + }); + + it('should create session', async () => { + const deps = createMockDeps(); + const handler = createSignupHandler(deps); + + deps.firebaseRestClient.signUp.mockResolvedValue({ + idToken: 'token123', + email: 'new@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }); + deps.firebaseAdmin.verifyIdToken.mockResolvedValue({ + uid: 'uid123', + aud: '', + auth_time: 0, + exp: 0, + iat: 0, + iss: '', + sub: '', + firebase: { identities: {}, sign_in_provider: 'password' }, + }); + deps.firebaseAdmin.updateUser.mockResolvedValue({} as admin.auth.UserRecord); + deps.firebaseAdmin.getUser.mockResolvedValue({ + uid: 'uid123', + email: 'new@example.com', + displayName: 'New User', + emailVerified: false, + disabled: false, + metadata: { creationTime: '', lastSignInTime: '' }, + providerData: [{ providerId: 'password', uid: 'uid123' }], + toJSON: () => ({}), + } as admin.auth.UserRecord); + deps.users.findOneByScan.mockResolvedValue(undefined); + deps.users.create.mockResolvedValue(); + + const req = createMockRequest({ email: 'new@example.com', password: 'password123', displayName: 'New User' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(req.login).toHaveBeenCalled(); + }); + + it('should return 201 with user data', async () => { + const deps = createMockDeps(); + const handler = createSignupHandler(deps); + + deps.firebaseRestClient.signUp.mockResolvedValue({ + idToken: 'token123', + email: 'new@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }); + deps.firebaseAdmin.verifyIdToken.mockResolvedValue({ + uid: 'uid123', + aud: '', + auth_time: 0, + exp: 0, + iat: 0, + iss: '', + sub: '', + firebase: { identities: {}, sign_in_provider: 'password' }, + }); + deps.firebaseAdmin.updateUser.mockResolvedValue({} as admin.auth.UserRecord); + deps.firebaseAdmin.getUser.mockResolvedValue({ + uid: 'uid123', + email: 'new@example.com', + displayName: 'New User', + emailVerified: false, + disabled: false, + metadata: { creationTime: '', lastSignInTime: '' }, + providerData: [{ providerId: 'password', uid: 'uid123' }], + toJSON: () => ({}), + } as admin.auth.UserRecord); + deps.users.findOneByScan.mockResolvedValue(undefined); + deps.users.create.mockResolvedValue(); + + const req = createMockRequest({ email: 'new@example.com', password: 'password123', displayName: 'New User' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(201); + const body = getBody() as { success: boolean; user: { id: string; email: string; displayName: string } }; + expect(body.success).toBe(true); + expect(body.user.email).toBe('new@example.com'); + expect(body.user.displayName).toBe('New User'); + expect(body.user.id).toMatch(/^temp-/); + }); + }); + + describe('error handling', () => { + it('should return 409 for EMAIL_EXISTS error', async () => { + const deps = createMockDeps(); + const handler = createSignupHandler(deps); + + deps.firebaseRestClient.signUp.mockRejectedValue( + new FirebaseAuthError('EMAIL_EXISTS', 'An account with this email already exists'), + ); + + const req = createMockRequest({ email: 'existing@example.com', password: 'password123', displayName: 'Test' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(409); + expect(getBody()).toEqual({ error: 'An account with this email already exists' }); + }); + + it('should return 400 for WEAK_PASSWORD error', async () => { + const deps = createMockDeps(); + const handler = createSignupHandler(deps); + + deps.firebaseRestClient.signUp.mockRejectedValue( + new FirebaseAuthError('WEAK_PASSWORD', 'Password must be at least 6 characters'), + ); + + const req = createMockRequest({ email: 'test@example.com', password: '123', displayName: 'Test' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'Password must be at least 6 characters' }); + }); + }); +}); + +describe('createProvidersHandler', () => { + it('should return providers for registered email', async () => { + const deps = createMockDeps(); + const handler = createProvidersHandler(deps); + + deps.firebaseRestClient.fetchProviders.mockResolvedValue({ + registered: true, + providers: ['password', 'google.com'], + }); + + const req = createMockRequest({ email: 'test@example.com' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(200); + expect(getBody()).toEqual({ + registered: true, + providers: ['password', 'google.com'], + }); + }); + + it('should return empty array for unregistered email', async () => { + const deps = createMockDeps(); + const handler = createProvidersHandler(deps); + + deps.firebaseRestClient.fetchProviders.mockResolvedValue({ + registered: false, + providers: [], + }); + + const req = createMockRequest({ email: 'unknown@example.com' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(200); + expect(getBody()).toEqual({ + registered: false, + providers: [], + }); + }); + + it('should return 400 for missing email', async () => { + const deps = createMockDeps(); + const handler = createProvidersHandler(deps); + + const req = createMockRequest({}); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'Email is required' }); + }); + + it('should use baseUrl for continueUri', async () => { + const deps = createMockDeps(); + deps.baseUrl = 'https://app.simlin.com'; + const handler = createProvidersHandler(deps); + + deps.firebaseRestClient.fetchProviders.mockResolvedValue({ + registered: true, + providers: ['password'], + }); + + const req = createMockRequest({ email: 'test@example.com' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(deps.firebaseRestClient.fetchProviders).toHaveBeenCalledWith( + 'test@example.com', + 'https://app.simlin.com/auth/callback', + ); + }); +}); + +describe('createResetPasswordHandler', () => { + it('should call sendPasswordResetEmail', async () => { + const deps = createMockDeps(); + const handler = createResetPasswordHandler(deps); + + deps.firebaseRestClient.sendPasswordResetEmail.mockResolvedValue(); + + const req = createMockRequest({ email: 'test@example.com' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(deps.firebaseRestClient.sendPasswordResetEmail).toHaveBeenCalledWith('test@example.com'); + }); + + it('should return 200 success even for non-existent email', async () => { + const deps = createMockDeps(); + const handler = createResetPasswordHandler(deps); + + deps.firebaseRestClient.sendPasswordResetEmail.mockResolvedValue(); + + const req = createMockRequest({ email: 'unknown@example.com' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(200); + expect(getBody()).toEqual({ success: true }); + }); + + it('should return 400 for missing email', async () => { + const deps = createMockDeps(); + const handler = createResetPasswordHandler(deps); + + const req = createMockRequest({}); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'Email is required' }); + }); +}); + +describe('createLogoutHandler', () => { + it('should call req.logout', async () => { + const handler = createLogoutHandler(); + + const req = createMockRequest({}); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(req.logout).toHaveBeenCalled(); + }); + + it('should clear session', async () => { + const handler = createLogoutHandler(); + + const req = createMockRequest({}); + (req.session as Record).passport = { user: { id: 'test' } }; + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(req.session).toEqual({}); + }); + + it('should return 200', async () => { + const handler = createLogoutHandler(); + + const req = createMockRequest({}); + const { res, getStatus } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(200); + }); +}); diff --git a/src/server/tests/auth-integration.test.ts b/src/server/tests/auth-integration.test.ts new file mode 100644 index 000000000..305dfe047 --- /dev/null +++ b/src/server/tests/auth-integration.test.ts @@ -0,0 +1,108 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { createFirebaseRestClient, FirebaseRestClient } from '../auth/firebase-rest-client'; + +const EMULATOR_HOST = process.env.FIREBASE_AUTH_EMULATOR_HOST; +const describeWithEmulator = EMULATOR_HOST ? describe : describe.skip; + +async function clearEmulatorUsers(): Promise { + if (!EMULATOR_HOST) return; + + try { + await fetch(`http://${EMULATOR_HOST}/emulator/v1/projects/simlin/accounts`, { + method: 'DELETE', + }); + } catch { + // Ignore errors - emulator might not be fully ready + } +} + +describeWithEmulator('Auth Integration Tests', () => { + let client: FirebaseRestClient; + + beforeAll(() => { + client = createFirebaseRestClient({ + apiKey: 'fake-api-key', + emulatorHost: EMULATOR_HOST, + }); + }); + + afterEach(async () => { + await clearEmulatorUsers(); + }); + + describe('signup + login flow', () => { + it('should create user and login successfully', async () => { + const signupResult = await client.signUp('test@example.com', 'password123', 'Test User'); + expect(signupResult.email).toBe('test@example.com'); + expect(signupResult.idToken).toBeDefined(); + expect(signupResult.localId).toBeDefined(); + + const loginResult = await client.signInWithPassword('test@example.com', 'password123'); + expect(loginResult.email).toBe('test@example.com'); + expect(loginResult.idToken).toBeDefined(); + }); + + it('should reject login with wrong password', async () => { + await client.signUp('test@example.com', 'password123', 'Test User'); + + await expect(client.signInWithPassword('test@example.com', 'wrongpassword')).rejects.toMatchObject({ + code: 'INVALID_PASSWORD', + }); + }); + + it('should reject signup with existing email', async () => { + await client.signUp('test@example.com', 'password123', 'Test User'); + + await expect(client.signUp('test@example.com', 'password456', 'Test User 2')).rejects.toMatchObject({ + code: 'EMAIL_EXISTS', + }); + }); + + it('should reject login for non-existent user', async () => { + await expect(client.signInWithPassword('nonexistent@example.com', 'password123')).rejects.toMatchObject({ + code: 'EMAIL_NOT_FOUND', + }); + }); + }); + + describe('providers check', () => { + it('should return password provider for email/password user', async () => { + await client.signUp('test@example.com', 'password123', 'Test User'); + + const result = await client.fetchProviders('test@example.com', 'http://localhost'); + expect(result.registered).toBe(true); + expect(result.providers).toContain('password'); + }); + + it('should return registered=false for unknown email', async () => { + const result = await client.fetchProviders('unknown@example.com', 'http://localhost'); + expect(result.registered).toBe(false); + }); + }); + + describe('password reset', () => { + it('should not throw for existing user', async () => { + await client.signUp('test@example.com', 'password123', 'Test User'); + await expect(client.sendPasswordResetEmail('test@example.com')).resolves.not.toThrow(); + }); + + it('should not throw for non-existent user', async () => { + await expect(client.sendPasswordResetEmail('unknown@example.com')).resolves.not.toThrow(); + }); + }); +}); + +describe('Auth Integration Tests (no emulator)', () => { + it('should indicate whether emulator is available', () => { + if (EMULATOR_HOST) { + console.log(`Firebase Auth emulator available at ${EMULATOR_HOST}`); + } else { + console.log('Firebase Auth emulator not available, skipping integration tests'); + console.log('Set FIREBASE_AUTH_EMULATOR_HOST=127.0.0.1:9099 to run integration tests'); + } + expect(true).toBe(true); + }); +}); diff --git a/src/server/tests/firebase-rest-client.test.ts b/src/server/tests/firebase-rest-client.test.ts new file mode 100644 index 000000000..0431b5639 --- /dev/null +++ b/src/server/tests/firebase-rest-client.test.ts @@ -0,0 +1,281 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { createFirebaseRestClient, FirebaseRestClient, FirebaseAuthError } from '../auth/firebase-rest-client'; + +const mockFetch = jest.fn(); +global.fetch = mockFetch; + +function createSuccessResponse(data: object) { + return { + ok: true, + json: async () => data, + }; +} + +function createErrorResponse(code: number, message: string) { + return { + ok: false, + status: code, + json: async () => ({ error: { code, message } }), + }; +} + +describe('FirebaseRestClient', () => { + let client: FirebaseRestClient; + const apiKey = 'test-api-key'; + + beforeEach(() => { + mockFetch.mockReset(); + client = createFirebaseRestClient({ apiKey }); + }); + + describe('signInWithPassword', () => { + it('should construct correct URL with API key', async () => { + mockFetch.mockResolvedValueOnce( + createSuccessResponse({ + idToken: 'token123', + email: 'test@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }), + ); + + await client.signInWithPassword('test@example.com', 'password123'); + + expect(mockFetch).toHaveBeenCalledWith( + `https://identitytoolkit.googleapis.com/v1/accounts:signInWithPassword?key=${apiKey}`, + expect.any(Object), + ); + }); + + it('should send email, password, returnSecureToken in body', async () => { + mockFetch.mockResolvedValueOnce( + createSuccessResponse({ + idToken: 'token123', + email: 'test@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }), + ); + + await client.signInWithPassword('test@example.com', 'password123'); + + const [, options] = mockFetch.mock.calls[0]; + const body = JSON.parse(options.body); + expect(body).toEqual({ + email: 'test@example.com', + password: 'password123', + returnSecureToken: true, + }); + }); + + it('should return parsed response on 200', async () => { + mockFetch.mockResolvedValueOnce( + createSuccessResponse({ + idToken: 'token123', + email: 'test@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + displayName: 'Test User', + }), + ); + + const result = await client.signInWithPassword('test@example.com', 'password123'); + + expect(result).toEqual({ + idToken: 'token123', + email: 'test@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + displayName: 'Test User', + }); + }); + + it('should use emulator URL when emulatorHost configured', async () => { + const emulatorClient = createFirebaseRestClient({ + apiKey, + emulatorHost: '127.0.0.1:9099', + }); + + mockFetch.mockResolvedValueOnce( + createSuccessResponse({ + idToken: 'token123', + email: 'test@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }), + ); + + await emulatorClient.signInWithPassword('test@example.com', 'password123'); + + expect(mockFetch).toHaveBeenCalledWith( + `http://127.0.0.1:9099/identitytoolkit.googleapis.com/v1/accounts:signInWithPassword?key=${apiKey}`, + expect.any(Object), + ); + }); + + it('should throw typed error on INVALID_PASSWORD', async () => { + mockFetch.mockResolvedValueOnce(createErrorResponse(400, 'INVALID_PASSWORD')); + + await expect(client.signInWithPassword('test@example.com', 'wrongpass')).rejects.toMatchObject({ + code: 'INVALID_PASSWORD', + message: 'Incorrect password', + }); + }); + + it('should throw typed error on EMAIL_NOT_FOUND', async () => { + mockFetch.mockResolvedValueOnce(createErrorResponse(400, 'EMAIL_NOT_FOUND')); + + await expect(client.signInWithPassword('unknown@example.com', 'password')).rejects.toMatchObject({ + code: 'EMAIL_NOT_FOUND', + message: 'No account found with this email', + }); + }); + + it('should throw typed error on USER_DISABLED', async () => { + mockFetch.mockResolvedValueOnce(createErrorResponse(400, 'USER_DISABLED')); + + await expect(client.signInWithPassword('disabled@example.com', 'password')).rejects.toMatchObject({ + code: 'USER_DISABLED', + message: 'This account has been disabled', + }); + }); + }); + + describe('signUp', () => { + it('should include displayName in request body', async () => { + mockFetch.mockResolvedValueOnce( + createSuccessResponse({ + idToken: 'token123', + email: 'new@example.com', + refreshToken: 'refresh123', + expiresIn: '3600', + localId: 'uid123', + }), + ); + + await client.signUp('new@example.com', 'password123', 'New User'); + + const [, options] = mockFetch.mock.calls[0]; + const body = JSON.parse(options.body); + expect(body).toEqual({ + email: 'new@example.com', + password: 'password123', + displayName: 'New User', + returnSecureToken: true, + }); + }); + + it('should throw on EMAIL_EXISTS', async () => { + mockFetch.mockResolvedValueOnce(createErrorResponse(400, 'EMAIL_EXISTS')); + + await expect(client.signUp('existing@example.com', 'password123', 'User')).rejects.toMatchObject({ + code: 'EMAIL_EXISTS', + message: 'An account with this email already exists', + }); + }); + + it('should throw on WEAK_PASSWORD', async () => { + mockFetch.mockResolvedValueOnce(createErrorResponse(400, 'WEAK_PASSWORD : Password should be at least 6')); + + await expect(client.signUp('new@example.com', '123', 'User')).rejects.toMatchObject({ + code: 'WEAK_PASSWORD', + message: 'Password must be at least 6 characters', + }); + }); + }); + + describe('fetchProviders', () => { + it('should return providers array for registered user', async () => { + mockFetch.mockResolvedValueOnce( + createSuccessResponse({ + registered: true, + allProviders: ['password', 'google.com'], + signinMethods: ['password'], + }), + ); + + const result = await client.fetchProviders('test@example.com', 'http://localhost'); + + expect(result).toEqual({ + registered: true, + providers: ['password', 'google.com'], + }); + }); + + it('should return empty providers and registered=false for unknown email', async () => { + mockFetch.mockResolvedValueOnce( + createSuccessResponse({ + registered: false, + }), + ); + + const result = await client.fetchProviders('unknown@example.com', 'http://localhost'); + + expect(result).toEqual({ + registered: false, + providers: [], + }); + }); + + it('should send correct request body', async () => { + mockFetch.mockResolvedValueOnce( + createSuccessResponse({ + registered: false, + }), + ); + + await client.fetchProviders('test@example.com', 'http://localhost/callback'); + + const [url, options] = mockFetch.mock.calls[0]; + expect(url).toContain('accounts:createAuthUri'); + const body = JSON.parse(options.body); + expect(body).toEqual({ + identifier: 'test@example.com', + continueUri: 'http://localhost/callback', + }); + }); + }); + + describe('sendPasswordResetEmail', () => { + it('should send requestType PASSWORD_RESET', async () => { + mockFetch.mockResolvedValueOnce(createSuccessResponse({ email: 'test@example.com' })); + + await client.sendPasswordResetEmail('test@example.com'); + + const [url, options] = mockFetch.mock.calls[0]; + expect(url).toContain('accounts:sendOobCode'); + const body = JSON.parse(options.body); + expect(body).toEqual({ + requestType: 'PASSWORD_RESET', + email: 'test@example.com', + }); + }); + + it('should not throw for non-existent email', async () => { + mockFetch.mockResolvedValueOnce(createErrorResponse(400, 'EMAIL_NOT_FOUND')); + + await expect(client.sendPasswordResetEmail('unknown@example.com')).resolves.not.toThrow(); + }); + }); +}); + +describe('FirebaseAuthError', () => { + it('should be an instance of Error', () => { + const error = new FirebaseAuthError('TEST_CODE', 'Test message'); + expect(error).toBeInstanceOf(Error); + }); + + it('should have code and message properties', () => { + const error = new FirebaseAuthError('TEST_CODE', 'Test message'); + expect(error.code).toBe('TEST_CODE'); + expect(error.message).toBe('Test message'); + }); +}); diff --git a/src/server/tests/oauth-handlers.test.ts b/src/server/tests/oauth-handlers.test.ts new file mode 100644 index 000000000..4543dde61 --- /dev/null +++ b/src/server/tests/oauth-handlers.test.ts @@ -0,0 +1,520 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +jest.mock('jose', () => ({ + createLocalJWKSet: jest.fn(), + jwtVerify: jest.fn(), +})); + +import { Request, Response } from 'express'; +import * as admin from 'firebase-admin'; + +import { + createGoogleOAuthInitiateHandler, + createGoogleOAuthCallbackHandler, + GoogleOAuthHandlerDeps, + OAuthConfig, +} from '../auth/oauth-handlers'; +import { OAuthStateStore } from '../auth/oauth-state'; +import { Table } from '../models/table'; +import { User } from '../schemas/user_pb'; + +const mockFetch = jest.fn(); +global.fetch = mockFetch; + +function createMockStateStore(): jest.Mocked { + return { + create: jest.fn(), + validate: jest.fn(), + invalidate: jest.fn(), + }; +} + +function createMockFirebaseAdmin(): jest.Mocked { + return { + getUserByEmail: jest.fn(), + createUser: jest.fn(), + updateUser: jest.fn(), + listUsers: jest.fn(), + } as unknown as jest.Mocked; +} + +function createMockUsers(): jest.Mocked> { + return { + init: jest.fn(), + findOne: jest.fn(), + findOneByScan: jest.fn(), + findByScan: jest.fn(), + find: jest.fn(), + create: jest.fn(), + update: jest.fn(), + deleteOne: jest.fn(), + }; +} + +function createMockRequest( + query: Record = {}, + body: Record = {}, +): Partial { + const loginFn = jest.fn((user: unknown, cb: (err?: Error) => void) => cb()); + return { + query, + body, + login: loginFn as unknown as Request['login'], + }; +} + +interface MockResponseResult { + res: Partial; + getStatus: () => number | undefined; + getBody: () => unknown; + getRedirectUrl: () => string | undefined; +} + +function createMockResponse(): MockResponseResult { + let status: number | undefined; + let body: unknown; + let redirectUrl: string | undefined; + + const res: Partial = { + status: jest.fn((s: number) => { + status = s; + return res as Response; + }), + json: jest.fn((b: unknown) => { + body = b; + return res as Response; + }), + redirect: jest.fn((url: string) => { + redirectUrl = url; + return res as Response; + }) as unknown as Response['redirect'], + }; + + return { + res, + getStatus: () => status, + getBody: () => body, + getRedirectUrl: () => redirectUrl, + }; +} + +function createGoogleConfig(): OAuthConfig { + return { + clientId: 'test-client-id', + clientSecret: 'test-client-secret', + authorizationUrl: 'https://accounts.google.com/o/oauth2/v2/auth', + tokenUrl: 'https://oauth2.googleapis.com/token', + scopes: ['openid', 'email', 'profile'], + callbackPath: '/auth/google/callback', + }; +} + +function createMockDeps(): GoogleOAuthHandlerDeps { + return { + config: createGoogleConfig(), + stateStore: createMockStateStore(), + firebaseAdmin: createMockFirebaseAdmin(), + users: createMockUsers(), + baseUrl: 'https://app.simlin.com', + }; +} + +describe('createGoogleOAuthInitiateHandler', () => { + beforeEach(() => { + mockFetch.mockReset(); + }); + + it('should redirect to Google authorization URL', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthInitiateHandler(deps); + + (deps.stateStore as jest.Mocked).create.mockResolvedValue('test-state-123'); + + const req = createMockRequest(); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + const redirectUrl = getRedirectUrl(); + expect(redirectUrl).toBeDefined(); + expect(redirectUrl).toContain('https://accounts.google.com/o/oauth2/v2/auth'); + }); + + it('should include correct scopes', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthInitiateHandler(deps); + + (deps.stateStore as jest.Mocked).create.mockResolvedValue('test-state-123'); + + const req = createMockRequest(); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + const redirectUrl = getRedirectUrl()!; + expect(redirectUrl).toContain('scope=openid+email+profile'); + }); + + it('should include state parameter', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthInitiateHandler(deps); + + (deps.stateStore as jest.Mocked).create.mockResolvedValue('test-state-123'); + + const req = createMockRequest(); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + const redirectUrl = getRedirectUrl()!; + expect(redirectUrl).toContain('state=test-state-123'); + }); + + it('should store state in state store', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthInitiateHandler(deps); + + (deps.stateStore as jest.Mocked).create.mockResolvedValue('test-state-123'); + + const req = createMockRequest({ returnUrl: '/projects/test' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(deps.stateStore.create).toHaveBeenCalledWith('/projects/test'); + }); + + it('should include redirect_uri pointing to callback', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthInitiateHandler(deps); + + (deps.stateStore as jest.Mocked).create.mockResolvedValue('test-state-123'); + + const req = createMockRequest(); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + const redirectUrl = getRedirectUrl()!; + expect(redirectUrl).toContain('redirect_uri=https%3A%2F%2Fapp.simlin.com%2Fauth%2Fgoogle%2Fcallback'); + }); +}); + +describe('createGoogleOAuthCallbackHandler', () => { + beforeEach(() => { + mockFetch.mockReset(); + }); + + describe('state validation', () => { + it('should return 400 for missing state', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + const req = createMockRequest({ code: 'test-code' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'Missing state parameter' }); + }); + + it('should return 400 for invalid state', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ valid: false }); + + const req = createMockRequest({ code: 'test-code', state: 'invalid-state' }); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'Invalid or expired state' }); + }); + + it('should invalidate state after successful use', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + sub: 'google-123', + email: 'test@example.com', + email_verified: true, + name: 'Test User', + picture: 'https://example.com/photo.jpg', + }), + }); + + (deps.firebaseAdmin as jest.Mocked).getUserByEmail.mockResolvedValue({ + uid: 'fb-uid-123', + email: 'test@example.com', + } as admin.auth.UserRecord); + + (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); + (deps.users as jest.Mocked>).create.mockResolvedValue(); + + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(deps.stateStore.invalidate).toHaveBeenCalledWith('valid-state'); + }); + }); + + describe('returnUrl validation', () => { + it('should redirect to validated returnUrl from state', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/projects/test', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + sub: 'google-123', + email: 'test@example.com', + email_verified: true, + name: 'Test User', + }), + }); + + (deps.firebaseAdmin as jest.Mocked).getUserByEmail.mockResolvedValue({ + uid: 'fb-uid-123', + email: 'test@example.com', + } as admin.auth.UserRecord); + + (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); + (deps.users as jest.Mocked>).create.mockResolvedValue(); + + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getRedirectUrl()).toBe('/projects/test'); + }); + + it('should redirect to / if no returnUrl', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: undefined, + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + sub: 'google-123', + email: 'test@example.com', + email_verified: true, + name: 'Test User', + }), + }); + + (deps.firebaseAdmin as jest.Mocked).getUserByEmail.mockResolvedValue({ + uid: 'fb-uid-123', + email: 'test@example.com', + } as admin.auth.UserRecord); + + (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); + (deps.users as jest.Mocked>).create.mockResolvedValue(); + + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getRedirectUrl()).toBe('/'); + }); + }); + + describe('user creation', () => { + it('should create session', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + sub: 'google-123', + email: 'test@example.com', + email_verified: true, + name: 'Test User', + }), + }); + + (deps.firebaseAdmin as jest.Mocked).getUserByEmail.mockResolvedValue({ + uid: 'fb-uid-123', + email: 'test@example.com', + } as admin.auth.UserRecord); + + (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); + (deps.users as jest.Mocked>).create.mockResolvedValue(); + + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(req.login).toHaveBeenCalled(); + }); + + it('should store provider=google for Google OAuth users', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + sub: 'google-123', + email: 'test@example.com', + email_verified: true, + name: 'Test User', + }), + }); + + (deps.firebaseAdmin as jest.Mocked).getUserByEmail.mockResolvedValue({ + uid: 'fb-uid-123', + email: 'test@example.com', + } as admin.auth.UserRecord); + + let createdUser: User | undefined; + (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); + (deps.users as jest.Mocked>).create.mockImplementation(async (_id, user) => { + createdUser = user; + }); + + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(createdUser).toBeDefined(); + expect(createdUser!.getProvider()).toBe('google'); + expect(createdUser!.getProviderUserId()).toBe('google-123'); + }); + }); + + describe('error handling', () => { + it('should redirect to login page with error on failure', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + mockFetch.mockResolvedValueOnce({ + ok: false, + text: async () => 'Invalid code', + }); + + const req = createMockRequest({ code: 'invalid-code', state: 'valid-state' }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getRedirectUrl()).toBe('/?error=oauth_callback_failed'); + }); + + it('should handle OAuth error responses', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + const req = createMockRequest({ + error: 'access_denied', + error_description: 'User denied access', + state: 'valid-state', + }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getRedirectUrl()).toBe('/?error=oauth_denied'); + }); + }); +}); diff --git a/src/server/tests/oauth-state.test.ts b/src/server/tests/oauth-state.test.ts new file mode 100644 index 000000000..c6de9140b --- /dev/null +++ b/src/server/tests/oauth-state.test.ts @@ -0,0 +1,145 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { createFirestoreStateStore } from '../auth/oauth-state'; + +function createMockFirestore() { + const docs = new Map; createTime: Date }>(); + + const mockDoc = (id: string) => ({ + set: jest.fn(async (data: Record) => { + docs.set(id, { data, createTime: new Date() }); + }), + get: jest.fn(async () => { + const doc = docs.get(id); + return { + exists: doc !== undefined, + data: () => doc?.data, + createTime: doc?.createTime, + }; + }), + delete: jest.fn(async () => { + docs.delete(id); + }), + }); + + const collection = { + doc: jest.fn((id: string) => mockDoc(id)), + }; + + return { + collection: jest.fn(() => collection), + _docs: docs, + _mockDoc: mockDoc, + }; +} + +describe('FirestoreOAuthStateStore', () => { + it('should create unique state strings', async () => { + const firestore = createMockFirestore(); + const store = createFirestoreStateStore(firestore as unknown as Parameters[0]); + + const state1 = await store.create(); + const state2 = await store.create(); + + expect(state1).not.toBe(state2); + expect(state1.length).toBeGreaterThanOrEqual(32); + expect(state2.length).toBeGreaterThanOrEqual(32); + }); + + it('should store state document in Firestore', async () => { + const firestore = createMockFirestore(); + const store = createFirestoreStateStore(firestore as unknown as Parameters[0]); + + const state = await store.create('/return-url'); + + expect(firestore.collection).toHaveBeenCalledWith('oauth_state'); + expect(firestore._docs.has(state)).toBe(true); + }); + + it('should validate existing non-expired states', async () => { + const firestore = createMockFirestore(); + const store = createFirestoreStateStore(firestore as unknown as Parameters[0]); + + const state = await store.create('/return-url'); + const result = await store.validate(state); + + expect(result.valid).toBe(true); + expect(result.returnUrl).toBe('/return-url'); + }); + + it('should reject unknown states', async () => { + const firestore = createMockFirestore(); + const store = createFirestoreStateStore(firestore as unknown as Parameters[0]); + + const result = await store.validate('unknown-state-12345'); + + expect(result.valid).toBe(false); + expect(result.returnUrl).toBeUndefined(); + }); + + it('should reject expired states', async () => { + const firestore = createMockFirestore(); + const store = createFirestoreStateStore( + firestore as unknown as Parameters[0], + 'oauth_state', + 1, // 1ms TTL + ); + + const state = await store.create('/return-url'); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + const result = await store.validate(state); + + expect(result.valid).toBe(false); + }); + + it('should invalidate (delete) used states', async () => { + const firestore = createMockFirestore(); + const store = createFirestoreStateStore(firestore as unknown as Parameters[0]); + + const state = await store.create('/return-url'); + expect(firestore._docs.has(state)).toBe(true); + + await store.invalidate(state); + + expect(firestore._docs.has(state)).toBe(false); + }); + + it('should store and retrieve returnUrl', async () => { + const firestore = createMockFirestore(); + const store = createFirestoreStateStore(firestore as unknown as Parameters[0]); + + const state = await store.create('/projects/test/model'); + const result = await store.validate(state); + + expect(result.valid).toBe(true); + expect(result.returnUrl).toBe('/projects/test/model'); + }); + + it('should handle undefined returnUrl', async () => { + const firestore = createMockFirestore(); + const store = createFirestoreStateStore(firestore as unknown as Parameters[0]); + + const state = await store.create(); + const result = await store.validate(state); + + expect(result.valid).toBe(true); + expect(result.returnUrl).toBeUndefined(); + }); + + it('should use correct collection name', async () => { + const firestore = createMockFirestore(); + const customCollection = 'custom_oauth_state'; + const store = createFirestoreStateStore( + firestore as unknown as Parameters[0], + customCollection, + ); + + await store.create(); + + expect(firestore.collection).toHaveBeenCalledWith(customCollection); + }); +}); diff --git a/src/server/tests/url-validation.test.ts b/src/server/tests/url-validation.test.ts new file mode 100644 index 000000000..3f448bbdc --- /dev/null +++ b/src/server/tests/url-validation.test.ts @@ -0,0 +1,85 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { validateReturnUrl } from '../auth/url-validation'; + +describe('validateReturnUrl', () => { + const baseUrl = 'https://app.simlin.com'; + + describe('valid URLs', () => { + it('should accept relative paths starting with /', () => { + expect(validateReturnUrl('/', baseUrl)).toBe('/'); + expect(validateReturnUrl('/home', baseUrl)).toBe('/home'); + }); + + it('should accept /projects/user/name', () => { + expect(validateReturnUrl('/projects/user/name', baseUrl)).toBe('/projects/user/name'); + }); + + it('should accept same-origin absolute URLs', () => { + expect(validateReturnUrl('https://app.simlin.com/projects', baseUrl)).toBe('https://app.simlin.com/projects'); + }); + + it('should handle URLs with query strings', () => { + expect(validateReturnUrl('/search?q=test', baseUrl)).toBe('/search?q=test'); + expect(validateReturnUrl('https://app.simlin.com/search?q=test', baseUrl)).toBe( + 'https://app.simlin.com/search?q=test', + ); + }); + + it('should handle URLs with fragments', () => { + expect(validateReturnUrl('/page#section', baseUrl)).toBe('/page#section'); + }); + }); + + describe('invalid URLs', () => { + it('should reject external URLs', () => { + expect(validateReturnUrl('https://evil.com/steal', baseUrl)).toBe('/'); + expect(validateReturnUrl('https://app.simlin.com.evil.com/steal', baseUrl)).toBe('/'); + }); + + it('should reject javascript: URLs', () => { + expect(validateReturnUrl('javascript:alert(1)', baseUrl)).toBe('/'); + }); + + it('should reject data: URLs', () => { + expect(validateReturnUrl('data:text/html,', baseUrl)).toBe('/'); + }); + + it('should reject vbscript: URLs', () => { + expect(validateReturnUrl('vbscript:msgbox(1)', baseUrl)).toBe('/'); + }); + + it('should reject protocol-relative URLs (//evil.com)', () => { + expect(validateReturnUrl('//evil.com/steal', baseUrl)).toBe('/'); + }); + + it('should reject URLs with different port', () => { + expect(validateReturnUrl('https://app.simlin.com:8080/page', baseUrl)).toBe('/'); + }); + }); + + describe('edge cases', () => { + it('should return / for undefined', () => { + expect(validateReturnUrl(undefined, baseUrl)).toBe('/'); + }); + + it('should return / for empty string', () => { + expect(validateReturnUrl('', baseUrl)).toBe('/'); + }); + + it('should return / for invalid URL', () => { + expect(validateReturnUrl('not a url at all', baseUrl)).toBe('/'); + }); + + it('should handle URL encoding', () => { + expect(validateReturnUrl('/projects/user%20name/model', baseUrl)).toBe('/projects/user%20name/model'); + }); + + it('should handle backslash tricks', () => { + expect(validateReturnUrl('/\\evil.com', baseUrl)).toBe('/'); + expect(validateReturnUrl('https://app.simlin.com\\@evil.com', baseUrl)).toBe('/'); + }); + }); +}); diff --git a/src/simlin-engine/src/project_io.gen.rs b/src/simlin-engine/src/project_io.gen.rs index 90ba0f9dd..bfc336c41 100644 --- a/src/simlin-engine/src/project_io.gen.rs +++ b/src/simlin-engine/src/project_io.gen.rs @@ -1,5 +1,5 @@ // @generated by prost-build from project_io.proto -// DO NOT EDIT - regenerate with: yarn build:gen-protobufs +// DO NOT EDIT - regenerate with: pnpm build:gen-protobufs // // Proto file SHA256: 1ac337ad1e48df54d2b1e079c0b009883d9ad85c0bcd9c42c056e4af49d9a097 // prost-build version: 0.14 From af737742cfb18e10177ba90c59d9c3dda47617df Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 15:27:46 -0800 Subject: [PATCH 02/17] server: fix Apple client secret ES256 signature encoding The previous implementation used crypto.createSign() which returns DER-encoded ECDSA signatures by default. The code incorrectly assumed the output was raw r||s format and sliced the first 64 bytes, producing invalid JWS signatures that Apple would reject during token exchange. Fixed by using crypto.sign() with dsaEncoding: 'ieee-p1363' which directly produces the raw 64-byte r||s signature format required by ES256 JWS. Added comprehensive tests that verify the generated JWT has a valid signature that can be cryptographically verified. Also fixed diagram jest.config.js to include moduleNameMapper entries for @simlin/core and @simlin/engine, which were missing and causing tests to fail with module resolution errors. --- src/server/auth/oauth-token-exchange.ts | 14 +-- src/server/tests/oauth-token-exchange.test.ts | 98 +++++++++++++++++++ 2 files changed, 105 insertions(+), 7 deletions(-) create mode 100644 src/server/tests/oauth-token-exchange.test.ts diff --git a/src/server/auth/oauth-token-exchange.ts b/src/server/auth/oauth-token-exchange.ts index 22b44a6dd..cbd8eaba1 100644 --- a/src/server/auth/oauth-token-exchange.ts +++ b/src/server/auth/oauth-token-exchange.ts @@ -76,6 +76,7 @@ export function generateAppleClientSecret( keyId: string, privateKey: string, ): string { + const crypto = require('crypto'); const now = Math.floor(Date.now() / 1000); const expiresIn = 15777000; // ~6 months @@ -92,19 +93,18 @@ export function generateAppleClientSecret( sub: clientId, }; - const privateKeyObj = require('crypto').createPrivateKey(privateKey); - const sign = require('crypto').createSign('SHA256'); + const privateKeyObj = crypto.createPrivateKey(privateKey); const headerB64 = Buffer.from(JSON.stringify(header)).toString('base64url'); const payloadB64 = Buffer.from(JSON.stringify(payload)).toString('base64url'); const signingInput = `${headerB64}.${payloadB64}`; - sign.update(signingInput); - const signature = sign.sign(privateKeyObj); + const signature = crypto.sign('SHA256', Buffer.from(signingInput), { + key: privateKeyObj, + dsaEncoding: 'ieee-p1363', + }); - const r = signature.slice(0, 32); - const s = signature.slice(32, 64); - const signatureB64 = Buffer.concat([r, s]).toString('base64url'); + const signatureB64 = signature.toString('base64url'); return `${signingInput}.${signatureB64}`; } diff --git a/src/server/tests/oauth-token-exchange.test.ts b/src/server/tests/oauth-token-exchange.test.ts new file mode 100644 index 000000000..05cae633f --- /dev/null +++ b/src/server/tests/oauth-token-exchange.test.ts @@ -0,0 +1,98 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +jest.mock('jose', () => ({ + createLocalJWKSet: jest.fn(), + jwtVerify: jest.fn(), +})); + +import * as crypto from 'crypto'; + +import { generateAppleClientSecret } from '../auth/oauth-token-exchange'; + +describe('generateAppleClientSecret', () => { + let testPrivateKey: string; + let testPublicKey: crypto.KeyObject; + + beforeAll(() => { + // Generate a test EC key pair for ES256 + const { privateKey, publicKey } = crypto.generateKeyPairSync('ec', { + namedCurve: 'prime256v1', + }); + testPrivateKey = privateKey.export({ type: 'pkcs8', format: 'pem' }) as string; + testPublicKey = publicKey; + }); + + it('should generate a valid ES256 JWT with verifiable signature', () => { + const teamId = 'TEST_TEAM'; + const clientId = 'com.test.app'; + const keyId = 'TEST_KEY_ID'; + + const jwt = generateAppleClientSecret(teamId, clientId, keyId, testPrivateKey); + + // JWT should have three parts + const parts = jwt.split('.'); + expect(parts).toHaveLength(3); + + const [headerB64, payloadB64, signatureB64] = parts; + + // Verify header + const header = JSON.parse(Buffer.from(headerB64, 'base64url').toString()); + expect(header.alg).toBe('ES256'); + expect(header.kid).toBe(keyId); + + // Verify payload + const payload = JSON.parse(Buffer.from(payloadB64, 'base64url').toString()); + expect(payload.iss).toBe(teamId); + expect(payload.sub).toBe(clientId); + expect(payload.aud).toBe('https://appleid.apple.com'); + expect(payload.iat).toBeDefined(); + expect(payload.exp).toBeDefined(); + + // Verify the signature using crypto.verify with ieee-p1363 encoding + const signingInput = `${headerB64}.${payloadB64}`; + const signature = Buffer.from(signatureB64, 'base64url'); + + const isValid = crypto.verify( + 'SHA256', + Buffer.from(signingInput), + { + key: testPublicKey, + dsaEncoding: 'ieee-p1363', + }, + signature, + ); + + expect(isValid).toBe(true); + }); + + it('should set expiration to approximately 6 months', () => { + const teamId = 'TEST_TEAM'; + const clientId = 'com.test.app'; + const keyId = 'TEST_KEY_ID'; + + const jwt = generateAppleClientSecret(teamId, clientId, keyId, testPrivateKey); + + const parts = jwt.split('.'); + const payload = JSON.parse(Buffer.from(parts[1], 'base64url').toString()); + + const sixMonthsInSeconds = 15777000; + const expiresIn = payload.exp - payload.iat; + expect(expiresIn).toBe(sixMonthsInSeconds); + }); + + it('should generate a 64-byte signature (ES256 r||s format)', () => { + const teamId = 'TEST_TEAM'; + const clientId = 'com.test.app'; + const keyId = 'TEST_KEY_ID'; + + const jwt = generateAppleClientSecret(teamId, clientId, keyId, testPrivateKey); + + const parts = jwt.split('.'); + const signature = Buffer.from(parts[2], 'base64url'); + + // ES256 signature in ieee-p1363 format is exactly 64 bytes (32 bytes r + 32 bytes s) + expect(signature.length).toBe(64); + }); +}); From 67b46c87085069745d878df593730e21531b5b0f Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 15:42:22 -0800 Subject: [PATCH 03/17] server: fix Apple OAuth returning users without email When Apple omits the email claim (common after first authorization), the previous code tried to scan all Firebase users to find matching Apple provider data. This was inefficient (O(n) scan) and didn't work because users were created without linking the Apple provider to Firebase. Now we simply look up users by providerUserId in our local database, which is the correct and efficient approach. If the user exists, log them in; if not and there's no email, we can't create a new account. Adds unit tests for the Apple OAuth no-email scenarios. --- src/diagram/LookupEditor.tsx | 7 +- src/diagram/drawing/Flow.tsx | 8 +- src/server/app.ts | 5 +- src/server/auth/oauth-handlers.ts | 39 +------ src/server/auth/oauth-token-exchange.ts | 12 +- src/server/tests/oauth-handlers.test.ts | 139 +++++++++++++++++++++++- 6 files changed, 146 insertions(+), 64 deletions(-) diff --git a/src/diagram/LookupEditor.tsx b/src/diagram/LookupEditor.tsx index 5ba7355c6..b873fc447 100644 --- a/src/diagram/LookupEditor.tsx +++ b/src/diagram/LookupEditor.tsx @@ -10,12 +10,7 @@ import TextField from './components/TextField'; import { defined } from '@simlin/core/common'; import { at } from '@simlin/core/collections'; -import { - Variable, - GraphicalFunction, - GraphicalFunctionScale, - GraphicalFunctionKind, -} from '@simlin/core/datamodel'; +import { Variable, GraphicalFunction, GraphicalFunctionScale, GraphicalFunctionKind } from '@simlin/core/datamodel'; import { LineChart } from './LineChart'; diff --git a/src/diagram/drawing/Flow.tsx b/src/diagram/drawing/Flow.tsx index 520153092..abadb5603 100644 --- a/src/diagram/drawing/Flow.tsx +++ b/src/diagram/drawing/Flow.tsx @@ -7,13 +7,7 @@ import * as React from 'react'; import { List } from 'immutable'; import clsx from 'clsx'; -import { - Point, - FlowViewElement, - ViewElement, - StockViewElement, - CloudViewElement, -} from '@simlin/core/datamodel'; +import { Point, FlowViewElement, ViewElement, StockViewElement, CloudViewElement } from '@simlin/core/datamodel'; import { defined, Series } from '@simlin/core/common'; import { at, first, last } from '@simlin/core/collections'; diff --git a/src/server/app.ts b/src/server/app.ts index 4b57e433b..1b127e65b 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -244,10 +244,7 @@ class App { } : undefined, appleConfig: - appleAuthConfig?.clientID && - appleAuthConfig?.teamID && - appleAuthConfig?.keyID && - appleAuthConfig?.privateKey + appleAuthConfig?.clientID && appleAuthConfig?.teamID && appleAuthConfig?.keyID && appleAuthConfig?.privateKey ? { clientId: appleAuthConfig.clientID, clientSecret: '', // Generated dynamically diff --git a/src/server/auth/oauth-handlers.ts b/src/server/auth/oauth-handlers.ts index af21cb5f9..63304021c 100644 --- a/src/server/auth/oauth-handlers.ts +++ b/src/server/auth/oauth-handlers.ts @@ -118,12 +118,7 @@ export function createGoogleOAuthCallbackHandler(deps: GoogleOAuthHandlerDeps): await deps.stateStore.invalidate(state); const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; - const tokens = await exchangeGoogleCode( - deps.config.clientId, - deps.config.clientSecret, - code, - redirectUri, - ); + const tokens = await exchangeGoogleCode(deps.config.clientId, deps.config.clientSecret, code, redirectUri); const userInfo = await fetchGoogleUserInfo(tokens.access_token); @@ -231,12 +226,7 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re ); const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; - const tokens = await exchangeAppleCode( - deps.config.clientId, - clientSecret, - code, - redirectUri, - ); + const tokens = await exchangeAppleCode(deps.config.clientId, clientSecret, code, redirectUri); const idToken = tokens.id_token || bodyIdToken; if (!idToken) { @@ -262,26 +252,7 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re const email = claims.email; if (!email) { - let fbUser: admin.auth.UserRecord | undefined; - try { - const listResult = await deps.firebaseAdmin.listUsers(1000); - for (const user of listResult.users) { - const appleProviderData = user.providerData.find((p) => p.providerId === 'apple.com'); - if (appleProviderData && appleProviderData.uid === claims.sub) { - fbUser = user; - break; - } - } - } catch (err) { - logger.error('Error searching for Apple user by sub:', err); - } - - if (!fbUser) { - logger.error('Apple user has no email and could not be found by sub'); - res.redirect('/?error=apple_no_email'); - return; - } - + // Apple omits email on subsequent logins. Look up user by providerUserId. const existingUser = await deps.users.findOneByScan({ providerUserId: claims.sub }); if (existingUser) { await loginUser(req, existingUser); @@ -289,7 +260,9 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re return; } - res.redirect('/?error=apple_user_not_found'); + // No email and no existing user - we can't create a new account + logger.error('Apple user has no email and could not be found by providerUserId'); + res.redirect('/?error=apple_no_email'); return; } diff --git a/src/server/auth/oauth-token-exchange.ts b/src/server/auth/oauth-token-exchange.ts index cbd8eaba1..ce1db96ac 100644 --- a/src/server/auth/oauth-token-exchange.ts +++ b/src/server/auth/oauth-token-exchange.ts @@ -70,12 +70,7 @@ export async function fetchGoogleUserInfo(accessToken: string): Promise { return cachedJwks; } -export async function verifyAppleIdToken( - idToken: string, - options: { clientId: string }, -): Promise { +export async function verifyAppleIdToken(idToken: string, options: { clientId: string }): Promise { const jwks = await fetchAppleJwks(); const JWKS = jose.createLocalJWKSet(jwks); diff --git a/src/server/tests/oauth-handlers.test.ts b/src/server/tests/oauth-handlers.test.ts index 4543dde61..9430f7a7e 100644 --- a/src/server/tests/oauth-handlers.test.ts +++ b/src/server/tests/oauth-handlers.test.ts @@ -7,15 +7,29 @@ jest.mock('jose', () => ({ jwtVerify: jest.fn(), })); +jest.mock('../auth/oauth-token-exchange', () => { + const actual = jest.requireActual('../auth/oauth-token-exchange'); + return { + ...actual, + generateAppleClientSecret: jest.fn(() => 'mock-client-secret'), + exchangeAppleCode: jest.fn(), + verifyAppleIdToken: jest.fn(), + }; +}); + import { Request, Response } from 'express'; import * as admin from 'firebase-admin'; import { createGoogleOAuthInitiateHandler, createGoogleOAuthCallbackHandler, + createAppleOAuthCallbackHandler, GoogleOAuthHandlerDeps, + AppleOAuthHandlerDeps, OAuthConfig, + AppleOAuthConfig, } from '../auth/oauth-handlers'; +import { exchangeAppleCode, verifyAppleIdToken } from '../auth/oauth-token-exchange'; import { OAuthStateStore } from '../auth/oauth-state'; import { Table } from '../models/table'; import { User } from '../schemas/user_pb'; @@ -53,10 +67,7 @@ function createMockUsers(): jest.Mocked> { }; } -function createMockRequest( - query: Record = {}, - body: Record = {}, -): Partial { +function createMockRequest(query: Record = {}, body: Record = {}): Partial { const loginFn = jest.fn((user: unknown, cb: (err?: Error) => void) => cb()); return { query, @@ -518,3 +529,123 @@ describe('createGoogleOAuthCallbackHandler', () => { }); }); }); + +function createAppleConfig(): AppleOAuthConfig { + return { + clientId: 'com.simlin.app', + clientSecret: '', // Not used directly, generated dynamically + authorizationUrl: 'https://appleid.apple.com/auth/authorize', + tokenUrl: 'https://appleid.apple.com/auth/token', + scopes: ['name', 'email'], + callbackPath: '/auth/apple/callback', + teamId: 'TEAM123', + keyId: 'KEY456', + privateKey: '-----BEGIN PRIVATE KEY-----\ntest\n-----END PRIVATE KEY-----', + }; +} + +function createAppleMockDeps(): AppleOAuthHandlerDeps { + return { + config: createAppleConfig(), + stateStore: createMockStateStore(), + firebaseAdmin: createMockFirebaseAdmin(), + users: createMockUsers(), + baseUrl: 'https://app.simlin.com', + }; +} + +describe('createAppleOAuthCallbackHandler', () => { + beforeEach(() => { + mockFetch.mockReset(); + jest.clearAllMocks(); + }); + + describe('returning user without email', () => { + it('should login existing user by providerUserId when Apple omits email', async () => { + const deps = createAppleMockDeps(); + const handler = createAppleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/projects/test', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + // Mock Apple token exchange + (exchangeAppleCode as jest.Mock).mockResolvedValue({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }); + + // Mock verifyAppleIdToken to return claims WITHOUT email (returning user) + (verifyAppleIdToken as jest.Mock).mockResolvedValue({ + sub: 'apple-user-123', + // no email - common for returning Apple users + }); + + // User exists in local database by providerUserId + const existingUser = new User(); + existingUser.setId('user-id-123'); + existingUser.setEmail('user@example.com'); + existingUser.setProvider('apple'); + existingUser.setProviderUserId('apple-user-123'); + + (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(existingUser); + + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + // Should find user by providerUserId + expect(deps.users.findOneByScan).toHaveBeenCalledWith({ providerUserId: 'apple-user-123' }); + + // Should login the existing user + expect(req.login).toHaveBeenCalledWith(existingUser, expect.any(Function)); + + // Should redirect to the returnUrl + expect(getRedirectUrl()).toBe('/projects/test'); + }); + + it('should return error only if no email AND user not found by providerUserId', async () => { + const deps = createAppleMockDeps(); + const handler = createAppleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + // Mock Apple token exchange + (exchangeAppleCode as jest.Mock).mockResolvedValue({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }); + + // Mock verifyAppleIdToken to return claims WITHOUT email + (verifyAppleIdToken as jest.Mock).mockResolvedValue({ + sub: 'apple-user-unknown', + // no email + }); + + // User does NOT exist in local database + (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); + + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + // Should try to find user by providerUserId + expect(deps.users.findOneByScan).toHaveBeenCalledWith({ providerUserId: 'apple-user-unknown' }); + + // Should redirect with error since user not found and no email to create one + expect(getRedirectUrl()).toBe('/?error=apple_no_email'); + }); + }); +}); From 84e3ef714efa2eb76518e7912e895181455f0936 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 15:55:15 -0800 Subject: [PATCH 04/17] server: persist providerUserId when matching user by email When a user first authenticates via OAuth provider (e.g., Apple) but was previously matched by email (e.g., from password signup or different OAuth provider), we now save the providerUserId to enable future logins when the provider doesn't send the email (common with Apple after first authorization). This fixes the issue where returning Apple users were redirected to apple_no_email error even though their account existed, because providerUserId was never backfilled on the email-matched user record. Adds unit tests for getOrCreateUserFromVerifiedInfo. --- src/server/authn.ts | 11 +++ src/server/tests/authn.test.ts | 172 +++++++++++++++++++++++++++++++++ 2 files changed, 183 insertions(+) create mode 100644 src/server/tests/authn.test.ts diff --git a/src/server/authn.ts b/src/server/authn.ts index d1275c178..785613238 100644 --- a/src/server/authn.ts +++ b/src/server/authn.ts @@ -183,12 +183,23 @@ export async function getOrCreateUserFromVerifiedInfo( } let user: User | undefined; + let matchedByEmail = false; try { if (info.providerUserId) { user = await users.findOneByScan({ providerUserId: info.providerUserId }); } if (!user && info.email) { user = await users.findOneByScan({ email: info.email }); + if (user) { + matchedByEmail = true; + } + } + if (user && matchedByEmail && info.providerUserId && user.getProviderUserId() !== info.providerUserId) { + // User was found by email but has different (or no) providerUserId. + // Update to use the new provider info so future logins without email work. + user.setProviderUserId(info.providerUserId); + user.setProvider(info.provider); + await users.update(user.getId(), {}, user); } if (!user) { const created = new Timestamp(); diff --git a/src/server/tests/authn.test.ts b/src/server/tests/authn.test.ts new file mode 100644 index 000000000..e937d02e6 --- /dev/null +++ b/src/server/tests/authn.test.ts @@ -0,0 +1,172 @@ +// Copyright 2025 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +import { getOrCreateUserFromVerifiedInfo, VerifiedUserInfo } from '../authn'; +import { Table } from '../models/table'; +import { User } from '../schemas/user_pb'; + +function createMockUsers(): jest.Mocked> { + return { + init: jest.fn(), + findOne: jest.fn(), + findOneByScan: jest.fn(), + findByScan: jest.fn(), + find: jest.fn(), + create: jest.fn(), + update: jest.fn(), + deleteOne: jest.fn(), + }; +} + +describe('getOrCreateUserFromVerifiedInfo', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('when user exists by providerUserId', () => { + it('should return existing user', async () => { + const users = createMockUsers(); + + const existingUser = new User(); + existingUser.setId('user-123'); + existingUser.setEmail('test@example.com'); + existingUser.setProvider('google'); + existingUser.setProviderUserId('google-123'); + + users.findOneByScan.mockResolvedValueOnce(existingUser); + + const info: VerifiedUserInfo = { + email: 'test@example.com', + displayName: 'Test User', + provider: 'google', + providerUserId: 'google-123', + }; + + const [user, err] = await getOrCreateUserFromVerifiedInfo(users, info); + + expect(err).toBeUndefined(); + expect(user).toBe(existingUser); + expect(users.findOneByScan).toHaveBeenCalledWith({ providerUserId: 'google-123' }); + }); + }); + + describe('when user exists by email but not providerUserId', () => { + it('should update providerUserId on the existing user', async () => { + const users = createMockUsers(); + + // User exists with password auth (no providerUserId) + const existingUser = new User(); + existingUser.setId('user-123'); + existingUser.setEmail('test@example.com'); + existingUser.setProvider('password'); + existingUser.setProviderUserId(''); + + // First call: findOneByScan by providerUserId returns nothing + // Second call: findOneByScan by email returns existing user + users.findOneByScan.mockResolvedValueOnce(undefined).mockResolvedValueOnce(existingUser); + users.update.mockResolvedValue(existingUser); + + const info: VerifiedUserInfo = { + email: 'test@example.com', + displayName: 'Test User', + provider: 'apple', + providerUserId: 'apple-sub-456', + }; + + const [user, err] = await getOrCreateUserFromVerifiedInfo(users, info); + + expect(err).toBeUndefined(); + expect(user).toBeDefined(); + + // Should have searched by providerUserId first + expect(users.findOneByScan).toHaveBeenNthCalledWith(1, { providerUserId: 'apple-sub-456' }); + // Then by email + expect(users.findOneByScan).toHaveBeenNthCalledWith(2, { email: 'test@example.com' }); + + // Should have updated the user with the new providerUserId + expect(users.update).toHaveBeenCalledWith('user-123', {}, expect.any(User)); + const updatedUser = users.update.mock.calls[0][2] as User; + expect(updatedUser.getProviderUserId()).toBe('apple-sub-456'); + expect(updatedUser.getProvider()).toBe('apple'); + }); + + it('should not update if providerUserId already matches', async () => { + const users = createMockUsers(); + + const existingUser = new User(); + existingUser.setId('user-123'); + existingUser.setEmail('test@example.com'); + existingUser.setProvider('apple'); + existingUser.setProviderUserId('apple-sub-456'); + + // findOneByScan by providerUserId returns nothing (edge case: different email lookup first) + // findOneByScan by email returns existing user + users.findOneByScan.mockResolvedValueOnce(undefined).mockResolvedValueOnce(existingUser); + + const info: VerifiedUserInfo = { + email: 'test@example.com', + displayName: 'Test User', + provider: 'apple', + providerUserId: 'apple-sub-456', + }; + + const [user, err] = await getOrCreateUserFromVerifiedInfo(users, info); + + expect(err).toBeUndefined(); + expect(user).toBe(existingUser); + + // Should NOT have updated since providerUserId already matches + expect(users.update).not.toHaveBeenCalled(); + }); + }); + + describe('when no user exists', () => { + it('should create new user with providerUserId', async () => { + const users = createMockUsers(); + + // No user found by providerUserId or email + users.findOneByScan.mockResolvedValue(undefined); + users.create.mockResolvedValue(); + + const info: VerifiedUserInfo = { + email: 'newuser@example.com', + displayName: 'New User', + provider: 'google', + providerUserId: 'google-new-789', + photoUrl: 'https://example.com/photo.jpg', + }; + + const [user, err] = await getOrCreateUserFromVerifiedInfo(users, info); + + expect(err).toBeUndefined(); + expect(user).toBeDefined(); + expect(user!.getEmail()).toBe('newuser@example.com'); + expect(user!.getDisplayName()).toBe('New User'); + expect(user!.getProvider()).toBe('google'); + expect(user!.getProviderUserId()).toBe('google-new-789'); + expect(user!.getPhotoUrl()).toBe('https://example.com/photo.jpg'); + + expect(users.create).toHaveBeenCalled(); + }); + }); + + describe('error handling', () => { + it('should return error if no email provided', async () => { + const users = createMockUsers(); + + const info: VerifiedUserInfo = { + email: '', + displayName: 'Test User', + provider: 'google', + providerUserId: 'google-123', + }; + + const [user, err] = await getOrCreateUserFromVerifiedInfo(users, info); + + expect(user).toBeUndefined(); + expect(err).toBeDefined(); + expect(err!.message).toContain('expected user to have an email'); + }); + }); +}); From 8a756486e3a1d43b8315ab9ee1538aaa92bdaf73 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 16:04:27 -0800 Subject: [PATCH 05/17] server: block OAuth login for disabled Firebase users The OAuth callbacks were not checking the Firebase user's disabled status, allowing disabled users to sign in via Google/Apple OAuth. This was a regression from the client-SDK flow that properly rejected disabled accounts. Now we check the UserRecord.disabled flag after fetching/creating the Firebase user and redirect with account_disabled error if true. Adds tests for disabled user handling in both Google and Apple OAuth. --- src/server/auth/oauth-handlers.ts | 20 ++++-- src/server/tests/oauth-handlers.test.ts | 94 +++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/src/server/auth/oauth-handlers.ts b/src/server/auth/oauth-handlers.ts index 63304021c..16bf33c91 100644 --- a/src/server/auth/oauth-handlers.ts +++ b/src/server/auth/oauth-handlers.ts @@ -122,12 +122,13 @@ export function createGoogleOAuthCallbackHandler(deps: GoogleOAuthHandlerDeps): const userInfo = await fetchGoogleUserInfo(tokens.access_token); + let fbUser: admin.auth.UserRecord | undefined; try { - await deps.firebaseAdmin.getUserByEmail(userInfo.email); + fbUser = await deps.firebaseAdmin.getUserByEmail(userInfo.email); } catch (err: unknown) { const adminErr = err as { code?: string }; if (adminErr.code === 'auth/user-not-found') { - await deps.firebaseAdmin.createUser({ + fbUser = await deps.firebaseAdmin.createUser({ email: userInfo.email, displayName: userInfo.name, photoURL: userInfo.picture, @@ -138,6 +139,11 @@ export function createGoogleOAuthCallbackHandler(deps: GoogleOAuthHandlerDeps): } } + if (fbUser?.disabled) { + res.redirect('/?error=account_disabled'); + return; + } + const [user, userErr] = await getOrCreateUserFromVerifiedInfo(deps.users, { email: userInfo.email, displayName: userInfo.name, @@ -266,12 +272,13 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re return; } + let fbUser: admin.auth.UserRecord | undefined; try { - await deps.firebaseAdmin.getUserByEmail(email); + fbUser = await deps.firebaseAdmin.getUserByEmail(email); } catch (err: unknown) { const adminErr = err as { code?: string }; if (adminErr.code === 'auth/user-not-found') { - await deps.firebaseAdmin.createUser({ + fbUser = await deps.firebaseAdmin.createUser({ email, displayName, emailVerified: claims.email_verified ?? false, @@ -281,6 +288,11 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re } } + if (fbUser?.disabled) { + res.redirect('/?error=account_disabled'); + return; + } + const [user, userErr] = await getOrCreateUserFromVerifiedInfo(deps.users, { email, displayName, diff --git a/src/server/tests/oauth-handlers.test.ts b/src/server/tests/oauth-handlers.test.ts index 9430f7a7e..7dd6e06d0 100644 --- a/src/server/tests/oauth-handlers.test.ts +++ b/src/server/tests/oauth-handlers.test.ts @@ -527,6 +527,55 @@ describe('createGoogleOAuthCallbackHandler', () => { expect(getRedirectUrl()).toBe('/?error=oauth_denied'); }); + + it('should reject disabled Firebase users', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + sub: 'google-123', + email: 'disabled@example.com', + email_verified: true, + name: 'Disabled User', + }), + }); + + // Return a disabled Firebase user + (deps.firebaseAdmin as jest.Mocked).getUserByEmail.mockResolvedValue({ + uid: 'fb-uid-123', + email: 'disabled@example.com', + disabled: true, + } as admin.auth.UserRecord); + + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + // Should redirect with account disabled error + expect(getRedirectUrl()).toBe('/?error=account_disabled'); + + // Should NOT have called login + expect(req.login).not.toHaveBeenCalled(); + }); }); }); @@ -560,6 +609,51 @@ describe('createAppleOAuthCallbackHandler', () => { jest.clearAllMocks(); }); + describe('disabled user handling', () => { + it('should reject disabled Firebase users', async () => { + const deps = createAppleMockDeps(); + const handler = createAppleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + // Mock Apple token exchange + (exchangeAppleCode as jest.Mock).mockResolvedValue({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }); + + // Mock verifyAppleIdToken + (verifyAppleIdToken as jest.Mock).mockResolvedValue({ + sub: 'apple-123', + email: 'disabled@example.com', + }); + + // Return a disabled Firebase user + (deps.firebaseAdmin as jest.Mocked).getUserByEmail.mockResolvedValue({ + uid: 'fb-uid-123', + email: 'disabled@example.com', + disabled: true, + } as admin.auth.UserRecord); + + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + // Should redirect with account disabled error + expect(getRedirectUrl()).toBe('/?error=account_disabled'); + + // Should NOT have called login + expect(req.login).not.toHaveBeenCalled(); + }); + }); + describe('returning user without email', () => { it('should login existing user by providerUserId when Apple omits email', async () => { const deps = createAppleMockDeps(); From 19f5cb3b77c556171be0a5e80c53814c6fb6a594 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 16:10:05 -0800 Subject: [PATCH 06/17] server: preserve password whitespace during authentication Passwords were being trimmed before sending to Firebase, which caused login failures for users whose passwords contain leading or trailing spaces (Firebase treats these as significant). This was a regression from the client SDK flow. Email addresses are still trimmed (as they don't have meaningful whitespace), but passwords are now passed through unchanged. Updates the test to verify this behavior. --- src/server/auth/auth-handlers.ts | 4 ++-- src/server/tests/auth-handlers.test.ts | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/server/auth/auth-handlers.ts b/src/server/auth/auth-handlers.ts index 1cd62a5a7..b2149c1e6 100644 --- a/src/server/auth/auth-handlers.ts +++ b/src/server/auth/auth-handlers.ts @@ -52,7 +52,7 @@ function loginUser(req: Request, user: User): Promise { export function createLoginHandler(deps: AuthHandlerDeps): RequestHandler { return async (req: Request, res: Response): Promise => { const email = typeof req.body?.email === 'string' ? req.body.email.trim() : ''; - const password = typeof req.body?.password === 'string' ? req.body.password.trim() : ''; + const password = typeof req.body?.password === 'string' ? req.body.password : ''; if (!email) { res.status(400).json({ error: 'Email is required' }); @@ -104,7 +104,7 @@ export function createLoginHandler(deps: AuthHandlerDeps): RequestHandler { export function createSignupHandler(deps: AuthHandlerDeps): RequestHandler { return async (req: Request, res: Response): Promise => { const email = typeof req.body?.email === 'string' ? req.body.email.trim() : ''; - const password = typeof req.body?.password === 'string' ? req.body.password.trim() : ''; + const password = typeof req.body?.password === 'string' ? req.body.password : ''; const displayName = typeof req.body?.displayName === 'string' ? req.body.displayName.trim() : ''; if (!email) { diff --git a/src/server/tests/auth-handlers.test.ts b/src/server/tests/auth-handlers.test.ts index f85de4259..9b4ade703 100644 --- a/src/server/tests/auth-handlers.test.ts +++ b/src/server/tests/auth-handlers.test.ts @@ -146,7 +146,7 @@ describe('createLoginHandler', () => { expect(getBody()).toEqual({ error: 'Email is required' }); }); - it('should trim whitespace from email and password', async () => { + it('should trim whitespace from email but not password', async () => { const deps = createMockDeps(); const handler = createLoginHandler(deps); @@ -178,12 +178,15 @@ describe('createLoginHandler', () => { } as admin.auth.UserRecord); deps.users.findOneByScan.mockResolvedValue(createMockUser('testuser', 'test@example.com', 'Test User')); + // Email should be trimmed, but password should NOT be trimmed + // (Firebase treats leading/trailing spaces in passwords as significant) const req = createMockRequest({ email: ' test@example.com ', password: ' password123 ' }); const { res } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); - expect(deps.firebaseRestClient.signInWithPassword).toHaveBeenCalledWith('test@example.com', 'password123'); + // Email trimmed, password preserved with spaces + expect(deps.firebaseRestClient.signInWithPassword).toHaveBeenCalledWith('test@example.com', ' password123 '); }); }); From dd1713c9f76f2108b8fa8f7ecd52e17ce1c32ac0 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 16:18:53 -0800 Subject: [PATCH 07/17] server: normalize Apple email_verified to boolean Apple's ID token may encode email_verified as a string "true"/"false" instead of a boolean. The previous code cast this directly to boolean, which could cause "false" (string) to be treated as truthy, or cause the Firebase Admin SDK to reject the non-boolean value. Now we explicitly convert string values to proper booleans. Adds tests for the email_verified normalization. --- src/server/auth/oauth-token-exchange.ts | 11 ++- src/server/tests/oauth-token-exchange.test.ts | 98 ++++++++++++++++++- 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/src/server/auth/oauth-token-exchange.ts b/src/server/auth/oauth-token-exchange.ts index ce1db96ac..d66d2d22c 100644 --- a/src/server/auth/oauth-token-exchange.ts +++ b/src/server/auth/oauth-token-exchange.ts @@ -161,10 +161,19 @@ export async function verifyAppleIdToken(idToken: string, options: { clientId: s audience: options.clientId, }); + // Apple may send email_verified as a string "true"/"false" instead of boolean + const rawEmailVerified = payload.email_verified; + let emailVerified: boolean | undefined; + if (rawEmailVerified === 'true' || rawEmailVerified === true) { + emailVerified = true; + } else if (rawEmailVerified === 'false' || rawEmailVerified === false) { + emailVerified = false; + } + return { sub: payload.sub as string, email: payload.email as string | undefined, - email_verified: payload.email_verified as boolean | undefined, + email_verified: emailVerified, name: (payload as { name?: string }).name, }; } diff --git a/src/server/tests/oauth-token-exchange.test.ts b/src/server/tests/oauth-token-exchange.test.ts index 05cae633f..0a6293c27 100644 --- a/src/server/tests/oauth-token-exchange.test.ts +++ b/src/server/tests/oauth-token-exchange.test.ts @@ -9,7 +9,7 @@ jest.mock('jose', () => ({ import * as crypto from 'crypto'; -import { generateAppleClientSecret } from '../auth/oauth-token-exchange'; +import { generateAppleClientSecret, verifyAppleIdToken, clearJwksCache } from '../auth/oauth-token-exchange'; describe('generateAppleClientSecret', () => { let testPrivateKey: string; @@ -96,3 +96,99 @@ describe('generateAppleClientSecret', () => { expect(signature.length).toBe(64); }); }); + +describe('verifyAppleIdToken', () => { + const jose = require('jose'); + + beforeEach(() => { + clearJwksCache(); + jest.clearAllMocks(); + }); + + it('should convert email_verified string "true" to boolean true', async () => { + // Mock fetch for JWKS + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + json: async () => ({ keys: [] }), + }); + + // Mock jose functions + jose.createLocalJWKSet.mockReturnValue(jest.fn()); + jose.jwtVerify.mockResolvedValue({ + payload: { + sub: 'apple-user-123', + email: 'test@example.com', + email_verified: 'true', // Apple sends string, not boolean + }, + }); + + const result = await verifyAppleIdToken('mock-token', { clientId: 'test-client-id' }); + + expect(result.sub).toBe('apple-user-123'); + expect(result.email).toBe('test@example.com'); + expect(result.email_verified).toBe(true); // Should be boolean, not string + expect(typeof result.email_verified).toBe('boolean'); + }); + + it('should convert email_verified string "false" to boolean false', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + json: async () => ({ keys: [] }), + }); + + jose.createLocalJWKSet.mockReturnValue(jest.fn()); + jose.jwtVerify.mockResolvedValue({ + payload: { + sub: 'apple-user-456', + email: 'test@example.com', + email_verified: 'false', // Apple sends string, not boolean + }, + }); + + const result = await verifyAppleIdToken('mock-token', { clientId: 'test-client-id' }); + + expect(result.email_verified).toBe(false); + expect(typeof result.email_verified).toBe('boolean'); + }); + + it('should handle boolean email_verified values', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + json: async () => ({ keys: [] }), + }); + + jose.createLocalJWKSet.mockReturnValue(jest.fn()); + jose.jwtVerify.mockResolvedValue({ + payload: { + sub: 'apple-user-789', + email: 'test@example.com', + email_verified: true, // In case Apple sends actual boolean + }, + }); + + const result = await verifyAppleIdToken('mock-token', { clientId: 'test-client-id' }); + + expect(result.email_verified).toBe(true); + expect(typeof result.email_verified).toBe('boolean'); + }); + + it('should handle missing email_verified', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + json: async () => ({ keys: [] }), + }); + + jose.createLocalJWKSet.mockReturnValue(jest.fn()); + jose.jwtVerify.mockResolvedValue({ + payload: { + sub: 'apple-user-000', + email: 'test@example.com', + // no email_verified + }, + }); + + const result = await verifyAppleIdToken('mock-token', { clientId: 'test-client-id' }); + + expect(result.email_verified).toBeUndefined(); + }); +}); From db274629807a14f00789c2ce6c51d9a57cc696ba Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 16:28:25 -0800 Subject: [PATCH 08/17] server: add Firebase provider fallback for Apple user migration Existing Apple users created before the providerUserId migration may not have providerUserId set in our database. When these users try to sign in and Apple omits the email, we now fall back to looking up the Firebase user by their Apple provider UID. If found, we retrieve their email from Firebase, find the local user, update their providerUserId for future logins, and log them in. This prevents pre-migration Apple users from being locked out. Adds test for the Firebase provider fallback migration path. --- src/server/auth/oauth-handlers.ts | 26 +++++++++- src/server/tests/oauth-handlers.test.ts | 69 +++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/server/auth/oauth-handlers.ts b/src/server/auth/oauth-handlers.ts index 16bf33c91..95dcacd32 100644 --- a/src/server/auth/oauth-handlers.ts +++ b/src/server/auth/oauth-handlers.ts @@ -259,13 +259,37 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re if (!email) { // Apple omits email on subsequent logins. Look up user by providerUserId. - const existingUser = await deps.users.findOneByScan({ providerUserId: claims.sub }); + let existingUser = await deps.users.findOneByScan({ providerUserId: claims.sub }); if (existingUser) { await loginUser(req, existingUser); res.redirect(returnUrl); return; } + // Fallback for users created before providerUserId migration: try to find via Firebase + // provider link. This handles users who signed in with Apple before we started storing + // the Apple sub as providerUserId. + try { + const fbUser = await deps.firebaseAdmin.getUserByProviderUid('apple.com', claims.sub); + if (fbUser && !fbUser.disabled && fbUser.email) { + // Found Firebase user with this Apple ID - look up local user by email + existingUser = await deps.users.findOneByScan({ email: fbUser.email }); + if (existingUser) { + // Update providerUserId so future logins work directly + existingUser.setProviderUserId(claims.sub); + existingUser.setProvider('apple'); + await deps.users.update(existingUser.getId(), {}, existingUser); + + await loginUser(req, existingUser); + res.redirect(returnUrl); + return; + } + } + } catch (err) { + // getUserByProviderUid throws if user not found - that's expected + logger.debug('No Firebase user found with Apple provider:', err); + } + // No email and no existing user - we can't create a new account logger.error('Apple user has no email and could not be found by providerUserId'); res.redirect('/?error=apple_no_email'); diff --git a/src/server/tests/oauth-handlers.test.ts b/src/server/tests/oauth-handlers.test.ts index 7dd6e06d0..748bf5bd0 100644 --- a/src/server/tests/oauth-handlers.test.ts +++ b/src/server/tests/oauth-handlers.test.ts @@ -48,6 +48,7 @@ function createMockStateStore(): jest.Mocked { function createMockFirebaseAdmin(): jest.Mocked { return { getUserByEmail: jest.fn(), + getUserByProviderUid: jest.fn(), createUser: jest.fn(), updateUser: jest.fn(), listUsers: jest.fn(), @@ -741,5 +742,73 @@ describe('createAppleOAuthCallbackHandler', () => { // Should redirect with error since user not found and no email to create one expect(getRedirectUrl()).toBe('/?error=apple_no_email'); }); + + it('should fall back to Firebase provider lookup for pre-migration users without email', async () => { + const deps = createAppleMockDeps(); + const handler = createAppleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/projects/migrated', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + // Mock Apple token exchange + (exchangeAppleCode as jest.Mock).mockResolvedValue({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }); + + // Mock verifyAppleIdToken to return claims WITHOUT email (returning user) + (verifyAppleIdToken as jest.Mock).mockResolvedValue({ + sub: 'apple-pre-migration-user', + // no email - common for returning Apple users + }); + + // User does NOT exist by providerUserId (wasn't stored before migration) + // But DOES exist by email (found via Firebase provider lookup) + const existingUser = new User(); + existingUser.setId('user-legacy-456'); + existingUser.setEmail('legacy@example.com'); + existingUser.setProvider('password'); // Was originally password user who added Apple + existingUser.setProviderUserId(''); // No providerUserId before migration + + // First findOneByScan (by providerUserId) returns nothing + // Second findOneByScan (by email) returns the user + (deps.users as jest.Mocked>).findOneByScan + .mockResolvedValueOnce(undefined) // providerUserId lookup + .mockResolvedValueOnce(existingUser); // email lookup + + // Firebase has this user with Apple provider linked + (deps.firebaseAdmin as jest.Mocked).getUserByProviderUid.mockResolvedValue({ + uid: 'fb-legacy-user', + email: 'legacy@example.com', + disabled: false, + } as admin.auth.UserRecord); + + (deps.users as jest.Mocked>).update.mockResolvedValue(existingUser); + + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + // Should have looked up Firebase user by Apple provider + expect(deps.firebaseAdmin.getUserByProviderUid).toHaveBeenCalledWith('apple.com', 'apple-pre-migration-user'); + + // Should have updated the user's providerUserId for future logins + expect(deps.users.update).toHaveBeenCalled(); + const updateCall = (deps.users.update as jest.Mock).mock.calls[0]; + expect(updateCall[0]).toBe('user-legacy-456'); + const updatedUser = updateCall[2] as User; + expect(updatedUser.getProviderUserId()).toBe('apple-pre-migration-user'); + expect(updatedUser.getProvider()).toBe('apple'); + + // Should login and redirect + expect(req.login).toHaveBeenCalled(); + expect(getRedirectUrl()).toBe('/projects/migrated'); + }); }); }); From 42ab7644575213328b416e633cdbaf810f77f58e Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 16:40:22 -0800 Subject: [PATCH 09/17] server: downgrade jose to v4 for CommonJS compatibility Jose v6 is ESM-only, but the server is compiled as CommonJS. This would cause ERR_REQUIRE_ESM at runtime when trying to load the auth module. Jose v4 has dual CJS/ESM exports and provides the same API we use (createLocalJWKSet, jwtVerify), so this is a safe downgrade. --- pnpm-lock.yaml | 75 ++--------------------------------------- src/server/package.json | 2 +- 2 files changed, 3 insertions(+), 74 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f62c76139..3980cef23 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -51,12 +51,6 @@ importers: specifier: ^19.0.0 version: 19.2.4(react@19.2.4) devDependencies: - '@firebase/app': - specifier: ^0.13.2 - version: 0.13.2 - '@firebase/auth': - specifier: ^1.10.8 - version: 1.12.0(@firebase/app@0.13.2) '@rsbuild/core': specifier: ^1.3.22 version: 1.7.2 @@ -372,8 +366,8 @@ importers: specifier: ^5.0.3 version: 5.1.4 jose: - specifier: ^6.1.3 - version: 6.1.3 + specifier: ^4.15.9 + version: 4.15.9 node-fetch: specifier: ^3.0.0 version: 3.3.2 @@ -771,27 +765,9 @@ packages: '@firebase/app-types@0.9.3': resolution: {integrity: sha512-kRVpIl4vVGJ4baogMDINbyrIOtOxqhkZQg4jTq3l8Lw6WSk0xfpEYzezFu+Kl4ve4fbPl79dvwRtaFqAC/ucCw==} - '@firebase/app@0.13.2': - resolution: {integrity: sha512-jwtMmJa1BXXDCiDx1vC6SFN/+HfYG53UkfJa6qeN5ogvOunzbFDO3wISZy5n9xgYFUrEP6M7e8EG++riHNTv9w==} - engines: {node: '>=18.0.0'} - '@firebase/auth-interop-types@0.2.4': resolution: {integrity: sha512-JPgcXKCuO+CWqGDnigBtvo09HeBs5u/Ktc2GaFj2m01hLarbxthLNm7Fk8iOP1aqAtXV+fnnGj7U28xmk7IwVA==} - '@firebase/auth@1.12.0': - resolution: {integrity: sha512-zkvLpsrxynWHk07qGrUDfCSqKf4AvfZGEqJ7mVCtYGjNNDbGE71k0Yn84rg8QEZu4hQw1BC0qDEHzpNVBcSVmA==} - engines: {node: '>=20.0.0'} - peerDependencies: - '@firebase/app': 0.x - '@react-native-async-storage/async-storage': ^2.2.0 - peerDependenciesMeta: - '@react-native-async-storage/async-storage': - optional: true - - '@firebase/component@0.6.18': - resolution: {integrity: sha512-n28kPCkE2dL2U28fSxZJjzPPVpKsQminJ6NrzcKXAI0E/lYC8YhfwpyllScqVEvAI3J2QgJZWYgrX+1qGI+SQQ==} - engines: {node: '>=18.0.0'} - '@firebase/component@0.7.0': resolution: {integrity: sha512-wR9En2A+WESUHexjmRHkqtaVH94WLNKt6rmeqZhSLBybg4Wyf0Umk04SZsS6sBq4102ZsDBFwoqMqJYj2IoDSg==} engines: {node: '>=20.0.0'} @@ -807,18 +783,10 @@ packages: resolution: {integrity: sha512-gM6MJFae3pTyNLoc9VcJNuaUDej0ctdjn3cVtILo3D5lpp0dmUHHLFN/pUKe7ImyeB1KAvRlEYxvIHNF04Filg==} engines: {node: '>=20.0.0'} - '@firebase/logger@0.4.4': - resolution: {integrity: sha512-mH0PEh1zoXGnaR8gD1DeGeNZtWFKbnz9hDO91dIml3iou1gpOnLqXQ2dJfB71dj6dpmUjcQ6phY3ZZJbjErr9g==} - engines: {node: '>=18.0.0'} - '@firebase/logger@0.5.0': resolution: {integrity: sha512-cGskaAvkrnh42b3BA3doDWeBmuHFO/Mx5A83rbRDYakPjO9bJtRL3dX7javzc2Rr/JHZf4HlterTW2lUkfeN4g==} engines: {node: '>=20.0.0'} - '@firebase/util@1.12.1': - resolution: {integrity: sha512-zGlBn/9Dnya5ta9bX/fgEoNC3Cp8s6h+uYPYaDieZsFOAdHP/ExzQ/eaDgxD3GOROdPkLKpvKY0iIzr9adle0w==} - engines: {node: '>=18.0.0'} - '@firebase/util@1.13.0': resolution: {integrity: sha512-0AZUyYUfpMNcztR5l09izHwXkZpghLgCUaAGjtMwXnCg3bj4ml5VgiwqOMOxJ+Nw4qN/zJAaOQBcJ7KGkWStqQ==} engines: {node: '>=20.0.0'} @@ -4384,9 +4352,6 @@ packages: resolution: {integrity: sha512-im9DjEDQ55s9fL4EYzOAv0yMqmMBSZp6G0VvFyTMPKWxiSBHUj9NW/qqLmXUwXrrM7AvqSlTCfvqRb0cM8yYqw==} engines: {node: '>=0.10.0'} - idb@7.1.1: - resolution: {integrity: sha512-gchesWBzyvGHRO9W8tzUWFDycow5gwjvFKfyV9FF32Y7F50yZMp7mP+T2mJIWFx49zicqyC4uefHM17o6xKIVQ==} - ieee754@1.2.1: resolution: {integrity: sha512-dcyqhDvX1C46lXZcVqCpK+FtMRQVdIMN6/Df5js2zouUsqG7I6sFxitIC+7KYK29KdXOLHdu9zL4sFnoVQnqaA==} @@ -4933,9 +4898,6 @@ packages: jose@4.15.9: resolution: {integrity: sha512-1vUQX+IdDMVPj4k8kOxgUqlcK518yluMuGZwqlr44FS1ppZB/5GWh4rZG89erpOBOJjU/OBsnCVFfapsRz6nEA==} - jose@6.1.3: - resolution: {integrity: sha512-0TpaTfihd4QMNwrz/ob2Bp7X04yuxJkjRGi4aKmOqwhov54i6u79oCv7T+C7lo70MKH6BesI3vscD1yb/yzKXQ==} - js-base64@3.7.8: resolution: {integrity: sha512-hNngCeKxIUQiEUN3GPJOkz4wF/YvdUdbNL9hsBcMQTkKzboD7T/q3OYOuuPZLUE6dBxSGpwhk5mwuDud7JVAow==} @@ -7975,29 +7937,8 @@ snapshots: '@firebase/app-types@0.9.3': {} - '@firebase/app@0.13.2': - dependencies: - '@firebase/component': 0.6.18 - '@firebase/logger': 0.4.4 - '@firebase/util': 1.12.1 - idb: 7.1.1 - tslib: 2.8.1 - '@firebase/auth-interop-types@0.2.4': {} - '@firebase/auth@1.12.0(@firebase/app@0.13.2)': - dependencies: - '@firebase/app': 0.13.2 - '@firebase/component': 0.7.0 - '@firebase/logger': 0.5.0 - '@firebase/util': 1.13.0 - tslib: 2.8.1 - - '@firebase/component@0.6.18': - dependencies: - '@firebase/util': 1.12.1 - tslib: 2.8.1 - '@firebase/component@0.7.0': dependencies: '@firebase/util': 1.13.0 @@ -8027,18 +7968,10 @@ snapshots: faye-websocket: 0.11.4 tslib: 2.8.1 - '@firebase/logger@0.4.4': - dependencies: - tslib: 2.8.1 - '@firebase/logger@0.5.0': dependencies: tslib: 2.8.1 - '@firebase/util@1.12.1': - dependencies: - tslib: 2.8.1 - '@firebase/util@1.13.0': dependencies: tslib: 2.8.1 @@ -12474,8 +12407,6 @@ snapshots: dependencies: safer-buffer: 2.1.2 - idb@7.1.1: {} - ieee754@1.2.1: {} ignore@5.3.2: {} @@ -13344,8 +13275,6 @@ snapshots: jose@4.15.9: {} - jose@6.1.3: {} - js-base64@3.7.8: {} js-tokens@4.0.0: {} diff --git a/src/server/package.json b/src/server/package.json index 0d1d9c057..c9de08de6 100644 --- a/src/server/package.json +++ b/src/server/package.json @@ -22,7 +22,7 @@ "google-protobuf": "^4.0.0", "helmet": "^8.0.0", "immutable": "^5.0.3", - "jose": "^6.1.3", + "jose": "^4.15.9", "node-fetch": "^3.0.0", "passport": "^0.5.3", "passport-strategy": "^1.0.0", From 918b18155691f3774cf48c3a2f6f6e1d2a2f1c6c Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 16:49:44 -0800 Subject: [PATCH 10/17] server: scope providerUserId lookups by provider Include provider in providerUserId lookups to prevent potential cross-provider collisions. While unlikely, different OAuth providers (Google, Apple) could theoretically have the same user ID, which could cause account mixups. Now we always query by both providerUserId AND provider to ensure matches are provider-specific. --- src/server/auth/oauth-handlers.ts | 3 ++- src/server/authn.ts | 3 ++- src/server/tests/authn.test.ts | 7 ++++--- src/server/tests/oauth-handlers.test.ts | 8 ++++---- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/server/auth/oauth-handlers.ts b/src/server/auth/oauth-handlers.ts index 95dcacd32..ce70b8181 100644 --- a/src/server/auth/oauth-handlers.ts +++ b/src/server/auth/oauth-handlers.ts @@ -259,7 +259,8 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re if (!email) { // Apple omits email on subsequent logins. Look up user by providerUserId. - let existingUser = await deps.users.findOneByScan({ providerUserId: claims.sub }); + // Include provider to prevent cross-provider collisions. + let existingUser = await deps.users.findOneByScan({ providerUserId: claims.sub, provider: 'apple' }); if (existingUser) { await loginUser(req, existingUser); res.redirect(returnUrl); diff --git a/src/server/authn.ts b/src/server/authn.ts index 785613238..913618574 100644 --- a/src/server/authn.ts +++ b/src/server/authn.ts @@ -186,7 +186,8 @@ export async function getOrCreateUserFromVerifiedInfo( let matchedByEmail = false; try { if (info.providerUserId) { - user = await users.findOneByScan({ providerUserId: info.providerUserId }); + // Include provider in lookup to prevent cross-provider collisions + user = await users.findOneByScan({ providerUserId: info.providerUserId, provider: info.provider }); } if (!user && info.email) { user = await users.findOneByScan({ email: info.email }); diff --git a/src/server/tests/authn.test.ts b/src/server/tests/authn.test.ts index e937d02e6..42d7478ed 100644 --- a/src/server/tests/authn.test.ts +++ b/src/server/tests/authn.test.ts @@ -47,7 +47,8 @@ describe('getOrCreateUserFromVerifiedInfo', () => { expect(err).toBeUndefined(); expect(user).toBe(existingUser); - expect(users.findOneByScan).toHaveBeenCalledWith({ providerUserId: 'google-123' }); + // Should include provider in lookup to prevent cross-provider collisions + expect(users.findOneByScan).toHaveBeenCalledWith({ providerUserId: 'google-123', provider: 'google' }); }); }); @@ -79,8 +80,8 @@ describe('getOrCreateUserFromVerifiedInfo', () => { expect(err).toBeUndefined(); expect(user).toBeDefined(); - // Should have searched by providerUserId first - expect(users.findOneByScan).toHaveBeenNthCalledWith(1, { providerUserId: 'apple-sub-456' }); + // Should have searched by providerUserId AND provider first + expect(users.findOneByScan).toHaveBeenNthCalledWith(1, { providerUserId: 'apple-sub-456', provider: 'apple' }); // Then by email expect(users.findOneByScan).toHaveBeenNthCalledWith(2, { email: 'test@example.com' }); diff --git a/src/server/tests/oauth-handlers.test.ts b/src/server/tests/oauth-handlers.test.ts index 748bf5bd0..646ff0353 100644 --- a/src/server/tests/oauth-handlers.test.ts +++ b/src/server/tests/oauth-handlers.test.ts @@ -694,8 +694,8 @@ describe('createAppleOAuthCallbackHandler', () => { await handler(req as Request, res as Response, jest.fn()); - // Should find user by providerUserId - expect(deps.users.findOneByScan).toHaveBeenCalledWith({ providerUserId: 'apple-user-123' }); + // Should find user by providerUserId AND provider (prevents cross-provider collisions) + expect(deps.users.findOneByScan).toHaveBeenCalledWith({ providerUserId: 'apple-user-123', provider: 'apple' }); // Should login the existing user expect(req.login).toHaveBeenCalledWith(existingUser, expect.any(Function)); @@ -736,8 +736,8 @@ describe('createAppleOAuthCallbackHandler', () => { await handler(req as Request, res as Response, jest.fn()); - // Should try to find user by providerUserId - expect(deps.users.findOneByScan).toHaveBeenCalledWith({ providerUserId: 'apple-user-unknown' }); + // Should try to find user by providerUserId AND provider + expect(deps.users.findOneByScan).toHaveBeenCalledWith({ providerUserId: 'apple-user-unknown', provider: 'apple' }); // Should redirect with error since user not found and no email to create one expect(getRedirectUrl()).toBe('/?error=apple_no_email'); From 30ef1ccc72c0db19d96f7e75558d6d70cc9b887c Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 17:13:06 -0800 Subject: [PATCH 11/17] server: support multi-key queries in findByScan The findByScan method previously only supported single-key queries, which caused the provider-scoped providerUserId lookups to fail at runtime. Now we chain multiple .where() clauses in Firestore, allowing queries like { providerUserId: '...', provider: 'apple' } to work correctly. --- src/server/models/table-firestore.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/server/models/table-firestore.ts b/src/server/models/table-firestore.ts index 9b10aaf95..89fa23b3c 100644 --- a/src/server/models/table-firestore.ts +++ b/src/server/models/table-firestore.ts @@ -2,7 +2,7 @@ // Use of this source code is governed by the Apache License, // Version 2.0, that can be found in the LICENSE file. -import { CollectionReference, Firestore } from '@google-cloud/firestore'; +import { CollectionReference, Firestore, Query } from '@google-cloud/firestore'; import { FieldPath } from '@google-cloud/firestore/build/src'; import { Message } from 'google-protobuf'; @@ -69,11 +69,17 @@ export class FirestoreTable implements Table { async findByScan(query: any): Promise { const keys = Object.keys(query); - if (keys.length !== 1) { - throw new Error('findByScan: expected single query key'); + if (keys.length === 0) { + throw new Error('findByScan: expected at least one query key'); } - const key = keys[0]; - const querySnapshot = await this.collection.where(key, '==', query[key]).get(); + + // Build query with all conditions (Firestore supports chaining where clauses) + let firestoreQuery: Query = this.collection; + for (const key of keys) { + firestoreQuery = firestoreQuery.where(key, '==', query[key]); + } + + const querySnapshot = await firestoreQuery.get(); if (!querySnapshot || querySnapshot.empty) { return undefined; } From 31789a753cb2b3988c00e5bbbd39b1f75e444557 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 17:21:49 -0800 Subject: [PATCH 12/17] server: check Firebase disabled status in Apple no-email path When Apple omits email (common on returning users), we now check the Firebase account's disabled status before logging in. This prevents disabled accounts from bypassing the disabled check that exists in the email-present path. Adds test for the disabled check in the no-email login flow. --- src/server/auth/oauth-handlers.ts | 12 +++++ src/server/tests/oauth-handlers.test.ts | 60 +++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/server/auth/oauth-handlers.ts b/src/server/auth/oauth-handlers.ts index ce70b8181..40da74971 100644 --- a/src/server/auth/oauth-handlers.ts +++ b/src/server/auth/oauth-handlers.ts @@ -262,6 +262,18 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re // Include provider to prevent cross-provider collisions. let existingUser = await deps.users.findOneByScan({ providerUserId: claims.sub, provider: 'apple' }); if (existingUser) { + // Check if Firebase account is disabled before logging in + try { + const fbUser = await deps.firebaseAdmin.getUserByProviderUid('apple.com', claims.sub); + if (fbUser?.disabled) { + res.redirect('/?error=account_disabled'); + return; + } + } catch { + // If we can't find the Firebase user, proceed with login + // (the user might not have been created via Firebase) + } + await loginUser(req, existingUser); res.redirect(returnUrl); return; diff --git a/src/server/tests/oauth-handlers.test.ts b/src/server/tests/oauth-handlers.test.ts index 646ff0353..f7082d312 100644 --- a/src/server/tests/oauth-handlers.test.ts +++ b/src/server/tests/oauth-handlers.test.ts @@ -656,6 +656,60 @@ describe('createAppleOAuthCallbackHandler', () => { }); describe('returning user without email', () => { + it('should reject disabled Firebase users even in no-email path', async () => { + const deps = createAppleMockDeps(); + const handler = createAppleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/projects/test', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + // Mock Apple token exchange + (exchangeAppleCode as jest.Mock).mockResolvedValue({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }); + + // Mock verifyAppleIdToken to return claims WITHOUT email + (verifyAppleIdToken as jest.Mock).mockResolvedValue({ + sub: 'apple-disabled-user', + // no email + }); + + // User exists in local database by providerUserId + const existingUser = new User(); + existingUser.setId('user-disabled-123'); + existingUser.setEmail('disabled@example.com'); + existingUser.setProvider('apple'); + existingUser.setProviderUserId('apple-disabled-user'); + + (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(existingUser); + + // Firebase says user is disabled + (deps.firebaseAdmin as jest.Mocked).getUserByProviderUid.mockResolvedValue({ + uid: 'fb-disabled-user', + disabled: true, + } as admin.auth.UserRecord); + + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + // Should check Firebase disabled status + expect(deps.firebaseAdmin.getUserByProviderUid).toHaveBeenCalledWith('apple.com', 'apple-disabled-user'); + + // Should redirect with account disabled error + expect(getRedirectUrl()).toBe('/?error=account_disabled'); + + // Should NOT have called login + expect(req.login).not.toHaveBeenCalled(); + }); + it('should login existing user by providerUserId when Apple omits email', async () => { const deps = createAppleMockDeps(); const handler = createAppleOAuthCallbackHandler(deps); @@ -689,6 +743,12 @@ describe('createAppleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(existingUser); + // Firebase says user is NOT disabled + (deps.firebaseAdmin as jest.Mocked).getUserByProviderUid.mockResolvedValue({ + uid: 'fb-user-123', + disabled: false, + } as admin.auth.UserRecord); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); From 792266b3c3c7089fad0cad48b2fcbb7a65b24592 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 17:32:05 -0800 Subject: [PATCH 13/17] app: preserve current page URL in OAuth redirect When initiating Google or Apple OAuth, include the current page URL as the returnUrl query parameter. This ensures users return to where they started after authentication, rather than always landing on the home page. Fixes a regression from the Firebase signInWithRedirect migration. --- src/app/Login.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/app/Login.tsx b/src/app/Login.tsx index fcf452da1..5e9e063bb 100644 --- a/src/app/Login.tsx +++ b/src/app/Login.tsx @@ -68,10 +68,14 @@ export class Login extends React.Component { } appleLoginClick = () => { - window.location.href = '/auth/apple'; + const currentPath = window.location.pathname + window.location.search; + const returnUrl = encodeURIComponent(currentPath); + window.location.href = `/auth/apple?returnUrl=${returnUrl}`; }; googleLoginClick = () => { - window.location.href = '/auth/google'; + const currentPath = window.location.pathname + window.location.search; + const returnUrl = encodeURIComponent(currentPath); + window.location.href = `/auth/google?returnUrl=${returnUrl}`; }; emailLoginClick = () => { this.setState({ emailLoginFlow: 'showEmail' }); From 926822e389dd56cbdf0b18c90a7b41719b28423d Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 17:48:34 -0800 Subject: [PATCH 14/17] server: bind OAuth state to session to prevent login CSRF Store the OAuth state in the user's session when initiating the flow, and verify it matches on callback. This prevents an attacker from tricking a victim into logging into the attacker's account by having the victim visit a callback URL with the attacker's OAuth code and state. Both Google and Apple OAuth flows now: 1. Store state in req.session.oauthState during initiation 2. Verify req.session.oauthState matches the callback state 3. Clear oauthState from session after successful validation Adds comprehensive tests for the session state validation. --- src/server/auth/oauth-handlers.ts | 26 +++++++ src/server/tests/oauth-handlers.test.ts | 97 +++++++++++++++++++++---- 2 files changed, 110 insertions(+), 13 deletions(-) diff --git a/src/server/auth/oauth-handlers.ts b/src/server/auth/oauth-handlers.ts index 40da74971..85f797327 100644 --- a/src/server/auth/oauth-handlers.ts +++ b/src/server/auth/oauth-handlers.ts @@ -67,6 +67,9 @@ export function createGoogleOAuthInitiateHandler(deps: GoogleOAuthHandlerDeps): const returnUrl = typeof req.query.returnUrl === 'string' ? req.query.returnUrl : undefined; const state = await deps.stateStore.create(returnUrl); + // Store state in session to prevent login CSRF attacks + (req.session as Record).oauthState = state; + const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; const params = new URLSearchParams({ client_id: deps.config.clientId, @@ -113,8 +116,18 @@ export function createGoogleOAuthCallbackHandler(deps: GoogleOAuthHandlerDeps): return; } + // Verify state matches what was stored in session to prevent login CSRF + const sessionState = (req.session as Record)?.oauthState; + if (sessionState !== state) { + res.status(400).json({ error: 'State mismatch - possible CSRF attack' }); + return; + } + const returnUrl = validateReturnUrl(stateResult.returnUrl, deps.baseUrl); + // Clear oauthState from session after successful validation + delete (req.session as Record).oauthState; + await deps.stateStore.invalidate(state); const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; @@ -175,6 +188,9 @@ export function createAppleOAuthInitiateHandler(deps: AppleOAuthHandlerDeps): Re const returnUrl = typeof req.query.returnUrl === 'string' ? req.query.returnUrl : undefined; const state = await deps.stateStore.create(returnUrl); + // Store state in session to prevent login CSRF attacks + (req.session as Record).oauthState = state; + const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; const params = new URLSearchParams({ client_id: deps.config.clientId, @@ -220,8 +236,18 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re return; } + // Verify state matches what was stored in session to prevent login CSRF + const sessionState = (req.session as Record)?.oauthState; + if (sessionState !== state) { + res.status(400).json({ error: 'State mismatch - possible CSRF attack' }); + return; + } + const returnUrl = validateReturnUrl(stateResult.returnUrl, deps.baseUrl); + // Clear oauthState from session after successful validation + delete (req.session as Record).oauthState; + await deps.stateStore.invalidate(state); const clientSecret = generateAppleClientSecret( diff --git a/src/server/tests/oauth-handlers.test.ts b/src/server/tests/oauth-handlers.test.ts index f7082d312..d78ab90db 100644 --- a/src/server/tests/oauth-handlers.test.ts +++ b/src/server/tests/oauth-handlers.test.ts @@ -68,11 +68,16 @@ function createMockUsers(): jest.Mocked> { }; } -function createMockRequest(query: Record = {}, body: Record = {}): Partial { +function createMockRequest( + query: Record = {}, + body: Record = {}, + session: Record = {}, +): Partial { const loginFn = jest.fn((user: unknown, cb: (err?: Error) => void) => cb()); return { query, body, + session: session as Request['session'], login: loginFn as unknown as Request['login'], }; } @@ -212,6 +217,21 @@ describe('createGoogleOAuthInitiateHandler', () => { const redirectUrl = getRedirectUrl()!; expect(redirectUrl).toContain('redirect_uri=https%3A%2F%2Fapp.simlin.com%2Fauth%2Fgoogle%2Fcallback'); }); + + it('should store state in session to prevent login CSRF', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthInitiateHandler(deps); + + (deps.stateStore as jest.Mocked).create.mockResolvedValue('test-state-csrf'); + + const session: Record = {}; + const req = createMockRequest({}, {}, session); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(session.oauthState).toBe('test-state-csrf'); + }); }); describe('createGoogleOAuthCallbackHandler', () => { @@ -248,6 +268,38 @@ describe('createGoogleOAuthCallbackHandler', () => { expect(getBody()).toEqual({ error: 'Invalid or expired state' }); }); + it('should return 400 when session state does not match to prevent login CSRF', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ valid: true }); + + const session = { oauthState: 'different-state' }; + const req = createMockRequest({ code: 'test-code', state: 'attacker-state' }, {}, session); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'State mismatch - possible CSRF attack' }); + }); + + it('should return 400 when session has no oauthState', async () => { + const deps = createMockDeps(); + const handler = createGoogleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ valid: true }); + + const session: Record = {}; + const req = createMockRequest({ code: 'test-code', state: 'test-state' }, {}, session); + const { res, getStatus, getBody } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(getStatus()).toBe(400); + expect(getBody()).toEqual({ error: 'State mismatch - possible CSRF attack' }); + }); + it('should invalidate state after successful use', async () => { const deps = createMockDeps(); const handler = createGoogleOAuthCallbackHandler(deps); @@ -287,7 +339,7 @@ describe('createGoogleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); (deps.users as jest.Mocked>).create.mockResolvedValue(); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); const { res } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -335,7 +387,7 @@ describe('createGoogleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); (deps.users as jest.Mocked>).create.mockResolvedValue(); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -381,7 +433,7 @@ describe('createGoogleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); (deps.users as jest.Mocked>).create.mockResolvedValue(); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -429,7 +481,7 @@ describe('createGoogleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); (deps.users as jest.Mocked>).create.mockResolvedValue(); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); const { res } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -478,7 +530,7 @@ describe('createGoogleOAuthCallbackHandler', () => { createdUser = user; }); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); const { res } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -505,7 +557,7 @@ describe('createGoogleOAuthCallbackHandler', () => { text: async () => 'Invalid code', }); - const req = createMockRequest({ code: 'invalid-code', state: 'valid-state' }); + const req = createMockRequest({ code: 'invalid-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -566,7 +618,7 @@ describe('createGoogleOAuthCallbackHandler', () => { disabled: true, } as admin.auth.UserRecord); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -604,6 +656,25 @@ function createAppleMockDeps(): AppleOAuthHandlerDeps { }; } +describe('createAppleOAuthInitiateHandler', () => { + it('should store state in session to prevent login CSRF', async () => { + const { createAppleOAuthInitiateHandler } = await import('../auth/oauth-handlers'); + + const deps = createAppleMockDeps(); + const handler = createAppleOAuthInitiateHandler(deps); + + (deps.stateStore as jest.Mocked).create.mockResolvedValue('apple-state-csrf'); + + const session: Record = {}; + const req = createMockRequest({}, {}, session); + const { res } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + expect(session.oauthState).toBe('apple-state-csrf'); + }); +}); + describe('createAppleOAuthCallbackHandler', () => { beforeEach(() => { mockFetch.mockReset(); @@ -642,7 +713,7 @@ describe('createAppleOAuthCallbackHandler', () => { disabled: true, } as admin.auth.UserRecord); - const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }, { oauthState: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -695,7 +766,7 @@ describe('createAppleOAuthCallbackHandler', () => { disabled: true, } as admin.auth.UserRecord); - const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }, { oauthState: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -749,7 +820,7 @@ describe('createAppleOAuthCallbackHandler', () => { disabled: false, } as admin.auth.UserRecord); - const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }, { oauthState: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -791,7 +862,7 @@ describe('createAppleOAuthCallbackHandler', () => { // User does NOT exist in local database (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); - const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }, { oauthState: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -850,7 +921,7 @@ describe('createAppleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).update.mockResolvedValue(existingUser); - const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }, { oauthState: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); From 5e0f947411416aec665421736a1c69cfddf40350 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 18:02:27 -0800 Subject: [PATCH 15/17] server: remove session-based OAuth state check; preserve provider IDs The session-based OAuth state check breaks OAuth flows because SameSite cookie restrictions prevent the session cookie from being sent on cross-site redirects from Apple/Google. The Firestore-based state store provides sufficient CSRF protection: it uses cryptographically random 32-byte tokens, single-use invalidation, and a 10-minute TTL. Also fixes account linking to preserve existing providerUserId when a user signs in with a different provider. Previously, signing in with Google after creating an account with Apple would overwrite the Apple providerUserId, breaking future Apple re-logins (which often omit email). Now we only set providerUserId if it's not already set. Adds test for the provider preservation behavior. --- src/server/auth/oauth-handlers.ts | 26 ------- src/server/authn.ts | 7 +- src/server/tests/authn.test.ts | 34 +++++++++ src/server/tests/oauth-handlers.test.ts | 96 ++++--------------------- 4 files changed, 52 insertions(+), 111 deletions(-) diff --git a/src/server/auth/oauth-handlers.ts b/src/server/auth/oauth-handlers.ts index 85f797327..40da74971 100644 --- a/src/server/auth/oauth-handlers.ts +++ b/src/server/auth/oauth-handlers.ts @@ -67,9 +67,6 @@ export function createGoogleOAuthInitiateHandler(deps: GoogleOAuthHandlerDeps): const returnUrl = typeof req.query.returnUrl === 'string' ? req.query.returnUrl : undefined; const state = await deps.stateStore.create(returnUrl); - // Store state in session to prevent login CSRF attacks - (req.session as Record).oauthState = state; - const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; const params = new URLSearchParams({ client_id: deps.config.clientId, @@ -116,18 +113,8 @@ export function createGoogleOAuthCallbackHandler(deps: GoogleOAuthHandlerDeps): return; } - // Verify state matches what was stored in session to prevent login CSRF - const sessionState = (req.session as Record)?.oauthState; - if (sessionState !== state) { - res.status(400).json({ error: 'State mismatch - possible CSRF attack' }); - return; - } - const returnUrl = validateReturnUrl(stateResult.returnUrl, deps.baseUrl); - // Clear oauthState from session after successful validation - delete (req.session as Record).oauthState; - await deps.stateStore.invalidate(state); const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; @@ -188,9 +175,6 @@ export function createAppleOAuthInitiateHandler(deps: AppleOAuthHandlerDeps): Re const returnUrl = typeof req.query.returnUrl === 'string' ? req.query.returnUrl : undefined; const state = await deps.stateStore.create(returnUrl); - // Store state in session to prevent login CSRF attacks - (req.session as Record).oauthState = state; - const redirectUri = `${deps.baseUrl}${deps.config.callbackPath}`; const params = new URLSearchParams({ client_id: deps.config.clientId, @@ -236,18 +220,8 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re return; } - // Verify state matches what was stored in session to prevent login CSRF - const sessionState = (req.session as Record)?.oauthState; - if (sessionState !== state) { - res.status(400).json({ error: 'State mismatch - possible CSRF attack' }); - return; - } - const returnUrl = validateReturnUrl(stateResult.returnUrl, deps.baseUrl); - // Clear oauthState from session after successful validation - delete (req.session as Record).oauthState; - await deps.stateStore.invalidate(state); const clientSecret = generateAppleClientSecret( diff --git a/src/server/authn.ts b/src/server/authn.ts index 913618574..75fc139f6 100644 --- a/src/server/authn.ts +++ b/src/server/authn.ts @@ -195,9 +195,12 @@ export async function getOrCreateUserFromVerifiedInfo( matchedByEmail = true; } } - if (user && matchedByEmail && info.providerUserId && user.getProviderUserId() !== info.providerUserId) { - // User was found by email but has different (or no) providerUserId. + if (user && matchedByEmail && info.providerUserId && !user.getProviderUserId()) { + // User was found by email but has no providerUserId set. // Update to use the new provider info so future logins without email work. + // Only do this if the user doesn't already have a providerUserId - we don't + // want to overwrite an existing provider link as that would break re-login + // via the original provider (e.g., Apple often omits email on re-login). user.setProviderUserId(info.providerUserId); user.setProvider(info.provider); await users.update(user.getId(), {}, user); diff --git a/src/server/tests/authn.test.ts b/src/server/tests/authn.test.ts index 42d7478ed..fd12e2328 100644 --- a/src/server/tests/authn.test.ts +++ b/src/server/tests/authn.test.ts @@ -120,6 +120,40 @@ describe('getOrCreateUserFromVerifiedInfo', () => { // Should NOT have updated since providerUserId already matches expect(users.update).not.toHaveBeenCalled(); }); + + it('should preserve existing providerUserId when signing in with different provider', async () => { + const users = createMockUsers(); + + // User originally signed up with Apple + const existingUser = new User(); + existingUser.setId('user-123'); + existingUser.setEmail('test@example.com'); + existingUser.setProvider('apple'); + existingUser.setProviderUserId('apple-sub-original'); + + // First lookup by providerUserId+provider fails (different provider) + // Second lookup by email succeeds + users.findOneByScan.mockResolvedValueOnce(undefined).mockResolvedValueOnce(existingUser); + + const info: VerifiedUserInfo = { + email: 'test@example.com', + displayName: 'Test User', + provider: 'google', + providerUserId: 'google-sub-new', + }; + + const [user, err] = await getOrCreateUserFromVerifiedInfo(users, info); + + expect(err).toBeUndefined(); + expect(user).toBeDefined(); + + // Should NOT have updated - preserving Apple providerUserId for re-login + expect(users.update).not.toHaveBeenCalled(); + + // User should still have original Apple provider info + expect(existingUser.getProviderUserId()).toBe('apple-sub-original'); + expect(existingUser.getProvider()).toBe('apple'); + }); }); describe('when no user exists', () => { diff --git a/src/server/tests/oauth-handlers.test.ts b/src/server/tests/oauth-handlers.test.ts index d78ab90db..d7589eec4 100644 --- a/src/server/tests/oauth-handlers.test.ts +++ b/src/server/tests/oauth-handlers.test.ts @@ -68,16 +68,11 @@ function createMockUsers(): jest.Mocked> { }; } -function createMockRequest( - query: Record = {}, - body: Record = {}, - session: Record = {}, -): Partial { +function createMockRequest(query: Record = {}, body: Record = {}): Partial { const loginFn = jest.fn((user: unknown, cb: (err?: Error) => void) => cb()); return { query, body, - session: session as Request['session'], login: loginFn as unknown as Request['login'], }; } @@ -218,20 +213,6 @@ describe('createGoogleOAuthInitiateHandler', () => { expect(redirectUrl).toContain('redirect_uri=https%3A%2F%2Fapp.simlin.com%2Fauth%2Fgoogle%2Fcallback'); }); - it('should store state in session to prevent login CSRF', async () => { - const deps = createMockDeps(); - const handler = createGoogleOAuthInitiateHandler(deps); - - (deps.stateStore as jest.Mocked).create.mockResolvedValue('test-state-csrf'); - - const session: Record = {}; - const req = createMockRequest({}, {}, session); - const { res } = createMockResponse(); - - await handler(req as Request, res as Response, jest.fn()); - - expect(session.oauthState).toBe('test-state-csrf'); - }); }); describe('createGoogleOAuthCallbackHandler', () => { @@ -268,38 +249,6 @@ describe('createGoogleOAuthCallbackHandler', () => { expect(getBody()).toEqual({ error: 'Invalid or expired state' }); }); - it('should return 400 when session state does not match to prevent login CSRF', async () => { - const deps = createMockDeps(); - const handler = createGoogleOAuthCallbackHandler(deps); - - (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ valid: true }); - - const session = { oauthState: 'different-state' }; - const req = createMockRequest({ code: 'test-code', state: 'attacker-state' }, {}, session); - const { res, getStatus, getBody } = createMockResponse(); - - await handler(req as Request, res as Response, jest.fn()); - - expect(getStatus()).toBe(400); - expect(getBody()).toEqual({ error: 'State mismatch - possible CSRF attack' }); - }); - - it('should return 400 when session has no oauthState', async () => { - const deps = createMockDeps(); - const handler = createGoogleOAuthCallbackHandler(deps); - - (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ valid: true }); - - const session: Record = {}; - const req = createMockRequest({ code: 'test-code', state: 'test-state' }, {}, session); - const { res, getStatus, getBody } = createMockResponse(); - - await handler(req as Request, res as Response, jest.fn()); - - expect(getStatus()).toBe(400); - expect(getBody()).toEqual({ error: 'State mismatch - possible CSRF attack' }); - }); - it('should invalidate state after successful use', async () => { const deps = createMockDeps(); const handler = createGoogleOAuthCallbackHandler(deps); @@ -339,7 +288,7 @@ describe('createGoogleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); (deps.users as jest.Mocked>).create.mockResolvedValue(); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); const { res } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -387,7 +336,7 @@ describe('createGoogleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); (deps.users as jest.Mocked>).create.mockResolvedValue(); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -433,7 +382,7 @@ describe('createGoogleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); (deps.users as jest.Mocked>).create.mockResolvedValue(); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -481,7 +430,7 @@ describe('createGoogleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); (deps.users as jest.Mocked>).create.mockResolvedValue(); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); const { res } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -530,7 +479,7 @@ describe('createGoogleOAuthCallbackHandler', () => { createdUser = user; }); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); const { res } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -557,7 +506,7 @@ describe('createGoogleOAuthCallbackHandler', () => { text: async () => 'Invalid code', }); - const req = createMockRequest({ code: 'invalid-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); + const req = createMockRequest({ code: 'invalid-code', state: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -618,7 +567,7 @@ describe('createGoogleOAuthCallbackHandler', () => { disabled: true, } as admin.auth.UserRecord); - const req = createMockRequest({ code: 'test-code', state: 'valid-state' }, {}, { oauthState: 'valid-state' }); + const req = createMockRequest({ code: 'test-code', state: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -656,25 +605,6 @@ function createAppleMockDeps(): AppleOAuthHandlerDeps { }; } -describe('createAppleOAuthInitiateHandler', () => { - it('should store state in session to prevent login CSRF', async () => { - const { createAppleOAuthInitiateHandler } = await import('../auth/oauth-handlers'); - - const deps = createAppleMockDeps(); - const handler = createAppleOAuthInitiateHandler(deps); - - (deps.stateStore as jest.Mocked).create.mockResolvedValue('apple-state-csrf'); - - const session: Record = {}; - const req = createMockRequest({}, {}, session); - const { res } = createMockResponse(); - - await handler(req as Request, res as Response, jest.fn()); - - expect(session.oauthState).toBe('apple-state-csrf'); - }); -}); - describe('createAppleOAuthCallbackHandler', () => { beforeEach(() => { mockFetch.mockReset(); @@ -713,7 +643,7 @@ describe('createAppleOAuthCallbackHandler', () => { disabled: true, } as admin.auth.UserRecord); - const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }, { oauthState: 'valid-state' }); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -766,7 +696,7 @@ describe('createAppleOAuthCallbackHandler', () => { disabled: true, } as admin.auth.UserRecord); - const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }, { oauthState: 'valid-state' }); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -820,7 +750,7 @@ describe('createAppleOAuthCallbackHandler', () => { disabled: false, } as admin.auth.UserRecord); - const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }, { oauthState: 'valid-state' }); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -862,7 +792,7 @@ describe('createAppleOAuthCallbackHandler', () => { // User does NOT exist in local database (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(undefined); - const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }, { oauthState: 'valid-state' }); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); @@ -921,7 +851,7 @@ describe('createAppleOAuthCallbackHandler', () => { (deps.users as jest.Mocked>).update.mockResolvedValue(existingUser); - const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }, { oauthState: 'valid-state' }); + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); const { res, getRedirectUrl } = createMockResponse(); await handler(req as Request, res as Response, jest.fn()); From ff3f786a73e4b216e3a6a5769b11d405f7633c88 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 18:14:29 -0800 Subject: [PATCH 16/17] server: update provider info when password user signs in with OAuth Password users have providerUserId set to their Firebase UID, which isn't useful for OAuth provider lookups. When a password user signs in with Apple or Google for the first time (with email), we now update their providerUserId to the OAuth provider's sub/ID. This enables subsequent OAuth logins that don't include email (common with Apple). The fix preserves the distinction: OAuth-to-OAuth changes are still blocked to protect re-login via the original OAuth provider. --- src/server/authn.ts | 21 +++++++++++-------- src/server/tests/authn.test.ts | 38 ++++++++++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/server/authn.ts b/src/server/authn.ts index 75fc139f6..2bd3ac271 100644 --- a/src/server/authn.ts +++ b/src/server/authn.ts @@ -195,15 +195,18 @@ export async function getOrCreateUserFromVerifiedInfo( matchedByEmail = true; } } - if (user && matchedByEmail && info.providerUserId && !user.getProviderUserId()) { - // User was found by email but has no providerUserId set. - // Update to use the new provider info so future logins without email work. - // Only do this if the user doesn't already have a providerUserId - we don't - // want to overwrite an existing provider link as that would break re-login - // via the original provider (e.g., Apple often omits email on re-login). - user.setProviderUserId(info.providerUserId); - user.setProvider(info.provider); - await users.update(user.getId(), {}, user); + if (user && matchedByEmail && info.providerUserId) { + const existingProvider = user.getProvider(); + // Update if: user has no providerUserId, OR existing provider is 'password' + // (password provider uses Firebase UID as providerUserId, not useful for lookups). + // DON'T update if existing provider is an OAuth provider (google/apple) - + // that would break re-login via the original OAuth provider since they + // often omit email on subsequent logins. + if (!user.getProviderUserId() || existingProvider === 'password') { + user.setProviderUserId(info.providerUserId); + user.setProvider(info.provider); + await users.update(user.getId(), {}, user); + } } if (!user) { const created = new Timestamp(); diff --git a/src/server/tests/authn.test.ts b/src/server/tests/authn.test.ts index fd12e2328..a1ce8d125 100644 --- a/src/server/tests/authn.test.ts +++ b/src/server/tests/authn.test.ts @@ -121,10 +121,10 @@ describe('getOrCreateUserFromVerifiedInfo', () => { expect(users.update).not.toHaveBeenCalled(); }); - it('should preserve existing providerUserId when signing in with different provider', async () => { + it('should preserve existing providerUserId when signing in with different OAuth provider', async () => { const users = createMockUsers(); - // User originally signed up with Apple + // User originally signed up with Apple (OAuth provider) const existingUser = new User(); existingUser.setId('user-123'); existingUser.setEmail('test@example.com'); @@ -154,6 +154,40 @@ describe('getOrCreateUserFromVerifiedInfo', () => { expect(existingUser.getProviderUserId()).toBe('apple-sub-original'); expect(existingUser.getProvider()).toBe('apple'); }); + + it('should update providerUserId when password user signs in with OAuth', async () => { + const users = createMockUsers(); + + // User originally signed up with password (providerUserId is Firebase UID) + const existingUser = new User(); + existingUser.setId('user-123'); + existingUser.setEmail('test@example.com'); + existingUser.setProvider('password'); + existingUser.setProviderUserId('firebase-uid-123'); + + // First lookup by providerUserId+provider fails (different provider) + // Second lookup by email succeeds + users.findOneByScan.mockResolvedValueOnce(undefined).mockResolvedValueOnce(existingUser); + users.update.mockResolvedValue(existingUser); + + const info: VerifiedUserInfo = { + email: 'test@example.com', + displayName: 'Test User', + provider: 'apple', + providerUserId: 'apple-sub-123', + }; + + const [user, err] = await getOrCreateUserFromVerifiedInfo(users, info); + + expect(err).toBeUndefined(); + expect(user).toBeDefined(); + + // Should have updated since existing provider is 'password' + expect(users.update).toHaveBeenCalledWith('user-123', {}, expect.any(User)); + const updatedUser = users.update.mock.calls[0][2] as User; + expect(updatedUser.getProviderUserId()).toBe('apple-sub-123'); + expect(updatedUser.getProvider()).toBe('apple'); + }); }); describe('when no user exists', () => { From 8cd08bdee273889a71df8d1ee502309bbd19964f Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Tue, 3 Feb 2026 18:26:43 -0800 Subject: [PATCH 17/17] server: fallback to email check when Apple provider lookup fails When checking if a user is disabled in the no-email Apple path, we now fallback to getUserByEmail if getUserByProviderUid fails. This handles the case where a user was created via createUser during first Apple login but doesn't have an Apple provider link in Firebase. Without this fix, disabled accounts could bypass the check if the Firebase provider lookup throws an error. --- src/server/auth/oauth-handlers.ts | 22 +++++++--- src/server/tests/oauth-handlers.test.ts | 58 +++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/server/auth/oauth-handlers.ts b/src/server/auth/oauth-handlers.ts index 40da74971..3135f3994 100644 --- a/src/server/auth/oauth-handlers.ts +++ b/src/server/auth/oauth-handlers.ts @@ -263,15 +263,25 @@ export function createAppleOAuthCallbackHandler(deps: AppleOAuthHandlerDeps): Re let existingUser = await deps.users.findOneByScan({ providerUserId: claims.sub, provider: 'apple' }); if (existingUser) { // Check if Firebase account is disabled before logging in + let isDisabled = false; try { const fbUser = await deps.firebaseAdmin.getUserByProviderUid('apple.com', claims.sub); - if (fbUser?.disabled) { - res.redirect('/?error=account_disabled'); - return; - } + isDisabled = fbUser?.disabled ?? false; } catch { - // If we can't find the Firebase user, proceed with login - // (the user might not have been created via Firebase) + // If provider lookup fails, fallback to email lookup + if (existingUser.getEmail()) { + try { + const fbUser = await deps.firebaseAdmin.getUserByEmail(existingUser.getEmail()); + isDisabled = fbUser?.disabled ?? false; + } catch { + // If neither lookup works, proceed with login + } + } + } + + if (isDisabled) { + res.redirect('/?error=account_disabled'); + return; } await loginUser(req, existingUser); diff --git a/src/server/tests/oauth-handlers.test.ts b/src/server/tests/oauth-handlers.test.ts index d7589eec4..b9abd2d0e 100644 --- a/src/server/tests/oauth-handlers.test.ts +++ b/src/server/tests/oauth-handlers.test.ts @@ -711,6 +711,64 @@ describe('createAppleOAuthCallbackHandler', () => { expect(req.login).not.toHaveBeenCalled(); }); + it('should fallback to email check when provider lookup fails and block disabled users', async () => { + const deps = createAppleMockDeps(); + const handler = createAppleOAuthCallbackHandler(deps); + + (deps.stateStore as jest.Mocked).validate.mockResolvedValue({ + valid: true, + returnUrl: '/projects/test', + }); + (deps.stateStore as jest.Mocked).invalidate.mockResolvedValue(); + + (exchangeAppleCode as jest.Mock).mockResolvedValue({ + access_token: 'test-access-token', + id_token: 'test-id-token', + expires_in: 3600, + token_type: 'Bearer', + }); + + // Apple ID token without email + (verifyAppleIdToken as jest.Mock).mockResolvedValue({ + sub: 'apple-user-no-link', + }); + + // User exists in local database + const existingUser = new User(); + existingUser.setId('user-123'); + existingUser.setEmail('disabled@example.com'); + existingUser.setProvider('apple'); + existingUser.setProviderUserId('apple-user-no-link'); + + (deps.users as jest.Mocked>).findOneByScan.mockResolvedValue(existingUser); + + // getUserByProviderUid throws (no Apple provider link in Firebase) + (deps.firebaseAdmin as jest.Mocked).getUserByProviderUid.mockRejectedValue( + new Error('User not found'), + ); + + // getUserByEmail finds the user but they're disabled + (deps.firebaseAdmin as jest.Mocked).getUserByEmail.mockResolvedValue({ + uid: 'fb-user-123', + email: 'disabled@example.com', + disabled: true, + } as admin.auth.UserRecord); + + const req = createMockRequest({}, { code: 'test-code', state: 'valid-state' }); + const { res, getRedirectUrl } = createMockResponse(); + + await handler(req as Request, res as Response, jest.fn()); + + // Should fallback to email check + expect(deps.firebaseAdmin.getUserByEmail).toHaveBeenCalledWith('disabled@example.com'); + + // Should redirect with account disabled error + expect(getRedirectUrl()).toBe('/?error=account_disabled'); + + // Should NOT have called login + expect(req.login).not.toHaveBeenCalled(); + }); + it('should login existing user by providerUserId when Apple omits email', async () => { const deps = createAppleMockDeps(); const handler = createAppleOAuthCallbackHandler(deps);