From d341dc35c63fb402cfabf3a9ee1284e1ef54ba87 Mon Sep 17 00:00:00 2001 From: fflorent Date: Mon, 13 Jan 2025 17:55:28 +0100 Subject: [PATCH] Support POST /Groups --- app/gen-server/lib/homedb/GroupsManager.ts | 18 ++- app/gen-server/lib/homedb/UsersManager.ts | 15 ++ app/server/lib/scim/v2/BaseController.ts | 18 ++- app/server/lib/scim/v2/ScimGroupController.ts | 28 +++- app/server/lib/scim/v2/ScimUserController.ts | 13 +- app/server/lib/scim/v2/ScimUtils.ts | 29 +++- test/server/lib/Scim.ts | 144 +++++++++++++++++- 7 files changed, 245 insertions(+), 20 deletions(-) diff --git a/app/gen-server/lib/homedb/GroupsManager.ts b/app/gen-server/lib/homedb/GroupsManager.ts index cee614d7b3..b9699c8c2b 100644 --- a/app/gen-server/lib/homedb/GroupsManager.ts +++ b/app/gen-server/lib/homedb/GroupsManager.ts @@ -287,8 +287,8 @@ export class GroupsManager { const group = Group.create({ type: groupDescriptor.type, name: groupDescriptor.name, - memberUsers: await this._usersManager.getUsersByIds(groupDescriptor.memberUsers ?? [], manager), - memberGroups: await this._getGroupsByIds(groupDescriptor.memberGroups ?? [], manager), + memberUsers: await this._usersManager.getUsersByIdsStrict(groupDescriptor.memberUsers ?? [], manager), + memberGroups: await this._getGroupsByIdsStrict(groupDescriptor.memberGroups ?? [], manager), }); return await manager.save(group); }); @@ -306,8 +306,8 @@ export class GroupsManager { id, type: groupDescriptor.type, name: groupDescriptor.name, - memberUsers: await this._usersManager.getUsersByIds(groupDescriptor.memberUsers ?? [], manager), - memberGroups: await this._getGroupsByIds(groupDescriptor.memberGroups ?? [], manager), + memberUsers: await this._usersManager.getUsersByIdsStrict(groupDescriptor.memberUsers ?? [], manager), + memberGroups: await this._getGroupsByIdsStrict(groupDescriptor.memberGroups ?? [], manager), }); return await manager.save(updatedGroup); }); @@ -365,6 +365,16 @@ export class GroupsManager { }); } + private async _getGroupsByIdsStrict(groupIds: number[], optManager?: EntityManager): Promise { + const groups = await this._getGroupsByIds(groupIds, optManager); + if (groups.length !== groupIds.length) { + const foundGroupIds = new Set(groups.map(group => group.id)); + const missingGroupIds = groupIds.filter(id => !foundGroupIds.has(id)); + throw new ApiError('Groups not found: ' + missingGroupIds.join(', '), 404); + } + return groups; + } + private _getGroupsQueryBuilder(manager: EntityManager) { return manager.createQueryBuilder() .select('groups') diff --git a/app/gen-server/lib/homedb/UsersManager.ts b/app/gen-server/lib/homedb/UsersManager.ts index 5115aafcd5..f180fc1804 100644 --- a/app/gen-server/lib/homedb/UsersManager.ts +++ b/app/gen-server/lib/homedb/UsersManager.ts @@ -734,6 +734,21 @@ export class UsersManager { }); } + /** + * Returns a Promise for an array of User entites for the given userIds. + * Throws an error if any of the users are not found. + * This is useful when we expect all users to exist, and want to throw an error if they don't. + */ + public async getUsersByIdsStrict(userIds: number[], optManager?: EntityManager): Promise { + const users = await this.getUsersByIds(userIds, optManager); + if (users.length !== userIds.length) { + const foundUserIds = new Set(users.map(user => user.id)); + const missingUserIds = userIds.filter(userId => !foundUserIds.has(userId)); + throw new ApiError('Users not found: ' + missingUserIds.join(', '), 404); + } + return users; + } + /** * Don't add everyone@ as a guest, unless also sharing with anon@. * This means that material shared with everyone@ doesn't become diff --git a/app/server/lib/scim/v2/BaseController.ts b/app/server/lib/scim/v2/BaseController.ts index 73dd9f121a..2ebd6cc3dc 100644 --- a/app/server/lib/scim/v2/BaseController.ts +++ b/app/server/lib/scim/v2/BaseController.ts @@ -6,20 +6,22 @@ import { RequestContext } from 'app/server/lib/scim/v2/ScimTypes'; import SCIMMY from "scimmy"; export class BaseController { - protected static getIdFromResource(resource: any) { + protected logger = new LogMethods(this.constructor.name, () => ({})); + + constructor( + protected dbManager: HomeDBManager, + protected checkAccess: (context: RequestContext) => void, + private _invalidIdError: string + ) {} + + protected getIdFromResource(resource: any) { const id = parseInt(resource.id, 10); if (Number.isNaN(id)) { - throw new SCIMMY.Types.Error(400, 'invalidValue', 'Invalid passed group ID'); + throw new SCIMMY.Types.Error(400, 'invalidValue', this._invalidIdError); } return id; } - protected logger = new LogMethods(this.constructor.name, () => ({})); - - constructor( - protected dbManager: HomeDBManager, - protected checkAccess: (context: RequestContext) => void - ) {} /** * Runs the passed callback and handles any errors that might occur. diff --git a/app/server/lib/scim/v2/ScimGroupController.ts b/app/server/lib/scim/v2/ScimGroupController.ts index 1b24051dc7..6af71d69ed 100644 --- a/app/server/lib/scim/v2/ScimGroupController.ts +++ b/app/server/lib/scim/v2/ScimGroupController.ts @@ -2,11 +2,18 @@ import { Group } from 'app/gen-server/entity/Group'; import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; import { BaseController } from 'app/server/lib/scim/v2/BaseController'; import { RequestContext } from 'app/server/lib/scim/v2/ScimTypes'; -import { toSCIMMYGroup } from 'app/server/lib/scim/v2/ScimUtils'; +import { toGroupDescriptor, toSCIMMYGroup } from 'app/server/lib/scim/v2/ScimUtils'; import SCIMMY from 'scimmy'; class ScimGroupController extends BaseController { + public constructor( + dbManager: HomeDBManager, + checkAccess: (context: RequestContext) => void + ) { + super(dbManager, checkAccess, 'Invalid passed group ID'); + } + /** * Gets a single group with the passed ID. * @@ -15,7 +22,7 @@ class ScimGroupController extends BaseController { */ public async getSingleGroup(resource: any, context: RequestContext) { return this.runAndHandleErrors(context, async () => { - const id = ScimGroupController.getIdFromResource(resource); + const id = this.getIdFromResource(resource); const group = await this.dbManager.getGroupWithMembersById(id); if (!group || group.type !== Group.RESOURCE_USERS_TYPE) { throw new SCIMMY.Types.Error(404, null!, `Group with ID ${id} not found`); @@ -38,6 +45,20 @@ class ScimGroupController extends BaseController { return filter ? filter.match(scimmyGroup) : scimmyGroup; }); } + + /** + * Creates a new group with the passed data. + * + * @param data The data to create the group with + * @param context The request context + */ + public async createGroup(data: any, context: RequestContext) { + return this.runAndHandleErrors(context, async () => { + const groupDescriptor = toGroupDescriptor(data); + const group = await this.dbManager.createGroup(groupDescriptor); + return toSCIMMYGroup(group); + }); + } } export const getScimGroupConfig = ( @@ -52,5 +73,8 @@ export const getScimGroupConfig = ( } return await controller.getGroups(resource, context); }, + ingress: async (resource: any, data: any, context: RequestContext) => { + return await controller.createGroup(data, context); + }, }; }; diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts index 881f5aca8f..b10aa93eaa 100644 --- a/app/server/lib/scim/v2/ScimUserController.ts +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -7,6 +7,13 @@ import { toSCIMMYUser, toUserProfile } from 'app/server/lib/scim/v2/ScimUtils'; import SCIMMY from 'scimmy'; class ScimUserController extends BaseController { + public constructor( + dbManager: HomeDBManager, + checkAccess: (context: RequestContext) => void + ) { + super(dbManager, checkAccess, 'Invalid passed user ID'); + } + /** * Gets a single user with the passed ID. * @@ -15,7 +22,7 @@ class ScimUserController extends BaseController { */ public async getSingleUser(resource: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { - const id = BaseController.getIdFromResource(resource); + const id = this.getIdFromResource(resource); const user = await this.dbManager.getUser(id); if (!user) { throw new SCIMMY.Types.Error(404, null!, `User with ID ${id} not found`); @@ -64,7 +71,7 @@ class ScimUserController extends BaseController { */ public async overwriteUser(resource: any, data: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { - const id = BaseController.getIdFromResource(resource); + const id = this.getIdFromResource(resource); if (this.dbManager.getSpecialUserIds().includes(id)) { throw new SCIMMY.Types.Error(403, null!, 'System user modification not permitted.'); } @@ -82,7 +89,7 @@ class ScimUserController extends BaseController { */ public async deleteUser(resource: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { - const id = BaseController.getIdFromResource(resource); + const id = this.getIdFromResource(resource); if (this.dbManager.getSpecialUserIds().includes(id)) { throw new SCIMMY.Types.Error(403, null!, 'System user deletion not permitted.'); } diff --git a/app/server/lib/scim/v2/ScimUtils.ts b/app/server/lib/scim/v2/ScimUtils.ts index 940028ddaf..b3db7534d3 100644 --- a/app/server/lib/scim/v2/ScimUtils.ts +++ b/app/server/lib/scim/v2/ScimUtils.ts @@ -4,8 +4,11 @@ import { User } from "app/gen-server/entity/User"; import { Group } from "app/gen-server/entity/Group"; import SCIMMY from "scimmy"; import log from 'app/server/lib/log'; +import { GroupWithMembersDescriptor } from "app/gen-server/lib/homedb/Interfaces"; const SCIM_API_BASE_PATH = '/api/scim/v2'; +const SCIMMY_USER_TYPE = 'User'; +const SCIMMY_GROUP_TYPE = 'Group'; /** * Converts a user from your database to a SCIMMY user @@ -59,15 +62,37 @@ export function toSCIMMYGroup(group: Group) { value: String(member.id), display: member.name, $ref: `${SCIM_API_BASE_PATH}/Users/${member.id}`, - type: 'User', + type: SCIMMY_USER_TYPE, })), // As of 2025-01-12, we don't support nested groups, so it should always be empty ...group.memberGroups.map((member: any) => ({ value: String(member.id), display: member.name, $ref: `${SCIM_API_BASE_PATH}/Groups/${member.id}`, - type: 'Group', + type: SCIMMY_GROUP_TYPE, })), ], }); } + +function parseId(id: string, type: typeof SCIMMY_USER_TYPE | typeof SCIMMY_GROUP_TYPE): number { + const parsedId = parseInt(id, 10); + if (Number.isNaN(parsedId)) { + throw new SCIMMY.Types.Error(400, 'invalidValue', `Invalid ${type} member ID: ${id}`); + } + return parsedId; +} + +export function toGroupDescriptor(scimGroup: any): GroupWithMembersDescriptor { + const members = scimGroup.members ?? []; + return { + name: scimGroup.displayName, + type: Group.RESOURCE_USERS_TYPE, + memberUsers: members + .filter((member: any) => member.type === SCIMMY_USER_TYPE) + .map((member: any) => parseId(member.value, SCIMMY_USER_TYPE)), + memberGroups: members + .filter((member: any) => member.type === SCIMMY_GROUP_TYPE) + .map((member: any) => parseId(member.value, SCIMMY_GROUP_TYPE)), + }; +} diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index d92b21ec77..f4ece382b1 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -34,7 +34,7 @@ const USER_CONFIG_BY_NAME = { type UserConfigByName = typeof USER_CONFIG_BY_NAME; describe('Scim', () => { - testUtils.setTmpLogLevel('error'); + testUtils.setTmpLogLevel('alert'); const setupTestServer = (env: NodeJS.ProcessEnv) => { let homeUrl: string; @@ -192,6 +192,7 @@ describe('Scim', () => { sandbox.stub(getDbManager(), 'getGroupWithMembersById').throws(error); sandbox.stub(getDbManager(), 'getGroupsWithMembersByType').throws(error); sandbox.stub(getDbManager(), 'getGroupsWithMembers').throws(error); + sandbox.stub(getDbManager(), 'createGroup').throws(error); const res = await makeCallWith('chimpy'); assert.deepEqual(res.data, { @@ -758,6 +759,147 @@ describe('Scim', () => { checkCommonErrors('get', '/Groups'); }); + + describe('POST /Groups', function () { + it(`should create a new ${Group.RESOURCE_USERS_TYPE} group`, async function () { + await withGroupName('test-group', async (groupName) => { + const res = await axios.post(scimUrl('/Groups'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Group'], + displayName: groupName, + members: [ + { value: String(userIdByName['chimpy']), display: 'Chimpy', type: 'User' }, + { value: String(userIdByName['kiwi']), display: 'Kiwi', type: 'User' }, + ] + }, chimpy); + assert.equal(res.status, 201); + const newGroupId = parseInt(res.data.id); + assert.deepEqual(res.data, { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Group'], + id: String(newGroupId), + displayName: groupName, + members: [ + { value: '1', display: 'Chimpy', $ref: '/api/scim/v2/Users/1', type: 'User' }, + { value: '2', display: 'Kiwi', $ref: '/api/scim/v2/Users/2', type: 'User' }, + ], + meta: { resourceType: 'Group', location: `/api/scim/v2/Groups/${newGroupId}` } + }); + }); + }); + + it('should allow to create a group without members', function () { + return withGroupName('test-group-without-members', async (groupName) => { + const res = await axios.post(scimUrl('/Groups'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Group'], + displayName: groupName, + }, chimpy); + assert.equal(res.status, 201); + assert.deepEqual(res.data, { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Group'], + id: res.data.id, + displayName: groupName, + members: [], + meta: { resourceType: 'Group', location: `/api/scim/v2/Groups/${res.data.id}` } + }); + }); + }); + + it('should return 400 when the group name is missing', async function () { + const res = await axios.post(scimUrl('/Groups'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Group'], + members: [ + { value: String(userIdByName['chimpy']), display: 'Chimpy', type: 'User' }, + ] + }, chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '400', + detail: "Required attribute 'displayName' is missing", + scimType: 'invalidValue' + }); + assert.equal(res.status, 400); + }); + + it('should return 400 when the group members contain an invalid user id', async function () { + const res = await axios.post(scimUrl('/Groups'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Group'], + displayName: 'test-group', + members: [ + { value: 'not-an-id', display: 'Non-Existing User', type: 'User' }, + ] + }, chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '400', + detail: 'Invalid User member ID: not-an-id', + scimType: 'invalidValue' + }); + assert.equal(res.status, 400); + }); + + it('should return 400 when the group members contain an invalid group id', async function () { + const res = await axios.post(scimUrl('/Groups'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Group'], + displayName: 'test-group', + members: [ + { value: 'not-an-id', display: 'Non-Existing Group', type: 'Group' }, + ] + }, chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '400', + detail: 'Invalid Group member ID: not-an-id', + scimType: 'invalidValue' + }); + assert.equal(res.status, 400); + }); + + it('should return 404 when the group members contain a non-existing user id', async function () { + const res = await axios.post(scimUrl('/Groups'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Group'], + displayName: 'test-group', + members: [ + { value: String(userIdByName['chimpy']), display: 'Chimpy', + $ref: `/api/scim/v2/Users/${String(userIdByName['chimpy'])}`, type: 'User' }, + { value: '1000', display: 'Non-Existing User', type: 'User' }, + ] + }, chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '404', + detail: 'Users not found: 1000' + }); + assert.equal(res.status, 404); + }); + + it('should return 404 when the group members contain a non-existing group id', async function () { + const res = await axios.post(scimUrl('/Groups'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Group'], + displayName: 'test-group', + members: [ + { value: '1000', display: 'Non-Existing Group', type: 'Group' }, + ] + }, chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '404', + detail: 'Groups not found: 1000' + }); + assert.equal(res.status, 404); + }); + + checkCommonErrors('post', '/Groups', { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Group'], + displayName: 'test-group', + // We need to differ the moment we call userIdByName['chimpy'] (set during a "before()" hook) + get members() { + return [ + { value: String(userIdByName['chimpy']), display: 'Chimpy', type: 'User' }, + { value: String(userIdByName['kiwi']), display: 'Kiwi', type: 'User' }, + ]; + } + }); + }); + }); describe('POST /Bulk', function () {