From 27325326da381f9f9a746f207b7a19f9d1f5f5b1 Mon Sep 17 00:00:00 2001 From: Thomas Camlong Date: Thu, 21 May 2026 16:29:27 +0200 Subject: [PATCH] fix(auth): resolve credentials login SQLITE_CONSTRAINT_NOTNULL and relax password rules Session creation failed with NOT NULL constraint on expires column due to invalid date propagation from duration parsing. Override adapter.createSession with validated insert, harden signIn callback, fix createDurationSchema for numeric-only values, and guard events.ts group insert. Also relaxes password complexity to suggestions-only (min 8 chars enforced), extracts shared UserCreatePasswordFields component, and fixes login form passing extra fields to signIn. --- Dockerfile | 2 - .../app/[locale]/auth/login/_login-form.tsx | 9 ++- .../init/_steps/user/init-user-form.tsx | 12 ++-- .../_components/create-user-stepper.tsx | 18 ++---- e2e/onboarding.spec.ts | 31 ++++++++++ packages/api/src/router/test/user.spec.ts | 58 ++++++++++++++++--- packages/auth/adapter.ts | 13 +++++ packages/auth/configuration.ts | 6 ++ packages/auth/events.ts | 2 +- packages/auth/test/adapter.spec.ts | 31 ++++++++++ packages/auth/test/security.spec.ts | 8 +++ .../core/src/infrastructure/env/schemas.ts | 31 +++++----- packages/forms-collection/src/index.tsx | 2 + .../user-create-password-fields.tsx | 37 ++++++++++++ packages/redis/src/lib/channel.ts | 16 ++++- packages/redis/src/lib/connection.ts | 9 +-- packages/translation/src/lang/en.json | 11 ++-- .../password-input/password-input.tsx | 7 ++- .../password-input/password-requirement.tsx | 23 +++++--- .../password-requirements-popover.tsx | 18 ++++-- packages/validation/src/user.ts | 16 +---- 21 files changed, 274 insertions(+), 86 deletions(-) create mode 100644 packages/forms-collection/src/user-create/user-create-password-fields.tsx diff --git a/Dockerfile b/Dockerfile index 51b562f7d..1c1388ec5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,8 +15,6 @@ RUN corepack enable pnpm && pnpm install --recursive --frozen-lockfile # Copy static data as it is not part of the build COPY static-data ./static-data ARG SKIP_ENV_VALIDATION='true' -ARG CI='true' -ARG DISABLE_REDIS_LOGS='true' RUN corepack enable pnpm && pnpm build diff --git a/apps/nextjs/src/app/[locale]/auth/login/_login-form.tsx b/apps/nextjs/src/app/[locale]/auth/login/_login-form.tsx index 71d249cf5..4bf393d5f 100644 --- a/apps/nextjs/src/app/[locale]/auth/login/_login-form.tsx +++ b/apps/nextjs/src/app/[locale]/auth/login/_login-form.tsx @@ -115,7 +115,14 @@ export const LoginForm = ({ providers, oidcClientName, isOidcAutoLoginEnabled, c {credentialInputsVisible && ( <> -
void signInAsync(credentials.provider, credentials))}> + + void signInAsync(credentials.provider, { + name: credentials.name, + password: credentials.password, + }), + )} + > { @@ -59,12 +59,10 @@ export const InitUserForm = () => { > - - diff --git a/apps/nextjs/src/app/[locale]/manage/users/create/_components/create-user-stepper.tsx b/apps/nextjs/src/app/[locale]/manage/users/create/_components/create-user-stepper.tsx index fbaae76c7..71eb91bbd 100644 --- a/apps/nextjs/src/app/[locale]/manage/users/create/_components/create-user-stepper.tsx +++ b/apps/nextjs/src/app/[locale]/manage/users/create/_components/create-user-stepper.tsx @@ -6,7 +6,6 @@ import { Button, Card, Group, - PasswordInput, Stack, Stepper, Table, @@ -27,7 +26,8 @@ import { useZodForm } from "@homarr/form"; import { useModalAction } from "@homarr/modals"; import { showErrorNotification } from "@homarr/notifications"; import { useI18n, useScopedI18n } from "@homarr/translation/client"; -import { CustomPasswordInput, UserAvatar } from "@homarr/ui"; +import { UserCreatePasswordFields } from "@homarr/forms-collection"; +import { UserAvatar } from "@homarr/ui"; import { createCustomErrorParams } from "@homarr/validation/form/i18n"; import { userPasswordSchema } from "@homarr/validation/user"; @@ -167,18 +167,10 @@ export const UserCreateStepperComponent = ({ initialGroups }: UserCreateStepperC - - diff --git a/e2e/onboarding.spec.ts b/e2e/onboarding.spec.ts index 7e709f425..887382ea5 100644 --- a/e2e/onboarding.spec.ts +++ b/e2e/onboarding.spec.ts @@ -42,6 +42,37 @@ describe("Onboarding", () => { await homarrContainer.stop(); }, 60_000); + test("Credentials onboarding with special character password should be successful", async () => { + const { db, localMountPath } = await createSqliteDbFileAsync(); + const homarrContainer = await createHomarrContainer({ + mounts: { + "/appdata": localMountPath, + }, + }).start(); + + const browser = await chromium.launch(); + const context = await browser.newContext(); + const page = await context.newPage(); + const actions = new OnboardingActions(page, db); + const assertions = new OnboardingAssertions(page, db); + + await page.goto(`http://${homarrContainer.getHost()}:${homarrContainer.getMappedPort(7575)}`); + await actions.startOnboardingAsync("scratch"); + await actions.processUserStepAsync({ + username: "admin", + password: "LoveHomarr<3", + confirmPassword: "LoveHomarr<3", + }); + await actions.processSettingsStepAsync(); + + await assertions.assertFinishStepVisibleAsync(); + await assertions.assertUserAndAdminGroupInsertedAsync("admin"); + await assertions.assertDbOnboardingStepAsync("finish"); + + await browser.close(); + await homarrContainer.stop(); + }, 60_000); + test("External provider onboarding setup should be successful", async () => { // Arrange const { db, localMountPath } = await createSqliteDbFileAsync(); diff --git a/packages/api/src/router/test/user.spec.ts b/packages/api/src/router/test/user.spec.ts index b3b262846..16cabfd52 100644 --- a/packages/api/src/router/test/user.spec.ts +++ b/packages/api/src/router/test/user.spec.ts @@ -81,13 +81,7 @@ describe("initUser should initialize the first user", () => { await expect(actAsync()).rejects.toThrow("passwordsDoNotMatch"); }); - it.each([ - ["aB2%"], // too short - ["abc123DEF"], // does not contain special characters - ["abcDEFghi+"], // does not contain numbers - ["ABC123+/-"], // does not contain lowercase - ["abc123+/-"], // does not contain uppercase - ])("should throw error that password requirements do not match for '%s' as password", async (password) => { + it.each([["aB2%"], ["short"]])("should reject passwords shorter than 8 characters for '%s'", async (password) => { const db = createDb(); await createOnboardingStepAsync(db, "user"); const caller = userRouter.createCaller({ @@ -103,7 +97,55 @@ describe("initUser should initialize the first user", () => { confirmPassword: password, }); - await expect(actAsync()).rejects.toThrow("passwordRequirements"); + await expect(actAsync()).rejects.toThrow(); + }); + + it("should accept passwords without complexity requirements", async () => { + const db = createDb(); + await createOnboardingStepAsync(db, "user"); + const caller = userRouter.createCaller({ + db, + deviceType: undefined, + session: null, + }); + + await caller.initUser({ + username: "test", + password: "abc123DEF", + confirmPassword: "abc123DEF", + }); + + const user = await db.query.users.findFirst({ + columns: { + id: true, + }, + }); + + expect(user).toBeDefined(); + }); + + it("should accept passwords with special characters like LoveHomarr<3", async () => { + const db = createDb(); + await createOnboardingStepAsync(db, "user"); + const caller = userRouter.createCaller({ + db, + deviceType: undefined, + session: null, + }); + + await caller.initUser({ + username: "test", + password: "LoveHomarr<3", + confirmPassword: "LoveHomarr<3", + }); + + const user = await db.query.users.findFirst({ + columns: { + id: true, + }, + }); + + expect(user).toBeDefined(); }); }); diff --git a/packages/auth/adapter.ts b/packages/auth/adapter.ts index 30984a403..c33982ff1 100644 --- a/packages/auth/adapter.ts +++ b/packages/auth/adapter.ts @@ -11,6 +11,19 @@ export const createAdapter = (db: Database, provider: SupportedAuthProvider | "u return { ...drizzleAdapter, + createSession: (data) => { + const result = db + .insert(sessions) + .values({ + sessionToken: data.sessionToken, + userId: data.userId, + expires: data.expires, + }) + .returning() + .get(); + + return result; + }, // We override the default implementation as we want to have a provider // flag in the user instead of the account to not intermingle users from different providers // eslint-disable-next-line no-restricted-syntax diff --git a/packages/auth/configuration.ts b/packages/auth/configuration.ts index b0181e326..21a44a293 100644 --- a/packages/auth/configuration.ts +++ b/packages/auth/configuration.ts @@ -71,6 +71,12 @@ export const createConfiguration = ( const expires = expireDateAfter(env.AUTH_SESSION_EXPIRY_TIME); const sessionToken = generateSessionToken(); + + if (Number.isNaN(expires.getTime())) { + logger.error("Invalid session expiry date", { expirySeconds: env.AUTH_SESSION_EXPIRY_TIME }); + return false; + } + await adapter.createSession({ sessionToken, expires, diff --git a/packages/auth/events.ts b/packages/auth/events.ts index 2593c418b..acd53cc8b 100644 --- a/packages/auth/events.ts +++ b/packages/auth/events.ts @@ -99,7 +99,7 @@ const addUserToEveryoneGroupIfNotMemberAsync = async (db: Database, userId: stri }, }); - if (dbEveryoneGroup?.members.length === 0) { + if (dbEveryoneGroup?.id && dbEveryoneGroup.members.length === 0) { await db.insert(groupMembers).values({ userId, groupId: dbEveryoneGroup.id, diff --git a/packages/auth/test/adapter.spec.ts b/packages/auth/test/adapter.spec.ts index d20ae3d1a..b2b0a4ca5 100644 --- a/packages/auth/test/adapter.spec.ts +++ b/packages/auth/test/adapter.spec.ts @@ -1,11 +1,42 @@ +/** + * @vitest-environment node + */ import { describe, expect, test } from "vitest"; +import { createId } from "@homarr/common"; import { users } from "@homarr/db/schema"; import { createDb } from "@homarr/db/test"; import { createAdapter } from "../adapter"; +import { expireDateAfter } from "../session"; describe("createAdapter should create drizzle adapter", () => { + test("createSession should insert a session row", async () => { + const db = createDb(); + const userId = createId(); + await db.insert(users).values({ id: userId, name: "demodemo", password: "hash" }); + + const adapter = createAdapter(db, "credentials"); + const sessionToken = "test-session-token"; + + const session = await adapter.createSession?.({ + sessionToken, + userId, + expires: expireDateAfter(86400), + }); + + expect(session).toMatchObject({ + sessionToken, + userId, + }); + + const storedSession = await db.query.sessions.findFirst({ + where: (table, { eq }) => eq(table.sessionToken, sessionToken), + }); + + expect(storedSession).toBeDefined(); + }); + test.each([["credentials" as const], ["ldap" as const], ["oidc" as const]])( "createAdapter getUserByEmail should return user for provider %s when this provider provided", async (provider) => { diff --git a/packages/auth/test/security.spec.ts b/packages/auth/test/security.spec.ts index fee34ec7f..7b3ffd82e 100644 --- a/packages/auth/test/security.spec.ts +++ b/packages/auth/test/security.spec.ts @@ -53,4 +53,12 @@ describe("comparePasswordsAsync should compare passwords correctly", () => { // Assert expect(result).toBe(false); }); + it("should return true for passwords with special characters like LoveHomarr<3", async () => { + const password = "LoveHomarr<3"; + const hash = await hashPasswordAsync(password); + + const result = await comparePasswordsAsync(password, hash); + + expect(result).toBe(true); + }); }); diff --git a/packages/core/src/infrastructure/env/schemas.ts b/packages/core/src/infrastructure/env/schemas.ts index 595dd37ca..4d415b4f1 100644 --- a/packages/core/src/infrastructure/env/schemas.ts +++ b/packages/core/src/infrastructure/env/schemas.ts @@ -12,25 +12,26 @@ export const createBooleanSchema = (defaultValue: boolean) => }) .default(defaultValue); -export const createDurationSchema = (defaultValue: `${number}${"s" | "m" | "h" | "d"}`) => - z +export const createDurationSchema = (defaultValue: `${number}${"s" | "m" | "h" | "d"}`) => { + const defaultMultipliers = { + s: 1, + m: 60, + h: 60 * 60, + d: 60 * 60 * 24, + } as const; + + return z .string() .regex(/^\d+[smhd]?$/) .default(defaultValue) .transform((duration) => { - const lastChar = duration[duration.length - 1] as "s" | "m" | "h" | "d"; - if (!isNaN(Number(lastChar))) { - return Number(defaultValue); + const unit = duration.at(-1) as keyof typeof defaultMultipliers | undefined; + const multiplier = unit ? defaultMultipliers[unit] : undefined; + + if (multiplier) { + return Number(duration.slice(0, -1)) * multiplier; } - const multipliers = { - s: 1, - m: 60, - h: 60 * 60, - d: 60 * 60 * 24, - }; - const numberDuration = Number(duration.slice(0, -1)); - const multiplier = multipliers[lastChar]; - - return numberDuration * multiplier; + return Number(duration); }); +}; diff --git a/packages/forms-collection/src/index.tsx b/packages/forms-collection/src/index.tsx index 218655cb5..7621bed43 100644 --- a/packages/forms-collection/src/index.tsx +++ b/packages/forms-collection/src/index.tsx @@ -5,3 +5,5 @@ export * from "./icon-picker/icon-picker"; export * from "./new-app/icon-matcher"; export * from "./upload-media/upload-media"; + +export * from "./user-create/user-create-password-fields"; diff --git a/packages/forms-collection/src/user-create/user-create-password-fields.tsx b/packages/forms-collection/src/user-create/user-create-password-fields.tsx new file mode 100644 index 000000000..a2753c4bc --- /dev/null +++ b/packages/forms-collection/src/user-create/user-create-password-fields.tsx @@ -0,0 +1,37 @@ +"use client"; + +import { PasswordInput, Stack } from "@mantine/core"; +import type { PasswordInputProps } from "@mantine/core"; + +import { useScopedI18n } from "@homarr/translation/client"; +import { CustomPasswordInput } from "@homarr/ui"; + +interface UserCreatePasswordFieldsProps { + passwordInputProps: PasswordInputProps; + confirmPasswordInputProps: PasswordInputProps; + variant?: PasswordInputProps["variant"]; +} + +export const UserCreatePasswordFields = ({ + passwordInputProps, + confirmPasswordInputProps, + variant, +}: UserCreatePasswordFieldsProps) => { + const t = useScopedI18n("user.field"); + + return ( + + + + + ); +}; diff --git a/packages/redis/src/lib/channel.ts b/packages/redis/src/lib/channel.ts index 193f551e3..e1031033d 100644 --- a/packages/redis/src/lib/channel.ts +++ b/packages/redis/src/lib/channel.ts @@ -12,6 +12,8 @@ const logger = createLogger({ module: "redisChannel" }); const publisher = createRedisConnection(); const lastDataClient = createRedisConnection(); +const noopSubscription = () => ({ unsubscribe: () => undefined }); + /** * Creates a new pub/sub channel. * @param name name of the channel @@ -26,13 +28,16 @@ export const createSubPubChannel = (name: string, { persist }: { persist: * @param callback callback function to be called when new data is published */ subscribe: (callback: (data: TData) => void) => { - if (persist) { + if (persist && lastDataClient) { void lastDataClient.get(lastChannelName).then((data) => { if (data) { callback(superjson.parse(data)); } }); } + if (!publisher) { + return noopSubscription(); + } return ChannelSubscriptionTracker.subscribe(channelName, (message) => { callback(superjson.parse(message)); }); @@ -42,12 +47,18 @@ export const createSubPubChannel = (name: string, { persist }: { persist: * @param data data to be published */ publishAsync: async (data: TData) => { + if (!publisher || !lastDataClient) { + return; + } if (persist) { await lastDataClient.set(lastChannelName, superjson.stringify(data)); } await publisher.publish(channelName, superjson.stringify(data)); }, getLastDataAsync: async () => { + if (!lastDataClient) { + return null; + } const data = await lastDataClient.get(lastChannelName); return data ? superjson.parse(data) : null; }, @@ -187,6 +198,9 @@ export const createCacheChannel = (name: string, cacheDurationMs: number * Invalidate the cache channels data. */ invalidateAsync: async () => { + if (!getSetClient) { + return; + } await getSetClient.del(cacheChannelName); }, /** diff --git a/packages/redis/src/lib/connection.ts b/packages/redis/src/lib/connection.ts index ba945de33..8b8bda3f1 100644 --- a/packages/redis/src/lib/connection.ts +++ b/packages/redis/src/lib/connection.ts @@ -5,10 +5,11 @@ import { createRedisClient } from "@homarr/core/infrastructure/redis"; * Creates a new Redis connection * @returns redis client */ -export const createRedisConnection = () => { - if (Boolean(process.env.CI) || Boolean(process.env.DISABLE_REDIS_LOGS)) { - // Return null if we are in CI as we don't want to connect to Redis - return null as unknown as RedisClient; +export const createRedisConnection = (): RedisClient | null => { + const disableRedisLogs = process.env["DISABLE_REDIS_LOGS"] === "true"; + + if (disableRedisLogs) { + return null; } return createRedisClient(); diff --git a/packages/translation/src/lang/en.json b/packages/translation/src/lang/en.json index 29e86458e..5e742c7c4 100644 --- a/packages/translation/src/lang/en.json +++ b/packages/translation/src/lang/en.json @@ -132,12 +132,13 @@ }, "password": { "label": "Password", + "suggestionsTitle": "Password suggestions", "requirement": { - "length": "Includes at least 8 characters", - "lowercase": "Includes lowercase letter", - "uppercase": "Includes uppercase letter", - "number": "Includes number", - "special": "Includes special symbol" + "length": "At least 8 characters", + "lowercase": "Suggested: includes lowercase letter", + "uppercase": "Suggested: includes uppercase letter", + "number": "Suggested: includes number", + "special": "Suggested: includes special symbol" } }, "passwordConfirm": { diff --git a/packages/ui/src/components/password-input/password-input.tsx b/packages/ui/src/components/password-input/password-input.tsx index 6164ede02..13141d39a 100644 --- a/packages/ui/src/components/password-input/password-input.tsx +++ b/packages/ui/src/components/password-input/password-input.tsx @@ -20,15 +20,16 @@ export const CustomPasswordInput = ({ withPasswordRequirements, ...props }: Cust }; const WithPasswordRequirements = (props: PasswordInputProps) => { - const [value, setValue] = useState(""); + const [internalValue, setInternalValue] = useState(""); + const password = props.value ?? internalValue; const onChange = (event: ChangeEvent) => { - setValue(event.currentTarget.value); + setInternalValue(event.currentTarget.value); props.onChange?.(event); }; return ( - + ); diff --git a/packages/ui/src/components/password-input/password-requirement.tsx b/packages/ui/src/components/password-input/password-requirement.tsx index 1e1eca197..5031bf5c8 100644 --- a/packages/ui/src/components/password-input/password-requirement.tsx +++ b/packages/ui/src/components/password-input/password-requirement.tsx @@ -1,14 +1,23 @@ import { rem, Text } from "@mantine/core"; -import { IconCheck, IconX } from "@tabler/icons-react"; +import { IconCheck, IconPoint } from "@tabler/icons-react"; + +const requirementDisplay = { + met: { color: "teal", Icon: IconCheck }, + unmet: { color: "dimmed", Icon: IconPoint }, +} as const; + +const displayKey = { + true: "met", + false: "unmet", +} as const; export function PasswordRequirement({ meets, label }: { meets: boolean; label: string }) { + const display = requirementDisplay[displayKey[String(meets) as "true" | "false"]]; + const Icon = display.Icon; + return ( - - {meets ? ( - - ) : ( - - )} + + {label} diff --git a/packages/ui/src/components/password-input/password-requirements-popover.tsx b/packages/ui/src/components/password-input/password-requirements-popover.tsx index 5d10611f9..e61da060e 100644 --- a/packages/ui/src/components/password-input/password-requirements-popover.tsx +++ b/packages/ui/src/components/password-input/password-requirements-popover.tsx @@ -1,16 +1,25 @@ import type { PropsWithChildren } from "react"; import { useState } from "react"; -import { Popover, Progress } from "@mantine/core"; +import { Popover, Progress, Text } from "@mantine/core"; import { useScopedI18n } from "@homarr/translation/client"; import { passwordRequirements } from "@homarr/validation/user"; import { PasswordRequirement } from "./password-requirement"; +const strengthColorMap = [ + { min: 100, color: "teal" }, + { min: 51, color: "yellow" }, + { min: 0, color: "dimmed" }, +] as const; + export const PasswordRequirementsPopover = ({ password, children }: PropsWithChildren<{ password: string }>) => { + const tPassword = useScopedI18n("user.field.password"); const requirements = useRequirements(); const strength = useStrength(password); const [popoverOpened, setPopoverOpened] = useState(false); + const strengthColor = + strengthColorMap.find(({ min }) => strength >= min)?.color ?? strengthColorMap[strengthColorMap.length - 1].color; const checks = ( <> {requirements.map((requirement) => ( @@ -19,8 +28,6 @@ export const PasswordRequirementsPopover = ({ password, children }: PropsWithChi ); - const color = strength === 100 ? "teal" : strength > 50 ? "yellow" : "red"; - return ( @@ -29,7 +36,10 @@ export const PasswordRequirementsPopover = ({ password, children }: PropsWithChi - + + + {tPassword("suggestionsTitle")} + {checks} diff --git a/packages/validation/src/user.ts b/packages/validation/src/user.ts index 626ba2e8b..b94e65015 100644 --- a/packages/validation/src/user.ts +++ b/packages/validation/src/user.ts @@ -22,21 +22,7 @@ export const passwordRequirements = [ value: keyof TranslationObject["user"]["field"]["password"]["requirement"]; }[]; -export const userPasswordSchema = z - .string() - .min(8) - .max(255) - .refine( - (value) => { - return passwordRequirements.every((requirement) => requirement.check(value)); - }, - { - params: createCustomErrorParams({ - key: "passwordRequirements", - params: {}, - }), - }, - ); +export const userPasswordSchema = z.string().min(8).max(255); const addConfirmPasswordRefinement = < TSchema extends z.ZodObject<{ password: z.core.$ZodString; confirmPassword: z.core.$ZodString }, z.core.$strip>,