diff --git a/server/account.go b/server/account.go index daeffef..6537fc5 100644 --- a/server/account.go +++ b/server/account.go @@ -54,7 +54,7 @@ func (s *Server) register(w http.ResponseWriter, req *http.Request) { var registerResponse RegisterResponse - var token auth.VerifyTokenString + var token *auth.VerifyTokenString modes: switch verificationMode { @@ -78,7 +78,8 @@ modes: case env.AccountVerificationModeEmailVerify: // Not verified until they click their email link. registerResponse.Verified = false - token, err = s.auth.NewVerifyTokenString() + newToken, err := s.auth.NewVerifyTokenString() + token = &newToken if err != nil { internalServiceErrorJson(w, err, "Error generating verify token string") @@ -102,8 +103,8 @@ modes: return } - if len(token) > 0 { - err = s.mail.SendVerificationEmail(registerRequest.Email, token) + if token != nil { + err = s.mail.SendVerificationEmail(registerRequest.Email, *token) } if err != nil { diff --git a/server/account_test.go b/server/account_test.go index 855d44b..cdce6ac 100644 --- a/server/account_test.go +++ b/server/account_test.go @@ -243,7 +243,7 @@ func TestServerRegisterAccountVerification(t *testing.T) { if testStore.Called.CreateAccount == nil { t.Fatalf("Expected CreateAccount to be called") } - tokenPassedIn := testStore.Called.CreateAccount.VerifyToken != "" + tokenPassedIn := testStore.Called.CreateAccount.VerifyToken != nil if tc.expectedVerified && tokenPassedIn { t.Errorf("Expected new account to be verified, thus expected verifyToken *not to be passed in* to call to CreateAccount.") } diff --git a/server/server_test.go b/server/server_test.go index 8508248..d434eed 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -90,7 +90,7 @@ type CreateAccountCall struct { Email auth.Email Password auth.Password ClientSaltSeed auth.ClientSaltSeed - VerifyToken auth.VerifyTokenString + VerifyToken *auth.VerifyTokenString } // Whether functions are called, and sometimes what they're called with @@ -154,7 +154,7 @@ func (s *TestStore) GetUserId(auth.Email, auth.Password) (auth.UserId, error) { return 0, s.Errors.GetUserId } -func (s *TestStore) CreateAccount(email auth.Email, password auth.Password, seed auth.ClientSaltSeed, verifyToken auth.VerifyTokenString) error { +func (s *TestStore) CreateAccount(email auth.Email, password auth.Password, seed auth.ClientSaltSeed, verifyToken *auth.VerifyTokenString) error { s.Called.CreateAccount = &CreateAccountCall{ Email: email, Password: password, diff --git a/store/account_test.go b/store/account_test.go index aa4181f..7a3ef23 100644 --- a/store/account_test.go +++ b/store/account_test.go @@ -18,14 +18,14 @@ func expectAccountMatch( expectedEmail auth.Email, password auth.Password, seed auth.ClientSaltSeed, - expectedVerifyTokenString auth.VerifyTokenString, + expectedVerifyTokenString *auth.VerifyTokenString, approxVerifyExpiration *time.Time, ) { var key auth.KDFKey var salt auth.ServerSalt var email auth.Email var verifyExpiration *time.Time - var verifyTokenString auth.VerifyTokenString + var verifyTokenString *auth.VerifyTokenString err := s.db.QueryRow( `SELECT key, server_salt, email, verify_token, verify_expiration from accounts WHERE normalized_email=? AND client_salt_seed=?`, @@ -47,14 +47,24 @@ func expectAccountMatch( t.Fatalf("Email case not as expected. Want: %s Got: %s", email, expectedEmail) } - if verifyTokenString != expectedVerifyTokenString { + if (verifyTokenString == nil) != (expectedVerifyTokenString == nil) { t.Fatalf( - "Verify token string not as expected. Want: %s Got: %s", - expectedVerifyTokenString, - verifyTokenString, + "Verify token string nil-ness not as expected. Want: %v Got: %v", + expectedVerifyTokenString == nil, + verifyTokenString == nil, ) } + if expectedVerifyTokenString != nil { + if *verifyTokenString != *expectedVerifyTokenString { + t.Fatalf( + "Verify token string not as expected. Want: %s Got: %s", + *expectedVerifyTokenString, + *verifyTokenString, + ) + } + } + if approxVerifyExpiration != nil { if verifyExpiration == nil { t.Fatalf("Expected verify expiration to not be nil") @@ -104,18 +114,18 @@ func TestStoreCreateAccount(t *testing.T) { // Create an account. Make it verified (i.e. no token) for the usual // case. We'll test unverified (with token) separately. - if err := s.CreateAccount(email, password, seed, ""); err != nil { + if err := s.CreateAccount(email, password, seed, nil); err != nil { t.Fatalf("Unexpected error in CreateAccount: %+v", err) } // Get and confirm the account we just put in - expectAccountMatch(t, &s, normEmail, email, password, seed, "", nil) + expectAccountMatch(t, &s, normEmail, email, password, seed, nil, nil) newPassword := auth.Password("xyz") // Try to create a new account with the same email and different password, // fail because email already exists - if err := s.CreateAccount(email, newPassword, seed, ""); err != ErrDuplicateAccount { + if err := s.CreateAccount(email, newPassword, seed, nil); err != ErrDuplicateAccount { t.Fatalf(`CreateAccount err: wanted "%+v", got "%+v"`, ErrDuplicateAccount, err) } @@ -123,43 +133,79 @@ func TestStoreCreateAccount(t *testing.T) { // 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 { + if err := s.CreateAccount(differentCaseEmail, password, seed, nil); 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, normEmail, email, password, seed, "", nil) + expectAccountMatch(t, &s, normEmail, email, password, seed, nil, nil) } -// Test that I can call CreateAccount twice -// Try CreateAccount twice with different emails and different password, succeed both times -// This is in response to https://github.com/lbryio/wallet-sync-server/issues/13 -func TestStoreCreateTwoAccounts(t *testing.T) { +// Test that I can use CreateAccount twice for different emails with no veriy token +// This is related to https://github.com/lbryio/wallet-sync-server/issues/13 +func TestStoreCreateAccountTwoVerifiedSucceed(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - email, normEmail := auth.Email("Abc@Example.Com"), auth.NormalizedEmail("abc@example.com") - password, seed := auth.Password("123"), auth.ClientSaltSeed("abcd1234abcd1234") + email1, normEmail1 := auth.Email("Abc@Example.Com"), auth.NormalizedEmail("abc@example.com") + password1, seed1 := auth.Password("123"), auth.ClientSaltSeed("abcd1234abcd1234") - // Create an account. Make it verified (i.e. no token) for the usual + email2, normEmail2 := auth.Email("Abc2@Example.Com"), auth.NormalizedEmail("abc2@example.com") + password2, seed2 := auth.Password("123"), auth.ClientSaltSeed("abcd1234abcd1234") + + // Create a couple accounts. Don't care if they have the same password. + // Make them verified (i.e. no token) for the usual // case. We'll test unverified (with token) separately. - if err := s.CreateAccount(email, password, seed, ""); err != nil { + if err := s.CreateAccount(email1, password1, seed1, nil); err != nil { t.Fatalf("Unexpected error in CreateAccount: %+v", err) } - email, normEmail = auth.Email("Abc2@Example.Com"), auth.NormalizedEmail("abc2@example.com") - password, seed = auth.Password("123"), auth.ClientSaltSeed("abcd1234abcd1234") - - // Create another account. Don't care if it has the same password as the first one. - // Make it verified (i.e. no token) for the usual - // case. We'll test unverified (with token) separately. - if err := s.CreateAccount(email, password, seed, ""); err != nil { + if err := s.CreateAccount(email2, password2, seed2, nil); err != nil { t.Fatalf("Unexpected error in CreateAccount: %+v", err) } - // Get and confirm the account we just put in - expectAccountMatch(t, &s, normEmail, email, password, seed, "", nil) + // Get and confirm the accounts we just put in + expectAccountMatch(t, &s, normEmail1, email1, password1, seed1, nil, nil) + expectAccountMatch(t, &s, normEmail2, email2, password2, seed2, nil, nil) +} +// Test that I cannot use CreateAccount twice with the same verify token, but +// I can with different verify tokens +// This is related to https://github.com/lbryio/wallet-sync-server/issues/13 +func TestStoreCreateAccountTwoSameVerfiyTokenFail(t *testing.T) { + s, sqliteTmpFile := StoreTestInit(t) + defer StoreTestCleanup(sqliteTmpFile) + + email1, normEmail1 := auth.Email("Abc@Example.Com"), auth.NormalizedEmail("abc@example.com") + password1, seed1 := auth.Password("123"), auth.ClientSaltSeed("abcd1234abcd1234") + verifyToken1 := auth.VerifyTokenString("abcd1234abcd1234abcd1234abcd1234") + + email2, normEmail2 := auth.Email("Abc2@Example.Com"), auth.NormalizedEmail("abc2@example.com") + password2, seed2 := auth.Password("xyz"), auth.ClientSaltSeed("abcd1234abcd1234") + verifyToken2 := auth.VerifyTokenString("00001234abcd1234abcd123400000000") + + // Create the first account + if err := s.CreateAccount(email1, password1, seed1, &verifyToken1); err != nil { + t.Fatalf("Unexpected error in CreateAccount: %+v", err) + } + + // Try to create the second account with the same verify token, fail + if err := s.CreateAccount(email2, password2, seed2, &verifyToken1); err != ErrDuplicateAccount { + t.Fatalf(`CreateAccount err: wanted "%+v", got "%+v"`, ErrDuplicateAccount, err) + } + + // Confirm that it didn't save + expectAccountNotExists(t, &s, normEmail2) + + // Create the second account with a different verify token + if err := s.CreateAccount(email2, password2, seed2, &verifyToken2); err != nil { + t.Fatalf("Unexpected error in CreateAccount: %+v", err) + } + + // Get and confirm the accounts we just put in + approxVerifyExpiration := time.Now().Add(time.Hour * 24 * 2).UTC() + expectAccountMatch(t, &s, normEmail1, email1, password1, seed1, &verifyToken1, &approxVerifyExpiration) + expectAccountMatch(t, &s, normEmail2, email2, password2, seed2, &verifyToken2, &approxVerifyExpiration) } // Try CreateAccount with a verification string, thus unverified @@ -171,13 +217,14 @@ func TestStoreCreateAccountUnverified(t *testing.T) { password, seed := auth.Password("123"), auth.ClientSaltSeed("abcd1234abcd1234") // Create an account - if err := s.CreateAccount(email, password, seed, "abcd1234abcd1234abcd1234abcd1234"); err != nil { + verifyToken := auth.VerifyTokenString("abcd1234abcd1234abcd1234abcd1234") + if err := s.CreateAccount(email, password, seed, &verifyToken); err != nil { t.Fatalf("Unexpected error in CreateAccount: %+v", err) } // Get and confirm the account we just put in approxVerifyExpiration := time.Now().Add(time.Hour * 24 * 2).UTC() - expectAccountMatch(t, &s, normEmail, email, password, seed, "abcd1234abcd1234abcd1234abcd1234", &approxVerifyExpiration) + expectAccountMatch(t, &s, normEmail, email, password, seed, &verifyToken, &approxVerifyExpiration) } // Test GetUserId for nonexisting email @@ -197,7 +244,7 @@ func TestStoreGetUserIdAccountExists(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - createdUserId, email, password, _ := makeTestUser(t, &s, "", nil) + createdUserId, email, password, _ := makeTestUser(t, &s, nil, nil) // Check that the userId is correct for the email, irrespective of the case of // the characters in the email. @@ -225,7 +272,8 @@ func TestStoreGetUserIdAccountUnverified(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - _, email, password, _ := makeTestUser(t, &s, "abcd1234abcd1234abcd1234abcd1234", &time.Time{}) + verifyToken := auth.VerifyTokenString("abcd1234abcd1234abcd1234abcd1234") + _, email, password, _ := makeTestUser(t, &s, &verifyToken, &time.Time{}) // Check that it won't return if the account is unverified if userId, err := s.GetUserId(email, password); err != ErrNotVerified || userId != 0 { @@ -264,7 +312,7 @@ func TestStoreAccountEmptyFields(t *testing.T) { var sqliteErr sqlite3.Error - err := s.CreateAccount(tc.email, tc.password, tc.clientSaltSeed, "") + err := s.CreateAccount(tc.email, tc.password, tc.clientSaltSeed, nil) if errors.As(err, &sqliteErr) { if errors.Is(sqliteErr.ExtendedCode, sqlite3.ErrConstraintCheck) { return // We got the error we expected @@ -280,7 +328,7 @@ func TestStoreGetClientSaltSeedAccountExists(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - _, email, _, createdSeed := makeTestUser(t, &s, "", nil) + _, email, _, createdSeed := makeTestUser(t, &s, nil, nil) // Check that the seed is correct for the email, irrespective of the case of // the characters in the email. @@ -315,7 +363,7 @@ func TestUpdateVerifyTokenStringSuccess(t *testing.T) { verifyTokenString1 := auth.VerifyTokenString("00000000000000000000000000000000") time1 := time.Time{} - _, email, password, createdSeed := makeTestUser(t, &s, verifyTokenString1, &time1) + _, email, password, createdSeed := makeTestUser(t, &s, &verifyTokenString1, &time1) // we're not testing normalization features so we'll just use this here normEmail := email.Normalize() @@ -332,12 +380,12 @@ func TestUpdateVerifyTokenStringSuccess(t *testing.T) { if err := s.UpdateVerifyTokenString(lowerEmail, verifyTokenString2); err != nil { t.Fatalf("Unexpected error in UpdateVerifyTokenString: err: %+v", err) } - expectAccountMatch(t, &s, normEmail, email, password, createdSeed, verifyTokenString2, &approxVerifyExpiration) + expectAccountMatch(t, &s, normEmail, email, password, createdSeed, &verifyTokenString2, &approxVerifyExpiration) if err := s.UpdateVerifyTokenString(upperEmail, verifyTokenString3); err != nil { t.Fatalf("Unexpected error in UpdateVerifyTokenString: err: %+v", err) } - expectAccountMatch(t, &s, normEmail, email, password, createdSeed, verifyTokenString3, &approxVerifyExpiration) + expectAccountMatch(t, &s, normEmail, email, password, createdSeed, &verifyTokenString3, &approxVerifyExpiration) } // Test UpdateVerifyTokenString for nonexisting email @@ -357,7 +405,7 @@ func TestStoreUpdateVerifyTokenStringAccountVerified(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - _, email, _, _ := makeTestUser(t, &s, "", nil) + _, email, _, _ := makeTestUser(t, &s, nil, nil) if err := s.UpdateVerifyTokenString(email, "abcd1234abcd1234abcd1234abcd1234"); err != ErrNoTokenForUser { t.Fatalf(`UpdateVerifyTokenString error for already verified account: wanted "%+v", got "%+v."`, ErrNoTokenForUser, err) @@ -372,7 +420,7 @@ func TestUpdateVerifyAccountSuccess(t *testing.T) { verifyTokenString := auth.VerifyTokenString("abcd1234abcd1234abcd1234abcd1234") time1 := time.Time{} - _, email, password, createdSeed := makeTestUser(t, &s, verifyTokenString, &time1) + _, email, password, createdSeed := makeTestUser(t, &s, &verifyTokenString, &time1) // we're not testing normalization features so we'll just use this here normEmail := email.Normalize() @@ -380,7 +428,7 @@ func TestUpdateVerifyAccountSuccess(t *testing.T) { if err := s.VerifyAccount(verifyTokenString); err != nil { t.Fatalf("Unexpected error in VerifyAccount: err: %+v", err) } - expectAccountMatch(t, &s, normEmail, email, password, createdSeed, "", nil) + expectAccountMatch(t, &s, normEmail, email, password, createdSeed, nil, nil) } // Test VerifyAccount for nonexisting token diff --git a/store/auth_test.go b/store/auth_test.go index 3686d16..b47ebdb 100644 --- a/store/auth_test.go +++ b/store/auth_test.go @@ -77,7 +77,7 @@ func TestStoreInsertToken(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _, _ := makeTestUser(t, &s, "", nil) + userId, _, _, _ := makeTestUser(t, &s, nil, nil) // created for addition to the DB (no expiration attached) authToken1 := auth.AuthToken{ @@ -123,7 +123,7 @@ func TestStoreUpdateToken(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _, _ := makeTestUser(t, &s, "", nil) + userId, _, _, _ := makeTestUser(t, &s, nil, nil) // created for addition to the DB (no expiration attached) authTokenUpdate := auth.AuthToken{ @@ -181,7 +181,7 @@ func TestStoreSaveToken(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _, _ := makeTestUser(t, &s, "", nil) + userId, _, _, _ := makeTestUser(t, &s, nil, nil) // Version 1 of the token for both devices // created for addition to the DB (no expiration attached) @@ -274,7 +274,7 @@ func TestStoreGetToken(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _, _ := makeTestUser(t, &s, "", nil) + userId, _, _, _ := makeTestUser(t, &s, nil, nil) // created for addition to the DB (no expiration attached) authToken := auth.AuthToken{ @@ -327,7 +327,7 @@ func TestStoreTokenUTC(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _, _ := makeTestUser(t, &s, "", nil) + userId, _, _, _ := makeTestUser(t, &s, nil, nil) authToken := auth.AuthToken{ Token: "seekrit-1", @@ -394,7 +394,7 @@ func TestStoreTokenEmptyFields(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - tc.authToken.UserId, _, _, _ = makeTestUser(t, &s, "", nil) + tc.authToken.UserId, _, _, _ = makeTestUser(t, &s, nil, nil) var sqliteErr sqlite3.Error diff --git a/store/password_test.go b/store/password_test.go index bde9355..e5ca535 100644 --- a/store/password_test.go +++ b/store/password_test.go @@ -16,7 +16,7 @@ func TestStoreChangePasswordSuccess(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, email, oldPassword, _ := makeTestUser(t, &s, "", nil) + userId, email, oldPassword, _ := makeTestUser(t, &s, nil, nil) token := auth.AuthTokenString("my-token") _, err := s.db.Exec( @@ -47,7 +47,7 @@ func TestStoreChangePasswordSuccess(t *testing.T) { t.Errorf("ChangePasswordWithWallet (lower case email): unexpected error: %+v", err) } - expectAccountMatch(t, &s, email.Normalize(), email, newPassword, newSeed, "", nil) + expectAccountMatch(t, &s, email.Normalize(), email, newPassword, newSeed, nil, nil) expectWalletExists(t, &s, userId, encryptedWallet, sequence, hmac) expectTokenNotExists(t, &s, token) @@ -63,17 +63,18 @@ func TestStoreChangePasswordSuccess(t *testing.T) { t.Errorf("ChangePasswordWithWallet (upper case email): unexpected error: %+v", err) } - expectAccountMatch(t, &s, email.Normalize(), email, newNewPassword, newNewSeed, "", nil) + expectAccountMatch(t, &s, email.Normalize(), email, newNewPassword, newNewSeed, nil, nil) } func TestStoreChangePasswordErrors(t *testing.T) { + verifyToken := auth.VerifyTokenString("aoeu1234aoeu1234aoeu1234aoeu1234") tt := []struct { name string hasWallet bool sequence wallet.Sequence emailSuffix auth.Email oldPasswordSuffix auth.Password - verifyToken auth.VerifyTokenString + verifyToken *auth.VerifyTokenString verifyExpiration *time.Time expectedError error }{ @@ -90,7 +91,7 @@ func TestStoreChangePasswordErrors(t *testing.T) { sequence: wallet.Sequence(2), // sequence is correct emailSuffix: auth.Email(""), // the email is correct oldPasswordSuffix: auth.Password(""), // the password is correct - verifyToken: auth.VerifyTokenString("aoeu1234aoeu1234aoeu1234aoeu1234"), + verifyToken: &verifyToken, verifyExpiration: &time.Time{}, expectedError: ErrNotVerified, }, { @@ -187,7 +188,7 @@ func TestStoreChangePasswordNoWalletSuccess(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, email, oldPassword, _ := makeTestUser(t, &s, "", nil) + userId, email, oldPassword, _ := makeTestUser(t, &s, nil, nil) token := auth.AuthTokenString("my-token") _, err := s.db.Exec( @@ -207,7 +208,7 @@ func TestStoreChangePasswordNoWalletSuccess(t *testing.T) { t.Errorf("ChangePasswordNoWallet (lower case email): unexpected error: %+v", err) } - expectAccountMatch(t, &s, email.Normalize(), email, newPassword, newSeed, "", nil) + expectAccountMatch(t, &s, email.Normalize(), email, newPassword, newSeed, nil, nil) expectWalletNotExists(t, &s, userId) expectTokenNotExists(t, &s, token) @@ -220,16 +221,18 @@ func TestStoreChangePasswordNoWalletSuccess(t *testing.T) { t.Errorf("ChangePasswordNoWallet (upper case email): unexpected error: %+v", err) } - expectAccountMatch(t, &s, email.Normalize(), email, newNewPassword, newNewSeed, "", nil) + expectAccountMatch(t, &s, email.Normalize(), email, newNewPassword, newNewSeed, nil, nil) } func TestStoreChangePasswordNoWalletErrors(t *testing.T) { + verifyToken := auth.VerifyTokenString("aoeu1234aoeu1234aoeu1234aoeu1234") + tt := []struct { name string hasWallet bool emailSuffix auth.Email oldPasswordSuffix auth.Password - verifyToken auth.VerifyTokenString + verifyToken *auth.VerifyTokenString verifyExpiration *time.Time expectedError error }{ @@ -250,7 +253,7 @@ func TestStoreChangePasswordNoWalletErrors(t *testing.T) { hasWallet: false, // we don't have the wallet, as expected for this function emailSuffix: auth.Email(""), // the email is correct oldPasswordSuffix: auth.Password(""), // the password is correct - verifyToken: auth.VerifyTokenString("aoeu1234aoeu1234aoeu1234aoeu1234"), + verifyToken: &verifyToken, verifyExpiration: &time.Time{}, expectedError: ErrNotVerified, }, { diff --git a/store/store.go b/store/store.go index 05afde7..08baae8 100644 --- a/store/store.go +++ b/store/store.go @@ -46,7 +46,7 @@ type StoreInterface interface { SetWallet(auth.UserId, wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac) error GetWallet(auth.UserId) (wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac, error) GetUserId(auth.Email, auth.Password) (auth.UserId, error) - CreateAccount(auth.Email, auth.Password, auth.ClientSaltSeed, auth.VerifyTokenString) error + CreateAccount(auth.Email, auth.Password, auth.ClientSaltSeed, *auth.VerifyTokenString) error UpdateVerifyTokenString(auth.Email, auth.VerifyTokenString) error VerifyAccount(auth.VerifyTokenString) error ChangePasswordWithWallet(auth.Email, auth.Password, auth.Password, auth.ClientSaltSeed, wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac) error @@ -125,7 +125,13 @@ func (s *Store) Migrate() error { key TEXT NOT NULL, client_salt_seed TEXT NOT NULL, server_salt TEXT NOT NULL, - verify_token TEXT NOT NULL UNIQUE, -- will query by token when verifying + + -- UNIQUE because we will query by token when verifying + -- + -- Nullable because we want to use null to represent verified users. We can't use empty string + -- because multiple accounts with empty string will trigger the unique constraint, unlike null. + verify_token TEXT UNIQUE, + verify_expiration DATETIME, user_id INTEGER PRIMARY KEY AUTOINCREMENT, CHECK ( @@ -358,7 +364,7 @@ func (s *Store) GetUserId(email auth.Email, password auth.Password) (userId auth var verified bool err = s.db.QueryRow( - `SELECT user_id, key, server_salt, verify_token="" from accounts WHERE normalized_email=?`, + `SELECT user_id, key, server_salt, verify_token is null from accounts WHERE normalized_email=?`, email.Normalize(), ).Scan(&userId, &key, &salt, &verified) if err == sql.ErrNoRows { @@ -383,14 +389,14 @@ func (s *Store) GetUserId(email auth.Email, password auth.Password) (userId auth // Account // ///////////// -func (s *Store) CreateAccount(email auth.Email, password auth.Password, seed auth.ClientSaltSeed, verifyToken auth.VerifyTokenString) (err error) { +func (s *Store) CreateAccount(email auth.Email, password auth.Password, seed auth.ClientSaltSeed, verifyToken *auth.VerifyTokenString) (err error) { key, salt, err := password.Create() if err != nil { return } var verifyExpiration *time.Time - if len(verifyToken) > 0 { + if verifyToken != nil { verifyExpiration = new(time.Time) *verifyExpiration = time.Now().UTC().Add(VerifyTokenLifespan) } @@ -420,7 +426,7 @@ func (s *Store) UpdateVerifyTokenString(email auth.Email, verifyTokenString auth expiration := time.Now().UTC().Add(VerifyTokenLifespan) res, err := s.db.Exec( - `UPDATE accounts SET verify_token=?, verify_expiration=? WHERE normalized_email=? and verify_token!=""`, + `UPDATE accounts SET verify_token=?, verify_expiration=? WHERE normalized_email=? and verify_token is not null`, verifyTokenString, expiration, email.Normalize(), ) if err != nil { @@ -451,8 +457,8 @@ func (s *Store) UpdateVerifyTokenString(email auth.Email, verifyTokenString auth func (s *Store) VerifyAccount(verifyTokenString auth.VerifyTokenString) (err error) { res, err := s.db.Exec( - "UPDATE accounts SET verify_token=?, verify_expiration=? WHERE verify_token=?", - "", nil, verifyTokenString, + "UPDATE accounts SET verify_token=null, verify_expiration=null WHERE verify_token=?", + verifyTokenString, ) if err != nil { return @@ -553,7 +559,7 @@ func (s *Store) changePassword( var verified bool err = tx.QueryRow( - `SELECT user_id, key, server_salt, verify_token="" from accounts WHERE normalized_email=?`, + `SELECT user_id, key, server_salt, verify_token is null from accounts WHERE normalized_email=?`, email.Normalize(), ).Scan(&userId, &oldKey, &oldSalt, &verified) if err == sql.ErrNoRows { diff --git a/store/store_test.go b/store/store_test.go index e7f235c..e78b429 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -37,7 +37,7 @@ func StoreTestCleanup(tmpFile *os.File) { func makeTestUser( t *testing.T, s *Store, - verifyToken auth.VerifyTokenString, + verifyToken *auth.VerifyTokenString, verifyExpiration *time.Time, ) (userId auth.UserId, email auth.Email, password auth.Password, seed auth.ClientSaltSeed) { // email with caps to trigger possible problems diff --git a/store/wallet_test.go b/store/wallet_test.go index d9c27cb..512eb4b 100644 --- a/store/wallet_test.go +++ b/store/wallet_test.go @@ -86,7 +86,7 @@ func TestStoreInsertWallet(t *testing.T) { defer StoreTestCleanup(sqliteTmpFile) // Get a valid userId - userId, _, _, _ := makeTestUser(t, &s, "", nil) + userId, _, _, _ := makeTestUser(t, &s, nil, nil) // Get a wallet, come back empty expectWalletNotExists(t, &s, userId) @@ -118,7 +118,7 @@ func TestStoreUpdateWallet(t *testing.T) { defer StoreTestCleanup(sqliteTmpFile) // Get a valid userId - userId, _, _, _ := makeTestUser(t, &s, "", nil) + userId, _, _, _ := makeTestUser(t, &s, nil, nil) // Try to update a wallet, fail for nothing to update if err := s.updateWalletToSequence(userId, wallet.EncryptedWallet("my-enc-wallet-a"), wallet.Sequence(1), wallet.WalletHmac("my-hmac-a")); err != ErrNoWallet { @@ -174,7 +174,7 @@ func TestStoreSetWallet(t *testing.T) { defer StoreTestCleanup(sqliteTmpFile) // Get a valid userId - userId, _, _, _ := makeTestUser(t, &s, "", nil) + userId, _, _, _ := makeTestUser(t, &s, nil, nil) // Sequence 2 - fails - out of sequence (behind the scenes, tries to update but there's nothing there yet) if err := s.SetWallet(userId, wallet.EncryptedWallet("my-enc-wallet-a"), wallet.Sequence(2), wallet.WalletHmac("my-hmac-a")); err != ErrWrongSequence { @@ -221,7 +221,7 @@ func TestStoreGetWallet(t *testing.T) { defer StoreTestCleanup(sqliteTmpFile) // Get a valid userId - userId, _, _, _ := makeTestUser(t, &s, "", nil) + userId, _, _, _ := makeTestUser(t, &s, nil, nil) // GetWallet fails when there's no wallet encryptedWallet, sequence, hmac, err := s.GetWallet(userId) @@ -264,7 +264,7 @@ func TestStoreWalletEmptyFields(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _, _ := makeTestUser(t, &s, "", nil) + userId, _, _, _ := makeTestUser(t, &s, nil, nil) var sqliteErr sqlite3.Error