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

LdapAccountsManager - skip organization creation of org is empty #80

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

pmauduit
Copy link
Member

This one supersedes #79, takes back my previous one (#78), but since the preauthentication PR has now been merged, it allows me to add an integration test to reveal the bug it attempts to fix.

some IdP (e.g. FranceConnect) seem to return a blank ("") organization, we cannot rely on the try {} catch (NameNotFoundException) here, but at least we can just skip the org creation if we don't have at least a name to create it.

We don't handle the case "put the user into a default organization" on purpose, as I don't think this has been specified.

Tests: updated testsuite to take into account blank org.

some IdP (e.g. FranceConnect) seem to return a blank ("") organization,
we cannot rely on the `try {} catch (NameNotFoundException)` here, but
at least we can just skip the org creation if we don't have at least a
name to create it.

We don't handle the case "put the user into a default organization" on
purpose, as I don't think this has been specified.

Tests: updated testsuite to take into account blank org.
@pmauduit pmauduit requested a review from groldan November 13, 2023 17:20
Copy link
Member

@groldan groldan left a comment

Choose a reason for hiding this comment

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

Thanks

@pmauduit pmauduit merged commit 1b50552 into main Nov 16, 2023
@pmauduit pmauduit deleted the fix-if-org-empty branch November 16, 2023 11:27
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.

2 participants