From 8ec9fb247f7f04b43b24f9ca7f5a41b266c1a965 Mon Sep 17 00:00:00 2001 From: Alex <$(pass /github/email)> Date: Mon, 3 Mar 2025 12:33:07 +0100 Subject: [PATCH] hardened password validation, added tests --- frontend/src/lib/locales/de.js | 6 ++ .../auth/password/change/[id]/+page.svelte | 12 +++- go-backend/go.mod | 1 + go-backend/go.sum | 2 + .../internal/controllers/controllers_test.go | 2 +- .../internal/controllers/user_Password.go | 10 +++ .../controllers/user_controller_test.go | 49 ++++++++++++-- .../internal/validation/user_validation.go | 65 ++++++++++++++----- 8 files changed, 122 insertions(+), 25 deletions(-) diff --git a/frontend/src/lib/locales/de.js b/frontend/src/lib/locales/de.js index f4aaa9d..a1b032c 100644 --- a/frontend/src/lib/locales/de.js +++ b/frontend/src/lib/locales/de.js @@ -75,6 +75,12 @@ export default { user_not_found_or_wrong_password: 'Existiert nicht oder falsches Passwort', email_already_registered: 'Ein Mitglied wurde schon mit dieser Emailadresse erstellt.', password_already_changed: 'Das Passwort wurde schon geändert.', + insecure: 'unsicheres Passwort, versuchen Sie {message}', + longer: 'oder verwenden Sie ein längeres Passwort', + special: 'mehr Sonderzeichen einzufügen', + lowercase: 'Kleinbuchstaben zu verwenden', + uppercase: 'Großbuchstaben zu verwenden', + numbers: 'Zahlen zu verwenden', alphanumunicode: 'beinhaltet nicht erlaubte Zeichen', safe_content: 'I see what you did there! Do not cross this line!', iban: 'Ungültig. Format: DE07123412341234123412', diff --git a/frontend/src/routes/auth/password/change/[id]/+page.svelte b/frontend/src/routes/auth/password/change/[id]/+page.svelte index dc79774..b31e598 100644 --- a/frontend/src/routes/auth/password/change/[id]/+page.svelte +++ b/frontend/src/routes/auth/password/change/[id]/+page.svelte @@ -41,13 +41,21 @@

{$t('change_password')}

{#if form?.errors} - {#each form?.errors as error (error.id)} + {#each form.errors as error (error.id)}

- {$t(error.key)} + {$t(error.key, { + values: { + message: + error.field + .split(' ') + .map((tag) => $t(tag)) + .join(', ') || '' + } + })}

{/each} {/if} diff --git a/go-backend/go.mod b/go-backend/go.mod index 0e96648..da59fae 100644 --- a/go-backend/go.mod +++ b/go-backend/go.mod @@ -19,6 +19,7 @@ require ( github.com/kelseyhightower/envconfig v1.4.0 github.com/mocktools/go-smtp-mock/v2 v2.3.1 github.com/stretchr/testify v1.9.0 + github.com/wagslane/go-password-validator v0.3.0 ) require ( diff --git a/go-backend/go.sum b/go-backend/go.sum index 2c38a5a..d83acf4 100644 --- a/go-backend/go.sum +++ b/go-backend/go.sum @@ -91,6 +91,8 @@ github.com/twitchyliquid64/golang-asm v0.15.1 h1:SU5vSMR7hnwNxj24w34ZyCi/FmDZTkS github.com/twitchyliquid64/golang-asm v0.15.1/go.mod h1:a1lVb/DtPvCB8fslRZhAngC2+aY1QWCk3Cedj/Gdt08= github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65EE= github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= +github.com/wagslane/go-password-validator v0.3.0 h1:vfxOPzGHkz5S146HDpavl0cw1DSVP061Ry2PX0/ON6I= +github.com/wagslane/go-password-validator v0.3.0/go.mod h1:TI1XJ6T5fRdRnHqHt14pvy1tNVnrwe7m3/f1f2fDphQ= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/arch v0.0.0-20210923205945-b76863e36670/go.mod h1:5om86z9Hs0C8fWVUuoMHwpExlXzs5Tkyp9hOrfG7pp8= golang.org/x/arch v0.8.0 h1:3wRIsP3pM4yUptoR96otTUOXI367OS0+c9eeRi9doIc= diff --git a/go-backend/internal/controllers/controllers_test.go b/go-backend/internal/controllers/controllers_test.go index 8d2f28d..d8c3830 100644 --- a/go-backend/internal/controllers/controllers_test.go +++ b/go-backend/internal/controllers/controllers_test.go @@ -276,7 +276,7 @@ func getBaseUser() models.User { Membership: models.Membership{SubscriptionModel: models.SubscriptionModel{Name: "Basic"}}, Licence: nil, ProfilePicture: "", - Password: "password123", + Password: "passw@#$#%$!-ord123", Company: "", RoleID: 8, } diff --git a/go-backend/internal/controllers/user_Password.go b/go-backend/internal/controllers/user_Password.go index 4e3a0fd..c3468a8 100644 --- a/go-backend/internal/controllers/user_Password.go +++ b/go-backend/internal/controllers/user_Password.go @@ -8,6 +8,8 @@ import ( "strconv" "github.com/gin-gonic/gin" + "github.com/gin-gonic/gin/binding" + "github.com/go-playground/validator/v10" ) func (uc *UserController) RequestPasswordChangeHandler(c *gin.Context) { @@ -86,6 +88,14 @@ func (uc *UserController) ChangePassword(c *gin.Context) { user.ID = verification.UserID user.Password = input.Password + // Get Gin's binding validator engine with all registered validators + validate := binding.Validator.Engine().(*validator.Validate) + + // Validate the populated user struct + if err := validate.Struct(user); err != nil { + utils.HandleValidationError(c, err) + return + } _, err = uc.Service.UpdateUser(user) if err != nil { utils.HandleUserUpdateError(c, err) diff --git a/go-backend/internal/controllers/user_controller_test.go b/go-backend/internal/controllers/user_controller_test.go index fa9ff58..91da0c1 100644 --- a/go-backend/internal/controllers/user_controller_test.go +++ b/go-backend/internal/controllers/user_controller_test.go @@ -195,7 +195,7 @@ func testLoginHandler(t *testing.T) (string, http.Cookie) { name: "Valid login", input: `{ "email": "john.doe@example.com", - "password": "password123" + "password": "passw@#$#%$!-ord123" }`, wantStatusCode: http.StatusOK, wantToken: true, @@ -204,7 +204,7 @@ func testLoginHandler(t *testing.T) (string, http.Cookie) { name: "Invalid email", input: `{ "email": "nonexistent@example.com", - "password": "password123" + "password": "passw@#$#%$!-ord123" }`, wantStatusCode: http.StatusNotFound, wantToken: false, @@ -645,6 +645,23 @@ func testUpdateUser(t *testing.T, loginCookie http.Cookie, adminCookie http.Cook {"field": "user.user", "key": "server.error.unauthorized"}, }, }, + { + name: "Password Update low entropy should fail", + setupCookie: func(req *http.Request) { + req.AddCookie(&loginCookie) + }, + updateFunc: func(u *models.User) { + u.FirstName = "John Updated" + u.LastName = "Doe Updated" + u.Phone = "01738484994" + u.Licence.Number = "B072RRE2I50" + u.Password = "newpassword" + }, + expectedErrors: []map[string]string{ + {"field": "server.validation.special server.validation.uppercase server.validation.numbers server.validation.longer", "key": "server.validation.insecure"}, + }, + expectedStatus: http.StatusBadRequest, + }, { name: "Password Update", setupCookie: func(req *http.Request) { @@ -655,7 +672,7 @@ func testUpdateUser(t *testing.T, loginCookie http.Cookie, adminCookie http.Cook u.LastName = "Doe Updated" u.Phone = "01738484994" u.Licence.Number = "B072RRE2I50" - u.Password = "NewPassword" + u.Password = "NewPa0293409@#-!ssword" }, expectedReturn: func(u *models.User) { u.Password = "" @@ -1069,7 +1086,7 @@ func getTestUsers() []RegisterUserTest { Assert: false, Input: GenerateInputJSON(customizeInput(func(user models.User) models.User { user.BankAccount.IBAN = "" - user.RoleID = 0 + user.RoleID = 1 return user })), }, @@ -1078,9 +1095,33 @@ func getTestUsers() []RegisterUserTest { WantResponse: http.StatusBadRequest, WantDBData: map[string]interface{}{"email": "john.doe@example.com"}, Assert: false, + Input: GenerateInputJSON(customizeInput(func(user models.User) models.User { + user.BankAccount.IBAN = "DE1234234123134" + user.RoleID = 1 + return user + })), + }, + { + Name: "invalid IBAN should fail when supporter", + WantResponse: http.StatusBadRequest, + WantDBData: map[string]interface{}{"email": "john.supporter@example.com"}, + Assert: false, Input: GenerateInputJSON(customizeInput(func(user models.User) models.User { user.BankAccount.IBAN = "DE1234234123134" user.RoleID = 0 + user.Email = "john.supporter@example.com" + return user + })), + }, + { + Name: "empty IBAN should pass when supporter", + WantResponse: http.StatusCreated, + WantDBData: map[string]interface{}{"email": "john.supporter@example.com"}, + Assert: true, + Input: GenerateInputJSON(customizeInput(func(user models.User) models.User { + user.BankAccount.IBAN = "" + user.RoleID = 0 + user.Email = "john.supporter@example.com" return user })), }, diff --git a/go-backend/internal/validation/user_validation.go b/go-backend/internal/validation/user_validation.go index 26929be..7ef2665 100644 --- a/go-backend/internal/validation/user_validation.go +++ b/go-backend/internal/validation/user_validation.go @@ -5,27 +5,27 @@ import ( "GoMembership/internal/models" "GoMembership/internal/repositories" "GoMembership/pkg/logger" + "strings" "time" "github.com/go-playground/validator/v10" + passwordvalidator "github.com/wagslane/go-password-validator" ) -func validateUser(sl validator.StructLevel) { +var passwordErrorTranslations = map[string]string{ + "insecure password, try ": "", + "or using a longer password": "server.validation.longer", + "including more special characters": "server.validation.special", + "using lowercase letters": "server.validation.lowercase", + "using uppercase letters": "server.validation.uppercase", + "using numbers": "server.validation.numbers", +} + +func ValidateUser(sl validator.StructLevel) { user := sl.Current().Interface().(models.User) isSuper := user.RoleID >= constants.Roles.Admin - - if user.RoleID > constants.Roles.Member && user.Password == "" { - passwordExists, err := repositories.PasswordExists(&user.ID) - if err != nil || !passwordExists { - logger.Error.Printf("Error checking password exists for user %v: %v", user.Email, err) - sl.ReportError(user.Password, "Password", "password", "required", "") - } - } - // Validate User > 18 years old - if user.RoleID > constants.Roles.Supporter && user.DateOfBirth.After(time.Now().AddDate(-18, 0, 0)) { - sl.ReportError(user.DateOfBirth, "user.user", "user.dateofbirth", "age", "") - } + isSupporter := user.RoleID == constants.Roles.Supporter // validate subscriptionModel if user.Membership.SubscriptionModel.Name == "" { sl.ReportError(user.Membership.SubscriptionModel.Name, "subscription.name", "name", "required", "") @@ -38,12 +38,41 @@ func validateUser(sl validator.StructLevel) { user.Membership.SubscriptionModel = *selectedModel } } + if isSupporter { + if user.BankAccount.IBAN != "" { + validateBankAccount(sl) + } + return + } - validateMembership(sl) - if !isSuper { - validateBankAccount(sl) - if user.Licence != nil && user.RoleID > constants.Roles.Supporter { - validateDriverslicence(sl) + if user.Password != "" { + if err := passwordvalidator.Validate(user.Password, 60); err != nil { + sl.ReportError(user.Password, translatePasswordError(err), "password", "insecure", "") } } + // Validate User > 18 years old + if user.DateOfBirth.After(time.Now().AddDate(-18, 0, 0)) { + sl.ReportError(user.DateOfBirth, "user.user", "user.dateofbirth", "age", "") + } + validateMembership(sl) + + if isSuper { + return + } + + validateBankAccount(sl) + if user.Licence != nil { + validateDriverslicence(sl) + } +} + +func translatePasswordError(err error) string { + errMsg := err.Error() + // Translate each part of the error message + translatedMsg := errMsg + for eng, translated := range passwordErrorTranslations { + translatedMsg = strings.Replace(translatedMsg, eng, translated, -1) + } + + return strings.Replace(translatedMsg, ",", "", -1) }