Skip to content
Merged
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
1 change: 1 addition & 0 deletions integration-tests/testkit/scim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export type SCIMListQuery = {
count?: string | number;
startIndex?: string | number;
filter?: string;
excludedAttributes?: 'members';
};

export function createScimTestkit({ baseUrl, headers }: { baseUrl: string; headers: HeadersInit }) {
Expand Down
61 changes: 61 additions & 0 deletions integration-tests/tests/api/auth/scim.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2922,6 +2922,67 @@ describe.concurrent('/Groups', () => {
});
});
describe.concurrent('GET', () => {
test.concurrent(
'lists members by default and excludes them when requested',
async ({ expect }) => {
const seed = initSeed();
const owner = await seed.createOwner();
const org = await owner.createOrg();
await org.setFeatureFlag('scim', true);
const oidc = await org.createOIDCIntegration();
const domain = await oidc.registerFakeDomain();
const accessToken = await org.createOrganizationAccessToken({
permissions: ['scim:provision'],
resources: { mode: ResourceAssignmentModeType.Granular },
});
const scim = createScimTestkit({
baseUrl,
headers: {
'Content-Type': 'application/scim+json',
Authorization: 'Bearer ' + accessToken.privateAccessKey,
},
});
const user = await scim
.createUser({
...newUserValues(),
emails: [{ primary: true, type: 'work', value: 'listed-member@' + domain }],
})
.then(response => response.body);
const group = await scim
.createGroup({
...newGroupValues(),
displayName: 'Group with listed member',
members: [{ value: user.id }],
})
.then(response => response.body);
const expectedMembers = [
{
value: user.id,
$ref: baseUrl + '/scim/v2/Users/' + user.id,
},
];

const listResponse = await scim.listGroups();
expect(listResponse.body.Resources).toContainEqual(
expect.objectContaining({ id: group.id, members: expectedMembers }),
);

const filteredResponse = await scim.listGroups({
filter: `id eq "${group.id}"`,
});
expect(filteredResponse.body.Resources[0]?.members).toEqual(expectedMembers);

const excludedListResponse = await scim.listGroups({ excludedAttributes: 'members' });
expect(excludedListResponse.body.Resources[0]).not.toHaveProperty('members');

const excludedFilteredResponse = await scim.listGroups({
filter: `id eq "${group.id}"`,
excludedAttributes: 'members',
});
expect(excludedFilteredResponse.body.Resources[0]).not.toHaveProperty('members');
},
);

test.concurrent('excludes members when requested', async ({ expect }) => {
const seed = initSeed();
const owner = await seed.createOwner();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,28 @@ export class GroupMemberStore {
return z.array(GroupMemberModel).parse(result);
}

async getGroupMembersForOrganizationIdAndGroupIds(
organizationId: string,
groupIds: Array<string>,
) {
const result = await this.pool.any(psql`
SELECT ${groupMemberFields}
FROM "group_members"
WHERE
"organization_id" = ${organizationId}
AND "group_id" = ANY(${psql.array(groupIds, 'uuid')})
`);
const records = z.array(GroupMemberModel).parse(result);
const groupMembersByGroupId = new Map<string, Array<GroupMember>>();
for (const groupMember of records) {
const members = groupMembersByGroupId.get(groupMember.groupId) ?? [];
members.push(groupMember);
groupMembersByGroupId.set(groupMember.groupId, members);
}

return groupMembersByGroupId;
}

async addGroupMembersToGroupByOrganizationIdAndGroupId(
organizationId: string,
groupId: string,
Expand Down
39 changes: 31 additions & 8 deletions packages/services/server/src/scim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ const GetGroupQueryModel = z.object({
excludedAttributes: z.literal('members').optional(),
});

const GetGroupsQueryModel = QuerySchemaModel.merge(GetGroupQueryModel);

const SharedUserRouteParams = z.object({
userId: z.string().uuid(),
});
Expand Down Expand Up @@ -1194,7 +1196,7 @@ export const createSCIMPlugin =
return reply.status(result.error.status).send(result.error);
}

const queryParse = QuerySchemaModel.safeParse(req.query);
const queryParse = GetGroupsQueryModel.safeParse(req.query);
if (queryParse.error) {
return reply.status(403).send(
createSCIMError({
Expand All @@ -1205,6 +1207,7 @@ export const createSCIMPlugin =
}

const groupStore = new GroupStore(result.logger, pool);
const groupMemberStore = new GroupMemberStore(result.logger, pool);

const startIndex = queryParse.data.startIndex ?? 1;
const count = queryParse.data.count ?? 100;
Expand Down Expand Up @@ -1252,12 +1255,20 @@ export const createSCIMPlugin =
}
}

const groupMembers =
group && queryParse.data.excludedAttributes !== 'members'
? await groupMemberStore.getGroupMembersForOrganizationIdAndGroupId(
result.organizationId,
group.id,
)
: undefined;

return reply.status(200).send({
schemas: ['urn:ietf:params:scim:api:messages:2.0:ListResponse'],
totalResults: group ? 1 : 0,
startIndex,
itemsPerPage: group ? 1 : 0,
Resources: group ? [createSCIMGroupObjectFromGroup(baseUri, group)] : [],
Resources: group ? [createSCIMGroupObjectFromGroup(baseUri, group, groupMembers)] : [],
} satisfies SCIMListResponseObject);
}

Expand All @@ -1269,8 +1280,24 @@ export const createSCIMPlugin =
},
);

const groupMembersByGroupId =
queryParse.data.excludedAttributes !== 'members' && pagedGroups.length > 0
? await groupMemberStore.getGroupMembersForOrganizationIdAndGroupIds(
result.organizationId,
pagedGroups.map(group => group.id),
)
: null;

for (const group of pagedGroups) {
groups.push(createSCIMGroupObjectFromGroup(baseUri, group));
groups.push(
createSCIMGroupObjectFromGroup(
baseUri,
group,
groupMembersByGroupId === null
? undefined
: (groupMembersByGroupId.get(group.id) ?? []),
),
);
}

return reply.status(200).send({
Expand Down Expand Up @@ -1957,11 +1984,7 @@ function createSCIMGroupObjectFromGroup(
baseUri: string,
group: Group,
/**
* The members are optional as they do not need to be included within actions such as
* "list all groups".
*
* Only when a specific group object is requested or updated we include the list of members
* so the SCIM provider can see if a user is or is not a member of an organization.
* Members are omitted when the client requests `excludedAttributes=members`.
*/
members?: Array<GroupMember>,
): SCIMGroupObject {
Expand Down
Loading