Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 34 additions & 14 deletions npm/src/directory-sync/scim/DirectoryUsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,24 @@ export class DirectoryUsers {
}

public async create(directory: Directory, body: any): Promise<DirectorySyncResponse> {
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 = {
Expand Down Expand Up @@ -72,11 +83,15 @@ export class DirectoryUsers {
public async update(directory: Directory, user: User, body: any): Promise<DirectorySyncResponse> {
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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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',
},
};
Expand Down
26 changes: 21 additions & 5 deletions npm/src/directory-sync/scim/Users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@ export class Users extends Base {

// Create a new user
public async create(user: User & { directoryId: string }): Promise<Response<User>> {
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(
id,
user,
{
name: indexNames.directoryIdUsername,
value: keyFromParts(directoryId, email),
value: keyFromParts(directoryId, indexValue),
},
{
name: indexNames.directoryId,
Expand Down Expand Up @@ -115,7 +117,7 @@ export class Users extends Base {
}

// Update the user data
public async update(id: string, user: User): Promise<Response<User>> {
public async update(id: string, user: User, directoryId: string): Promise<Response<User>> {
const { raw } = user;

raw['id'] = id;
Expand All @@ -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);
Expand Down Expand Up @@ -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 };
Expand Down
87 changes: 87 additions & 0 deletions npm/test/dsync/users.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});