LBRYCommunity/wallet-sync-server
1
0
Fork 0
Code Issues 11 Pull requests Projects Releases 2 Packages Wiki Activity

validate() functions return error messages

Browse source
This commit is contained in:
Daniel Krol 2022-07-11 09:42:08 -04:00
parent 18e30bd6b1
commit 6646e9a4f9
10 changed files with 115 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
server/account.go
View file

@ -15,8 +15,14 @@ type RegisterRequest struct {
Password auth.Password `json:"password"`
}
func (r *RegisterRequest) validate() bool {
return validateEmail(r.Email) && r.Password != ""
func (r *RegisterRequest) validate() error {
if !validateEmail(r.Email) {
return fmt.Errorf("Invalid 'email'")
}
if r.Password == "" {
return fmt.Errorf("Missing 'password'")
}
return nil
}
func (s *Server) register(w http.ResponseWriter, req *http.Request) {

17
server/account_test.go
View file

@ -6,6 +6,7 @@ import (
"io/ioutil"
"net/http"
"net/http/httptest"
"strings"
"testing"
"orblivion/lbry-id/store"
@ -48,7 +49,7 @@ func TestServerRegisterErrors(t *testing.T) {
name: "validation error", // missing email address
email: "",
expectedStatusCode: http.StatusBadRequest,
expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation",
expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation: Invalid 'email'",
// Just check one validation error (missing email address) to make sure the
// validate function is called. We'll check the rest of the validation
@ -96,12 +97,13 @@ func TestServerRegisterErrors(t *testing.T) {
func TestServerValidateRegisterRequest(t *testing.T) {
registerRequest := RegisterRequest{Email: "joe@example.com", Password: "aoeu"}
if !registerRequest.validate() {
if registerRequest.validate() != nil {
t.Fatalf("Expected valid RegisterRequest to successfully validate")
}
registerRequest = RegisterRequest{Email: "joe-example.com", Password: "aoeu"}
if registerRequest.validate() {
err := registerRequest.validate()
if !strings.Contains(err.Error(), "email") {
t.Fatalf("Expected RegisterRequest with invalid email to not successfully validate")
}
@ -109,17 +111,20 @@ func TestServerValidateRegisterRequest(t *testing.T) {
// "Joe <joe@example.com>" so we need to make sure to avoid accepting it. See
// the implementation.
registerRequest = RegisterRequest{Email: "Joe <joe@example.com>", Password: "aoeu"}
if registerRequest.validate() {
err = registerRequest.validate()
if !strings.Contains(err.Error(), "email") {
t.Fatalf("Expected RegisterRequest with email with unexpected formatting to not successfully validate")
}
registerRequest = RegisterRequest{Password: "aoeu"}
if registerRequest.validate() {
err = registerRequest.validate()
if !strings.Contains(err.Error(), "email") {
t.Fatalf("Expected RegisterRequest with missing email to not successfully validate")
}
registerRequest = RegisterRequest{Email: "joe@example.com"}
if registerRequest.validate() {
err = registerRequest.validate()
if !strings.Contains(err.Error(), "password") {
t.Fatalf("Expected RegisterRequest with missing password to not successfully validate")
}
}

13
server/auth.go
View file

@ -18,8 +18,17 @@ type AuthRequest struct {
// TODO - validate funcs probably should return error rather than bool for
// idiomatic golang
func (r *AuthRequest) validate() bool {
return r.DeviceId != "" && r.Password != auth.Password("") && validateEmail(r.Email)
func (r *AuthRequest) validate() error {
if !validateEmail(r.Email) {
return fmt.Errorf("Invalid 'email'")
}
if r.Password == "" {
return fmt.Errorf("Missing 'password'")
}
if r.DeviceId == "" {
return fmt.Errorf("Missing 'deviceId'")
}
return nil
}
func (s *Server) getAuthToken(w http.ResponseWriter, req *http.Request) {

18
server/auth_test.go
View file

@ -9,6 +9,7 @@ import (
"net/http/httptest"
"orblivion/lbry-id/auth"
"orblivion/lbry-id/store"
"strings"
"testing"
)
@ -53,7 +54,7 @@ func TestServerAuthHandlerErrors(t *testing.T) {
name: "validation error", // missing email address
email: "",
expectedStatusCode: http.StatusBadRequest,
expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation",
expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation: Invalid 'email'",
// Just check one validation error (missing email address) to make sure the
// validate function is called. We'll check the rest of the validation
@ -113,36 +114,43 @@ func TestServerAuthHandlerErrors(t *testing.T) {
func TestServerValidateAuthRequest(t *testing.T) {
authRequest := AuthRequest{DeviceId: "dId", Email: "joe@example.com", Password: "aoeu"}
if !authRequest.validate() {
if authRequest.validate() != nil {
t.Fatalf("Expected valid AuthRequest to successfully validate")
}
tt := []struct {
authRequest AuthRequest
failureDescription string
authRequest AuthRequest
expectedErrorSubstr string
failureDescription string
}{
{
AuthRequest{Email: "joe@example.com", Password: "aoeu"},
"deviceId",
"Expected AuthRequest with missing device to not successfully validate",
}, {
AuthRequest{DeviceId: "dId", Email: "joe-example.com", Password: "aoeu"},
"email",
"Expected AuthRequest with invalid email to not successfully validate",
}, {
// Note that Golang's email address parser, which I use, will accept
// "Joe <joe@example.com>" so we need to make sure to avoid accepting it. See
// the implementation.
AuthRequest{DeviceId: "dId", Email: "Joe <joe@example.com>", Password: "aoeu"},
"email",
"Expected AuthRequest with email with unexpected formatting to not successfully validate",
}, {
AuthRequest{DeviceId: "dId", Password: "aoeu"},
"email",
"Expected AuthRequest with missing email to not successfully validate",
}, {
AuthRequest{DeviceId: "dId", Email: "joe@example.com"},
"password",
"Expected AuthRequest with missing password to not successfully validate",
},
}
for _, tc := range tt {
if tc.authRequest.validate() {
err := tc.authRequest.validate()
if !strings.Contains(err.Error(), tc.expectedErrorSubstr) {
t.Errorf(tc.failureDescription)
}
}

23
server/password.go
View file

@ -19,16 +19,27 @@ type ChangePasswordRequest struct {
NewPassword auth.Password `json:"newPassword"`
}
func (r *ChangePasswordRequest) validate() bool {
func (r *ChangePasswordRequest) validate() error {
// 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))
if !validateEmail(r.Email) {
return fmt.Errorf("Invalid 'email'")
}
if r.OldPassword == "" {
return fmt.Errorf("Missing 'oldPassword'")
}
if r.NewPassword == "" {
return fmt.Errorf("Missing 'newPassword'")
}
if r.OldPassword == r.NewPassword {
return fmt.Errorf("'oldPassword' and 'newPassword' should not be the same")
}
if !walletPresent && !walletAbsent {
return fmt.Errorf("Fields 'encryptedWallet', 'sequence', and 'hmac' should be all non-empty and non-zero, or all omitted")
}
return nil
}
func (s *Server) changePassword(w http.ResponseWriter, req *http.Request) {

20
server/password_test.go
View file

@ -6,6 +6,7 @@ import (
"io/ioutil"
"net/http"
"net/http/httptest"
"strings"
"testing"
"orblivion/lbry-id/auth"
@ -105,7 +106,7 @@ func TestServerChangePassword(t *testing.T) {
}, {
name: "validation error",
expectedStatusCode: http.StatusBadRequest,
expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation",
expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation: Invalid 'email'",
// Just check one validation error (missing email address) to make sure
// the validate function is called. We'll check the rest of the
@ -234,7 +235,7 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
OldPassword: "123",
NewPassword: "456",
}
if !changePasswordRequest.validate() {
if changePasswordRequest.validate() != nil {
t.Errorf("Expected valid ChangePasswordRequest with wallet fields to successfully validate")
}
@ -243,12 +244,13 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
OldPassword: "123",
NewPassword: "456",
}
if !changePasswordRequest.validate() {
if changePasswordRequest.validate() != nil {
t.Errorf("Expected valid ChangePasswordRequest without wallet fields to successfully validate")
}
tt := []struct {
changePasswordRequest ChangePasswordRequest
expectedErrorSubstr string
failureDescription string
}{
{
@ -260,6 +262,7 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
OldPassword: "123",
NewPassword: "456",
},
"email",
"Expected WalletRequest with invalid email to not successfully validate",
}, {
// Note that Golang's email address parser, which I use, will accept
@ -273,6 +276,7 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
OldPassword: "123",
NewPassword: "456",
},
"email",
"Expected WalletRequest with email with unexpected formatting to not successfully validate",
}, {
ChangePasswordRequest{
@ -282,6 +286,7 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
OldPassword: "123",
NewPassword: "456",
},
"email",
"Expected WalletRequest with missing email to not successfully validate",
}, {
ChangePasswordRequest{
@ -291,6 +296,7 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
Email: "abc@example.com",
NewPassword: "456",
},
"oldPassword",
"Expected WalletRequest with missing old password to not successfully validate",
}, {
ChangePasswordRequest{
@ -300,6 +306,7 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
Email: "abc@example.com",
OldPassword: "123",
},
"newPassword",
"Expected WalletRequest with missing new password to not successfully validate",
}, {
ChangePasswordRequest{
@ -309,6 +316,7 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
OldPassword: "123",
NewPassword: "456",
},
"'encryptedWallet', 'sequence', and 'hmac'", // More likely to fail when we change the error message but whatever
"Expected WalletRequest with missing encrypted wallet (but with other wallet fields present) to not successfully validate",
}, {
ChangePasswordRequest{
@ -318,6 +326,7 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
OldPassword: "123",
NewPassword: "456",
},
"'encryptedWallet', 'sequence', and 'hmac'", // More likely to fail when we change the error message but whatever
"Expected WalletRequest with missing hmac (but with other wallet fields present) to not successfully validate",
}, {
ChangePasswordRequest{
@ -328,6 +337,7 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
OldPassword: "123",
NewPassword: "456",
},
"'encryptedWallet', 'sequence', and 'hmac'", // More likely to fail when we change the error message but whatever
"Expected WalletRequest with sequence < 1 (but with other wallet fields present) to not successfully validate",
}, {
ChangePasswordRequest{
@ -338,11 +348,13 @@ func TestServerValidateChangePasswordRequest(t *testing.T) {
OldPassword: "123",
NewPassword: "123",
},
"should not be the same",
"Expected WalletRequest with password that does not change to not successfully validate",
},
}
for _, tc := range tt {
if tc.changePasswordRequest.validate() {
err := tc.changePasswordRequest.validate()
if !strings.Contains(err.Error(), tc.expectedErrorSubstr) {
t.Errorf(tc.failureDescription)
}

9
server/server.go
View file

@ -87,8 +87,9 @@ func requestOverhead(w http.ResponseWriter, req *http.Request, method string) bo
}
// All structs representing incoming json request body should implement this
// The contents of `error` should be safe for an API response (public-facing)
type PostRequest interface {
validate() bool
validate() error
}
// TODO decoder.DisallowUnknownFields?
@ -119,9 +120,9 @@ func getPostData(w http.ResponseWriter, req *http.Request, reqStruct PostRequest
return false
}
if !reqStruct.validate() {
// TODO validate() should return useful error messages instead of a bool.
errorJson(w, http.StatusBadRequest, "Request failed validation")
err = reqStruct.validate()
if err != nil {
errorJson(w, http.StatusBadRequest, "Request failed validation: "+err.Error())
return false
}

10
server/server_test.go
View file

@ -283,7 +283,13 @@ func TestServerHelperGetGetDataErrors(t *testing.T) {
type TestReqStruct struct{ key string }
func (t *TestReqStruct) validate() bool { return t.key != "" }
func (t *TestReqStruct) validate() error {
if t.key == "" {
return fmt.Errorf("TestReq Error")
} else {
return nil
}
}
func TestServerHelperGetPostDataSuccess(t *testing.T) {
requestBody := []byte(`{}`)
@ -330,7 +336,7 @@ func TestServerHelperGetPostDataErrors(t *testing.T) {
method: http.MethodPost,
requestBody: "{}",
expectedStatusCode: http.StatusBadRequest,
expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation",
expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation: TestReq Error",
},
}
for _, tc := range tt {

19
server/wallet.go
View file

@ -16,11 +16,20 @@ type WalletRequest struct {
Hmac wallet.WalletHmac `json:"hmac"`
}
func (r *WalletRequest) validate() bool {
return (r.Token != auth.TokenString("") &&
r.EncryptedWallet != wallet.EncryptedWallet("") &&
r.Hmac != wallet.WalletHmac("") &&
r.Sequence >= wallet.Sequence(1))
func (r *WalletRequest) validate() error {
if r.Token == "" {
return fmt.Errorf("Missing 'token'")
}
if r.EncryptedWallet == "" {
return fmt.Errorf("Missing 'encryptedWallet'")
}
if r.Hmac == "" {
return fmt.Errorf("Missing 'hmac'")
}
if r.Sequence < 1 {
return fmt.Errorf("Missing or zero-value 'sequence'")
}
return nil
}
type WalletResponse struct {

18
server/wallet_test.go
View file

@ -7,6 +7,7 @@ import (
"io/ioutil"
"net/http"
"net/http/httptest"
"strings"
"testing"
"orblivion/lbry-id/auth"
@ -176,7 +177,7 @@ func TestServerPostWallet(t *testing.T) {
}, {
name: "validation error",
expectedStatusCode: http.StatusBadRequest,
expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation",
expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation: Missing 'encryptedWallet'",
skipAuthCheck: true, // we can't get an auth token without the data we just failed to validate
// Just check one validation error (empty encrypted wallet) to make sure the
@ -275,32 +276,37 @@ func TestServerPostWallet(t *testing.T) {
func TestServerValidateWalletRequest(t *testing.T) {
walletRequest := WalletRequest{Token: "seekrit", EncryptedWallet: "my-encrypted-wallet", Hmac: "my-hmac", Sequence: 2}
if !walletRequest.validate() {
if walletRequest.validate() != nil {
t.Fatalf("Expected valid WalletRequest to successfully validate")
}
tt := []struct {
walletRequest WalletRequest
failureDescription string
walletRequest WalletRequest
expectedErrorSubstr string
failureDescription string
}{
{
WalletRequest{EncryptedWallet: "my-encrypted-wallet", Hmac: "my-hmac", Sequence: 2},
"token",
"Expected WalletRequest with missing token to not successfully validate",
}, {
WalletRequest{Token: "seekrit", Hmac: "my-hmac", Sequence: 2},
"encryptedWallet",
"Expected WalletRequest with missing encrypted wallet to not successfully validate",
}, {
WalletRequest{Token: "seekrit", EncryptedWallet: "my-encrypted-wallet", Sequence: 2},
"hmac",
"Expected WalletRequest with missing hmac to not successfully validate",
}, {
WalletRequest{Token: "seekrit", EncryptedWallet: "my-encrypted-wallet", Hmac: "my-hmac", Sequence: 0},
"sequence",
"Expected WalletRequest with sequence < 1 to not successfully validate",
},
}
for _, tc := range tt {
if tc.walletRequest.validate() {
err := tc.walletRequest.validate()
if err == nil || !strings.Contains(err.Error(), tc.expectedErrorSubstr) {
t.Errorf(tc.failureDescription)
}
}
}