From 490ab39da33297863a3fed983c0cf9f4187cdb08 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 15 Mar 2026 00:53:30 +0900 Subject: [PATCH 1/5] feat: add password reset for default admin/user accounts --- src/db/index.ts | 2 + src/db/types.ts | 2 + src/service/passport/local.ts | 47 +++- src/service/passport/passwordChangeHandler.ts | 27 +++ src/service/passport/types.ts | 8 + src/service/routes/auth.ts | 107 ++++++++- src/service/routes/index.ts | 8 +- src/service/routes/utils.ts | 11 +- src/ui/components/RouteGuard/RouteGuard.tsx | 4 + src/ui/views/Login/Login.tsx | 194 ++++++++-------- .../Login/components/AuthFormSection.tsx | 213 ++++++++++++++++++ test/services/passport/local.test.ts | 181 +++++++++++++++ test/services/routes/auth.test.ts | 101 ++++++++- test/testDb.test.ts | 1 + tests/e2e/password-reset.test.ts | 152 +++++++++++++ 15 files changed, 955 insertions(+), 103 deletions(-) create mode 100644 src/service/passport/passwordChangeHandler.ts create mode 100644 src/ui/views/Login/components/AuthFormSection.tsx create mode 100644 test/services/passport/local.test.ts create mode 100644 tests/e2e/password-reset.test.ts diff --git a/src/db/index.ts b/src/db/index.ts index f9048fb3b..6dd997c66 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -60,6 +60,7 @@ export const createUser = async ( gitAccount: string, admin: boolean = false, oidcId: string = '', + mustChangePassword: boolean = false, ) => { console.log( `creating user @@ -76,6 +77,7 @@ export const createUser = async ( gitAccount: gitAccount, email: email, admin: admin, + mustChangePassword, }; if (isBlank(username)) { diff --git a/src/db/types.ts b/src/db/types.ts index 74ead38b5..d9bd834e3 100644 --- a/src/db/types.ts +++ b/src/db/types.ts @@ -74,6 +74,7 @@ export class User { gitAccount: string; email: string; admin: boolean; + mustChangePassword?: boolean; oidcId?: string | null; displayName?: string | null; title?: string | null; @@ -105,6 +106,7 @@ export interface PublicUser { title: string; gitAccount: string; admin: boolean; + mustChangePassword?: boolean; } export interface Sink { diff --git a/src/service/passport/local.ts b/src/service/passport/local.ts index d925b3c66..fdfa98220 100644 --- a/src/service/passport/local.ts +++ b/src/service/passport/local.ts @@ -18,9 +18,34 @@ import bcrypt from 'bcryptjs'; import { IVerifyOptions, Strategy as LocalStrategy } from 'passport-local'; import type { PassportStatic } from 'passport'; import * as db from '../../db'; - +import type { DefaultLocalUser } from './types'; export const type = 'local'; +const DEFAULT_LOCAL_USERS: DefaultLocalUser[] = [ + { + username: 'admin', + password: 'admin', + email: 'admin@place.com', + gitAccount: 'none', + admin: true, + }, + { + username: 'user', + password: 'user', + email: 'user@place.com', + gitAccount: 'none', + admin: false, + }, +]; + +const isProduction = (): boolean => process.env.NODE_ENV === 'production'; +const isKnownDefaultCredentialAttempt = (username: string, password: string): boolean => + DEFAULT_LOCAL_USERS.some( + (defaultUser) => + defaultUser.username.toLowerCase() === username.toLowerCase() && + defaultUser.password === password, + ); + // Dynamic import to always get the current db module instance // This is necessary for test environments where modules may be reset const getDb = () => import('../../db'); @@ -45,6 +70,19 @@ export const configure = async (passport: PassportStatic): Promise { ) => { const user = await db.findUser(username); if (!user) { - await db.createUser(username, password, email, type, isAdmin); + await db.createUser(username, password, email, type, isAdmin, '', isProduction()); } }; - await createIfNotExists('admin', 'admin', 'admin@place.com', 'none', true); - await createIfNotExists('user', 'user', 'user@place.com', 'none', false); + for (const u of DEFAULT_LOCAL_USERS) { + await createIfNotExists(u.username, u.password, u.email, u.gitAccount, u.admin); + } }; diff --git a/src/service/passport/passwordChangeHandler.ts b/src/service/passport/passwordChangeHandler.ts new file mode 100644 index 000000000..f6f2865f4 --- /dev/null +++ b/src/service/passport/passwordChangeHandler.ts @@ -0,0 +1,27 @@ +/** + * Copyright 2026 GitProxy Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { RequestHandler } from 'express'; +import { mustChangePassword } from '../routes/utils'; + +export const passwordChangeHandler: RequestHandler = (req, res, next) => { + if (mustChangePassword(req.user)) { + return res.status(428).send({ + message: 'Password change required before accessing this endpoint', + }); + } + return next(); +}; diff --git a/src/service/passport/types.ts b/src/service/passport/types.ts index 6adb86a21..9ebcee6a1 100644 --- a/src/service/passport/types.ts +++ b/src/service/passport/types.ts @@ -53,3 +53,11 @@ export type ADProfileJson = { }; export type ADVerifyCallback = (err: Error | null, user: ADProfile | null) => void; + +export type DefaultLocalUser = { + username: string; + password: string; + email: string; + gitAccount: string; + admin: boolean; +}; diff --git a/src/service/routes/auth.ts b/src/service/routes/auth.ts index 84ac60907..475923530 100644 --- a/src/service/routes/auth.ts +++ b/src/service/routes/auth.ts @@ -15,6 +15,7 @@ */ import express, { Request, Response, NextFunction } from 'express'; +import bcrypt from 'bcryptjs'; import { getPassport, authStrategies } from '../passport'; import { getAuthMethods } from '../../config'; @@ -25,7 +26,7 @@ import * as passportAD from '../passport/activeDirectory'; import { User } from '../../db/types'; import { AuthenticationElement } from '../../config/generated/config'; -import { isAdminUser, toPublicUser } from './utils'; +import { isAdminUser, mustChangePassword, toPublicUser } from './utils'; import { handleAndLogError } from '../../utils/errors'; const router = express.Router(); @@ -34,6 +35,32 @@ const passport = getPassport(); const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', GIT_PROXY_UI_PORT: uiPort = 3000 } = process.env; +const PASSWORD_MIN_LENGTH = 8; +const PASSWORD_CHANGE_ALLOWED_PATHS = new Set([ + '/', + '/config', + '/login', + '/logout', + '/profile', + '/change-password', + '/openidconnect', + '/openidconnect/callback', +]); + +router.use((req: Request, res: Response, next: NextFunction) => { + if (!mustChangePassword(req.user)) { + return next(); + } + + if (PASSWORD_CHANGE_ALLOWED_PATHS.has(req.path)) { + return next(); + } + + return res.status(428).send({ + message: 'Password change required before accessing this endpoint', + }); +}); + router.get('/', (_req: Request, res: Response) => { res.status(200).json({ login: { @@ -151,6 +178,84 @@ router.post('/logout', (req: Request, res: Response, next: NextFunction) => { res.send({ isAuth: req.isAuthenticated(), user: req.user }); }); +router.post('/change-password', async (req: Request, res: Response) => { + if (!req.user) { + res + .status(401) + .send({ + message: 'Not logged in', + }) + .end(); + return; + } + + const { currentPassword, newPassword } = req.body ?? {}; + if ( + typeof currentPassword !== 'string' || + typeof newPassword !== 'string' || + currentPassword.trim().length === 0 || + newPassword.trim().length < PASSWORD_MIN_LENGTH + ) { + res + .status(400) + .send({ + message: `currentPassword and newPassword are required, and newPassword must be at least ${PASSWORD_MIN_LENGTH} characters`, + }) + .end(); + return; + } + + if (currentPassword === newPassword) { + res + .status(400) + .send({ + message: 'newPassword must be different from currentPassword', + }) + .end(); + return; + } + + try { + const user = await db.findUser((req.user as User).username); + if (!user) { + res.status(404).send({ message: 'User not found' }).end(); + return; + } + + if (!user.password) { + res + .status(400) + .send({ message: 'Password changes are not supported for this account' }) + .end(); + return; + } + + const currentPasswordCorrect = await bcrypt.compare(currentPassword, user.password ?? ''); + if (!currentPasswordCorrect) { + res.status(401).send({ message: 'Current password is incorrect' }).end(); + return; + } + + const hashedPassword = await bcrypt.hash(newPassword, 10); + await db.updateUser({ + username: user.username, + password: hashedPassword, + mustChangePassword: false, + }); + (req.user as User).mustChangePassword = false; + + res.status(200).send({ message: 'Password updated successfully' }).end(); + } catch (error: unknown) { + const msg = handleAndLogError(error, 'Failed to update password'); + res + .status(500) + .send({ + message: msg, + }) + .end(); + } +}); + router.get('/profile', async (req: Request, res: Response) => { if (!req.user) { res diff --git a/src/service/routes/index.ts b/src/service/routes/index.ts index 80d6c315d..df9e7e59f 100644 --- a/src/service/routes/index.ts +++ b/src/service/routes/index.ts @@ -23,16 +23,18 @@ import users from './users'; import healthcheck from './healthcheck'; import config from './config'; import { jwtAuthHandler } from '../passport/jwtAuthHandler'; +import { passwordChangeHandler } from '../passport/passwordChangeHandler'; import { Proxy } from '../../proxy'; const routes = (proxy: Proxy) => { const router = express.Router(); + router.use('/api', home); router.use('/api/auth', auth.router); router.use('/api/v1/healthcheck', healthcheck); - router.use('/api/v1/push', jwtAuthHandler(), push); - router.use('/api/v1/repo', jwtAuthHandler(), repo(proxy)); - router.use('/api/v1/user', jwtAuthHandler(), users); + router.use('/api/v1/push', jwtAuthHandler(), passwordChangeHandler, push); + router.use('/api/v1/repo', jwtAuthHandler(), passwordChangeHandler, repo(proxy)); + router.use('/api/v1/user', jwtAuthHandler(), passwordChangeHandler, users); router.use('/api/v1/config', config); return router; }; diff --git a/src/service/routes/utils.ts b/src/service/routes/utils.ts index 83e548408..8f3cc376a 100644 --- a/src/service/routes/utils.ts +++ b/src/service/routes/utils.ts @@ -19,6 +19,7 @@ import { PublicUser, User as DbUser } from '../../db/types'; interface User extends Express.User { username: string; admin?: boolean; + mustChangePassword?: boolean; } export function isAdminUser(user?: Express.User): user is User & { admin: true } { @@ -26,7 +27,7 @@ export function isAdminUser(user?: Express.User): user is User & { admin: true } } export const toPublicUser = (user: DbUser): PublicUser => { - return { + const publicUser: PublicUser = { username: user.username || '', displayName: user.displayName || '', email: user.email || '', @@ -34,4 +35,12 @@ export const toPublicUser = (user: DbUser): PublicUser => { gitAccount: user.gitAccount || '', admin: user.admin || false, }; + if (user.mustChangePassword) { + publicUser.mustChangePassword = true; + } + return publicUser; +}; + +export const mustChangePassword = (user?: Express.User): boolean => { + return user !== null && user !== undefined && (user as User).mustChangePassword === true; }; diff --git a/src/ui/components/RouteGuard/RouteGuard.tsx b/src/ui/components/RouteGuard/RouteGuard.tsx index f4c8d6354..a05dc1924 100644 --- a/src/ui/components/RouteGuard/RouteGuard.tsx +++ b/src/ui/components/RouteGuard/RouteGuard.tsx @@ -57,6 +57,10 @@ const RouteGuard = ({ component: Component, fullRoutePath }: RouteGuardProps) => return ; } + if (user?.mustChangePassword) { + return ; + } + if (adminOnly && !user?.admin) { return ; } diff --git a/src/ui/views/Login/Login.tsx b/src/ui/views/Login/Login.tsx index 999ddd2fe..e7f15c8bc 100644 --- a/src/ui/views/Login/Login.tsx +++ b/src/ui/views/Login/Login.tsx @@ -16,27 +16,23 @@ import React, { useState, FormEvent, useEffect } from 'react'; import { useNavigate, Navigate } from 'react-router-dom'; -import FormControl from '@material-ui/core/FormControl'; -import InputLabel from '@material-ui/core/InputLabel'; import GridItem from '../../components/Grid/GridItem'; import GridContainer from '../../components/Grid/GridContainer'; -import Input from '@material-ui/core/Input'; -import Button from '../../components/CustomButtons/Button'; import Card from '../../components/Card/Card'; import CardHeader from '../../components/Card/CardHeader'; -import CardBody from '../../components/Card/CardBody'; -import CardFooter from '../../components/Card/CardFooter'; import axios, { AxiosError } from 'axios'; import logo from '../../assets/img/git-proxy.png'; -import { Badge, CircularProgress, FormLabel, Snackbar } from '@material-ui/core'; +import { Badge, Snackbar } from '@material-ui/core'; import { useAuth } from '../../auth/AuthProvider'; import { getBaseUrl } from '../../services/apiConfig'; import { getAxiosConfig, processAuthError } from '../../services/auth'; import { BackendResponse } from '../../types'; +import { PublicUser } from '../../../db/types'; +import AuthFormSection from './components/AuthFormSection'; interface LoginResponse { - username: string; - password: string; + message: string; + user: PublicUser; } const Login: React.FC = () => { @@ -51,6 +47,10 @@ const Login: React.FC = () => { const [isLoading, setIsLoading] = useState(false); const [authMethods, setAuthMethods] = useState([]); const [usernamePasswordMethod, setUsernamePasswordMethod] = useState(''); + const [requirePasswordChange, setRequirePasswordChange] = useState(false); + const [currentPasswordForChange, setCurrentPasswordForChange] = useState(''); + const [newPassword, setNewPassword] = useState(''); + const [confirmNewPassword, setConfirmNewPassword] = useState(''); useEffect(() => { const fetchAuthConfig = async () => { @@ -70,12 +70,28 @@ const Login: React.FC = () => { fetchAuthConfig(); }, []); + useEffect(() => { + if (authContext.user?.mustChangePassword) { + setRequirePasswordChange(true); + } + }, [authContext.user]); + function validateForm(): boolean { return ( username.length > 0 && username.length < 100 && password.length > 0 && password.length < 200 ); } + function validatePasswordChangeForm(): boolean { + return ( + currentPasswordForChange.length > 0 && + newPassword.length >= 8 && + confirmNewPassword.length >= 8 && + newPassword === confirmNewPassword && + newPassword !== currentPasswordForChange + ); + } + async function handleAuthMethodLogin(authMethod: string): Promise { const baseUrl = await getBaseUrl(); window.location.href = `${baseUrl}/api/auth/${authMethod}`; @@ -88,7 +104,18 @@ const Login: React.FC = () => { try { const baseUrl = await getBaseUrl(); const loginUrl = `${baseUrl}/api/auth/login`; - await axios.post(loginUrl, { username, password }, getAxiosConfig()); + const response = await axios.post( + loginUrl, + { username, password }, + getAxiosConfig(), + ); + if (response.data?.user?.mustChangePassword) { + setRequirePasswordChange(true); + setCurrentPasswordForChange(password); + setMessage('For security, you must change your password before continuing.'); + await authContext.refreshUser(); + return; + } window.sessionStorage.setItem('git.proxy.login', 'success'); setMessage('Success!'); setSuccess(true); @@ -112,6 +139,47 @@ const Login: React.FC = () => { } } + async function handlePasswordChangeSubmit(event: FormEvent): Promise { + event.preventDefault(); + setIsLoading(true); + + if (!validatePasswordChangeForm()) { + setMessage( + 'Please provide your current password and a new password (minimum 8 characters) that matches confirmation.', + ); + setIsLoading(false); + return; + } + + try { + const baseUrl = await getBaseUrl(); + await axios.post( + `${baseUrl}/api/auth/change-password`, + { + currentPassword: currentPasswordForChange, + newPassword, + }, + getAxiosConfig(), + ); + await authContext.refreshUser(); + setRequirePasswordChange(false); + setSuccess(true); + setMessage('Password updated successfully.'); + navigate('/dashboard/repo', { replace: true }); + } catch (error: unknown) { + if (error instanceof AxiosError) { + const backendMessage = + (error.response?.data as BackendResponse | undefined)?.message ?? + 'Unable to update password'; + setMessage(backendMessage); + } else { + setMessage('Unable to update password'); + } + } finally { + setIsLoading(false); + } + } + if (gitAccountError) { return ; } @@ -120,8 +188,12 @@ const Login: React.FC = () => { return ; } + if (authContext.user && !authContext.user.mustChangePassword && !requirePasswordChange) { + return ; + } + return ( -
+ { /> - {usernamePasswordMethod ? ( - - - - - Login - - - Username - setUsername(e.target.value)} - autoFocus - data-test='username' - /> - - - - - - - Password - setPassword(e.target.value)} - data-test='password' - /> - - - - - ) : ( - - - Username/password authentication is not enabled at this time. - - - )} - {/* Show login buttons if available (one on top of the other) */} - - {!isLoading ? ( - <> - {usernamePasswordMethod && ( - - )} - {authMethods.map((am) => ( - - ))} - - ) : ( -
- -
- )} - +
void; + onPasswordChange: (value: string) => void; + onCurrentPasswordForChangeChange: (value: string) => void; + onNewPasswordChange: (value: string) => void; + onConfirmNewPasswordChange: (value: string) => void; + onAuthMethodLogin: (authMethod: string) => void; +}; + +const AuthFormSection: React.FC = ({ + requirePasswordChange, + isLoading, + usernamePasswordMethod, + authMethods, + username, + password, + currentPasswordForChange, + newPassword, + confirmNewPassword, + canSubmitLogin, + canSubmitPasswordChange, + onUsernameChange, + onPasswordChange, + onCurrentPasswordForChangeChange, + onNewPasswordChange, + onConfirmNewPasswordChange, + onAuthMethodLogin, +}) => { + return ( + <> + {requirePasswordChange ? ( + + + + + Password change required + + + This account is using an insecure default password. Update it now to continue. + + + Current password + onCurrentPasswordForChangeChange(e.target.value)} + data-test='current-password' + /> + + + New password + onNewPasswordChange(e.target.value)} + data-test='new-password' + /> + + + Confirm new password + onConfirmNewPasswordChange(e.target.value)} + data-test='confirm-new-password' + /> + + + + + ) : usernamePasswordMethod ? ( + + + + + Login + + + Username + onUsernameChange(e.target.value)} + autoFocus + data-test='username' + /> + + + + + + + Password + onPasswordChange(e.target.value)} + data-test='password' + /> + + + + + ) : ( + + + Username/password authentication is not enabled at this time. + + + )} + + {!isLoading ? ( + <> + {requirePasswordChange ? ( + + ) : ( + usernamePasswordMethod && ( + + ) + )} + {!requirePasswordChange && + authMethods.map((authMethod) => ( + + ))} + + ) : ( +
+ +
+ )} + + + ); +}; + +export default AuthFormSection; diff --git a/test/services/passport/local.test.ts b/test/services/passport/local.test.ts new file mode 100644 index 000000000..65d904bfe --- /dev/null +++ b/test/services/passport/local.test.ts @@ -0,0 +1,181 @@ +/** + * Copyright 2026 GitProxy Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +describe('local auth defaults', () => { + const originalNodeEnv = process.env.NODE_ENV; + + beforeEach(() => { + vi.resetModules(); + }); + + afterEach(() => { + vi.clearAllMocks(); + vi.resetModules(); + process.env.NODE_ENV = originalNodeEnv; + }); + + it('creates default local users in production and marks them for password change', async () => { + process.env.NODE_ENV = 'production'; + + const dbStub = { + findUser: vi.fn().mockResolvedValue(null), + createUser: vi.fn().mockResolvedValue(undefined), + }; + vi.doMock('../../../src/db', () => dbStub); + + const { createDefaultAdmin } = await import('../../../src/service/passport/local'); + await createDefaultAdmin(); + + expect(dbStub.findUser).toHaveBeenCalledWith('admin'); + expect(dbStub.findUser).toHaveBeenCalledWith('user'); + expect(dbStub.createUser).toHaveBeenCalledWith( + 'admin', + 'admin', + 'admin@place.com', + 'none', + true, + '', + true, + ); + expect(dbStub.createUser).toHaveBeenCalledWith( + 'user', + 'user', + 'user@place.com', + 'none', + false, + '', + true, + ); + }); + + it('creates default local users outside production when missing', async () => { + process.env.NODE_ENV = 'test'; + + const dbStub = { + findUser: vi.fn().mockResolvedValue(null), + createUser: vi.fn().mockResolvedValue(undefined), + }; + vi.doMock('../../../src/db', () => dbStub); + + const { createDefaultAdmin } = await import('../../../src/service/passport/local'); + await createDefaultAdmin(); + + expect(dbStub.findUser).toHaveBeenCalledWith('admin'); + expect(dbStub.findUser).toHaveBeenCalledWith('user'); + expect(dbStub.createUser).toHaveBeenCalledWith( + 'admin', + 'admin', + 'admin@place.com', + 'none', + true, + '', + false, + ); + expect(dbStub.createUser).toHaveBeenCalledWith( + 'user', + 'user', + 'user@place.com', + 'none', + false, + '', + false, + ); + }); +}); + +describe('local auth login hardening', () => { + const originalNodeEnv = process.env.NODE_ENV; + let verifyCallback: + | (( + username: string, + password: string, + done: (err: unknown, user?: unknown, info?: unknown) => void, + ) => Promise) + | undefined; + + beforeEach(() => { + vi.resetModules(); + verifyCallback = undefined; + }); + + afterEach(() => { + vi.clearAllMocks(); + vi.resetModules(); + process.env.NODE_ENV = originalNodeEnv; + }); + + it('marks default-credential logins for password change in production', async () => { + process.env.NODE_ENV = 'production'; + + vi.doMock('bcryptjs', () => ({ + default: { + compare: vi.fn().mockResolvedValue(true), + }, + })); + + const dbStub = { + findUser: vi.fn().mockResolvedValue({ + username: 'admin', + password: 'hashed-admin', + email: 'admin@place.com', + gitAccount: 'none', + admin: true, + mustChangePassword: false, + }), + createUser: vi.fn(), + updateUser: vi.fn().mockResolvedValue(undefined), + }; + const passportStub = { + use: vi.fn(), + serializeUser: vi.fn(), + deserializeUser: vi.fn(), + }; + + vi.doMock('../../../src/db', () => dbStub); + vi.doMock('passport-local', () => ({ + Strategy: class { + constructor( + callback: ( + username: string, + password: string, + done: (err: unknown, user?: unknown, info?: unknown) => void, + ) => Promise, + ) { + verifyCallback = callback; + } + }, + })); + + const { configure } = await import('../../../src/service/passport/local'); + await configure(passportStub as any); + + expect(verifyCallback).toBeDefined(); + const done = vi.fn(); + await verifyCallback!('admin', 'admin', done); + + expect(dbStub.findUser).toHaveBeenCalledWith('admin'); + expect(dbStub.updateUser).toHaveBeenCalledWith({ + username: 'admin', + mustChangePassword: true, + }); + expect(done).toHaveBeenCalledTimes(1); + const [_err, user, info] = done.mock.calls[0]; + expect(info).toBeUndefined(); + expect((user as { mustChangePassword?: boolean }).mustChangePassword).toBe(true); + }); +}); diff --git a/test/services/routes/auth.test.ts b/test/services/routes/auth.test.ts index 4008cf9ed..5846d500a 100644 --- a/test/services/routes/auth.test.ts +++ b/test/services/routes/auth.test.ts @@ -20,13 +20,13 @@ import express, { Express, Request, Response } from 'express'; import authRoutes from '../../../src/service/routes/auth'; import * as db from '../../../src/db'; -const newApp = (username?: string): Express => { +const newApp = (username?: string, options?: { mustChangePassword?: boolean }): Express => { const app = express(); app.use(express.json()); if (username) { app.use((req, _res, next) => { - req.user = { username }; + req.user = { username, mustChangePassword: options?.mustChangePassword }; next(); }); } @@ -208,6 +208,103 @@ describe('Auth API', () => { title: '', }); }); + + it('should return 428 when password change is required', async () => { + const res = await request(newApp('alice', { mustChangePassword: true })) + .post('/auth/gitAccount') + .send({ + username: 'alice', + gitAccount: 'UPDATED_GIT_ACCOUNT', + }); + + expect(res.status).toBe(428); + expect(res.body).toEqual({ + message: 'Password change required before accessing this endpoint', + }); + }); + }); + + describe('POST /change-password', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should return 401 if user is not logged in', async () => { + const res = await request(newApp()).post('/auth/change-password').send({ + currentPassword: 'admin', + newPassword: 'new-password-123', + }); + + expect(res.status).toBe(401); + }); + + it('should return 400 for invalid payload', async () => { + const res = await request(newApp('alice')).post('/auth/change-password').send({ + currentPassword: 'admin', + newPassword: 'short', + }); + + expect(res.status).toBe(400); + }); + + it('should return 404 if user is not found', async () => { + vi.spyOn(db, 'findUser').mockResolvedValue(null); + + const res = await request(newApp('alice')).post('/auth/change-password').send({ + currentPassword: 'admin-password', + newPassword: 'new-password-123', + }); + + expect(res.status).toBe(404); + }); + + it('should return 401 when current password is incorrect', async () => { + const bcrypt = await import('bcryptjs'); + const hashedPassword = await bcrypt.default.hash('correct-password', 10); + vi.spyOn(db, 'findUser').mockResolvedValue({ + username: 'alice', + password: hashedPassword, + email: 'alice@example.com', + displayName: 'Alice Munro', + gitAccount: 'ORIGINAL_GIT_ACCOUNT', + admin: true, + title: '', + } as any); + + const res = await request(newApp('alice')).post('/auth/change-password').send({ + currentPassword: 'wrong-password', + newPassword: 'new-password-123', + }); + + expect(res.status).toBe(401); + }); + + it('should reset mustChangePassword after successful password update', async () => { + const bcrypt = await import('bcryptjs'); + const hashedPassword = await bcrypt.default.hash('admin', 10); + const updateUserSpy = vi.spyOn(db, 'updateUser').mockResolvedValue(); + vi.spyOn(db, 'findUser').mockResolvedValue({ + username: 'alice', + password: hashedPassword, + email: 'alice@example.com', + displayName: 'Alice Munro', + gitAccount: 'ORIGINAL_GIT_ACCOUNT', + admin: true, + title: '', + } as any); + + const res = await request(newApp('alice')).post('/auth/change-password').send({ + currentPassword: 'admin', + newPassword: 'new-password-123', + }); + + expect(res.status).toBe(200); + expect(updateUserSpy).toHaveBeenCalledWith({ + username: 'alice', + password: expect.any(String), + mustChangePassword: false, + }); + }); }); describe('loginSuccessHandler', () => { diff --git a/test/testDb.test.ts b/test/testDb.test.ts index 91fdb6eca..9b08aa026 100644 --- a/test/testDb.test.ts +++ b/test/testDb.test.ts @@ -41,6 +41,7 @@ const TEST_USER = { gitAccount: 'db-test-user', email: 'db-test@test.com', admin: true, + mustChangePassword: false, }; const TEST_PUSH = new Action( diff --git a/tests/e2e/password-reset.test.ts b/tests/e2e/password-reset.test.ts new file mode 100644 index 000000000..cf35e6a66 --- /dev/null +++ b/tests/e2e/password-reset.test.ts @@ -0,0 +1,152 @@ +/** + * Copyright 2026 GitProxy Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { MongoClient } from 'mongodb'; +import { testConfig } from './setup'; + +type LoginResponse = { + message: string; + user?: { + username: string; + mustChangePassword?: boolean; + }; +}; + +const getCookieHeader = (response: Response): string => { + const setCookie = response.headers.get('set-cookie'); + if (!setCookie) { + throw new Error('No session cookie returned by login endpoint'); + } + return setCookie.split(';')[0]; +}; + +const login = async ( + username: string, + password: string, +): Promise<{ response: Response; body: LoginResponse; cookie: string }> => { + const response = await fetch(`${testConfig.gitProxyUiUrl}/api/auth/login`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ username, password }), + }); + let body: LoginResponse = { message: '' }; + try { + body = (await response.json()) as LoginResponse; + } catch { + // ignore non-json error bodies + } + const cookie = response.ok ? getCookieHeader(response) : ''; + return { response, body, cookie }; +}; + +describe.sequential('Git Proxy E2E - Password reset flow', () => { + let mongoClient: MongoClient; + const mongoConnectionString = process.env.E2E_MONGO_URL || 'mongodb://localhost:27017/gitproxy'; + + beforeAll(async () => { + mongoClient = new MongoClient(mongoConnectionString); + await mongoClient.connect(); + }); + + afterAll(async () => { + await mongoClient.close(); + }); + + it( + 'forces password update before protected API access and allows access after reset', + async () => { + const now = Date.now(); + const username = `pwdreset-${now}`; + const initialPassword = `Initial-${now}-pass`; + const updatedPassword = `Updated-${now}-pass`; + const email = `${username}@example.com`; + + // Create a local user via admin API. + const adminLogin = await login('admin', 'admin'); + expect(adminLogin.response.status).toBe(200); + const createResponse = await fetch(`${testConfig.gitProxyUiUrl}/api/auth/create-user`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Cookie: adminLogin.cookie, + }, + body: JSON.stringify({ + username, + password: initialPassword, + email, + gitAccount: username, + admin: false, + }), + }); + expect(createResponse.status).toBe(201); + + // Simulate production first-login requirement by toggling the same server-side flag + // used by production default accounts. + const usersCollection = mongoClient.db('gitproxy').collection('users'); + await usersCollection.updateOne({ username }, { $set: { mustChangePassword: true } }); + + const userLogin = await login(username, initialPassword); + expect(userLogin.response.status).toBe(200); + expect(userLogin.body.user?.mustChangePassword).toBe(true); + + // Protected endpoints should be blocked until password change. + const blockedResponse = await fetch(`${testConfig.gitProxyUiUrl}/api/v1/repo`, { + method: 'GET', + headers: { + Cookie: userLogin.cookie, + }, + }); + expect(blockedResponse.status).toBe(428); + + // Change password through the dedicated endpoint. + const changePasswordResponse = await fetch( + `${testConfig.gitProxyUiUrl}/api/auth/change-password`, + { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Cookie: userLogin.cookie, + }, + body: JSON.stringify({ + currentPassword: initialPassword, + newPassword: updatedPassword, + }), + }, + ); + expect(changePasswordResponse.status).toBe(200); + + // Access should be restored after password update. + const unblockedResponse = await fetch(`${testConfig.gitProxyUiUrl}/api/v1/repo`, { + method: 'GET', + headers: { + Cookie: userLogin.cookie, + }, + }); + expect(unblockedResponse.status).toBe(200); + + // Old password no longer works. + const oldPasswordLogin = await login(username, initialPassword); + expect(oldPasswordLogin.response.status).toBe(401); + + // New password succeeds and no longer requires reset. + const newPasswordLogin = await login(username, updatedPassword); + expect(newPasswordLogin.response.status).toBe(200); + expect(newPasswordLogin.body.user?.mustChangePassword).not.toBe(true); + }, + testConfig.timeout * 2, + ); +}); From 725f8a16b64df3709c852d4d0abaaa89c8ce42cb Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 19 Mar 2026 15:02:27 +0900 Subject: [PATCH 2/5] fix: regenerate session after password change, remove unused query param --- src/service/routes/auth.ts | 6 ++++++ src/ui/components/RouteGuard/RouteGuard.tsx | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/service/routes/auth.ts b/src/service/routes/auth.ts index 475923530..ac9e6fbec 100644 --- a/src/service/routes/auth.ts +++ b/src/service/routes/auth.ts @@ -242,6 +242,12 @@ router.post('/change-password', async (req: Request, res: Response) => { password: hashedPassword, mustChangePassword: false, }); + + req.session?.regenerate((err: unknown) => { + handleAndLogError(err, 'Failed to regenerate session'); + res.status(500).send({ message: 'Failed to regenerate session' }).end(); + return; + }); (req.user as User).mustChangePassword = false; res.status(200).send({ message: 'Password updated successfully' }).end(); diff --git a/src/ui/components/RouteGuard/RouteGuard.tsx b/src/ui/components/RouteGuard/RouteGuard.tsx index a05dc1924..1b0d0ae94 100644 --- a/src/ui/components/RouteGuard/RouteGuard.tsx +++ b/src/ui/components/RouteGuard/RouteGuard.tsx @@ -58,7 +58,7 @@ const RouteGuard = ({ component: Component, fullRoutePath }: RouteGuardProps) => } if (user?.mustChangePassword) { - return ; + return ; } if (adminOnly && !user?.admin) { From bde1ef0d412e0a12b8cdc3391357d9a9d3e851f4 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 19 Mar 2026 16:13:07 +0900 Subject: [PATCH 3/5] test: increase auth routes edge case coverage --- src/service/routes/auth.ts | 11 ++--- test/services/routes/auth.test.ts | 78 +++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 8 deletions(-) diff --git a/src/service/routes/auth.ts b/src/service/routes/auth.ts index 876aa42e0..081178cab 100644 --- a/src/service/routes/auth.ts +++ b/src/service/routes/auth.ts @@ -332,16 +332,11 @@ router.post('/gitAccount', async (req: Request, res: Response) => { } user.gitAccount = req.body.gitAccount; - db.updateUser(user); - res.status(200).end(); + await db.updateUser(user); + return res.status(200).send({ message: 'Git account updated successfully' }).end(); } catch (error: unknown) { const msg = handleErrorAndLog(error, 'Failed to update git account'); - res - .status(500) - .send({ - message: msg, - }) - .end(); + return res.status(500).send({ message: msg }).end(); } }); diff --git a/test/services/routes/auth.test.ts b/test/services/routes/auth.test.ts index 5846d500a..88c6ef309 100644 --- a/test/services/routes/auth.test.ts +++ b/test/services/routes/auth.test.ts @@ -19,6 +19,8 @@ import request from 'supertest'; import express, { Express, Request, Response } from 'express'; import authRoutes from '../../../src/service/routes/auth'; import * as db from '../../../src/db'; +import * as config from '../../../src/config'; +import bcryptjs from 'bcryptjs'; const newApp = (username?: string, options?: { mustChangePassword?: boolean }): Express => { const app = express(); @@ -222,6 +224,25 @@ describe('Auth API', () => { message: 'Password change required before accessing this endpoint', }); }); + + it('should return 500 if an error occurs', async () => { + vi.spyOn(db, 'updateUser').mockRejectedValue(new Error('Error')); + vi.spyOn(db, 'findUser').mockResolvedValue({ + username: 'alice', + password: await bcryptjs.hash('secret-password', 10), + email: 'alice@example.com', + displayName: 'Alice Munro', + gitAccount: 'ORIGINAL_GIT_ACCOUNT', + admin: true, + title: '', + } as any); + const res = await request(newApp('alice')).post('/auth/gitAccount').send({ + username: 'alice', + gitAccount: 'UPDATED_GIT_ACCOUNT', + }); + expect(res.status).toBe(500); + expect(res.body).toEqual({ message: 'Failed to update git account: Error' }); + }); }); describe('POST /change-password', () => { @@ -305,6 +326,49 @@ describe('Auth API', () => { mustChangePassword: false, }); }); + + it('should return 400 if current password is the same as the new password', async () => { + const res = await request(newApp('alice')).post('/auth/change-password').send({ + currentPassword: 'secret-password', + newPassword: 'secret-password', + }); + + expect(res.status).toBe(400); + expect(res.body).toEqual({ message: 'newPassword must be different from currentPassword' }); + }); + + it('should return 400 if current password is missing (i.e: OIDC login)', async () => { + const res = await request(newApp('alice')).post('/auth/change-password').send({ + currentPassword: undefined, + newPassword: 'new-password-123', + }); + + expect(res.status).toBe(400); + expect(res.body).toEqual({ + message: + 'currentPassword and newPassword are required, and newPassword must be at least 8 characters', + }); + }); + + it('should return 500 if an error occurs', async () => { + vi.spyOn(db, 'updateUser').mockRejectedValue(new Error('Error')); + vi.spyOn(db, 'findUser').mockResolvedValue({ + username: 'alice', + password: await bcryptjs.hash('secret-password', 10), + email: 'alice@example.com', + displayName: 'Alice Munro', + gitAccount: 'ORIGINAL_GIT_ACCOUNT', + admin: true, + title: '', + } as any); + const res = await request(newApp('alice')).post('/auth/change-password').send({ + currentPassword: 'secret-password', + newPassword: 'new-password-123', + }); + + expect(res.status).toBe(500); + expect(res.body).toEqual({ message: 'Failed to update password: Error' }); + }); }); describe('loginSuccessHandler', () => { @@ -411,5 +475,19 @@ describe('Auth API', () => { otherMethods: [], }); }); + + it('should return null usernamePasswordMethod if no username/password auth method is enabled', async () => { + // Mock the getAuthMethods function to return an empty array + vi.spyOn(config, 'getAuthMethods').mockReturnValue([]); + + const res = await request(newApp()).get('/auth/config'); + expect(res.status).toBe(200); + expect(res.body.usernamePasswordMethod).toBeNull(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.resetModules(); + }); }); }); From bcc818e728480f7145da609abf8e21e8c7ba2db3 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 19 Mar 2026 16:55:02 +0900 Subject: [PATCH 4/5] test: add test for allowed routes edge case --- test/services/routes/auth.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/services/routes/auth.test.ts b/test/services/routes/auth.test.ts index 88c6ef309..0f25f42a2 100644 --- a/test/services/routes/auth.test.ts +++ b/test/services/routes/auth.test.ts @@ -225,6 +225,15 @@ describe('Auth API', () => { }); }); + it('should skip password change requirement if path is in PASSWORD_CHANGE_ALLOWED_PATHS', async () => { + const res = await request(newApp('alice', { mustChangePassword: true })).get('/auth/config'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + usernamePasswordMethod: 'local', + otherMethods: [], + }); + }); + it('should return 500 if an error occurs', async () => { vi.spyOn(db, 'updateUser').mockRejectedValue(new Error('Error')); vi.spyOn(db, 'findUser').mockResolvedValue({ From c71b28a8a2085884895081b4156483059ed07b10 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 20 Mar 2026 15:51:48 +0900 Subject: [PATCH 5/5] fix: remove session regeneration Removing this as testing is more trouble than it's worth (app is internal, /change-password only works with local auth and only used on first login) --- src/service/routes/auth.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/service/routes/auth.ts b/src/service/routes/auth.ts index 081178cab..530957dd8 100644 --- a/src/service/routes/auth.ts +++ b/src/service/routes/auth.ts @@ -242,11 +242,6 @@ router.post('/change-password', async (req: Request, res: Response) => { mustChangePassword: false, }); - req.session?.regenerate((err: unknown) => { - handleErrorAndLog(err, 'Failed to regenerate session'); - res.status(500).send({ message: 'Failed to regenerate session' }).end(); - return; - }); (req.user as User).mustChangePassword = false; res.status(200).send({ message: 'Password updated successfully' }).end();