Make emails case insensitive (for now).

Prevents duplicate accounts. Also allows case insensitive search (user id, salt seed, etc) while still having an index. This is done by storing normalized as a separate field from originally formated (which we'll use for sending emails, etc).
This commit is contained in:
Daniel Krol 2022-07-21 21:19:24 -04:00
parent f10cc8aa26
commit aefda1245b
6 changed files with 120 additions and 38 deletions

View file

@ -4,12 +4,14 @@ import (
"crypto/rand"
"encoding/hex"
"fmt"
"strings"
"time"
"golang.org/x/crypto/scrypt"
)
type UserId int32
type NormalizedEmail string // Should always contain a normalized value
type Email string
type DeviceId string
type Password string
@ -110,3 +112,10 @@ func (p Password) Check(checkKey KDFKey, salt ServerSalt) (match bool, err error
}
return
}
// TODO consider unicode. Also some providers might be case sensitive, and/or
// may have other ways of having email addresses be equivalent (which we may
// not care about though)
func (e Email) Normalize() NormalizedEmail {
return NormalizedEmail(strings.ToLower(string(e)))
}

View file

@ -123,3 +123,9 @@ func TestCheckPassword(t *testing.T) {
t.Error("Expected password check to fail with invalid salt")
}
}
func TestEmailNormalize(t *testing.T) {
if got, want := Email("aBc@eXaMpLe.CoM").Normalize(), NormalizedEmail("abc@example.com"); got != want {
t.Errorf("Email normalization failed. got: %s want: %s", got, want)
}
}

View file

@ -2,6 +2,7 @@ package store
import (
"errors"
"strings"
"testing"
"github.com/mattn/go-sqlite3"
@ -9,16 +10,17 @@ import (
"lbryio/lbry-id/auth"
)
func expectAccountMatch(t *testing.T, s *Store, email auth.Email, password auth.Password, seed auth.ClientSaltSeed) {
func expectAccountMatch(t *testing.T, s *Store, normEmail auth.NormalizedEmail, expectedEmail auth.Email, password auth.Password, seed auth.ClientSaltSeed) {
var key auth.KDFKey
var salt auth.ServerSalt
var email auth.Email
err := s.db.QueryRow(
`SELECT key, server_salt from accounts WHERE email=? AND client_salt_seed=?`,
email, seed,
).Scan(&key, &salt)
`SELECT key, server_salt, email from accounts WHERE normalized_email=? AND client_salt_seed=?`,
normEmail, seed,
).Scan(&key, &salt, &email)
if err != nil {
t.Fatalf("Error finding account for: %s %s - %+v", email, password, err)
t.Fatalf("Error finding account for: %s %s - %+v", normEmail, password, err)
}
match, err := password.Check(key, salt)
@ -26,22 +28,26 @@ func expectAccountMatch(t *testing.T, s *Store, email auth.Email, password auth.
t.Fatalf("Error checking password for: %s %s - %+v", email, password, err)
}
if !match {
t.Fatalf("Expected account for: %s %s", email, password)
t.Fatalf("Password incorrect for: %s %s", email, password)
}
if email != expectedEmail {
t.Fatalf("Email case not as expected. Want: %s Got: %s", email, expectedEmail)
}
}
func expectAccountNotExists(t *testing.T, s *Store, email auth.Email) {
func expectAccountNotExists(t *testing.T, s *Store, normEmail auth.NormalizedEmail) {
rows, err := s.db.Query(
`SELECT 1 from accounts WHERE email=?`,
email,
`SELECT 1 from accounts WHERE normalized_email=?`,
normEmail,
)
if err != nil {
t.Fatalf("Error finding account for: %s - %+v", email, err)
t.Fatalf("Error finding account for: %s - %+v", normEmail, err)
}
defer rows.Close()
for rows.Next() {
t.Fatalf("Expected no account for: %s", email)
t.Fatalf("Expected no account for: %s", normEmail)
}
// found nothing, we're good
@ -53,10 +59,11 @@ func TestStoreCreateAccount(t *testing.T) {
s, sqliteTmpFile := StoreTestInit(t)
defer StoreTestCleanup(sqliteTmpFile)
email, password, seed := auth.Email("abc@example.com"), auth.Password("123"), auth.ClientSaltSeed("abcd1234abcd1234")
email, normEmail := auth.Email("Abc@Example.Com"), auth.NormalizedEmail("abc@example.com")
password, seed := auth.Password("123"), auth.ClientSaltSeed("abcd1234abcd1234")
// Get an account, come back empty
expectAccountNotExists(t, &s, email)
expectAccountNotExists(t, &s, normEmail)
// Create an account
if err := s.CreateAccount(email, password, seed); err != nil {
@ -64,7 +71,7 @@ func TestStoreCreateAccount(t *testing.T) {
}
// Get and confirm the account we just put in
expectAccountMatch(t, &s, email, password, seed)
expectAccountMatch(t, &s, normEmail, email, password, seed)
newPassword := auth.Password("xyz")
@ -74,8 +81,16 @@ func TestStoreCreateAccount(t *testing.T) {
t.Fatalf(`CreateAccount err: wanted "%+v", got "%+v"`, ErrDuplicateAccount, err)
}
differentCaseEmail := auth.Email("aBC@examplE.CoM")
// Try to create a new account with the same email different capitalization.
// fail because email already exists
if err := s.CreateAccount(differentCaseEmail, password, seed); err != ErrDuplicateAccount {
t.Fatalf(`CreateAccount err (for case insensitivity check): wanted "%+v", got "%+v"`, ErrDuplicateAccount, err)
}
// Get the email and same *first* password we successfully put in
expectAccountMatch(t, &s, email, password, seed)
expectAccountMatch(t, &s, normEmail, email, password, seed)
}
// Test GetUserId for nonexisting email
@ -98,8 +113,18 @@ func TestStoreGetUserIdAccountExists(t *testing.T) {
createdUserId, email, password, _ := makeTestUser(t, &s)
// Check that the userId is correct for the email, irrespective of the case of
// the characters in the email.
lowerEmail := auth.Email(strings.ToLower(string(email)))
upperEmail := auth.Email(strings.ToUpper(string(email)))
// Check that there's now a user id for the email and password
if userId, err := s.GetUserId(email, password); err != nil || userId != createdUserId {
if userId, err := s.GetUserId(lowerEmail, password); err != nil || userId != createdUserId {
t.Fatalf("Unexpected error in GetUserId: err: %+v userId: %v", err, userId)
}
// Check that there's now a user id for the email and password
if userId, err := s.GetUserId(upperEmail, password); err != nil || userId != createdUserId {
t.Fatalf("Unexpected error in GetUserId: err: %+v userId: %v", err, userId)
}
@ -158,8 +183,15 @@ func TestStoreGetClientSaltSeedAccountSuccess(t *testing.T) {
_, email, _, createdSeed := makeTestUser(t, &s)
// Check that there's now a user id for the email
if seed, err := s.GetClientSaltSeed(email); err != nil || seed != createdSeed {
// Check that the seed is correct for the email, irrespective of the case of
// the characters in the email.
lowerEmail := auth.Email(strings.ToLower(string(email)))
upperEmail := auth.Email(strings.ToUpper(string(email)))
if seed, err := s.GetClientSaltSeed(lowerEmail); err != nil || seed != createdSeed {
t.Fatalf("Unexpected error in GetClientSaltSeed: err: %+v seed: %v", err, seed)
}
if seed, err := s.GetClientSaltSeed(upperEmail); err != nil || seed != createdSeed {
t.Fatalf("Unexpected error in GetClientSaltSeed: err: %+v seed: %v", err, seed)
}
}

View file

@ -1,6 +1,7 @@
package store
import (
"strings"
"testing"
"time"
@ -40,13 +41,29 @@ func TestStoreChangePasswordSuccess(t *testing.T) {
sequence := wallet.Sequence(2)
hmac := wallet.WalletHmac("my-hmac-2")
if err := s.ChangePasswordWithWallet(email, oldPassword, newPassword, newSeed, encryptedWallet, sequence, hmac); err != nil {
t.Errorf("ChangePasswordWithWallet: unexpected error: %+v", err)
lowerEmail := auth.Email(strings.ToLower(string(email)))
if err := s.ChangePasswordWithWallet(lowerEmail, oldPassword, newPassword, newSeed, encryptedWallet, sequence, hmac); err != nil {
t.Errorf("ChangePasswordWithWallet (lower case email): unexpected error: %+v", err)
}
expectAccountMatch(t, &s, email, newPassword, newSeed)
expectAccountMatch(t, &s, email.Normalize(), email, newPassword, newSeed)
expectWalletExists(t, &s, userId, encryptedWallet, sequence, hmac)
expectTokenNotExists(t, &s, token)
newNewPassword := newPassword + auth.Password("_new")
newNewSeed := auth.ClientSaltSeed("00008765edf98765edf98765edf98765edf98765edf98765edf98765edf98765")
newEncryptedWallet := wallet.EncryptedWallet("my-enc-wallet-3")
newSequence := wallet.Sequence(3)
newHmac := wallet.WalletHmac("my-hmac-3")
upperEmail := auth.Email(strings.ToUpper(string(email)))
if err := s.ChangePasswordWithWallet(upperEmail, newPassword, newNewPassword, newNewSeed, newEncryptedWallet, newSequence, newHmac); err != nil {
t.Errorf("ChangePasswordWithWallet (upper case email): unexpected error: %+v", err)
}
expectAccountMatch(t, &s, email.Normalize(), email, newNewPassword, newNewSeed)
}
func TestStoreChangePasswordErrors(t *testing.T) {
@ -144,7 +161,7 @@ func TestStoreChangePasswordErrors(t *testing.T) {
// This tests the transaction rollbacks in particular, given the errors
// that are at a couple different stages of the txn, triggered by these
// tests.
expectAccountMatch(t, &s, email, oldPassword, oldSeed)
expectAccountMatch(t, &s, email.Normalize(), email, oldPassword, oldSeed)
if tc.hasWallet {
expectWalletExists(t, &s, userId, oldEncryptedWallet, oldSequence, oldHmac)
} else {
@ -173,13 +190,26 @@ func TestStoreChangePasswordNoWalletSuccess(t *testing.T) {
newPassword := oldPassword + auth.Password("_new")
newSeed := auth.ClientSaltSeed("edf98765edf98765edf98765edf98765edf98765edf98765edf98765edf98765")
if err := s.ChangePasswordNoWallet(email, oldPassword, newPassword, newSeed); err != nil {
t.Errorf("ChangePasswordNoWallet: unexpected error: %+v", err)
lowerEmail := auth.Email(strings.ToLower(string(email)))
if err := s.ChangePasswordNoWallet(lowerEmail, oldPassword, newPassword, newSeed); err != nil {
t.Errorf("ChangePasswordNoWallet (lower case email): unexpected error: %+v", err)
}
expectAccountMatch(t, &s, email, newPassword, newSeed)
expectAccountMatch(t, &s, email.Normalize(), email, newPassword, newSeed)
expectWalletNotExists(t, &s, userId)
expectTokenNotExists(t, &s, token)
newNewPassword := newPassword + auth.Password("_new")
newNewSeed := auth.ClientSaltSeed("00008765edf98765edf98765edf98765edf98765edf98765edf98765edf98765")
upperEmail := auth.Email(strings.ToUpper(string(email)))
if err := s.ChangePasswordNoWallet(upperEmail, newPassword, newNewPassword, newNewSeed); err != nil {
t.Errorf("ChangePasswordNoWallet (upper case email): unexpected error: %+v", err)
}
expectAccountMatch(t, &s, email.Normalize(), email, newNewPassword, newNewSeed)
}
func TestStoreChangePasswordNoWalletErrors(t *testing.T) {
@ -262,7 +292,7 @@ func TestStoreChangePasswordNoWalletErrors(t *testing.T) {
// deleted. This tests the transaction rollbacks in particular, given the
// errors that are at a couple different stages of the txn, triggered by
// these tests.
expectAccountMatch(t, &s, email, oldPassword, oldSeed)
expectAccountMatch(t, &s, email.Normalize(), email, oldPassword, oldSeed)
if tc.hasWallet {
expectWalletExists(t, &s, userId, encryptedWallet, sequence, hmac)
} else {

View file

@ -111,13 +111,15 @@ func (s *Store) Migrate() error {
)
);
CREATE TABLE IF NOT EXISTS accounts(
email TEXT NOT NULL UNIQUE,
normalized_email TEXT NOT NULL UNIQUE,
email TEXT NOT NULL,
key TEXT NOT NULL,
client_salt_seed TEXT NOT NULL,
server_salt TEXT NOT NULL,
user_id INTEGER PRIMARY KEY AUTOINCREMENT,
CHECK (
email <> '' AND
normalized_email <> '' AND
key <> '' AND
client_salt_seed <> '' AND
server_salt <> ''
@ -336,8 +338,8 @@ func (s *Store) GetUserId(email auth.Email, password auth.Password) (userId auth
var key auth.KDFKey
var salt auth.ServerSalt
err = s.db.QueryRow(
`SELECT user_id, key, server_salt from accounts WHERE email=?`,
email,
`SELECT user_id, key, server_salt from accounts WHERE normalized_email=?`,
email.Normalize(),
).Scan(&userId, &key, &salt)
if err == sql.ErrNoRows {
err = ErrWrongCredentials
@ -362,10 +364,11 @@ func (s *Store) CreateAccount(email auth.Email, password auth.Password, seed aut
if err != nil {
return
}
// userId auto-increments
_, err = s.db.Exec(
"INSERT INTO accounts (email, key, server_salt, client_salt_seed) VALUES(?,?,?,?)",
email, key, salt, seed,
"INSERT INTO accounts (normalized_email, email, key, server_salt, client_salt_seed) VALUES(?,?,?,?,?)",
email.Normalize(), email, key, salt, seed,
)
var sqliteErr sqlite3.Error
if errors.As(err, &sqliteErr) {
@ -461,8 +464,8 @@ func (s *Store) changePassword(
var oldSalt auth.ServerSalt
err = tx.QueryRow(
`SELECT user_id, key, server_salt from accounts WHERE email=?`,
email,
`SELECT user_id, key, server_salt from accounts WHERE normalized_email=?`,
email.Normalize(),
).Scan(&userId, &oldKey, &oldSalt)
if err == sql.ErrNoRows {
err = ErrWrongCredentials
@ -543,8 +546,8 @@ func (s *Store) changePassword(
func (s *Store) GetClientSaltSeed(email auth.Email) (seed auth.ClientSaltSeed, err error) {
err = s.db.QueryRow(
`SELECT client_salt_seed from accounts WHERE email=?`,
email,
`SELECT client_salt_seed from accounts WHERE normalized_email=?`,
email.Normalize(),
).Scan(&seed)
if err == sql.ErrNoRows {
err = ErrWrongCredentials

View file

@ -34,7 +34,9 @@ func StoreTestCleanup(tmpFile *os.File) {
}
func makeTestUser(t *testing.T, s *Store) (userId auth.UserId, email auth.Email, password auth.Password, seed auth.ClientSaltSeed) {
email, password = auth.Email("abc@example.com"), auth.Password("123")
// email with caps to trigger possible problems
email, password = auth.Email("Abc@Example.Com"), auth.Password("123")
normEmail := auth.NormalizedEmail("abc@example.com")
key, salt, err := password.Create()
if err != nil {
t.Fatalf("Error creating password")
@ -43,8 +45,8 @@ func makeTestUser(t *testing.T, s *Store) (userId auth.UserId, email auth.Email,
seed = auth.ClientSaltSeed("abcd1234abcd1234")
rows, err := s.db.Query(
"INSERT INTO accounts (email, key, server_salt, client_salt_seed) values(?,?,?,?) returning user_id",
email, key, salt, seed,
"INSERT INTO accounts (normalized_email, email, key, server_salt, client_salt_seed) values(?,?,?,?,?) returning user_id",
normEmail, email, key, salt, seed,
)
if err != nil {
t.Fatalf("Error setting up account: %+v", err)