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: make email comparison for SAML case insensitive #1173

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fastlorenzo
Copy link
Contributor

Email comparison when doing SSO with SAML is currently case sensitive.
However, email addresses should be treated as case insensitive.

This PR makes sure that:

  1. Email address of each user is saved as lowercase in the database
  2. Email address received from IDP when doing SSO is converted to lowercase prior to comparison

@eric-intuitem
Copy link
Collaborator

eric-intuitem commented Dec 11, 2024

LGTM, but we need to adapt functional tests. @nas-tabchiche ?

@ab-smith
Copy link
Contributor

we will merge it after the Ebios RM sprint and fix the test on our side before releasing

@fastlorenzo
Copy link
Contributor Author

we will merge it after the Ebios RM sprint and fix the test on our side before releasing

Thanks, no worries, now that we know the current behavior we've fixed it in the IDP by forcing lowercase email conversion

@nas-tabchiche nas-tabchiche changed the title Make email comparison for SAML case insensitive fix: make email comparison for SAML case insensitive Feb 13, 2025
@nas-tabchiche
Copy link
Contributor

Hi @fastlorenzo, we just merged #1498 which should make functional tests pass on this PR. I can't manually re-run the tests (maybe PR is too old). I'm closing this and re-opening to force CI to run.

@nas-tabchiche nas-tabchiche reopened this Feb 13, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Feb 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants