diff --git a/npm/src/directory-sync/scim/DirectoryUsers.ts b/npm/src/directory-sync/scim/DirectoryUsers.ts index 9e7991335..104b7731a 100644 --- a/npm/src/directory-sync/scim/DirectoryUsers.ts +++ b/npm/src/directory-sync/scim/DirectoryUsers.ts @@ -31,13 +31,24 @@ export class DirectoryUsers { } public async create(directory: Directory, body: any): Promise { + const userName = body?.userName; + + if (!userName || typeof userName !== 'string') { + return this.respondWithError({ code: 400, message: 'userName is required' }); + } + const userAttributes = extractStandardUserAttributes(body); - // Check if the user already exists - const { data: users } = await this.users.search(userAttributes.email, directory.id); + // Dedupe by userName (uniqueness=server) instead of email (uniqueness=none). + const { data: users } = await this.users.search(userName, directory.id); if (users && users.length > 0) { - return this.respondWithError({ code: 409, message: 'User already exists' }); + // RFC 7644 ยง3.12: scimType=uniqueness so the IdP retries as PATCH. + return this.respondWithError({ + code: 409, + message: 'User already exists', + scimType: 'uniqueness', + }); } const newUser = { @@ -72,11 +83,15 @@ export class DirectoryUsers { public async update(directory: Directory, user: User, body: any): Promise { const userAttributes = extractStandardUserAttributes(body); - const { data: updatedUser } = await this.users.update(user.id, { - ...userAttributes, - id: user.id, - raw: 'rawAttributes' in body ? body.rawAttributes : body, - }); + const { data: updatedUser } = await this.users.update( + user.id, + { + ...userAttributes, + id: user.id, + raw: 'rawAttributes' in body ? body.rawAttributes : body, + }, + directory.id + ); await sendEvent('user.updated', { directory, user: updatedUser }, this.callback); @@ -107,11 +122,15 @@ export class DirectoryUsers { }; } - const { data: updatedUser } = await this.users.update(user.id, { - ...user, - ...attributes, - raw: updateRawUserAttributes(user.raw, rawAttributes), - }); + const { data: updatedUser } = await this.users.update( + user.id, + { + ...user, + ...attributes, + raw: updateRawUserAttributes(user.raw, rawAttributes), + }, + directory.id + ); await sendEvent('user.updated', { directory, user: updatedUser }, this.callback); @@ -172,11 +191,12 @@ export class DirectoryUsers { }; } - private respondWithError(error: ApiError | null) { + private respondWithError(error: (ApiError & { scimType?: string }) | null) { return { status: error ? error.code : 500, data: { schemas: ['urn:ietf:params:scim:api:messages:2.0:Error'], + ...(error?.scimType ? { scimType: error.scimType } : {}), detail: error ? error.message : 'Internal Server Error', }, }; diff --git a/npm/src/directory-sync/scim/Users.ts b/npm/src/directory-sync/scim/Users.ts index 4ef1fabd5..990899781 100644 --- a/npm/src/directory-sync/scim/Users.ts +++ b/npm/src/directory-sync/scim/Users.ts @@ -11,7 +11,9 @@ export class Users extends Base { // Create a new user public async create(user: User & { directoryId: string }): Promise> { - const { directoryId, id, email } = user; + const { directoryId, id } = user; + // Index by userName (RFC 7643: uniqueness=server, caseExact=false) instead of email. + const indexValue = (user.raw?.userName || user.email).toLowerCase(); try { await this.store('users').put( @@ -19,7 +21,7 @@ export class Users extends Base { user, { name: indexNames.directoryIdUsername, - value: keyFromParts(directoryId, email), + value: keyFromParts(directoryId, indexValue), }, { name: indexNames.directoryId, @@ -115,7 +117,7 @@ export class Users extends Base { } // Update the user data - public async update(id: string, user: User): Promise> { + public async update(id: string, user: User, directoryId: string): Promise> { const { raw } = user; raw['id'] = id; @@ -125,8 +127,21 @@ export class Users extends Base { raw, }; + const indexValue = (updatedUser.raw?.userName || updatedUser.email).toLowerCase(); + try { - await this.store('users').put(id, updatedUser); + await this.store('users').put( + id, + updatedUser, + { + name: indexNames.directoryIdUsername, + value: keyFromParts(directoryId, indexValue), + }, + { + name: indexNames.directoryId, + value: directoryId, + } + ); return { data: updatedUser, error: null }; } catch (err: any) { return apiError(err); @@ -155,7 +170,8 @@ export class Users extends Base { try { const { data: users } = await this.store('users').getByIndex({ name: indexNames.directoryIdUsername, - value: keyFromParts(directoryId, userName), + // Lowercase to match the index built in create() (RFC 7643: caseExact=false). + value: keyFromParts(directoryId, userName.toLowerCase()), }); return { data: users, error: null }; diff --git a/npm/test/dsync/users.test.ts b/npm/test/dsync/users.test.ts index edffca3d6..0682b086f 100644 --- a/npm/test/dsync/users.test.ts +++ b/npm/test/dsync/users.test.ts @@ -571,4 +571,91 @@ tap.test('Directory users /', async (t) => { ); }); }); + + t.test('userName-based dedup contract /', async (t) => { + t.afterEach(async () => { + directorySync.users.setTenantAndProduct(directory.tenant, directory.product); + await directorySync.users.deleteAll(directory.id); + }); + + t.test('POST duplicate userName should return 409 with scimType=uniqueness', async (t) => { + await directorySync.requests.handle(requests.create(directory, users[0])); + + const { status, data } = await directorySync.requests.handle(requests.create(directory, users[0])); + + t.equal(status, 409); + t.equal(data.scimType, 'uniqueness'); + t.equal(data.detail, 'User already exists'); + }); + + t.test('POST same userName different email should return 409', async (t) => { + await directorySync.requests.handle(requests.create(directory, users[0])); + + const { status } = await directorySync.requests.handle( + requests.create(directory, { + ...users[0], + emails: [{ primary: true, value: 'different@example.com', type: 'work' }], + }) + ); + + t.equal(status, 409); + }); + + t.test('POST different userName same email should return 201 (LEV-956)', async (t) => { + await directorySync.requests.handle(requests.create(directory, users[0])); + + const { status } = await directorySync.requests.handle( + requests.create(directory, { + ...users[0], + userName: 'different-user@boxyhq.com', + }) + ); + + t.equal(status, 201); + }); + + t.test('POST case-insensitive userName match should return 409', async (t) => { + await directorySync.requests.handle(requests.create(directory, users[0])); + + const { status } = await directorySync.requests.handle( + requests.create(directory, { + ...users[0], + userName: users[0].userName.toUpperCase(), + }) + ); + + t.equal(status, 409); + }); + + t.test('PUT rename then POST with old userName should return 201', async (t) => { + const { data: created } = await directorySync.requests.handle(requests.create(directory, users[0])); + + await directorySync.requests.handle( + requests.updateById(directory, created.id, { + ...users[0], + userName: 'renamed@boxyhq.com', + }) + ); + + const { status } = await directorySync.requests.handle( + requests.create(directory, { + ...users[2], + userName: users[0].userName, + }) + ); + + t.equal(status, 201); + }); + + t.test('POST without userName should return 400', async (t) => { + const { userName, ...userWithoutUserName } = users[0]; + + const { status, data } = await directorySync.requests.handle( + requests.create(directory, userWithoutUserName) + ); + + t.equal(status, 400); + t.equal(data.detail, 'userName is required'); + }); + }); });