From 01cea549a57c91a64ef77ac8f352dcec749e9cfd Mon Sep 17 00:00:00 2001 From: Greg Linklater Date: Fri, 17 Sep 2021 03:45:37 +0200 Subject: [PATCH] feat: map preferred_username claim to user record (#2569) --- .env.sample | 4 ++++ server/commands/accountProvisioner.js | 2 ++ server/commands/accountProvisioner.test.js | 7 +++++++ server/commands/userCreator.js | 5 ++++- server/commands/userCreator.test.js | 5 +++++ server/routes/auth/providers/oidc.js | 6 ++++++ server/test/factories.js | 1 + 7 files changed, 29 insertions(+), 1 deletion(-) diff --git a/.env.sample b/.env.sample index a2503804..874b2cef 100644 --- a/.env.sample +++ b/.env.sample @@ -89,6 +89,10 @@ OIDC_AUTH_URI= OIDC_TOKEN_URI= OIDC_USERINFO_URI= +# Specify which claims to derive user information from +# Supports any valid JSON path with the JWT payload +OIDC_USERNAME_CLAIM=preferred_username + # Display name for OIDC authentication OIDC_DISPLAY_NAME=OpenID Connect diff --git a/server/commands/accountProvisioner.js b/server/commands/accountProvisioner.js index 186ee948..55b08ee6 100644 --- a/server/commands/accountProvisioner.js +++ b/server/commands/accountProvisioner.js @@ -17,6 +17,7 @@ type Props = {| name: string, email: string, avatarUrl?: string, + username?: string, |}, team: {| name: string, @@ -74,6 +75,7 @@ export default async function accountProvisioner({ const result = await userCreator({ name: userParams.name, email: userParams.email, + username: userParams.username, isAdmin: isNewTeam, avatarUrl: userParams.avatarUrl, teamId: team.id, diff --git a/server/commands/accountProvisioner.test.js b/server/commands/accountProvisioner.test.js index 082216c0..8d4ce1fc 100644 --- a/server/commands/accountProvisioner.test.js +++ b/server/commands/accountProvisioner.test.js @@ -32,6 +32,7 @@ describe("accountProvisioner", () => { name: "Jenny Tester", email: "jenny@example.com", avatarUrl: "https://example.com/avatar.png", + username: "jtester", }, team: { name: "New team", @@ -57,6 +58,7 @@ describe("accountProvisioner", () => { expect(auth.scopes[0]).toEqual("read"); expect(team.name).toEqual("New team"); expect(user.email).toEqual("jenny@example.com"); + expect(user.username).toEqual("jtester"); expect(isNewUser).toEqual(true); expect(isNewTeam).toEqual(true); expect(mailer.sendTemplate).toHaveBeenCalled(); @@ -73,6 +75,7 @@ describe("accountProvisioner", () => { const authentications = await existing.getAuthentications(); const authentication = authentications[0]; const newEmail = "test@example.com"; + const newUsername = "tname"; const { user, isNewUser, isNewTeam } = await accountProvisioner({ ip, @@ -80,6 +83,7 @@ describe("accountProvisioner", () => { name: existing.name, email: newEmail, avatarUrl: existing.avatarUrl, + username: newUsername, }, team: { name: existingTeam.name, @@ -102,6 +106,7 @@ describe("accountProvisioner", () => { expect(auth.scopes.length).toEqual(1); expect(auth.scopes[0]).toEqual("read"); expect(user.email).toEqual(newEmail); + expect(user.username).toEqual(newUsername); expect(isNewTeam).toEqual(false); expect(isNewUser).toEqual(false); expect(mailer.sendTemplate).not.toHaveBeenCalled(); @@ -162,6 +167,7 @@ describe("accountProvisioner", () => { name: "Jenny Tester", email: "jenny@example.com", avatarUrl: "https://example.com/avatar.png", + username: "jtester", }, team: { name: team.name, @@ -186,6 +192,7 @@ describe("accountProvisioner", () => { expect(auth.scopes.length).toEqual(1); expect(auth.scopes[0]).toEqual("read"); expect(user.email).toEqual("jenny@example.com"); + expect(user.username).toEqual("jtester"); expect(isNewUser).toEqual(true); expect(mailer.sendTemplate).toHaveBeenCalled(); diff --git a/server/commands/userCreator.js b/server/commands/userCreator.js index 8b41c60c..31fdeb37 100644 --- a/server/commands/userCreator.js +++ b/server/commands/userCreator.js @@ -14,6 +14,7 @@ type UserCreatorResult = {| export default async function userCreator({ name, email, + username, isAdmin, avatarUrl, teamId, @@ -22,6 +23,7 @@ export default async function userCreator({ }: {| name: string, email: string, + username?: string, isAdmin?: boolean, avatarUrl?: string, teamId: string, @@ -63,7 +65,7 @@ export default async function userCreator({ } if (user) { - await user.update({ email }); + await user.update({ email, username }); await auth.update(rest); return { user, authentication: auth, isNewUser: false }; @@ -128,6 +130,7 @@ export default async function userCreator({ { name, email, + username, isAdmin, teamId, avatarUrl, diff --git a/server/commands/userCreator.test.js b/server/commands/userCreator.test.js index b4781de2..3d37191b 100644 --- a/server/commands/userCreator.test.js +++ b/server/commands/userCreator.test.js @@ -13,10 +13,12 @@ describe("userCreator", () => { const authentications = await existing.getAuthentications(); const existingAuth = authentications[0]; const newEmail = "test@example.com"; + const newUsername = "tname"; const result = await userCreator({ name: existing.name, email: newEmail, + username: newUsername, avatarUrl: existing.avatarUrl, teamId: existing.teamId, ip, @@ -34,6 +36,7 @@ describe("userCreator", () => { expect(authentication.scopes.length).toEqual(1); expect(authentication.scopes[0]).toEqual("read"); expect(user.email).toEqual(newEmail); + expect(user.username).toEqual(newUsername); expect(isNewUser).toEqual(false); }); @@ -101,6 +104,7 @@ describe("userCreator", () => { const result = await userCreator({ name: "Test Name", email: "test@example.com", + username: "tname", teamId: team.id, ip, authentication: { @@ -117,6 +121,7 @@ describe("userCreator", () => { expect(authentication.scopes.length).toEqual(1); expect(authentication.scopes[0]).toEqual("read"); expect(user.email).toEqual("test@example.com"); + expect(user.username).toEqual("tname"); expect(isNewUser).toEqual(true); }); diff --git a/server/routes/auth/providers/oidc.js b/server/routes/auth/providers/oidc.js index 57ede3dc..d5128191 100644 --- a/server/routes/auth/providers/oidc.js +++ b/server/routes/auth/providers/oidc.js @@ -2,6 +2,7 @@ import passport from "@outlinewiki/koa-passport"; import fetch from "fetch-with-proxy"; import Router from "koa-router"; +import get from "lodash/get"; import { Strategy } from "passport-oauth2"; import accountProvisioner from "../../../commands/accountProvisioner"; import env from "../../../env"; @@ -22,6 +23,8 @@ const OIDC_AUTH_URI = process.env.OIDC_AUTH_URI; const OIDC_TOKEN_URI = process.env.OIDC_TOKEN_URI; const OIDC_USERINFO_URI = process.env.OIDC_USERINFO_URI; const OIDC_SCOPES = process.env.OIDC_SCOPES || ""; +const OIDC_USERNAME_CLAIM = + process.env.OIDC_USERNAME_CLAIM || "preferred_username"; const allowedDomains = getAllowedDomains(); export const config = { @@ -103,6 +106,9 @@ if (OIDC_CLIENT_ID) { name: profile.name, email: profile.email, avatarUrl: profile.picture, + // Claim name can be overriden using an env variable. + // Default is 'preferred_username' as per OIDC spec. + username: get(profile, OIDC_USERNAME_CLAIM), }, authenticationProvider: { name: providerName, diff --git a/server/test/factories.js b/server/test/factories.js index bc180dad..7fdf2ce4 100644 --- a/server/test/factories.js +++ b/server/test/factories.js @@ -104,6 +104,7 @@ export async function buildUser(overrides: Object = {}) { { email: `user${count}@example.com`, name: `User ${count}`, + username: `user${count}`, createdAt: new Date("2018-01-01T00:00:00.000Z"), lastActiveAt: new Date("2018-01-01T00:00:00.000Z"), authentications: [