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

Sso create new user #4207

Merged
merged 14 commits into from
Jul 24, 2024
Merged

Sso create new user #4207

merged 14 commits into from
Jul 24, 2024

Conversation

hardillb
Copy link
Contributor

closes #4051

Description

Allows SSO configurations to be configured to create new users when they sign in for the first time.

Currently only supports SAML SSO providers.

Adds new flag to SSO config.

Related Issue(s)

#4051

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 Jul 18, 2024
@hardillb
Copy link
Contributor Author

hardillb commented Jul 18, 2024

Need to work out how to test this

Also will need a doc update.

And before review will need the large chuck of commented out code removing.

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

Attention: Patch coverage is 35.22013% with 103 lines in your changes missing coverage. Please review.

Project coverage is 78.16%. Comparing base (1465065) to head (f0b546e).
Report is 92 commits behind head on main.

Files Patch % Lines
forge/ee/lib/sso/index.js 7.69% 60 Missing ⚠️
forge/ee/routes/sso/auth.js 8.82% 31 Missing ⚠️
forge/lib/userTeam.js 80.00% 11 Missing ⚠️
forge/routes/auth/index.js 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4207      +/-   ##
==========================================
- Coverage   78.63%   78.16%   -0.48%     
==========================================
  Files         286      287       +1     
  Lines       13106    13261     +155     
  Branches     2931     2965      +34     
==========================================
+ Hits        10306    10365      +59     
- Misses       2800     2896      +96     
Flag Coverage Δ
backend 78.16% <35.22%> (-0.48%) ⬇️

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.

@cstns cstns mentioned this pull request Jul 19, 2024
10 tasks
@knolleary knolleary self-requested a review July 23, 2024 15:17
@hardillb hardillb marked this pull request as ready for review July 24, 2024 08:09
@knolleary
Copy link
Member

Have tested against Entra and all working nicely.

Next up - will verify behaviour with Google workspace and LDAP

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.

Testing with Azure and Google - working nicely.

One set of changes requested - renaming the extracted function to completeUserSignup as it does more than just create the team. I think I've proposed all the necessary changes. Also modified it to export an object, rather than the bare function.

forge/lib/userTeam.js Outdated Show resolved Hide resolved
forge/lib/userTeam.js Show resolved Hide resolved
forge/routes/auth/index.js Outdated Show resolved Hide resolved
forge/ee/lib/sso/index.js Outdated Show resolved Hide resolved
forge/ee/lib/sso/index.js Outdated Show resolved Hide resolved
forge/ee/routes/sso/auth.js Outdated Show resolved Hide resolved
forge/ee/routes/sso/auth.js Outdated Show resolved Hide resolved
forge/routes/auth/index.js Outdated Show resolved Hide resolved
forge/ee/routes/sso/auth.js Outdated Show resolved Hide resolved
hardillb and others added 2 commits July 24, 2024 12:27
Co-authored-by: Nick O'Leary <nick.oleary@gmail.com>
Co-authored-by: Nick O'Leary <nick.oleary@gmail.com>
@knolleary
Copy link
Member

Will do one final test through with those last changes then good to merge

@knolleary knolleary merged commit 0277dcd into main Jul 24, 2024
13 of 14 checks passed
@knolleary knolleary deleted the sso-create-new-user branch July 24, 2024 11:39
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.

Allow user registeration on first sign-in via SSO
2 participants