Change password endpoint implemented and tested

server/password.go

@@ -0,0 +1,84 @@
+package server
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+
+	"orblivion/lbry-id/auth"
+	"orblivion/lbry-id/store"
+	"orblivion/lbry-id/wallet"
+)
+
+type ChangePasswordRequest struct {
+	EncryptedWallet wallet.EncryptedWallet `json:"encryptedWallet"`
+	Sequence        wallet.Sequence        `json:"sequence"`
+	Hmac            wallet.WalletHmac      `json:"hmac"`
+	Email           auth.Email             `json:"email"`
+	OldPassword     auth.Password          `json:"oldPassword"`
+	NewPassword     auth.Password          `json:"newPassword"`
+}
+
+func (r *ChangePasswordRequest) validate() bool {
+	// The wallet should be here or not. Not partially here.
+	walletPresent := (r.EncryptedWallet != "" && r.Hmac != "" && r.Sequence > 0)
+	walletAbsent := (r.EncryptedWallet == "" && r.Hmac == "" && r.Sequence == 0)
+
+	return (validateEmail(r.Email) &&
+		r.OldPassword != "" &&
+		r.NewPassword != "" &&
+		r.OldPassword != r.NewPassword &&
+		(walletPresent || walletAbsent))
+}
+
+func (s *Server) changePassword(w http.ResponseWriter, req *http.Request) {
+	var changePasswordRequest ChangePasswordRequest
+	if !getPostData(w, req, &changePasswordRequest) {
+		return
+	}
+
+	var err error
+	if changePasswordRequest.EncryptedWallet != "" {
+		err = s.store.ChangePasswordWithWallet(
+			changePasswordRequest.Email,
+			changePasswordRequest.OldPassword,
+			changePasswordRequest.NewPassword,
+			changePasswordRequest.EncryptedWallet,
+			changePasswordRequest.Sequence,
+			changePasswordRequest.Hmac)
+		if err == store.ErrWrongSequence {
+			errorJson(w, http.StatusConflict, "Bad sequence number or wallet does not exist")
+			return
+		}
+	} else {
+		err = s.store.ChangePasswordNoWallet(
+			changePasswordRequest.Email,
+			changePasswordRequest.OldPassword,
+			changePasswordRequest.NewPassword,
+		)
+		if err == store.ErrUnexpectedWallet {
+			errorJson(w, http.StatusConflict, "Wallet exists; need an updated wallet when changing password")
+			return
+		}
+	}
+	if err == store.ErrWrongCredentials {
+		errorJson(w, http.StatusUnauthorized, "No match for email and password")
+		return
+	}
+	if err != nil {
+		internalServiceErrorJson(w, err, "Error changing password")
+		return
+	}
+
+	var changePasswordResponse struct{} // no data to respond with, but keep it JSON
+	var response []byte
+	response, err = json.Marshal(changePasswordResponse)
+
+	if err != nil {
+		internalServiceErrorJson(w, err, "Error generating change password response")
+		return
+	}
+
+	w.WriteHeader(http.StatusOK)
+	fmt.Fprintf(w, string(response))
+}

server/password_test.go

@@ -0,0 +1,350 @@
+package server
+
+import (
+	"bytes"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"orblivion/lbry-id/auth"
+	"orblivion/lbry-id/store"
+	"orblivion/lbry-id/wallet"
+)
+
+func TestServerChangePassword(t *testing.T) {
+	tt := []struct {
+		name string
+
+		expectedStatusCode  int
+		expectedErrorString string
+
+		// Whether we expect the call to ChangePassword*Wallet to happen
+		expectChangePasswordCall bool
+
+		// `new...` refers to what is being passed into the via POST request (and
+		//   what we expect to get passed into SetWallet for the *non-error* cases
+		//   below)
+		newEncryptedWallet wallet.EncryptedWallet
+		newSequence        wallet.Sequence
+		newHmac            wallet.WalletHmac
+
+		email auth.Email
+
+		storeErrors TestStoreFunctionsErrors
+	}{
+		{
+			name: "success with wallet",
+
+			expectedStatusCode: http.StatusOK,
+
+			expectChangePasswordCall: true,
+
+			newEncryptedWallet: "my-enc-wallet",
+			newSequence:        2,
+			newHmac:            "my-hmac",
+
+			email: "abc@example.com",
+		}, {
+			name: "success no wallet",
+
+			expectedStatusCode: http.StatusOK,
+
+			expectChangePasswordCall: true,
+
+			email: "abc@example.com",
+		}, {
+			name:                "conflict with wallet",
+			expectedStatusCode:  http.StatusConflict,
+			expectedErrorString: http.StatusText(http.StatusConflict) + ": Bad sequence number or wallet does not exist",
+
+			expectChangePasswordCall: true,
+
+			newEncryptedWallet: "my-enc-wallet",
+			newSequence:        2,
+			newHmac:            "my-hmac",
+
+			email: "abc@example.com",
+
+			storeErrors: TestStoreFunctionsErrors{ChangePasswordWithWallet: store.ErrWrongSequence},
+		}, {
+			name:                "conflict no wallet",
+			expectedStatusCode:  http.StatusConflict,
+			expectedErrorString: http.StatusText(http.StatusConflict) + ": Wallet exists; need an updated wallet when changing password",
+
+			expectChangePasswordCall: true,
+
+			email: "abc@example.com",
+
+			storeErrors: TestStoreFunctionsErrors{ChangePasswordNoWallet: store.ErrUnexpectedWallet},
+		}, {
+			name:                "incorrect email with wallet",
+			expectedStatusCode:  http.StatusUnauthorized,
+			expectedErrorString: http.StatusText(http.StatusUnauthorized) + ": No match for email and password",
+
+			expectChangePasswordCall: true,
+
+			newEncryptedWallet: "my-enc-wallet",
+			newSequence:        2,
+			newHmac:            "my-hmac",
+
+			email: "abc@example.com",
+
+			storeErrors: TestStoreFunctionsErrors{ChangePasswordWithWallet: store.ErrWrongCredentials},
+		}, {
+			name:                "incorrect email no wallet",
+			expectedStatusCode:  http.StatusUnauthorized,
+			expectedErrorString: http.StatusText(http.StatusUnauthorized) + ": No match for email and password",
+
+			expectChangePasswordCall: true,
+
+			email: "abc@example.com",
+
+			storeErrors: TestStoreFunctionsErrors{ChangePasswordNoWallet: store.ErrWrongCredentials},
+		}, {
+			name:                "validation error",
+			expectedStatusCode:  http.StatusBadRequest,
+			expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation",
+
+			// Just check one validation error (missing email address) to make sure
+			// the validate function is called. We'll check the rest of the
+			// validation errors in the other test below.
+
+			expectChangePasswordCall: false,
+		}, {
+			name:                     "db error changing password with wallet",
+			expectedStatusCode:       http.StatusInternalServerError,
+			expectedErrorString:      http.StatusText(http.StatusInternalServerError),
+			expectChangePasswordCall: true,
+
+			// Putting in valid data here so it's clear that this isn't what causes
+			// the error
+			newEncryptedWallet: "my-encrypted-wallet",
+			newSequence:        2,
+			newHmac:            "my-hmac",
+
+			email: "abc@example.com",
+
+			// What causes the error
+			storeErrors: TestStoreFunctionsErrors{ChangePasswordWithWallet: fmt.Errorf("Some random db problem")},
+		}, {
+			name:                     "db error changing password no wallet",
+			expectedStatusCode:       http.StatusInternalServerError,
+			expectedErrorString:      http.StatusText(http.StatusInternalServerError),
+			expectChangePasswordCall: true,
+
+			email: "abc@example.com",
+
+			// What causes the error
+			storeErrors: TestStoreFunctionsErrors{ChangePasswordNoWallet: fmt.Errorf("Some random db problem")},
+		},
+	}
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			testAuth := TestAuth{}
+			testStore := TestStore{Errors: tc.storeErrors}
+			s := Server{&testAuth, &testStore}
+
+			// Whether we passed in wallet fields (these test cases should be passing
+			// in all of them or none of them, so we only test EncryptedWallet). This
+			// determines whether we expect a call to ChangePasswordWithWallet (as
+			// opposed to ChangePasswordNoWallet).
+			withWallet := (tc.newEncryptedWallet != "")
+
+			const oldPassword = "old password"
+			const newPassword = "new password"
+
+			requestBody := []byte(
+				fmt.Sprintf(`{
+          "encryptedWallet": "%s",
+          "sequence": %d,
+          "hmac": "%s",
+          "email": "%s",
+          "oldPassword": "%s",
+          "newPassword": "%s"
+        }`, tc.newEncryptedWallet, tc.newSequence, tc.newHmac, tc.email, oldPassword, newPassword),
+			)
+
+			req := httptest.NewRequest(http.MethodPost, PathPassword, bytes.NewBuffer(requestBody))
+			w := httptest.NewRecorder()
+
+			s.changePassword(w, req)
+
+			body, _ := ioutil.ReadAll(w.Body)
+
+			expectStatusCode(t, w, tc.expectedStatusCode)
+			expectErrorString(t, body, tc.expectedErrorString)
+
+			if tc.expectedErrorString == "" && string(body) != "{}" {
+				t.Errorf("Expected change password response to be \"{}\": result: %+v", string(body))
+			}
+
+			if tc.expectChangePasswordCall {
+				if withWallet {
+					// Called ChangePasswordWithWallet with the expected parameters
+					if want, got := (ChangePasswordWithWalletCall{
+						EncryptedWallet: tc.newEncryptedWallet,
+						Sequence:        tc.newSequence,
+						Hmac:            tc.newHmac,
+						Email:           tc.email,
+						OldPassword:     oldPassword,
+						NewPassword:     newPassword,
+					}), testStore.Called.ChangePasswordWithWallet; want != got {
+						t.Errorf("Store.ChangePasswordWithWallet called with: expected %+v, got %+v", want, got)
+					}
+
+					// Did *not* call ChangePasswordNoWallet
+					if want, got := (ChangePasswordNoWalletCall{}), testStore.Called.ChangePasswordNoWallet; want != got {
+						t.Errorf("Store.ChangePasswordNoWallet unexpectly called with: %+v", got)
+					}
+				} else {
+					// Called ChangePasswordNoWallet with the expected parameters
+					if want, got := (ChangePasswordNoWalletCall{
+						Email:       tc.email,
+						OldPassword: oldPassword,
+						NewPassword: newPassword,
+					}), testStore.Called.ChangePasswordNoWallet; want != got {
+						t.Errorf("Store.ChangePasswordNoWallet called with: expected %+v, got %+v", want, got)
+					}
+
+					// Did *not* call ChangePasswordWithWallet
+					if want, got := (ChangePasswordWithWalletCall{}), testStore.Called.ChangePasswordWithWallet; want != got {
+						t.Errorf("Store.ChangePasswordWithWallet unexpectly called with: %+v", got)
+					}
+				}
+			} else {
+				if want, got := (ChangePasswordWithWalletCall{}), testStore.Called.ChangePasswordWithWallet; want != got {
+					t.Errorf("Store.ChangePasswordWithWallet unexpectly called with: %+v", got)
+				}
+				if want, got := (ChangePasswordNoWalletCall{}), testStore.Called.ChangePasswordNoWallet; want != got {
+					t.Errorf("Store.ChangePasswordNoWallet unexpectly called with: %+v", got)
+				}
+			}
+		})
+	}
+}
+
+func TestServerValidateChangePasswordRequest(t *testing.T) {
+	changePasswordRequest := ChangePasswordRequest{
+		EncryptedWallet: "my-encrypted-wallet",
+		Hmac:            "my-hmac",
+		Sequence:        2,
+		Email:           "abc@example.com",
+		OldPassword:     "123",
+		NewPassword:     "456",
+	}
+	if !changePasswordRequest.validate() {
+		t.Errorf("Expected valid ChangePasswordRequest with wallet fields to successfully validate")
+	}
+
+	changePasswordRequest = ChangePasswordRequest{
+		Email:       "abc@example.com",
+		OldPassword: "123",
+		NewPassword: "456",
+	}
+	if !changePasswordRequest.validate() {
+		t.Errorf("Expected valid ChangePasswordRequest without wallet fields to successfully validate")
+	}
+
+	tt := []struct {
+		changePasswordRequest ChangePasswordRequest
+		failureDescription    string
+	}{
+		{
+			ChangePasswordRequest{
+				EncryptedWallet: "my-encrypted-wallet",
+				Hmac:            "my-hmac",
+				Sequence:        2,
+				Email:           "abc-example.com",
+				OldPassword:     "123",
+				NewPassword:     "456",
+			},
+			"Expected WalletRequest with invalid email to not successfully validate",
+		}, {
+			// Note that Golang's email address parser, which I use, will accept
+			// "Abc <abc@example.com>" so we need to make sure to avoid accepting it. See
+			// the implementation.
+			ChangePasswordRequest{
+				EncryptedWallet: "my-encrypted-wallet",
+				Hmac:            "my-hmac",
+				Sequence:        2,
+				Email:           "Abc <abc@example.com>",
+				OldPassword:     "123",
+				NewPassword:     "456",
+			},
+			"Expected WalletRequest with email with unexpected formatting to not successfully validate",
+		}, {
+			ChangePasswordRequest{
+				EncryptedWallet: "my-encrypted-wallet",
+				Hmac:            "my-hmac",
+				Sequence:        2,
+				OldPassword:     "123",
+				NewPassword:     "456",
+			},
+			"Expected WalletRequest with missing email to not successfully validate",
+		}, {
+			ChangePasswordRequest{
+				EncryptedWallet: "my-encrypted-wallet",
+				Hmac:            "my-hmac",
+				Sequence:        2,
+				Email:           "abc@example.com",
+				NewPassword:     "456",
+			},
+			"Expected WalletRequest with missing old password to not successfully validate",
+		}, {
+			ChangePasswordRequest{
+				EncryptedWallet: "my-encrypted-wallet",
+				Hmac:            "my-hmac",
+				Sequence:        2,
+				Email:           "abc@example.com",
+				OldPassword:     "123",
+			},
+			"Expected WalletRequest with missing new password to not successfully validate",
+		}, {
+			ChangePasswordRequest{
+				Hmac:        "my-hmac",
+				Sequence:    2,
+				Email:       "abc@example.com",
+				OldPassword: "123",
+				NewPassword: "456",
+			},
+			"Expected WalletRequest with missing encrypted wallet (but with other wallet fields present) to not successfully validate",
+		}, {
+			ChangePasswordRequest{
+				EncryptedWallet: "my-encrypted-wallet",
+				Sequence:        2,
+				Email:           "abc@example.com",
+				OldPassword:     "123",
+				NewPassword:     "456",
+			},
+			"Expected WalletRequest with missing hmac (but with other wallet fields present) to not successfully validate",
+		}, {
+			ChangePasswordRequest{
+				EncryptedWallet: "my-encrypted-wallet",
+				Hmac:            "my-hmac",
+				Sequence:        0,
+				Email:           "abc@example.com",
+				OldPassword:     "123",
+				NewPassword:     "456",
+			},
+			"Expected WalletRequest with sequence < 1 (but with other wallet fields present) to not successfully validate",
+		}, {
+			ChangePasswordRequest{
+				EncryptedWallet: "my-encrypted-wallet",
+				Hmac:            "my-hmac",
+				Sequence:        2,
+				Email:           "abc@example.com",
+				OldPassword:     "123",
+				NewPassword:     "123",
+			},
+			"Expected WalletRequest with password that does not change to not successfully validate",
+		},
+	}
+	for _, tc := range tt {
+		if tc.changePasswordRequest.validate() {
+			t.Errorf(tc.failureDescription)
+		}
+
+	}
+}

server/server.go

@@ -17,6 +17,7 @@ const PathPrefix = "/api/" + ApiVersion
 
 const PathAuthToken = PathPrefix + "/auth/full"
 const PathRegister = PathPrefix + "/signup"
+const PathPassword = PathPrefix + "/password"
 const PathWallet = PathPrefix + "/wallet"
 
 type Server struct {
@@ -178,6 +179,7 @@ func (s *Server) Serve() {
 	http.HandleFunc(PathAuthToken, s.getAuthToken)
 	http.HandleFunc(PathWallet, s.handleWallet)
 	http.HandleFunc(PathRegister, s.register)
+	http.HandleFunc(PathPassword, s.changePassword)
 
 	fmt.Println("Serving at localhost:8090")
 	http.ListenAndServe("localhost:8090", nil)

server/server_test.go

@@ -35,6 +35,21 @@ type SetWalletCall struct {
 	Hmac            wallet.WalletHmac
 }
 
+type ChangePasswordNoWalletCall struct {
+	Email       auth.Email
+	OldPassword auth.Password
+	NewPassword auth.Password
+}
+
+type ChangePasswordWithWalletCall struct {
+	EncryptedWallet wallet.EncryptedWallet
+	Sequence        wallet.Sequence
+	Hmac            wallet.WalletHmac
+	Email           auth.Email
+	OldPassword     auth.Password
+	NewPassword     auth.Password
+}
+
 // Whether functions are called, and sometimes what they're called with
 type TestStoreFunctionsCalled struct {
 	SaveToken                auth.TokenString
@@ -43,6 +58,8 @@ type TestStoreFunctionsCalled struct {
 	CreateAccount            bool
 	SetWallet                SetWalletCall
 	GetWallet                bool
+	ChangePasswordWithWallet ChangePasswordWithWalletCall
+	ChangePasswordNoWallet   ChangePasswordNoWalletCall
 }
 
 type TestStoreFunctionsErrors struct {
@@ -52,6 +69,8 @@ type TestStoreFunctionsErrors struct {
 	CreateAccount            error
 	SetWallet                error
 	GetWallet                error
+	ChangePasswordWithWallet error
+	ChangePasswordNoWallet   error
 }
 
 type TestStore struct {
@@ -110,6 +129,38 @@ func (s *TestStore) GetWallet(userId auth.UserId) (encryptedWallet wallet.Encryp
 	return
 }
 
+func (s *TestStore) ChangePasswordWithWallet(
+	email auth.Email,
+	oldPassword auth.Password,
+	newPassword auth.Password,
+	encryptedWallet wallet.EncryptedWallet,
+	sequence wallet.Sequence,
+	hmac wallet.WalletHmac,
+) (err error) {
+	s.Called.ChangePasswordWithWallet = ChangePasswordWithWalletCall{
+		EncryptedWallet: encryptedWallet,
+		Sequence:        sequence,
+		Hmac:            hmac,
+		Email:           email,
+		OldPassword:     oldPassword,
+		NewPassword:     newPassword,
+	}
+	return s.Errors.ChangePasswordWithWallet
+}
+
+func (s *TestStore) ChangePasswordNoWallet(
+	email auth.Email,
+	oldPassword auth.Password,
+	newPassword auth.Password,
+) (err error) {
+	s.Called.ChangePasswordNoWallet = ChangePasswordNoWalletCall{
+		Email:       email,
+		OldPassword: oldPassword,
+		NewPassword: newPassword,
+	}
+	return s.Errors.ChangePasswordNoWallet
+}
+
 // expectStatusCode: A helper to call in functions that test that request
 // handlers responded with a certain status code. Cuts down on noise.
 func expectStatusCode(t *testing.T, w *httptest.ResponseRecorder, expectedStatusCode int) {

store/store.go

@@ -42,7 +42,9 @@ type StoreInterface interface {
 	SetWallet(auth.UserId, wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac) error
 	GetWallet(auth.UserId) (wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac, error)
 	GetUserId(auth.Email, auth.Password) (auth.UserId, error)
-	CreateAccount(auth.Email, auth.Password) (err error)
+	CreateAccount(auth.Email, auth.Password) error
+	ChangePasswordWithWallet(auth.Email, auth.Password, auth.Password, wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac) error
+	ChangePasswordNoWallet(auth.Email, auth.Password, auth.Password) error
 }
 
 type Store struct {