SCIM users endpoint (#1199)

## Context As an IT asset administrator, I can create account of my users using a centralized solution like an SSO so they can log on Grist. It's quite convenient because I don't have to worry about creating accounts specifically for them. Also Grist handles the update of Users when reconnect. There are things the administrator cannot do though: - assign users to Groups (like the `owners of an team site`); - change immediately user information; - delete the user when they are removed from the SSO; - get the list of users or groups in a normalized way; - ... ## Proposed solution SCIM is a standard proposed by the IETF through [RFC7644](https://www.rfc-editor.org/rfc/rfc7644) and [RFC7643](https://www.rfc-editor.org/rfc/rfc7643) which aims to through a simple Rest API provide solution for the above use cases. Here is the abstract of the RFC7644 which introduces SCIM: > The System for Cross-domain Identity Management (SCIM) specification is an HTTP-based protocol that makes managing identities in multi-domain scenarios easier to support via a standardized service. > Examples include, but are not limited to, enterprise-to-cloud service providers and inter-cloud scenarios. The specification suite seeks to build upon experience with existing schemas and deployments, placing specific emphasis on simplicity of development and integration, while applying existing authentication, authorization, and privacy models. SCIM's intent is to reduce the cost and complexity of user management operations by providing a common user schema, an extension model, and a service protocol defined by this document. This PR provides the implementation of SCIM for Users Resources (Group will come in a future PR), and supports: - All the basic actions (`POST /Users/`, `PUT /Users/:id`, `GET /Users/`, `GET /Users/:id` and `DELETE /Users/:id`). - The `/Schemas`, `/ServiceProviderConfig`, `/ResourceTypes` endpoints; - The `/Me` endpoint (it takes advantage of the `id` you returned in the authentication middleware); - The `POST /Bulk` endpoint - The `POST /Resources/.search` by using the Filters (actually to use them, you must have to fetch all the Resources from the DB, the filtering is done in JS, which is probably fine for small projects, I would just be cautious when using big databases + an ORM); - There are some error utilities to help you; - The `PATCH /Resources/:id` endpoint! It reads a resource using the egress method, applies the asked changes, and calls the ingress method to update the record ; - The pagination To do that, I take advantage of two libraries: [scimmy](https://github.com/scimmyjs/scimmy) and [scimmy-routers](https://github.com/scimmyjs/scimmy-routers). Scimmy is lightweight (0 dependency), and scimmy-routers will also be dependency-free be in a future version (reported in that [issue](scimmyjs/scimmy-routers#20) and already fixed). Two variables are introduced: - `GRIST_ENABLE_SCIM` to let the administrator enable the scim API (defaults to false); - `GRIST_SCIM_EMAIL` to let the administrator specify a user that is allowed, they are granted rights to do any operation using SCIM (just like the administrators of `GRIST_DEFAULT_EMAIL` and `GRIST_SUPPORT_EMAIL`); ## Assumption regarding the SCIM implementation - the ID is the technical ID in the database; - SCIM's `userName` corresponds to the normalized email (`logins.email`), the SCIM `emails` corresponds to the `displayEmail`; - I don't allow more than an email to be passed (as the Grist code requires currently IIRC); - Anonymous users cannot call any SCIM endpoint; - Authenticated non-admin and non-GRIST_SCIM_EMAIL users can only request [these endpoints](https://github.com/gristlabs/grist-core/pull/1199/files#diff-718d3e2193a1261bcf5cd6f6bba04e22bec1b0c4b162c6adbf0ae4847f107912R10); ## How to test manually? I can document the API in grist-help upon request (otherwise I will do that after this PR is merged). You may: 1. run a local Grist server setting either the GRIST_DEFAULT_EMAIL, GRIST_SUPPORT_EMAIL or GRIST_SCIM_EMAIL env variable without omitting to enable SCIM using `GRIST_ENABLE_SCIM`: ```bash GRIST_SCIM_EMAIL="[email protected]" GRIST_ENABLE_SCIM=1 yarn start ``` 2. Generate a bearer for [email protected] 3. then you may start using SCIM: ```bash $ export BEARER=<paste the bearer here> $ curl -H 'Content-Type: application/scim+json' -H "Authorization: Bearer $BEARER" -X POST -d '{"schemas": ["urn:ietf:params:scim:api:messages:2.0:SearchRequest"], "sortBy": "userName", "sortOrder": "descending"}' https://localhost:8484/api/scim/v2/Users/.search ``` I described some examples of the SCIM API usage here (they need to be adaptated for the context of Grist): https://github.com/scimmyjs/scimmy-routers/blob/8ffa2221b542054c3f0cfb765ea6957f29ebe5e1/example/README.md#play-with-the-scim-server ## Limitations of the current implementation - The user bearer not renewed automatically, so it does not comply with the request of limiting their lifetime ([source](https://www.rfc-editor.org/rfc/rfc7644#section-7.4)); - Only an administrator (with the `GRIST_DEFAULT_EMAIL` or the support account) or the user with `GRIST_SCIM_EMAIL` are allowed to make operations on resources (other user are limited to use `/Me`). - A dedicated account (like `GRIST_SCIM_EMAIL`) is required to have access to the endpoints, which should have their API key generated. I considered instead having a bearer directly set in an env variable, but I rejected the idea because it would [have been rejected by the Authorizer](https://github.com/gristlabs/grist-core/blob/30e2cc47d2916410579421fe8af21b609a73f468/app/server/lib/Authorizer.ts#L177-L182). - The `/Me` endpoint implementation seems partial ([issue](scimmyjs/scimmy-routers#27)); - I forgot to add tests for the pagination… I am noting to do that; - The SCIMMY and scimmy-routers libraries lack of typing support, so you may see many `any` types or some casts until that is fixed ([issue](scimmyjs/scimmy#45) and [issue](scimmyjs/scimmy-routers#24)); - [now fixed] The `Content-Type` must be `application/scim+json`, currently `application/json` is not supported ([will be fixed in the next scimmy-routers release](scimmyjs/scimmy-routers#22)) ## Documentation I opened this PR in draft to start documenting SCIM: gristlabs/grist-help#434 It can be previewed here: - the documentation (including an intro): https://deploy-preview-434--grist-help-preview.netlify.app/install/scim/ - the API reference: https://deploy-preview-434--grist-help-preview.netlify.app/api/#tag/scim ## Related issues It partly implements #870 (Users resource only for now). ## Has this been tested?  - [x] 👍 yes, I added tests to the test suite - [ ] 💭 no, because this PR is a draft and still needs work - [ ] 🙅 no, because this is not relevant here - [ ] 🙋 no, because I need help  ## Original commit messages. * WIP * SCIM: Implement egress + tests * Implement ingress * Add tests * SCIM: Implement DELETE * More tests and cleanups * Rebase fix + check GRIST_SCIM_USER * Add GRIST_ENABLE_SCIM env variable * Move logic for Users to its own controller * An unknown error should return a 500 * Add a test for pagination * Add tests for the new UsersManager methods * Document methods of the controller * Rename ex → err in catch block * Bump Scimmy and Scimmy-Routers * Log errors * Only warn when the userName differ from the primary email * Rename overrideUser → overwriteUser * Use full path for import * Improve user deletion test description Co-authored-by: jordigh <[email protected]> * Improve error message for anonymous users Co-authored-by: jordigh <[email protected]> * Fix styling issue * Disallow deleting technical users * Disallow technical user modifications * Update FIXME in ScimUserController Co-authored-by: jordigh <[email protected]> * rename "technical user" to "system user" * Document SCIM feature flag in the README --------- Co-authored-by: jordigh <[email protected]>
gristlabs · Dec 4, 2024 · efefc96 · efefc96
1 parent 00739d8
commit efefc96
Show file tree

Hide file tree

Showing 16 changed files with 1,260 additions and 4 deletions.
diff --git 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):
 

diff --git 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);
   }
@@ -559,6 +563,10 @@ export class HomeDBManager extends EventEmitter {
     return this._usersManager.deleteUser(scope, userIdToDelete, name);
   }
 
+  public async overwriteUser(userId: number, props: UserProfile) {
+    return this._usersManager.overwriteUser(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
@@ -2610,6 +2618,10 @@ export class HomeDBManager extends EventEmitter {
     return this._usersManager.getAnonymousUser();
   }
 
+  public getSpecialUserIds() {
+    return this._usersManager.getSpecialUserIds();
+  }
+
   public getAnonymousUserId() {
     return this._usersManager.getAnonymousUserId();
   }
@@ -3599,7 +3611,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
@@ -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.
@@ -395,7 +402,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 +589,32 @@ 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 overwriteUser(userId: number, props: UserProfile): Promise<User> {
+    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))!;
+    });
+  }
+
+  public async getUsers() {
+    return await User.find({relations: ["logins"]});
+  }
+
+
   /**
    * ==================================
    *
@@ -688,7 +721,7 @@ export class UsersManager {
   /**
    * Returns a Promise for an array of User entites for the given userIds.
    */
-  public async getUsers(userIds: number[], optManager?: EntityManager): Promise<User[]> {
+  public async getUsersByIds(userIds: number[], optManager?: EntityManager): Promise<User[]> {
     if (userIds.length === 0) {
       return [];
     }
@@ -772,6 +805,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/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
@@ -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 '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:
@@ -896,6 +897,19 @@ 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; }
+
+    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);
+  }
+
+
   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
@@ -0,0 +1,14 @@
+import * as express from 'express';
+
+import { buildScimRouterv2 } from 'app/server/lib/scim/v2/ScimV2Api';
+import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager';
+import { InstallAdmin } from 'app/server/lib/InstallAdmin';
+
+const buildScimRouter = (dbManager: HomeDBManager, installAdmin: InstallAdmin) => {
+  const v2 = buildScimRouterv2(dbManager, installAdmin);
+  const scim = express.Router();
+  scim.use('/v2', v2);
+  return scim;
+};
+
+export { buildScimRouter };
diff --git a/app/server/lib/scim/v2/ScimTypes.ts 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
@@ -0,0 +1,175 @@
+import { ApiError } from 'app/common/ApiError';
+import { HomeDBManager, Scope } from 'app/gen-server/lib/homedb/HomeDBManager';
+import SCIMMY from 'scimmy';
+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 {
+  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
+  ) {}
+
+  /**
+   * 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);
+      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);
+    });
+  }
+
+  /**
+   * 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;
+      const scimmyUsers = (await this._dbManager.getUsers()).map(user => toSCIMMYUser(user));
+      return filter ? filter.match(scimmyUsers) : scimmyUsers;
+    });
+  }
+
+  /**
+   * 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._checkEmailCanBeUsed(data.userName);
+      const userProfile = toUserProfile(data);
+      const newUser = await this._dbManager.getUserByLoginWithRetry(userProfile.email, {
+        profile: userProfile
+      });
+      return toSCIMMYUser(newUser);
+    });
+  }
+
+  /**
+   * 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 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!, 'System user modification not permitted.');
+      }
+      await this._checkEmailCanBeUsed(data.userName, id);
+      const updatedUser = await this._dbManager.overwriteUser(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);
+      if (this._dbManager.getSpecialUserIds().includes(id)) {
+        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
+      //        the scope creation to a controller.
+      await this._dbManager.deleteUser(fakeScope, id);
+    });
+  }
+
+  /**
+   * 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<T>(context: RequestContext, cb: () => Promise<T>): Promise<T> {
+    try {
+      this._checkAccess(context);
+      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);
+    }
+  }
+
+  /**
+   * 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 !== userIdToUpdate) {
+      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.overwriteUser(resource, data, context);
+      }
+      return await controller.createUser(data, context);
+    },
+    degress: async (resource: any, context: RequestContext) => {
+      return await controller.deleteUser(resource, context);
+    }
+  };
+};