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

subtle installation issue with shiro + uppercase email #281

Closed
benjamin-richter-bkg opened this issue Apr 25, 2023 · 2 comments
Closed

subtle installation issue with shiro + uppercase email #281

benjamin-richter-bkg opened this issue Apr 25, 2023 · 2 comments
Labels
solved Solution developed and accepted, not yet deployed Type: Bug Something isn't working
Milestone

Comments

@benjamin-richter-bkg
Copy link

benjamin-richter-bkg commented Apr 25, 2023

In https://github.com/ec-jrc/re3gistry/blob/v2.4.2/sources/Re3gistry2Base/src/main/java/eu/europa/ec/re3gistry2/base/utility/UserHelper.java#L126 (current version as of 2023-04-25) the email address is turned into lower case. However, this case transformation does not seem to happen when the admin user is created during installation (https://github.com/ec-jrc/re3gistry/blob/v2.4.2/sources/Re3gistry2JavaAPI/src/main/java/eu/europa/ec/re3gistry2/javaapi/handler/RegInstallationStep3Handler.java#L49). This leads to login failures when typing an email address with uppercase letters.

As the exception handler https://github.com/ec-jrc/re3gistry/blob/v2.4.2/sources/Re3gistry2Base/src/main/java/eu/europa/ec/re3gistry2/base/utility/UserHelper.java#L130 logs the email without the lowercase transformation, it was quite an adventure to pinpoint this issue.

I would suggest dropping the toLowerCase() call from UserHelper and using a case insensitive comparison in the RegUser.findByEmail query instead https://github.com/ec-jrc/re3gistry/blob/v2.4.2/sources/Re3gistry2Model/src/main/java/eu/europa/ec/re3gistry2/model/RegUser.java#L53

Something like LOWER(r.email) = LOWER(:email) could be a quick fix. However I'm not sure about the interactions with the persistence framework. For example, an index on LOWER(email) would be needed. As far as I understand the more correct way to do a case insensitive comparison in PostgreSQL would be using a case insensitive collation (https://www.postgresql.org/docs/current/collation.html) for that column, which seems even more complex with the persistence framework.

Does anyone with more experience in this codebase have an opinion?

@oruscalleda oruscalleda added Type: Bug Something isn't working under analysis A first analysis is performing labels Apr 26, 2023
@oruscalleda oruscalleda self-assigned this Apr 26, 2023
@arantzaetxebarria arantzaetxebarria added ready for testing Solution is ready to test and removed under analysis A first analysis is performing labels Feb 12, 2024
@arantzaetxebarria arantzaetxebarria added this to the v2.5.3 milestone Feb 12, 2024
@arantzaetxebarria
Copy link
Collaborator

Dear @benjamin-richter-bkg

A solution to this problem has been developed and it is fixed. It will be available in the next release v2.5.3.

Best regards

unaibrrgn added a commit that referenced this issue Mar 15, 2024
@iratigarzon
Copy link
Collaborator

After analyzing the issue, we found that if an admin user was created during installation and their email contained both uppercase and lowercase letters, they were unable to log in to the registry. Therefore, we made modifications to allow successful login in such cases.

@arantzaetxebarria arantzaetxebarria added solved Solution developed and accepted, not yet deployed and removed ready for testing Solution is ready to test labels Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solved Solution developed and accepted, not yet deployed Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants