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>,