Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Make password reset email field case insensitive #1170

Merged
merged 6 commits into from Oct 19, 2016

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Oct 14, 2016

Canonicalizes email addresses to lowercase when adding them: this is the very first step of migrating towards HSes having knowledge of and validating threepids. They still use the ID server to validate emails though.

Adds an index on email address: we didn't have one at all before, we probably should do.

Fixes SYN-788

@erikjohnston
Copy link
Member

You'll want to merge in develop

@dbkr
Copy link
Member Author

dbkr commented Oct 14, 2016

done

@erikjohnston
Copy link
Member

Looks like sqlite doesn't like your index sql

@dbkr
Copy link
Member Author

dbkr commented Oct 14, 2016

Huh... it was fine on mine. :/ Maybe an sqlite version thing?

@dbkr
Copy link
Member Author

dbkr commented Oct 14, 2016

Yep: https://www.sqlite.org/expridx.html - so we need version 3.9.0. Sigh. What do we want to do?

@erikjohnston
Copy link
Member

The usual trick is to store the lower cased email in the database, so that the normal index can be used. This also neatly side steps the issues where not all 3pids are necessarily case insensitive.

@dbkr
Copy link
Member Author

dbkr commented Oct 14, 2016

Hmm, true. I guess we could have a db script to migrate the current ones. wdyt?

@erikjohnston
Copy link
Member

Given it can be done as a simple update, if there aren't too many rows its probably easiest just to do the update in a .sql delta

older sqlite doesn't support indexes on expressions, lets just
store things lowercase in the db
And db migration sql to convert existing addresses.
@erikjohnston
Copy link
Member

LGTM

@erikjohnston
Copy link
Member

Oh, except you'll want the delta to be under 37/

@dbkr dbkr merged commit 7d2cf7e into develop Oct 19, 2016
@richvdh richvdh deleted the dbkr/password_reset_case_insensitive branch December 1, 2016 14:09
dbkr added a commit that referenced this pull request Jan 18, 2017
Since we store all emails in the DB in lowercase
(#1170)
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.

None yet

2 participants