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

SCIM: implement Groups #1357

Draft
wants to merge 29 commits into
base: main
Choose a base branch
from
Draft

Conversation

fflorent
Copy link
Collaborator

@fflorent fflorent commented Jan 3, 2025

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 so they can easily access to the resources. But currently, these groups are not represented in Grist.

Proposed solution

This PR proposes to distinguish 2 kind of Groups:

  • Resource Users groups, which is aimed to gather users so they are granted access to some resources;
  • Role groups, which are the role available for each resources ("Owners", "Editors", "Viewers", ...)

Currently only the Roles were represented in the "groups" table. This PR introduces a "type" column to distinguish the two kinds of groups.

Known limitations

  1. 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.

Related issues

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

Screenshots / Screencasts

@fflorent fflorent force-pushed the scim-groups-endpoints branch 3 times, most recently from fbce58e to 2c32360 Compare January 3, 2025 19:08
@fflorent fflorent force-pushed the scim-groups-endpoints branch 4 times, most recently from 8ecb5ce to 7f924fe Compare January 17, 2025 12:21
@fflorent fflorent force-pushed the scim-groups-endpoints branch from 0f8575a to 2e91be2 Compare January 31, 2025 12:50
.from(User, 'users')
.where('users.id IN (:...userIds)', {userIds});
return await queryBuilder.getMany();
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

I can revert if you have doubts.

});

// 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() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -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 () {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Do you agree for this renaming?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant