admin: clear TOTP secret when expiring password (#231)

fixes #225
keratin · Dec 7, 2023 · 90fc454 · 90fc454
1 parent 0bffb11
commit 90fc454
Show file tree

Hide file tree

Showing 7 changed files with 9 additions and 6 deletions.
diff --git a/app/data/mock/account_store.go b/app/data/mock/account_store.go
@@ -174,6 +174,8 @@ func (s *accountStore) RequireNewPassword(id int) (bool, error) {
 
 	account.RequireNewPassword = true
 	account.UpdatedAt = time.Now()
+	account.TOTPSecret = sql.NullString{}
+
 	return true, nil
 }
 

diff --git a/app/data/mysql/account_store.go b/app/data/mysql/account_store.go
@@ -119,7 +119,7 @@ func (db *AccountStore) Unlock(id int) (bool, error) {
 }
 
 func (db *AccountStore) RequireNewPassword(id int) (bool, error) {
-	result, err := db.Exec("UPDATE accounts SET require_new_password = ?, updated_at = ? WHERE id = ?", true, time.Now(), id)
+	result, err := db.Exec("UPDATE accounts SET require_new_password = ?, updated_at = ?, totp_secret = null WHERE id = ?", true, time.Now(), id)
 	return ok(result, err)
 }
 

diff --git a/app/data/postgres/account_store.go b/app/data/postgres/account_store.go
@@ -137,7 +137,7 @@ func (db *AccountStore) Unlock(id int) (bool, error) {
 }
 
 func (db *AccountStore) RequireNewPassword(id int) (bool, error) {
-	result, err := db.Exec("UPDATE accounts SET require_new_password = $1, updated_at = $2 WHERE id = $3", true, time.Now(), id)
+	result, err := db.Exec("UPDATE accounts SET require_new_password = $1, updated_at = $2, totp_secret = null WHERE id = $3", true, time.Now(), id)
 	return ok(result, err)
 }
 

diff --git a/app/data/sqlite3/account_store.go b/app/data/sqlite3/account_store.go
@@ -119,7 +119,7 @@ func (db *AccountStore) Unlock(id int) (bool, error) {
 }
 
 func (db *AccountStore) RequireNewPassword(id int) (bool, error) {
-	result, err := db.Exec("UPDATE accounts SET require_new_password = ?, updated_at = ? WHERE id = ?", true, time.Now(), id)
+	result, err := db.Exec("UPDATE accounts SET require_new_password = ?, updated_at = ?, totp_secret = null WHERE id = ?", true, time.Now(), id)
 	return ok(result, err)
 }
 

diff --git a/app/services/password_expirer_test.go b/app/services/password_expirer_test.go
@@ -27,6 +27,7 @@ func TestPasswordExpirer(t *testing.T) {
 		account, err = accountStore.Find(account.ID)
 		require.NoError(t, err)
 		assert.NotEmpty(t, account.RequireNewPassword)
+		assert.False(t, account.TOTPSecret.Valid)
 
 		id, err := refreshStore.Find(token1)
 		require.NoError(t, err)

diff --git a/app/services/password_resetter_test.go b/app/services/password_resetter_test.go
@@ -165,10 +165,10 @@ func TestPasswordResetterWithOTP(t *testing.T) {
 	t.Run("without totp code", func(t *testing.T) {
 		expired, err := accountStore.Create("second@keratin.tech", []byte("old"))
 		require.NoError(t, err)
-		_, err = accountStore.SetTOTPSecret(expired.ID, totpSecretEnc)
-		require.NoError(t, err)
 		_, err = accountStore.RequireNewPassword(expired.ID)
 		require.NoError(t, err)
+		_, err = accountStore.SetTOTPSecret(expired.ID, totpSecretEnc)
+		require.NoError(t, err)
 
 		err = invoke(newToken(expired.ID, expired.PasswordChangedAt), "0a0b0c0d0e0f", "12345")
 		assert.Equal(t, services.FieldErrors{{"otp", "INVALID_OR_EXPIRED"}}, err)

diff --git a/docs/api.md b/docs/api.md
@@ -555,7 +555,7 @@ Visibility: Private
 | ------ | ---- | ----- |
 | `id` | integer | available from the JWT `sub` claim |
 
-Revokes all of the user's current sessions and flags the account for a required password change on their next login. This will manifest as an expired credentials error on what would normally have been a successful login.
+Revokes all of the user's current sessions, removes their TOTP secret and flags the account for a required password change on their next login. This will manifest as an expired credentials error on what would normally have been a successful login.
 
 #### Success: