SCIM: implement Groups #1357

fflorent · 2025-01-03T15:18:26Z

Context

Following #1199, this PR proposes to implement the /Groups Endpoint.

As an IT asset administrator, I can create groups for my users using a centralized solution (an Identity Provider) so they can easily access to the resources. But these groups and memberships are not pushed to Grist as of today. The administrator is either offered to remove access entirely to Grist, or access to Grist and modify the permissions on the resources. Most of all, as Teams are not represented through the UI, it can be a quite painful task.

Proposed solution

SCIM is a standard proposed by the IETF through RFC7644 and 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 a complement to the now existing /scim/v2/Users endpoint (documented here):

SCIM Endpoint	Group Type¹	Usage
`/scim/v2/Groups`	Team	Represents a team of users in your organisation, who are granted accesses toa set of resources through Roles
`/scim/v2/Roles` ²	Role	Grants users and teams a certain access (`owner`, `editor`, `viewer`, ...) to a given resource

Currently only the Roles were represented in the "groups" table. To achieve this distinction, this PR also introduces a "type" column, which only accept team or role values.

This PR provides the implementation of SCIM for Groups (aka Teams), and supports:

All the basic actions for Groups (POST /Groups/, PUT /Groups/:id, GET /Groups/, GET /Groups/:id and DELETE /Groups/:id).
The POST /Bulk endpoint
The POST /Groups/.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);
The PATCH /Groups/: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

The endpoint for /Roles are nearly the same, though:

Creation (trough POST) and the deletion are not allowed;
We can only update the memberships, which is helpful to assign a role to users or teams;
And the endpoint also return the resource ID associated to the role through the orgId, workspaceId or docId readonly attributes.

As in #1199, I take advantage of these two libraries: scimmy and scimmy-routers.

Known limitations

You cannot add a group as a member of a Resouce Users group, due to the fact that the depth for groups are limited to 4.
The groups are not displayed in the Users Management popup for now. This may be a bit confusing.

Related issues

Fixes #870

Has this been tested?

👍 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

In the Grist terminology, and as specified in the groups.type column in the database. ↩
This is a custom endpoint as allowed by the SCIM standard. ↩

This issue is now solved: scimmyjs/scimmy-routers#24

Won't implement this endpoint: scimmyjs/scimmy-routers#27

fflorent · 2025-01-31T13:53:42Z

app/gen-server/lib/homedb/UsersManager.ts

+        .from(User, 'users')
+        .where('users.id IN (:...userIds)', {userIds});
+      return await queryBuilder.getMany();
+    });


I may be wrong on this one, but it seems these two codes are equivalent.

I can revert if you have doubts.

fflorent · 2025-01-31T15:07:27Z

test/gen-server/migrations.ts

  });

  // a test to ensure the TeamMember migration works on databases with existing content
-  it('can perform TeamMember migration with seed data set', async function() {
+  it.skip('can perform TeamMember migration with seed data set', async function() {


I had to skip this test, unfortunately I have passed several hours on it without success.

Can you be more specific on what fails?
And add a description in the code of why it's skipped?

fflorent · 2025-01-31T15:09:35Z

test/server/lib/Scim.ts

@@ -222,402 +229,1170 @@ 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 () {


Removed it, it looks like this won't be fixed (for good reasons I think):
scimmyjs/scimmy-routers#27

fflorent · 2025-02-01T09:37:36Z

app/gen-server/entity/Group.ts


 @Entity({name: 'groups'})
 export class Group extends BaseEntity {
+  public static readonly ROLE_TYPE = 'role';
+  public static readonly RESOURCE_USERS_TYPE = 'resource users';


I am not very fan of this name, I admit. I found the name team here and it's sounds more accurate and understandable :

grist-core/app/gen-server/lib/homedb/HomeDBManager.ts

Line 4195 in 7d0bbf3

// for implementing something like teams in the future. It has no measurable effect on

Do you agree for this renaming?

hexaltation · 2025-02-03T08:49:44Z

app/server/lib/scim/v2/roles/SCIMMYRoleSchema.ts

+
+/**
+ * SCIMMY Role Schema.
+ * Heavily inspired by SCIMMY Group Schema.


As the author of the original code is Sam Lee-Lindsay maybe you can add his name.

Suggested change

* Heavily inspired by SCIMMY Group Schema.

* Heavily inspired by SCIMMY Group Schema by Sam Lee-Lindsay.

Does it need that this file have to be under MIT license following this rule

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

is not clear to me

hexaltation

Thank you @fflorent for this huge piece of work.

Two small changes asked.

And one concern about the stickyness of the code with SCIMMY.
How much of of work could it need:

To add a layer of abstraction to not use SCIMMY object in higher levels of grist,
To replace this lib contributed by only one guy the day he won't maintained it anymore.

But no doubt that it's the best solution for now.

hexaltation · 2025-02-03T09:11:59Z

test/gen-server/migrations.ts

  });

  // a test to ensure the TeamMember migration works on databases with existing content
-  it('can perform TeamMember migration with seed data set', async function() {
+  it.skip('can perform TeamMember migration with seed data set', async function() {


Can you be more specific on what fails?
And add a description in the code of why it's skipped?

fflorent force-pushed the scim-groups-endpoints branch 3 times, most recently from fbce58e to 2c32360 Compare January 3, 2025 19:08

fflorent force-pushed the scim-groups-endpoints branch 4 times, most recently from 8ecb5ce to 7f924fe Compare January 17, 2025 12:21

fflorent added 23 commits January 31, 2025 13:50

SCIM: add groups.type migration

f12509d

WIP: Group's CRUD

ddd7aa5

Factorize the tests

851433c

overwriteGroup + deleteGroup with tests

53686e4

GroupsManager: Fix tests

54ae03c

Dependencies: Bump Scimmy

2e00285

Scimmy: Remove the cast to express.Router

65d8c48

This issue is now solved: scimmyjs/scimmy-routers#24

Fix postgresql errors

624524f

Fix again

8633798

Skip TeamMember migration (I don't know how to fix it)

6b935b7

SCIM: Remove test for PATCH /me

cc02645

Won't implement this endpoint: scimmyjs/scimmy-routers#27

SCIM: Move /Users test in their own describe bloc

7aa5e22

Working version for /Groups and /Groups/:id

0b82613

Support POST /Groups

27ca44d

Implement PUT + PATCH /Groups/{id}

b811e40

SCIM: Fix tests for PUT and add PATCH ones

f9fba4a

SCIM: Implement DELETE /Groups/{id} endpoint

4ba7ca2

Scim: Ensure we can't alter roles

07c3361

WIP: Endpoint for the roles

85505b0

WIP for Roles

46c626a

Fix displayName of Roles is mutable

e30e283

SCIM: Working version of 501 returned by DELETE /Roles/:id

c64bf45

WIP: Fix tests for GroupsManager

f0a1927

Fix GroupsManager tests

2e91be2

fflorent force-pushed the scim-groups-endpoints branch from 0f8575a to 2e91be2 Compare January 31, 2025 12:50

fflorent added 5 commits January 31, 2025 13:59

Document methods in GroupsManager

5d17a91

SCIMMY Roles: Move Resource and Schema to their own modules

ecbe88d

SCIM Role: Add some comments

3aecd92

Few cleanups

6a84e17

Various cleanups

61ced92

fflorent commented Jan 31, 2025

View reviewed changes

fflorent commented Feb 1, 2025

View reviewed changes

fflorent added 4 commits February 1, 2025 14:50

cleanup: prepare renaming "Resource Users" to "Team"

fabd8ea

SCIM: Rename Resource Users groups to "Team" Groups

716a18f

SCIM: Add test that Get /Roles returns resource IDs

7f907ae

Scim: Ensure team name uniqueness

65c60d4

fflorent marked this pull request as ready for review February 2, 2025 21:19

fflorent requested a review from hexaltation February 2, 2025 21:19

Scim/migration: remove unique index when dropping groups.type

ec44eb9

hexaltation reviewed Feb 3, 2025

View reviewed changes

hexaltation suggested changes Feb 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SCIM: implement Groups #1357

SCIM: implement Groups #1357

fflorent commented Jan 3, 2025 •

edited

Loading

fflorent Jan 31, 2025

fflorent Jan 31, 2025

hexaltation Feb 3, 2025

fflorent Jan 31, 2025

fflorent Feb 1, 2025

hexaltation Feb 3, 2025 •

edited

Loading

hexaltation left a comment

hexaltation Feb 3, 2025

	* Heavily inspired by SCIMMY Group Schema.
	* Heavily inspired by SCIMMY Group Schema by Sam Lee-Lindsay.

SCIM: implement Groups #1357

Are you sure you want to change the base?

SCIM: implement Groups #1357

Conversation

fflorent commented Jan 3, 2025 • edited Loading

Context

Proposed solution

Known limitations

Related issues

Has this been tested?

Footnotes

fflorent Jan 31, 2025

Choose a reason for hiding this comment

fflorent Jan 31, 2025

Choose a reason for hiding this comment

hexaltation Feb 3, 2025

Choose a reason for hiding this comment

fflorent Jan 31, 2025

Choose a reason for hiding this comment

fflorent Feb 1, 2025

Choose a reason for hiding this comment

hexaltation Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

hexaltation left a comment

Choose a reason for hiding this comment

hexaltation Feb 3, 2025

Choose a reason for hiding this comment

fflorent commented Jan 3, 2025 •

edited

Loading

hexaltation Feb 3, 2025 •

edited

Loading