From 8f7ed056c435a66d095cdfd43b6e893dfc7e6da3 Mon Sep 17 00:00:00 2001 From: Meier Lukas Date: Thu, 26 Sep 2024 19:02:45 +0200 Subject: [PATCH] fix: issue with color scheme in layout (#1168) * fix: issue with color scheme in layout * test: add and adjust unit tests for sign-in-callback --- .../[locale]/_client-providers/mantine.tsx | 8 +- apps/nextjs/src/app/[locale]/layout.tsx | 4 +- packages/auth/callbacks.ts | 23 ++++- packages/auth/configuration.ts | 2 +- packages/auth/test/callbacks.spec.ts | 97 +++++++++++++++++-- packages/common/src/cookie.ts | 7 ++ 6 files changed, 127 insertions(+), 14 deletions(-) diff --git a/apps/nextjs/src/app/[locale]/_client-providers/mantine.tsx b/apps/nextjs/src/app/[locale]/_client-providers/mantine.tsx index 9a2f3aecb..fd01444d7 100644 --- a/apps/nextjs/src/app/[locale]/_client-providers/mantine.tsx +++ b/apps/nextjs/src/app/[locale]/_client-providers/mantine.tsx @@ -1,12 +1,14 @@ "use client"; -import { useState } from "react"; import type { PropsWithChildren } from "react"; +import { useState } from "react"; import type { MantineColorScheme, MantineColorSchemeManager } from "@mantine/core"; import { createTheme, isMantineColorScheme, MantineProvider } from "@mantine/core"; +import dayjs from "dayjs"; import { clientApi } from "@homarr/api/client"; import { useSession } from "@homarr/auth/client"; +import { parseCookies, setClientCookie } from "@homarr/common"; export const CustomMantineProvider = ({ children }: PropsWithChildren) => { const manager = useColorSchemeManager(); @@ -50,7 +52,8 @@ function useColorSchemeManager(): MantineColorSchemeManager { } try { - return (window.localStorage.getItem(key) as MantineColorScheme | undefined) ?? defaultValue; + const cookies = parseCookies(document.cookie); + return (cookies[key] as MantineColorScheme | undefined) ?? defaultValue; } catch { return defaultValue; } @@ -61,6 +64,7 @@ function useColorSchemeManager(): MantineColorSchemeManager { if (session) { mutateColorScheme({ colorScheme: value }); } + setClientCookie(key, value, { expires: dayjs().add(1, "year").toDate() }); window.localStorage.setItem(key, value); } catch (error) { console.warn("[@mantine/core] Local storage color scheme manager was unable to save color scheme.", error); diff --git a/apps/nextjs/src/app/[locale]/layout.tsx b/apps/nextjs/src/app/[locale]/layout.tsx index c1b37045b..92826b989 100644 --- a/apps/nextjs/src/app/[locale]/layout.tsx +++ b/apps/nextjs/src/app/[locale]/layout.tsx @@ -6,6 +6,8 @@ import "@homarr/spotlight/styles.css"; import "@homarr/ui/styles.css"; import "~/styles/scroll-area.scss"; +import { cookies } from "next/headers"; + import { env } from "@homarr/auth/env.mjs"; import { auth } from "@homarr/auth/next"; import { ModalProvider } from "@homarr/modals"; @@ -53,7 +55,7 @@ export const viewport: Viewport = { export default async function Layout(props: { children: React.ReactNode; params: { locale: string } }) { const session = await auth(); - const colorScheme = session?.user.colorScheme; + const colorScheme = cookies().get("homarr-color-scheme")?.value ?? "light"; const StackedProvider = composeWrappers([ (innerProps) => { diff --git a/packages/auth/callbacks.ts b/packages/auth/callbacks.ts index 3665a674f..35152cdc0 100644 --- a/packages/auth/callbacks.ts +++ b/packages/auth/callbacks.ts @@ -1,5 +1,6 @@ import { cookies } from "next/headers"; import type { Adapter } from "@auth/core/adapters"; +import dayjs from "dayjs"; import type { NextAuthConfig } from "next-auth"; import type { Database } from "@homarr/db"; @@ -52,12 +53,12 @@ export const createSessionCallback = (db: Database): NextAuthCallbackOf<"session }; export const createSignInCallback = - (adapter: Adapter, isCredentialsRequest: boolean): NextAuthCallbackOf<"signIn"> => + (adapter: Adapter, db: Database, isCredentialsRequest: boolean): NextAuthCallbackOf<"signIn"> => async ({ user }) => { if (!isCredentialsRequest) return true; // https://github.com/nextauthjs/next-auth/issues/6106 - if (!adapter.createSession) { + if (!adapter.createSession || !user.id) { return false; } @@ -66,8 +67,7 @@ export const createSignInCallback = await adapter.createSession({ sessionToken, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - userId: user.id!, + userId: user.id, expires: sessionExpires, }); @@ -79,6 +79,21 @@ export const createSignInCallback = secure: true, }); + const dbUser = await db.query.users.findFirst({ + where: eq(users.id, user.id), + columns: { + colorScheme: true, + }, + }); + + if (!dbUser) return false; + + // We use a cookie as localStorage is not shared with server (otherwise flickering would occur) + cookies().set("homarr-color-scheme", dbUser.colorScheme, { + path: "/", + expires: dayjs().add(1, "year").toDate(), + }); + return true; }; diff --git a/packages/auth/configuration.ts b/packages/auth/configuration.ts index 32b8f4f26..a3945a064 100644 --- a/packages/auth/configuration.ts +++ b/packages/auth/configuration.ts @@ -38,7 +38,7 @@ export const createConfiguration = (isCredentialsRequest: boolean, headers: Read ]), callbacks: { session: createSessionCallback(db), - signIn: createSignInCallback(adapter, isCredentialsRequest), + signIn: createSignInCallback(adapter, db, isCredentialsRequest), }, redirectProxyUrl: createRedirectUri(headers, "/api/auth"), secret: "secret-is-not-defined-yet", // TODO: This should be added later diff --git a/packages/auth/test/callbacks.spec.ts b/packages/auth/test/callbacks.spec.ts index e793c6749..20dbf17f2 100644 --- a/packages/auth/test/callbacks.spec.ts +++ b/packages/auth/test/callbacks.spec.ts @@ -5,7 +5,7 @@ import { cookies } from "next/headers"; import type { Adapter, AdapterUser } from "@auth/core/adapters"; import type { Account } from "next-auth"; import type { JWT } from "next-auth/jwt"; -import { describe, expect, it, test, vi } from "vitest"; +import { describe, expect, test, vi } from "vitest"; import { groupMembers, groupPermissions, groups, users } from "@homarr/db/schema/sqlite"; import { createDb } from "@homarr/db/test"; @@ -15,6 +15,7 @@ import { createSessionCallback, createSignInCallback, getCurrentUserPermissionsA describe("getCurrentUserPermissions", () => { test("should return empty permissions when non existing user requested", async () => { + // Arrange const db = createDb(); await db.insert(groups).values({ @@ -30,11 +31,16 @@ describe("getCurrentUserPermissions", () => { }); const userId = "1"; + + // Act const result = await getCurrentUserPermissionsAsync(db, userId); + + // Assert expect(result).toEqual([]); }); test("should return empty permissions when user has no groups", async () => { + // Arrange const db = createDb(); const userId = "1"; @@ -50,11 +56,15 @@ describe("getCurrentUserPermissions", () => { id: userId, }); + // Act const result = await getCurrentUserPermissionsAsync(db, userId); + + // Assert expect(result).toEqual([]); }); test("should return permissions for user", async () => { + // Arrange const db = createDb(); const getPermissionsWithChildrenMock = vi .spyOn(definitions, "getPermissionsWithChildren") @@ -77,14 +87,18 @@ describe("getCurrentUserPermissions", () => { permission: "admin", }); + // Act const result = await getCurrentUserPermissionsAsync(db, mockId); + + // Assert expect(result).toEqual(["board-create"]); expect(getPermissionsWithChildrenMock).toHaveBeenCalledWith(["admin"]); }); }); describe("session callback", () => { - it("should add id and name to session user", async () => { + test("should add id and name to session user", async () => { + // Arrange const user: AdapterUser = { id: "id", name: "name", @@ -94,6 +108,8 @@ describe("session callback", () => { const token: JWT = {}; const db = createDb(); const callback = createSessionCallback(db); + + // Act const result = await callback({ session: { user: { @@ -112,6 +128,8 @@ describe("session callback", () => { trigger: "update", newSession: {}, }); + + // Assert expect(result.user).toBeDefined(); expect(result.user!.id).toEqual(user.id); expect(result.user!.name).toEqual(user.name); @@ -169,37 +187,56 @@ vi.mock("next/headers", async (importOriginal) => { }); describe("createSignInCallback", () => { - it("should return true if not credentials request", async () => { + test("should return true if not credentials request and set colorScheme & sessionToken cookie", async () => { + // Arrange const isCredentialsRequest = false; - const signInCallback = createSignInCallback(createAdapter(), isCredentialsRequest); + const db = await prepareDbForSigninAsync("1"); + const signInCallback = createSignInCallback(createAdapter(), db, isCredentialsRequest); + + // Act const result = await signInCallback({ user: { id: "1", emailVerified: new Date("2023-01-13") }, account: {} as Account, }); + + // Assert expect(result).toBe(true); }); - it("should return false if no adapter.createSession", async () => { + test("should return false if no adapter.createSession", async () => { + // Arrange const isCredentialsRequest = true; + const db = await prepareDbForSigninAsync("1"); const signInCallback = createSignInCallback( // https://github.com/nextauthjs/next-auth/issues/6106 { createSession: undefined } as unknown as Adapter, + db, isCredentialsRequest, ); + + // Act const result = await signInCallback({ user: { id: "1", emailVerified: new Date("2023-01-13") }, account: {} as Account, }); + + // Assert expect(result).toBe(false); }); test("should call adapter.createSession with correct input", async () => { + // Arrange const adapter = createAdapter(); const isCredentialsRequest = true; - const signInCallback = createSignInCallback(adapter, isCredentialsRequest); + const db = await prepareDbForSigninAsync("1"); + const signInCallback = createSignInCallback(adapter, db, isCredentialsRequest); const user = { id: "1", emailVerified: new Date("2023-01-13") }; const account = {} as Account; + + // Act await signInCallback({ user, account }); + + // Assert expect(adapter.createSession).toHaveBeenCalledWith({ sessionToken: mockSessionToken, userId: user.id, @@ -213,4 +250,52 @@ describe("createSignInCallback", () => { secure: true, }); }); + + test("should set colorScheme from db as cookie", async () => { + // Arrange + const isCredentialsRequest = false; + const db = await prepareDbForSigninAsync("1"); + const signInCallback = createSignInCallback(createAdapter(), db, isCredentialsRequest); + + // Act + const result = await signInCallback({ + user: { id: "1", emailVerified: new Date("2023-01-13") }, + account: {} as Account, + }); + + // Assert + expect(result).toBe(true); + expect(cookies().set).toHaveBeenCalledWith( + "homarr-color-scheme", + "dark", + expect.objectContaining({ + path: "/", + }), + ); + }); + + test("should return false if user not found in db", async () => { + // Arrange + const isCredentialsRequest = true; + const db = await prepareDbForSigninAsync("other-id"); + const signInCallback = createSignInCallback(createAdapter(), db, isCredentialsRequest); + + // Act + const result = await signInCallback({ + user: { id: "1", emailVerified: new Date("2023-01-13") }, + account: {} as Account, + }); + + // Assert + expect(result).toBe(false); + }); }); + +const prepareDbForSigninAsync = async (userId: string) => { + const db = createDb(); + await db.insert(users).values({ + id: userId, + colorScheme: "dark", + }); + return db; +}; diff --git a/packages/common/src/cookie.ts b/packages/common/src/cookie.ts index 699dd9252..fc2e5df9e 100644 --- a/packages/common/src/cookie.ts +++ b/packages/common/src/cookie.ts @@ -1,3 +1,6 @@ +import type { CookieSerializeOptions } from "cookie"; +import { serialize } from "cookie"; + export function parseCookies(cookieString: string) { const list: Record = {}; const cookieHeader = cookieString; @@ -15,3 +18,7 @@ export function parseCookies(cookieString: string) { return list; } + +export function setClientCookie(name: string, value: string, options: CookieSerializeOptions = {}) { + document.cookie = serialize(name, value, options); +}