hardened password validation, added tests

This commit is contained in:
Alex
2025-03-03 12:33:07 +01:00
parent f5df70fba8
commit 8ec9fb247f
8 changed files with 122 additions and 25 deletions

View File

@@ -75,6 +75,12 @@ export default {
user_not_found_or_wrong_password: 'Existiert nicht oder falsches Passwort', user_not_found_or_wrong_password: 'Existiert nicht oder falsches Passwort',
email_already_registered: 'Ein Mitglied wurde schon mit dieser Emailadresse erstellt.', email_already_registered: 'Ein Mitglied wurde schon mit dieser Emailadresse erstellt.',
password_already_changed: 'Das Passwort wurde schon geändert.', 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', alphanumunicode: 'beinhaltet nicht erlaubte Zeichen',
safe_content: 'I see what you did there! Do not cross this line!', safe_content: 'I see what you did there! Do not cross this line!',
iban: 'Ungültig. Format: DE07123412341234123412', iban: 'Ungültig. Format: DE07123412341234123412',

View File

@@ -41,13 +41,21 @@
<form class="content" method="POST" use:enhance={handleChange}> <form class="content" method="POST" use:enhance={handleChange}>
<h1 class="step-title title">{$t('change_password')}</h1> <h1 class="step-title title">{$t('change_password')}</h1>
{#if form?.errors} {#if form?.errors}
{#each form?.errors as error (error.id)} {#each form.errors as error (error.id)}
<h4 <h4
class="step-subtitle warning" class="step-subtitle warning"
in:receive={{ key: error.id }} in:receive={{ key: error.id }}
out:send={{ key: error.id }} out:send={{ key: error.id }}
> >
{$t(error.key)} {$t(error.key, {
values: {
message:
error.field
.split(' ')
.map((tag) => $t(tag))
.join(', ') || ''
}
})}
</h4> </h4>
{/each} {/each}
{/if} {/if}

View File

@@ -19,6 +19,7 @@ require (
github.com/kelseyhightower/envconfig v1.4.0 github.com/kelseyhightower/envconfig v1.4.0
github.com/mocktools/go-smtp-mock/v2 v2.3.1 github.com/mocktools/go-smtp-mock/v2 v2.3.1
github.com/stretchr/testify v1.9.0 github.com/stretchr/testify v1.9.0
github.com/wagslane/go-password-validator v0.3.0
) )
require ( require (

View File

@@ -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/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 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65EE=
github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= 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= 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.0.0-20210923205945-b76863e36670/go.mod h1:5om86z9Hs0C8fWVUuoMHwpExlXzs5Tkyp9hOrfG7pp8=
golang.org/x/arch v0.8.0 h1:3wRIsP3pM4yUptoR96otTUOXI367OS0+c9eeRi9doIc= golang.org/x/arch v0.8.0 h1:3wRIsP3pM4yUptoR96otTUOXI367OS0+c9eeRi9doIc=

View File

@@ -276,7 +276,7 @@ func getBaseUser() models.User {
Membership: models.Membership{SubscriptionModel: models.SubscriptionModel{Name: "Basic"}}, Membership: models.Membership{SubscriptionModel: models.SubscriptionModel{Name: "Basic"}},
Licence: nil, Licence: nil,
ProfilePicture: "", ProfilePicture: "",
Password: "password123", Password: "passw@#$#%$!-ord123",
Company: "", Company: "",
RoleID: 8, RoleID: 8,
} }

View File

@@ -8,6 +8,8 @@ import (
"strconv" "strconv"
"github.com/gin-gonic/gin" "github.com/gin-gonic/gin"
"github.com/gin-gonic/gin/binding"
"github.com/go-playground/validator/v10"
) )
func (uc *UserController) RequestPasswordChangeHandler(c *gin.Context) { func (uc *UserController) RequestPasswordChangeHandler(c *gin.Context) {
@@ -86,6 +88,14 @@ func (uc *UserController) ChangePassword(c *gin.Context) {
user.ID = verification.UserID user.ID = verification.UserID
user.Password = input.Password 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) _, err = uc.Service.UpdateUser(user)
if err != nil { if err != nil {
utils.HandleUserUpdateError(c, err) utils.HandleUserUpdateError(c, err)

View File

@@ -195,7 +195,7 @@ func testLoginHandler(t *testing.T) (string, http.Cookie) {
name: "Valid login", name: "Valid login",
input: `{ input: `{
"email": "john.doe@example.com", "email": "john.doe@example.com",
"password": "password123" "password": "passw@#$#%$!-ord123"
}`, }`,
wantStatusCode: http.StatusOK, wantStatusCode: http.StatusOK,
wantToken: true, wantToken: true,
@@ -204,7 +204,7 @@ func testLoginHandler(t *testing.T) (string, http.Cookie) {
name: "Invalid email", name: "Invalid email",
input: `{ input: `{
"email": "nonexistent@example.com", "email": "nonexistent@example.com",
"password": "password123" "password": "passw@#$#%$!-ord123"
}`, }`,
wantStatusCode: http.StatusNotFound, wantStatusCode: http.StatusNotFound,
wantToken: false, wantToken: false,
@@ -645,6 +645,23 @@ func testUpdateUser(t *testing.T, loginCookie http.Cookie, adminCookie http.Cook
{"field": "user.user", "key": "server.error.unauthorized"}, {"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", name: "Password Update",
setupCookie: func(req *http.Request) { 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.LastName = "Doe Updated"
u.Phone = "01738484994" u.Phone = "01738484994"
u.Licence.Number = "B072RRE2I50" u.Licence.Number = "B072RRE2I50"
u.Password = "NewPassword" u.Password = "NewPa0293409@#-!ssword"
}, },
expectedReturn: func(u *models.User) { expectedReturn: func(u *models.User) {
u.Password = "" u.Password = ""
@@ -1069,7 +1086,7 @@ func getTestUsers() []RegisterUserTest {
Assert: false, Assert: false,
Input: GenerateInputJSON(customizeInput(func(user models.User) models.User { Input: GenerateInputJSON(customizeInput(func(user models.User) models.User {
user.BankAccount.IBAN = "" user.BankAccount.IBAN = ""
user.RoleID = 0 user.RoleID = 1
return user return user
})), })),
}, },
@@ -1078,9 +1095,33 @@ func getTestUsers() []RegisterUserTest {
WantResponse: http.StatusBadRequest, WantResponse: http.StatusBadRequest,
WantDBData: map[string]interface{}{"email": "john.doe@example.com"}, WantDBData: map[string]interface{}{"email": "john.doe@example.com"},
Assert: false, 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 { Input: GenerateInputJSON(customizeInput(func(user models.User) models.User {
user.BankAccount.IBAN = "DE1234234123134" user.BankAccount.IBAN = "DE1234234123134"
user.RoleID = 0 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 return user
})), })),
}, },

View File

@@ -5,27 +5,27 @@ import (
"GoMembership/internal/models" "GoMembership/internal/models"
"GoMembership/internal/repositories" "GoMembership/internal/repositories"
"GoMembership/pkg/logger" "GoMembership/pkg/logger"
"strings"
"time" "time"
"github.com/go-playground/validator/v10" "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) user := sl.Current().Interface().(models.User)
isSuper := user.RoleID >= constants.Roles.Admin isSuper := user.RoleID >= constants.Roles.Admin
isSupporter := user.RoleID == constants.Roles.Supporter
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", "")
}
// validate subscriptionModel // validate subscriptionModel
if user.Membership.SubscriptionModel.Name == "" { if user.Membership.SubscriptionModel.Name == "" {
sl.ReportError(user.Membership.SubscriptionModel.Name, "subscription.name", "name", "required", "") sl.ReportError(user.Membership.SubscriptionModel.Name, "subscription.name", "name", "required", "")
@@ -38,12 +38,41 @@ func validateUser(sl validator.StructLevel) {
user.Membership.SubscriptionModel = *selectedModel user.Membership.SubscriptionModel = *selectedModel
} }
} }
if isSupporter {
validateMembership(sl) if user.BankAccount.IBAN != "" {
if !isSuper {
validateBankAccount(sl) validateBankAccount(sl)
if user.Licence != nil && user.RoleID > constants.Roles.Supporter { }
return
}
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) 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)
} }