diff --git a/app/routes/auth/oidc-callback.ts b/app/routes/auth/oidc-callback.ts index 01b494f..15b79ce 100644 --- a/app/routes/auth/oidc-callback.ts +++ b/app/routes/auth/oidc-callback.ts @@ -74,6 +74,7 @@ export async function loader({ request, context, url }: Route.LoaderArgs) { }, { initialRole: claimedRole ?? config.oidc?.default_role, + syncRole: claimedRole, }, ); diff --git a/app/server/web/auth.ts b/app/server/web/auth.ts index fa48565..910675b 100644 --- a/app/server/web/auth.ts +++ b/app/server/web/auth.ts @@ -89,7 +89,7 @@ export interface AuthService { findOrCreateUser( subject: string, profile?: { name?: string; email?: string; picture?: string }, - options?: { initialRole?: string }, + options?: { initialRole?: string; syncRole?: string }, ): Promise; linkHeadscaleUser(userId: string, headscaleUserId: string): Promise; @@ -583,17 +583,21 @@ export function createAuthService(opts: AuthServiceOptions): AuthService { async function findOrCreateUser( subject: string, profile?: { name?: string; email?: string; picture?: string }, - options?: { initialRole?: string }, + options?: { initialRole?: string; syncRole?: string }, ): Promise { const [existing] = await opts.db.select().from(users).where(eq(users.sub, subject)).limit(1); if (existing) { + const syncedRole = normalizeInitialRole(options?.syncRole); await opts.db .update(users) .set({ name: profile?.name, email: profile?.email, picture: profile?.picture, + ...(syncedRole && existing.role !== "owner" + ? { role: syncedRole, caps: capsForRole(syncedRole) } + : {}), last_login_at: new Date(), updated_at: new Date(), }) diff --git a/config.example.yaml b/config.example.yaml index da40854..7f90821 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -288,10 +288,11 @@ integration: # # auditor, viewer, member. The owner role is only granted to the first user. # default_role: "member" # -# # Optional OIDC claim to read a Headplane role from when creating a new user. +# # Optional OIDC claim to read a Headplane role from when creating or syncing a user. # # This can be a string claim, or an array claim containing one of the valid # # roles above. If present and valid, it takes precedence over default_role. -# # Configure your IdP to map groups or client roles into this claim. +# # Configure your IdP to map groups or client roles into this claim. Existing +# # non-owner users are updated on their next OIDC sign-in when this claim changes. # role_claim: "headplane_role" # # # Allow ID token verification with legacy RSA keys smaller than 2048 bits. diff --git a/docs/features/sso.md b/docs/features/sso.md index 374039a..bc58723 100644 --- a/docs/features/sso.md +++ b/docs/features/sso.md @@ -257,10 +257,12 @@ valid roles. This lets providers such as Keycloak map groups or client roles to a final Headplane role before login. When both `role_claim` and `default_role` are configured, a valid role claim takes precedence for new users. -Automatic role assignment only applies when Headplane creates a user for the -first time. It does not overwrite roles that were already assigned in Headplane. -The **Owner** role is reserved for the first-login bootstrap and cannot be -granted by `default_role` or `role_claim`. +For users that already exist in Headplane, a valid `role_claim` is synced on +each OIDC login. If their IdP groups or client roles start matching a different +Headplane role, their Headplane permissions are updated at their next sign-in. +`default_role` remains a creation-time fallback only and does not overwrite +existing roles. The **Owner** role is reserved for the first-login bootstrap and +cannot be granted or overwritten by `default_role` or `role_claim`. ### API Key Sessions diff --git a/tests/unit/auth/auth-service.test.ts b/tests/unit/auth/auth-service.test.ts index 27240b7..b32cd88 100644 --- a/tests/unit/auth/auth-service.test.ts +++ b/tests/unit/auth/auth-service.test.ts @@ -65,6 +65,35 @@ describe("findOrCreateUser", () => { expect(user?.name).toBe("New"); expect(user?.email).toBe("new@test.com"); }); + + test("syncs an existing non-owner role when explicitly provided", async () => { + await auth.findOrCreateUser("sub-owner"); + await auth.findOrCreateUser("sub-1", { name: "User" }); + + await auth.findOrCreateUser("sub-1", { name: "User" }, { syncRole: "admin" }); + + const role = await auth.roleForSubject("sub-1"); + expect(role).toBe("admin"); + }); + + test("does not apply initial role to an existing user", async () => { + await auth.findOrCreateUser("sub-owner"); + await auth.findOrCreateUser("sub-1", { name: "User" }); + + await auth.findOrCreateUser("sub-1", { name: "User" }, { initialRole: "admin" }); + + const role = await auth.roleForSubject("sub-1"); + expect(role).toBe("member"); + }); + + test("does not sync the owner role", async () => { + await auth.findOrCreateUser("sub-owner"); + + await auth.findOrCreateUser("sub-owner", { name: "Owner" }, { syncRole: "admin" }); + + const role = await auth.roleForSubject("sub-owner"); + expect(role).toBe("owner"); + }); }); describe("linkHeadscaleUser", () => {