From 66ccbe4cd7a91f2c599731cd34b267d2265c7991 Mon Sep 17 00:00:00 2001 From: Chuck Driesler Date: Mon, 24 Feb 2025 11:39:36 +0000 Subject: [PATCH] chore(sso): clearer sso user taken error (#4054) * chore(sso): clearer error * fix(sso): safe user information --- .../server/modules/workspaces/errors/sso.ts | 18 ++++++++++++ .../server/modules/workspaces/rest/sso.ts | 29 ++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/packages/server/modules/workspaces/errors/sso.ts b/packages/server/modules/workspaces/errors/sso.ts index 2dab028eb..e90ef2d02 100644 --- a/packages/server/modules/workspaces/errors/sso.ts +++ b/packages/server/modules/workspaces/errors/sso.ts @@ -1,3 +1,5 @@ +import { UserEmail } from '@/modules/core/domain/userEmails/types' +import { User } from '@/modules/core/domain/users/types' import { BaseError } from '@/modules/shared/errors/base' export class SsoSessionMissingOrExpiredError extends BaseError { @@ -69,6 +71,22 @@ export class SsoUserClaimedError extends BaseError { static defaultMessage = 'OIDC provider user already associated with another Speckle account.' static code = 'SSO_USER_ALREADY_CLAIMED_ERROR' + constructor(params: { + currentUser: User + currentUserEmails: UserEmail[] + existingUser: User + existingUserEmail: string + }) { + super( + [ + 'User from SSO provider already exists as another Speckle user.', + `Currently signed in as ${params.currentUser.name}`, + `(${params.currentUserEmails.map((record) => record.email).join(',')})`, + `but attempted to sign in as ${params.existingUser.name}`, + `(${params.existingUserEmail})` + ].join(' ') + ) + } } export class SsoUserInviteRequiredError extends BaseError { diff --git a/packages/server/modules/workspaces/rest/sso.ts b/packages/server/modules/workspaces/rest/sso.ts index 6598d66e4..d48a3bb91 100644 --- a/packages/server/modules/workspaces/rest/sso.ts +++ b/packages/server/modules/workspaces/rest/sso.ts @@ -94,7 +94,10 @@ import { UpsertUserSsoSession } from '@/modules/workspaces/domain/sso/operations' import { GetUser } from '@/modules/core/domain/users/operations' -import { FindEmail } from '@/modules/core/domain/userEmails/operations' +import { + FindEmail, + FindEmailsByUserId +} from '@/modules/core/domain/userEmails/operations' import { buildAuthErrorRedirectUrl, buildAuthFinalizeRedirectUrl, @@ -288,7 +291,8 @@ export const getSsoRouter = (): Router => { getOidcProviderUserData: getOidcProviderUserDataFactory(), tryGetSpeckleUserData: tryGetSpeckleUserDataFactory({ findEmail: findEmailFactory({ db: trx }), - getUser: getUserFactory({ db: trx }) + getUser: getUserFactory({ db: trx }), + getUserEmails: findEmailsByUserIdFactory({ db: trx }) }), createWorkspaceUserFromSsoProfile: createWorkspaceUserFromSsoProfileFactory({ createUser: createUserFactory({ @@ -677,7 +681,15 @@ const getOidcProviderUserDataFactory = } const tryGetSpeckleUserDataFactory = - ({ findEmail, getUser }: { findEmail: FindEmail; getUser: GetUser }) => + ({ + findEmail, + getUser, + getUserEmails + }: { + findEmail: FindEmail + getUser: GetUser + getUserEmails: FindEmailsByUserId + }) => async ( req: Request, oidcProviderUserData: UserinfoResponse @@ -704,7 +716,16 @@ const tryGetSpeckleUserDataFactory = // Confirm existing user matches signed-in user, if both are present if (!!currentSessionUser && !!existingSpeckleUser) { if (currentSessionUser.id !== existingSpeckleUser.id) { - throw new SsoUserClaimedError() + const currentSessionUserEmails = await getUserEmails({ + userId: currentSessionUser.id + }) + + throw new SsoUserClaimedError({ + currentUser: currentSessionUser, + currentUserEmails: currentSessionUserEmails, + existingUser: existingSpeckleUser, + existingUserEmail: providerEmail + }) } }