Merge pull request #542 from openziti/password-reset-fix

Fix for multiple password reset requests per account (#452)

CHANGELOG.md

@@ -8,6 +8,8 @@ CHANGE: Creating a reserved share checks for token collision and returns a more
 
 CHANGE: Update UI to add a 'true' value on `reserved` boolean (https://github.com/openziti/zrok/issues/443)
 
+FIX: Fixed bug where a second password reset request would for any account would fail (https://github.com/openziti/zrok/issues/452)
+
 ## v0.4.24
 
 FEATURE: New `socks` backend mode for use with private sharing. Use `zrok share private --backend-mode socks` and then `zrok access private` that share from somewhere else... very lightweight VPN-like functionality (https://github.com/openziti/zrok/issues/558)

controller/store/password_reset_request.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/jmoiron/sqlx"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 type PasswordResetRequest struct {
@@ -17,7 +18,11 @@ type PasswordResetRequest struct {
 }
 
 func (str *Store) CreatePasswordResetRequest(prr *PasswordResetRequest, tx *sqlx.Tx) (int, error) {
-	stmt, err := tx.Prepare("insert into password_reset_requests (account_id, token) values ($1, $2) ON CONFLICT(account_id) DO UPDATE SET token=$2 returning id")
+	if err := str.DeletePasswordResetRequestsByAccountId(prr.AccountId, tx); err != nil {
+		logrus.Errorf("unable to delete old password reset requests for account '%v', but continuing: %v", prr.AccountId, err)
+	}
+
+	stmt, err := tx.Prepare("insert into password_reset_requests (account_id, token) values ($1, $2) returning id")
 	if err != nil {
 		return 0, errors.Wrap(err, "error preparing password_reset_requests insert statement")
 	}
@@ -98,3 +103,15 @@ func (str *Store) DeleteMultiplePasswordResetRequests(ids []int, tx *sqlx.Tx) er
 	}
 	return nil
 }
+
+func (str *Store) DeletePasswordResetRequestsByAccountId(accountId int, tx *sqlx.Tx) error {
+	stmt, err := tx.Prepare("update password_reset_requests set updated_at = current_timestamp, deleted = true where account_id = $1")
+	if err != nil {
+		return errors.Wrap(err, "error preparing password_reset_requests delete by account_id statement")
+	}
+	_, err = stmt.Exec(accountId)
+	if err != nil {
+		return errors.Wrap(err, "error executing password_reset_requests delete by account_id statement")
+	}
+	return nil
+}

controller/store/sql/postgresql/018_v0_4_23_password_reset_request_unique.sql

@@ -0,0 +1,7 @@
+-- +migrate Up
+
+-- remove the old unique index (users might need multiple password resets)
+ALTER TABLE password_reset_requests DROP CONSTRAINT password_reset_requests_account_id_key;
+
+-- add new constraint which doesnt mind having multiple resets for account ids
+ALTER TABLE password_reset_requests ADD CONSTRAINT password_reset_requests_account_id_key FOREIGN KEY (account_id) REFERENCES accounts (id);

controller/store/sql/sqlite3/018_v0_4_23_password_reset_request_unique.sql

@@ -0,0 +1,17 @@
+-- +migrate Up
+
+alter table password_reset_requests rename to password_reset_requests_old;
+
+CREATE TABLE password_reset_requests (
+  id                    integer             primary key,
+  token                 string              not null unique,
+  created_at            datetime            not null default(strftime('%Y-%m-%d %H:%M:%f', 'now')),
+  updated_at            datetime            not null default(strftime('%Y-%m-%d %H:%M:%f', 'now')),
+  account_id            integer             not null constraint fk_accounts_password_reset_requests references accounts,
+  deleted               boolean             not null default(false),
+
+  constraint chk_token check(token <> '')
+);
+
+insert into password_reset_requests select * from password_reset_requests_old;
+drop table password_reset_requests_old;