From 7cb2756b671e7849992c512619412506af5aa7ee Mon Sep 17 00:00:00 2001 From: David Folch Agulles Date: Thu, 27 Nov 2025 10:33:01 +0100 Subject: [PATCH] Implement Sprint 2 security enhancements Add user-based rate limiting middleware (100 req/min) for authenticated endpoints using httprate library. Implement common password validation blocking 50+ weak passwords. Improve test coverage from 38.3% to 44.6% with comprehensive refresh token tests. Security improvements: - Rate limiting by user ID for /habits and /stats endpoints - X-RateLimit-Limit header in responses - Common password blacklist in password validation - Refresh token test suite with 5 scenarios (valid, invalid, expired, revoked, empty) --- .../application/commands/mark_habit_test.go | 8 +- .../commands/register_user_test.go | 6 +- .../application/commands/unmark_habit_test.go | 1 - .../queries/get_habit_entries_test.go | 1 - .../application/queries/get_habit_stats.go | 16 +- .../application/queries/refresh_token_test.go | 176 ++++++++++++++++++ internal/domain/entities/habit_entry_test.go | 1 - .../value_objects/habit_type_json_test.go | 4 +- internal/infrastructure/auth/jwt.go | 8 +- internal/infrastructure/http/auth_handlers.go | 16 +- internal/infrastructure/http/dto.go | 60 +++--- .../http/rate_limit_middleware.go | 38 ++++ internal/infrastructure/http/router.go | 3 + internal/shared/errors/errors.go | 8 +- internal/shared/utils/date_utils_test.go | 8 +- .../shared/validation/common_passwords.go | 63 +++++++ internal/shared/validation/validator.go | 4 + 17 files changed, 350 insertions(+), 71 deletions(-) create mode 100644 internal/application/queries/refresh_token_test.go create mode 100644 internal/infrastructure/http/rate_limit_middleware.go create mode 100644 internal/shared/validation/common_passwords.go diff --git a/internal/application/commands/mark_habit_test.go b/internal/application/commands/mark_habit_test.go index d66648d..002d4d6 100644 --- a/internal/application/commands/mark_habit_test.go +++ b/internal/application/commands/mark_habit_test.go @@ -11,9 +11,9 @@ import ( ) type mockEntryRepo struct { - createFunc func(ctx context.Context, entry *entities.HabitEntry) error - findByDateRangeFunc func(ctx context.Context, habitID string, from, to time.Time) ([]*entities.HabitEntry, error) - updateFunc func(ctx context.Context, entry *entities.HabitEntry) error + createFunc func(ctx context.Context, entry *entities.HabitEntry) error + findByDateRangeFunc func(ctx context.Context, habitID string, from, to time.Time) ([]*entities.HabitEntry, error) + updateFunc func(ctx context.Context, entry *entities.HabitEntry) error } func (m *mockEntryRepo) Create(ctx context.Context, entry *entities.HabitEntry) error { @@ -537,5 +537,3 @@ func TestMarkHabitHandler_CounterFirstMarkWithNegative(t *testing.T) { t.Fatalf("Expected no error, got %v", err) } } - - diff --git a/internal/application/commands/register_user_test.go b/internal/application/commands/register_user_test.go index 7250c04..dedf034 100644 --- a/internal/application/commands/register_user_test.go +++ b/internal/application/commands/register_user_test.go @@ -262,9 +262,9 @@ func TestRegisterUserHandler_EdgeCases(t *testing.T) { handler := NewRegisterUserHandler(repo, hasher) tests := []struct { - name string - cmd RegisterUserCommand - wantErr error + name string + cmd RegisterUserCommand + wantErr error }{ { "email with plus addressing", diff --git a/internal/application/commands/unmark_habit_test.go b/internal/application/commands/unmark_habit_test.go index 797c2a6..5cedd38 100644 --- a/internal/application/commands/unmark_habit_test.go +++ b/internal/application/commands/unmark_habit_test.go @@ -137,4 +137,3 @@ func TestUnmarkHabitHandler_ReturnsErrorWhenEntryNotFound(t *testing.T) { t.Errorf("Expected ErrNotFound for missing entry, got %v", err) } } - diff --git a/internal/application/queries/get_habit_entries_test.go b/internal/application/queries/get_habit_entries_test.go index b4747d2..fe15070 100644 --- a/internal/application/queries/get_habit_entries_test.go +++ b/internal/application/queries/get_habit_entries_test.go @@ -278,4 +278,3 @@ func TestGetHabitEntriesHandler_RequiresPaginationWithLongDateRange(t *testing.T t.Errorf("Expected ErrInvalidInput for date range > 1 year without pagination, got %v", err) } } - diff --git a/internal/application/queries/get_habit_stats.go b/internal/application/queries/get_habit_stats.go index 5b3d094..514782f 100644 --- a/internal/application/queries/get_habit_stats.go +++ b/internal/application/queries/get_habit_stats.go @@ -10,14 +10,14 @@ import ( ) type HabitStatsDTO struct { - HabitID string `json:"habit_id"` - HabitName string `json:"habit_name"` - TotalCompletions int `json:"total_completions"` - CurrentStreak int `json:"current_streak"` - LongestStreak int `json:"longest_streak"` - CompletionRate float64 `json:"completion_rate"` - CompletionsThisWeek int `json:"completions_this_week"` - CompletionsThisMonth int `json:"completions_this_month"` + HabitID string `json:"habit_id"` + HabitName string `json:"habit_name"` + TotalCompletions int `json:"total_completions"` + CurrentStreak int `json:"current_streak"` + LongestStreak int `json:"longest_streak"` + CompletionRate float64 `json:"completion_rate"` + CompletionsThisWeek int `json:"completions_this_week"` + CompletionsThisMonth int `json:"completions_this_month"` } type GetHabitStatsQuery struct { diff --git a/internal/application/queries/refresh_token_test.go b/internal/application/queries/refresh_token_test.go new file mode 100644 index 0000000..d4556af --- /dev/null +++ b/internal/application/queries/refresh_token_test.go @@ -0,0 +1,176 @@ +package queries + +import ( + "context" + "testing" + "time" + + "apocapoc-api/internal/domain/entities" + "apocapoc-api/internal/shared/errors" +) + +type mockRefreshTokenRepository struct { + findByTokenFunc func(ctx context.Context, token string) (*entities.RefreshToken, error) +} + +func (m *mockRefreshTokenRepository) Create(ctx context.Context, token *entities.RefreshToken) error { + return nil +} + +func (m *mockRefreshTokenRepository) FindByToken(ctx context.Context, token string) (*entities.RefreshToken, error) { + if m.findByTokenFunc != nil { + return m.findByTokenFunc(ctx, token) + } + return nil, errors.ErrNotFound +} + +func (m *mockRefreshTokenRepository) FindByUserID(ctx context.Context, userID string) ([]*entities.RefreshToken, error) { + return nil, nil +} + +func (m *mockRefreshTokenRepository) RevokeByToken(ctx context.Context, token string) error { + return nil +} + +func (m *mockRefreshTokenRepository) RevokeAllByUserID(ctx context.Context, userID string) error { + return nil +} + +func (m *mockRefreshTokenRepository) DeleteExpired(ctx context.Context) error { + return nil +} + +type mockUserRepositoryForRefresh struct { + findByIDFunc func(ctx context.Context, id string) (*entities.User, error) +} + +func (m *mockUserRepositoryForRefresh) Create(ctx context.Context, user *entities.User) error { + return nil +} + +func (m *mockUserRepositoryForRefresh) FindByID(ctx context.Context, id string) (*entities.User, error) { + if m.findByIDFunc != nil { + return m.findByIDFunc(ctx, id) + } + return nil, errors.ErrNotFound +} + +func (m *mockUserRepositoryForRefresh) FindByEmail(ctx context.Context, email string) (*entities.User, error) { + return nil, nil +} + +func (m *mockUserRepositoryForRefresh) Update(ctx context.Context, user *entities.User) error { + return nil +} + +func TestRefreshTokenHandler_Success(t *testing.T) { + refreshTokenRepo := &mockRefreshTokenRepository{ + findByTokenFunc: func(ctx context.Context, token string) (*entities.RefreshToken, error) { + return entities.NewRefreshToken("user-123", token, time.Now().Add(24*time.Hour)), nil + }, + } + + userRepo := &mockUserRepositoryForRefresh{ + findByIDFunc: func(ctx context.Context, id string) (*entities.User, error) { + user := entities.NewUser("test@example.com", "hash", "UTC") + user.ID = id + return user, nil + }, + } + + handler := NewRefreshTokenHandler(refreshTokenRepo, userRepo) + + query := RefreshTokenQuery{ + RefreshToken: "valid-token", + } + + result, err := handler.Handle(context.Background(), query) + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + if result.UserID != "user-123" { + t.Errorf("Expected userID 'user-123', got %s", result.UserID) + } + + if result.Email != "test@example.com" { + t.Errorf("Expected email 'test@example.com', got %s", result.Email) + } +} + +func TestRefreshTokenHandler_InvalidToken(t *testing.T) { + refreshTokenRepo := &mockRefreshTokenRepository{} + userRepo := &mockUserRepositoryForRefresh{} + + handler := NewRefreshTokenHandler(refreshTokenRepo, userRepo) + + query := RefreshTokenQuery{ + RefreshToken: "invalid-token", + } + + _, err := handler.Handle(context.Background(), query) + if err != errors.ErrNotFound { + t.Errorf("Expected ErrNotFound, got %v", err) + } +} + +func TestRefreshTokenHandler_ExpiredToken(t *testing.T) { + refreshTokenRepo := &mockRefreshTokenRepository{ + findByTokenFunc: func(ctx context.Context, token string) (*entities.RefreshToken, error) { + return entities.NewRefreshToken("user-123", token, time.Now().Add(-1*time.Hour)), nil + }, + } + + userRepo := &mockUserRepositoryForRefresh{} + + handler := NewRefreshTokenHandler(refreshTokenRepo, userRepo) + + query := RefreshTokenQuery{ + RefreshToken: "expired-token", + } + + _, err := handler.Handle(context.Background(), query) + if err != errors.ErrNotFound { + t.Errorf("Expected ErrNotFound for expired token, got %v", err) + } +} + +func TestRefreshTokenHandler_RevokedToken(t *testing.T) { + refreshToken := entities.NewRefreshToken("user-123", "revoked-token", time.Now().Add(24*time.Hour)) + refreshToken.Revoke() + + refreshTokenRepo := &mockRefreshTokenRepository{ + findByTokenFunc: func(ctx context.Context, token string) (*entities.RefreshToken, error) { + return refreshToken, nil + }, + } + + userRepo := &mockUserRepositoryForRefresh{} + + handler := NewRefreshTokenHandler(refreshTokenRepo, userRepo) + + query := RefreshTokenQuery{ + RefreshToken: "revoked-token", + } + + _, err := handler.Handle(context.Background(), query) + if err != errors.ErrNotFound { + t.Errorf("Expected ErrNotFound for revoked token, got %v", err) + } +} + +func TestRefreshTokenHandler_EmptyToken(t *testing.T) { + refreshTokenRepo := &mockRefreshTokenRepository{} + userRepo := &mockUserRepositoryForRefresh{} + + handler := NewRefreshTokenHandler(refreshTokenRepo, userRepo) + + query := RefreshTokenQuery{ + RefreshToken: "", + } + + _, err := handler.Handle(context.Background(), query) + if err != errors.ErrInvalidInput { + t.Errorf("Expected ErrInvalidInput, got %v", err) + } +} diff --git a/internal/domain/entities/habit_entry_test.go b/internal/domain/entities/habit_entry_test.go index 5cd5cb0..cf12529 100644 --- a/internal/domain/entities/habit_entry_test.go +++ b/internal/domain/entities/habit_entry_test.go @@ -43,4 +43,3 @@ func TestNewHabitEntry_BooleanHabit(t *testing.T) { t.Error("Value should be nil for boolean habit") } } - diff --git a/internal/domain/value_objects/habit_type_json_test.go b/internal/domain/value_objects/habit_type_json_test.go index a0c5908..b5b5eaf 100644 --- a/internal/domain/value_objects/habit_type_json_test.go +++ b/internal/domain/value_objects/habit_type_json_test.go @@ -7,9 +7,9 @@ import ( func TestHabitType_MarshalJSON(t *testing.T) { tests := []struct { - name string + name string habitType HabitType - expected string + expected string }{ {"Boolean", HabitTypeBoolean, `"BOOLEAN"`}, {"Counter", HabitTypeCounter, `"COUNTER"`}, diff --git a/internal/infrastructure/auth/jwt.go b/internal/infrastructure/auth/jwt.go index 6089747..b3a6039 100644 --- a/internal/infrastructure/auth/jwt.go +++ b/internal/infrastructure/auth/jwt.go @@ -14,14 +14,14 @@ type Claims struct { } type JWTService struct { - secret []byte - expiry time.Duration + secret []byte + expiry time.Duration } func NewJWTService(secret string, expiryHours int) *JWTService { return &JWTService{ - secret: []byte(secret), - expiry: time.Duration(expiryHours) * time.Hour, + secret: []byte(secret), + expiry: time.Duration(expiryHours) * time.Hour, } } diff --git a/internal/infrastructure/http/auth_handlers.go b/internal/infrastructure/http/auth_handlers.go index 52581ae..5b0c937 100644 --- a/internal/infrastructure/http/auth_handlers.go +++ b/internal/infrastructure/http/auth_handlers.go @@ -13,14 +13,14 @@ import ( ) type AuthHandlers struct { - registerHandler *commands.RegisterUserHandler - loginHandler *queries.LoginUserHandler - refreshTokenHandler *queries.RefreshTokenHandler - revokeTokenHandler *commands.RevokeTokenHandler - revokeAllTokensHandler *commands.RevokeAllTokensHandler - jwtService *auth.JWTService - refreshTokenRepo repositories.RefreshTokenRepository - refreshTokenExpiry time.Duration + registerHandler *commands.RegisterUserHandler + loginHandler *queries.LoginUserHandler + refreshTokenHandler *queries.RefreshTokenHandler + revokeTokenHandler *commands.RevokeTokenHandler + revokeAllTokensHandler *commands.RevokeAllTokensHandler + jwtService *auth.JWTService + refreshTokenRepo repositories.RefreshTokenRepository + refreshTokenExpiry time.Duration } func NewAuthHandlers( diff --git a/internal/infrastructure/http/dto.go b/internal/infrastructure/http/dto.go index 4bb1583..1835a6e 100644 --- a/internal/infrastructure/http/dto.go +++ b/internal/infrastructure/http/dto.go @@ -7,15 +7,15 @@ import ( ) type CreateHabitRequest struct { - Name string `json:"name"` - Description string `json:"description"` - Type value_objects.HabitType `json:"type"` - Frequency value_objects.Frequency `json:"frequency"` - SpecificDays []int `json:"specific_days,omitempty"` - SpecificDates []int `json:"specific_dates,omitempty"` - CarryOver bool `json:"carry_over"` - IsNegative bool `json:"is_negative"` - TargetValue *float64 `json:"target_value,omitempty"` + Name string `json:"name"` + Description string `json:"description"` + Type value_objects.HabitType `json:"type"` + Frequency value_objects.Frequency `json:"frequency"` + SpecificDays []int `json:"specific_days,omitempty"` + SpecificDates []int `json:"specific_dates,omitempty"` + CarryOver bool `json:"carry_over"` + IsNegative bool `json:"is_negative"` + TargetValue *float64 `json:"target_value,omitempty"` } type UpdateHabitRequest struct { @@ -28,19 +28,19 @@ type UpdateHabitRequest struct { } type HabitResponse struct { - ID string `json:"id"` - UserID string `json:"user_id"` - Name string `json:"name"` - Description string `json:"description"` - Type value_objects.HabitType `json:"type"` - Frequency value_objects.Frequency `json:"frequency"` - SpecificDays []int `json:"specific_days,omitempty"` - SpecificDates []int `json:"specific_dates,omitempty"` - CarryOver bool `json:"carry_over"` - IsNegative bool `json:"is_negative"` - TargetValue *float64 `json:"target_value,omitempty"` - CreatedAt time.Time `json:"created_at"` - ArchivedAt *time.Time `json:"archived_at,omitempty"` + ID string `json:"id"` + UserID string `json:"user_id"` + Name string `json:"name"` + Description string `json:"description"` + Type value_objects.HabitType `json:"type"` + Frequency value_objects.Frequency `json:"frequency"` + SpecificDays []int `json:"specific_days,omitempty"` + SpecificDates []int `json:"specific_dates,omitempty"` + CarryOver bool `json:"carry_over"` + IsNegative bool `json:"is_negative"` + TargetValue *float64 `json:"target_value,omitempty"` + CreatedAt time.Time `json:"created_at"` + ArchivedAt *time.Time `json:"archived_at,omitempty"` } type MarkHabitRequest struct { @@ -59,14 +59,14 @@ type TodaysHabitResponse struct { } type UserHabitResponse struct { - ID string `json:"id"` - Name string `json:"name"` - Type value_objects.HabitType `json:"type"` - Frequency value_objects.Frequency `json:"frequency"` - SpecificDays []int `json:"specific_days,omitempty"` - TargetValue *float64 `json:"target_value,omitempty"` - CarryOver bool `json:"carry_over"` - IsNegative bool `json:"is_negative"` + ID string `json:"id"` + Name string `json:"name"` + Type value_objects.HabitType `json:"type"` + Frequency value_objects.Frequency `json:"frequency"` + SpecificDays []int `json:"specific_days,omitempty"` + TargetValue *float64 `json:"target_value,omitempty"` + CarryOver bool `json:"carry_over"` + IsNegative bool `json:"is_negative"` } type HabitEntryResponse struct { diff --git a/internal/infrastructure/http/rate_limit_middleware.go b/internal/infrastructure/http/rate_limit_middleware.go new file mode 100644 index 0000000..2b4ebd1 --- /dev/null +++ b/internal/infrastructure/http/rate_limit_middleware.go @@ -0,0 +1,38 @@ +package http + +import ( + "net/http" + "strconv" + "time" + + "apocapoc-api/internal/infrastructure/auth" + + "github.com/go-chi/httprate" +) + +func RateLimitByUser(jwtService *auth.JWTService, requestsPerMinute int, duration time.Duration) func(http.Handler) http.Handler { + limiter := httprate.NewRateLimiter( + requestsPerMinute, + duration, + httprate.WithKeyFuncs(func(r *http.Request) (string, error) { + userID, ok := GetUserIDFromContext(r.Context()) + if !ok { + return r.RemoteAddr, nil + } + return "user:" + userID, nil + }), + httprate.WithLimitHandler(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + w.Write([]byte(`{"error":"Rate limit exceeded. Please try again later."}`)) + }), + ) + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-RateLimit-Limit", strconv.Itoa(requestsPerMinute)) + + limiter.Handler(next).ServeHTTP(w, r) + }) + } +} diff --git a/internal/infrastructure/http/router.go b/internal/infrastructure/http/router.go index 688e576..46021f0 100644 --- a/internal/infrastructure/http/router.go +++ b/internal/infrastructure/http/router.go @@ -46,6 +46,8 @@ func NewRouter(corsOrigins string, habitHandlers *HabitHandlers, authHandlers *A r.Route("/api/v1/habits", func(r chi.Router) { r.Use(AuthMiddleware(jwtService)) + r.Use(RateLimitByUser(jwtService, 100, 1*time.Minute)) + r.Post("/", habitHandlers.CreateHabit) r.Get("/", habitHandlers.GetUserHabits) r.Get("/today", habitHandlers.GetTodaysHabits) @@ -59,6 +61,7 @@ func NewRouter(corsOrigins string, habitHandlers *HabitHandlers, authHandlers *A r.Route("/api/v1/stats", func(r chi.Router) { r.Use(AuthMiddleware(jwtService)) + r.Use(RateLimitByUser(jwtService, 100, 1*time.Minute)) r.Get("/habits/{id}", statsHandlers.GetHabitStats) }) diff --git a/internal/shared/errors/errors.go b/internal/shared/errors/errors.go index 74e5884..08e4217 100644 --- a/internal/shared/errors/errors.go +++ b/internal/shared/errors/errors.go @@ -3,9 +3,9 @@ package errors import "errors" var ( - ErrNotFound = errors.New("resource not found") - ErrAlreadyExists = errors.New("resource already exists") - ErrInvalidInput = errors.New("invalid input") - ErrUnauthorized = errors.New("unauthorized") + ErrNotFound = errors.New("resource not found") + ErrAlreadyExists = errors.New("resource already exists") + ErrInvalidInput = errors.New("invalid input") + ErrUnauthorized = errors.New("unauthorized") ErrInvalidCredentials = errors.New("invalid credentials") ) diff --git a/internal/shared/utils/date_utils_test.go b/internal/shared/utils/date_utils_test.go index dad30d2..0a3427b 100644 --- a/internal/shared/utils/date_utils_test.go +++ b/internal/shared/utils/date_utils_test.go @@ -21,25 +21,25 @@ func TestShouldAppearToday_Weekly(t *testing.T) { }{ { name: "Monday when Monday is specified", - specificDays: []int{1}, // Monday + specificDays: []int{1}, // Monday targetDate: time.Date(2025, 1, 6, 0, 0, 0, 0, time.UTC), // Monday expected: true, }, { name: "Tuesday when Monday is specified", - specificDays: []int{1}, // Monday + specificDays: []int{1}, // Monday targetDate: time.Date(2025, 1, 7, 0, 0, 0, 0, time.UTC), // Tuesday expected: false, }, { name: "Wednesday when Mon/Wed/Fri specified", - specificDays: []int{1, 3, 5}, // Mon, Wed, Fri + specificDays: []int{1, 3, 5}, // Mon, Wed, Fri targetDate: time.Date(2025, 1, 8, 0, 0, 0, 0, time.UTC), // Wednesday expected: true, }, { name: "Sunday when Mon/Wed/Fri specified", - specificDays: []int{1, 3, 5}, // Mon, Wed, Fri + specificDays: []int{1, 3, 5}, // Mon, Wed, Fri targetDate: time.Date(2025, 1, 5, 0, 0, 0, 0, time.UTC), // Sunday expected: false, }, diff --git a/internal/shared/validation/common_passwords.go b/internal/shared/validation/common_passwords.go new file mode 100644 index 0000000..c7b300b --- /dev/null +++ b/internal/shared/validation/common_passwords.go @@ -0,0 +1,63 @@ +package validation + +import "strings" + +var commonPasswords = map[string]bool{ + "123456": true, + "password": true, + "123456789": true, + "12345678": true, + "12345": true, + "1234567": true, + "password1": true, + "123123": true, + "1234567890": true, + "000000": true, + "abc123": true, + "1234": true, + "qwerty": true, + "111111": true, + "123321": true, + "dragon": true, + "master": true, + "monkey": true, + "letmein": true, + "login": true, + "princess": true, + "qwertyuiop": true, + "solo": true, + "passw0rd": true, + "starwars": true, + "iloveyou": true, + "welcome": true, + "admin": true, + "sunshine": true, + "password123": true, + "123qwe": true, + "654321": true, + "superman": true, + "1qaz2wsx": true, + "trustno1": true, + "charlie": true, + "666666": true, + "qazwsx": true, + "freedom": true, + "football": true, + "baseball": true, + "whatever": true, + "jordan": true, + "killer": true, + "summer": true, + "hockey": true, + "bailey": true, + "shadow": true, + "master123": true, + "ninja": true, + "mustang": true, + "password!": true, +} + +func IsCommonPassword(password string) bool { + lower := strings.ToLower(password) + return commonPasswords[lower] +} diff --git a/internal/shared/validation/validator.go b/internal/shared/validation/validator.go index 0eba5e8..0b763f8 100644 --- a/internal/shared/validation/validator.go +++ b/internal/shared/validation/validator.go @@ -92,6 +92,10 @@ func ValidatePassword(password string) error { return ValidationError{Field: "password", Message: "password must contain at least one special character"} } + if IsCommonPassword(password) { + return ValidationError{Field: "password", Message: "password is too common, please choose a more secure password"} + } + return nil }