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

Force email addresses to be lower case #4880

Open
8 tasks
carlobeltrame opened this issue Apr 4, 2024 · 2 comments · May be fixed by #4906
Open
8 tasks

Force email addresses to be lower case #4880

carlobeltrame opened this issue Apr 4, 2024 · 2 comments · May be fixed by #4906
Labels
type: API WIP Work in progress

Comments

@carlobeltrame
Copy link
Member

carlobeltrame commented Apr 4, 2024

This issue needs to be re-specified, it's not ready for implementation right now.

We currently allow to store email addresses containing capital letters. This leads to some minor bugs, e.g. it is possible to invite the same email address multiple times with different capitalization: castor@example.com is treated as different from Castor@example.com. This circumvents our unique index on the inviteEmail column of CampCollaboration.

For modifying the input which users submit in various input fields, we have input filters. One example is InputFilter\Trim, which is used to trim whitespace from the start and end of input fields.

To fix the mixed-case email addresses, we should:

  • Create a new input filter InputFilter\Lowercase, along with the InputFilter\LowercaseFilter class which actually performs the modification
  • Add the #[InputFilter\Lowercase] annotation to Profile#email and Profile#newEmail
  • Add the #[InputFilter\Lowercase] annotation to CampCollaboration#inviteEmail
  • Add the #[InputFilter\Lowercase] annotation to ResetPassword#email
  • Add unit tests for the InputFilter\LowercaseFilter, similar to the tests for the trim filter
  • Add API tests for all API operations which use the modified fields (create and update on these three entities), similar to the already existing trim tests
  • After the oauth flow we need to lowercase the email too, + search for the existing email with the lowercase email from the oauth flow
  • Optional: Write a data migration to convert all existing data in the database to lower case. ⚠️ If there are already duplicate emails, we need a conflict resolution.

Maybe possible alternative:

  • Use a collation for this field which ignores the case.
@usu
Copy link
Member

usu commented Apr 12, 2024

I believe email addresses are actually case sensitive. Even though no large email provider will treat emails as case sensitive, we could not entirely rule out, someone would use an own server which treats addresses case sensitive, right?

https://stackoverflow.com/a/1503679
(the answers here are quite old, but generally I believe still valid; unless the standard has changed in the meantime)

@carlobeltrame carlobeltrame added the Meeting Discuss Am nächsten Core-Meeting besprechen label Apr 12, 2024
@manuelmeister
Copy link
Member

Core Meeting Decision

We want to keep case sensitive email addresses for sending email.
However we will disallow creating accounts with same email with different cases.
We will store the email address case sensitive and add a lowercase index with a unique constraint (for inviteEmail & email,…?). Besides that we create a data migration for the existing 26 user accounts.
When matching an invite email with an existing user account we will need to do this in a case insensitive way.

@manuelmeister manuelmeister removed the Meeting Discuss Am nächsten Core-Meeting besprechen label Apr 12, 2024
@carlobeltrame carlobeltrame added WIP Work in progress and removed Good first issue labels Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: API WIP Work in progress
Projects
Status: Design
Development

Successfully merging a pull request may close this issue.

3 participants