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

Allow for prefix/suffix to SSO GroupNames #4902

Merged
merged 8 commits into from
Dec 13, 2024
Merged

Allow for prefix/suffix to SSO GroupNames #4902

merged 8 commits into from
Dec 13, 2024

Conversation

hardillb
Copy link
Contributor

@hardillb hardillb commented Dec 11, 2024

fixes #4768

Description

Allows a prefix and suffix length to be defined for SSO group names, this works for both SAML and LDAP.

e.g.

ff requires groupnames in the following format ff-teamslug-role, this allows an arbitrary number of characters to be before and after this format. e.g.

with prefix and suffix length set to 5 the following group would match

test_ff-teamslug-role_test

image

Related Issue(s)

#4768

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

@hardillb hardillb self-assigned this Dec 11, 2024
@hardillb
Copy link
Contributor Author

hardillb commented Dec 11, 2024

Needs a doc update

Copy link

codecov bot commented Dec 11, 2024

Codecov Report

Attention: Patch coverage is 53.84615% with 6 lines in your changes missing coverage. Please review.

Project coverage is 78.69%. Comparing base (b545283) to head (9d3306b).
Report is 36 commits behind head on main.

Files with missing lines Patch % Lines
forge/ee/lib/sso/index.js 53.84% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4902      +/-   ##
==========================================
- Coverage   78.74%   78.69%   -0.06%     
==========================================
  Files         323      323              
  Lines       15221    15242      +21     
  Branches     3496     3504       +8     
==========================================
+ Hits        11986    11994       +8     
- Misses       3235     3248      +13     
Flag Coverage Δ
backend 78.69% <53.84%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hardillb
Copy link
Contributor Author

Tests for SAML pattern, the LDAP function needs a functional LDAP server to run against.

I did test both against my local KeyCloak and LDAP system

docs/admin/sso/ldap.md Outdated Show resolved Hide resolved
docs/admin/sso/saml.md Outdated Show resolved Hide resolved
Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

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

Approved, but some doc/text/updates to apply first.

hardillb and others added 2 commits December 13, 2024 11:23
Co-authored-by: Nick O'Leary <nick.oleary@gmail.com>
Co-authored-by: Nick O'Leary <nick.oleary@gmail.com>
@knolleary knolleary merged commit 529c85c into main Dec 13, 2024
20 of 21 checks passed
@knolleary knolleary deleted the sso-group-prefix branch December 13, 2024 13:49
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.

Ability to map exisitng LDAP SSO groups to teams
2 participants