From aee351a2b15c12fabc4d36416270f2c1490cd8fa Mon Sep 17 00:00:00 2001
From: Daniel Krol <orblivion@gmail.com>
Date: Tue, 26 Jul 2022 11:18:43 -0400
Subject: [PATCH] Don't allow password change for unverified accounts

Mainly because wallet change is tied up in it
---
 server/password.go      | 19 +++++++++++++++++++
 server/password_test.go | 24 ++++++++++++++++++++++++
 store/store.go          |  2 +-
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/server/password.go b/server/password.go
index 5c3ee78..5192a00 100644
--- a/server/password.go
+++ b/server/password.go
@@ -53,6 +53,21 @@ func (s *Server) changePassword(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
+	// To be cautious, we will block password changes for unverified accounts.
+	// The only reason I can think of for allowing them is if the user
+	// accidentally put in a bad password that they desperately want to change,
+	// and the verification email isn't working. However unlikely such a scenario
+	// is, with the salting and the KDF and all that, it seems all the less a big
+	// deal.
+	//
+	// Changing a password when unverified as such isn't a big deal, but I'm
+	// concerned with wallet creation. This endpoint currently doesn't allow you
+	// to _create_ a wallet if you don't already have one, so as of now we don't
+	// strictly need this restriction. However this seems too precarious and
+	// tricky. We might forget about it and allow wallet creation here later.
+	// Someone might find a loophole I'm not thinking of. So I'm just blocking
+	// unverified accounts here for simplicity.
+
 	var err error
 	if changePasswordRequest.EncryptedWallet != "" {
 		err = s.store.ChangePasswordWithWallet(
@@ -83,6 +98,10 @@ func (s *Server) changePassword(w http.ResponseWriter, req *http.Request) {
 		errorJson(w, http.StatusUnauthorized, "No match for email and password")
 		return
 	}
+	if err == store.ErrNotVerified {
+		errorJson(w, http.StatusUnauthorized, "Account is not verified")
+		return
+	}
 	if err != nil {
 		internalServiceErrorJson(w, err, "Error changing password")
 		return
diff --git a/server/password_test.go b/server/password_test.go
index 11dc456..52f2cca 100644
--- a/server/password_test.go
+++ b/server/password_test.go
@@ -103,6 +103,30 @@ func TestServerChangePassword(t *testing.T) {
 			email: "abc@example.com",
 
 			storeErrors: TestStoreFunctionsErrors{ChangePasswordNoWallet: store.ErrWrongCredentials},
+		}, {
+			name:                "unverified account with wallet",
+			expectedStatusCode:  http.StatusUnauthorized,
+			expectedErrorString: http.StatusText(http.StatusUnauthorized) + ": Account is not verified",
+
+			expectChangePasswordCall: true,
+
+			newEncryptedWallet: "my-enc-wallet",
+			newSequence:        2,
+			newHmac:            "my-hmac",
+
+			email: "abc@example.com",
+
+			storeErrors: TestStoreFunctionsErrors{ChangePasswordWithWallet: store.ErrNotVerified},
+		}, {
+			name:                "unverified account no wallet",
+			expectedStatusCode:  http.StatusUnauthorized,
+			expectedErrorString: http.StatusText(http.StatusUnauthorized) + ": Account is not verified",
+
+			expectChangePasswordCall: true,
+
+			email: "abc@example.com",
+
+			storeErrors: TestStoreFunctionsErrors{ChangePasswordNoWallet: store.ErrNotVerified},
 		}, {
 			name:                "validation error",
 			expectedStatusCode:  http.StatusBadRequest,
diff --git a/store/store.go b/store/store.go
index dd7f575..eaa4033 100644
--- a/store/store.go
+++ b/store/store.go
@@ -30,7 +30,7 @@ var (
 	ErrDuplicateAccount = fmt.Errorf("User already has an account")
 
 	ErrWrongCredentials = fmt.Errorf("No match for email and password")
-	ErrNotVerified = fmt.Errorf("User account is not verified")
+	ErrNotVerified      = fmt.Errorf("User account is not verified")
 )
 
 // For test stubs