diff --git a/apps/docs/docs/advanced/single-sign-on/index.mdx b/apps/docs/docs/advanced/single-sign-on/index.mdx index 505fe9867..634b867ad 100644 --- a/apps/docs/docs/advanced/single-sign-on/index.mdx +++ b/apps/docs/docs/advanced/single-sign-on/index.mdx @@ -202,9 +202,9 @@ Homarr supports multiple authentication options, from internal userbase (credent | ``AUTH_OIDC_CLIENT_NAME`` | Display name of provider (in login screen) | OIDC | | ``AUTH_OIDC_AUTO_LOGIN`` | Automatically redirect to OIDC login | false | | ``AUTH_OIDC_SCOPE_OVERWRITE`` | Overwrite default scopes (openid, profile, email) | openid email profile groups | - | ``AUTH_OIDC_GROUPS_ATTRIBUTE`` | Attribute used for groups (roles) claim | groups | + | ``AUTH_OIDC_GROUPS_ATTRIBUTE`` | Attribute used for groups (roles) claim. Dot-separated paths such as `resource_access.homarr.roles` are supported. | groups | | ``AUTH_OIDC_GROUPS_LOCAL_MANAGEMENT`` | Stop syncing group memberships of OIDC users from the groups claim and manage them locally via the group members page instead | false | - | ``AUTH_OIDC_NAME_ATTRIBUTE_OVERWRITE`` | Overwrite name attribute. By default it will use preferred_username if it does not contain a `@` and otherwise name. | --- | + | ``AUTH_OIDC_NAME_ATTRIBUTE_OVERWRITE`` | Overwrite name attribute. Dot-separated paths are supported. By default it will use preferred_username if it does not contain a `@` and otherwise name. | --- | | ``AUTH_OIDC_FORCE_USERINFO`` | Force userinfo endpoint to be used for user information. | false | | ``AUTH_OIDC_ENABLE_DANGEROUS_ACCOUNT_LINKING`` | Enable account linking for OIDC provider. This will link the OIDC accounts by email. Be sure that you have verified emails to prevent stealing accounts. | false | | ``AUTH_OIDC_ENABLE_DANGEROUS_CREDENTIALS_LINKING`` | Link an OIDC identity to an existing credentials user with the same email. The credentials login and existing user data are preserved. Requires the OIDC profile to contain `email_verified: true`. | false | diff --git a/packages/auth/events.ts b/packages/auth/events.ts index 0a729fd52..c33037fa1 100644 --- a/packages/auth/events.ts +++ b/packages/auth/events.ts @@ -9,7 +9,7 @@ import { groupMembers, groups, users } from "@homarr/db/schema"; import { colorSchemeCookieKey, everyoneGroup } from "@homarr/definitions"; import { env } from "./env"; -import { extractProfileName } from "./providers/oidc/oidc-provider"; +import { extractProfileName, getProfileValueByPath } from "./providers/oidc/profile"; const logger = createLogger({ module: "authEvents" }); @@ -31,16 +31,11 @@ export const createSignInEventHandler = (db: Database): Exclude => ({ @@ -76,12 +77,3 @@ export const OidcProvider = (headers: ReadonlyHeaders | null): OIDCConfig { - if (!env.AUTH_OIDC_NAME_ATTRIBUTE_OVERWRITE) { - // Use the name as the username if the preferred_username is an email address - return profile.preferred_username?.includes("@") ? profile.name : profile.preferred_username; - } - - return profile[env.AUTH_OIDC_NAME_ATTRIBUTE_OVERWRITE as keyof typeof profile] as string; -}; diff --git a/packages/auth/providers/oidc/profile.ts b/packages/auth/providers/oidc/profile.ts new file mode 100644 index 000000000..f4863da80 --- /dev/null +++ b/packages/auth/providers/oidc/profile.ts @@ -0,0 +1,23 @@ +import type { Profile } from "@auth/core/types"; + +import { env } from "../../env"; + +export const getProfileValueByPath = (profile: Profile, path: string): unknown => { + return path.split(".").reduce((value, key) => { + if (typeof value !== "object" || value === null || !Object.hasOwn(value, key)) { + return undefined; + } + + return (value as Record)[key]; + }, profile); +}; + +export const extractProfileName = (profile: Profile) => { + if (!env.AUTH_OIDC_NAME_ATTRIBUTE_OVERWRITE) { + // Use the name as the username if the preferred_username is an email address + return profile.preferred_username?.includes("@") ? profile.name : profile.preferred_username; + } + + const profileName = getProfileValueByPath(profile, env.AUTH_OIDC_NAME_ATTRIBUTE_OVERWRITE); + return typeof profileName === "string" ? profileName : undefined; +}; diff --git a/packages/auth/providers/oidc/test/profile.spec.ts b/packages/auth/providers/oidc/test/profile.spec.ts new file mode 100644 index 000000000..aefac3753 --- /dev/null +++ b/packages/auth/providers/oidc/test/profile.spec.ts @@ -0,0 +1,59 @@ +import type { Profile } from "@auth/core/types"; +import { afterEach, describe, expect, test, vi } from "vitest"; + +const mockEnv = vi.hoisted(() => ({ + AUTH_OIDC_NAME_ATTRIBUTE_OVERWRITE: undefined as string | undefined, +})); + +vi.mock("../../../env", () => ({ env: mockEnv })); + +import { extractProfileName, getProfileValueByPath } from "../profile"; + +afterEach(() => { + mockEnv.AUTH_OIDC_NAME_ATTRIBUTE_OVERWRITE = undefined; +}); + +describe("getProfileValueByPath", () => { + const profile = { + sub: "user-id", + resource_access: { + homarr: { + roles: ["admins"], + }, + }, + } satisfies Profile; + + test("returns a top-level claim", () => { + expect(getProfileValueByPath(profile, "sub")).toBe("user-id"); + }); + + test("returns a nested claim", () => { + expect(getProfileValueByPath(profile, "resource_access.homarr.roles")).toEqual(["admins"]); + }); + + test("returns undefined when the path does not exist", () => { + expect(getProfileValueByPath(profile, "resource_access.missing.roles")).toBeUndefined(); + }); +}); + +describe("extractProfileName", () => { + test("uses a nested name claim when configured", () => { + mockEnv.AUTH_OIDC_NAME_ATTRIBUTE_OVERWRITE = "user.profile.display_name"; + const profile = { + sub: "user-id", + user: { profile: { display_name: "Nested User" } }, + } satisfies Profile; + + expect(extractProfileName(profile)).toBe("Nested User"); + }); + + test("returns undefined when the configured claim is not a string", () => { + mockEnv.AUTH_OIDC_NAME_ATTRIBUTE_OVERWRITE = "user.profile"; + const profile = { + sub: "user-id", + user: { profile: { display_name: "Nested User" } }, + } satisfies Profile; + + expect(extractProfileName(profile)).toBeUndefined(); + }); +}); diff --git a/packages/auth/test/events.spec.ts b/packages/auth/test/events.spec.ts index c94df69b8..615dbac96 100644 --- a/packages/auth/test/events.spec.ts +++ b/packages/auth/test/events.spec.ts @@ -1,7 +1,7 @@ import type { ResponseCookie } from "next/dist/compiled/@edge-runtime/cookies"; import type { ReadonlyRequestCookies } from "next/dist/server/web/spec-extension/adapters/request-cookies"; import { cookies } from "next/headers"; -import { describe, expect, test, vi } from "vitest"; +import { afterEach, describe, expect, test, vi } from "vitest"; import { eq } from "@homarr/db"; import type { Database } from "@homarr/db"; @@ -21,6 +21,12 @@ vi.mock("../env", () => { env: mockEnv, }; }); + +afterEach(() => { + mockEnv.AUTH_OIDC_GROUPS_ATTRIBUTE = "someRandomGroupsKey"; + mockEnv.AUTH_OIDC_GROUPS_LOCAL_MANAGEMENT = false; +}); + // eslint-disable-next-line @typescript-eslint/consistent-type-imports type HeadersExport = typeof import("next/headers"); vi.mock("next/headers", async (importOriginal) => { @@ -152,6 +158,30 @@ describe("createSignInEventHandler should create signInEventHandler", () => { }); expect(dbGroupMembers?.groupId).toBe("1"); }); + test("should add missing group membership from a nested claim", async () => { + // Arrange + mockEnv.AUTH_OIDC_GROUPS_ATTRIBUTE = "resource_access.homarr.roles"; + const db = createDb(); + await createUserAsync(db); + await createGroupAsync(db); + const eventHandler = createSignInEventHandler(db); + + // Act + await eventHandler?.({ + user: { id: "1", name: "test" }, + profile: { + preferred_username: "test", + resource_access: { homarr: { roles: ["test"] } }, + }, + account: null, + }); + + // Assert + const dbGroupMembers = await db.query.groupMembers.findFirst({ + where: eq(groupMembers.userId, "1"), + }); + expect(dbGroupMembers?.groupId).toBe("1"); + }); test("should remove group membership", async () => { // Arrange const db = createDb();