-
Notifications
You must be signed in to change notification settings - Fork 63
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
Adds LDAP group support #4407
Adds LDAP group support #4407
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4407 +/- ##
==========================================
- Coverage 78.52% 78.18% -0.34%
==========================================
Files 294 294
Lines 13694 13753 +59
Branches 3082 3099 +17
==========================================
Hits 10753 10753
- Misses 2941 3000 +59
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Co-authored-by: Stephen McLaughlin <44235289+Steve-Mcl@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code wise, it all looks good however I have no way of trying this out and in the spirit of "done" I have to ask about tests. There is no coverage for the new functionality
Happy to approve if there is a follow up raised and @joepavitt agrees.
@Steve-Mcl all the new functions require a live LDAP server. And Nick is probably the only other person with a LDAP setup to test against. |
Indeed which is why I would be ok with approving this so long as there is a follow up task and @joepavitt is on-board. |
Agreed. Happy to proceed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved pending follow up issue raised to bring test coverage up
fixes #4002
Description
Adds support for controlling team membership with LDAP groups
I've tested this with my local LDAP instance.
Updated UI
It reuses parts from the SAML group support where possible.
There may be some code duplication in the group managemnts, but as they exist in different areas I didn't re-factor them out to a shared function.
Related Issue(s)
#4002
Checklist
flowforge.yml
?FlowFuse/helm
to update ConfigMap TemplateFlowFuse/CloudProject
to update values for Staging/ProductionLabels
area:migration
label