fix(auth): make account lockout recoverable and visible

The lockout was checked before the password was verified, so a correct password on a locked
account returned the same generic invalid-credentials error as a wrong one. Users had no way to
tell the two apart, and the natural response was to keep retrying a password that was already
right.

Completing a password reset did not clear the lock either, so the one recovery path the UI
advertises appeared to fail for up to fifteen minutes after a successful reset.

Verify the password first and report the lockout, with the remaining wait, only when the supplied
password was correct. Someone guessing wrong passwords still gets the generic failure, so account
existence is not leaked. Clear the lock whenever a password is reset or changed, and give admins
an explicit unlock action for the case where waiting is not acceptable.
This commit is contained in:
neon
2026-08-09 05:05:50 +00:00
parent 9a349311cf
commit 6769bf4b0f
21 changed files with 366 additions and 58 deletions
+5 -28
View File
@@ -8,9 +8,7 @@ Closes #
- **Maintainer approval:** (link to the approval comment)
- **Assigned contributor:** @
- **Type:** bug fix | feature | refactor | docs | chore
- **UI changed:** yes | no
- **How I ran this:** `pnpm dev` | Docker | I did not run it
- **Evidence captured at commit:** (run `git rev-parse --short HEAD`)
## What changed
@@ -38,33 +36,13 @@ summarize it.
## Evidence
Upload a recording that proves the changed behavior works in the real client. Show the starting state,
the user action, and the final result. If the workflow crosses multiple clients, show the result in
each affected client.
Upload a video of yourself using the change in the app. Show what it looked like before, what you
did, and what happened.
Ready-for-review PRs without the required evidence will not be reviewed. Keep the PR in draft until
the evidence has been uploaded.
Record it yourself. Videos made by a model, or by a coding agent driving the app, do not count, and
the PR will be closed without review.
Examples:
- **Web:** Start from a page load, perform the affected workflow, and show the saved result after
refreshing the page.
- **KOReader:** Perform the affected workflow inside KOReader and show the result there. For sync
changes, also show the corresponding result in BookOrbit.
- **Kobo:** Perform the affected workflow on the device, run the sync, and show the resulting state on
the device and in BookOrbit where applicable.
- **Scanner, import, or metadata:** Add or modify a real book, run the operation through its normal
BookOrbit control, and show the resulting book state in the BookOrbit UI.
- **Server code used by a client:** Trigger the behavior from that client and show the result in the
client. Calling the endpoint directly is not end-to-end evidence.
"No UI changes," written testing claims, automated test output, logs, API requests, database queries,
and direct service calls are not evidence. They may support the recording but cannot replace it.
Evidence must match the final commit. If the workflow cannot be demonstrated in its real client or
device, keep the PR in draft and state what remains untested.
<!-- Upload the recording here. -->
<!-- Video here. -->
## Authorship and review
@@ -83,7 +61,6 @@ AI assistance is welcome on this project; unreviewed AI output is not. See the
- [ ] One focused change, linked above to an issue a maintainer approved
- [ ] I discussed my approach and was assigned the issue before writing code or opening this PR
- [ ] I read every line of the final diff
- [ ] `pnpm verify` passes against the final diff
- [ ] Tests added or updated per the [testing expectations](https://github.com/bookorbit/bookorbit/blob/main/docs/CONTRIBUTING.md#phase-7-write-and-run-tests)
- [ ] User-facing text uses Vue I18n keys added only to `client/src/locales/en.json`; non-English catalogs are unchanged, and accessibility was checked
+42 -4
View File
@@ -1,7 +1,7 @@
<script setup lang="ts">
import { computed, onMounted, ref } from 'vue'
import { useI18n } from 'vue-i18n'
import { KeyRound, MoreVertical, Pencil, Save, Search, ShieldAlert, ShieldCheck, Trash2, UserPlus } from '@lucide/vue'
import { KeyRound, LockOpen, MoreVertical, Pencil, Save, Search, ShieldAlert, ShieldCheck, Trash2, UserPlus } from '@lucide/vue'
import { Permission, type AuthUser, type DefaultLibraryAccessConfig } from '@bookorbit/types'
import { api } from '@/lib/api'
import { formatNumber } from '@/i18n/formatters'
@@ -123,6 +123,20 @@ function resetPasswordHint(user: UserRow): string {
return t('adminFeature.usersPage.resetPassword')
}
function isLocked(user: UserRow): boolean {
return Boolean(user.lockedUntil && new Date(user.lockedUntil).getTime() > Date.now())
}
async function handleUnlock(userId: number) {
actionError.value = null
const res = await api(`/api/v1/users/${userId}/unlock`, { method: 'POST' })
if (!res.ok) {
actionError.value = t('adminFeature.usersPage.errors.unlock')
return
}
await load()
}
async function handleResetPassword(userId: number) {
const res = await api(`/api/v1/users/${userId}/reset-password`, { method: 'POST' })
if (!res.ok) {
@@ -303,9 +317,14 @@ async function saveDefaultLibraryAccess() {
</div>
</td>
<td class="px-3 py-2">
<StatusPill :tone="user.active ? 'success' : 'danger'">
{{ user.active ? t('adminFeature.usersPage.statusActive') : t('adminFeature.usersPage.statusInactive') }}
</StatusPill>
<div class="flex flex-wrap items-center gap-1.5">
<StatusPill :tone="user.active ? 'success' : 'danger'">
{{ user.active ? t('adminFeature.usersPage.statusActive') : t('adminFeature.usersPage.statusInactive') }}
</StatusPill>
<StatusPill v-if="isLocked(user)" tone="warning">
{{ t('adminFeature.usersPage.lockedBadge') }}
</StatusPill>
</div>
</td>
<td class="px-3 py-2">
<div class="flex items-center justify-end gap-1">
@@ -323,6 +342,19 @@ async function saveDefaultLibraryAccess() {
</TooltipTrigger>
<TooltipContent>{{ t('common.edit') }}</TooltipContent>
</Tooltip>
<Tooltip v-if="isLocked(user)">
<TooltipTrigger as-child>
<button
type="button"
class="rounded p-1.5 text-muted-foreground transition-colors hover:bg-muted hover:text-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring"
:aria-label="t('adminFeature.usersPage.unlockAria', { name: user.name })"
@click="handleUnlock(user.id)"
>
<LockOpen :size="14" aria-hidden="true" />
</button>
</TooltipTrigger>
<TooltipContent>{{ t('adminFeature.usersPage.unlockHint') }}</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger as-child>
<span>
@@ -383,6 +415,9 @@ async function saveDefaultLibraryAccess() {
<ShieldAlert :size="11" aria-hidden="true" />
{{ t('adminFeature.usersPage.filteredBadge') }}
</StatusPill>
<StatusPill v-if="isLocked(user)" tone="warning">
{{ t('adminFeature.usersPage.lockedBadge') }}
</StatusPill>
</div>
<div v-if="canManage(user)" class="mt-3 flex items-center gap-2 border-t border-border pt-3">
<button
@@ -403,6 +438,9 @@ async function saveDefaultLibraryAccess() {
</button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end" class="w-56">
<DropdownMenuItem v-if="isLocked(user)" @click="handleUnlock(user.id)">
{{ t('adminFeature.usersPage.unlockAccount') }}
</DropdownMenuItem>
<DropdownMenuItem :disabled="!isPasswordResettable(user)" @click="handleResetPassword(user.id)">
{{ t('adminFeature.usersPage.resetPassword') }}
</DropdownMenuItem>
@@ -151,6 +151,35 @@ describe('UsersPage', () => {
expect(wrapper.find('button[aria-label="Delete Ada Lovelace"]').exists()).toBe(true)
})
it('offers the unlock action only while the account is actually locked', async () => {
const wrapper = mount(UsersPage)
await flushPromises()
expect(wrapper.find('button[aria-label="Unlock Ada Lovelace"]').exists()).toBe(false)
stubApi({ users: [{ ...USER, lockedUntil: new Date(Date.now() - 60_000).toISOString() }] })
const expired = mount(UsersPage)
await flushPromises()
expect(expired.find('button[aria-label="Unlock Ada Lovelace"]').exists()).toBe(false)
stubApi({ users: [{ ...USER, lockedUntil: new Date(Date.now() + 60_000).toISOString() }] })
const locked = mount(UsersPage)
await flushPromises()
expect(locked.find('button[aria-label="Unlock Ada Lovelace"]').exists()).toBe(true)
expect(locked.text()).toContain('Locked')
})
it('clears the lockout through the unlock endpoint and reloads', async () => {
stubApi({ users: [{ ...USER, lockedUntil: new Date(Date.now() + 60_000).toISOString() }] })
const wrapper = mount(UsersPage)
await flushPromises()
await wrapper.find('button[aria-label="Unlock Ada Lovelace"]').trigger('click')
await flushPromises()
expect(apiMock).toHaveBeenCalledWith('/api/v1/users/4/unlock', { method: 'POST' })
expect(listUrls().length).toBeGreaterThan(1)
})
it('shows a filtered empty state that differs from the unfiltered one', async () => {
stubApi({ users: [], total: 0 })
const wrapper = mount(UsersPage)
@@ -6,6 +6,7 @@ import { api } from '@/lib/api'
export interface UserRow extends AuthUser {
id: number
hasContentFilters?: boolean
lockedUntil?: string | null
}
export type UserLibrary = Pick<Library, 'id' | 'name'>
+8 -3
View File
@@ -1,10 +1,10 @@
<script setup lang="ts">
import { computed, onMounted, ref } from 'vue'
import { useI18n } from 'vue-i18n'
import type { OidcProviderPublic } from '@bookorbit/types'
import { LoginErrorCode, type OidcProviderPublic } from '@bookorbit/types'
import { Moon, Sun, Wallpaper } from '@lucide/vue'
import { ACCENT_OPTIONS, ACCENT_ROWS, RADIUS_OPTIONS, BACKGROUND_OPTIONS, useThemeStore } from '@/stores/theme'
import { useAuth } from './composables/useAuth'
import { LoginError, useAuth } from './composables/useAuth'
import { useOidc } from './composables/useOidc'
import { useSetupStatus } from './composables/useSetupStatus'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
@@ -64,6 +64,11 @@ const oidcLoadingSlug = ref<string | null>(null)
function resolveLoginError(err: unknown): string {
if (!(err instanceof Error)) return t('auth.login.errors.invalidCredentials')
if (err instanceof LoginError && err.errorCode === LoginErrorCode.ACCOUNT_LOCKED) {
const minutes = Math.max(1, Math.ceil((err.retryAfterSeconds ?? 0) / 60))
return t('auth.login.errors.accountLocked', { minutes })
}
const normalizedMessage = err.message.trim().toLowerCase()
if (normalizedMessage.includes('too many requests')) {
return t('auth.login.errors.tooManyAttempts')
@@ -262,7 +267,7 @@ async function handleOidcLogin(provider: OidcProviderPublic) {
/>
</div>
<div v-if="error" class="text-sm text-destructive animate-shake">{{ error }}</div>
<div v-if="error" role="alert" class="text-sm text-destructive animate-shake">{{ error }}</div>
<div v-if="setupStatusError" class="text-sm text-destructive animate-shake">{{ setupStatusError }}</div>
<button
@@ -97,6 +97,25 @@ describe('useAuth', () => {
)
})
it('carries the lockout code and retry window off a rejected login', async () => {
vi.stubGlobal(
'fetch',
vi.fn<typeof fetch>().mockResolvedValue({
ok: false,
json: async () => ({ message: 'Account temporarily locked', errorCode: 'account_locked', retryAfterSeconds: 540 }),
} as Response),
)
const { useAuth, LoginError } = await import('../useAuth')
const error = await useAuth()
.login('ada', 'correct-horse')
.catch((e: unknown) => e)
expect(error).toBeInstanceOf(LoginError)
expect((error as InstanceType<typeof LoginError>).errorCode).toBe('account_locked')
expect((error as InstanceType<typeof LoginError>).retryAfterSeconds).toBe(540)
})
it('clears user-scoped sidebar caches on logout', async () => {
const { useAuth } = await import('../useAuth')
@@ -94,6 +94,16 @@ async function hydratePreferences(options: { refreshUser?: boolean } = {}): Prom
initLocaleSync()
}
export class LoginError extends Error {
constructor(
public readonly errorCode: string | undefined,
public readonly retryAfterSeconds: number | undefined,
message: string,
) {
super(message)
}
}
export function useAuth() {
async function init(): Promise<void> {
isLoading.value = true
@@ -122,7 +132,7 @@ export function useAuth() {
if (!res.ok) {
const err = await res.json().catch(() => ({}))
throw new Error(err.message ?? 'Invalid credentials')
throw new LoginError(err.errorCode, err.retryAfterSeconds, err.message ?? 'Invalid credentials')
}
const data: AuthResponse = await res.json()
+9 -2
View File
@@ -2536,6 +2536,10 @@
"contentRestrictionsActive": "Content restrictions active",
"statusActive": "Active",
"statusInactive": "Inactive",
"lockedBadge": "Locked",
"unlockAccount": "Unlock account",
"unlockHint": "Clear the sign-in lockout",
"unlockAria": "Unlock {name}",
"resetPassword": "Reset password",
"resetPasswordOidcHint": "OIDC users reset passwords in their identity provider",
"resetPasswordSharedHint": "Shared accounts do not have passwords",
@@ -2548,7 +2552,8 @@
"errors": {
"load": "Failed to load",
"deleteUser": "Failed to delete user",
"resetPassword": "Failed to create a password reset link"
"resetPassword": "Failed to create a password reset link",
"unlock": "Failed to unlock the account"
},
"defaultLibraryAccess": {
"title": "Default Library Access",
@@ -2880,6 +2885,7 @@
"UserSuperuserEnable": "Enable superuser access",
"UserSuperuserDisable": "Disable superuser access",
"UserContentFiltersSet": "Update content filters",
"UserUnlock": "Unlock account",
"ReadingInsightsSharingUpdate": "Update insight sharing",
"ReadingInsightsProfileView": "View shared insights",
"LibraryCreate": "Create library",
@@ -2987,7 +2993,8 @@
"forgotPassword": "Forgot password?",
"errors": {
"invalidCredentials": "Invalid username or password",
"tooManyAttempts": "Too many login attempts. Please wait a minute and try again."
"tooManyAttempts": "Too many login attempts. Please wait a minute and try again.",
"accountLocked": "Your password is correct, but this account is temporarily locked after too many failed sign-in attempts. Try again in {minutes, plural, one {# minute} other {# minutes}}, or reset your password to unlock it now."
}
},
"oidc": {
+1
View File
@@ -28,6 +28,7 @@ export enum AuditAction {
UserSuperuserEnable = "user.superuser.enable",
UserSuperuserDisable = "user.superuser.disable",
UserContentFiltersSet = "user.content_filters.set",
UserUnlock = "user.unlock",
ReadingInsightsSharingUpdate = "reading_insights.sharing.update",
ReadingInsightsProfileView = "reading_insights.profile.view",
+4
View File
@@ -1,3 +1,7 @@
export enum LoginErrorCode {
ACCOUNT_LOCKED = "account_locked",
}
export enum OidcErrorCode {
STATE_EXPIRED = "oidc_state_expired",
TOKEN_EXCHANGE_FAILED = "oidc_token_exchange_failed",
@@ -27,6 +27,7 @@ export class GlobalExceptionFilter implements ExceptionFilter {
const rawObject = typeof raw === 'object' && raw !== null ? (raw as Record<string, unknown>) : undefined;
const message = typeof raw === 'string' ? raw : ((rawObject?.message as string) ?? (exc?.message as string) ?? 'An error occurred');
const errorCode = typeof rawObject?.errorCode === 'string' ? rawObject.errorCode : undefined;
const retryAfterSeconds = typeof rawObject?.retryAfterSeconds === 'number' ? rawObject.retryAfterSeconds : undefined;
if (status >= (HttpStatus.INTERNAL_SERVER_ERROR as number)) {
this.logger.error(exception);
@@ -43,6 +44,7 @@ export class GlobalExceptionFilter implements ExceptionFilter {
timestamp: new Date().toISOString(),
requestId: request.id,
...(errorCode ? { errorCode } : {}),
...(retryAfterSeconds !== undefined ? { retryAfterSeconds } : {}),
});
}
}
@@ -1,6 +1,7 @@
import { BadRequestException, ConflictException, ForbiddenException, ServiceUnavailableException, UnauthorizedException } from '@nestjs/common';
import fastifyCookie from '@fastify/cookie';
import Fastify from 'fastify';
import { LoginErrorCode } from '@bookorbit/types';
vi.mock('bcryptjs', () => ({
hash: vi.fn((value: string) => Promise.resolve(`mock-hash:${value}`)),
@@ -331,6 +332,46 @@ describe('AuthService', () => {
expect(db.update).not.toHaveBeenCalled();
});
it('reports the lockout with a retry time when the password is correct', async () => {
const { service, userService } = makeService();
userService.findByUsername.mockResolvedValue({
id: 1,
username: 'jdoe',
active: true,
passwordHash: 'mock-hash:pass',
tokenVersion: 1,
failedLoginAttempts: 0,
lockedUntil: new Date(Date.now() + 5 * 60_000),
});
const error = await service.login({ username: 'jdoe', password: 'pass' }, makeReply()).catch((e: unknown) => e);
expect(error).toBeInstanceOf(UnauthorizedException);
const response = (error as UnauthorizedException).getResponse() as { errorCode: string; retryAfterSeconds: number };
expect(response.errorCode).toBe(LoginErrorCode.ACCOUNT_LOCKED);
expect(response.retryAfterSeconds).toBeGreaterThan(4 * 60);
expect(response.retryAfterSeconds).toBeLessThanOrEqual(5 * 60);
});
it('hides the lockout behind the generic failure when the password is wrong', async () => {
const { service, db, userService } = makeService();
userService.findByUsername.mockResolvedValue({
id: 1,
username: 'jdoe',
active: true,
passwordHash: 'mock-hash:correct',
tokenVersion: 1,
failedLoginAttempts: 0,
lockedUntil: new Date(Date.now() + 5 * 60_000),
});
const error = await service.login({ username: 'jdoe', password: 'wrong' }, makeReply()).catch((e: unknown) => e);
expect(error).toBeInstanceOf(UnauthorizedException);
expect((error as UnauthorizedException).getResponse()).not.toHaveProperty('errorCode');
expect(db.update).not.toHaveBeenCalled();
});
it('increments failed login attempts for wrong password and non-locked account', async () => {
const { service, db, userService } = makeService();
userService.findByUsername.mockResolvedValue({
@@ -1169,6 +1210,21 @@ describe('AuthService', () => {
UnauthorizedException,
);
});
it('clears an active lockout on success', async () => {
const { service, db } = makeService();
(db.query as never as Record<string, Record<string, vi.Mock>>).users.findFirst.mockResolvedValue({
id: 1,
provisioningMethod: 'local',
passwordHash: 'mock-hash:old',
});
await service.changePassword(1, { currentPassword: 'old', newPassword: 'New@1234' }, makeReply());
expect((db as unknown as Record<string, vi.Mock>).set).toHaveBeenCalledWith(
expect.objectContaining({ failedLoginAttempts: 0, lockedUntil: null }),
);
});
});
describe('getSessions', () => {
@@ -1294,6 +1350,28 @@ describe('AuthService', () => {
await expect(service.resetPassword({ token: 'valid', newPassword: 'NewPassword1!' })).resolves.toBeUndefined();
expect(db.transaction).toHaveBeenCalled();
});
it('clears an active lockout so the new password works immediately', async () => {
const { service, db } = makeService();
(db.query as never as Record<string, Record<string, vi.Mock>>).passwordResetTokens.findFirst.mockResolvedValue({
id: 5,
userId: 1,
expiresAt: new Date(Date.now() + 10_000),
usedAt: null,
});
(db.query as never as Record<string, Record<string, vi.Mock>>).users.findFirst.mockResolvedValue({
id: 1,
username: 'jdoe',
active: true,
provisioningMethod: 'local',
});
await service.resetPassword({ token: 'valid', newPassword: 'NewPassword1!' });
expect((db as unknown as Record<string, vi.Mock>).set).toHaveBeenCalledWith(
expect.objectContaining({ failedLoginAttempts: 0, lockedUntil: null }),
);
});
});
describe('logout', () => {
+41 -18
View File
@@ -19,7 +19,7 @@ import { and, count, eq, gt, isNull, sql } from 'drizzle-orm';
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
import type { FastifyReply, FastifyRequest } from 'fastify';
import { AuditAction } from '@bookorbit/types';
import { AuditAction, LoginErrorCode } from '@bookorbit/types';
import { APP_SETTING_KEYS } from '../../common/constants/app-settings.constants';
import { DB } from '../../db/db.module';
@@ -203,25 +203,16 @@ export class AuthService {
const user = await this.userService.findByUsername(dto.username);
const now = new Date();
if (user?.lockedUntil && user.lockedUntil > now) {
this.logger.warn(
`[auth.login] [fail] userId=${user.id} username=${dto.username} ip=${ip ?? 'unknown'} errorClass=UnauthorizedException error="account locked" - login failed`,
);
this.auditEvents.emit(AUDIT_EVENT, {
userId: user.id,
actorUsername: user.username,
action: AuditAction.AuthLoginFailed,
description: `Failed login attempt for username '${dto.username}'`,
ip,
meta: { attemptedUsername: dto.username, lockout: true, lockedUntil: user.lockedUntil.toISOString() },
});
throw new UnauthorizedException('Invalid credentials');
}
const isLockedOut = Boolean(user?.lockedUntil && user.lockedUntil > now);
// The lockout is reported only to someone who supplied the correct password, so an attacker
// probing usernames still gets the generic failure and learns nothing about which accounts exist.
const passwordHash = user?.passwordHash ?? DUMMY_HASH;
const isPasswordValid = await compare(dto.password, passwordHash);
if (!user || !user.active || !isPasswordValid) {
const lockedUntil = user && user.active ? await this.recordFailedLoginAttempt(user.id, user.failedLoginAttempts ?? 0, now) : null;
const newlyLockedUntil =
user && user.active && !isLockedOut ? await this.recordFailedLoginAttempt(user.id, user.failedLoginAttempts ?? 0, now) : null;
const lockedUntil = newlyLockedUntil ?? (isLockedOut ? (user?.lockedUntil ?? null) : null);
this.logger.warn(
`[auth.login] [fail]${user ? ` userId=${user.id}` : ''} username=${dto.username} ip=${ip ?? 'unknown'} errorClass=UnauthorizedException error="${lockedUntil ? 'account locked' : 'invalid credentials'}" - login failed`,
);
@@ -238,6 +229,26 @@ export class AuthService {
throw new UnauthorizedException('Invalid credentials');
}
if (isLockedOut && user.lockedUntil) {
const retryAfterSeconds = Math.max(1, Math.ceil((user.lockedUntil.getTime() - now.getTime()) / 1000));
this.logger.warn(
`[auth.login] [fail] userId=${user.id} username=${dto.username} ip=${ip ?? 'unknown'} errorClass=UnauthorizedException error="account locked" - login failed`,
);
this.auditEvents.emit(AUDIT_EVENT, {
userId: user.id,
actorUsername: user.username,
action: AuditAction.AuthLoginFailed,
description: `Failed login attempt for username '${dto.username}'`,
ip,
meta: { attemptedUsername: dto.username, lockout: true, lockedUntil: user.lockedUntil.toISOString() },
});
throw new UnauthorizedException({
message: 'Account temporarily locked after too many failed sign-in attempts',
errorCode: LoginErrorCode.ACCOUNT_LOCKED,
retryAfterSeconds,
});
}
if ((user.failedLoginAttempts ?? 0) > 0 || user.lockedUntil) {
await this.db.update(schema.users).set({ failedLoginAttempts: 0, lockedUntil: null }).where(eq(schema.users.id, user.id));
}
@@ -564,7 +575,13 @@ export class AuthService {
await this.db.transaction(async (tx) => {
await tx
.update(schema.users)
.set({ passwordHash, isDefaultPassword: false, tokenVersion: sql`${schema.users.tokenVersion} + 1` })
.set({
passwordHash,
isDefaultPassword: false,
tokenVersion: sql`${schema.users.tokenVersion} + 1`,
failedLoginAttempts: 0,
lockedUntil: null,
})
.where(eq(schema.users.id, row.userId));
await tx.update(schema.passwordResetTokens).set({ usedAt: new Date() }).where(eq(schema.passwordResetTokens.id, row.id));
@@ -606,7 +623,13 @@ export class AuthService {
await this.db.transaction(async (tx) => {
await tx
.update(schema.users)
.set({ passwordHash, isDefaultPassword: false, tokenVersion: sql`${schema.users.tokenVersion} + 1` })
.set({
passwordHash,
isDefaultPassword: false,
tokenVersion: sql`${schema.users.tokenVersion} + 1`,
failedLoginAttempts: 0,
lockedUntil: null,
})
.where(eq(schema.users.id, userId));
await tx
@@ -36,6 +36,7 @@ describe('UserController', () => {
getLibraryIds: vi.fn(),
setLibraries: vi.fn(),
adminResetPassword: vi.fn(),
unlockUser: vi.fn(),
getContentFilters: vi.fn(),
setContentFilters: vi.fn(),
};
@@ -174,6 +175,7 @@ describe('UserController', () => {
await controller.getLibraries(8);
await controller.setLibraries(8, setLibrariesDto as any, requester);
await controller.adminResetPassword(8, requester);
await controller.unlockUser(8, requester);
expect(userService.findById).toHaveBeenCalledWith(8);
expect(userService.createUser).toHaveBeenCalledWith({ username: 'new' });
@@ -184,6 +186,7 @@ describe('UserController', () => {
expect(userService.getLibraryIds).toHaveBeenCalledWith(8);
expect(userService.setLibraries).toHaveBeenCalledWith(8, [1, 2], requester);
expect(userService.adminResetPassword).toHaveBeenCalledWith(8, requester);
expect(userService.unlockUser).toHaveBeenCalledWith(8, requester);
});
describe('content filter endpoints', () => {
@@ -294,6 +294,18 @@ export class UserController {
return this.userService.adminResetPassword(id, requestingUser);
}
@Post(':id/unlock')
@RequirePermission(Permission.ManageUsers)
@Auditable({
action: AuditAction.UserUnlock,
resource: AuditResource.User,
getResourceId: (req) => parseInt(req.params['id'] as string, 10),
description: (req) => `Cleared login lockout for user #${req.params['id']}`,
})
unlockUser(@Param('id', ParseIntPipe) id: number, @CurrentUser() requestingUser: RequestUser) {
return this.userService.unlockUser(id, requestingUser);
}
@Get('me/content-filters')
getMyContentFilters(@CurrentUser() user: RequestUser) {
return this.userService.getContentFilters(user.id, user);
@@ -398,6 +398,14 @@ describe('UserRepository', () => {
expect(updateWhere).toHaveBeenCalledWith(expect.objectContaining({ op: 'eq', left: schema.users.id, right: 12 }));
});
it('clearLockout resets the attempt counter and the lock timestamp', async () => {
await repo.clearLockout(12);
expect(db.update).toHaveBeenCalledWith(schema.users);
expect(updateSet).toHaveBeenCalledWith({ failedLoginAttempts: 0, lockedUntil: null });
expect(updateWhere).toHaveBeenCalledWith(expect.objectContaining({ op: 'eq', left: schema.users.id, right: 12 }));
});
it('setPermissions replaces permission rows in one transaction', async () => {
const txDeleteWhere = vi.fn().mockResolvedValue(undefined);
const txDelete = vi.fn().mockReturnValue({ where: txDeleteWhere });
@@ -64,6 +64,7 @@ export class UserRepository {
active: schema.users.active,
isSuperuser: schema.users.isSuperuser,
isDefaultPassword: schema.users.isDefaultPassword,
lockedUntil: schema.users.lockedUntil,
provisioningMethod: schema.users.provisioningMethod,
createdAt: schema.users.createdAt,
permissionName: schema.userPermissions.permissionName,
@@ -81,6 +82,7 @@ export class UserRepository {
active: boolean;
isSuperuser: boolean;
isDefaultPassword: boolean;
lockedUntil: Date | null;
provisioningMethod: string;
createdAt: Date;
permissions: Permission[];
@@ -98,6 +100,7 @@ export class UserRepository {
active: row.active,
isSuperuser: row.isSuperuser,
isDefaultPassword: row.isDefaultPassword,
lockedUntil: row.lockedUntil,
provisioningMethod: row.provisioningMethod,
createdAt: row.createdAt,
permissions: [],
@@ -315,6 +318,10 @@ export class UserRepository {
return rawToken;
}
async clearLockout(userId: number): Promise<void> {
await this.db.update(schema.users).set({ failedLoginAttempts: 0, lockedUntil: null }).where(eq(schema.users.id, userId));
}
async findByOidcSubject(subject: string, issuer: string) {
return this.db.query.users.findFirst({
where: and(eq(schema.users.oidcSubject, subject), eq(schema.users.oidcIssuer, issuer)),
+10
View File
@@ -329,6 +329,16 @@ export class UserService {
return { resetUrl: `${appUrl}/reset-password?token=${rawToken}` };
}
async unlockUser(targetUserId: number, requestingUser: RequestUser) {
const target = await this.userRepo.findByIdWithPermissions(targetUserId);
if (!target) throw new NotFoundException('User not found');
if (target.isSuperuser && !requestingUser.isSuperuser) {
throw new ForbiddenException('Only administrators can unlock administrator accounts');
}
await this.userRepo.clearLockout(targetUserId);
return { unlocked: true };
}
private uniquePermissions(permissionNames: Permission[]): Permission[] {
return Array.from(new Set(permissionNames));
}
@@ -295,6 +295,68 @@ describe('Auth session security (e2e)', () => {
});
});
describe('login lockout', () => {
async function attemptLogin(username: string, password: string) {
return context.app.inject({ method: 'POST', url: '/api/v1/auth/login', payload: { username, password } });
}
async function lockAccount(credentials: LocalUserCredentials) {
for (let attempt = 0; attempt < 5; attempt++) {
const response = await attemptLogin(credentials.username, 'DefinitelyWrong123');
expect(response.statusCode).toBe(401);
}
const locked = await context.db.query.users.findFirst({ where: eq(schema.users.id, credentials.userId) });
expect(locked?.lockedUntil).toBeInstanceOf(Date);
}
it('tells the owner of a correct password that the account is locked, with a retry window', async () => {
const credentials = await createLocalUser(context.db, { password: 'LockoutPass123' });
await lockAccount(credentials);
const response = await attemptLogin(credentials.username, credentials.password);
expect(response.statusCode).toBe(401);
const body = response.json() as { errorCode?: string; retryAfterSeconds?: number };
expect(body.errorCode).toBe('account_locked');
expect(body.retryAfterSeconds).toBeGreaterThan(0);
expect(body.retryAfterSeconds).toBeLessThanOrEqual(15 * 60);
expect(getSetCookieLines(response.headers)).toHaveLength(0);
});
it('does not reveal the lockout to someone who does not know the password', async () => {
const credentials = await createLocalUser(context.db, { password: 'LockoutPass123' });
await lockAccount(credentials);
const response = await attemptLogin(credentials.username, 'StillWrong123');
expect(response.statusCode).toBe(401);
expect(response.json()).not.toHaveProperty('errorCode');
expect(response.json()).not.toHaveProperty('retryAfterSeconds');
});
it('lets a completed password reset unlock the account immediately', async () => {
const credentials = await createLocalUser(context.db, { password: 'LockoutPass123' });
await lockAccount(credentials);
const rawToken = randomUUID();
await context.db.insert(schema.passwordResetTokens).values({
userId: credentials.userId,
tokenHash: sha256(rawToken),
expiresAt: new Date(Date.now() + 15 * 60_000),
});
const resetResponse = await context.app.inject({
method: 'POST',
url: '/api/v1/auth/reset-password',
payload: { token: rawToken, newPassword: 'UnlockedPass123' },
});
expect(resetResponse.statusCode).toBe(204);
const afterReset = await attemptLogin(credentials.username, 'UnlockedPass123');
expect(afterReset.statusCode).toBe(200);
});
});
describe('refresh failure paths', () => {
it('rejects refresh without cookie', async () => {
const response = await context.app.inject({
@@ -1,5 +1,5 @@
{
"totalRoutes": 371,
"totalRoutes": 372,
"byPermission": {
"(none)": 166,
"Permission.ManageMetadataConfig": 32,
@@ -8,7 +8,7 @@
"Permission.BookDockAccess": 17,
"Permission.ManageAppSettings": 22,
"Permission.ManageLibraries": 14,
"Permission.ManageUsers": 10,
"Permission.ManageUsers": 11,
"Permission.ManageEmail": 8,
"Permission.KoboSync": 7,
"Permission.NotificationAccess": 6,
@@ -23,6 +23,18 @@
"allowDefault": false,
"useGuards": []
},
{
"file": "server/src/modules/user/user.controller.ts",
"className": "UserController",
"methodName": "unlockUser",
"httpMethod": "POST",
"path": "/users/:id/unlock",
"permissions": ["Permission.ManageUsers"],
"libraryAccess": [],
"isPublic": false,
"allowDefault": false,
"useGuards": []
},
{
"file": "server/src/modules/user-statistics/user-statistics.controller.ts",
"className": "UserStatisticsController",