Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support SCIM #870

Open
fflorent opened this issue Feb 26, 2024 · 0 comments · May be fixed by #1357
Open

Support SCIM #870

fflorent opened this issue Feb 26, 2024 · 0 comments · May be fixed by #1357
Assignees
Labels
self-hosting Self-hosting setup that needs some love

Comments

@fflorent
Copy link
Collaborator

SCIM is a standard way to provision or deprovision users to a Service Provider (like Grist) as requested by an Identity Provider.

This could let the IdP to do the following on Grist:

  • create accounts for users and start assigning them access without waiting for them to connect the first time on Grist;
  • delete accounts and/or revoke access from organizations (and maybe workspaces);
  • change user informations in Grist;

It requires to implement a REST API that complies with the spec (see below).

IETF RFC (spec): https://www.rfc-editor.org/rfc/rfc7644
Okta resource: https://developer.okta.com/docs/concepts/scim/

@jordigh jordigh added the self-hosting Self-hosting setup that needs some love label Aug 2, 2024
@fflorent fflorent self-assigned this Sep 4, 2024
paulfitz pushed a commit that referenced this issue Sep 5, 2024
Context

HomeDBManager lacks of direct tests, which makes hard to make rework or refactorations.
Proposed solution

Specifically here, I introduce tests which call exposed UsersManager methods directly and check their result.

Also:

    I removed updateUserName which seems to me useless (updateUser does the same work)
    Taking a look at the getUserByLogin methods, it appears that Typescirpt infers it returns a Promise<User|null> while in no case it may resolve a nullish value, therefore I have forced to return a Promise<User> and have changed the call sites to reflect the change.

Related issues

I make this change for then working on #870
@fflorent fflorent mentioned this issue Sep 6, 2024
4 tasks
hexaltation pushed a commit to hexaltation/grist-core that referenced this issue Sep 24, 2024
Context

HomeDBManager lacks of direct tests, which makes hard to make rework or refactorations.
Proposed solution

Specifically here, I introduce tests which call exposed UsersManager methods directly and check their result.

Also:

    I removed updateUserName which seems to me useless (updateUser does the same work)
    Taking a look at the getUserByLogin methods, it appears that Typescirpt infers it returns a Promise<User|null> while in no case it may resolve a nullish value, therefore I have forced to return a Promise<User> and have changed the call sites to reflect the change.

Related issues

I make this change for then working on gristlabs#870
jordigh added a commit that referenced this issue Dec 4, 2024
## 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?

<!-- Put an `x` in the box that applies: -->

- [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 <!-- Detail how we can help you -->

## 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]>
paulfitz pushed a commit that referenced this issue Dec 13, 2024
## Context

I pave the way to implement Groups for SCIM.

## Proposed solution

Refactoring for moving the Groups Management outside of HomeDBManager.

## Related issues

#870 

## Has this been tested?

- [ ] 👍 yes, I added tests to the test suite
- [ ] 💭 no, because this PR is a draft and still needs work
- [x] 🙅 no, because this is not relevant here (no methods are used elsewhere than in `lib/homedb`)
- [ ] 🙋 no, because I need help <!-- Detail how we can help you -->
@fflorent fflorent linked a pull request Feb 1, 2025 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
self-hosting Self-hosting setup that needs some love
Projects
Development

Successfully merging a pull request may close this issue.

2 participants