diff --git a/.gitignore b/.gitignore index d941e23..66bcde4 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ internal/db/.env tmp/ .obsidian/ +TODO.md diff --git a/docs/LEARNING.md b/docs/LEARNING.md index e69de29..f8849f2 100644 --- a/docs/LEARNING.md +++ b/docs/LEARNING.md @@ -0,0 +1,11 @@ +## Date: 2025-11-14 +### Password Validation + +[OWASP](https://owasp.org/) no longer recommends strict composition rules like: +“must contain 1 uppercase” +“must contain 1 lowercase” +“must contain 1 number” +“must contain 1 special character” +Why? + +Because composition rules do NOT significantly improve security and they make passwords harder for users to remember! diff --git a/internal/domain/user/entity.go b/internal/domain/user/entity.go index ba8fcc5..34f6afe 100644 --- a/internal/domain/user/entity.go +++ b/internal/domain/user/entity.go @@ -10,7 +10,7 @@ import ( type User struct { ID int `gorm:"primaryKey" json:"id"` Email string `gorm:"uniqueIndex;size:255;not null" json:"email"` - Password string `gorm:"size:255;not null" json:"password,omitempty"` + Password string `gorm:"size:255;not null" json:"-"` Tasks []task.Task `json:"tasks" gorm:"foreignKey:UserID;constraint:OnDelete:CASCADE"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` diff --git a/internal/dto/user.go b/internal/dto/user.go index a812699..4e1e6fe 100644 --- a/internal/dto/user.go +++ b/internal/dto/user.go @@ -2,7 +2,7 @@ package dto type CreateUserRequest struct { Email string `json:"email" binding:"required,email" example:"john@example.com"` - Password string `json:"password" binding:"required,min=6" example:"strongpassword123"` + Password string `json:"password" binding:"required,min=8,max=128" example:"strongpassword123"` } type CreateUserResponse struct { @@ -12,7 +12,7 @@ type CreateUserResponse struct { type AuthRequest struct { Email string `json:"email" binding:"required,email" example:"john@example.com"` - Password string `json:"password" binding:"required" example:"strongpassword123"` + Password string `json:"password" binding:"required,min=6" example:"strongpassword123"` } type AuthResponse struct { @@ -31,8 +31,8 @@ type DeleteUserResponse struct { type UpdatePasswordRequest struct { ID int `json:"id" binding:"required" example:"1"` - OldPassword string `json:"old_password" binding:"required" example:"oldpassword123"` - NewPassword string `json:"new_password" binding:"required,min=6" example:"newsecurepassword456"` + OldPassword string `json:"old_password" binding:"required,min=8,max=128" example:"oldpassword123"` + NewPassword string `json:"new_password" binding:"required,min=8,max=128" example:"newsecurepassword456"` } type UpdatePasswordResponse struct { diff --git a/internal/handler/user/user_handler.go b/internal/handler/user/user_handler.go index d29b522..45ef5f2 100644 --- a/internal/handler/user/user_handler.go +++ b/internal/handler/user/user_handler.go @@ -38,6 +38,14 @@ var _ UserHandlerInterface = (*UserHandler)(nil) func (h *UserHandler) Register(c *gin.Context) { var req dto.CreateUserRequest if err := c.ShouldBindJSON(&req); err != nil { + + if err.Error() == "EOF" { + c.JSON(http.StatusBadRequest, common.ErrorResponse{ + Message: "Request body cannot be empty", + }) + return + } + c.JSON(http.StatusBadRequest, common.ErrorResponse{Message: err.Error()}) return } @@ -70,6 +78,14 @@ func (h *UserHandler) Register(c *gin.Context) { func (h *UserHandler) Login(c *gin.Context) { var req dto.AuthRequest if err := c.ShouldBindJSON(&req); err != nil { + + if err.Error() == "EOF" { + c.JSON(http.StatusBadRequest, common.ErrorResponse{ + Message: "Request body cannot be empty", + }) + return + } + c.JSON(http.StatusBadRequest, common.ErrorResponse{Message: err.Error()}) return } @@ -103,6 +119,14 @@ func (h *UserHandler) Login(c *gin.Context) { func (h *UserHandler) UpdatePassword(c *gin.Context) { var req dto.UpdatePasswordRequest if err := c.ShouldBindJSON(&req); err != nil { + + if err.Error() == "EOF" { + c.JSON(http.StatusBadRequest, common.ErrorResponse{ + Message: "Request body cannot be empty", + }) + return + } + c.JSON(http.StatusBadRequest, common.ErrorResponse{Message: err.Error()}) return } diff --git a/internal/handler/user/user_handler_test.go b/internal/handler/user/user_handler_test.go index 501b777..233209e 100644 --- a/internal/handler/user/user_handler_test.go +++ b/internal/handler/user/user_handler_test.go @@ -6,6 +6,7 @@ import ( "errors" "net/http" "net/http/httptest" + "taskflow/internal/auth" "taskflow/internal/common" "taskflow/internal/dto" user_service "taskflow/internal/service/user" @@ -75,6 +76,15 @@ func TestUserHandler_Register(t *testing.T) { Message: "invalid character '}' looking for beginning of value", }, }, + { + name: "failure case - empty body", + requestBody: ``, + setupMock: func() *user_service.UserServiceMock { return new(user_service.UserServiceMock) }, + expectedStatus: http.StatusBadRequest, + expectedBody: common.ErrorResponse{ + Message: "Request body cannot be empty", + }, + }, } for _, tt := range tests { @@ -114,3 +124,22 @@ func TestUserHandler_Register(t *testing.T) { }) } } + +func TestUserHandler_Login(t *testing.T) { + tests := []struct { + name string // description of this test case + // Named input parameters for receiver constructor. + s user_service.UserServiceInterface + ua auth.UserAuthInterface + // Named input parameters for target function. + c *gin.Context + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := NewUserHandler(tt.s, tt.ua) + h.Login(tt.c) + }) + } +} diff --git a/internal/repository/gorm/gorm_user/user_repository.go b/internal/repository/gorm/gorm_user/user_repository.go index 615b7ce..dd0cc3d 100644 --- a/internal/repository/gorm/gorm_user/user_repository.go +++ b/internal/repository/gorm/gorm_user/user_repository.go @@ -15,8 +15,7 @@ func NewUserRepository(db *gorm.DB) *UserRepository { return &UserRepository{db: db} } -// TODO: -// var _ UserRepositoryInterface = (*UserRepository)(nil) +var _ UserRepositoryInterface = (*UserRepository)(nil) func (r *UserRepository) Create(u *user.User) error { return r.db.Create(u).Error diff --git a/internal/service/user/user_service.go b/internal/service/user/user_service.go index bf8ab03..b71e05b 100644 --- a/internal/service/user/user_service.go +++ b/internal/service/user/user_service.go @@ -10,6 +10,7 @@ import ( "taskflow/internal/repository/gorm/gorm_user" "taskflow/pkg" "taskflow/pkg/jwt" + "taskflow/pkg/validator" "golang.org/x/crypto/bcrypt" "gorm.io/gorm" @@ -32,8 +33,10 @@ func (s *UserService) CreateUser(req *dto.CreateUserRequest) (*dto.CreateUserRes if req.Password == "" { return nil, errors.New("password is required") } - if len(req.Password) < 6 { - return nil, errors.New("password must be at least 6 characters") + + validator := validator.NewPasswordValidator() + if err := validator.Validate(req.Password); err != nil { + return nil, errors.New("password validation failed, choose a stronger password") } req.Email = strings.ToLower(strings.TrimSpace(req.Email)) @@ -103,8 +106,10 @@ func (s *UserService) UpdatePassword(req *dto.UpdatePasswordRequest) (*dto.Updat if req.NewPassword == "" { return nil, errors.New("new password is required") } - if len(req.NewPassword) < 6 { - return nil, errors.New("new password must be at least 6 characters") + + validator := validator.NewPasswordValidator() + if err := validator.Validate(req.NewPassword); err != nil { + return nil, errors.New("password validation failed, choose a stronger password") } u, err := s.repo.GetByID(req.ID) diff --git a/internal/service/user/user_service_test.go b/internal/service/user/user_service_test.go index e2e8d22..51b134a 100644 --- a/internal/service/user/user_service_test.go +++ b/internal/service/user/user_service_test.go @@ -27,6 +27,12 @@ func TestCreateUser(t *testing.T) { }, wantErr: false, }, + { + name: "weak password", + req: &dto.CreateUserRequest{Email: "dup@example.com", Password: "12345678"}, + mockSetup: nil, + wantErr: true, + }, { name: "duplicate email", req: &dto.CreateUserRequest{Email: "dup@example.com", Password: "secret123"}, @@ -139,7 +145,7 @@ func TestUpdatePassword(t *testing.T) { req: &dto.UpdatePasswordRequest{ ID: 2, OldPassword: "wrongpass", - NewPassword: "newpass", + NewPassword: "newstrongpassword", }, mockSetup: func(m *gorm_user.MockUserRepository) { u := &user.User{ID: 2, Email: "y@example.com", Password: oldHash} @@ -147,6 +153,13 @@ func TestUpdatePassword(t *testing.T) { }, wantErr: true, }, + + { + name: "weak new password", + req: &dto.UpdatePasswordRequest{OldPassword: "someoldpassword", NewPassword: "12345678"}, + mockSetup: nil, + wantErr: true, + }, } for _, tt := range tests { diff --git a/pkg/jwt/jwt.go b/pkg/jwt/jwt.go index a7e5780..4a53d6a 100644 --- a/pkg/jwt/jwt.go +++ b/pkg/jwt/jwt.go @@ -18,7 +18,7 @@ var ( ) const ( - TokenExpiration = 24 * time.Hour + TokenExpiration = 30 * time.Minute UserIDClaimKey = "user_id" ExpirationClaimKey = "exp" ) diff --git a/pkg/validator/password_validation.go b/pkg/validator/password_validation.go new file mode 100644 index 0000000..9d127ac --- /dev/null +++ b/pkg/validator/password_validation.go @@ -0,0 +1,130 @@ +package validator + +import ( + "errors" + "strings" + "unicode" +) + +var ( + ErrPasswordTooShort = errors.New("password must be at least 8 characters long") + ErrPasswordTooLong = errors.New("password must not exceed 128 characters") + ErrPasswordTooCommon = errors.New("password is too common and easily guessable") + ErrPasswordRepeating = errors.New("password contains too many repeating characters") + ErrPasswordAllNumeric = errors.New("password cannot be all numeric") + ErrPasswordWhitespace = errors.New("password cannot contain leading or trailing whitespace") +) + +var commonPasswords = map[string]bool{ + "password": true, + "12345678": true, + "123456789": true, + "password1": true, + "password123": true, + "qwerty": true, + "abc123": true, + "monkey": true, + "letmein": true, + "trustno1": true, + "dragon": true, + "baseball": true, + "iloveyou": true, + "master": true, + "sunshine": true, + "ashley": true, + "bailey": true, + "shadow": true, + "superman": true, +} + +// PasswordValidator provides comprehensive password validation +type PasswordValidator struct { + MinLength int + MaxLength int + CheckCommon bool + CheckRepeating bool + CheckAllNumeric bool + CheckWhitespace bool +} + +// NewPasswordValidator creates a validator with NIST compliant defaults +func NewPasswordValidator() *PasswordValidator { + return &PasswordValidator{ + MinLength: 8, + MaxLength: 128, + CheckCommon: true, + CheckRepeating: true, + CheckAllNumeric: true, + CheckWhitespace: true, + } +} + +// Validate performs comprehensive password validation +func (v *PasswordValidator) Validate(password string) error { + if v.CheckWhitespace && (strings.HasPrefix(password, " ") || strings.HasSuffix(password, " ")) { + return ErrPasswordWhitespace + } + + if len(password) < v.MinLength { + return ErrPasswordTooShort + } + if len(password) > v.MaxLength { + return ErrPasswordTooLong + } + + if v.CheckCommon && v.isCommonPassword(password) { + return ErrPasswordTooCommon + } + + if v.CheckAllNumeric && v.isAllNumeric(password) { + return ErrPasswordAllNumeric + } + + if v.CheckRepeating && v.hasExcessiveRepeating(password) { + return ErrPasswordRepeating + } + + return nil +} + +// isCommonPassword checks against known common passwords +func (v *PasswordValidator) isCommonPassword(password string) bool { + lower := strings.ToLower(password) + return commonPasswords[lower] +} + +// isAllNumeric checks if password is all numeric +func (v *PasswordValidator) isAllNumeric(password string) bool { + for _, r := range password { + if !unicode.IsDigit(r) { + return false + } + } + return len(password) > 0 +} + +// hasExcessiveRepeating checks for patterns like "aaaa" or "1111" +func (v *PasswordValidator) hasExcessiveRepeating(password string) bool { + if len(password) < 4 { + return false + } + + consecutiveCount := 1 + for i := 1; i < len(password); i++ { + if password[i] == password[i-1] { + consecutiveCount++ + if consecutiveCount >= 4 { + return true + } + } else { + consecutiveCount = 1 + } + } + return false +} + +// Custom Gin validator function +func ValidatePassword(password string) error { + validator := NewPasswordValidator() + return validator.Validate(password) +} diff --git a/pkg/validator/password_validation_test.go b/pkg/validator/password_validation_test.go new file mode 100644 index 0000000..5e5a598 --- /dev/null +++ b/pkg/validator/password_validation_test.go @@ -0,0 +1,375 @@ +package validator + +import ( + "testing" +) + +func TestPasswordValidator_Validate(t *testing.T) { + tests := []struct { + name string + password string + wantErr error + }{ + { + name: "valid strong password", + password: "MySecureP@ssw0rd", + wantErr: nil, + }, + { + name: "valid minimum length", + password: "abcd1234", + wantErr: nil, + }, + { + name: "valid long password", + password: "ThisIsAVeryLongPasswordThatIsStillValidAndSecure123456", + wantErr: nil, + }, + { + name: "valid with special characters", + password: "P@ssw0rd!#$%", + wantErr: nil, + }, + { + name: "valid with spaces in middle", + password: "my secure password 123", + wantErr: nil, + }, + + { + name: "too short - 7 characters", + password: "pass123", + wantErr: ErrPasswordTooShort, + }, + { + name: "too short - empty", + password: "", + wantErr: ErrPasswordTooShort, + }, + { + name: "too short - 1 character", + password: "a", + wantErr: ErrPasswordTooShort, + }, + { + name: "too long - 129 characters", + password: "a" + string(make([]byte, 128)), + wantErr: ErrPasswordTooLong, + }, + + { + name: "common password - password", + password: "password", + wantErr: ErrPasswordTooCommon, + }, + { + name: "common password - 12345678", + password: "12345678", + wantErr: ErrPasswordTooCommon, + }, + { + name: "common password - password123", + password: "password123", + wantErr: ErrPasswordTooCommon, + }, + { + name: "common password - iloveyou", + password: "iloveyou", + wantErr: ErrPasswordTooCommon, + }, + { + name: "common password - uppercase PASSWORD", + password: "PASSWORD", + wantErr: ErrPasswordTooCommon, + }, + { + name: "common password - mixed case PaSsWoRd", + password: "PaSsWoRd", + wantErr: ErrPasswordTooCommon, + }, + + { + name: "all numeric - 8 digits", + password: "87654321", + wantErr: ErrPasswordAllNumeric, + }, + { + name: "all numeric - long", + password: "123456789012345", + wantErr: ErrPasswordAllNumeric, + }, + + { + name: "excessive repeating - aaaa", + password: "aaaaaaaa", + wantErr: ErrPasswordRepeating, + }, + { + name: "excessive repeating - in middle", + password: "pass1111word", + wantErr: ErrPasswordRepeating, + }, + { + name: "excessive repeating - at end", + password: "password1111", + wantErr: ErrPasswordRepeating, + }, + { + name: "acceptable repeating - 3 consecutive", + password: "passs123", + wantErr: nil, + }, + { + name: "acceptable repeating - non-consecutive", + password: "abababab", + wantErr: nil, + }, + + { + name: "leading whitespace", + password: " password123", + wantErr: ErrPasswordWhitespace, + }, + { + name: "trailing whitespace", + password: "password123 ", + wantErr: ErrPasswordWhitespace, + }, + { + name: "both leading and trailing whitespace", + password: " password123 ", + wantErr: ErrPasswordWhitespace, + }, + { + name: "multiple leading spaces", + password: " password123", + wantErr: ErrPasswordWhitespace, + }, + { + name: "multiple trailing spaces", + password: "password123 ", + wantErr: ErrPasswordWhitespace, + }, + + { + name: "exactly 8 characters", + password: "abcdefgh", + wantErr: nil, + }, + { + name: "exactly 128 characters", + password: "alkfdjalskdfjal;skdjfa;kljf alksjfda;lkfq321jfal;kdjc alsdfj;lkafdsjdlkaj fslkdjasldkfja;lskdfj;alksjdf;alksjdf;alksjdf;alksjdwd", + wantErr: nil, + }, + { + name: "unicode characters", + password: "pässwörd123", + wantErr: nil, + }, + { + name: "emoji in password", + password: "password😀123", + wantErr: nil, + }, + } + + validator := NewPasswordValidator() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validator.Validate(tt.password) + + if err != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestValidatePassword(t *testing.T) { + tests := []struct { + name string + password string + wantErr error + }{ + { + name: "valid password", + password: "SecurePass123", + wantErr: nil, + }, + { + name: "invalid - too short", + password: "short1", + wantErr: ErrPasswordTooShort, + }, + { + name: "invalid - common", + password: "password", + wantErr: ErrPasswordTooCommon, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidatePassword(tt.password) + + if err != tt.wantErr { + t.Errorf("ValidatePassword() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestPasswordValidator_CustomConfig(t *testing.T) { + tests := []struct { + name string + validator *PasswordValidator + password string + wantErr error + }{ + { + name: "custom min length - 12", + validator: &PasswordValidator{ + MinLength: 12, + MaxLength: 128, + CheckCommon: false, + CheckRepeating: false, + CheckAllNumeric: false, + CheckWhitespace: false, + }, + password: "short123456", + wantErr: ErrPasswordTooShort, + }, + { + name: "disable common check", + validator: &PasswordValidator{ + MinLength: 8, + MaxLength: 128, + CheckCommon: false, + CheckRepeating: false, + CheckAllNumeric: false, + CheckWhitespace: false, + }, + password: "password", + wantErr: nil, + }, + { + name: "disable repeating check", + validator: &PasswordValidator{ + MinLength: 8, + MaxLength: 128, + CheckCommon: false, + CheckRepeating: false, + CheckAllNumeric: false, + CheckWhitespace: false, + }, + password: "aaaaaaaaaa", + wantErr: nil, + }, + { + name: "disable all numeric check", + validator: &PasswordValidator{ + MinLength: 8, + MaxLength: 128, + CheckCommon: false, + CheckRepeating: false, + CheckAllNumeric: false, + CheckWhitespace: false, + }, + password: "12345678", + wantErr: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.validator.Validate(tt.password) + + if err != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestPasswordValidator_isCommonPassword(t *testing.T) { + validator := NewPasswordValidator() + + tests := []struct { + name string + password string + want bool + }{ + {"common - password", "password", true}, + {"common - uppercase", "PASSWORD", true}, + {"common - mixed case", "PaSsWoRd", true}, + {"common - qwerty", "qwerty", true}, + {"not common", "UniqueP@ssw0rd", false}, + {"not common - similar to common", "password1234", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := validator.isCommonPassword(tt.password) + if got != tt.want { + t.Errorf("isCommonPassword() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestPasswordValidator_isAllNumeric(t *testing.T) { + validator := NewPasswordValidator() + + tests := []struct { + name string + password string + want bool + }{ + {"all numeric", "12345678", true}, + {"all numeric - long", "123456789012345", true}, + {"not numeric - letters", "abc12345", false}, + {"not numeric - special chars", "123!456", false}, + {"empty", "", false}, + {"single digit", "1", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := validator.isAllNumeric(tt.password) + if got != tt.want { + t.Errorf("isAllNumeric() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestPasswordValidator_hasExcessiveRepeating(t *testing.T) { + validator := NewPasswordValidator() + + tests := []struct { + name string + password string + want bool + }{ + {"4 consecutive same char", "aaaa", true}, + {"5 consecutive same char", "aaaaa", true}, + {"4 consecutive in middle", "pass1111word", true}, + {"3 consecutive - acceptable", "aaa", false}, + {"3 consecutive in password", "pass111word", false}, + {"non-consecutive repeating", "abababab", false}, + {"short password", "abc", false}, + {"no repeating", "abcdefgh", false}, + {"4 at end", "password1111", true}, + {"4 at start", "1111password", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := validator.hasExcessiveRepeating(tt.password) + if got != tt.want { + t.Errorf("hasExcessiveRepeating() = %v, want %v", got, tt.want) + } + }) + } +}