Skip to content

Commit

Permalink
feat: more informative error messages for password requirements (#549)
Browse files Browse the repository at this point in the history
  • Loading branch information
garrettladley authored Apr 15, 2024
1 parent 7211d51 commit 6d608b0
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 30 deletions.
39 changes: 39 additions & 0 deletions backend/src/auth/password.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package auth

import (
"regexp"
"strings"

"github.com/GenerateNU/sac/backend/src/constants"
"github.com/GenerateNU/sac/backend/src/errors"
)

func ValidatePassword(password string) *errors.Error {
if len(password) < 8 {
return &errors.InvalidPasswordNotLongEnough
}

if !hasDigit(password) {
return &errors.InvalidPasswordNoDigit
}

if !hasSpecialChar(password) {
return &errors.InvalidPasswordNoSpecialCharacter
}

return nil
}

func hasDigit(str string) bool {
return regexp.MustCompile(`[0-9]`).MatchString(str)
}

func hasSpecialChar(str string) bool {
for _, c := range constants.SPECIAL_CHARACTERS {
if strings.Contains(str, string(c)) {
return true
}
}

return false
}
2 changes: 2 additions & 0 deletions backend/src/constants/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ const (
ACCESS_TOKEN_EXPIRY time.Duration = time.Hour * 24 * 30 // temporary TODO: change to 60 minutes
REFRESH_TOKEN_EXPIRY time.Duration = time.Hour * 24 * 30
)

var SPECIAL_CHARACTERS = []rune{' ', '!', '"', '#', '$', '%', '&', '\'', '(', ')', '*', '+', ',', '-', '.', '/', ':', ';', '<', '=', '>', '?', '@', '[', '\\', ']', '^', '_', '`', '{', '|', '}', '~'} // see https://owasp.org/www-community/password-special-characters
12 changes: 12 additions & 0 deletions backend/src/errors/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,16 @@ var (
StatusCode: fiber.StatusNotFound,
Message: "otp not found",
}
InvalidPasswordNotLongEnough = Error{
StatusCode: fiber.StatusBadRequest,
Message: "password must be at least 8 characters long",
}
InvalidPasswordNoDigit = Error{
StatusCode: fiber.StatusBadRequest,
Message: "password must contain at least one digit",
}
InvalidPasswordNoSpecialCharacter = Error{
StatusCode: fiber.StatusBadRequest,
Message: "password must contain at least one special character",
}
)
8 changes: 4 additions & 4 deletions backend/src/models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ type CreateUserRequestBody struct {
FirstName string `json:"first_name" validate:"required,max=255"`
LastName string `json:"last_name" validate:"required,max=255"`
Email string `json:"email" validate:"required,email,neu_email,max=255"`
Password string `json:"password" validate:"required,password,min=8,max=255"`
Password string `json:"password" validate:"required,max=255"` // MARK: must be validated manually
// Optional fields
Major0 Major `json:"major0" validate:"not_equal_if_not_empty=Major1,not_equal_if_not_empty=Major2,omitempty,max=255,oneof=africanaStudies americanSignLanguage americanSignLanguage-EnglishInterpreting appliedPhysics architecturalStudies architecture art:ArtVisualStudies behavioralNeuroscience biochemistry bioengineering biology biomedicalPhysics businessAdministration businessAdministration:Accounting businessAdministration:AccountingAndAdvisoryServices businessAdministration:BrandManagement businessAdministration:BusinessAnalytics businessAdministration:CorporateInnovation businessAdministration:EntrepreneurialStartups businessAdministration:FamilyBusiness businessAdministration:Finance businessAdministration:Fintech businessAdministration:HealthcareManagementAndConsulting businessAdministration:Management businessAdministration:ManagementInformationSystems businessAdministration:Marketing businessAdministration:MarketingAnalytics businessAdministration:SocialInnovationAndEntrepreneurship businessAdministration:SupplyChainManagement cellAndMolecularBiology chemicalEngineering chemistry civilEngineering communicationStudies computerEngineering computerScience computingAndLaw criminologyAndCriminalJustice culturalAnthropology cybersecurity dataScience design economics electricalEngineering english environmentalAndSustainabilityStudies environmentalEngineering environmentalScience environmentalStudies gameArtAndAnimation gameDesign globalAsianStudies healthScience history historyCultureAndLaw humanServices industrialEngineering internationalAffairs internationalBusiness internationalBusiness:Accounting internationalBusiness:AccountingAndAdvisoryServices internationalBusiness:BrandManagement internationalBusiness:BusinessAnalytics internationalBusiness:CorporateInnovation internationalBusiness:EntrepreneurialStartups internationalBusiness:FamilyBusiness internationalBusiness:Finance internationalBusiness:Fintech internationalBusiness:HealthcareManagementAndConsulting internationalBusiness:Management internationalBusiness:ManagementInformationSystems internationalBusiness:Marketing internationalBusiness:MarketingAnalytics internationalBusiness:SocialInnovationAndEntrepreneurship internationalBusiness:SupplyChainManagement journalism landscapeArchitecture linguistics marineBiology mathematics mechanicalEngineering mediaAndScreenStudies mediaArts music musicTechnology nursing pharmaceuticalSciences pharmacy(PharmD) philosophy physics politicalScience politicsPhilosophyEconomics psychology publicHealth publicRelations religiousStudies sociology spanish speechLanguagePathologyAndAudiology theatre"`
Major1 Major `json:"major1" validate:"not_equal_if_not_empty=Major0,not_equal_if_not_empty=Major2,omitempty,max=255,oneof=africanaStudies americanSignLanguage americanSignLanguage-EnglishInterpreting appliedPhysics architecturalStudies architecture art:ArtVisualStudies behavioralNeuroscience biochemistry bioengineering biology biomedicalPhysics businessAdministration businessAdministration:Accounting businessAdministration:AccountingAndAdvisoryServices businessAdministration:BrandManagement businessAdministration:BusinessAnalytics businessAdministration:CorporateInnovation businessAdministration:EntrepreneurialStartups businessAdministration:FamilyBusiness businessAdministration:Finance businessAdministration:Fintech businessAdministration:HealthcareManagementAndConsulting businessAdministration:Management businessAdministration:ManagementInformationSystems businessAdministration:Marketing businessAdministration:MarketingAnalytics businessAdministration:SocialInnovationAndEntrepreneurship businessAdministration:SupplyChainManagement cellAndMolecularBiology chemicalEngineering chemistry civilEngineering communicationStudies computerEngineering computerScience computingAndLaw criminologyAndCriminalJustice culturalAnthropology cybersecurity dataScience design economics electricalEngineering english environmentalAndSustainabilityStudies environmentalEngineering environmentalScience environmentalStudies gameArtAndAnimation gameDesign globalAsianStudies healthScience history historyCultureAndLaw humanServices industrialEngineering internationalAffairs internationalBusiness internationalBusiness:Accounting internationalBusiness:AccountingAndAdvisoryServices internationalBusiness:BrandManagement internationalBusiness:BusinessAnalytics internationalBusiness:CorporateInnovation internationalBusiness:EntrepreneurialStartups internationalBusiness:FamilyBusiness internationalBusiness:Finance internationalBusiness:Fintech internationalBusiness:HealthcareManagementAndConsulting internationalBusiness:Management internationalBusiness:ManagementInformationSystems internationalBusiness:Marketing internationalBusiness:MarketingAnalytics internationalBusiness:SocialInnovationAndEntrepreneurship internationalBusiness:SupplyChainManagement journalism landscapeArchitecture linguistics marineBiology mathematics mechanicalEngineering mediaAndScreenStudies mediaArts music musicTechnology nursing pharmaceuticalSciences pharmacy(PharmD) philosophy physics politicalScience politicsPhilosophyEconomics psychology publicHealth publicRelations religiousStudies sociology spanish speechLanguagePathologyAndAudiology theatre"`
Expand All @@ -198,12 +198,12 @@ type UpdateUserRequestBody struct {

type LoginUserResponseBody struct {
Email string `json:"email" validate:"required,email"`
Password string `json:"password" validate:"required,password,min=8,max=255"`
Password string `json:"password" validate:"required,max=255"` // MARK: must be validated manually
}

type UpdatePasswordRequestBody struct {
OldPassword string `json:"old_password" validate:"required,password,min=8,max=255"`
NewPassword string `json:"new_password" validate:"required,password,not_equal_if_not_empty=OldPassword,min=8,max=255"`
OldPassword string `json:"old_password" validate:"required,max=255"` // MARK: must be validated manually
NewPassword string `json:"new_password" validate:"required,not_equal_if_not_empty=OldPassword,max=255"` // MARK: must be validated manually
}

type RefreshTokenRequestBody struct {
Expand Down
4 changes: 4 additions & 0 deletions backend/src/services/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ func (a *AuthService) Login(loginBody models.LoginUserResponseBody) (*models.Use
return nil, nil, &errors.FailedToValidateUser
}

if pwordValErr := auth.ValidatePassword(loginBody.Password); pwordValErr != nil {
return nil, nil, pwordValErr
}

user, getUserByEmailErr := transactions.GetUserByEmail(a.DB, loginBody.Email)
if getUserByEmailErr != nil {
return nil, nil, getUserByEmailErr
Expand Down
12 changes: 12 additions & 0 deletions backend/src/services/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ func (u *UserService) CreateUser(userBody models.CreateUserRequestBody) (*models
return nil, nil, &errors.FailedToValidateUser
}

if pwordValErr := auth.ValidatePassword(userBody.Password); pwordValErr != nil {
return nil, nil, pwordValErr
}

user, err := utilities.MapRequestToModel(userBody, &models.User{})
if err != nil {
return nil, nil, &errors.FailedToMapRequestToModel
Expand Down Expand Up @@ -141,6 +145,14 @@ func (u *UserService) UpdatePassword(id string, passwordBody models.UpdatePasswo
return &errors.FailedToValidateUpdatePasswordBody
}

if pwordValErr := auth.ValidatePassword(passwordBody.OldPassword); pwordValErr != nil {
return pwordValErr
}

if pwordValErr := auth.ValidatePassword(passwordBody.NewPassword); pwordValErr != nil {
return pwordValErr
}

passwordHash, err := transactions.GetUserPasswordHash(u.DB, *idAsUUID)
if err != nil {
return err
Expand Down
15 changes: 0 additions & 15 deletions backend/src/utilities/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package utilities

import (
"reflect"
"regexp"
"strconv"
"strings"

Expand All @@ -22,10 +21,6 @@ func RegisterCustomValidators() (*validator.Validate, error) {
return nil, err
}

if err := validate.RegisterValidation("password", validatePassword); err != nil {
return nil, err
}

if err := validate.RegisterValidation("s3_url", validateS3URL); err != nil {
return nil, err
}
Expand Down Expand Up @@ -60,16 +55,6 @@ func validateEmail(fl validator.FieldLevel) bool {
return true
}

func validatePassword(fl validator.FieldLevel) bool {
password := fl.Field().String()

hasMinLength := len(password) >= 8
hasDigit, _ := regexp.MatchString(`[0-9]`, password)
hasSpecialChar, _ := regexp.MatchString(`[@#%&*+]`, password)

return hasMinLength && hasDigit && hasSpecialChar
}

func validateS3URL(fl validator.FieldLevel) bool {
return strings.HasPrefix(fl.Field().String(), "https://s3.amazonaws.com/")
}
Expand Down
47 changes: 36 additions & 11 deletions backend/tests/api/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ func TestCreateUserFailsIfUserWithEmailAlreadyExists(t *testing.T) {
}

func AssertCreateBadDataFails(t *testing.T, jsonKey string, badValues []interface{}) {
appAssert, _, _ := CreateSampleStudent(t, nil)
appAssert := h.InitTest(t)

sampleStudent, rawPassword := h.SampleStudentFactory()

Expand All @@ -498,7 +498,7 @@ func AssertCreateBadDataFails(t *testing.T, jsonKey string, badValues []interfac
},
h.ErrorWithTester{
Error: errors.FailedToValidateUser,
Tester: TestNumUsersRemainsAt2,
Tester: TestNumUsersRemainsAt1,
},
)
}
Expand All @@ -520,15 +520,40 @@ func TestCreateUserFailsOnInvalidEmail(t *testing.T) {
}

func TestCreateUserFailsOnInvalidPassword(t *testing.T) {
AssertCreateBadDataFails(t,
"password",
[]interface{}{
"",
"foo",
"abcdefg",
"abcdefg0",
"abcdefg@",
})
appAssert := h.InitTest(t)

sampleStudent, rawPassword := h.SampleStudentFactory()

inputsWithDesiredErr := []struct {
Password string
Error errors.Error
}{
{"0", errors.InvalidPasswordNotLongEnough},
{"foo", errors.InvalidPasswordNotLongEnough},
{"abcdefgh", errors.InvalidPasswordNoDigit},
{"abcdefg0", errors.InvalidPasswordNoSpecialCharacter},
{"abcdefg@", errors.InvalidPasswordNoDigit},
}

for _, inputWithDesiredErr := range inputsWithDesiredErr {
sampleUserPermutation := *h.SampleStudentJSONFactory(sampleStudent, rawPassword)
sampleUserPermutation["password"] = inputWithDesiredErr.Password

appAssert = appAssert.TestOnErrorAndTester(
h.TestRequest{
Method: fiber.MethodPost,
Path: "/api/v1/users/",
Body: &sampleUserPermutation,
Role: &models.Super,
},
h.ErrorWithTester{
Error: inputWithDesiredErr.Error,
Tester: TestNumUsersRemainsAt1,
},
)
}

appAssert.Close()
}

func TestCreateUserFailsOnMissingFields(t *testing.T) {
Expand Down

0 comments on commit 6d608b0

Please sign in to comment.