diff --git a/cmd/web/router/routes.go b/cmd/web/router/routes.go index ad42b18..4d83f5f 100644 --- a/cmd/web/router/routes.go +++ b/cmd/web/router/routes.go @@ -47,7 +47,7 @@ func RegisterAdminRoutes(e *echo.Echo, userHandler *user.Handler, companyHandler adminUsersGP.PUT("/:id", userHandler.UpdateUser) adminUsersGP.DELETE("/:id", userHandler.DeleteUser) adminUsersGP.PUT("/:id/status", userHandler.UpdateUserStatus) - adminUsersGP.POST("/:id/reset-password", userHandler.ResetUserPassword) + adminUsersGP.POST("/:id/reset-password", userHandler.ResetUserPassword, userHandler.AdminMiddleware()) } // Admin user profile diff --git a/internal/customer/admin_reset_password.go b/internal/customer/admin_reset_password.go index d669846..f887e6d 100644 --- a/internal/customer/admin_reset_password.go +++ b/internal/customer/admin_reset_password.go @@ -36,7 +36,7 @@ func (s *customerService) AdminResetPassword(ctx context.Context, actorID, targe customer, err := s.repository.GetByID(ctx, targetID) if err != nil { - if err.Error() == "customer not found" { + if errors.Is(err, ErrCustomerNotFound) { return nil, errCustomerNotFound } s.logger.Error("Failed to load customer for password reset", map[string]interface{}{ @@ -68,9 +68,10 @@ func (s *customerService) AdminResetPassword(ctx context.Context, actorID, targe return nil, errCustResetPasswordFailed } - customer.Password = string(hashedPassword) - customer.UpdatedBy = &actorID - if err := s.repository.Update(ctx, customer); err != nil { + if err := s.repository.UpdatePasswordReset(ctx, targetID, string(hashedPassword), actorID); err != nil { + if errors.Is(err, ErrCustomerNotFound) { + return nil, errCustomerNotFound + } s.logger.Error("Failed to persist reset password", map[string]interface{}{ "error": err.Error(), "customer_id": targetID, @@ -83,7 +84,6 @@ func (s *customerService) AdminResetPassword(ctx context.Context, actorID, targe "error": err.Error(), "customer_id": targetID, }) - return nil, errCustResetPasswordFailed } s.auditLogger.Log(ctx, audit.Entry{ diff --git a/internal/customer/errors.go b/internal/customer/errors.go new file mode 100644 index 0000000..28c0e8d --- /dev/null +++ b/internal/customer/errors.go @@ -0,0 +1,7 @@ +package customer + +import "errors" + +var ( + ErrCustomerNotFound = errors.New("customer not found") +) diff --git a/internal/customer/handler.go b/internal/customer/handler.go index 9e47d87..410296d 100644 --- a/internal/customer/handler.go +++ b/internal/customer/handler.go @@ -1,6 +1,7 @@ package customer import ( + "errors" "strings" "tm/internal/user" "tm/pkg/authorization" @@ -268,14 +269,14 @@ func (h *Handler) ResetCustomerPassword(c echo.Context) error { targetID := c.Param("id") result, err := h.service.AdminResetPassword(c.Request().Context(), actorID, targetID) if err != nil { - switch err.Error() { - case errInvalidCustomerID.Error(): + switch { + case errors.Is(err, errInvalidCustomerID): return response.BadRequest(c, "Invalid id", "") - case errCustomerNotFound.Error(): + case errors.Is(err, errCustomerNotFound): return response.NotFound(c, "Customer not found") - case errCannotResetCustPassword.Error(): + case errors.Is(err, errCannotResetCustPassword): return response.Conflict(c, "Cannot reset password for this user") - case errCustResetPasswordRate.Error(): + case errors.Is(err, errCustResetPasswordRate): return response.TooManyRequests(c, "Too many password reset requests") default: return response.InternalServerError(c, "Failed to reset password") diff --git a/internal/customer/repository.go b/internal/customer/repository.go index ba9bb24..2a409c0 100644 --- a/internal/customer/repository.go +++ b/internal/customer/repository.go @@ -3,12 +3,14 @@ package customer import ( "context" "errors" + "fmt" "time" "tm/pkg/logger" orm "tm/pkg/mongo" "tm/pkg/response" "go.mongodb.org/mongo-driver/v2/bson" + mongopkg "go.mongodb.org/mongo-driver/v2/mongo" ) // Repository defines the interface for customer data operations @@ -16,6 +18,7 @@ type Repository interface { Login(ctx context.Context, id, deviceToken string) error Register(ctx context.Context, customer *Customer) error Update(ctx context.Context, customer *Customer) error + UpdatePasswordReset(ctx context.Context, id, hashedPassword, updatedBy string) error UpdateStatus(ctx context.Context, id string, status CustomerStatus) error GetByID(ctx context.Context, id string) (*Customer, error) GetByEmail(ctx context.Context, email string) (*Customer, error) @@ -30,8 +33,9 @@ type Repository interface { // customerRepository implements the Repository interface using the MongoDB ORM type customerRepository struct { - ormRepo orm.Repository[Customer] - logger logger.Logger + ormRepo orm.Repository[Customer] + collection *mongopkg.Collection + logger logger.Logger } func collection() string { @@ -55,11 +59,13 @@ func NewRepository(mongoManager *orm.ConnectionManager, logger logger.Logger) Re } // Create ORM repository - ormRepo := orm.NewRepository[Customer](mongoManager.GetCollection(collection()), logger) + collection := mongoManager.GetCollection(collection()) + ormRepo := orm.NewRepository[Customer](collection, logger) return &customerRepository{ - ormRepo: ormRepo, - logger: logger, + ormRepo: ormRepo, + collection: collection, + logger: logger, } } @@ -128,7 +134,7 @@ func (r *customerRepository) GetByID(ctx context.Context, id string) (*Customer, customer, err := r.ormRepo.FindByID(ctx, id) if err != nil { if errors.Is(err, orm.ErrDocumentNotFound) { - return nil, errors.New("customer not found") + return nil, ErrCustomerNotFound } r.logger.Error("Failed to get customer by ID", map[string]interface{}{ "error": err.Error(), @@ -146,7 +152,7 @@ func (r *customerRepository) GetByEmail(ctx context.Context, email string) (*Cus customer, err := r.ormRepo.FindOne(ctx, filter) if err != nil { if errors.Is(err, orm.ErrDocumentNotFound) { - return nil, errors.New("customer not found") + return nil, ErrCustomerNotFound } r.logger.Error("Failed to get customer by email", map[string]interface{}{ "error": err.Error(), @@ -164,7 +170,7 @@ func (r *customerRepository) GetByUsername(ctx context.Context, username string) customer, err := r.ormRepo.FindOne(ctx, filter) if err != nil { if errors.Is(err, orm.ErrDocumentNotFound) { - return nil, errors.New("customer not found") + return nil, ErrCustomerNotFound } r.logger.Error("Failed to get customer by username", map[string]interface{}{ "error": err.Error(), @@ -295,6 +301,37 @@ func (r *customerRepository) Update(ctx context.Context, customer *Customer) err return nil } +// UpdatePasswordReset atomically persists a new password hash and audit fields. +func (r *customerRepository) UpdatePasswordReset(ctx context.Context, id, hashedPassword, updatedBy string) error { + now := time.Now().Unix() + updateDoc := bson.M{ + "$set": bson.M{ + "password": hashedPassword, + "updated_at": now, + "updated_by": updatedBy, + }, + } + + objectID, err := bson.ObjectIDFromHex(id) + if err != nil { + return fmt.Errorf("invalid id format: %w", err) + } + + result, err := r.collection.UpdateOne(ctx, bson.M{"_id": objectID}, updateDoc) + if err != nil { + r.logger.Error("Failed to update password reset", map[string]interface{}{ + "error": err.Error(), + "customer_id": id, + }) + return err + } + if result.MatchedCount == 0 { + return ErrCustomerNotFound + } + + return nil +} + // Delete deletes a customer (hard delete) func (r *customerRepository) Delete(ctx context.Context, id string) error { // Get customer first diff --git a/internal/user/errors.go b/internal/user/errors.go new file mode 100644 index 0000000..2b9a669 --- /dev/null +++ b/internal/user/errors.go @@ -0,0 +1,7 @@ +package user + +import "errors" + +var ( + ErrUserNotFound = errors.New("user not found") +) diff --git a/internal/user/handler.go b/internal/user/handler.go index 976f91d..f2c5960 100644 --- a/internal/user/handler.go +++ b/internal/user/handler.go @@ -1,6 +1,7 @@ package user import ( + "errors" "tm/pkg/authorization" "tm/pkg/logger" "tm/pkg/response" @@ -484,17 +485,24 @@ func (h *Handler) ResetUserPassword(c echo.Context) error { return response.Unauthorized(c, "Unauthorized") } - targetID := c.Param("id") - result, err := h.service.AdminResetPassword(c.Request().Context(), actorID, targetID) + actorRole, err := GetUserRoleFromContext(c) if err != nil { - switch err.Error() { - case errInvalidUserID.Error(): + return response.Unauthorized(c, "Unauthorized") + } + + targetID := c.Param("id") + result, err := h.service.AdminResetPassword(c.Request().Context(), actorID, actorRole, targetID) + if err != nil { + switch { + case errors.Is(err, errInvalidUserID): return response.BadRequest(c, "Invalid id", "") - case errAdminUserNotFound.Error(): + case errors.Is(err, errAdminUserNotFound): return response.NotFound(c, "Admin not found") - case errCannotResetPassword.Error(): + case errors.Is(err, errCannotResetPassword): return response.Conflict(c, "Cannot reset password for this user") - case errResetPasswordRateHit.Error(): + case errors.Is(err, errResetPasswordForbidden): + return response.Forbidden(c, "You are not allowed to reset this password") + case errors.Is(err, errResetPasswordRateHit): return response.TooManyRequests(c, "Too many password reset requests") default: return response.InternalServerError(c, "Failed to reset password") diff --git a/internal/user/repository.go b/internal/user/repository.go index 1e923ec..09796d3 100644 --- a/internal/user/repository.go +++ b/internal/user/repository.go @@ -19,6 +19,7 @@ type Repository interface { Update(ctx context.Context, user *User) error Delete(ctx context.Context, id string) error GetByID(ctx context.Context, id string) (*User, error) + UpdatePasswordReset(ctx context.Context, id, hashedPassword, updatedBy string) error GetByEmail(ctx context.Context, email string) (*User, error) GetByUsername(ctx context.Context, username string) (*User, error) GetByIDs(ctx context.Context, userIDs []string) ([]User, error) @@ -177,7 +178,7 @@ func (r *userRepository) GetByID(ctx context.Context, id string) (*User, error) return legacyUser, nil } if errors.Is(legacyErr, orm.ErrDocumentNotFound) { - return nil, errors.New("user not found") + return nil, ErrUserNotFound } r.logger.Error("Failed to get user by legacy string ID", map[string]interface{}{ "error": legacyErr.Error(), @@ -195,6 +196,43 @@ func (r *userRepository) GetByID(ctx context.Context, id string) (*User, error) return user, nil } +// UpdatePasswordReset atomically persists a new password hash and audit fields. +func (r *userRepository) UpdatePasswordReset(ctx context.Context, id, hashedPassword, updatedBy string) error { + now := time.Now().Unix() + updateDoc := bson.M{ + "$set": bson.M{ + "password": hashedPassword, + "updated_at": now, + "updated_by": updatedBy, + }, + } + + objectID, err := bson.ObjectIDFromHex(id) + if err == nil { + result, updateErr := r.collection.UpdateOne(ctx, bson.M{"_id": objectID}, updateDoc) + if updateErr == nil && result.MatchedCount > 0 { + return nil + } + if updateErr != nil { + return updateErr + } + } + + result, err := r.collection.UpdateOne(ctx, bson.M{"_id": id}, updateDoc) + if err != nil { + r.logger.Error("Failed to update password reset", map[string]interface{}{ + "error": err.Error(), + "user_id": id, + }) + return err + } + if result.MatchedCount == 0 { + return ErrUserNotFound + } + + return nil +} + // GetByEmail retrieves a user by email func (r *userRepository) GetByEmail(ctx context.Context, email string) (*User, error) { user, err := r.ormRepo.FindOne(ctx, bson.M{"email": email}) diff --git a/internal/user/reset_password.go b/internal/user/reset_password.go index 72e3760..75d626d 100644 --- a/internal/user/reset_password.go +++ b/internal/user/reset_password.go @@ -15,17 +15,22 @@ import ( ) var ( - errInvalidUserID = errors.New("invalid id") - errAdminUserNotFound = errors.New("admin not found") - errCannotResetPassword = errors.New("cannot reset password for this user") - errResetPasswordFailed = errors.New("failed to reset password") - errResetPasswordRateHit = errors.New("too many password reset requests") + errInvalidUserID = errors.New("invalid id") + errAdminUserNotFound = errors.New("admin not found") + errCannotResetPassword = errors.New("cannot reset password for this user") + errResetPasswordFailed = errors.New("failed to reset password") + errResetPasswordRateHit = errors.New("too many password reset requests") + errResetPasswordForbidden = errors.New("you are not allowed to reset this password") ) const adminPasswordResetRateLimit = 10 // AdminResetPassword generates a new password for the target admin user. -func (s *userService) AdminResetPassword(ctx context.Context, actorID, targetID string) (*AdminResetPasswordResponse, error) { +func (s *userService) AdminResetPassword(ctx context.Context, actorID, actorRole, targetID string) (*AdminResetPasswordResponse, error) { + if UserRole(actorRole) != UserRoleAdmin { + return nil, errResetPasswordForbidden + } + if _, err := bson.ObjectIDFromHex(targetID); err != nil { return nil, errInvalidUserID } @@ -36,7 +41,7 @@ func (s *userService) AdminResetPassword(ctx context.Context, actorID, targetID user, err := s.repository.GetByID(ctx, targetID) if err != nil { - if err.Error() == "user not found" { + if errors.Is(err, ErrUserNotFound) { return nil, errAdminUserNotFound } s.logger.Error("Failed to load user for password reset", map[string]interface{}{ @@ -68,9 +73,10 @@ func (s *userService) AdminResetPassword(ctx context.Context, actorID, targetID return nil, errResetPasswordFailed } - user.Password = string(hashedPassword) - user.UpdatedBy = &actorID - if err := s.repository.Update(ctx, user); err != nil { + if err := s.repository.UpdatePasswordReset(ctx, targetID, string(hashedPassword), actorID); err != nil { + if errors.Is(err, ErrUserNotFound) { + return nil, errAdminUserNotFound + } s.logger.Error("Failed to persist reset password", map[string]interface{}{ "error": err.Error(), "target_id": targetID, @@ -83,7 +89,6 @@ func (s *userService) AdminResetPassword(ctx context.Context, actorID, targetID "error": err.Error(), "target_id": targetID, }) - return nil, errResetPasswordFailed } s.auditLogger.Log(ctx, audit.Entry{ diff --git a/internal/user/service.go b/internal/user/service.go index 7cc7035..fbfac0b 100644 --- a/internal/user/service.go +++ b/internal/user/service.go @@ -28,7 +28,7 @@ type Service interface { GetUsersByIDs(ctx context.Context, userIDs []string) (*UserListResponse, error) Search(ctx context.Context, form *SearchUsersForm, pagination *response.Pagination) (*UserListResponse, error) Delete(ctx context.Context, id string) error - AdminResetPassword(ctx context.Context, actorID, targetID string) (*AdminResetPasswordResponse, error) + AdminResetPassword(ctx context.Context, actorID, actorRole, targetID string) (*AdminResetPasswordResponse, error) // Profile Login(ctx context.Context, form *LoginForm) (*AuthResponse, error) diff --git a/pkg/authorization/authorization.go b/pkg/authorization/authorization.go index d280af0..236a67e 100644 --- a/pkg/authorization/authorization.go +++ b/pkg/authorization/authorization.go @@ -318,6 +318,12 @@ func (s *Service) ValidateToken(tokenString string) (*TokenValidationResult, err "user_id": claims.UserID, "error": err.Error(), }) + return &TokenValidationResult{ + Valid: false, + Claims: nil, + Error: "session validation unavailable", + Expired: false, + }, nil } else if invalidated { return &TokenValidationResult{ Valid: false, diff --git a/pkg/authorization/authorization_test.go b/pkg/authorization/authorization_test.go index e74345c..c65563d 100644 --- a/pkg/authorization/authorization_test.go +++ b/pkg/authorization/authorization_test.go @@ -25,14 +25,9 @@ func (m *MockRedisClient) Get(ctx context.Context, key string) (string, error) { return args.String(0), args.Error(1) } -func (m *MockRedisClient) Incr(ctx context.Context, key string) (int64, error) { - args := m.Called(ctx, key) - return args.Get(0).(int64), args.Error(1) -} - -func (m *MockRedisClient) Expire(ctx context.Context, key string, expiration time.Duration) error { +func (m *MockRedisClient) IncrWithExpire(ctx context.Context, key string, expiration time.Duration) (int64, error) { args := m.Called(ctx, key, expiration) - return args.Error(0) + return args.Get(0).(int64), args.Error(1) } func (m *MockRedisClient) Del(ctx context.Context, keys ...string) error { diff --git a/pkg/ratelimit/limit.go b/pkg/ratelimit/limit.go index 50b6b41..ef0fd11 100644 --- a/pkg/ratelimit/limit.go +++ b/pkg/ratelimit/limit.go @@ -5,8 +5,6 @@ import ( "time" "tm/pkg/redis" - - redisv9 "github.com/redis/go-redis/v9" ) // Allow returns whether the request is within limit for the given key. @@ -15,21 +13,10 @@ func Allow(ctx context.Context, client redis.Client, key string, limit int64, wi return true, nil } - count, err := client.Incr(ctx, key) + count, err := client.IncrWithExpire(ctx, key, window) if err != nil { return false, err } - if count == 1 { - if err := client.Expire(ctx, key, window); err != nil { - return false, err - } - } - return count <= limit, nil } - -// IsNotFound reports whether err indicates a missing Redis key. -func IsNotFound(err error) bool { - return err == redisv9.Nil -} diff --git a/pkg/redis/connection.go b/pkg/redis/connection.go index c7bf674..b1ca4fd 100644 --- a/pkg/redis/connection.go +++ b/pkg/redis/connection.go @@ -23,8 +23,7 @@ type Config struct { type Client interface { Set(ctx context.Context, key string, value interface{}, expiration time.Duration) error Get(ctx context.Context, key string) (string, error) - Incr(ctx context.Context, key string) (int64, error) - Expire(ctx context.Context, key string, expiration time.Duration) error + IncrWithExpire(ctx context.Context, key string, expiration time.Duration) (int64, error) Del(ctx context.Context, keys ...string) error Exists(ctx context.Context, keys ...string) (int64, error) Close() error @@ -72,6 +71,28 @@ func NewClient(config *Config, logger logger.Logger) (Client, error) { return &client{rdb: rdb}, nil } +const incrWithExpireScript = ` +local current = redis.call("INCR", KEYS[1]) +if current == 1 then + redis.call("EXPIRE", KEYS[1], ARGV[1]) +else + local ttl = redis.call("TTL", KEYS[1]) + if ttl == -1 then + redis.call("EXPIRE", KEYS[1], ARGV[1]) + end +end +return current +` + +// IncrWithExpire atomically increments a counter and ensures the key has a TTL. +func (c *client) IncrWithExpire(ctx context.Context, key string, expiration time.Duration) (int64, error) { + seconds := int64(expiration.Seconds()) + if seconds < 1 { + seconds = 1 + } + return c.rdb.Eval(ctx, incrWithExpireScript, []string{key}, seconds).Int64() +} + // Set sets a key-value pair with expiration func (c *client) Set(ctx context.Context, key string, value interface{}, expiration time.Duration) error { return c.rdb.Set(ctx, key, value, expiration).Err() @@ -82,16 +103,6 @@ func (c *client) Get(ctx context.Context, key string) (string, error) { return c.rdb.Get(ctx, key).Result() } -// Incr increments a key's integer value. -func (c *client) Incr(ctx context.Context, key string) (int64, error) { - return c.rdb.Incr(ctx, key).Result() -} - -// Expire sets a timeout on a key. -func (c *client) Expire(ctx context.Context, key string, expiration time.Duration) error { - return c.rdb.Expire(ctx, key, expiration).Err() -} - // Del deletes one or more keys func (c *client) Del(ctx context.Context, keys ...string) error { return c.rdb.Del(ctx, keys...).Err()