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

Sync groups with SAML provider #6299

Merged
merged 6 commits into from
Nov 20, 2023
Merged

Sync groups with SAML provider #6299

merged 6 commits into from
Nov 20, 2023

Conversation

aelgasser
Copy link
Contributor

This PR enables syncing groups from the IdP in SAML strategies.

It adds two new fields to the SAML configuration:

image

The field's value is sent by the IdP into profile.attributes through the Group attributes field when enabled (see below my setup with Jumpcloud, all SAML-compliant providers such as Okta or Keycloak expose such attribute):

image

Once connected, the groups coming from the IdP will overwrite the groups of the user in the Wiki (I copied the behavior from the LDAP group sync; which is a good assumption as the enterprise directory must be the ground truth for user access in that case).

One final note, to allow JIT provisioning and IdP-initiated login, wiki admins must enable the "Allow self-registration" option in the SAML strategy and leave the Assign to group field blank (it'll be overwritten anyway):

image

@aelgasser
Copy link
Contributor Author

@NGPixel do you think you can review and approve this?

@aelgasser
Copy link
Contributor Author

Hi @NGPixel , @andreleoni , @elomarcorreia , @enjoeiluis, is there anything missing to merge this PR?

@NGPixel NGPixel merged commit 38a46e6 into requarks:main Nov 20, 2023
@NGPixel
Copy link
Member

NGPixel commented Nov 20, 2023

Thanks! Sorry for the delay.

@aelgasser
Copy link
Contributor Author

Hi @NGPixel thanks for merging!

I just worked on a modification of this feature (here https://github.com/aelgasser/wiki-js/pull/1/files) to control how the group assignments are synchronized. In my case, I wanted to keep the default admin group of WikiJS assigned when a user signs on, and I couldn't use the "Administrators" group name in our identity provider as it's already used to define the administrators of our organization and I needed to separate the WikiJs admins from the organization admins.

Is it worth adding to the main codebase (and probably to be added to the LDAP implementation, though I don't have any way to test it locally) or should I just keep it for me?

jionggyu pushed a commit to jionggyu/wiki-2.5.302-patch that referenced this pull request Jul 9, 2024
* feat: added implementation for group mapping in SAML strategies

---------

Co-authored-by: Abderraouf El Gasser <abderraouf.elgasser@iktos.com>
Co-authored-by: Nicolas Giard <github@ngpixel.com>
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.

5 participants