Lowercase input filter #4906
Lowercase input filter #4906
Conversation
manuelmeister
temporarily deployed
to
feature-branch
GitHub Actions
Inactive
⛔ Feature branch deployment currently inactive.If the PR is still open, you can add the |
krm-shrftdnv
temporarily deployed
to
feature-branch
GitHub Actions
Inactive
There was a problem hiding this comment.
Looking great already, thanks a lot for contributing!
One part of one point of the spec isn't met yet, see comment.
Also, the data migration seems more and more of a requirement to me, the longer I think about it. To implement a data migration, you could just copy pone of the migrations in api/migrations/schema and write SQL code inside which looks something like UPDATE xy SET email=LOWER(email). No need for a down implementation in the migration in this case.
| $profiles = $profileRepository->findBy(['email' => strtolower($email)]); | ||
| $profile = count($profiles) > 0 ? $profiles[0] : null; |
There was a problem hiding this comment.
As long as we don't do the data migration, this will break the login for people who log in via OAuth and have capital letters in their email address on the external identity provider.
We should probably do the data migration, and also make sure that the email address which we get from the OAuth providers (here and here) is lower cased.
Another idea I had to go without data migration would be to compare here in the authenticators using SQL WHERE LOWER("email") = ?. But that might introduce be a security issue, and would break our undocumented feature which allows users to have both a normal login and an OAuth login to the same account.
manuelmeister
removed
the
deploy!
|
Hey @krm-shrftdnv |
Fixes #4880
I did not implement migration to change existing data, because this is a rather crucial moment, and I have not yet fully figured out the scheme