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

fix(core): Create a personal project if a user signs up via SAML #9310

Merged
merged 3 commits into from May 6, 2024

Conversation

despairblue
Copy link
Contributor

@despairblue despairblue commented May 6, 2024

Summary

With this PR a personal project is also created for users that are created via SAML.

This is best reviewed commit by commit.

Additionally there is a small fix that makes sure that SAML errors contain the message from samlify, instead of just undefined.

I also refactored createUserFromSamlAttributes to run inside a transaction, so that either the user, project and auth identity are created, or nothing. I removed some code that was unnecessary according to the types.

Related tickets and issues

none

Review / Merge checklist

  • PR title and summary are descriptive. Remember, the title automatically goes into the changelog. Use (no-changelog) otherwise. (conventions)
  • Tests included.

    A bug is not considered fixed, unless a test is added to prevent it from happening again.
    A feature is not complete without tests.

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels May 6, 2024
@despairblue despairblue marked this pull request as ready for review May 6, 2024 09:16
@despairblue despairblue marked this pull request as draft May 6, 2024 14:32
@despairblue despairblue marked this pull request as ready for review May 6, 2024 14:41
Copy link
Contributor

github-actions bot commented May 6, 2024

⚠️ Some Cypress E2E specs are failing, please fix them before merging

Copy link

cypress bot commented May 6, 2024

8 failed and 3 flaky tests on run #4875 ↗︎

8 352 12 0 Flakiness 3

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 despairblue 🗃️ e2e/*
Project: n8n Commit: 9578819b59
Status: Failed Duration: 06:13 💡
Started: May 6, 2024 3:47 PM Ended: May 6, 2024 3:54 PM
Failed  cypress/e2e/17-sharing.cy.ts • 8 failed tests

View Output Video

Test Artifacts
Sharing > should create C1, W1, W2, share W1 with U3, as U2 Test Replay Screenshots Video
Sharing > should create C2, share C2 with U1 and U2, as U3 Test Replay Screenshots Video
Sharing > should open W1, add node using C2 as U3 Test Replay Screenshots Video
Sharing > should open W1, add node using C2 as U2 Test Replay Screenshots Video
Sharing > should not have access to W2, as U3 Test Replay Screenshots Video
Sharing > should have access to W1, W2, as U1 Test Replay Screenshots Video
Sharing > should automatically test C2 when opened by U2 sharee Test Replay Screenshots Video
Sharing > should work for admin role on credentials created by others (also can share it with themselves) Test Replay Screenshots Video
Flakiness  5-ndv.cy.ts • 2 flaky tests

View Output Video

Test Artifacts
NDV > should not retrieve remote options when required params throw errors Test Replay Screenshots Video
NDV > Stop listening for trigger event from NDV Test Replay Screenshots Video
Flakiness  19-execution.cy.ts • 1 flaky test

View Output Video

Test Artifacts
Execution > should test manual workflow stop Test Replay Screenshots Video

Review all test suite changes for PR #9310 ↗︎

@despairblue despairblue merged commit afffa36 into feature/rbac May 6, 2024
28 of 31 checks passed
@despairblue despairblue deleted the fix-ldap-user-creation branch May 6, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants