Create and test password change method

Includes updating the wallet if the user has one
This commit is contained in:
Daniel Krol 2022-06-30 18:44:59 -04:00
parent f8e5ada1ee
commit 37fae6436b
6 changed files with 271 additions and 31 deletions

View file

@ -9,7 +9,7 @@ import (
"orblivion/lbry-id/auth"
)
func expectAccountExists(t *testing.T, s *Store, email auth.Email, password auth.Password) {
func expectAccountMatch(t *testing.T, s *Store, email auth.Email, password auth.Password) {
rows, err := s.db.Query(
`SELECT 1 from accounts WHERE email=? AND password=?`,
email, password.Obfuscate(),
@ -26,7 +26,7 @@ func expectAccountExists(t *testing.T, s *Store, email auth.Email, password auth
t.Fatalf("Expected account for: %s %s", email, password)
}
func expectAccountNotExists(t *testing.T, s *Store, email auth.Email, password auth.Password) {
func expectAccountNotMatch(t *testing.T, s *Store, email auth.Email, password auth.Password) {
rows, err := s.db.Query(
`SELECT 1 from accounts WHERE email=? AND password=?`,
email, password.Obfuscate(),
@ -52,7 +52,7 @@ func TestStoreCreateAccount(t *testing.T) {
email, password := auth.Email("abc@example.com"), auth.Password("123")
// Get an account, come back empty
expectAccountNotExists(t, &s, email, password)
expectAccountNotMatch(t, &s, email, password)
// Create an account
if err := s.CreateAccount(email, password); err != nil {
@ -60,7 +60,7 @@ func TestStoreCreateAccount(t *testing.T) {
}
// Get and confirm the account we just put in
expectAccountExists(t, &s, email, password)
expectAccountMatch(t, &s, email, password)
newPassword := auth.Password("xyz")
@ -71,8 +71,8 @@ func TestStoreCreateAccount(t *testing.T) {
}
// Get the email and same *first* password we successfully put in, but not the second
expectAccountExists(t, &s, email, password)
expectAccountNotExists(t, &s, email, newPassword)
expectAccountMatch(t, &s, email, password)
expectAccountNotMatch(t, &s, email, newPassword)
}
// Test GetUserId, using CreateAccount as a helper
@ -84,8 +84,8 @@ func TestStoreGetUserId(t *testing.T) {
email, password := auth.Email("abc@example.com"), auth.Password("123")
// Check that there's no user id for email and password first
if userId, err := s.GetUserId(email, password); err != ErrNoUId || userId != 0 {
t.Fatalf(`CreateAccount err: wanted "%+v", got "%+v. userId: %v"`, ErrNoUId, err, userId)
if userId, err := s.GetUserId(email, password); err != ErrWrongCredentials || userId != 0 {
t.Fatalf(`CreateAccount err: wanted "%+v", got "%+v. userId: %v"`, ErrWrongCredentials, err, userId)
}
// Create the account

150
store/password_test.go Normal file
View file

@ -0,0 +1,150 @@
package store
import (
"testing"
"time"
"orblivion/lbry-id/auth"
"orblivion/lbry-id/wallet"
)
func TestStoreChangePasswordSuccess(t *testing.T) {
s, sqliteTmpFile := StoreTestInit(t)
defer StoreTestCleanup(sqliteTmpFile)
userId, email, oldPassword := makeTestUser(t, &s)
token := auth.TokenString("my-token")
_, err := s.db.Exec(
"INSERT INTO auth_tokens (token, user_id, device_id, scope, expiration) VALUES(?,?,?,?,?)",
token, userId, "my-dev-id", "*", time.Now().UTC().Add(time.Hour*24*14),
)
if err != nil {
t.Fatalf("Error creating token")
}
_, err = s.db.Exec(
"INSERT INTO wallets (user_id, encrypted_wallet, sequence, hmac) VALUES(?,?,?,?)",
userId, "my-enc-wallet", 1, "my-hmac",
)
if err != nil {
t.Fatalf("Error creating test wallet")
}
newPassword := oldPassword + auth.Password("_new")
encryptedWallet := wallet.EncryptedWallet("my-enc-wallet-2")
sequence := wallet.Sequence(2)
hmac := wallet.WalletHmac("my-hmac-2")
if err := s.ChangePassword(email, oldPassword, newPassword, encryptedWallet, sequence, hmac); err != nil {
t.Errorf("ChangePassword: unexpected error: %+v", err)
}
expectAccountMatch(t, &s, email, newPassword)
expectWalletExists(t, &s, userId, encryptedWallet, sequence, hmac)
expectTokenNotExists(t, &s, token)
}
func TestStoreChangePasswordErrors(t *testing.T) {
tt := []struct {
name string
hasWallet bool
sequence wallet.Sequence
emailSuffix auth.Email
oldPasswordSuffix auth.Password
expectedError error
}{
{
name: "wrong email",
hasWallet: true, // we have the requisite wallet
sequence: wallet.Sequence(2), // sequence is correct
emailSuffix: auth.Email("_wrong"), // the email is *incorrect*
oldPasswordSuffix: auth.Password(""), // the password is correct
expectedError: ErrWrongCredentials,
}, {
name: "wrong old password",
hasWallet: true, // we have the requisite wallet
sequence: wallet.Sequence(2), // sequence is correct
emailSuffix: auth.Email(""), // the email is correct
oldPasswordSuffix: auth.Password("_wrong"), // the old password is *incorrect*
expectedError: ErrWrongCredentials,
}, {
name: "wrong sequence",
hasWallet: true, // we have the requisite wallet
sequence: wallet.Sequence(3), // sequence is *incorrect*
emailSuffix: auth.Email(""), // the email is correct
oldPasswordSuffix: auth.Password(""), // the password is correct
expectedError: ErrWrongSequence,
}, {
name: "no wallet to replace",
hasWallet: false, // we have the requisite wallet
sequence: wallet.Sequence(1), // sequence is correct (for there being no wallets)
emailSuffix: auth.Email(""), // the email is correct
oldPasswordSuffix: auth.Password(""), // the password is correct
// Maybe ErrNoWallet once we sort it out. See TODO item by its definition.
expectedError: ErrWrongSequence,
},
}
for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
s, sqliteTmpFile := StoreTestInit(t)
defer StoreTestCleanup(sqliteTmpFile)
userId, email, oldPassword := makeTestUser(t, &s)
expiration := time.Now().UTC().Add(time.Hour * 24 * 14)
authToken := auth.AuthToken{
Token: auth.TokenString("my-token"),
DeviceId: auth.DeviceId("my-dev-id"),
UserId: userId,
Scope: auth.AuthScope("*"),
Expiration: &expiration,
}
_, err := s.db.Exec(
"INSERT INTO auth_tokens (token, user_id, device_id, scope, expiration) VALUES(?,?,?,?,?)",
authToken.Token, authToken.UserId, authToken.DeviceId, authToken.Scope, authToken.Expiration,
)
if err != nil {
t.Fatalf("Error creating token")
}
oldEncryptedWallet := wallet.EncryptedWallet("my-enc-wallet-old")
newEncryptedWallet := wallet.EncryptedWallet("my-enc-wallet-new")
oldHmac := wallet.WalletHmac("my-hmac-old")
newHmac := wallet.WalletHmac("my-hmac-new")
oldSequence := wallet.Sequence(1)
if tc.hasWallet {
_, err := s.db.Exec(
"INSERT INTO wallets (user_id, encrypted_wallet, sequence, hmac) VALUES(?,?,?,?)",
userId, oldEncryptedWallet, oldSequence, oldHmac,
)
if err != nil {
t.Fatalf("Error creating test wallet")
}
}
submittedEmail := email + tc.emailSuffix // Possibly make it the wrong email
submittedOldPassword := oldPassword + tc.oldPasswordSuffix // Possibly make it the wrong password
newPassword := oldPassword + auth.Password("_new") // Possibly make the new password different (as it should be)
if err := s.ChangePassword(submittedEmail, submittedOldPassword, newPassword, newEncryptedWallet, tc.sequence, newHmac); err != tc.expectedError {
t.Errorf("ChangePassword: unexpected value for err. want: %+v, got: %+v", tc.expectedError, err)
}
// The password and wallet didn't change, the token didn't get 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)
if tc.hasWallet {
expectWalletExists(t, &s, userId, oldEncryptedWallet, oldSequence, oldHmac)
} else {
expectWalletNotExists(t, &s, userId)
}
expectTokenExists(t, &s, authToken)
})
}
}

View file

@ -20,14 +20,18 @@ var (
ErrNoToken = fmt.Errorf("Token does not exist for this user and device")
ErrDuplicateWallet = fmt.Errorf("Wallet already exists for this user")
ErrNoWallet = fmt.Errorf("Wallet does not exist for this user at this sequence")
// TODO - the use of this error message is a bit confusing. The phrasing is
// not correct for GetWallet. And maybe the other uses probably should just
// return ErrWrongSequence to begin with. But think about it.
ErrNoWallet = fmt.Errorf("Wallet does not exist for this user at this sequence")
ErrWrongSequence = fmt.Errorf("Wallet could not be updated to this sequence")
ErrDuplicateEmail = fmt.Errorf("Email already exists for this user")
ErrDuplicateAccount = fmt.Errorf("User already has an account")
ErrNoUId = fmt.Errorf("User Id not found with these credentials")
ErrWrongCredentials = fmt.Errorf("No match for email and password")
)
// For test stubs
@ -162,7 +166,7 @@ func (s *Store) GetToken(token auth.TokenString) (*auth.AuthToken, error) {
func (s *Store) insertToken(authToken *auth.AuthToken, expiration time.Time) (err error) {
_, err = s.db.Exec(
"INSERT INTO auth_tokens (token, user_id, device_id, scope, expiration) values(?,?,?,?,?)",
"INSERT INTO auth_tokens (token, user_id, device_id, scope, expiration) VALUES(?,?,?,?,?)",
authToken.Token, authToken.UserId, authToken.DeviceId, authToken.Scope, expiration,
)
@ -199,6 +203,7 @@ func (s *Store) updateToken(authToken *auth.AuthToken, experation time.Time) (er
func (s *Store) SaveToken(token *auth.AuthToken) (err error) {
// TODO: For psql, do upsert here instead of separate insertToken and updateToken functions
// Actually it may even be available for SQLite?
// TODO - Should we auto-delete expired tokens?
@ -261,7 +266,7 @@ func (s *Store) insertFirstWallet(
// The database will enforce that this will not be set if this user already
// has a wallet.
_, err = s.db.Exec(
"INSERT INTO wallets (user_id, encrypted_wallet, sequence, hmac) values(?,?,?,?)",
"INSERT INTO wallets (user_id, encrypted_wallet, sequence, hmac) VALUES(?,?,?,?)",
userId, encryptedWallet, 1, hmac,
)
@ -347,7 +352,7 @@ func (s *Store) GetUserId(email auth.Email, password auth.Password) (userId auth
err = rows.Scan(&userId)
return
}
err = ErrNoUId
err = ErrWrongCredentials
return
}
@ -358,7 +363,7 @@ func (s *Store) GetUserId(email auth.Email, password auth.Password) (userId auth
func (s *Store) CreateAccount(email auth.Email, password auth.Password) (err error) {
// userId auto-increments
_, err = s.db.Exec(
"INSERT INTO accounts (email, password) values(?,?)",
"INSERT INTO accounts (email, password) VALUES(?,?)",
email, password.Obfuscate(),
)
@ -371,3 +376,90 @@ func (s *Store) CreateAccount(email auth.Email, password auth.Password) (err err
return
}
// Change password. For the user, this requires changing their root password,
// which changes the encryption key for the wallet as well. Thus, we should
// update the wallet at the same time to avoid ever having a situation where
// these two don't match.
//
// Also delete all auth tokens to force clients to update their root password
// to get a new token. This prevents other clients from posting a wallet
// encrypted with the old key.
func (s *Store) ChangePassword(
email auth.Email,
oldPassword auth.Password,
newPassword auth.Password,
encryptedWallet wallet.EncryptedWallet,
sequence wallet.Sequence,
hmac wallet.WalletHmac,
) (err error) {
var userId auth.UserId
tx, err := s.db.Begin()
if err != nil {
return
}
// Lots of error conditions. Just defer this. However, we need to make sure to
// make sure the variable `err` is set to the error before we return, instead
// of doing `return <error>`.
endTxn := func() {
if err != nil {
tx.Rollback()
} else {
tx.Commit()
}
}
defer endTxn()
err = tx.QueryRow(
"SELECT user_id from accounts WHERE email=? AND password=?",
email, oldPassword.Obfuscate(),
).Scan(&userId)
if err == sql.ErrNoRows {
err = ErrWrongCredentials
return
}
if err != nil {
return
}
res, err := tx.Exec(
"UPDATE accounts SET password=? WHERE user_id=?",
newPassword.Obfuscate(), userId,
)
if err != nil {
return
}
numRows, err := res.RowsAffected()
if err != nil {
return
}
if numRows != 1 {
// Very unexpected error!
err = fmt.Errorf("Password failed to update")
return
}
res, err = tx.Exec(
`UPDATE wallets SET encrypted_wallet=?, sequence=?, hmac=?
WHERE user_id=? AND sequence=?`,
encryptedWallet, sequence, hmac, userId, sequence-1,
)
if err != nil {
return
}
numRows, err = res.RowsAffected()
if err != nil {
return
}
if numRows != 1 {
err = ErrWrongSequence
return
}
// Don't care how many I delete here. Might even be zero. No login token while
// changing password seems plausible.
_, err = tx.Exec("DELETE FROM auth_tokens WHERE user_id=?", userId)
return
}

View file

@ -33,8 +33,8 @@ func StoreTestCleanup(tmpFile *os.File) {
}
}
func makeTestUserId(t *testing.T, s *Store) auth.UserId {
email, password := auth.Email("abc@example.com"), auth.Password("123")
func makeTestUser(t *testing.T, s *Store) (userId auth.UserId, email auth.Email, password auth.Password) {
email, password = auth.Email("abc@example.com"), auth.Password("123")
rows, err := s.db.Query(
"INSERT INTO accounts (email, password) values(?,?) returning user_id",
@ -45,13 +45,12 @@ func makeTestUserId(t *testing.T, s *Store) auth.UserId {
}
defer rows.Close()
for rows.Next() {
var userId auth.UserId
err := rows.Scan(&userId)
if err != nil {
t.Fatalf("Error setting up account")
}
return userId
return
}
t.Fatalf("Error setting up account")
return auth.UserId(0)
return
}

View file

@ -77,7 +77,7 @@ func TestStoreInsertToken(t *testing.T) {
s, sqliteTmpFile := StoreTestInit(t)
defer StoreTestCleanup(sqliteTmpFile)
userId := makeTestUserId(t, &s)
userId, _, _ := makeTestUser(t, &s)
// 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 := makeTestUserId(t, &s)
userId, _, _ := makeTestUser(t, &s)
// 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 := makeTestUserId(t, &s)
userId, _, _ := makeTestUser(t, &s)
// Version 1 of the token for both devices
// created for addition to the DB (no expiration attached)
@ -262,7 +262,7 @@ func TestStoreGetToken(t *testing.T) {
s, sqliteTmpFile := StoreTestInit(t)
defer StoreTestCleanup(sqliteTmpFile)
userId := makeTestUserId(t, &s)
userId, _, _ := makeTestUser(t, &s)
// created for addition to the DB (no expiration attached)
authToken := auth.AuthToken{
@ -315,7 +315,7 @@ func TestStoreTokenUTC(t *testing.T) {
s, sqliteTmpFile := StoreTestInit(t)
defer StoreTestCleanup(sqliteTmpFile)
userId := makeTestUserId(t, &s)
userId, _, _ := makeTestUser(t, &s)
authToken := auth.AuthToken{
Token: "seekrit-1",
@ -353,7 +353,6 @@ func TestStoreTokenUTC(t *testing.T) {
}
func TestStoreTokenEmptyFields(t *testing.T) {
// Make sure expiration doesn't get set if sanitization fails
tt := []struct {
name string
authToken auth.AuthToken
@ -383,7 +382,7 @@ func TestStoreTokenEmptyFields(t *testing.T) {
s, sqliteTmpFile := StoreTestInit(t)
defer StoreTestCleanup(sqliteTmpFile)
tc.authToken.UserId = makeTestUserId(t, &s)
tc.authToken.UserId, _, _ = makeTestUser(t, &s)
var sqliteErr sqlite3.Error

View file

@ -86,7 +86,7 @@ func TestStoreInsertWallet(t *testing.T) {
defer StoreTestCleanup(sqliteTmpFile)
// Get a valid userId
userId := makeTestUserId(t, &s)
userId, _, _ := makeTestUser(t, &s)
// 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 := makeTestUserId(t, &s)
userId, _, _ := makeTestUser(t, &s)
// 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 := makeTestUserId(t, &s)
userId, _, _ := makeTestUser(t, &s)
// 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 := makeTestUserId(t, &s)
userId, _, _ := makeTestUser(t, &s)
// 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 := makeTestUserId(t, &s)
userId, _, _ := makeTestUser(t, &s)
var sqliteErr sqlite3.Error