From b47c66f41002629843346e3506a5cedf426cc608 Mon Sep 17 00:00:00 2001 From: fflorent Date: Thu, 29 Aug 2024 10:13:48 +0200 Subject: [PATCH 01/27] WIP --- app/server/lib/scim/index.ts | 24 +++++++++++++++++++++ app/server/lib/scim/v2/users.ts | 37 +++++++++++++++++++++++++++++++++ package.json | 1 + yarn.lock | 15 ++++++++++++- 4 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 app/server/lib/scim/index.ts create mode 100644 app/server/lib/scim/v2/users.ts diff --git a/app/server/lib/scim/index.ts b/app/server/lib/scim/index.ts new file mode 100644 index 0000000000..54031c3531 --- /dev/null +++ b/app/server/lib/scim/index.ts @@ -0,0 +1,24 @@ +import * as express from 'express'; +import { buildUsersRoute, checkPermissionToUsersEndpoint } from './v2/users'; +import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; +import SCIMMY from "scimmy"; +import SCIMMYRouters from "scimmy-routers"; + +type SCIMMYResource = typeof SCIMMY.Types.Resource; + +const buildScimRouter = (dbManager: HomeDBManager) => { + const v2 = express.Router(); + v2.use('/Users', checkPermissionToUsersEndpoint, buildUsersRoute(dbManager)); + + SCIMMY.Resources.User.ingress(handler) + SCIMMY.Resources.declare(SCIMMY.Resources.User) + .ingress((resource: SCIMMYResource, data) => { + + + }); + const scim = express.Router(); + scim.use('/v2', v2); + return scim; +}; + +export { buildScimRouter }; diff --git a/app/server/lib/scim/v2/users.ts b/app/server/lib/scim/v2/users.ts new file mode 100644 index 0000000000..1b4175bdac --- /dev/null +++ b/app/server/lib/scim/v2/users.ts @@ -0,0 +1,37 @@ +import express, { NextFunction, Request, Response } from 'express'; +import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; +import { expressWrap } from '../../expressWrap'; +import { integerParam } from '../../requestUtils'; +import { ApiError } from 'app/common/ApiError'; +import { RequestWithLogin } from '../../Authorizer'; + +function checkPermissionToUsersEndpoint(req: Request, res: Response, next: NextFunction) { + const mreq = req as RequestWithLogin; + const adminEmail = process.env.GRIST_DEFAULT_EMAIL; + if (!adminEmail || mreq.user?.loginEmail !== adminEmail) { + throw new ApiError('Permission denied', 403); + } + return next(); +} + +const buildUsersRoute = (dbManager: HomeDBManager) => { + const userRoute = express.Router(); + + async function findUserOrFail(userId: number) { + const user = await dbManager.getUser(userId); + if (!user) { + throw new ApiError('User not found', 404); + } + return user; + } + + + userRoute.get('/:id', expressWrap(async (req, res) => { + const userId = integerParam(req.params.id, 'id'); + const user = await findUserOrFail(userId); + res.status(200).json(user); + })); + return userRoute; +}; + +export { buildUsersRoute, checkPermissionToUsersEndpoint }; diff --git a/package.json b/package.json index 43fb32b9cc..5c5b4a7591 100644 --- a/package.json +++ b/package.json @@ -189,6 +189,7 @@ "redis": "3.1.1", "redlock": "3.1.2", "saml2-js": "4.0.2", + "scimmy-routers": "^1.2.0", "short-uuid": "3.1.1", "slugify": "1.6.6", "swagger-ui-dist": "5.11.0", diff --git a/yarn.lock b/yarn.lock index f1f54e2599..9d30a6587b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3799,7 +3799,7 @@ express-rate-limit@7.2.0: resolved "https://registry.yarnpkg.com/express-rate-limit/-/express-rate-limit-7.2.0.tgz#06ce387dd5388f429cab8263c514fc07bf90a445" integrity sha512-T7nul1t4TNyfZMJ7pKRKkdeVJWa2CqB8NA1P8BwYaoDI5QSBZARv5oMS43J7b7I5P+4asjVXjb7ONuwDKucahg== -express@4.19.2: +express@4.19.2, express@^4.18.2: version "4.19.2" resolved "https://registry.yarnpkg.com/express/-/express-4.19.2.tgz#e25437827a3aa7f2a827bc8171bbbb664a356465" integrity sha512-5T6nhjsT+EOMzuck8JjBHARTHfMht0POzlA60WV2pMD3gyXw2LZnZ+ueGdNxG+0calOJcWKbpFcuzLZ91YWq9Q== @@ -7344,6 +7344,19 @@ schema-utils@^3.2.0: ajv "^6.12.5" ajv-keywords "^3.5.2" +scimmy-routers@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/scimmy-routers/-/scimmy-routers-1.2.0.tgz#42090c616f127aefce194ebf4a3c8f4a4f62e30b" + integrity sha512-+dT8yRglz2gMu0X1LlUYTi/PDgW6Zzu1YRWiv362I/wA64kud24XjYIoufNiW5OhskvBPQGT/P1aYOffcmxxsQ== + dependencies: + express "^4.18.2" + scimmy "^1.2.0" + +scimmy@^1.2.0: + version "1.2.3" + resolved "https://registry.yarnpkg.com/scimmy/-/scimmy-1.2.3.tgz#56ca9dbf11749b272e18090c923f81dbad4bc911" + integrity sha512-16oXCvnieVeKxTDQqi275bLuyOCwXci8Jywm2/M+4dWNNYoduUz0Crj1nFY0ETYMsuYvCdareWov6/Mebu92xA== + selenium-webdriver@^4.20.0: version "4.20.0" resolved "https://registry.yarnpkg.com/selenium-webdriver/-/selenium-webdriver-4.20.0.tgz#14941ab4a59e8956a5e4b4491a8ba2bd6619d1ac" From 163ade3d8f20aa6871441b152ac9eec274714422 Mon Sep 17 00:00:00 2001 From: fflorent Date: Mon, 2 Sep 2024 17:50:42 +0200 Subject: [PATCH 02/27] SCIM: Implement egress + tests --- app/gen-server/lib/homedb/HomeDBManager.ts | 6 +- app/gen-server/lib/homedb/UsersManager.ts | 6 +- app/server/MergedServer.ts | 1 + app/server/lib/FlexServer.ts | 7 ++ app/server/lib/scim/index.ts | 20 ++---- app/server/lib/scim/v2/ScimUserUtils.ts | 31 +++++++++ app/server/lib/scim/v2/ScimV2Api.ts | 76 ++++++++++++++++++++++ app/server/lib/scim/v2/users.ts | 37 ----------- package.json | 3 +- test/gen-server/seed.ts | 1 + test/server/lib/Authorizer.ts | 1 + yarn.lock | 16 ++--- 12 files changed, 142 insertions(+), 63 deletions(-) create mode 100644 app/server/lib/scim/v2/ScimUserUtils.ts create mode 100644 app/server/lib/scim/v2/ScimV2Api.ts delete mode 100644 app/server/lib/scim/v2/users.ts diff --git a/app/gen-server/lib/homedb/HomeDBManager.ts b/app/gen-server/lib/homedb/HomeDBManager.ts index b1d84bce12..a5d962d701 100644 --- a/app/gen-server/lib/homedb/HomeDBManager.ts +++ b/app/gen-server/lib/homedb/HomeDBManager.ts @@ -452,6 +452,10 @@ export class HomeDBManager extends EventEmitter { return this._usersManager.getUser(userId, options); } + public async getUsers() { + return this._usersManager.getUsers(); + } + public async getFullUser(userId: number) { return this._usersManager.getFullUser(userId); } @@ -3599,7 +3603,7 @@ export class HomeDBManager extends EventEmitter { // Get the user objects which map to non-null values in the userDelta. const userIds = Object.keys(userDelta).filter(userId => userDelta[userId]) .map(userIdStr => parseInt(userIdStr, 10)); - const users = await this._usersManager.getUsers(userIds, manager); + const users = await this._usersManager.getUsersByIds(userIds, manager); // Add unaffected users to the delta so that we have a record of where they are. groups.forEach(grp => { diff --git a/app/gen-server/lib/homedb/UsersManager.ts b/app/gen-server/lib/homedb/UsersManager.ts index 6a1e8bd386..edfdf520fc 100644 --- a/app/gen-server/lib/homedb/UsersManager.ts +++ b/app/gen-server/lib/homedb/UsersManager.ts @@ -685,10 +685,14 @@ export class UsersManager { return [this.getSupportUserId(), this.getAnonymousUserId(), this.getEveryoneUserId()]; } + public async getUsers() { + return await User.find({relations: ["logins"]}); + } + /** * Returns a Promise for an array of User entites for the given userIds. */ - public async getUsers(userIds: number[], optManager?: EntityManager): Promise { + public async getUsersByIds(userIds: number[], optManager?: EntityManager): Promise { if (userIds.length === 0) { return []; } diff --git a/app/server/MergedServer.ts b/app/server/MergedServer.ts index 33cf2b0bac..1a16e9321a 100644 --- a/app/server/MergedServer.ts +++ b/app/server/MergedServer.ts @@ -164,6 +164,7 @@ export class MergedServer { this.flexServer.addUpdatesCheck(); // todo: add support for home api to standalone app this.flexServer.addHomeApi(); + this.flexServer.addScimApi(); this.flexServer.addBillingApi(); this.flexServer.addNotifier(); this.flexServer.addAuditLogger(); diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 2d81551972..60c2faa0e7 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -88,6 +88,7 @@ import * as path from 'path'; import * as serveStatic from "serve-static"; import {ConfigBackendAPI} from "app/server/lib/ConfigBackendAPI"; import {IGristCoreConfig} from "app/server/lib/configCore"; +import { buildScimRouter } from './scim'; // Health checks are a little noisy in the logs, so we don't show them all. // We show the first N health checks: @@ -896,6 +897,12 @@ export class FlexServer implements GristServer { new ApiServer(this, this.app, this._dbManager, this._widgetRepository = buildWidgetRepository(this)); } + public addScimApi() { + if (this._check('scim', 'api', 'homedb', 'json', 'api-mw')) { return; } + this.app.use('/api/scim', buildScimRouter(this._dbManager, this._installAdmin)); + } + + public addBillingApi() { if (this._check('billing-api', 'homedb', 'json', 'api-mw')) { return; } this.getBilling().addEndpoints(this.app); diff --git a/app/server/lib/scim/index.ts b/app/server/lib/scim/index.ts index 54031c3531..2ee49303f4 100644 --- a/app/server/lib/scim/index.ts +++ b/app/server/lib/scim/index.ts @@ -1,21 +1,11 @@ import * as express from 'express'; -import { buildUsersRoute, checkPermissionToUsersEndpoint } from './v2/users'; -import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; -import SCIMMY from "scimmy"; -import SCIMMYRouters from "scimmy-routers"; - -type SCIMMYResource = typeof SCIMMY.Types.Resource; - -const buildScimRouter = (dbManager: HomeDBManager) => { - const v2 = express.Router(); - v2.use('/Users', checkPermissionToUsersEndpoint, buildUsersRoute(dbManager)); - - SCIMMY.Resources.User.ingress(handler) - SCIMMY.Resources.declare(SCIMMY.Resources.User) - .ingress((resource: SCIMMYResource, data) => { +import { buildScimRouterv2 } from './v2/ScimV2Api'; +import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; +import { InstallAdmin } from '../InstallAdmin'; - }); +const buildScimRouter = (dbManager: HomeDBManager, installAdmin: InstallAdmin) => { + const v2 = buildScimRouterv2(dbManager, installAdmin); const scim = express.Router(); scim.use('/v2', v2); return scim; diff --git a/app/server/lib/scim/v2/ScimUserUtils.ts b/app/server/lib/scim/v2/ScimUserUtils.ts new file mode 100644 index 0000000000..b9db4bf0de --- /dev/null +++ b/app/server/lib/scim/v2/ScimUserUtils.ts @@ -0,0 +1,31 @@ +import { User } from "app/gen-server/entity/User.js"; +import { SCIMMY } from "scimmy-routers"; + +/** + * Converts a user from your database to a SCIMMY user + */ +export function toSCIMMYUser(user: User) { + if (!user.logins) { + throw new Error("User must have at least one login"); + } + const preferredLanguage = user.options?.locale ?? "en"; + return new SCIMMY.Schemas.User({ + id: String(user.id), + userName: user.loginEmail, + displayName: user.name, + name: { + formatted: user.name, + }, + preferredLanguage, + locale: preferredLanguage, // Assume locale is the same as preferredLanguage + photos: user.picture ? [{ + value: user.picture, + type: "photo", + primary: true + }] : undefined, + emails: [{ + value: user.loginEmail, + primary: true, + }], + }); +} diff --git a/app/server/lib/scim/v2/ScimV2Api.ts b/app/server/lib/scim/v2/ScimV2Api.ts new file mode 100644 index 0000000000..05026251ec --- /dev/null +++ b/app/server/lib/scim/v2/ScimV2Api.ts @@ -0,0 +1,76 @@ +import * as express from 'express'; +import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; +import SCIMMY from "scimmy"; +import SCIMMYRouters from "scimmy-routers"; +import { RequestWithLogin } from '../../Authorizer'; +import { InstallAdmin } from '../../InstallAdmin'; +import { toSCIMMYUser } from './ScimUserUtils'; + +const WHITELISTED_PATHS_FOR_NON_ADMINS = [ "/Me", "/Schemas", "/ResourceTypes", "/ServiceProviderConfig" ]; + +async function isAuthorizedAction(mreq: RequestWithLogin, installAdmin: InstallAdmin): Promise { + const isAdmin = await installAdmin.isAdminReq(mreq) + const isScimUser = Boolean(process.env.GRIST_SCIM_EMAIL && mreq.user?.loginEmail === process.env.GRIST_SCIM_EMAIL); + return isAdmin || isScimUser || WHITELISTED_PATHS_FOR_NON_ADMINS.includes(mreq.path); +} + +const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) => { + const v2 = express.Router(); + + SCIMMY.Resources.declare(SCIMMY.Resources.User, { + egress: async (resource: any) => { + const { id, filter } = resource; + if (id) { + const user = await dbManager.getUser(id); + if (!user) { + throw new SCIMMY.Types.Error(404, null!, `User with ID ${id} not found`); + } + return toSCIMMYUser(user); + } + const scimmyUsers = (await dbManager.getUsers()).map(user => toSCIMMYUser(user)); + return filter ? filter.match(scimmyUsers) : scimmyUsers; + }, + ingress: async (resource: any) => { + try { + const { id } = resource; + if (id) { + return null; + } + return []; + } catch (ex) { + // FIXME: remove this + if (Math.random() > 1) { + return null; + } + throw ex; + } + }, + degress: () => { + return null; + } + }); + + const scimmyRouter = new SCIMMYRouters({ + type: 'bearer', + handler: async (request: express.Request) => { + const mreq = request as RequestWithLogin; + if (mreq.userId === undefined) { + // Note that any Error thrown here is automatically converted into a 403 response by SCIMMYRouters. + throw new Error('You are not authorized to access this resource!'); + } + + if (mreq.userId === dbManager.getAnonymousUserId()) { + throw new Error('Anonymous user cannot access SCIM resources'); + } + + if (!await isAuthorizedAction(mreq, installAdmin)) { + throw new SCIMMY.Types.Error(403, null!, 'Resource disallowed for non-admin users'); + } + return String(mreq.userId); // HACK: SCIMMYRouters requires the userId to be a string. + } + }) as express.Router; // Have to cast it into express.Router. See https://github.com/scimmyjs/scimmy-routers/issues/24 + + return v2.use('/', scimmyRouter); +}; + +export { buildScimRouterv2 }; diff --git a/app/server/lib/scim/v2/users.ts b/app/server/lib/scim/v2/users.ts deleted file mode 100644 index 1b4175bdac..0000000000 --- a/app/server/lib/scim/v2/users.ts +++ /dev/null @@ -1,37 +0,0 @@ -import express, { NextFunction, Request, Response } from 'express'; -import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; -import { expressWrap } from '../../expressWrap'; -import { integerParam } from '../../requestUtils'; -import { ApiError } from 'app/common/ApiError'; -import { RequestWithLogin } from '../../Authorizer'; - -function checkPermissionToUsersEndpoint(req: Request, res: Response, next: NextFunction) { - const mreq = req as RequestWithLogin; - const adminEmail = process.env.GRIST_DEFAULT_EMAIL; - if (!adminEmail || mreq.user?.loginEmail !== adminEmail) { - throw new ApiError('Permission denied', 403); - } - return next(); -} - -const buildUsersRoute = (dbManager: HomeDBManager) => { - const userRoute = express.Router(); - - async function findUserOrFail(userId: number) { - const user = await dbManager.getUser(userId); - if (!user) { - throw new ApiError('User not found', 404); - } - return user; - } - - - userRoute.get('/:id', expressWrap(async (req, res) => { - const userId = integerParam(req.params.id, 'id'); - const user = await findUserOrFail(userId); - res.status(200).json(user); - })); - return userRoute; -}; - -export { buildUsersRoute, checkPermissionToUsersEndpoint }; diff --git a/package.json b/package.json index 5c5b4a7591..4c80c1efdf 100644 --- a/package.json +++ b/package.json @@ -189,7 +189,8 @@ "redis": "3.1.1", "redlock": "3.1.2", "saml2-js": "4.0.2", - "scimmy-routers": "^1.2.0", + "scimmy": "1.2.3", + "scimmy-routers": "1.2.1", "short-uuid": "3.1.1", "slugify": "1.6.6", "swagger-ui-dist": "5.11.0", diff --git a/test/gen-server/seed.ts b/test/gen-server/seed.ts index 121a01ac15..11b6f4cfd1 100644 --- a/test/gen-server/seed.ts +++ b/test/gen-server/seed.ts @@ -631,6 +631,7 @@ export async function createServer(port: number, initDb = createInitialDb): Prom flexServer.addAccessMiddleware(); flexServer.addApiMiddleware(); flexServer.addHomeApi(); + flexServer.addScimApi(); flexServer.addApiErrorHandlers(); await initDb(flexServer.getHomeDBManager().connection); flexServer.summary(); diff --git a/test/server/lib/Authorizer.ts b/test/server/lib/Authorizer.ts index 7a43e33e27..8dc2c1ecf1 100644 --- a/test/server/lib/Authorizer.ts +++ b/test/server/lib/Authorizer.ts @@ -35,6 +35,7 @@ async function activateServer(home: FlexServer, docManager: DocManager) { await home.addLandingPages(); home.addHomeApi(); home.addAuditLogger(); + home.addScimApi(); await home.addTelemetry(); await home.addDoc(); home.addApiErrorHandlers(); diff --git a/yarn.lock b/yarn.lock index 9d30a6587b..16b6db082d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3799,7 +3799,7 @@ express-rate-limit@7.2.0: resolved "https://registry.yarnpkg.com/express-rate-limit/-/express-rate-limit-7.2.0.tgz#06ce387dd5388f429cab8263c514fc07bf90a445" integrity sha512-T7nul1t4TNyfZMJ7pKRKkdeVJWa2CqB8NA1P8BwYaoDI5QSBZARv5oMS43J7b7I5P+4asjVXjb7ONuwDKucahg== -express@4.19.2, express@^4.18.2: +express@4.19.2, express@^4.19.2: version "4.19.2" resolved "https://registry.yarnpkg.com/express/-/express-4.19.2.tgz#e25437827a3aa7f2a827bc8171bbbb664a356465" integrity sha512-5T6nhjsT+EOMzuck8JjBHARTHfMht0POzlA60WV2pMD3gyXw2LZnZ+ueGdNxG+0calOJcWKbpFcuzLZ91YWq9Q== @@ -7344,15 +7344,15 @@ schema-utils@^3.2.0: ajv "^6.12.5" ajv-keywords "^3.5.2" -scimmy-routers@^1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/scimmy-routers/-/scimmy-routers-1.2.0.tgz#42090c616f127aefce194ebf4a3c8f4a4f62e30b" - integrity sha512-+dT8yRglz2gMu0X1LlUYTi/PDgW6Zzu1YRWiv362I/wA64kud24XjYIoufNiW5OhskvBPQGT/P1aYOffcmxxsQ== +scimmy-routers@1.2.1: + version "1.2.1" + resolved "https://registry.yarnpkg.com/scimmy-routers/-/scimmy-routers-1.2.1.tgz#a535ced83f051b2a0374e8df02da6e17424e8be0" + integrity sha512-PDX4/mwOScSd+TjUPX0k3gH6v50WeYVgK1bisZ8f3p3eyJ0Qy4qYebDo6gzHqYCBjXNQniQxGSQvtlvG22NLdA== dependencies: - express "^4.18.2" - scimmy "^1.2.0" + express "^4.19.2" + scimmy "^1.2.3" -scimmy@^1.2.0: +scimmy@1.2.3, scimmy@^1.2.3: version "1.2.3" resolved "https://registry.yarnpkg.com/scimmy/-/scimmy-1.2.3.tgz#56ca9dbf11749b272e18090c923f81dbad4bc911" integrity sha512-16oXCvnieVeKxTDQqi275bLuyOCwXci8Jywm2/M+4dWNNYoduUz0Crj1nFY0ETYMsuYvCdareWov6/Mebu92xA== From 642bca8d6ecc1d3c4e994cd4f7e2b3b148e70726 Mon Sep 17 00:00:00 2001 From: fflorent Date: Tue, 3 Sep 2024 23:53:25 +0200 Subject: [PATCH 03/27] Implement ingress --- app/gen-server/lib/homedb/HomeDBManager.ts | 4 +++ app/gen-server/lib/homedb/UsersManager.ts | 27 ++++++++++++++- app/server/lib/scim/v2/ScimUserUtils.ts | 25 +++++++++++--- app/server/lib/scim/v2/ScimV2Api.ts | 38 +++++++++++++++++----- 4 files changed, 80 insertions(+), 14 deletions(-) diff --git a/app/gen-server/lib/homedb/HomeDBManager.ts b/app/gen-server/lib/homedb/HomeDBManager.ts index a5d962d701..75b77356ab 100644 --- a/app/gen-server/lib/homedb/HomeDBManager.ts +++ b/app/gen-server/lib/homedb/HomeDBManager.ts @@ -563,6 +563,10 @@ export class HomeDBManager extends EventEmitter { return this._usersManager.deleteUser(scope, userIdToDelete, name); } + public async overrideUser(userId: number, props: UserProfile) { + return this._usersManager.overrideUser(userId, props); + } + /** * Returns a QueryResult for the given organization. The orgKey * can be a string (the domain from url) or the id of an org. If it is diff --git a/app/gen-server/lib/homedb/UsersManager.ts b/app/gen-server/lib/homedb/UsersManager.ts index edfdf520fc..3652049298 100644 --- a/app/gen-server/lib/homedb/UsersManager.ts +++ b/app/gen-server/lib/homedb/UsersManager.ts @@ -395,7 +395,7 @@ export class UsersManager { // Set the user's name if our provider knows it. Otherwise use their username // from email, for lack of something better. If we don't have a profile at this // time, then leave the name blank in the hopes of learning it when the user logs in. - user.name = (profile && (profile.name || email.split('@')[0])) || ''; + user.name = (profile && this._getNameOrDeduceFromEmail(profile.name, email)) || ''; needUpdate = true; } if (!user.picture && profile && profile.picture) { @@ -582,6 +582,27 @@ export class UsersManager { .filter(fullProfile => fullProfile); } + /** + * Update users with passed property. Optional user properties that are missing will be reset to their default value. + */ + public async overrideUser(userId: number, props: UserProfile): Promise { + return await this._connection.transaction(async manager => { + const user = await this.getUser(userId, {includePrefs: true}); + if (!user) { throw new ApiError("unable to find user to update", 404); } + const login = user.logins[0]; + user.name = this._getNameOrDeduceFromEmail(props.name, props.email); + user.picture = props.picture || ''; + user.options = {...(user.options || {}), locale: props.locale ?? undefined}; + if (props.email) { + login.email = normalizeEmail(props.email); + login.displayEmail = props.email; + } + await manager.save([user, login]); + + return (await this.getUser(userId))!; + }); + } + /** * ================================== * @@ -776,6 +797,10 @@ export class UsersManager { return id; } + private _getNameOrDeduceFromEmail(name: string, email: string) { + return name || email.split('@')[0]; + } + // This deals with the problem posed by receiving a PermissionDelta specifying a // role for both alice@x and Alice@x. We do not distinguish between such emails. // If there are multiple indistinguishabe emails, we preserve just one of them, diff --git a/app/server/lib/scim/v2/ScimUserUtils.ts b/app/server/lib/scim/v2/ScimUserUtils.ts index b9db4bf0de..0ddaffe9e1 100644 --- a/app/server/lib/scim/v2/ScimUserUtils.ts +++ b/app/server/lib/scim/v2/ScimUserUtils.ts @@ -1,5 +1,7 @@ +import { normalizeEmail } from "app/common/emails"; +import { UserProfile } from "app/common/LoginSessionAPI"; import { User } from "app/gen-server/entity/User.js"; -import { SCIMMY } from "scimmy-routers"; +import SCIMMY from "scimmy"; /** * Converts a user from your database to a SCIMMY user @@ -8,7 +10,7 @@ export function toSCIMMYUser(user: User) { if (!user.logins) { throw new Error("User must have at least one login"); } - const preferredLanguage = user.options?.locale ?? "en"; + const locale = user.options?.locale ?? "en"; return new SCIMMY.Schemas.User({ id: String(user.id), userName: user.loginEmail, @@ -16,16 +18,29 @@ export function toSCIMMYUser(user: User) { name: { formatted: user.name, }, - preferredLanguage, - locale: preferredLanguage, // Assume locale is the same as preferredLanguage + locale, + preferredLanguage: locale, // Assume preferredLanguage is the same as locale photos: user.picture ? [{ value: user.picture, type: "photo", primary: true }] : undefined, emails: [{ - value: user.loginEmail, + value: user.logins[0].displayEmail, primary: true, }], }); } + +export function toUserProfile(scimUser: any, existingUser?: User): UserProfile { + const emailValue = scimUser.emails?.[0]?.value; + if (emailValue && normalizeEmail(emailValue) !== normalizeEmail(scimUser.userName)) { + throw new SCIMMY.Types.Error(400, 'invalidValue', 'Email and userName must be the same'); + } + return { + name: scimUser.displayName ?? existingUser?.name, + picture: scimUser.photos?.[0]?.value, + locale: scimUser.locale, + email: emailValue ?? scimUser.userName ?? existingUser?.loginEmail, + }; +} diff --git a/app/server/lib/scim/v2/ScimV2Api.ts b/app/server/lib/scim/v2/ScimV2Api.ts index 05026251ec..ebd0c6db4f 100644 --- a/app/server/lib/scim/v2/ScimV2Api.ts +++ b/app/server/lib/scim/v2/ScimV2Api.ts @@ -4,12 +4,13 @@ import SCIMMY from "scimmy"; import SCIMMYRouters from "scimmy-routers"; import { RequestWithLogin } from '../../Authorizer'; import { InstallAdmin } from '../../InstallAdmin'; -import { toSCIMMYUser } from './ScimUserUtils'; +import { toSCIMMYUser, toUserProfile } from './ScimUserUtils'; +import { ApiError } from 'app/common/ApiError'; const WHITELISTED_PATHS_FOR_NON_ADMINS = [ "/Me", "/Schemas", "/ResourceTypes", "/ServiceProviderConfig" ]; async function isAuthorizedAction(mreq: RequestWithLogin, installAdmin: InstallAdmin): Promise { - const isAdmin = await installAdmin.isAdminReq(mreq) + const isAdmin = await installAdmin.isAdminReq(mreq); const isScimUser = Boolean(process.env.GRIST_SCIM_EMAIL && mreq.user?.loginEmail === process.env.GRIST_SCIM_EMAIL); return isAdmin || isScimUser || WHITELISTED_PATHS_FOR_NON_ADMINS.includes(mreq.path); } @@ -30,18 +31,39 @@ const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) const scimmyUsers = (await dbManager.getUsers()).map(user => toSCIMMYUser(user)); return filter ? filter.match(scimmyUsers) : scimmyUsers; }, - ingress: async (resource: any) => { + ingress: async (resource: any, data: any) => { try { const { id } = resource; if (id) { - return null; + const updatedUser = await dbManager.overrideUser(id, toUserProfile(data)); + return toSCIMMYUser(updatedUser); } - return []; + const userProfileToInsert = toUserProfile(data); + const maybeExistingUser = await dbManager.getExistingUserByLogin(userProfileToInsert.email); + if (maybeExistingUser !== undefined) { + throw new SCIMMY.Types.Error(409, 'uniqueness', 'An existing user with the passed email exist.'); + } + const newUser = await dbManager.getUserByLoginWithRetry(userProfileToInsert.email, { + profile: userProfileToInsert + }); + return toSCIMMYUser(newUser!); } catch (ex) { - // FIXME: remove this - if (Math.random() > 1) { - return null; + if (ex instanceof ApiError) { + if (ex.status === 409) { + throw new SCIMMY.Types.Error(ex.status, 'uniqueness', ex.message); + } + throw new SCIMMY.Types.Error(ex.status, null!, ex.message); + } + + if (ex.code?.startsWith('SQLITE')) { + switch (ex.code) { + case 'SQLITE_CONSTRAINT': + throw new SCIMMY.Types.Error(409, 'uniqueness', ex.message); + default: + throw new SCIMMY.Types.Error(500, 'serverError', ex.message); + } } + throw ex; } }, From a9f51939284e9651e0a5f8615de25b8bf7a3e1a2 Mon Sep 17 00:00:00 2001 From: fflorent Date: Tue, 3 Sep 2024 23:53:37 +0200 Subject: [PATCH 04/27] Add tests --- test/server/lib/Scim.ts | 363 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 363 insertions(+) create mode 100644 test/server/lib/Scim.ts diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts new file mode 100644 index 0000000000..cbfc58065a --- /dev/null +++ b/test/server/lib/Scim.ts @@ -0,0 +1,363 @@ +import axios from 'axios'; +import capitalize from 'lodash/capitalize'; +import { assert } from 'chai'; +import { TestServer } from 'test/gen-server/apiUtils'; +import { configForUser } from 'test/gen-server/testUtils'; +import * as testUtils from 'test/server/testUtils'; + +function scimConfigForUser(user: string) { + const config = configForUser(user); + return { + ...config, + headers: { + ...config.headers, + 'Content-Type': 'application/scim+json' + } + }; +} + +const chimpy = scimConfigForUser('Chimpy'); +const kiwi = scimConfigForUser('Kiwi'); +const anon = configForUser('Anonymous'); + +const USER_CONFIG_BY_NAME = { + chimpy, + kiwi, + anon, +}; + +type UserConfigByName = typeof USER_CONFIG_BY_NAME; + +describe('Scim', () => { + let oldEnv: testUtils.EnvironmentSnapshot; + let server: TestServer; + let homeUrl: string; + const userIdByName: {[name in keyof UserConfigByName]?: number} = {}; + + const scimUrl = (path: string) => (homeUrl + '/api/scim/v2' + path); + + before(async function () { + oldEnv = new testUtils.EnvironmentSnapshot(); + process.env.GRIST_DEFAULT_EMAIL = 'chimpy@getgrist.com'; + process.env.TYPEORM_DATABASE = ':memory:'; + server = new TestServer(this); + homeUrl = await server.start(); + const userNames = Object.keys(USER_CONFIG_BY_NAME) as Array; + for (const user of userNames) { + userIdByName[user] = await getUserId(user); + } + }); + + after(async () => { + oldEnv.restore(); + await server.stop(); + }); + + function personaToSCIMMYUserWithId(user: keyof UserConfigByName) { + return toSCIMUserWithId(user, userIdByName[user]!); + } + + function toSCIMUserWithId(user: string, id: number) { + return { + ...toSCIMUserWithoutId(user), + id: String(id), + meta: { resourceType: 'User', location: '/api/scim/v2/Users/' + id }, + }; + } + + function toSCIMUserWithoutId(user: string) { + return { + schemas: [ 'urn:ietf:params:scim:schemas:core:2.0:User' ], + userName: user + '@getgrist.com', + name: { formatted: capitalize(user) }, + displayName: capitalize(user), + preferredLanguage: 'en', + locale: 'en', + emails: [ { value: user + '@getgrist.com', primary: true } ] + }; + } + + async function getUserId(user: string) { + return (await server.dbManager.getUserByLogin(user + '@getgrist.com'))!.id; + } + + function checkEndpointNotAccessibleForNonAdminUsers( + method: 'get' | 'post' | 'put' | 'patch' | 'delete', + path: string + ) { + function makeCallWith(user: keyof UserConfigByName) { + if (method === 'get' || method === 'delete') { + return axios[method](scimUrl(path), USER_CONFIG_BY_NAME[user]); + } + return axios[method](scimUrl(path), {}, USER_CONFIG_BY_NAME[user]); + } + it('should return 401 for anonymous', async function () { + const res: any = await makeCallWith('anon'); + assert.equal(res.status, 401); + }); + + it('should return 401 for kiwi', async function () { + const res: any = await makeCallWith('kiwi'); + assert.equal(res.status, 401); + }); + } + + describe('/Me', function () { + async function checkGetMeAs(user: keyof UserConfigByName, expected: any) { + const res = await axios.get(scimUrl('/Me'), USER_CONFIG_BY_NAME[user]); + assert.equal(res.status, 200); + assert.deepInclude(res.data, expected); + } + + it(`should return the current user for chimpy`, async function () { + return checkGetMeAs('chimpy', personaToSCIMMYUserWithId('chimpy')); + }); + + it(`should return the current user for kiwi`, async function () { + return checkGetMeAs('kiwi', personaToSCIMMYUserWithId('kiwi')); + }); + + it('should return 401 for anonymous', async function () { + const res = await axios.get(scimUrl('/Me'), anon); + assert.equal(res.status, 401); + }); + }); + + describe('/Users/{id}', function () { + + it('should return the user of id=1 for chimpy', async function () { + const res = await axios.get(scimUrl('/Users/1'), chimpy); + + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + id: '1', + displayName: 'Chimpy', + userName: 'chimpy@getgrist.com' + }); + }); + + it('should return 404 when the user is not found', async function () { + const res = await axios.get(scimUrl('/Users/1000'), chimpy); + assert.equal(res.status, 404); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '404', + detail: 'User with ID 1000 not found' + }); + }); + + checkEndpointNotAccessibleForNonAdminUsers('get', '/Users/1'); + }); + + describe('GET /Users', function () { + it('should return all users for chimpy', async function () { + const res = await axios.get(scimUrl('/Users'), chimpy); + assert.equal(res.status, 200); + assert.isAbove(res.data.totalResults, 0, 'should have retrieved some users'); + assert.deepInclude(res.data.Resources, personaToSCIMMYUserWithId('chimpy')); + assert.deepInclude(res.data.Resources, personaToSCIMMYUserWithId('kiwi')); + }); + + checkEndpointNotAccessibleForNonAdminUsers('get', '/Users'); + }); + + describe('POST /Users/.search', function () { + const SEARCH_SCHEMA = 'urn:ietf:params:scim:api:messages:2.0:SearchRequest'; + it('should return all users for chimpy order by userName in descending order', async function () { + const res = await axios.post(scimUrl('/Users/.search'), { + schemas: [SEARCH_SCHEMA], + sortBy: 'userName', + sortOrder: 'descending', + }, chimpy); + assert.equal(res.status, 200); + assert.isAbove(res.data.totalResults, 0, 'should have retrieved some users'); + const users = res.data.Resources.map((r: any) => r.userName); + assert.include(users, 'chimpy@getgrist.com'); + assert.include(users, 'kiwi@getgrist.com'); + const indexOfChimpy = users.indexOf('chimpy@getgrist.com'); + const indexOfKiwi = users.indexOf('kiwi@getgrist.com'); + assert.isBelow(indexOfKiwi, indexOfChimpy, 'kiwi should come before chimpy'); + }); + + it('should filter the users by userName', async function () { + const res = await axios.post(scimUrl('/Users/.search'), { + schemas: [SEARCH_SCHEMA], + attributes: ['userName'], + filter: 'userName sw "chimpy"', + }, chimpy); + assert.equal(res.status, 200); + assert.equal(res.data.totalResults, 1); + assert.deepEqual(res.data.Resources[0], { id: String(userIdByName['chimpy']), userName: 'chimpy@getgrist.com' }, + "should have retrieved only chimpy's username and not other attribute"); + }); + + checkEndpointNotAccessibleForNonAdminUsers('post', '/Users/.search'); + }); + + describe('POST /Users', function () { // Create a new users + it('should create a new user', async function () { + const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId('newuser1'), chimpy); + assert.equal(res.status, 201); + const newUserId = await getUserId('newuser1'); + assert.deepEqual(res.data, toSCIMUserWithId('newuser1', newUserId)); + }); + + it('should allow creating a new user given only their email passed as username', async function () { + const res = await axios.post(scimUrl('/Users'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: 'new.user2@getgrist.com', + }, chimpy); + assert.equal(res.status, 201); + assert.equal(res.data.userName, 'new.user2@getgrist.com'); + assert.equal(res.data.displayName, 'new.user2'); + }); + + it('should reject when passed email differs from username', async function () { + const res = await axios.post(scimUrl('/Users'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: 'username@getgrist.com', + emails: [{ value: 'emails.value@getgrist.com' }], + }, chimpy); + assert.equal(res.status, 400); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '400', + detail: 'Email and userName must be the same', + scimType: 'invalidValue' + }); + }); + + it('should disallow creating a user with the same email', async function () { + const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId('chimpy'), chimpy); + assert.equal(res.status, 409); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '409', + detail: 'An existing user with the passed email exist.', + scimType: 'uniqueness' + }); + }); + + checkEndpointNotAccessibleForNonAdminUsers('post', '/Users'); + }); + + describe('PUT /Users/{id}', function () { + let userToUpdateId: number; + const userToUpdateEmailLocalPart = 'user-to-update'; + + beforeEach(async function () { + userToUpdateId = await getUserId(userToUpdateEmailLocalPart); + }); + afterEach(async function () { + await server.dbManager.deleteUser({ userId: userToUpdateId }, userToUpdateId); + }); + + it('should update an existing user', async function () { + const userToUpdateProperties = { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: userToUpdateEmailLocalPart + '-now-updated@getgrist.com', + displayName: 'User to Update', + photos: [{ value: 'https://example.com/photo.jpg', type: 'photo', primary: true }], + locale: 'fr', + }; + const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), userToUpdateProperties, chimpy); + assert.equal(res.status, 200); + const refreshedUser = await axios.get(scimUrl(`/Users/${userToUpdateId}`), chimpy); + assert.deepEqual(refreshedUser.data, { + ...userToUpdateProperties, + id: String(userToUpdateId), + meta: { resourceType: 'User', location: `/api/scim/v2/Users/${userToUpdateId}` }, + emails: [ { value: userToUpdateProperties.userName, primary: true } ], + name: { formatted: userToUpdateProperties.displayName }, + preferredLanguage: 'fr', + }); + }); + + it('should reject when passed email differs from username', async function () { + const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: userToUpdateEmailLocalPart + '@getgrist.com', + emails: [{ value: 'whatever@getgrist.com', primary: true }], + }, chimpy); + assert.equal(res.status, 400); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '400', + detail: 'Email and userName must be the same', + scimType: 'invalidValue' + }); + }); + + it('should disallow updating a user with the same email', async function () { + const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), toSCIMUserWithoutId('chimpy'), chimpy); + assert.equal(res.status, 409); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '409', + detail: 'SQLITE_CONSTRAINT: UNIQUE constraint failed: logins.email', + scimType: 'uniqueness' + }); + }); + + it('should return 404 when the user is not found', async function () { + const res = await axios.put(scimUrl('/Users/1000'), toSCIMUserWithoutId('chimpy'), chimpy); + assert.equal(res.status, 404); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '404', + detail: 'unable to find user to update' + }); + }); + + it('should deduce the name from the displayEmail when not provided', async function () { + const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: 'my-email@getgrist.com', + }, chimpy); + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + id: String(userToUpdateId), + userName: 'my-email@getgrist.com', + displayName: 'my-email', + }); + }); + + it('should normalize the passed email for the userName and keep the case for email.value', async function () { + const newEmail = 'my-EMAIL@getgrist.com'; + const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: newEmail, + }, chimpy); + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + id: String(userToUpdateId), + userName: newEmail.toLowerCase(), + displayName: 'my-EMAIL', + emails: [{ value: newEmail, primary: true }] + }); + }); + + checkEndpointNotAccessibleForNonAdminUsers('put', '/Users/1'); + }); + + describe('PATCH /Users/{id}', function () { + it('should update the user of id=1', async function () { + throw new Error("This is a reminder :)"); + }); + checkEndpointNotAccessibleForNonAdminUsers('patch', '/Users/1'); + }); + + describe('DELETE /Users/{id}', function () { + it('should delete the user of id=1', async function () { + throw new Error("This is a reminder :)"); + }); + checkEndpointNotAccessibleForNonAdminUsers('delete', '/Users/1'); + }); + + it('should fix the 401 response for authenticated users', function () { + throw new Error("This is a reminder :)"); + }); +}); From 6db073738f70db5083fca0084b758246745d69ed Mon Sep 17 00:00:00 2001 From: fflorent Date: Thu, 5 Sep 2024 21:27:27 +0200 Subject: [PATCH 05/27] SCIM: Implement DELETE --- app/server/lib/scim/v2/ScimV2Api.ts | 27 ++++++++-- test/server/lib/Scim.ts | 78 +++++++++++++++++++++++++---- 2 files changed, 91 insertions(+), 14 deletions(-) diff --git a/app/server/lib/scim/v2/ScimV2Api.ts b/app/server/lib/scim/v2/ScimV2Api.ts index ebd0c6db4f..69aeac3d8b 100644 --- a/app/server/lib/scim/v2/ScimV2Api.ts +++ b/app/server/lib/scim/v2/ScimV2Api.ts @@ -1,11 +1,12 @@ import * as express from 'express'; -import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; +import { HomeDBManager, Scope } from 'app/gen-server/lib/homedb/HomeDBManager'; import SCIMMY from "scimmy"; import SCIMMYRouters from "scimmy-routers"; import { RequestWithLogin } from '../../Authorizer'; import { InstallAdmin } from '../../InstallAdmin'; import { toSCIMMYUser, toUserProfile } from './ScimUserUtils'; import { ApiError } from 'app/common/ApiError'; +import { parseInt } from 'lodash'; const WHITELISTED_PATHS_FOR_NON_ADMINS = [ "/Me", "/Schemas", "/ResourceTypes", "/ServiceProviderConfig" ]; @@ -20,7 +21,8 @@ const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) SCIMMY.Resources.declare(SCIMMY.Resources.User, { egress: async (resource: any) => { - const { id, filter } = resource; + const { filter } = resource; + const id = parseInt(resource.id, 10); if (id) { const user = await dbManager.getUser(id); if (!user) { @@ -33,7 +35,7 @@ const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) }, ingress: async (resource: any, data: any) => { try { - const { id } = resource; + const id = parseInt(resource.id, 10); if (id) { const updatedUser = await dbManager.overrideUser(id, toUserProfile(data)); return toSCIMMYUser(updatedUser); @@ -55,9 +57,13 @@ const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) throw new SCIMMY.Types.Error(ex.status, null!, ex.message); } + // FIXME: Remove this part and find another way to detect a constraint error. if (ex.code?.startsWith('SQLITE')) { switch (ex.code) { case 'SQLITE_CONSTRAINT': + // Return a 409 error if a conflict is detected (e.g. email already exists) + // "uniqueness" is an error code expected by the SCIM RFC for this case. + // FIXME: the emails are unique in the database, but this is not enforced in the schema. throw new SCIMMY.Types.Error(409, 'uniqueness', ex.message); default: throw new SCIMMY.Types.Error(500, 'serverError', ex.message); @@ -67,8 +73,19 @@ const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) throw ex; } }, - degress: () => { - return null; + degress: async (resource: any) => { + const id = parseInt(resource.id, 10); + const fakeScope: Scope = { userId: id }; // FIXME: deleteUser should probably better not requiring a scope. + try { + await dbManager.deleteUser(fakeScope, id); + } catch (ex) { + console.error('Error deleting user', ex); + if (ex instanceof ApiError) { + throw new SCIMMY.Types.Error(ex.status, null!, ex.message); + } + + throw new SCIMMY.Types.Error(500, 'serverError', ex.message); + } } }); diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index cbfc58065a..64e7531fda 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -44,7 +44,7 @@ describe('Scim', () => { homeUrl = await server.start(); const userNames = Object.keys(USER_CONFIG_BY_NAME) as Array; for (const user of userNames) { - userIdByName[user] = await getUserId(user); + userIdByName[user] = await getOrCreateUserId(user); } }); @@ -77,10 +77,16 @@ describe('Scim', () => { }; } - async function getUserId(user: string) { + async function getOrCreateUserId(user: string) { return (await server.dbManager.getUserByLogin(user + '@getgrist.com'))!.id; } + async function cleanupUser(userId: number) { + if (await server.dbManager.getUser(userId)) { + await server.dbManager.deleteUser({ userId: userId }, userId); + } + } + function checkEndpointNotAccessibleForNonAdminUsers( method: 'get' | 'post' | 'put' | 'patch' | 'delete', path: string @@ -199,7 +205,7 @@ describe('Scim', () => { it('should create a new user', async function () { const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId('newuser1'), chimpy); assert.equal(res.status, 201); - const newUserId = await getUserId('newuser1'); + const newUserId = await getOrCreateUserId('newuser1'); assert.deepEqual(res.data, toSCIMUserWithId('newuser1', newUserId)); }); @@ -247,10 +253,10 @@ describe('Scim', () => { const userToUpdateEmailLocalPart = 'user-to-update'; beforeEach(async function () { - userToUpdateId = await getUserId(userToUpdateEmailLocalPart); + userToUpdateId = await getOrCreateUserId(userToUpdateEmailLocalPart); }); afterEach(async function () { - await server.dbManager.deleteUser({ userId: userToUpdateId }, userToUpdateId); + await cleanupUser(userToUpdateId); }); it('should update an existing user', async function () { @@ -344,15 +350,69 @@ describe('Scim', () => { }); describe('PATCH /Users/{id}', function () { - it('should update the user of id=1', async function () { - throw new Error("This is a reminder :)"); + let userToPatchId: number; + const userToPatchEmailLocalPart = 'user-to-patch'; + beforeEach(async function () { + userToPatchId = await getOrCreateUserId(userToPatchEmailLocalPart); }); + afterEach(async function () { + await cleanupUser(userToPatchId); + }); + + it('should replace values of an existing user', async function () { + const newName = 'User to Patch new Name'; + const res = await axios.patch(scimUrl(`/Users/${userToPatchId}`), { + schemas: ['urn:ietf:params:scim:api:messages:2.0:PatchOp'], + Operations: [{ + op: "replace", + path: "displayName", + value: newName, + }, { + op: "replace", + path: "locale", + value: 'fr' + }], + }, chimpy); + assert.equal(res.status, 200); + const refreshedUser = await axios.get(scimUrl(`/Users/${userToPatchId}`), chimpy); + assert.deepEqual(refreshedUser.data, { + ...toSCIMUserWithId(userToPatchEmailLocalPart, userToPatchId), + displayName: newName, + name: { formatted: newName }, + locale: 'fr', + preferredLanguage: 'fr', + }); + }); + checkEndpointNotAccessibleForNonAdminUsers('patch', '/Users/1'); }); describe('DELETE /Users/{id}', function () { - it('should delete the user of id=1', async function () { - throw new Error("This is a reminder :)"); + let userToDeleteId: number; + const userToDeleteEmailLocalPart = 'user-to-delete'; + + beforeEach(async function () { + userToDeleteId = await getOrCreateUserId(userToDeleteEmailLocalPart); + }); + afterEach(async function () { + await cleanupUser(userToDeleteId); + }); + + it('should delete some user', async function () { + const res = await axios.delete(scimUrl(`/Users/${userToDeleteId}`), chimpy); + assert.equal(res.status, 204); + const refreshedUser = await axios.get(scimUrl(`/Users/${userToDeleteId}`), chimpy); + assert.equal(refreshedUser.status, 404); + }); + + it('should return 404 when the user is not found', async function () { + const res = await axios.delete(scimUrl('/Users/1000'), chimpy); + assert.equal(res.status, 404); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '404', + detail: 'user not found' + }); }); checkEndpointNotAccessibleForNonAdminUsers('delete', '/Users/1'); }); From 132bc3fa2b921e40f046c8a25d48fae1b1b55596 Mon Sep 17 00:00:00 2001 From: fflorent Date: Fri, 6 Sep 2024 17:02:14 +0200 Subject: [PATCH 06/27] More tests and cleanups --- app/server/lib/scim/v2/ScimV2Api.ts | 74 +++--- test/server/lib/Scim.ts | 341 +++++++++++++++++++++++----- 2 files changed, 329 insertions(+), 86 deletions(-) diff --git a/app/server/lib/scim/v2/ScimV2Api.ts b/app/server/lib/scim/v2/ScimV2Api.ts index 69aeac3d8b..2d1add3778 100644 --- a/app/server/lib/scim/v2/ScimV2Api.ts +++ b/app/server/lib/scim/v2/ScimV2Api.ts @@ -10,20 +10,36 @@ import { parseInt } from 'lodash'; const WHITELISTED_PATHS_FOR_NON_ADMINS = [ "/Me", "/Schemas", "/ResourceTypes", "/ServiceProviderConfig" ]; -async function isAuthorizedAction(mreq: RequestWithLogin, installAdmin: InstallAdmin): Promise { - const isAdmin = await installAdmin.isAdminReq(mreq); - const isScimUser = Boolean(process.env.GRIST_SCIM_EMAIL && mreq.user?.loginEmail === process.env.GRIST_SCIM_EMAIL); - return isAdmin || isScimUser || WHITELISTED_PATHS_FOR_NON_ADMINS.includes(mreq.path); +interface RequestContext { + path: string; + isAdmin: boolean; + isScimUser: boolean; +} + +function checkAccess(context: RequestContext) { + const {isAdmin, isScimUser, path } = context; + if (!isAdmin && !isScimUser && !WHITELISTED_PATHS_FOR_NON_ADMINS.includes(path)) { + throw new SCIMMY.Types.Error(403, null!, 'You are not authorized to access this resource'); + } +} + +async function checkEmailIsUnique(dbManager: HomeDBManager, email: string, id?: number) { + const existingUser = await dbManager.getExistingUserByLogin(email); + if (existingUser !== undefined && existingUser.id !== id) { + throw new SCIMMY.Types.Error(409, 'uniqueness', 'An existing user with the passed email exist.'); + } } const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) => { const v2 = express.Router(); SCIMMY.Resources.declare(SCIMMY.Resources.User, { - egress: async (resource: any) => { + egress: async (resource: any, context: RequestContext) => { + checkAccess(context); + const { filter } = resource; const id = parseInt(resource.id, 10); - if (id) { + if (!isNaN(id)) { const user = await dbManager.getUser(id); if (!user) { throw new SCIMMY.Types.Error(404, null!, `User with ID ${id} not found`); @@ -33,18 +49,18 @@ const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) const scimmyUsers = (await dbManager.getUsers()).map(user => toSCIMMYUser(user)); return filter ? filter.match(scimmyUsers) : scimmyUsers; }, - ingress: async (resource: any, data: any) => { + ingress: async (resource: any, data: any, context: RequestContext) => { + checkAccess(context); + try { const id = parseInt(resource.id, 10); - if (id) { + if (!isNaN(id)) { + await checkEmailIsUnique(dbManager, data.userName, id); const updatedUser = await dbManager.overrideUser(id, toUserProfile(data)); return toSCIMMYUser(updatedUser); } + await checkEmailIsUnique(dbManager, data.userName); const userProfileToInsert = toUserProfile(data); - const maybeExistingUser = await dbManager.getExistingUserByLogin(userProfileToInsert.email); - if (maybeExistingUser !== undefined) { - throw new SCIMMY.Types.Error(409, 'uniqueness', 'An existing user with the passed email exist.'); - } const newUser = await dbManager.getUserByLoginWithRetry(userProfileToInsert.email, { profile: userProfileToInsert }); @@ -57,29 +73,20 @@ const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) throw new SCIMMY.Types.Error(ex.status, null!, ex.message); } - // FIXME: Remove this part and find another way to detect a constraint error. - if (ex.code?.startsWith('SQLITE')) { - switch (ex.code) { - case 'SQLITE_CONSTRAINT': - // Return a 409 error if a conflict is detected (e.g. email already exists) - // "uniqueness" is an error code expected by the SCIM RFC for this case. - // FIXME: the emails are unique in the database, but this is not enforced in the schema. - throw new SCIMMY.Types.Error(409, 'uniqueness', ex.message); - default: - throw new SCIMMY.Types.Error(500, 'serverError', ex.message); - } - } - throw ex; } }, - degress: async (resource: any) => { + degress: async (resource: any, context: RequestContext) => { + checkAccess(context); + const id = parseInt(resource.id, 10); + if (isNaN(id)) { + throw new SCIMMY.Types.Error(400, null!, 'Invalid ID'); + } const fakeScope: Scope = { userId: id }; // FIXME: deleteUser should probably better not requiring a scope. try { await dbManager.deleteUser(fakeScope, id); } catch (ex) { - console.error('Error deleting user', ex); if (ex instanceof ApiError) { throw new SCIMMY.Types.Error(ex.status, null!, ex.message); } @@ -102,10 +109,15 @@ const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) throw new Error('Anonymous user cannot access SCIM resources'); } - if (!await isAuthorizedAction(mreq, installAdmin)) { - throw new SCIMMY.Types.Error(403, null!, 'Resource disallowed for non-admin users'); - } - return String(mreq.userId); // HACK: SCIMMYRouters requires the userId to be a string. + return String(mreq.userId); // SCIMMYRouters requires the userId to be a string. + }, + context: async (mreq: RequestWithLogin): Promise => { + const isAdmin = await installAdmin.isAdminReq(mreq); + const isScimUser = Boolean( + process.env.GRIST_SCIM_EMAIL && mreq.user?.loginEmail === process.env.GRIST_SCIM_EMAIL + ); + const path = mreq.path; + return { isAdmin, isScimUser, path }; } }) as express.Router; // Have to cast it into express.Router. See https://github.com/scimmyjs/scimmy-routers/issues/24 diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index 64e7531fda..feb964e6db 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -18,7 +18,7 @@ function scimConfigForUser(user: string) { const chimpy = scimConfigForUser('Chimpy'); const kiwi = scimConfigForUser('Kiwi'); -const anon = configForUser('Anonymous'); +const anon = scimConfigForUser('Anonymous'); const USER_CONFIG_BY_NAME = { chimpy, @@ -89,13 +89,14 @@ describe('Scim', () => { function checkEndpointNotAccessibleForNonAdminUsers( method: 'get' | 'post' | 'put' | 'patch' | 'delete', - path: string + path: string, + validBody: object = {} ) { function makeCallWith(user: keyof UserConfigByName) { if (method === 'get' || method === 'delete') { return axios[method](scimUrl(path), USER_CONFIG_BY_NAME[user]); } - return axios[method](scimUrl(path), {}, USER_CONFIG_BY_NAME[user]); + return axios[method](scimUrl(path), validBody, USER_CONFIG_BY_NAME[user]); } it('should return 401 for anonymous', async function () { const res: any = await makeCallWith('anon'); @@ -104,7 +105,12 @@ describe('Scim', () => { it('should return 401 for kiwi', async function () { const res: any = await makeCallWith('kiwi'); - assert.equal(res.status, 401); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '403', + detail: 'You are not authorized to access this resource' + }); + assert.equal(res.status, 403); }); } @@ -127,6 +133,27 @@ describe('Scim', () => { const res = await axios.get(scimUrl('/Me'), anon); assert.equal(res.status, 401); }); + + it.skip('should allow operation like PATCH for kiwi', async function () { + // SKIPPING this test: only the GET verb is currently implemented by SCIMMY for the /Me endpoint. + // Issue created here: https://github.com/scimmyjs/scimmy/issues/47 + const patchBody = { + schemas: ['urn:ietf:params:scim:api:messages:2.0:PatchOp'], + Operations: [{ + op: "replace", + path: 'locale', + value: 'fr', + }], + }; + const res = await axios.patch(scimUrl('/Me'), patchBody, kiwi); + assert.equal(res.status, 200); + assert.deepEqual(res.data, { + ...personaToSCIMMYUserWithId('kiwi'), + locale: 'fr', + preferredLanguage: 'en', + }); + }); + }); describe('/Users/{id}', function () { @@ -170,12 +197,15 @@ describe('Scim', () => { describe('POST /Users/.search', function () { const SEARCH_SCHEMA = 'urn:ietf:params:scim:api:messages:2.0:SearchRequest'; + + const searchExample = { + schemas: [SEARCH_SCHEMA], + sortBy: 'userName', + sortOrder: 'descending', + }; + it('should return all users for chimpy order by userName in descending order', async function () { - const res = await axios.post(scimUrl('/Users/.search'), { - schemas: [SEARCH_SCHEMA], - sortBy: 'userName', - sortOrder: 'descending', - }, chimpy); + const res = await axios.post(scimUrl('/Users/.search'), searchExample, chimpy); assert.equal(res.status, 200); assert.isAbove(res.data.totalResults, 0, 'should have retrieved some users'); const users = res.data.Resources.map((r: any) => r.userName); @@ -198,54 +228,70 @@ describe('Scim', () => { "should have retrieved only chimpy's username and not other attribute"); }); - checkEndpointNotAccessibleForNonAdminUsers('post', '/Users/.search'); + checkEndpointNotAccessibleForNonAdminUsers('post', '/Users/.search', searchExample); }); describe('POST /Users', function () { // Create a new users + async function withUserName(userName: string, cb: (userName: string) => Promise) { + try { + await cb(userName); + } finally { + const user = await server.dbManager.getExistingUserByLogin(userName + "@getgrist.com"); + if (user) { + await cleanupUser(user.id); + } + } + } it('should create a new user', async function () { - const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId('newuser1'), chimpy); - assert.equal(res.status, 201); - const newUserId = await getOrCreateUserId('newuser1'); - assert.deepEqual(res.data, toSCIMUserWithId('newuser1', newUserId)); + await withUserName('newuser1', async (userName) => { + const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId(userName), chimpy); + assert.equal(res.status, 201); + const newUserId = await getOrCreateUserId(userName); + assert.deepEqual(res.data, toSCIMUserWithId(userName, newUserId)); + }); }); it('should allow creating a new user given only their email passed as username', async function () { - const res = await axios.post(scimUrl('/Users'), { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - userName: 'new.user2@getgrist.com', - }, chimpy); - assert.equal(res.status, 201); - assert.equal(res.data.userName, 'new.user2@getgrist.com'); - assert.equal(res.data.displayName, 'new.user2'); + await withUserName('new.user2', async (userName) => { + const res = await axios.post(scimUrl('/Users'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: 'new.user2@getgrist.com', + }, chimpy); + assert.equal(res.status, 201); + assert.equal(res.data.userName, userName + '@getgrist.com'); + assert.equal(res.data.displayName, userName); + }); }); it('should reject when passed email differs from username', async function () { - const res = await axios.post(scimUrl('/Users'), { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - userName: 'username@getgrist.com', - emails: [{ value: 'emails.value@getgrist.com' }], - }, chimpy); - assert.equal(res.status, 400); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '400', - detail: 'Email and userName must be the same', - scimType: 'invalidValue' + await withUserName('username', async (userName) => { + const res = await axios.post(scimUrl('/Users'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: userName + '@getgrist.com', + emails: [{ value: 'emails.value@getgrist.com' }], + }, chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '400', + detail: 'Email and userName must be the same', + scimType: 'invalidValue' + }); + assert.equal(res.status, 400); }); }); it('should disallow creating a user with the same email', async function () { const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId('chimpy'), chimpy); - assert.equal(res.status, 409); assert.deepEqual(res.data, { schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], status: '409', detail: 'An existing user with the passed email exist.', scimType: 'uniqueness' }); + assert.equal(res.status, 409); }); - checkEndpointNotAccessibleForNonAdminUsers('post', '/Users'); + checkEndpointNotAccessibleForNonAdminUsers('post', '/Users', toSCIMUserWithoutId('some-user')); }); describe('PUT /Users/{id}', function () { @@ -286,34 +332,34 @@ describe('Scim', () => { userName: userToUpdateEmailLocalPart + '@getgrist.com', emails: [{ value: 'whatever@getgrist.com', primary: true }], }, chimpy); - assert.equal(res.status, 400); assert.deepEqual(res.data, { schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], status: '400', detail: 'Email and userName must be the same', scimType: 'invalidValue' }); + assert.equal(res.status, 400); }); - it('should disallow updating a user with the same email', async function () { + it('should disallow updating a user with the same email as another user\'s', async function () { const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), toSCIMUserWithoutId('chimpy'), chimpy); - assert.equal(res.status, 409); assert.deepEqual(res.data, { schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], status: '409', - detail: 'SQLITE_CONSTRAINT: UNIQUE constraint failed: logins.email', + detail: 'An existing user with the passed email exist.', scimType: 'uniqueness' }); + assert.equal(res.status, 409); }); it('should return 404 when the user is not found', async function () { - const res = await axios.put(scimUrl('/Users/1000'), toSCIMUserWithoutId('chimpy'), chimpy); - assert.equal(res.status, 404); + const res = await axios.put(scimUrl('/Users/1000'), toSCIMUserWithoutId('whoever'), chimpy); assert.deepEqual(res.data, { schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], status: '404', detail: 'unable to find user to update' }); + assert.equal(res.status, 404); }); it('should deduce the name from the displayEmail when not provided', async function () { @@ -346,7 +392,7 @@ describe('Scim', () => { }); }); - checkEndpointNotAccessibleForNonAdminUsers('put', '/Users/1'); + checkEndpointNotAccessibleForNonAdminUsers('put', '/Users/1', toSCIMUserWithoutId('chimpy')); }); describe('PATCH /Users/{id}', function () { @@ -359,20 +405,22 @@ describe('Scim', () => { await cleanupUser(userToPatchId); }); - it('should replace values of an existing user', async function () { - const newName = 'User to Patch new Name'; - const res = await axios.patch(scimUrl(`/Users/${userToPatchId}`), { - schemas: ['urn:ietf:params:scim:api:messages:2.0:PatchOp'], - Operations: [{ - op: "replace", - path: "displayName", - value: newName, - }, { + const validPatchBody = (newName: string) => ({ + schemas: ['urn:ietf:params:scim:api:messages:2.0:PatchOp'], + Operations: [{ + op: "replace", + path: "displayName", + value: newName, + }, { op: "replace", path: "locale", value: 'fr' }], - }, chimpy); + }); + + it('should replace values of an existing user', async function () { + const newName = 'User to Patch new Name'; + const res = await axios.patch(scimUrl(`/Users/${userToPatchId}`), validPatchBody(newName), chimpy); assert.equal(res.status, 200); const refreshedUser = await axios.get(scimUrl(`/Users/${userToPatchId}`), chimpy); assert.deepEqual(refreshedUser.data, { @@ -384,7 +432,7 @@ describe('Scim', () => { }); }); - checkEndpointNotAccessibleForNonAdminUsers('patch', '/Users/1'); + checkEndpointNotAccessibleForNonAdminUsers('patch', '/Users/1', validPatchBody('new name2')); }); describe('DELETE /Users/{id}', function () { @@ -407,17 +455,200 @@ describe('Scim', () => { it('should return 404 when the user is not found', async function () { const res = await axios.delete(scimUrl('/Users/1000'), chimpy); - assert.equal(res.status, 404); assert.deepEqual(res.data, { schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], status: '404', detail: 'user not found' }); + assert.equal(res.status, 404); }); checkEndpointNotAccessibleForNonAdminUsers('delete', '/Users/1'); }); - it('should fix the 401 response for authenticated users', function () { - throw new Error("This is a reminder :)"); + describe('POST /Bulk', function () { + let usersToCleanupEmails: string[]; + + beforeEach(async function () { + usersToCleanupEmails = []; + usersToCleanupEmails.push('bulk-user1@getgrist.com'); + usersToCleanupEmails.push('bulk-user2@getgrist.com'); + }); + + afterEach(async function () { + for (const email of usersToCleanupEmails) { + const user = await server.dbManager.getExistingUserByLogin(email); + if (user) { + await cleanupUser(user.id); + } + } + }); + + it('should return statuses for each operation', async function () { + const putOnUnknownResource = { method: 'PUT', path: '/Users/1000', value: toSCIMUserWithoutId('chimpy') }; + const validCreateOperation = { + method: 'POST', path: '/Users/', data: toSCIMUserWithoutId('bulk-user3'), bulkId: '1' + }; + usersToCleanupEmails.push('bulk-user3'); + const createOperationWithUserNameConflict = { + method: 'POST', path: '/Users/', data: toSCIMUserWithoutId('chimpy'), bulkId: '2' + }; + const res = await axios.post(scimUrl('/Bulk'), { + schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], + Operations: [ + putOnUnknownResource, + validCreateOperation, + createOperationWithUserNameConflict, + ], + }, chimpy); + assert.equal(res.status, 200); + assert.deepEqual(res.data, { + schemas: [ "urn:ietf:params:scim:api:messages:2.0:BulkResponse" ], + Operations: [ + { + method: "PUT", + location: "/api/scim/v2/Users/1000", + status: "400", + response: { + schemas: [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + status: "400", + scimType: "invalidSyntax", + detail: "Expected 'data' to be a single complex value in BulkRequest operation #1" + } + }, { + method: "POST", + bulkId: "1", + location: "/api/scim/v2/Users/26", + status: "201" + }, { + method: "POST", + bulkId: "2", + status: "409", + response: { + schemas: [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + status: "409", + scimType: "uniqueness", + detail: "An existing user with the passed email exist." + } + } + ] + }); + }); + + it('should return 400 when no operations are provided', async function () { + const res = await axios.post(scimUrl('/Bulk'), { + schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], + Operations: [], + }, chimpy); + assert.equal(res.status, 400); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '400', + detail: "BulkRequest request body must contain 'Operations' attribute with at least one operation", + scimType: 'invalidValue' + }); + }); + + it('should disallow accessing resources to kiwi', async function () { + const creationOperation = { + method: 'POST', path: '/Users', data: toSCIMUserWithoutId('bulk-user4'), bulkId: '1' + }; + usersToCleanupEmails.push('bulk-user4'); + const selfPutOperation = { method: 'PUT', path: '/Me', value: toSCIMUserWithoutId('kiwi') }; + const res = await axios.post(scimUrl('/Bulk'), { + schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], + Operations: [ + creationOperation, + selfPutOperation, + ], + }, kiwi); + assert.equal(res.status, 200); + assert.deepEqual(res.data, { + schemas: [ "urn:ietf:params:scim:api:messages:2.0:BulkResponse" ], + Operations: [ + { + method: "POST", + bulkId: "1", + status: "403", + response: { + detail: "You are not authorized to access this resource", + schemas: [ "urn:ietf:params:scim:api:messages:2.0:Error" ], + status: "403" + } + }, { + // When writing this test, the SCIMMY implementation does not yet support PUT operations on /Me. + // This reflects the current behavior, but it may change in the future. + // Change this test if the behavior changes. + // It is probably fine to allow altering oneself even for non-admins. + method: "PUT", + location: "/Me", + status: "400", + response: { + schemas: [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + status: "400", + detail: "Invalid 'path' value '/Me' in BulkRequest operation #2", + scimType: "invalidValue" + } + } + ] + }); + }); + + it('should disallow accessing resources to anonymous', async function () { + const creationOperation = { + method: 'POST', path: '/Users', data: toSCIMUserWithoutId('bulk-user5'), bulkId: '1' + }; + usersToCleanupEmails.push('bulk-user5'); + const res = await axios.post(scimUrl('/Bulk'), { + schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], + Operations: [creationOperation], + }, anon); + assert.equal(res.status, 401); + }); + }); + + it('should allow fetching the Scim schema when autenticated', async function () { + const res = await axios.get(scimUrl('/Schemas'), kiwi); + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:api:messages:2.0:ListResponse'], + }); + assert.property(res.data, 'Resources'); + assert.deepInclude(res.data.Resources[0], { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Schema'], + id: 'urn:ietf:params:scim:schemas:core:2.0:User', + name: 'User', + description: 'User Account', + }); + }); + + it('should allow fetching the Scim resource types when autenticated', async function () { + const res = await axios.get(scimUrl('/ResourceTypes'), kiwi); + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:api:messages:2.0:ListResponse'], + }); + assert.property(res.data, 'Resources'); + assert.deepInclude(res.data.Resources[0], { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:ResourceType'], + name: 'User', + endpoint: '/Users', + }); + }); + + it('should allow fetching the Scim service provider config when autenticated', async function () { + const res = await axios.get(scimUrl('/ServiceProviderConfig'), kiwi); + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:ServiceProviderConfig'], + }); + assert.property(res.data, 'patch'); + assert.property(res.data, 'bulk'); + assert.property(res.data, 'filter'); }); }); From a994ccd19df6f43b7e58ce6a88b5242c65659ec1 Mon Sep 17 00:00:00 2001 From: fflorent Date: Fri, 6 Sep 2024 19:11:13 +0200 Subject: [PATCH 07/27] Rebase fix + check GRIST_SCIM_USER --- app/server/lib/scim/v2/ScimV2Api.ts | 2 +- test/server/lib/Scim.ts | 22 +++++++++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/app/server/lib/scim/v2/ScimV2Api.ts b/app/server/lib/scim/v2/ScimV2Api.ts index 2d1add3778..df94f29fb9 100644 --- a/app/server/lib/scim/v2/ScimV2Api.ts +++ b/app/server/lib/scim/v2/ScimV2Api.ts @@ -64,7 +64,7 @@ const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) const newUser = await dbManager.getUserByLoginWithRetry(userProfileToInsert.email, { profile: userProfileToInsert }); - return toSCIMMYUser(newUser!); + return toSCIMMYUser(newUser); } catch (ex) { if (ex instanceof ApiError) { if (ex.status === 409) { diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index feb964e6db..124bc6f28f 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -18,6 +18,7 @@ function scimConfigForUser(user: string) { const chimpy = scimConfigForUser('Chimpy'); const kiwi = scimConfigForUser('Kiwi'); +const charon = scimConfigForUser('Charon'); const anon = scimConfigForUser('Anonymous'); const USER_CONFIG_BY_NAME = { @@ -36,9 +37,12 @@ describe('Scim', () => { const scimUrl = (path: string) => (homeUrl + '/api/scim/v2' + path); + testUtils.setTmpLogLevel('error'); + before(async function () { oldEnv = new testUtils.EnvironmentSnapshot(); process.env.GRIST_DEFAULT_EMAIL = 'chimpy@getgrist.com'; + process.env.GRIST_SCIM_EMAIL = 'charon@getgrist.com'; process.env.TYPEORM_DATABASE = ':memory:'; server = new TestServer(this); homeUrl = await server.start(); @@ -216,6 +220,11 @@ describe('Scim', () => { assert.isBelow(indexOfKiwi, indexOfChimpy, 'kiwi should come before chimpy'); }); + it('should also allow access for user Charon (the one refered in GRIST_SCIM_EMAIL)', async function () { + const res = await axios.post(scimUrl('/Users/.search'), searchExample, charon); + assert.equal(res.status, 200); + }); + it('should filter the users by userName', async function () { const res = await axios.post(scimUrl('/Users/.search'), { schemas: [SEARCH_SCHEMA], @@ -263,6 +272,13 @@ describe('Scim', () => { }); }); + it('should also allow user Charon to create a user (the one refered in GRIST_SCIM_EMAIL)', async function () { + await withUserName('new.user.by.charon', async (userName) => { + const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId(userName), charon); + assert.equal(res.status, 201); + }); + }); + it('should reject when passed email differs from username', async function () { await withUserName('username', async (userName) => { const res = await axios.post(scimUrl('/Users'), { @@ -470,8 +486,6 @@ describe('Scim', () => { beforeEach(async function () { usersToCleanupEmails = []; - usersToCleanupEmails.push('bulk-user1@getgrist.com'); - usersToCleanupEmails.push('bulk-user2@getgrist.com'); }); afterEach(async function () { @@ -501,6 +515,8 @@ describe('Scim', () => { ], }, chimpy); assert.equal(res.status, 200); + + const newUserID = await getOrCreateUserId('bulk-user3'); assert.deepEqual(res.data, { schemas: [ "urn:ietf:params:scim:api:messages:2.0:BulkResponse" ], Operations: [ @@ -519,7 +535,7 @@ describe('Scim', () => { }, { method: "POST", bulkId: "1", - location: "/api/scim/v2/Users/26", + location: "/api/scim/v2/Users/" + newUserID, status: "201" }, { method: "POST", From a57f042a07e4341bb941b31c0dad8715e9a368e5 Mon Sep 17 00:00:00 2001 From: fflorent Date: Fri, 6 Sep 2024 20:31:13 +0200 Subject: [PATCH 08/27] Add GRIST_ENABLE_SCIM env variable --- app/server/lib/FlexServer.ts | 7 +- test/server/lib/Scim.ts | 1081 +++++++++++++++++----------------- 2 files changed, 559 insertions(+), 529 deletions(-) diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 60c2faa0e7..a9c0b1c7d2 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -899,7 +899,12 @@ export class FlexServer implements GristServer { public addScimApi() { if (this._check('scim', 'api', 'homedb', 'json', 'api-mw')) { return; } - this.app.use('/api/scim', buildScimRouter(this._dbManager, this._installAdmin)); + const scimRouter = isAffirmative(process.env.GRIST_ENABLE_SCIM) ? + buildScimRouter(this._dbManager, this._installAdmin) : + () => { + throw new ApiError('SCIM API is not enabled', 501); + }; + this.app.use('/api/scim', scimRouter); } diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index 124bc6f28f..6fb1a804e4 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -30,261 +30,347 @@ const USER_CONFIG_BY_NAME = { type UserConfigByName = typeof USER_CONFIG_BY_NAME; describe('Scim', () => { - let oldEnv: testUtils.EnvironmentSnapshot; - let server: TestServer; - let homeUrl: string; - const userIdByName: {[name in keyof UserConfigByName]?: number} = {}; - - const scimUrl = (path: string) => (homeUrl + '/api/scim/v2' + path); - testUtils.setTmpLogLevel('error'); - before(async function () { - oldEnv = new testUtils.EnvironmentSnapshot(); - process.env.GRIST_DEFAULT_EMAIL = 'chimpy@getgrist.com'; - process.env.GRIST_SCIM_EMAIL = 'charon@getgrist.com'; - process.env.TYPEORM_DATABASE = ':memory:'; - server = new TestServer(this); - homeUrl = await server.start(); - const userNames = Object.keys(USER_CONFIG_BY_NAME) as Array; - for (const user of userNames) { - userIdByName[user] = await getOrCreateUserId(user); - } - }); + const setupTestServer = (env: NodeJS.ProcessEnv) => { + let homeUrl: string; + let oldEnv: testUtils.EnvironmentSnapshot; + let server: TestServer; - after(async () => { - oldEnv.restore(); - await server.stop(); - }); + before(async function () { + oldEnv = new testUtils.EnvironmentSnapshot(); + process.env.TYPEORM_DATABASE = ':memory:'; + Object.assign(process.env, env); + server = new TestServer(this); + homeUrl = await server.start(); + }); - function personaToSCIMMYUserWithId(user: keyof UserConfigByName) { - return toSCIMUserWithId(user, userIdByName[user]!); - } + after(async () => { + oldEnv.restore(); + await server.stop(); + }); - function toSCIMUserWithId(user: string, id: number) { return { - ...toSCIMUserWithoutId(user), - id: String(id), - meta: { resourceType: 'User', location: '/api/scim/v2/Users/' + id }, + scimUrl: (path: string) => (homeUrl + '/api/scim/v2' + path), + getDbManager: () => server.dbManager, }; - } + }; - function toSCIMUserWithoutId(user: string) { - return { - schemas: [ 'urn:ietf:params:scim:schemas:core:2.0:User' ], - userName: user + '@getgrist.com', - name: { formatted: capitalize(user) }, - displayName: capitalize(user), - preferredLanguage: 'en', - locale: 'en', - emails: [ { value: user + '@getgrist.com', primary: true } ] - }; - } + describe('when disabled', function () { + const { scimUrl } = setupTestServer({}); - async function getOrCreateUserId(user: string) { - return (await server.dbManager.getUserByLogin(user + '@getgrist.com'))!.id; - } + it('should return 501 for /api/scim/v2/Users', async function () { + const res = await axios.get(scimUrl('/Users'), chimpy); + assert.equal(res.status, 501); + assert.deepEqual(res.data, { error: 'SCIM API is not enabled' }); + }); + }); - async function cleanupUser(userId: number) { - if (await server.dbManager.getUser(userId)) { - await server.dbManager.deleteUser({ userId: userId }, userId); - } - } - - function checkEndpointNotAccessibleForNonAdminUsers( - method: 'get' | 'post' | 'put' | 'patch' | 'delete', - path: string, - validBody: object = {} - ) { - function makeCallWith(user: keyof UserConfigByName) { - if (method === 'get' || method === 'delete') { - return axios[method](scimUrl(path), USER_CONFIG_BY_NAME[user]); - } - return axios[method](scimUrl(path), validBody, USER_CONFIG_BY_NAME[user]); - } - it('should return 401 for anonymous', async function () { - const res: any = await makeCallWith('anon'); - assert.equal(res.status, 401); + describe('when enabled using GRIST_ENABLE_SCIM=1', function () { + const { scimUrl, getDbManager } = setupTestServer({ + GRIST_ENABLE_SCIM: '1', + GRIST_DEFAULT_EMAIL: 'chimpy@getgrist.com', + GRIST_SCIM_EMAIL: 'charon@getgrist.com', }); + const userIdByName: {[name in keyof UserConfigByName]?: number} = {}; - it('should return 401 for kiwi', async function () { - const res: any = await makeCallWith('kiwi'); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '403', - detail: 'You are not authorized to access this resource' - }); - assert.equal(res.status, 403); + before(async function () { + const userNames = Object.keys(USER_CONFIG_BY_NAME) as Array; + for (const user of userNames) { + userIdByName[user] = await getOrCreateUserId(user); + } }); - } - describe('/Me', function () { - async function checkGetMeAs(user: keyof UserConfigByName, expected: any) { - const res = await axios.get(scimUrl('/Me'), USER_CONFIG_BY_NAME[user]); - assert.equal(res.status, 200); - assert.deepInclude(res.data, expected); + function personaToSCIMMYUserWithId(user: keyof UserConfigByName) { + return toSCIMUserWithId(user, userIdByName[user]!); } - it(`should return the current user for chimpy`, async function () { - return checkGetMeAs('chimpy', personaToSCIMMYUserWithId('chimpy')); - }); + function toSCIMUserWithId(user: string, id: number) { + return { + ...toSCIMUserWithoutId(user), + id: String(id), + meta: { resourceType: 'User', location: '/api/scim/v2/Users/' + id }, + }; + } - it(`should return the current user for kiwi`, async function () { - return checkGetMeAs('kiwi', personaToSCIMMYUserWithId('kiwi')); - }); + function toSCIMUserWithoutId(user: string) { + return { + schemas: [ 'urn:ietf:params:scim:schemas:core:2.0:User' ], + userName: user + '@getgrist.com', + name: { formatted: capitalize(user) }, + displayName: capitalize(user), + preferredLanguage: 'en', + locale: 'en', + emails: [ { value: user + '@getgrist.com', primary: true } ] + }; + } - it('should return 401 for anonymous', async function () { - const res = await axios.get(scimUrl('/Me'), anon); - assert.equal(res.status, 401); - }); + async function getOrCreateUserId(user: string) { + return (await getDbManager().getUserByLogin(user + '@getgrist.com'))!.id; + } - it.skip('should allow operation like PATCH for kiwi', async function () { - // SKIPPING this test: only the GET verb is currently implemented by SCIMMY for the /Me endpoint. - // Issue created here: https://github.com/scimmyjs/scimmy/issues/47 - const patchBody = { - schemas: ['urn:ietf:params:scim:api:messages:2.0:PatchOp'], - Operations: [{ - op: "replace", - path: 'locale', - value: 'fr', - }], - }; - const res = await axios.patch(scimUrl('/Me'), patchBody, kiwi); - assert.equal(res.status, 200); - assert.deepEqual(res.data, { - ...personaToSCIMMYUserWithId('kiwi'), - locale: 'fr', - preferredLanguage: 'en', + async function cleanupUser(userId: number) { + if (await getDbManager().getUser(userId)) { + await getDbManager().deleteUser({ userId: userId }, userId); + } + } + + function checkEndpointNotAccessibleForNonAdminUsers( + method: 'get' | 'post' | 'put' | 'patch' | 'delete', + path: string, + validBody: object = {} + ) { + function makeCallWith(user: keyof UserConfigByName) { + if (method === 'get' || method === 'delete') { + return axios[method](scimUrl(path), USER_CONFIG_BY_NAME[user]); + } + return axios[method](scimUrl(path), validBody, USER_CONFIG_BY_NAME[user]); + } + it('should return 401 for anonymous', async function () { + const res: any = await makeCallWith('anon'); + assert.equal(res.status, 401); }); - }); - }); + it('should return 403 for kiwi', async function () { + const res: any = await makeCallWith('kiwi'); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '403', + detail: 'You are not authorized to access this resource' + }); + assert.equal(res.status, 403); + }); + } - describe('/Users/{id}', function () { + describe('/Me', function () { + async function checkGetMeAs(user: keyof UserConfigByName, expected: any) { + const res = await axios.get(scimUrl('/Me'), USER_CONFIG_BY_NAME[user]); + assert.equal(res.status, 200); + assert.deepInclude(res.data, expected); + } - it('should return the user of id=1 for chimpy', async function () { - const res = await axios.get(scimUrl('/Users/1'), chimpy); + it(`should return the current user for chimpy`, async function () { + return checkGetMeAs('chimpy', personaToSCIMMYUserWithId('chimpy')); + }); - assert.equal(res.status, 200); - assert.deepInclude(res.data, { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - id: '1', - displayName: 'Chimpy', - userName: 'chimpy@getgrist.com' + it(`should return the current user for kiwi`, async function () { + return checkGetMeAs('kiwi', personaToSCIMMYUserWithId('kiwi')); }); - }); - it('should return 404 when the user is not found', async function () { - const res = await axios.get(scimUrl('/Users/1000'), chimpy); - assert.equal(res.status, 404); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '404', - detail: 'User with ID 1000 not found' + it('should return 401 for anonymous', async function () { + const res = await axios.get(scimUrl('/Me'), anon); + assert.equal(res.status, 401); }); - }); - checkEndpointNotAccessibleForNonAdminUsers('get', '/Users/1'); - }); + it.skip('should allow operation like PATCH for kiwi', async function () { + // SKIPPING this test: only the GET verb is currently implemented by SCIMMY for the /Me endpoint. + // Issue created here: https://github.com/scimmyjs/scimmy/issues/47 + const patchBody = { + schemas: ['urn:ietf:params:scim:api:messages:2.0:PatchOp'], + Operations: [{ + op: "replace", + path: 'locale', + value: 'fr', + }], + }; + const res = await axios.patch(scimUrl('/Me'), patchBody, kiwi); + assert.equal(res.status, 200); + assert.deepEqual(res.data, { + ...personaToSCIMMYUserWithId('kiwi'), + locale: 'fr', + preferredLanguage: 'en', + }); + }); - describe('GET /Users', function () { - it('should return all users for chimpy', async function () { - const res = await axios.get(scimUrl('/Users'), chimpy); - assert.equal(res.status, 200); - assert.isAbove(res.data.totalResults, 0, 'should have retrieved some users'); - assert.deepInclude(res.data.Resources, personaToSCIMMYUserWithId('chimpy')); - assert.deepInclude(res.data.Resources, personaToSCIMMYUserWithId('kiwi')); }); - checkEndpointNotAccessibleForNonAdminUsers('get', '/Users'); - }); + describe('/Users/{id}', function () { - describe('POST /Users/.search', function () { - const SEARCH_SCHEMA = 'urn:ietf:params:scim:api:messages:2.0:SearchRequest'; + it('should return the user of id=1 for chimpy', async function () { + const res = await axios.get(scimUrl('/Users/1'), chimpy); - const searchExample = { - schemas: [SEARCH_SCHEMA], - sortBy: 'userName', - sortOrder: 'descending', - }; + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + id: '1', + displayName: 'Chimpy', + userName: 'chimpy@getgrist.com' + }); + }); - it('should return all users for chimpy order by userName in descending order', async function () { - const res = await axios.post(scimUrl('/Users/.search'), searchExample, chimpy); - assert.equal(res.status, 200); - assert.isAbove(res.data.totalResults, 0, 'should have retrieved some users'); - const users = res.data.Resources.map((r: any) => r.userName); - assert.include(users, 'chimpy@getgrist.com'); - assert.include(users, 'kiwi@getgrist.com'); - const indexOfChimpy = users.indexOf('chimpy@getgrist.com'); - const indexOfKiwi = users.indexOf('kiwi@getgrist.com'); - assert.isBelow(indexOfKiwi, indexOfChimpy, 'kiwi should come before chimpy'); + it('should return 404 when the user is not found', async function () { + const res = await axios.get(scimUrl('/Users/1000'), chimpy); + assert.equal(res.status, 404); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '404', + detail: 'User with ID 1000 not found' + }); + }); + + checkEndpointNotAccessibleForNonAdminUsers('get', '/Users/1'); }); - it('should also allow access for user Charon (the one refered in GRIST_SCIM_EMAIL)', async function () { - const res = await axios.post(scimUrl('/Users/.search'), searchExample, charon); - assert.equal(res.status, 200); + describe('GET /Users', function () { + it('should return all users for chimpy', async function () { + const res = await axios.get(scimUrl('/Users'), chimpy); + assert.equal(res.status, 200); + assert.isAbove(res.data.totalResults, 0, 'should have retrieved some users'); + assert.deepInclude(res.data.Resources, personaToSCIMMYUserWithId('chimpy')); + assert.deepInclude(res.data.Resources, personaToSCIMMYUserWithId('kiwi')); + }); + + checkEndpointNotAccessibleForNonAdminUsers('get', '/Users'); }); - it('should filter the users by userName', async function () { - const res = await axios.post(scimUrl('/Users/.search'), { + describe('POST /Users/.search', function () { + const SEARCH_SCHEMA = 'urn:ietf:params:scim:api:messages:2.0:SearchRequest'; + + const searchExample = { schemas: [SEARCH_SCHEMA], - attributes: ['userName'], - filter: 'userName sw "chimpy"', - }, chimpy); - assert.equal(res.status, 200); - assert.equal(res.data.totalResults, 1); - assert.deepEqual(res.data.Resources[0], { id: String(userIdByName['chimpy']), userName: 'chimpy@getgrist.com' }, - "should have retrieved only chimpy's username and not other attribute"); - }); + sortBy: 'userName', + sortOrder: 'descending', + }; - checkEndpointNotAccessibleForNonAdminUsers('post', '/Users/.search', searchExample); - }); + it('should return all users for chimpy order by userName in descending order', async function () { + const res = await axios.post(scimUrl('/Users/.search'), searchExample, chimpy); + assert.equal(res.status, 200); + assert.isAbove(res.data.totalResults, 0, 'should have retrieved some users'); + const users = res.data.Resources.map((r: any) => r.userName); + assert.include(users, 'chimpy@getgrist.com'); + assert.include(users, 'kiwi@getgrist.com'); + const indexOfChimpy = users.indexOf('chimpy@getgrist.com'); + const indexOfKiwi = users.indexOf('kiwi@getgrist.com'); + assert.isBelow(indexOfKiwi, indexOfChimpy, 'kiwi should come before chimpy'); + }); - describe('POST /Users', function () { // Create a new users - async function withUserName(userName: string, cb: (userName: string) => Promise) { - try { - await cb(userName); - } finally { - const user = await server.dbManager.getExistingUserByLogin(userName + "@getgrist.com"); - if (user) { - await cleanupUser(user.id); - } - } - } - it('should create a new user', async function () { - await withUserName('newuser1', async (userName) => { - const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId(userName), chimpy); - assert.equal(res.status, 201); - const newUserId = await getOrCreateUserId(userName); - assert.deepEqual(res.data, toSCIMUserWithId(userName, newUserId)); + it('should also allow access for user Charon (the one refered in GRIST_SCIM_EMAIL)', async function () { + const res = await axios.post(scimUrl('/Users/.search'), searchExample, charon); + assert.equal(res.status, 200); }); - }); - it('should allow creating a new user given only their email passed as username', async function () { - await withUserName('new.user2', async (userName) => { - const res = await axios.post(scimUrl('/Users'), { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - userName: 'new.user2@getgrist.com', + it('should filter the users by userName', async function () { + const res = await axios.post(scimUrl('/Users/.search'), { + schemas: [SEARCH_SCHEMA], + attributes: ['userName'], + filter: 'userName sw "chimpy"', }, chimpy); - assert.equal(res.status, 201); - assert.equal(res.data.userName, userName + '@getgrist.com'); - assert.equal(res.data.displayName, userName); + assert.equal(res.status, 200); + assert.equal(res.data.totalResults, 1); + assert.deepEqual(res.data.Resources[0], { id: String(userIdByName['chimpy']), userName: 'chimpy@getgrist.com' }, + "should have retrieved only chimpy's username and not other attribute"); }); + + checkEndpointNotAccessibleForNonAdminUsers('post', '/Users/.search', searchExample); }); - it('should also allow user Charon to create a user (the one refered in GRIST_SCIM_EMAIL)', async function () { - await withUserName('new.user.by.charon', async (userName) => { - const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId(userName), charon); - assert.equal(res.status, 201); + describe('POST /Users', function () { // Create a new users + async function withUserName(userName: string, cb: (userName: string) => Promise) { + try { + await cb(userName); + } finally { + const user = await getDbManager().getExistingUserByLogin(userName + "@getgrist.com"); + if (user) { + await cleanupUser(user.id); + } + } + } + it('should create a new user', async function () { + await withUserName('newuser1', async (userName) => { + const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId(userName), chimpy); + assert.equal(res.status, 201); + const newUserId = await getOrCreateUserId(userName); + assert.deepEqual(res.data, toSCIMUserWithId(userName, newUserId)); + }); + }); + + it('should allow creating a new user given only their email passed as username', async function () { + await withUserName('new.user2', async (userName) => { + const res = await axios.post(scimUrl('/Users'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: 'new.user2@getgrist.com', + }, chimpy); + assert.equal(res.status, 201); + assert.equal(res.data.userName, userName + '@getgrist.com'); + assert.equal(res.data.displayName, userName); + }); }); + + it('should also allow user Charon to create a user (the one refered in GRIST_SCIM_EMAIL)', async function () { + await withUserName('new.user.by.charon', async (userName) => { + const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId(userName), charon); + assert.equal(res.status, 201); + }); + }); + + it('should reject when passed email differs from username', async function () { + await withUserName('username', async (userName) => { + const res = await axios.post(scimUrl('/Users'), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: userName + '@getgrist.com', + emails: [{ value: 'emails.value@getgrist.com' }], + }, chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '400', + detail: 'Email and userName must be the same', + scimType: 'invalidValue' + }); + assert.equal(res.status, 400); + }); + }); + + it('should disallow creating a user with the same email', async function () { + const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId('chimpy'), chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '409', + detail: 'An existing user with the passed email exist.', + scimType: 'uniqueness' + }); + assert.equal(res.status, 409); + }); + + checkEndpointNotAccessibleForNonAdminUsers('post', '/Users', toSCIMUserWithoutId('some-user')); }); - it('should reject when passed email differs from username', async function () { - await withUserName('username', async (userName) => { - const res = await axios.post(scimUrl('/Users'), { + describe('PUT /Users/{id}', function () { + let userToUpdateId: number; + const userToUpdateEmailLocalPart = 'user-to-update'; + + beforeEach(async function () { + userToUpdateId = await getOrCreateUserId(userToUpdateEmailLocalPart); + }); + afterEach(async function () { + await cleanupUser(userToUpdateId); + }); + + it('should update an existing user', async function () { + const userToUpdateProperties = { schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - userName: userName + '@getgrist.com', - emails: [{ value: 'emails.value@getgrist.com' }], + userName: userToUpdateEmailLocalPart + '-now-updated@getgrist.com', + displayName: 'User to Update', + photos: [{ value: 'https://example.com/photo.jpg', type: 'photo', primary: true }], + locale: 'fr', + }; + const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), userToUpdateProperties, chimpy); + assert.equal(res.status, 200); + const refreshedUser = await axios.get(scimUrl(`/Users/${userToUpdateId}`), chimpy); + assert.deepEqual(refreshedUser.data, { + ...userToUpdateProperties, + id: String(userToUpdateId), + meta: { resourceType: 'User', location: `/api/scim/v2/Users/${userToUpdateId}` }, + emails: [ { value: userToUpdateProperties.userName, primary: true } ], + name: { formatted: userToUpdateProperties.displayName }, + preferredLanguage: 'fr', + }); + }); + + it('should reject when passed email differs from username', async function () { + const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: userToUpdateEmailLocalPart + '@getgrist.com', + emails: [{ value: 'whatever@getgrist.com', primary: true }], }, chimpy); assert.deepEqual(res.data, { schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], @@ -294,377 +380,316 @@ describe('Scim', () => { }); assert.equal(res.status, 400); }); - }); - it('should disallow creating a user with the same email', async function () { - const res = await axios.post(scimUrl('/Users'), toSCIMUserWithoutId('chimpy'), chimpy); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '409', - detail: 'An existing user with the passed email exist.', - scimType: 'uniqueness' + it('should disallow updating a user with the same email as another user\'s', async function () { + const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), toSCIMUserWithoutId('chimpy'), chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '409', + detail: 'An existing user with the passed email exist.', + scimType: 'uniqueness' + }); + assert.equal(res.status, 409); }); - assert.equal(res.status, 409); - }); - checkEndpointNotAccessibleForNonAdminUsers('post', '/Users', toSCIMUserWithoutId('some-user')); - }); - - describe('PUT /Users/{id}', function () { - let userToUpdateId: number; - const userToUpdateEmailLocalPart = 'user-to-update'; + it('should return 404 when the user is not found', async function () { + const res = await axios.put(scimUrl('/Users/1000'), toSCIMUserWithoutId('whoever'), chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '404', + detail: 'unable to find user to update' + }); + assert.equal(res.status, 404); + }); - beforeEach(async function () { - userToUpdateId = await getOrCreateUserId(userToUpdateEmailLocalPart); - }); - afterEach(async function () { - await cleanupUser(userToUpdateId); - }); + it('should deduce the name from the displayEmail when not provided', async function () { + const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: 'my-email@getgrist.com', + }, chimpy); + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + id: String(userToUpdateId), + userName: 'my-email@getgrist.com', + displayName: 'my-email', + }); + }); - it('should update an existing user', async function () { - const userToUpdateProperties = { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - userName: userToUpdateEmailLocalPart + '-now-updated@getgrist.com', - displayName: 'User to Update', - photos: [{ value: 'https://example.com/photo.jpg', type: 'photo', primary: true }], - locale: 'fr', - }; - const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), userToUpdateProperties, chimpy); - assert.equal(res.status, 200); - const refreshedUser = await axios.get(scimUrl(`/Users/${userToUpdateId}`), chimpy); - assert.deepEqual(refreshedUser.data, { - ...userToUpdateProperties, - id: String(userToUpdateId), - meta: { resourceType: 'User', location: `/api/scim/v2/Users/${userToUpdateId}` }, - emails: [ { value: userToUpdateProperties.userName, primary: true } ], - name: { formatted: userToUpdateProperties.displayName }, - preferredLanguage: 'fr', + it('should normalize the passed email for the userName and keep the case for email.value', async function () { + const newEmail = 'my-EMAIL@getgrist.com'; + const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + userName: newEmail, + }, chimpy); + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], + id: String(userToUpdateId), + userName: newEmail.toLowerCase(), + displayName: 'my-EMAIL', + emails: [{ value: newEmail, primary: true }] + }); }); - }); - it('should reject when passed email differs from username', async function () { - const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - userName: userToUpdateEmailLocalPart + '@getgrist.com', - emails: [{ value: 'whatever@getgrist.com', primary: true }], - }, chimpy); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '400', - detail: 'Email and userName must be the same', - scimType: 'invalidValue' - }); - assert.equal(res.status, 400); + checkEndpointNotAccessibleForNonAdminUsers('put', '/Users/1', toSCIMUserWithoutId('chimpy')); }); - it('should disallow updating a user with the same email as another user\'s', async function () { - const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), toSCIMUserWithoutId('chimpy'), chimpy); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '409', - detail: 'An existing user with the passed email exist.', - scimType: 'uniqueness' + describe('PATCH /Users/{id}', function () { + let userToPatchId: number; + const userToPatchEmailLocalPart = 'user-to-patch'; + beforeEach(async function () { + userToPatchId = await getOrCreateUserId(userToPatchEmailLocalPart); }); - assert.equal(res.status, 409); - }); - - it('should return 404 when the user is not found', async function () { - const res = await axios.put(scimUrl('/Users/1000'), toSCIMUserWithoutId('whoever'), chimpy); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '404', - detail: 'unable to find user to update' + afterEach(async function () { + await cleanupUser(userToPatchId); }); - assert.equal(res.status, 404); - }); - it('should deduce the name from the displayEmail when not provided', async function () { - const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - userName: 'my-email@getgrist.com', - }, chimpy); - assert.equal(res.status, 200); - assert.deepInclude(res.data, { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - id: String(userToUpdateId), - userName: 'my-email@getgrist.com', - displayName: 'my-email', + const validPatchBody = (newName: string) => ({ + schemas: ['urn:ietf:params:scim:api:messages:2.0:PatchOp'], + Operations: [{ + op: "replace", + path: "displayName", + value: newName, + }, { + op: "replace", + path: "locale", + value: 'fr' + }], }); - }); - it('should normalize the passed email for the userName and keep the case for email.value', async function () { - const newEmail = 'my-EMAIL@getgrist.com'; - const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - userName: newEmail, - }, chimpy); - assert.equal(res.status, 200); - assert.deepInclude(res.data, { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - id: String(userToUpdateId), - userName: newEmail.toLowerCase(), - displayName: 'my-EMAIL', - emails: [{ value: newEmail, primary: true }] + it('should replace values of an existing user', async function () { + const newName = 'User to Patch new Name'; + const res = await axios.patch(scimUrl(`/Users/${userToPatchId}`), validPatchBody(newName), chimpy); + assert.equal(res.status, 200); + const refreshedUser = await axios.get(scimUrl(`/Users/${userToPatchId}`), chimpy); + assert.deepEqual(refreshedUser.data, { + ...toSCIMUserWithId(userToPatchEmailLocalPart, userToPatchId), + displayName: newName, + name: { formatted: newName }, + locale: 'fr', + preferredLanguage: 'fr', + }); }); - }); - checkEndpointNotAccessibleForNonAdminUsers('put', '/Users/1', toSCIMUserWithoutId('chimpy')); - }); - - describe('PATCH /Users/{id}', function () { - let userToPatchId: number; - const userToPatchEmailLocalPart = 'user-to-patch'; - beforeEach(async function () { - userToPatchId = await getOrCreateUserId(userToPatchEmailLocalPart); - }); - afterEach(async function () { - await cleanupUser(userToPatchId); + checkEndpointNotAccessibleForNonAdminUsers('patch', '/Users/1', validPatchBody('new name2')); }); - const validPatchBody = (newName: string) => ({ - schemas: ['urn:ietf:params:scim:api:messages:2.0:PatchOp'], - Operations: [{ - op: "replace", - path: "displayName", - value: newName, - }, { - op: "replace", - path: "locale", - value: 'fr' - }], - }); + describe('DELETE /Users/{id}', function () { + let userToDeleteId: number; + const userToDeleteEmailLocalPart = 'user-to-delete'; - it('should replace values of an existing user', async function () { - const newName = 'User to Patch new Name'; - const res = await axios.patch(scimUrl(`/Users/${userToPatchId}`), validPatchBody(newName), chimpy); - assert.equal(res.status, 200); - const refreshedUser = await axios.get(scimUrl(`/Users/${userToPatchId}`), chimpy); - assert.deepEqual(refreshedUser.data, { - ...toSCIMUserWithId(userToPatchEmailLocalPart, userToPatchId), - displayName: newName, - name: { formatted: newName }, - locale: 'fr', - preferredLanguage: 'fr', + beforeEach(async function () { + userToDeleteId = await getOrCreateUserId(userToDeleteEmailLocalPart); + }); + afterEach(async function () { + await cleanupUser(userToDeleteId); }); - }); - - checkEndpointNotAccessibleForNonAdminUsers('patch', '/Users/1', validPatchBody('new name2')); - }); - - describe('DELETE /Users/{id}', function () { - let userToDeleteId: number; - const userToDeleteEmailLocalPart = 'user-to-delete'; - - beforeEach(async function () { - userToDeleteId = await getOrCreateUserId(userToDeleteEmailLocalPart); - }); - afterEach(async function () { - await cleanupUser(userToDeleteId); - }); - it('should delete some user', async function () { - const res = await axios.delete(scimUrl(`/Users/${userToDeleteId}`), chimpy); - assert.equal(res.status, 204); - const refreshedUser = await axios.get(scimUrl(`/Users/${userToDeleteId}`), chimpy); - assert.equal(refreshedUser.status, 404); - }); + it('should delete some user', async function () { + const res = await axios.delete(scimUrl(`/Users/${userToDeleteId}`), chimpy); + assert.equal(res.status, 204); + const refreshedUser = await axios.get(scimUrl(`/Users/${userToDeleteId}`), chimpy); + assert.equal(refreshedUser.status, 404); + }); - it('should return 404 when the user is not found', async function () { - const res = await axios.delete(scimUrl('/Users/1000'), chimpy); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '404', - detail: 'user not found' + it('should return 404 when the user is not found', async function () { + const res = await axios.delete(scimUrl('/Users/1000'), chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '404', + detail: 'user not found' + }); + assert.equal(res.status, 404); }); - assert.equal(res.status, 404); + checkEndpointNotAccessibleForNonAdminUsers('delete', '/Users/1'); }); - checkEndpointNotAccessibleForNonAdminUsers('delete', '/Users/1'); - }); - describe('POST /Bulk', function () { - let usersToCleanupEmails: string[]; + describe('POST /Bulk', function () { + let usersToCleanupEmails: string[]; - beforeEach(async function () { - usersToCleanupEmails = []; - }); + beforeEach(async function () { + usersToCleanupEmails = []; + }); - afterEach(async function () { - for (const email of usersToCleanupEmails) { - const user = await server.dbManager.getExistingUserByLogin(email); - if (user) { - await cleanupUser(user.id); + afterEach(async function () { + for (const email of usersToCleanupEmails) { + const user = await getDbManager().getExistingUserByLogin(email); + if (user) { + await cleanupUser(user.id); + } } - } - }); + }); - it('should return statuses for each operation', async function () { - const putOnUnknownResource = { method: 'PUT', path: '/Users/1000', value: toSCIMUserWithoutId('chimpy') }; - const validCreateOperation = { - method: 'POST', path: '/Users/', data: toSCIMUserWithoutId('bulk-user3'), bulkId: '1' - }; - usersToCleanupEmails.push('bulk-user3'); - const createOperationWithUserNameConflict = { - method: 'POST', path: '/Users/', data: toSCIMUserWithoutId('chimpy'), bulkId: '2' - }; - const res = await axios.post(scimUrl('/Bulk'), { - schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], - Operations: [ - putOnUnknownResource, - validCreateOperation, - createOperationWithUserNameConflict, - ], - }, chimpy); - assert.equal(res.status, 200); + it('should return statuses for each operation', async function () { + const putOnUnknownResource = { method: 'PUT', path: '/Users/1000', value: toSCIMUserWithoutId('chimpy') }; + const validCreateOperation = { + method: 'POST', path: '/Users/', data: toSCIMUserWithoutId('bulk-user3'), bulkId: '1' + }; + usersToCleanupEmails.push('bulk-user3'); + const createOperationWithUserNameConflict = { + method: 'POST', path: '/Users/', data: toSCIMUserWithoutId('chimpy'), bulkId: '2' + }; + const res = await axios.post(scimUrl('/Bulk'), { + schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], + Operations: [ + putOnUnknownResource, + validCreateOperation, + createOperationWithUserNameConflict, + ], + }, chimpy); + assert.equal(res.status, 200); - const newUserID = await getOrCreateUserId('bulk-user3'); - assert.deepEqual(res.data, { - schemas: [ "urn:ietf:params:scim:api:messages:2.0:BulkResponse" ], - Operations: [ - { - method: "PUT", - location: "/api/scim/v2/Users/1000", - status: "400", - response: { - schemas: [ - "urn:ietf:params:scim:api:messages:2.0:Error" - ], + const newUserID = await getOrCreateUserId('bulk-user3'); + assert.deepEqual(res.data, { + schemas: [ "urn:ietf:params:scim:api:messages:2.0:BulkResponse" ], + Operations: [ + { + method: "PUT", + location: "/api/scim/v2/Users/1000", status: "400", - scimType: "invalidSyntax", - detail: "Expected 'data' to be a single complex value in BulkRequest operation #1" - } - }, { - method: "POST", - bulkId: "1", - location: "/api/scim/v2/Users/" + newUserID, - status: "201" - }, { - method: "POST", - bulkId: "2", - status: "409", - response: { - schemas: [ - "urn:ietf:params:scim:api:messages:2.0:Error" - ], + response: { + schemas: [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + status: "400", + scimType: "invalidSyntax", + detail: "Expected 'data' to be a single complex value in BulkRequest operation #1" + } + }, { + method: "POST", + bulkId: "1", + location: "/api/scim/v2/Users/" + newUserID, + status: "201" + }, { + method: "POST", + bulkId: "2", status: "409", - scimType: "uniqueness", - detail: "An existing user with the passed email exist." + response: { + schemas: [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + status: "409", + scimType: "uniqueness", + detail: "An existing user with the passed email exist." + } } - } - ] + ] + }); }); - }); - it('should return 400 when no operations are provided', async function () { - const res = await axios.post(scimUrl('/Bulk'), { - schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], - Operations: [], - }, chimpy); - assert.equal(res.status, 400); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '400', - detail: "BulkRequest request body must contain 'Operations' attribute with at least one operation", - scimType: 'invalidValue' + it('should return 400 when no operations are provided', async function () { + const res = await axios.post(scimUrl('/Bulk'), { + schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], + Operations: [], + }, chimpy); + assert.equal(res.status, 400); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '400', + detail: "BulkRequest request body must contain 'Operations' attribute with at least one operation", + scimType: 'invalidValue' + }); }); - }); - it('should disallow accessing resources to kiwi', async function () { - const creationOperation = { - method: 'POST', path: '/Users', data: toSCIMUserWithoutId('bulk-user4'), bulkId: '1' - }; - usersToCleanupEmails.push('bulk-user4'); - const selfPutOperation = { method: 'PUT', path: '/Me', value: toSCIMUserWithoutId('kiwi') }; - const res = await axios.post(scimUrl('/Bulk'), { - schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], - Operations: [ - creationOperation, - selfPutOperation, - ], - }, kiwi); - assert.equal(res.status, 200); - assert.deepEqual(res.data, { - schemas: [ "urn:ietf:params:scim:api:messages:2.0:BulkResponse" ], - Operations: [ - { - method: "POST", - bulkId: "1", - status: "403", - response: { - detail: "You are not authorized to access this resource", - schemas: [ "urn:ietf:params:scim:api:messages:2.0:Error" ], - status: "403" - } - }, { - // When writing this test, the SCIMMY implementation does not yet support PUT operations on /Me. - // This reflects the current behavior, but it may change in the future. - // Change this test if the behavior changes. - // It is probably fine to allow altering oneself even for non-admins. - method: "PUT", - location: "/Me", - status: "400", - response: { - schemas: [ - "urn:ietf:params:scim:api:messages:2.0:Error" - ], + it('should disallow accessing resources to kiwi', async function () { + const creationOperation = { + method: 'POST', path: '/Users', data: toSCIMUserWithoutId('bulk-user4'), bulkId: '1' + }; + usersToCleanupEmails.push('bulk-user4'); + const selfPutOperation = { method: 'PUT', path: '/Me', value: toSCIMUserWithoutId('kiwi') }; + const res = await axios.post(scimUrl('/Bulk'), { + schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], + Operations: [ + creationOperation, + selfPutOperation, + ], + }, kiwi); + assert.equal(res.status, 200); + assert.deepEqual(res.data, { + schemas: [ "urn:ietf:params:scim:api:messages:2.0:BulkResponse" ], + Operations: [ + { + method: "POST", + bulkId: "1", + status: "403", + response: { + detail: "You are not authorized to access this resource", + schemas: [ "urn:ietf:params:scim:api:messages:2.0:Error" ], + status: "403" + } + }, { + // When writing this test, the SCIMMY implementation does not yet support PUT operations on /Me. + // This reflects the current behavior, but it may change in the future. + // Change this test if the behavior changes. + // It is probably fine to allow altering oneself even for non-admins. + method: "PUT", + location: "/Me", status: "400", - detail: "Invalid 'path' value '/Me' in BulkRequest operation #2", - scimType: "invalidValue" + response: { + schemas: [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + status: "400", + detail: "Invalid 'path' value '/Me' in BulkRequest operation #2", + scimType: "invalidValue" + } } - } - ] + ] + }); }); - }); - it('should disallow accessing resources to anonymous', async function () { - const creationOperation = { - method: 'POST', path: '/Users', data: toSCIMUserWithoutId('bulk-user5'), bulkId: '1' - }; - usersToCleanupEmails.push('bulk-user5'); - const res = await axios.post(scimUrl('/Bulk'), { - schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], - Operations: [creationOperation], - }, anon); - assert.equal(res.status, 401); + it('should disallow accessing resources to anonymous', async function () { + const creationOperation = { + method: 'POST', path: '/Users', data: toSCIMUserWithoutId('bulk-user5'), bulkId: '1' + }; + usersToCleanupEmails.push('bulk-user5'); + const res = await axios.post(scimUrl('/Bulk'), { + schemas: ['urn:ietf:params:scim:api:messages:2.0:BulkRequest'], + Operations: [creationOperation], + }, anon); + assert.equal(res.status, 401); + }); }); - }); - it('should allow fetching the Scim schema when autenticated', async function () { - const res = await axios.get(scimUrl('/Schemas'), kiwi); - assert.equal(res.status, 200); - assert.deepInclude(res.data, { - schemas: ['urn:ietf:params:scim:api:messages:2.0:ListResponse'], - }); - assert.property(res.data, 'Resources'); - assert.deepInclude(res.data.Resources[0], { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:Schema'], - id: 'urn:ietf:params:scim:schemas:core:2.0:User', - name: 'User', - description: 'User Account', + it('should allow fetching the Scim schema when autenticated', async function () { + const res = await axios.get(scimUrl('/Schemas'), kiwi); + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:api:messages:2.0:ListResponse'], + }); + assert.property(res.data, 'Resources'); + assert.deepInclude(res.data.Resources[0], { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:Schema'], + id: 'urn:ietf:params:scim:schemas:core:2.0:User', + name: 'User', + description: 'User Account', + }); }); - }); - it('should allow fetching the Scim resource types when autenticated', async function () { - const res = await axios.get(scimUrl('/ResourceTypes'), kiwi); - assert.equal(res.status, 200); - assert.deepInclude(res.data, { - schemas: ['urn:ietf:params:scim:api:messages:2.0:ListResponse'], - }); - assert.property(res.data, 'Resources'); - assert.deepInclude(res.data.Resources[0], { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:ResourceType'], - name: 'User', - endpoint: '/Users', + it('should allow fetching the Scim resource types when autenticated', async function () { + const res = await axios.get(scimUrl('/ResourceTypes'), kiwi); + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:api:messages:2.0:ListResponse'], + }); + assert.property(res.data, 'Resources'); + assert.deepInclude(res.data.Resources[0], { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:ResourceType'], + name: 'User', + endpoint: '/Users', + }); }); - }); - it('should allow fetching the Scim service provider config when autenticated', async function () { - const res = await axios.get(scimUrl('/ServiceProviderConfig'), kiwi); - assert.equal(res.status, 200); - assert.deepInclude(res.data, { - schemas: ['urn:ietf:params:scim:schemas:core:2.0:ServiceProviderConfig'], + it('should allow fetching the Scim service provider config when autenticated', async function () { + const res = await axios.get(scimUrl('/ServiceProviderConfig'), kiwi); + assert.equal(res.status, 200); + assert.deepInclude(res.data, { + schemas: ['urn:ietf:params:scim:schemas:core:2.0:ServiceProviderConfig'], + }); + assert.property(res.data, 'patch'); + assert.property(res.data, 'bulk'); + assert.property(res.data, 'filter'); }); - assert.property(res.data, 'patch'); - assert.property(res.data, 'bulk'); - assert.property(res.data, 'filter'); }); }); From 621522852af0e640dbe7ab0b82431cb956325748 Mon Sep 17 00:00:00 2001 From: fflorent Date: Fri, 6 Sep 2024 22:10:32 +0200 Subject: [PATCH 09/27] Move logic for Users to its own controller --- app/server/lib/FlexServer.ts | 2 + app/server/lib/scim/v2/ScimTypes.ts | 6 + app/server/lib/scim/v2/ScimUserController.ts | 121 +++++++++++++++++++ app/server/lib/scim/v2/ScimV2Api.ts | 99 ++------------- test/server/lib/Scim.ts | 11 ++ 5 files changed, 152 insertions(+), 87 deletions(-) create mode 100644 app/server/lib/scim/v2/ScimTypes.ts create mode 100644 app/server/lib/scim/v2/ScimUserController.ts diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index a9c0b1c7d2..524b60b156 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -899,11 +899,13 @@ export class FlexServer implements GristServer { public addScimApi() { if (this._check('scim', 'api', 'homedb', 'json', 'api-mw')) { return; } + const scimRouter = isAffirmative(process.env.GRIST_ENABLE_SCIM) ? buildScimRouter(this._dbManager, this._installAdmin) : () => { throw new ApiError('SCIM API is not enabled', 501); }; + this.app.use('/api/scim', scimRouter); } diff --git a/app/server/lib/scim/v2/ScimTypes.ts b/app/server/lib/scim/v2/ScimTypes.ts new file mode 100644 index 0000000000..d70aaa0d0d --- /dev/null +++ b/app/server/lib/scim/v2/ScimTypes.ts @@ -0,0 +1,6 @@ +export interface RequestContext { + path: string; + isAdmin: boolean; + isScimUser: boolean; +} + diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts new file mode 100644 index 0000000000..2106d984b1 --- /dev/null +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -0,0 +1,121 @@ +import { ApiError } from 'app/common/ApiError'; +import { HomeDBManager, Scope } from 'app/gen-server/lib/homedb/HomeDBManager'; +import SCIMMY from 'scimmy'; +import { toSCIMMYUser, toUserProfile } from './ScimUserUtils'; +import { RequestContext } from './ScimTypes'; + +class ScimUserController { + private static _getIdFromResource(resource: any) { + const id = parseInt(resource.id, 10); + if (Number.isNaN(id)) { + throw new SCIMMY.Types.Error(400, 'invalidValue', 'Invalid passed user ID'); + } + return id; + } + + constructor( + private _dbManager: HomeDBManager, + private _checkAccess: (context: RequestContext) => void + ) {} + + public async getSingleUser(resource: any, context: RequestContext) { + this._checkAccess(context); + + const id = ScimUserController._getIdFromResource(resource); + const user = await this._dbManager.getUser(id); + if (!user) { + throw new SCIMMY.Types.Error(404, null!, `User with ID ${id} not found`); + } + return toSCIMMYUser(user); + } + + public async getUsers(resource: any, context: RequestContext) { + this._checkAccess(context); + + const { filter } = resource; + const scimmyUsers = (await this._dbManager.getUsers()).map(user => toSCIMMYUser(user)); + return filter ? filter.match(scimmyUsers) : scimmyUsers; + } + + public async createUser(data: any, context: RequestContext) { + this._checkAccess(context); + + try { + await this._checkEmailIsUnique(data.userName); + const userProfile = toUserProfile(data); + const newUser = await this._dbManager.getUserByLoginWithRetry(userProfile.email, { + profile: userProfile + }); + return toSCIMMYUser(newUser); + } catch (ex) { + return this._toScimError(ex); + } + } + + public async overrideUser(resource: any, data: any, context: RequestContext) { + this._checkAccess(context); + + try { + const id = ScimUserController._getIdFromResource(resource); + await this._checkEmailIsUnique(data.userName, id); + const updatedUser = await this._dbManager.overrideUser(id, toUserProfile(data)); + return toSCIMMYUser(updatedUser); + } catch (ex) { + return this._toScimError(ex); + } + } + + public async deleteUser(resource: any, context: RequestContext) { + this._checkAccess(context); + + const id = ScimUserController._getIdFromResource(resource); + try { + const fakeScope: Scope = { userId: id }; + // FIXME: deleteUser should probably better not requiring a scope. + await this._dbManager.deleteUser(fakeScope, id); + } catch (ex) { + return this._toScimError(ex); + } + } + + private async _toScimError(ex: Error) { + if (ex instanceof ApiError) { + if (ex.status === 409) { + throw new SCIMMY.Types.Error(ex.status, 'uniqueness', ex.message); + } + throw new SCIMMY.Types.Error(ex.status, null!, ex.message); + } + throw ex; + } + + private async _checkEmailIsUnique(email: string, id?: number) { + const existingUser = await this._dbManager.getExistingUserByLogin(email); + if (existingUser !== undefined && existingUser.id !== id) { + throw new SCIMMY.Types.Error(409, 'uniqueness', 'An existing user with the passed email exist.'); + } + } +} + +export const getScimUserConfig = ( + dbManager: HomeDBManager, checkAccess: (context: RequestContext) => void +) => { + const controller = new ScimUserController(dbManager, checkAccess); + + return { + egress: async (resource: any, context: RequestContext) => { + if (resource.id) { + return await controller.getSingleUser(resource, context); + } + return await controller.getUsers(resource, context); + }, + ingress: async (resource: any, data: any, context: RequestContext) => { + if (resource.id) { + return await controller.overrideUser(resource, data, context); + } + return await controller.createUser(data, context); + }, + degress: async (resource: any, context: RequestContext) => { + return await controller.deleteUser(resource, context); + } + }; +}; diff --git a/app/server/lib/scim/v2/ScimV2Api.ts b/app/server/lib/scim/v2/ScimV2Api.ts index df94f29fb9..00d896a5ad 100644 --- a/app/server/lib/scim/v2/ScimV2Api.ts +++ b/app/server/lib/scim/v2/ScimV2Api.ts @@ -1,100 +1,25 @@ import * as express from 'express'; -import { HomeDBManager, Scope } from 'app/gen-server/lib/homedb/HomeDBManager'; +import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; import SCIMMY from "scimmy"; import SCIMMYRouters from "scimmy-routers"; -import { RequestWithLogin } from '../../Authorizer'; -import { InstallAdmin } from '../../InstallAdmin'; -import { toSCIMMYUser, toUserProfile } from './ScimUserUtils'; -import { ApiError } from 'app/common/ApiError'; -import { parseInt } from 'lodash'; +import { RequestWithLogin } from 'app/server/lib/Authorizer'; +import { InstallAdmin } from 'app/server/lib/InstallAdmin'; +import { RequestContext } from './ScimTypes'; +import { getScimUserConfig } from './ScimUserController'; const WHITELISTED_PATHS_FOR_NON_ADMINS = [ "/Me", "/Schemas", "/ResourceTypes", "/ServiceProviderConfig" ]; -interface RequestContext { - path: string; - isAdmin: boolean; - isScimUser: boolean; -} - -function checkAccess(context: RequestContext) { - const {isAdmin, isScimUser, path } = context; - if (!isAdmin && !isScimUser && !WHITELISTED_PATHS_FOR_NON_ADMINS.includes(path)) { - throw new SCIMMY.Types.Error(403, null!, 'You are not authorized to access this resource'); - } -} - -async function checkEmailIsUnique(dbManager: HomeDBManager, email: string, id?: number) { - const existingUser = await dbManager.getExistingUserByLogin(email); - if (existingUser !== undefined && existingUser.id !== id) { - throw new SCIMMY.Types.Error(409, 'uniqueness', 'An existing user with the passed email exist.'); - } -} - const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) => { const v2 = express.Router(); - SCIMMY.Resources.declare(SCIMMY.Resources.User, { - egress: async (resource: any, context: RequestContext) => { - checkAccess(context); - - const { filter } = resource; - const id = parseInt(resource.id, 10); - if (!isNaN(id)) { - const user = await dbManager.getUser(id); - if (!user) { - throw new SCIMMY.Types.Error(404, null!, `User with ID ${id} not found`); - } - return toSCIMMYUser(user); - } - const scimmyUsers = (await dbManager.getUsers()).map(user => toSCIMMYUser(user)); - return filter ? filter.match(scimmyUsers) : scimmyUsers; - }, - ingress: async (resource: any, data: any, context: RequestContext) => { - checkAccess(context); - - try { - const id = parseInt(resource.id, 10); - if (!isNaN(id)) { - await checkEmailIsUnique(dbManager, data.userName, id); - const updatedUser = await dbManager.overrideUser(id, toUserProfile(data)); - return toSCIMMYUser(updatedUser); - } - await checkEmailIsUnique(dbManager, data.userName); - const userProfileToInsert = toUserProfile(data); - const newUser = await dbManager.getUserByLoginWithRetry(userProfileToInsert.email, { - profile: userProfileToInsert - }); - return toSCIMMYUser(newUser); - } catch (ex) { - if (ex instanceof ApiError) { - if (ex.status === 409) { - throw new SCIMMY.Types.Error(ex.status, 'uniqueness', ex.message); - } - throw new SCIMMY.Types.Error(ex.status, null!, ex.message); - } - - throw ex; - } - }, - degress: async (resource: any, context: RequestContext) => { - checkAccess(context); - - const id = parseInt(resource.id, 10); - if (isNaN(id)) { - throw new SCIMMY.Types.Error(400, null!, 'Invalid ID'); - } - const fakeScope: Scope = { userId: id }; // FIXME: deleteUser should probably better not requiring a scope. - try { - await dbManager.deleteUser(fakeScope, id); - } catch (ex) { - if (ex instanceof ApiError) { - throw new SCIMMY.Types.Error(ex.status, null!, ex.message); - } - - throw new SCIMMY.Types.Error(500, 'serverError', ex.message); - } + function checkAccess(context: RequestContext) { + const {isAdmin, isScimUser, path } = context; + if (!isAdmin && !isScimUser && !WHITELISTED_PATHS_FOR_NON_ADMINS.includes(path)) { + throw new SCIMMY.Types.Error(403, null!, 'You are not authorized to access this resource'); } - }); + } + + SCIMMY.Resources.declare(SCIMMY.Resources.User, getScimUserConfig(dbManager, checkAccess)); const scimmyRouter = new SCIMMYRouters({ type: 'bearer', diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index 6fb1a804e4..934e53afaf 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -416,6 +416,17 @@ describe('Scim', () => { }); }); + it('should return 400 when the user id is malformed', async function () { + const res = await axios.put(scimUrl('/Users/not-an-id'), toSCIMUserWithoutId('whoever'), chimpy); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '400', + detail: 'Invalid passed user ID', + scimType: 'invalidValue' + }); + assert.equal(res.status, 400); + }); + it('should normalize the passed email for the userName and keep the case for email.value', async function () { const newEmail = 'my-EMAIL@getgrist.com'; const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { From 44f2ee5e5c239b3723c52e93d888a95517ddbcb8 Mon Sep 17 00:00:00 2001 From: fflorent Date: Sat, 7 Sep 2024 11:00:10 +0200 Subject: [PATCH 10/27] An unknown error should return a 500 --- app/server/lib/scim/v2/ScimUserController.ts | 73 ++++++++++---------- test/server/lib/Scim.ts | 49 +++++++++---- 2 files changed, 72 insertions(+), 50 deletions(-) diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts index 2106d984b1..fc0f552260 100644 --- a/app/server/lib/scim/v2/ScimUserController.ts +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -19,73 +19,70 @@ class ScimUserController { ) {} public async getSingleUser(resource: any, context: RequestContext) { - this._checkAccess(context); - - const id = ScimUserController._getIdFromResource(resource); - const user = await this._dbManager.getUser(id); - if (!user) { - throw new SCIMMY.Types.Error(404, null!, `User with ID ${id} not found`); - } - return toSCIMMYUser(user); + return this._runAndHandleErrors(context, async () => { + const id = ScimUserController._getIdFromResource(resource); + const user = await this._dbManager.getUser(id); + if (!user) { + throw new SCIMMY.Types.Error(404, null!, `User with ID ${id} not found`); + } + return toSCIMMYUser(user); + }); } public async getUsers(resource: any, context: RequestContext) { - this._checkAccess(context); - - const { filter } = resource; - const scimmyUsers = (await this._dbManager.getUsers()).map(user => toSCIMMYUser(user)); - return filter ? filter.match(scimmyUsers) : scimmyUsers; + return this._runAndHandleErrors(context, async () => { + const { filter } = resource; + const scimmyUsers = (await this._dbManager.getUsers()).map(user => toSCIMMYUser(user)); + return filter ? filter.match(scimmyUsers) : scimmyUsers; + }); } public async createUser(data: any, context: RequestContext) { - this._checkAccess(context); - - try { + return this._runAndHandleErrors(context, async () => { await this._checkEmailIsUnique(data.userName); const userProfile = toUserProfile(data); const newUser = await this._dbManager.getUserByLoginWithRetry(userProfile.email, { profile: userProfile }); return toSCIMMYUser(newUser); - } catch (ex) { - return this._toScimError(ex); - } + }); } public async overrideUser(resource: any, data: any, context: RequestContext) { - this._checkAccess(context); - - try { + return this._runAndHandleErrors(context, async () => { const id = ScimUserController._getIdFromResource(resource); await this._checkEmailIsUnique(data.userName, id); const updatedUser = await this._dbManager.overrideUser(id, toUserProfile(data)); return toSCIMMYUser(updatedUser); - } catch (ex) { - return this._toScimError(ex); - } + }); } public async deleteUser(resource: any, context: RequestContext) { - this._checkAccess(context); - - const id = ScimUserController._getIdFromResource(resource); - try { + return this._runAndHandleErrors(context, async () => { + const id = ScimUserController._getIdFromResource(resource); const fakeScope: Scope = { userId: id }; // FIXME: deleteUser should probably better not requiring a scope. await this._dbManager.deleteUser(fakeScope, id); - } catch (ex) { - return this._toScimError(ex); - } + }); } - private async _toScimError(ex: Error) { - if (ex instanceof ApiError) { - if (ex.status === 409) { - throw new SCIMMY.Types.Error(ex.status, 'uniqueness', ex.message); + private async _runAndHandleErrors(context: RequestContext, cb: () => Promise): Promise { + try { + this._checkAccess(context); + return await cb(); + } catch (ex) { + if (ex instanceof ApiError) { + if (ex.status === 409) { + throw new SCIMMY.Types.Error(ex.status, 'uniqueness', ex.message); + } + throw new SCIMMY.Types.Error(ex.status, null!, ex.message); + } + if (ex instanceof SCIMMY.Types.Error) { + throw ex; } - throw new SCIMMY.Types.Error(ex.status, null!, ex.message); + // By default, return a 500 error + throw new SCIMMY.Types.Error(500, null!, ex.message); } - throw ex; } private async _checkEmailIsUnique(email: string, id?: number) { diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index 934e53afaf..e6ff2abc92 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -1,6 +1,8 @@ import axios from 'axios'; import capitalize from 'lodash/capitalize'; import { assert } from 'chai'; +import Sinon from 'sinon'; + import { TestServer } from 'test/gen-server/apiUtils'; import { configForUser } from 'test/gen-server/testUtils'; import * as testUtils from 'test/server/testUtils'; @@ -115,7 +117,7 @@ describe('Scim', () => { } } - function checkEndpointNotAccessibleForNonAdminUsers( + function checkCommonErrors( method: 'get' | 'post' | 'put' | 'patch' | 'delete', path: string, validBody: object = {} @@ -127,12 +129,12 @@ describe('Scim', () => { return axios[method](scimUrl(path), validBody, USER_CONFIG_BY_NAME[user]); } it('should return 401 for anonymous', async function () { - const res: any = await makeCallWith('anon'); + const res = await makeCallWith('anon'); assert.equal(res.status, 401); }); it('should return 403 for kiwi', async function () { - const res: any = await makeCallWith('kiwi'); + const res = await makeCallWith('kiwi'); assert.deepEqual(res.data, { schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], status: '403', @@ -140,6 +142,30 @@ describe('Scim', () => { }); assert.equal(res.status, 403); }); + + it('should return a 500 in case of unknown Error', async function () { + const sandbox = Sinon.createSandbox(); + try { + const error = new Error('Some unexpected Error'); + + // Stub all the dbManager methods called by the controller + sandbox.stub(getDbManager(), 'getUsers').throws(error); + sandbox.stub(getDbManager(), 'getUser').throws(error); + sandbox.stub(getDbManager(), 'getUserByLoginWithRetry').throws(error); + sandbox.stub(getDbManager(), 'overrideUser').throws(error); + sandbox.stub(getDbManager(), 'deleteUser').throws(error); + + const res = await makeCallWith('chimpy'); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '500', + detail: error.message + }); + assert.equal(res.status, 500); + } finally { + sandbox.restore(); + } + }); } describe('/Me', function () { @@ -181,10 +207,9 @@ describe('Scim', () => { preferredLanguage: 'en', }); }); - }); - describe('/Users/{id}', function () { + describe('GET /Users/{id}', function () { it('should return the user of id=1 for chimpy', async function () { const res = await axios.get(scimUrl('/Users/1'), chimpy); @@ -208,7 +233,7 @@ describe('Scim', () => { }); }); - checkEndpointNotAccessibleForNonAdminUsers('get', '/Users/1'); + checkCommonErrors('get', '/Users/1'); }); describe('GET /Users', function () { @@ -220,7 +245,7 @@ describe('Scim', () => { assert.deepInclude(res.data.Resources, personaToSCIMMYUserWithId('kiwi')); }); - checkEndpointNotAccessibleForNonAdminUsers('get', '/Users'); + checkCommonErrors('get', '/Users'); }); describe('POST /Users/.search', function () { @@ -261,7 +286,7 @@ describe('Scim', () => { "should have retrieved only chimpy's username and not other attribute"); }); - checkEndpointNotAccessibleForNonAdminUsers('post', '/Users/.search', searchExample); + checkCommonErrors('post', '/Users/.search', searchExample); }); describe('POST /Users', function () { // Create a new users @@ -331,7 +356,7 @@ describe('Scim', () => { assert.equal(res.status, 409); }); - checkEndpointNotAccessibleForNonAdminUsers('post', '/Users', toSCIMUserWithoutId('some-user')); + checkCommonErrors('post', '/Users', toSCIMUserWithoutId('some-user')); }); describe('PUT /Users/{id}', function () { @@ -443,7 +468,7 @@ describe('Scim', () => { }); }); - checkEndpointNotAccessibleForNonAdminUsers('put', '/Users/1', toSCIMUserWithoutId('chimpy')); + checkCommonErrors('put', '/Users/1', toSCIMUserWithoutId('chimpy')); }); describe('PATCH /Users/{id}', function () { @@ -483,7 +508,7 @@ describe('Scim', () => { }); }); - checkEndpointNotAccessibleForNonAdminUsers('patch', '/Users/1', validPatchBody('new name2')); + checkCommonErrors('patch', '/Users/1', validPatchBody('new name2')); }); describe('DELETE /Users/{id}', function () { @@ -513,7 +538,7 @@ describe('Scim', () => { }); assert.equal(res.status, 404); }); - checkEndpointNotAccessibleForNonAdminUsers('delete', '/Users/1'); + checkCommonErrors('delete', '/Users/1'); }); describe('POST /Bulk', function () { From 5215e4f153ea016cad185ccc6572438b06589660 Mon Sep 17 00:00:00 2001 From: fflorent Date: Sat, 7 Sep 2024 11:18:02 +0200 Subject: [PATCH 11/27] Add a test for pagination --- test/server/lib/Scim.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index e6ff2abc92..3a9c39662e 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -128,6 +128,7 @@ describe('Scim', () => { } return axios[method](scimUrl(path), validBody, USER_CONFIG_BY_NAME[user]); } + it('should return 401 for anonymous', async function () { const res = await makeCallWith('anon'); assert.equal(res.status, 401); @@ -245,6 +246,25 @@ describe('Scim', () => { assert.deepInclude(res.data.Resources, personaToSCIMMYUserWithId('kiwi')); }); + it('should handle pagination', async function () { + const endpointPaginated = '/Users?count=1&sortBy=id'; + { + const firstPage = await axios.get(scimUrl(endpointPaginated), chimpy); + assert.equal(firstPage.status, 200); + assert.lengthOf(firstPage.data.Resources, 1); + const firstPageResourceId = parseInt(firstPage.data.Resources[0].id); + assert.equal(firstPageResourceId, 1); + } + + { + const secondPage = await axios.get(scimUrl(endpointPaginated + '&startIndex=2'), chimpy); + assert.equal(secondPage.status, 200); + assert.lengthOf(secondPage.data.Resources, 1); + const secondPageResourceId = parseInt(secondPage.data.Resources[0].id); + assert.equal(secondPageResourceId, 2); + } + }); + checkCommonErrors('get', '/Users'); }); From b0fe85ebad03db172390bd1cc9cfb005b448cdfd Mon Sep 17 00:00:00 2001 From: fflorent Date: Sat, 7 Sep 2024 11:59:12 +0200 Subject: [PATCH 12/27] Add tests for the new UsersManager methods --- app/gen-server/lib/homedb/UsersManager.ts | 9 ++-- test/gen-server/lib/homedb/UsersManager.ts | 63 ++++++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/app/gen-server/lib/homedb/UsersManager.ts b/app/gen-server/lib/homedb/UsersManager.ts index 3652049298..9e24a178bb 100644 --- a/app/gen-server/lib/homedb/UsersManager.ts +++ b/app/gen-server/lib/homedb/UsersManager.ts @@ -603,6 +603,11 @@ export class UsersManager { }); } + public async getUsers() { + return await User.find({relations: ["logins"]}); + } + + /** * ================================== * @@ -706,10 +711,6 @@ export class UsersManager { return [this.getSupportUserId(), this.getAnonymousUserId(), this.getEveryoneUserId()]; } - public async getUsers() { - return await User.find({relations: ["logins"]}); - } - /** * Returns a Promise for an array of User entites for the given userIds. */ diff --git a/test/gen-server/lib/homedb/UsersManager.ts b/test/gen-server/lib/homedb/UsersManager.ts index 5b304de1ac..f3309584c0 100644 --- a/test/gen-server/lib/homedb/UsersManager.ts +++ b/test/gen-server/lib/homedb/UsersManager.ts @@ -20,6 +20,7 @@ import { assert } from 'chai'; import Sinon, { SinonSandbox, SinonSpy } from 'sinon'; import { EntityManager } from 'typeorm'; import winston from 'winston'; +import omit from 'lodash/omit'; import {delay} from 'app/common/delay'; @@ -982,6 +983,68 @@ describe('UsersManager', function () { } }); }); + + describe('overrideUser()', function () { + it('should reject when user is not found', async function () { + disableLoggingLevel('debug'); + + const promise = db.overrideUser(NON_EXISTING_USER_ID, { + email: 'whatever@getgrist.com', + name: 'whatever', + }); + + await assert.isRejected(promise, 'unable to find user to update'); + }); + + it('should update user information', async function () { + const localPart = 'overrideuser-updates-user-info'; + const newLocalPart = 'overrideuser-updates-user-info-new'; + const user = await createUniqueUser(localPart); + const newInfo: UserProfile = { + name: 'new name', + email: makeEmail(newLocalPart).toUpperCase(), + picture: 'https://mypic.com/me.png', + locale: 'fr-FR', + }; + + await db.overrideUser(user.id, newInfo); + + const updatedUser = await getOrCreateUser(newLocalPart); + assert.deepInclude(updatedUser, { + id: user.id, + name: newInfo.name, + picture: newInfo.picture, + options: {locale: newInfo.locale}, + }); + assert.deepInclude(updatedUser.logins[0], { + email: newInfo.email.toLowerCase(), + displayEmail: newInfo.email, + }); + }); + }); + + describe('getUsers()', function () { + it('should return all users with their logins', async function () { + const localPart = 'getUsers-user'; + const existingUser = await createUniqueUser(localPart); + const users = await db.getUsers(); + assert.isAbove(users.length, 2); + const mapUsersById = new Map(users.map(user => [user.id, user])); + + // Check that we retrieve the existing user in the result with all their property + // except the personalOrg + const existingUserInResult = mapUsersById.get(existingUser.id); + assertExists(existingUserInResult); + assertExists(existingUserInResult.logins); + assert.lengthOf(existingUserInResult.logins, 1); + assert.deepEqual(existingUserInResult, omit(existingUser, 'personalOrg')); + + // Check that we retrieve special accounts among the result + assert.exists(mapUsersById.get(db.getSupportUserId())); + assert.exists(mapUsersById.get(db.getEveryoneUserId())); + assert.exists(mapUsersById.get(db.getAnonymousUserId())); + }); + }); }); describe('class method without db setup', function () { From 3041200732089e8ddefc9cff7dc9f5edb411b66a Mon Sep 17 00:00:00 2001 From: fflorent Date: Sat, 7 Sep 2024 15:02:57 +0200 Subject: [PATCH 13/27] Document methods of the controller --- app/server/lib/scim/v2/ScimUserController.ts | 54 ++++++++++++++++++-- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts index fc0f552260..fe1ef02f6c 100644 --- a/app/server/lib/scim/v2/ScimUserController.ts +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -18,6 +18,12 @@ class ScimUserController { private _checkAccess: (context: RequestContext) => void ) {} + /** + * Gets a single user with the passed ID. + * + * @param resource The SCIMMY user resource performing the operation + * @param context The request context + */ public async getSingleUser(resource: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { const id = ScimUserController._getIdFromResource(resource); @@ -29,6 +35,12 @@ class ScimUserController { }); } + /** + * Gets all users or filters them based on the passed filter. + * + * @param resource The SCIMMY user resource performing the operation + * @param context The request context + */ public async getUsers(resource: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { const { filter } = resource; @@ -37,9 +49,15 @@ class ScimUserController { }); } + /** + * Creates a new user with the passed data. + * + * @param data The data to create the user with + * @param context The request context + */ public async createUser(data: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { - await this._checkEmailIsUnique(data.userName); + await this._checkEmailCanBeUsed(data.userName); const userProfile = toUserProfile(data); const newUser = await this._dbManager.getUserByLoginWithRetry(userProfile.email, { profile: userProfile @@ -48,15 +66,28 @@ class ScimUserController { }); } + /** + * Overrides a user with the passed data. + * + * @param resource The SCIMMY user resource performing the operation + * @param data The data to override the user with + * @param context The request context + */ public async overrideUser(resource: any, data: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { const id = ScimUserController._getIdFromResource(resource); - await this._checkEmailIsUnique(data.userName, id); + await this._checkEmailCanBeUsed(data.userName, id); const updatedUser = await this._dbManager.overrideUser(id, toUserProfile(data)); return toSCIMMYUser(updatedUser); }); } + /** + * Deletes a user with the passed ID. + * + * @param resource The SCIMMY user resource performing the operation + * @param context The request context + */ public async deleteUser(resource: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { const id = ScimUserController._getIdFromResource(resource); @@ -66,6 +97,14 @@ class ScimUserController { }); } + /** + * Runs the passed callback and handles any errors that might occur. + * Also checks if the user has access to the operation. + * Any public method of this class should be run through this method. + * + * @param context The request context to check access for the user + * @param cb The callback to run + */ private async _runAndHandleErrors(context: RequestContext, cb: () => Promise): Promise { try { this._checkAccess(context); @@ -85,9 +124,16 @@ class ScimUserController { } } - private async _checkEmailIsUnique(email: string, id?: number) { + /** + * Checks if the passed email can be used for a new user or by the existing user. + * + * @param email The email to check + * @param userIdToUpdate The ID of the user to update. Pass this when updating a user, + * so it won't raise an error if the passed email is already used by this user. + */ + private async _checkEmailCanBeUsed(email: string, userIdToUpdate?: number) { const existingUser = await this._dbManager.getExistingUserByLogin(email); - if (existingUser !== undefined && existingUser.id !== id) { + if (existingUser !== undefined && existingUser.id !== userIdToUpdate) { throw new SCIMMY.Types.Error(409, 'uniqueness', 'An existing user with the passed email exist.'); } } From efa742d92996b12ac4b191525a07c977daae473a Mon Sep 17 00:00:00 2001 From: fflorent Date: Thu, 12 Sep 2024 16:00:05 +0200 Subject: [PATCH 14/27] =?UTF-8?q?Rename=20ex=20=E2=86=92=20err=20in=20catc?= =?UTF-8?q?h=20block?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/server/lib/scim/v2/ScimUserController.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts index fe1ef02f6c..4aec86d6b3 100644 --- a/app/server/lib/scim/v2/ScimUserController.ts +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -109,18 +109,18 @@ class ScimUserController { try { this._checkAccess(context); return await cb(); - } catch (ex) { - if (ex instanceof ApiError) { - if (ex.status === 409) { - throw new SCIMMY.Types.Error(ex.status, 'uniqueness', ex.message); + } catch (err) { + if (err instanceof ApiError) { + if (err.status === 409) { + throw new SCIMMY.Types.Error(err.status, 'uniqueness', err.message); } - throw new SCIMMY.Types.Error(ex.status, null!, ex.message); + throw new SCIMMY.Types.Error(err.status, null!, err.message); } - if (ex instanceof SCIMMY.Types.Error) { - throw ex; + if (err instanceof SCIMMY.Types.Error) { + throw err; } // By default, return a 500 error - throw new SCIMMY.Types.Error(500, null!, ex.message); + throw new SCIMMY.Types.Error(500, null!, err.message); } } From dc8478ea5c2f6bf0236913a03dcec888e0780dbb Mon Sep 17 00:00:00 2001 From: fflorent Date: Fri, 18 Oct 2024 14:42:19 +0200 Subject: [PATCH 15/27] Bump Scimmy and Scimmy-Routers --- package.json | 4 ++-- yarn.lock | 21 +++++++++------------ 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/package.json b/package.json index 4c80c1efdf..ded42ff2c1 100644 --- a/package.json +++ b/package.json @@ -189,8 +189,8 @@ "redis": "3.1.1", "redlock": "3.1.2", "saml2-js": "4.0.2", - "scimmy": "1.2.3", - "scimmy-routers": "1.2.1", + "scimmy": "1.2.4", + "scimmy-routers": "1.2.2", "short-uuid": "3.1.1", "slugify": "1.6.6", "swagger-ui-dist": "5.11.0", diff --git a/yarn.lock b/yarn.lock index 16b6db082d..993d4303be 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3799,7 +3799,7 @@ express-rate-limit@7.2.0: resolved "https://registry.yarnpkg.com/express-rate-limit/-/express-rate-limit-7.2.0.tgz#06ce387dd5388f429cab8263c514fc07bf90a445" integrity sha512-T7nul1t4TNyfZMJ7pKRKkdeVJWa2CqB8NA1P8BwYaoDI5QSBZARv5oMS43J7b7I5P+4asjVXjb7ONuwDKucahg== -express@4.19.2, express@^4.19.2: +express@4.19.2: version "4.19.2" resolved "https://registry.yarnpkg.com/express/-/express-4.19.2.tgz#e25437827a3aa7f2a827bc8171bbbb664a356465" integrity sha512-5T6nhjsT+EOMzuck8JjBHARTHfMht0POzlA60WV2pMD3gyXw2LZnZ+ueGdNxG+0calOJcWKbpFcuzLZ91YWq9Q== @@ -7344,18 +7344,15 @@ schema-utils@^3.2.0: ajv "^6.12.5" ajv-keywords "^3.5.2" -scimmy-routers@1.2.1: - version "1.2.1" - resolved "https://registry.yarnpkg.com/scimmy-routers/-/scimmy-routers-1.2.1.tgz#a535ced83f051b2a0374e8df02da6e17424e8be0" - integrity sha512-PDX4/mwOScSd+TjUPX0k3gH6v50WeYVgK1bisZ8f3p3eyJ0Qy4qYebDo6gzHqYCBjXNQniQxGSQvtlvG22NLdA== - dependencies: - express "^4.19.2" - scimmy "^1.2.3" +scimmy-routers@1.2.2: + version "1.2.2" + resolved "https://registry.yarnpkg.com/scimmy-routers/-/scimmy-routers-1.2.2.tgz#e1fa506a8cdb0ba04a25e09a365bd726cd781585" + integrity sha512-qDB7DKb2cnujJzEgVdON8EnjZfs6oY+MJQkkCHbihNrQeRjSaEOAC9ohb6dGfMZdahYS0CZIJwGhvZlS6rkKsg== -scimmy@1.2.3, scimmy@^1.2.3: - version "1.2.3" - resolved "https://registry.yarnpkg.com/scimmy/-/scimmy-1.2.3.tgz#56ca9dbf11749b272e18090c923f81dbad4bc911" - integrity sha512-16oXCvnieVeKxTDQqi275bLuyOCwXci8Jywm2/M+4dWNNYoduUz0Crj1nFY0ETYMsuYvCdareWov6/Mebu92xA== +scimmy@1.2.4: + version "1.2.4" + resolved "https://registry.yarnpkg.com/scimmy/-/scimmy-1.2.4.tgz#3d708d9a5f3c7b3e00d848dcb8f0910d7c409509" + integrity sha512-5i+LwGL7ON61jH+KxL6flpy5h/ABhgx7tc9AdL3KMh9TfHidWl7KHrbD0cJN5bJ5Fb1nOTze8d+PbFl2bZYEJQ== selenium-webdriver@^4.20.0: version "4.20.0" From 61e8cd96295313781d8734f3ad2efdf0a6e10692 Mon Sep 17 00:00:00 2001 From: fflorent Date: Fri, 22 Nov 2024 18:48:13 +0100 Subject: [PATCH 16/27] Log errors --- app/server/lib/scim/v2/ScimUserController.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts index 4aec86d6b3..48d2d4601b 100644 --- a/app/server/lib/scim/v2/ScimUserController.ts +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -3,6 +3,7 @@ import { HomeDBManager, Scope } from 'app/gen-server/lib/homedb/HomeDBManager'; import SCIMMY from 'scimmy'; import { toSCIMMYUser, toUserProfile } from './ScimUserUtils'; import { RequestContext } from './ScimTypes'; +import log from 'app/server/lib/log'; class ScimUserController { private static _getIdFromResource(resource: any) { @@ -111,15 +112,18 @@ class ScimUserController { return await cb(); } catch (err) { if (err instanceof ApiError) { + log.error('[ScimUserController] ApiError: ', err.status, err.message); if (err.status === 409) { throw new SCIMMY.Types.Error(err.status, 'uniqueness', err.message); } throw new SCIMMY.Types.Error(err.status, null!, err.message); } if (err instanceof SCIMMY.Types.Error) { + log.error('[ScimUserController] SCIMMY.Types.Error: ', err.message); throw err; } // By default, return a 500 error + log.error('[ScimUserController] Error: ', err.message); throw new SCIMMY.Types.Error(500, null!, err.message); } } From ebf81a0ebfd0fc5619f56c39c2899a014d255150 Mon Sep 17 00:00:00 2001 From: fflorent Date: Mon, 25 Nov 2024 14:35:46 +0100 Subject: [PATCH 17/27] Only warn when the userName differ from the primary email --- app/server/lib/scim/v2/ScimUserUtils.ts | 4 +- test/server/lib/Scim.ts | 55 +++++++++++++++++-------- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/app/server/lib/scim/v2/ScimUserUtils.ts b/app/server/lib/scim/v2/ScimUserUtils.ts index 0ddaffe9e1..8c51eada3f 100644 --- a/app/server/lib/scim/v2/ScimUserUtils.ts +++ b/app/server/lib/scim/v2/ScimUserUtils.ts @@ -2,6 +2,7 @@ import { normalizeEmail } from "app/common/emails"; import { UserProfile } from "app/common/LoginSessionAPI"; import { User } from "app/gen-server/entity/User.js"; import SCIMMY from "scimmy"; +import log from 'app/server/lib/log'; /** * Converts a user from your database to a SCIMMY user @@ -35,7 +36,8 @@ export function toSCIMMYUser(user: User) { export function toUserProfile(scimUser: any, existingUser?: User): UserProfile { const emailValue = scimUser.emails?.[0]?.value; if (emailValue && normalizeEmail(emailValue) !== normalizeEmail(scimUser.userName)) { - throw new SCIMMY.Types.Error(400, 'invalidValue', 'Email and userName must be the same'); + log.warn(`userName "${scimUser.userName}" differ from passed primary email "${emailValue}".` + + 'That should be OK, but be aware that the userName will be ignored in favor of the email to identify the user.'); } return { name: scimUser.displayName ?? existingUser?.name, diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index 3a9c39662e..3cd3dbb222 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -2,6 +2,7 @@ import axios from 'axios'; import capitalize from 'lodash/capitalize'; import { assert } from 'chai'; import Sinon from 'sinon'; +import log from 'app/server/lib/log'; import { TestServer } from 'test/gen-server/apiUtils'; import { configForUser } from 'test/gen-server/testUtils'; @@ -75,6 +76,8 @@ describe('Scim', () => { GRIST_SCIM_EMAIL: 'charon@getgrist.com', }); const userIdByName: {[name in keyof UserConfigByName]?: number} = {}; + let logWarnStub: Sinon.SinonStub; + let logErrorStub: Sinon.SinonStub; before(async function () { const userNames = Object.keys(USER_CONFIG_BY_NAME) as Array; @@ -83,6 +86,16 @@ describe('Scim', () => { } }); + beforeEach(() => { + logWarnStub = Sinon.stub(log, 'warn'); + logErrorStub = Sinon.stub(log, 'error'); + }); + + afterEach(() => { + logWarnStub.restore(); + logErrorStub.restore(); + }); + function personaToSCIMMYUserWithId(user: keyof UserConfigByName) { return toSCIMUserWithId(user, userIdByName[user]!); } @@ -348,20 +361,32 @@ describe('Scim', () => { }); }); - it('should reject when passed email differs from username', async function () { + it('should warn when passed email differs from username, and ignore the username', async function () { await withUserName('username', async (userName) => { const res = await axios.post(scimUrl('/Users'), { schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - userName: userName + '@getgrist.com', + userName: userName, emails: [{ value: 'emails.value@getgrist.com' }], }, chimpy); assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '400', - detail: 'Email and userName must be the same', - scimType: 'invalidValue' + schemas: [ 'urn:ietf:params:scim:schemas:core:2.0:User' ], + id: '12', + meta: { resourceType: 'User', location: '/api/scim/v2/Users/12' }, + userName: 'emails.value@getgrist.com', + name: { formatted: 'emails.value' }, + displayName: 'emails.value', + preferredLanguage: 'en', + locale: 'en', + emails: [ + { value: 'emails.value@getgrist.com', primary: true } + ] }); - assert.equal(res.status, 400); + assert.equal(res.status, 201); + assert.equal(logWarnStub.callCount, 1, "A warning should have been raised"); + assert.match( + logWarnStub.getCalls()[0].args[0], + new RegExp(`userName "${userName}" differ from passed primary email`) + ); }); }); @@ -411,19 +436,15 @@ describe('Scim', () => { }); }); - it('should reject when passed email differs from username', async function () { + it('should warn when passed email differs from username', async function () { const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], - userName: userToUpdateEmailLocalPart + '@getgrist.com', - emails: [{ value: 'whatever@getgrist.com', primary: true }], + userName: 'whatever@getgrist.com', + emails: [{ value: userToUpdateEmailLocalPart + '@getgrist.com', primary: true }], }, chimpy); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '400', - detail: 'Email and userName must be the same', - scimType: 'invalidValue' - }); - assert.equal(res.status, 400); + assert.equal(res.status, 200); + assert.equal(logWarnStub.callCount, 1, "A warning should have been raised"); + assert.match(logWarnStub.getCalls()[0].args[0], /differ from passed primary email/); }); it('should disallow updating a user with the same email as another user\'s', async function () { From 959f8b5b6346be1be532977af2019fd730a9183c Mon Sep 17 00:00:00 2001 From: fflorent Date: Tue, 26 Nov 2024 17:50:57 +0100 Subject: [PATCH 18/27] =?UTF-8?q?Rename=20overrideUser=20=E2=86=92=20overw?= =?UTF-8?q?riteUser?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/gen-server/lib/homedb/UsersManager.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/gen-server/lib/homedb/UsersManager.ts b/test/gen-server/lib/homedb/UsersManager.ts index f3309584c0..56f7d992be 100644 --- a/test/gen-server/lib/homedb/UsersManager.ts +++ b/test/gen-server/lib/homedb/UsersManager.ts @@ -984,11 +984,11 @@ describe('UsersManager', function () { }); }); - describe('overrideUser()', function () { + describe('overwriteUser()', function () { it('should reject when user is not found', async function () { disableLoggingLevel('debug'); - const promise = db.overrideUser(NON_EXISTING_USER_ID, { + const promise = db.overwriteUser(NON_EXISTING_USER_ID, { email: 'whatever@getgrist.com', name: 'whatever', }); @@ -997,8 +997,8 @@ describe('UsersManager', function () { }); it('should update user information', async function () { - const localPart = 'overrideuser-updates-user-info'; - const newLocalPart = 'overrideuser-updates-user-info-new'; + const localPart = 'overwriteUser-updates-user-info'; + const newLocalPart = 'overwriteUser-updates-user-info-new'; const user = await createUniqueUser(localPart); const newInfo: UserProfile = { name: 'new name', @@ -1007,7 +1007,7 @@ describe('UsersManager', function () { locale: 'fr-FR', }; - await db.overrideUser(user.id, newInfo); + await db.overwriteUser(user.id, newInfo); const updatedUser = await getOrCreateUser(newLocalPart); assert.deepInclude(updatedUser, { From bc3775179c3c915dbc9d258aa146775c5b16f38e Mon Sep 17 00:00:00 2001 From: fflorent Date: Tue, 26 Nov 2024 18:10:21 +0100 Subject: [PATCH 19/27] Use full path for import --- app/gen-server/lib/homedb/HomeDBManager.ts | 4 ++-- app/gen-server/lib/homedb/UsersManager.ts | 2 +- app/server/lib/FlexServer.ts | 2 +- app/server/lib/scim/index.ts | 4 ++-- app/server/lib/scim/v2/ScimUserController.ts | 10 +++++----- app/server/lib/scim/v2/ScimV2Api.ts | 4 ++-- test/server/lib/Scim.ts | 2 +- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app/gen-server/lib/homedb/HomeDBManager.ts b/app/gen-server/lib/homedb/HomeDBManager.ts index 75b77356ab..7f06e14e4b 100644 --- a/app/gen-server/lib/homedb/HomeDBManager.ts +++ b/app/gen-server/lib/homedb/HomeDBManager.ts @@ -563,8 +563,8 @@ export class HomeDBManager extends EventEmitter { return this._usersManager.deleteUser(scope, userIdToDelete, name); } - public async overrideUser(userId: number, props: UserProfile) { - return this._usersManager.overrideUser(userId, props); + public async overwriteUser(userId: number, props: UserProfile) { + return this._usersManager.overwriteUser(userId, props); } /** diff --git a/app/gen-server/lib/homedb/UsersManager.ts b/app/gen-server/lib/homedb/UsersManager.ts index 9e24a178bb..0a5a00d898 100644 --- a/app/gen-server/lib/homedb/UsersManager.ts +++ b/app/gen-server/lib/homedb/UsersManager.ts @@ -585,7 +585,7 @@ export class UsersManager { /** * Update users with passed property. Optional user properties that are missing will be reset to their default value. */ - public async overrideUser(userId: number, props: UserProfile): Promise { + public async overwriteUser(userId: number, props: UserProfile): Promise { return await this._connection.transaction(async manager => { const user = await this.getUser(userId, {includePrefs: true}); if (!user) { throw new ApiError("unable to find user to update", 404); } diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 524b60b156..c1b8e78b58 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -88,7 +88,7 @@ import * as path from 'path'; import * as serveStatic from "serve-static"; import {ConfigBackendAPI} from "app/server/lib/ConfigBackendAPI"; import {IGristCoreConfig} from "app/server/lib/configCore"; -import { buildScimRouter } from './scim'; +import { buildScimRouter } from 'app/server/lib/scim'; // Health checks are a little noisy in the logs, so we don't show them all. // We show the first N health checks: diff --git a/app/server/lib/scim/index.ts b/app/server/lib/scim/index.ts index 2ee49303f4..cd8ea963f9 100644 --- a/app/server/lib/scim/index.ts +++ b/app/server/lib/scim/index.ts @@ -1,8 +1,8 @@ import * as express from 'express'; -import { buildScimRouterv2 } from './v2/ScimV2Api'; +import { buildScimRouterv2 } from 'app/server/lib/scim/v2/ScimV2Api'; import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager'; -import { InstallAdmin } from '../InstallAdmin'; +import { InstallAdmin } from 'app/server/lib/InstallAdmin'; const buildScimRouter = (dbManager: HomeDBManager, installAdmin: InstallAdmin) => { const v2 = buildScimRouterv2(dbManager, installAdmin); diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts index 48d2d4601b..6276343f3a 100644 --- a/app/server/lib/scim/v2/ScimUserController.ts +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -1,8 +1,8 @@ import { ApiError } from 'app/common/ApiError'; import { HomeDBManager, Scope } from 'app/gen-server/lib/homedb/HomeDBManager'; import SCIMMY from 'scimmy'; -import { toSCIMMYUser, toUserProfile } from './ScimUserUtils'; -import { RequestContext } from './ScimTypes'; +import { toSCIMMYUser, toUserProfile } from 'app/server/lib/scim/v2/ScimUserUtils'; +import { RequestContext } from 'app/server/lib/scim/v2/ScimTypes'; import log from 'app/server/lib/log'; class ScimUserController { @@ -74,11 +74,11 @@ class ScimUserController { * @param data The data to override the user with * @param context The request context */ - public async overrideUser(resource: any, data: any, context: RequestContext) { + public async overwriteUser(resource: any, data: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { const id = ScimUserController._getIdFromResource(resource); await this._checkEmailCanBeUsed(data.userName, id); - const updatedUser = await this._dbManager.overrideUser(id, toUserProfile(data)); + const updatedUser = await this._dbManager.overwriteUser(id, toUserProfile(data)); return toSCIMMYUser(updatedUser); }); } @@ -157,7 +157,7 @@ export const getScimUserConfig = ( }, ingress: async (resource: any, data: any, context: RequestContext) => { if (resource.id) { - return await controller.overrideUser(resource, data, context); + return await controller.overwriteUser(resource, data, context); } return await controller.createUser(data, context); }, diff --git a/app/server/lib/scim/v2/ScimV2Api.ts b/app/server/lib/scim/v2/ScimV2Api.ts index 00d896a5ad..52648ea073 100644 --- a/app/server/lib/scim/v2/ScimV2Api.ts +++ b/app/server/lib/scim/v2/ScimV2Api.ts @@ -4,8 +4,8 @@ import SCIMMY from "scimmy"; import SCIMMYRouters from "scimmy-routers"; import { RequestWithLogin } from 'app/server/lib/Authorizer'; import { InstallAdmin } from 'app/server/lib/InstallAdmin'; -import { RequestContext } from './ScimTypes'; -import { getScimUserConfig } from './ScimUserController'; +import { RequestContext } from 'app/server/lib/scim/v2/ScimTypes'; +import { getScimUserConfig } from 'app/server/lib/scim/v2/ScimUserController'; const WHITELISTED_PATHS_FOR_NON_ADMINS = [ "/Me", "/Schemas", "/ResourceTypes", "/ServiceProviderConfig" ]; diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index 3cd3dbb222..8eb4da2559 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -166,7 +166,7 @@ describe('Scim', () => { sandbox.stub(getDbManager(), 'getUsers').throws(error); sandbox.stub(getDbManager(), 'getUser').throws(error); sandbox.stub(getDbManager(), 'getUserByLoginWithRetry').throws(error); - sandbox.stub(getDbManager(), 'overrideUser').throws(error); + sandbox.stub(getDbManager(), 'overwriteUser').throws(error); sandbox.stub(getDbManager(), 'deleteUser').throws(error); const res = await makeCallWith('chimpy'); From 72cfdf07cf514b21229696900918debb0f9d2bae Mon Sep 17 00:00:00 2001 From: Florent Date: Tue, 26 Nov 2024 18:26:22 +0100 Subject: [PATCH 20/27] Improve user deletion test description Co-authored-by: jordigh --- test/server/lib/Scim.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index 8eb4da2559..2acd52fa82 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -563,7 +563,7 @@ describe('Scim', () => { await cleanupUser(userToDeleteId); }); - it('should delete some user', async function () { + it('should delete a user', async function () { const res = await axios.delete(scimUrl(`/Users/${userToDeleteId}`), chimpy); assert.equal(res.status, 204); const refreshedUser = await axios.get(scimUrl(`/Users/${userToDeleteId}`), chimpy); From d252f5be513411480a46a2c7398f38e3247ab1cd Mon Sep 17 00:00:00 2001 From: Florent Date: Tue, 26 Nov 2024 18:35:35 +0100 Subject: [PATCH 21/27] Improve error message for anonymous users Co-authored-by: jordigh --- app/server/lib/scim/v2/ScimV2Api.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/lib/scim/v2/ScimV2Api.ts b/app/server/lib/scim/v2/ScimV2Api.ts index 52648ea073..6c19349c51 100644 --- a/app/server/lib/scim/v2/ScimV2Api.ts +++ b/app/server/lib/scim/v2/ScimV2Api.ts @@ -31,7 +31,7 @@ const buildScimRouterv2 = (dbManager: HomeDBManager, installAdmin: InstallAdmin) } if (mreq.userId === dbManager.getAnonymousUserId()) { - throw new Error('Anonymous user cannot access SCIM resources'); + throw new Error('Anonymous users cannot access SCIM resources'); } return String(mreq.userId); // SCIMMYRouters requires the userId to be a string. From c4f6712ef63eca0253eba45f29c916d8b12c543a Mon Sep 17 00:00:00 2001 From: fflorent Date: Tue, 26 Nov 2024 22:14:28 +0100 Subject: [PATCH 22/27] Fix styling issue --- app/server/lib/FlexServer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index c1b8e78b58..55d0f949b7 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -88,7 +88,7 @@ import * as path from 'path'; import * as serveStatic from "serve-static"; import {ConfigBackendAPI} from "app/server/lib/ConfigBackendAPI"; import {IGristCoreConfig} from "app/server/lib/configCore"; -import { buildScimRouter } from 'app/server/lib/scim'; +import {buildScimRouter} from 'app/server/lib/scim'; // Health checks are a little noisy in the logs, so we don't show them all. // We show the first N health checks: From 090398c8285cbd218517b40d73020f799821c9ee Mon Sep 17 00:00:00 2001 From: fflorent Date: Fri, 29 Nov 2024 12:28:09 +0100 Subject: [PATCH 23/27] Disallow deleting technical users --- app/gen-server/lib/homedb/HomeDBManager.ts | 4 ++++ app/gen-server/lib/homedb/UsersManager.ts | 7 +++++++ app/server/lib/scim/v2/ScimUserController.ts | 3 +++ test/gen-server/lib/homedb/UsersManager.ts | 11 ++++++++++- test/server/lib/Scim.ts | 20 ++++++++++++++++++++ 5 files changed, 44 insertions(+), 1 deletion(-) diff --git a/app/gen-server/lib/homedb/HomeDBManager.ts b/app/gen-server/lib/homedb/HomeDBManager.ts index 7f06e14e4b..721b18633f 100644 --- a/app/gen-server/lib/homedb/HomeDBManager.ts +++ b/app/gen-server/lib/homedb/HomeDBManager.ts @@ -2618,6 +2618,10 @@ export class HomeDBManager extends EventEmitter { return this._usersManager.getAnonymousUser(); } + public getSpecialUserIds() { + return this._usersManager.getSpecialUserIds(); + } + public getAnonymousUserId() { return this._usersManager.getAnonymousUserId(); } diff --git a/app/gen-server/lib/homedb/UsersManager.ts b/app/gen-server/lib/homedb/UsersManager.ts index 0a5a00d898..6a0f431c84 100644 --- a/app/gen-server/lib/homedb/UsersManager.ts +++ b/app/gen-server/lib/homedb/UsersManager.ts @@ -109,6 +109,13 @@ export class UsersManager { return this._specialUserIds[key]; } + /** + * Return the special user ids. + */ + public getSpecialUserIds() { + return Object.values(this._specialUserIds); + } + /** * * Get the id of the anonymous user. diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts index 6276343f3a..16a261c1f8 100644 --- a/app/server/lib/scim/v2/ScimUserController.ts +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -92,6 +92,9 @@ class ScimUserController { public async deleteUser(resource: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { const id = ScimUserController._getIdFromResource(resource); + if (this._dbManager.getSpecialUserIds().includes(id)) { + throw new SCIMMY.Types.Error(403, null!, 'Cannot delete technical user'); + } const fakeScope: Scope = { userId: id }; // FIXME: deleteUser should probably better not requiring a scope. await this._dbManager.deleteUser(fakeScope, id); diff --git a/test/gen-server/lib/homedb/UsersManager.ts b/test/gen-server/lib/homedb/UsersManager.ts index 56f7d992be..e8aedc6ccf 100644 --- a/test/gen-server/lib/homedb/UsersManager.ts +++ b/test/gen-server/lib/homedb/UsersManager.ts @@ -22,7 +22,7 @@ import { EntityManager } from 'typeorm'; import winston from 'winston'; import omit from 'lodash/omit'; -import {delay} from 'app/common/delay'; +import { delay } from 'app/common/delay'; describe('UsersManager', function () { this.timeout('3m'); @@ -293,6 +293,15 @@ describe('UsersManager', function () { it("getSupportUserId() should retrieve 'support' user id", function () { assert.strictEqual(db.getSupportUserId(), SUPPORT_USER_ID); }); + + it("getSpecialUserIds() should retrieve all the special user ids", function () { + assert.deepEqual(db.getSpecialUserIds(), [ + ANONYMOUS_USER_ID, + PREVIEWER_USER_ID, + EVERYONE_USER_ID, + SUPPORT_USER_ID + ]); + }); }); describe('getUserByKey()', function () { diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index 2acd52fa82..14ef7009ac 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -579,6 +579,26 @@ describe('Scim', () => { }); assert.equal(res.status, 404); }); + + it('should return 404 for technical users', async function () { + const db = getDbManager(); + const specialUsers = { + 'anonymous': db.getAnonymousUserId(), + 'support': db.getSupportUserId(), + 'everyone': db.getEveryoneUserId(), + 'preview': db.getPreviewerUserId(), + }; + for (const [label, id] of Object.entries(specialUsers)) { + const res = await axios.delete(scimUrl(`/Users/${id}`), chimpy); + assert.equal(res.status, 403, 'should forbid deletion of the special user ' + label); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '403', + detail: 'Cannot delete technical user' + }); + } + }); + checkCommonErrors('delete', '/Users/1'); }); From 8312aa96e747d3e6ef21ca0bc297796e6ebd9f46 Mon Sep 17 00:00:00 2001 From: fflorent Date: Fri, 29 Nov 2024 12:49:19 +0100 Subject: [PATCH 24/27] Disallow technical user modifications --- app/server/lib/scim/v2/ScimUserController.ts | 5 +- test/server/lib/Scim.ts | 54 +++++++++++++------- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts index 16a261c1f8..6846415b69 100644 --- a/app/server/lib/scim/v2/ScimUserController.ts +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -77,6 +77,9 @@ class ScimUserController { public async overwriteUser(resource: any, data: any, context: RequestContext) { return this._runAndHandleErrors(context, async () => { const id = ScimUserController._getIdFromResource(resource); + if (this._dbManager.getSpecialUserIds().includes(id)) { + throw new SCIMMY.Types.Error(403, null!, 'Technical user modification not permitted.'); + } await this._checkEmailCanBeUsed(data.userName, id); const updatedUser = await this._dbManager.overwriteUser(id, toUserProfile(data)); return toSCIMMYUser(updatedUser); @@ -93,7 +96,7 @@ class ScimUserController { return this._runAndHandleErrors(context, async () => { const id = ScimUserController._getIdFromResource(resource); if (this._dbManager.getSpecialUserIds().includes(id)) { - throw new SCIMMY.Types.Error(403, null!, 'Cannot delete technical user'); + throw new SCIMMY.Types.Error(403, null!, 'Technical user deletion not permitted.'); } const fakeScope: Scope = { userId: id }; // FIXME: deleteUser should probably better not requiring a scope. diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index 14ef7009ac..f5582d464a 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -1,4 +1,4 @@ -import axios from 'axios'; +import axios, { AxiosResponse } from 'axios'; import capitalize from 'lodash/capitalize'; import { assert } from 'chai'; import Sinon from 'sinon'; @@ -129,6 +129,27 @@ describe('Scim', () => { await getDbManager().deleteUser({ userId: userId }, userId); } } + async function checkOperationOnTechUserDisallowed({op, opType}: { + op: (id: number) => Promise, + opType: string + }) { + const db = getDbManager(); + const specialUsers = { + 'anonymous': db.getAnonymousUserId(), + 'support': db.getSupportUserId(), + 'everyone': db.getEveryoneUserId(), + 'preview': db.getPreviewerUserId(), + }; + for (const [label, id] of Object.entries(specialUsers)) { + const res = await op(id); + assert.equal(res.status, 403, `should forbid ${opType} of the special user ${label}`); + assert.deepEqual(res.data, { + schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], + status: '403', + detail: `Technical user ${opType} not permitted.` + }); + } + } function checkCommonErrors( method: 'get' | 'post' | 'put' | 'patch' | 'delete', @@ -468,6 +489,15 @@ describe('Scim', () => { assert.equal(res.status, 404); }); + it('should return 403 for technical users', async function () { + const data = toSCIMUserWithoutId('whoever'); + await checkOperationOnTechUserDisallowed({ + op: (id) => axios.put(scimUrl(`/Users/${id}`), data, chimpy), + opType: 'modification' + }); + }); + + it('should deduce the name from the displayEmail when not provided', async function () { const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), { schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'], @@ -580,23 +610,11 @@ describe('Scim', () => { assert.equal(res.status, 404); }); - it('should return 404 for technical users', async function () { - const db = getDbManager(); - const specialUsers = { - 'anonymous': db.getAnonymousUserId(), - 'support': db.getSupportUserId(), - 'everyone': db.getEveryoneUserId(), - 'preview': db.getPreviewerUserId(), - }; - for (const [label, id] of Object.entries(specialUsers)) { - const res = await axios.delete(scimUrl(`/Users/${id}`), chimpy); - assert.equal(res.status, 403, 'should forbid deletion of the special user ' + label); - assert.deepEqual(res.data, { - schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], - status: '403', - detail: 'Cannot delete technical user' - }); - } + it('should return 403 for technical users', async function () { + await checkOperationOnTechUserDisallowed({ + op: (id) => axios.delete(scimUrl(`/Users/${id}`), chimpy), + opType: 'deletion' + }); }); checkCommonErrors('delete', '/Users/1'); From 4beef42b75139fc5f196b9299ebc2baee1fd7d44 Mon Sep 17 00:00:00 2001 From: Florent Date: Fri, 29 Nov 2024 12:50:35 +0100 Subject: [PATCH 25/27] Update FIXME in ScimUserController Co-authored-by: jordigh --- app/server/lib/scim/v2/ScimUserController.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts index 6846415b69..9a24e3d724 100644 --- a/app/server/lib/scim/v2/ScimUserController.ts +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -99,7 +99,8 @@ class ScimUserController { throw new SCIMMY.Types.Error(403, null!, 'Technical user deletion not permitted.'); } const fakeScope: Scope = { userId: id }; - // FIXME: deleteUser should probably better not requiring a scope. + // FIXME: deleteUser should probably be rewritten to not require a scope. We should move + // the scope creation to a controller. await this._dbManager.deleteUser(fakeScope, id); }); } From 7ace7f80acf292b8e22b337bb01b64676059d501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Guti=C3=A9rrez=20Hermoso?= Date: Fri, 29 Nov 2024 11:59:38 -0500 Subject: [PATCH 26/27] rename "technical user" to "system user" --- app/server/lib/scim/v2/ScimUserController.ts | 4 ++-- test/server/lib/Scim.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts index 9a24e3d724..3c14c37f68 100644 --- a/app/server/lib/scim/v2/ScimUserController.ts +++ b/app/server/lib/scim/v2/ScimUserController.ts @@ -78,7 +78,7 @@ class ScimUserController { return this._runAndHandleErrors(context, async () => { const id = ScimUserController._getIdFromResource(resource); if (this._dbManager.getSpecialUserIds().includes(id)) { - throw new SCIMMY.Types.Error(403, null!, 'Technical user modification not permitted.'); + throw new SCIMMY.Types.Error(403, null!, 'System user modification not permitted.'); } await this._checkEmailCanBeUsed(data.userName, id); const updatedUser = await this._dbManager.overwriteUser(id, toUserProfile(data)); @@ -96,7 +96,7 @@ class ScimUserController { return this._runAndHandleErrors(context, async () => { const id = ScimUserController._getIdFromResource(resource); if (this._dbManager.getSpecialUserIds().includes(id)) { - throw new SCIMMY.Types.Error(403, null!, 'Technical user deletion not permitted.'); + throw new SCIMMY.Types.Error(403, null!, 'System user deletion not permitted.'); } const fakeScope: Scope = { userId: id }; // FIXME: deleteUser should probably be rewritten to not require a scope. We should move diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts index f5582d464a..b836856d53 100644 --- a/test/server/lib/Scim.ts +++ b/test/server/lib/Scim.ts @@ -146,7 +146,7 @@ describe('Scim', () => { assert.deepEqual(res.data, { schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ], status: '403', - detail: `Technical user ${opType} not permitted.` + detail: `System user ${opType} not permitted.` }); } } @@ -489,7 +489,7 @@ describe('Scim', () => { assert.equal(res.status, 404); }); - it('should return 403 for technical users', async function () { + it('should return 403 for system users', async function () { const data = toSCIMUserWithoutId('whoever'); await checkOperationOnTechUserDisallowed({ op: (id) => axios.put(scimUrl(`/Users/${id}`), data, chimpy), @@ -610,7 +610,7 @@ describe('Scim', () => { assert.equal(res.status, 404); }); - it('should return 403 for technical users', async function () { + it('should return 403 for system users', async function () { await checkOperationOnTechUserDisallowed({ op: (id) => axios.delete(scimUrl(`/Users/${id}`), chimpy), opType: 'deletion' From 6f97aecfbbd7cb4e754d5fb72572f5030321d9e2 Mon Sep 17 00:00:00 2001 From: fflorent Date: Fri, 29 Nov 2024 18:23:42 +0100 Subject: [PATCH 27/27] Document SCIM feature flag in the README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 5674c54f65..7fad9fbfeb 100644 --- a/README.md +++ b/README.md @@ -323,6 +323,7 @@ Grist can be configured in many ways. Here are the main environment variables it | GRIST_SNAPSHOT_TIME_CAP | optional. Define the caps for tracking buckets. Usage: {"hour": 25, "day": 32, "isoWeek": 12, "month": 96, "year": 1000} | | GRIST_SNAPSHOT_KEEP | optional. Number of recent snapshots to retain unconditionally for a document, regardless of when they were made | | GRIST_PROMCLIENT_PORT | optional. If set, serve the Prometheus metrics on the specified port number. ⚠️ Be sure to use a port which is not publicly exposed ⚠️. | +| GRIST_ENABLE_SCIM | optional. If set, enable the [SCIM API Endpoint](https://support.getgrist.com/install/scim/) (experimental) | #### AI Formula Assistant related variables (all optional):