No password change for unverified accounts

This commit is contained in:
Daniel Krol 2022-07-30 23:09:33 -04:00
parent 4a205bbda7
commit eabfa9d54c
4 changed files with 33 additions and 8 deletions

View file

@ -50,8 +50,8 @@ func expectAccountMatch(
if verifyTokenString != expectedVerifyTokenString { if verifyTokenString != expectedVerifyTokenString {
t.Fatalf( t.Fatalf(
"Verify token string not as expected. Want: %s Got: %s", "Verify token string not as expected. Want: %s Got: %s",
verifyTokenString,
expectedVerifyTokenString, expectedVerifyTokenString,
verifyTokenString,
) )
} }

View file

@ -73,6 +73,8 @@ func TestStoreChangePasswordErrors(t *testing.T) {
sequence wallet.Sequence sequence wallet.Sequence
emailSuffix auth.Email emailSuffix auth.Email
oldPasswordSuffix auth.Password oldPasswordSuffix auth.Password
verifyToken auth.VerifyTokenString
verifyExpiration *time.Time
expectedError error expectedError error
}{ }{
{ {
@ -82,6 +84,15 @@ func TestStoreChangePasswordErrors(t *testing.T) {
emailSuffix: auth.Email("_wrong"), // the email is *incorrect* emailSuffix: auth.Email("_wrong"), // the email is *incorrect*
oldPasswordSuffix: auth.Password(""), // the password is correct oldPasswordSuffix: auth.Password(""), // the password is correct
expectedError: ErrWrongCredentials, expectedError: ErrWrongCredentials,
}, {
name: "unverified account",
hasWallet: true, // we have the requisite wallet (even though it should be impossible for an unverified account)
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"),
verifyExpiration: &time.Time{},
expectedError: ErrNotVerified,
}, { }, {
name: "wrong old password", name: "wrong old password",
hasWallet: true, // we have the requisite wallet hasWallet: true, // we have the requisite wallet
@ -114,7 +125,7 @@ func TestStoreChangePasswordErrors(t *testing.T) {
s, sqliteTmpFile := StoreTestInit(t) s, sqliteTmpFile := StoreTestInit(t)
defer StoreTestCleanup(sqliteTmpFile) defer StoreTestCleanup(sqliteTmpFile)
userId, email, oldPassword, oldSeed := makeTestUser(t, &s, "", nil) userId, email, oldPassword, oldSeed := makeTestUser(t, &s, tc.verifyToken, tc.verifyExpiration)
expiration := time.Now().UTC().Add(time.Hour * 24 * 14) expiration := time.Now().UTC().Add(time.Hour * 24 * 14)
authToken := auth.AuthToken{ authToken := auth.AuthToken{
Token: auth.AuthTokenString("my-token"), Token: auth.AuthTokenString("my-token"),
@ -161,7 +172,7 @@ func TestStoreChangePasswordErrors(t *testing.T) {
// This tests the transaction rollbacks in particular, given the errors // This tests the transaction rollbacks in particular, given the errors
// that are at a couple different stages of the txn, triggered by these // that are at a couple different stages of the txn, triggered by these
// tests. // tests.
expectAccountMatch(t, &s, email.Normalize(), email, oldPassword, oldSeed, "", nil) expectAccountMatch(t, &s, email.Normalize(), email, oldPassword, oldSeed, tc.verifyToken, tc.verifyExpiration)
if tc.hasWallet { if tc.hasWallet {
expectWalletExists(t, &s, userId, oldEncryptedWallet, oldSequence, oldHmac) expectWalletExists(t, &s, userId, oldEncryptedWallet, oldSequence, oldHmac)
} else { } else {
@ -218,6 +229,8 @@ func TestStoreChangePasswordNoWalletErrors(t *testing.T) {
hasWallet bool hasWallet bool
emailSuffix auth.Email emailSuffix auth.Email
oldPasswordSuffix auth.Password oldPasswordSuffix auth.Password
verifyToken auth.VerifyTokenString
verifyExpiration *time.Time
expectedError error expectedError error
}{ }{
{ {
@ -232,6 +245,14 @@ func TestStoreChangePasswordNoWalletErrors(t *testing.T) {
emailSuffix: auth.Email(""), // the email is correct emailSuffix: auth.Email(""), // the email is correct
oldPasswordSuffix: auth.Password("_wrong"), // the old password is *incorrect* oldPasswordSuffix: auth.Password("_wrong"), // the old password is *incorrect*
expectedError: ErrWrongCredentials, expectedError: ErrWrongCredentials,
}, {
name: "unverified account",
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"),
verifyExpiration: &time.Time{},
expectedError: ErrNotVerified,
}, { }, {
name: "unexpected wallet", name: "unexpected wallet",
hasWallet: true, // we have a wallet which we shouldn't have at this point hasWallet: true, // we have a wallet which we shouldn't have at this point
@ -246,7 +267,7 @@ func TestStoreChangePasswordNoWalletErrors(t *testing.T) {
s, sqliteTmpFile := StoreTestInit(t) s, sqliteTmpFile := StoreTestInit(t)
defer StoreTestCleanup(sqliteTmpFile) defer StoreTestCleanup(sqliteTmpFile)
userId, email, oldPassword, oldSeed := makeTestUser(t, &s, "", nil) userId, email, oldPassword, oldSeed := makeTestUser(t, &s, tc.verifyToken, tc.verifyExpiration)
expiration := time.Now().UTC().Add(time.Hour * 24 * 14) expiration := time.Now().UTC().Add(time.Hour * 24 * 14)
authToken := auth.AuthToken{ authToken := auth.AuthToken{
Token: auth.AuthTokenString("my-token"), Token: auth.AuthTokenString("my-token"),
@ -292,7 +313,7 @@ func TestStoreChangePasswordNoWalletErrors(t *testing.T) {
// deleted. This tests the transaction rollbacks in particular, given the // deleted. This tests the transaction rollbacks in particular, given the
// errors that are at a couple different stages of the txn, triggered by // errors that are at a couple different stages of the txn, triggered by
// these tests. // these tests.
expectAccountMatch(t, &s, email.Normalize(), email, oldPassword, oldSeed, "", nil) expectAccountMatch(t, &s, email.Normalize(), email, oldPassword, oldSeed, tc.verifyToken, tc.verifyExpiration)
if tc.hasWallet { if tc.hasWallet {
expectWalletExists(t, &s, userId, encryptedWallet, sequence, hmac) expectWalletExists(t, &s, userId, encryptedWallet, sequence, hmac)
} else { } else {

View file

@ -488,11 +488,12 @@ func (s *Store) changePassword(
var oldKey auth.KDFKey var oldKey auth.KDFKey
var oldSalt auth.ServerSalt var oldSalt auth.ServerSalt
var verified bool
err = tx.QueryRow( err = tx.QueryRow(
`SELECT user_id, key, server_salt from accounts WHERE normalized_email=?`, `SELECT user_id, key, server_salt, verify_token="" from accounts WHERE normalized_email=?`,
email.Normalize(), email.Normalize(),
).Scan(&userId, &oldKey, &oldSalt) ).Scan(&userId, &oldKey, &oldSalt, &verified)
if err == sql.ErrNoRows { if err == sql.ErrNoRows {
err = ErrWrongCredentials err = ErrWrongCredentials
} }
@ -503,6 +504,9 @@ func (s *Store) changePassword(
if err == nil && !match { if err == nil && !match {
err = ErrWrongCredentials err = ErrWrongCredentials
} }
if err == nil && !verified {
err = ErrNotVerified
}
if err != nil { if err != nil {
return return
} }

View file

@ -51,7 +51,7 @@ func makeTestUser(
seed = auth.ClientSaltSeed("abcd1234abcd1234") seed = auth.ClientSaltSeed("abcd1234abcd1234")
rows, err := s.db.Query( rows, err := s.db.Query(
"INSERT INTO accounts (normalized_email, email, key, server_salt, client_salt_seed, verify_token) values(?,?,?,?,?,?) returning user_id", "INSERT INTO accounts (normalized_email, email, key, server_salt, client_salt_seed, verify_token, verify_expiration) values(?,?,?,?,?,?,?) returning user_id",
normEmail, email, key, salt, seed, verifyToken, verifyExpiration, normEmail, email, key, salt, seed, verifyToken, verifyExpiration,
) )
if err != nil { if err != nil {