diff --git a/internal/controller/user_controller.go b/internal/controller/user_controller.go index 65b12de0..479f8f05 100644 --- a/internal/controller/user_controller.go +++ b/internal/controller/user_controller.go @@ -11,6 +11,7 @@ import ( "github.com/tinyauthapp/tinyauth/internal/service" "github.com/tinyauthapp/tinyauth/internal/utils" "github.com/tinyauthapp/tinyauth/internal/utils/logger" + "github.com/tinyauthapp/tinyauth/pkg/cache" "go.uber.org/dig" "github.com/gin-gonic/gin" @@ -72,49 +73,59 @@ func (controller *UserController) loginHandler(c *gin.Context) { controller.log.App.Debug().Str("username", req.Username).Msg("Login attempt") - search, err := controller.auth.SearchUser(req.Username) + var search *model.UserSearch - if err != nil { - if errors.Is(err, service.ErrUserNotFound) { - controller.auth.DummyPasswordCheck(req.Password) - controller.log.App.Warn().Str("username", req.Username).Msg("User not found during login attempt") - controller.log.AuditLoginFailure(req.Username, "unknown", c.ClientIP(), "user not found") + err = controller.auth.WithLoginAttempt(func(actions cache.CacheStoreActions[service.LoginAttempt]) error { + search, err = controller.auth.SearchUser(req.Username) + + if err != nil { + if errors.Is(err, service.ErrUserNotFound) { + controller.auth.DummyPasswordCheck(req.Password) + controller.log.App.Warn().Str("username", req.Username).Msg("User not found during login attempt") + controller.log.AuditLoginFailure(req.Username, "unknown", c.ClientIP(), "user not found") + c.JSON(401, gin.H{ + "status": 401, + "message": "Unauthorized", + }) + return fmt.Errorf("user not found") + } + controller.log.App.Error().Err(err).Str("username", req.Username).Msg("Error searching for user during login attempt") + c.JSON(500, gin.H{ + "status": 500, + "message": "Internal Server Error", + }) + return fmt.Errorf("error searching for user") + } + + isLocked, remaining := controller.auth.IsAccountLocked(req.Username, actions) + + if isLocked { + controller.log.App.Warn().Str("username", req.Username).Msg("Account is locked due to too many failed login attempts") + controller.log.AuditLoginFailure(req.Username, search.Type.String(), c.ClientIP(), "account locked") + c.Writer.Header().Add("x-tinyauth-lock-locked", "true") + c.Writer.Header().Add("x-tinyauth-lock-reset", time.Now().Add(time.Duration(remaining)*time.Second).Format(time.RFC3339)) + c.JSON(429, gin.H{ + "status": 429, + "message": fmt.Sprintf("Too many failed login attempts. Try again in %d seconds", remaining), + }) + return fmt.Errorf("account locked") + } + + if err = controller.auth.CheckUserPassword(*search, req.Password); err != nil { + controller.log.App.Warn().Str("username", req.Username).Msg("Invalid password during login attempt") + controller.auth.RecordLoginAttempt(req.Username, false, actions) + controller.log.AuditLoginFailure(req.Username, search.Type.String(), c.ClientIP(), "invalid password") c.JSON(401, gin.H{ "status": 401, "message": "Unauthorized", }) - return + return fmt.Errorf("invalid password") } - controller.log.App.Error().Err(err).Str("username", req.Username).Msg("Error searching for user during login attempt") - c.JSON(500, gin.H{ - "status": 500, - "message": "Internal Server Error", - }) - return - } - - isLocked, remaining := controller.auth.IsAccountLocked(req.Username) - if isLocked { - controller.log.App.Warn().Str("username", req.Username).Msg("Account is locked due to too many failed login attempts") - controller.log.AuditLoginFailure(req.Username, search.Type.String(), c.ClientIP(), "account locked") - c.Writer.Header().Add("x-tinyauth-lock-locked", "true") - c.Writer.Header().Add("x-tinyauth-lock-reset", time.Now().Add(time.Duration(remaining)*time.Second).Format(time.RFC3339)) - c.JSON(429, gin.H{ - "status": 429, - "message": fmt.Sprintf("Too many failed login attempts. Try again in %d seconds", remaining), - }) - return - } + return nil + }) - if err := controller.auth.CheckUserPassword(*search, req.Password); err != nil { - controller.log.App.Warn().Str("username", req.Username).Msg("Invalid password during login attempt") - controller.auth.RecordLoginAttempt(req.Username, false) - controller.log.AuditLoginFailure(req.Username, search.Type.String(), c.ClientIP(), "invalid password") - c.JSON(401, gin.H{ - "status": 401, - "message": "Unauthorized", - }) + if err != nil { return } @@ -216,7 +227,10 @@ func (controller *UserController) loginHandler(c *gin.Context) { controller.log.AuditLoginSuccess(req.Username, search.Type.String(), c.ClientIP()) - controller.auth.RecordLoginAttempt(req.Username, true) + controller.auth.WithLoginAttempt(func(actions cache.CacheStoreActions[service.LoginAttempt]) error { + controller.auth.RecordLoginAttempt(req.Username, true, actions) + return nil + }) c.JSON(200, gin.H{ "status": 200, @@ -317,56 +331,66 @@ func (controller *UserController) totpHandler(c *gin.Context) { controller.log.App.Debug().Str("username", context.GetUsername()).Msg("TOTP verification attempt") - isLocked, remaining := controller.auth.IsAccountLocked(context.GetUsername()) + var user *model.LocalUser - if isLocked { - controller.log.App.Warn().Str("username", context.GetUsername()).Msg("Account is locked due to too many failed TOTP attempts") - controller.log.AuditLoginFailure(context.GetUsername(), "local", c.ClientIP(), "account locked") - c.Writer.Header().Add("x-tinyauth-lock-locked", "true") - c.Writer.Header().Add("x-tinyauth-lock-reset", time.Now().Add(time.Duration(remaining)*time.Second).Format(time.RFC3339)) - c.JSON(429, gin.H{ - "status": 429, - "message": fmt.Sprintf("Too many failed TOTP attempts. Try again in %d seconds", remaining), - }) - return - } + err = controller.auth.WithLoginAttempt(func(actions cache.CacheStoreActions[service.LoginAttempt]) error { + isLocked, remaining := controller.auth.IsAccountLocked(context.GetUsername(), actions) - user := controller.auth.GetLocalUser(context.GetUsername()) + if isLocked { + controller.log.App.Warn().Str("username", context.GetUsername()).Msg("Account is locked due to too many failed TOTP attempts") + controller.log.AuditLoginFailure(context.GetUsername(), "local", c.ClientIP(), "account locked") + c.Writer.Header().Add("x-tinyauth-lock-locked", "true") + c.Writer.Header().Add("x-tinyauth-lock-reset", time.Now().Add(time.Duration(remaining)*time.Second).Format(time.RFC3339)) + c.JSON(429, gin.H{ + "status": 429, + "message": fmt.Sprintf("Too many failed TOTP attempts. Try again in %d seconds", remaining), + }) + return fmt.Errorf("account locked") + } - if user == nil { - controller.log.App.Error().Str("username", context.GetUsername()).Msg("Local user not found during TOTP verification") - c.JSON(401, gin.H{ - "status": 401, - "message": "Unauthorized", - }) - return - } + user = controller.auth.GetLocalUser(context.GetUsername()) - ok := totp.Validate(req.Code, user.TOTPSecret) + if user == nil { + controller.log.App.Error().Str("username", context.GetUsername()).Msg("Local user not found during TOTP verification") + c.JSON(401, gin.H{ + "status": 401, + "message": "Unauthorized", + }) + return fmt.Errorf("local user not found") + } - if !ok { - controller.log.App.Warn().Str("username", context.GetUsername()).Msg("Invalid TOTP code during verification attempt") - controller.auth.RecordLoginAttempt(context.GetUsername(), false) - controller.log.AuditLoginFailure(context.GetUsername(), "local", c.ClientIP(), "invalid TOTP code") - c.JSON(401, gin.H{ - "status": 401, - "message": "Unauthorized", - }) - return - } + ok := totp.Validate(req.Code, user.TOTPSecret) - uuid, err := c.Cookie(controller.runtime.SessionCookieName) + if !ok { + controller.log.App.Warn().Str("username", context.GetUsername()).Msg("Invalid TOTP code during verification attempt") + controller.auth.RecordLoginAttempt(context.GetUsername(), false, actions) + controller.log.AuditLoginFailure(context.GetUsername(), "local", c.ClientIP(), "invalid TOTP code") + c.JSON(401, gin.H{ + "status": 401, + "message": "Unauthorized", + }) + return fmt.Errorf("invalid TOTP code") + } - if err == nil { - _, err = controller.auth.DeleteSession(c, uuid) - if err != nil { - controller.log.App.Error().Err(err).Msg("Failed to delete pending TOTP session after successful verification") + uuid, err := c.Cookie(controller.runtime.SessionCookieName) + + if err == nil { + _, err = controller.auth.DeleteSession(c, uuid) + if err != nil { + controller.log.App.Error().Err(err).Msg("Failed to delete pending TOTP session after successful verification") + } + } else { + controller.log.App.Warn().Err(err).Msg("Failed to retrieve session cookie for pending TOTP session, cannot delete it") } - } else { - controller.log.App.Warn().Err(err).Msg("Failed to retrieve session cookie for pending TOTP session, cannot delete it") - } - controller.auth.RecordLoginAttempt(context.GetUsername(), true) + controller.auth.RecordLoginAttempt(context.GetUsername(), true, actions) + + return nil + }) + + if err != nil { + return + } sessionCookie := repository.Session{ Username: user.Username, diff --git a/internal/middleware/context_middleware.go b/internal/middleware/context_middleware.go index 2cbb046e..6b920be4 100644 --- a/internal/middleware/context_middleware.go +++ b/internal/middleware/context_middleware.go @@ -12,6 +12,7 @@ import ( "github.com/tinyauthapp/tinyauth/internal/service" "github.com/tinyauthapp/tinyauth/internal/utils" "github.com/tinyauthapp/tinyauth/internal/utils/logger" + "github.com/tinyauthapp/tinyauth/pkg/cache" "go.uber.org/dig" "github.com/gin-gonic/gin" @@ -253,34 +254,50 @@ func (m *ContextMiddleware) cookieAuth(ctx context.Context, uuid string, ip stri // any response headers to set. func (m *ContextMiddleware) basicAuth(username string, password string) (*model.UserContext, map[string]string, error) { headers := make(map[string]string) + errLocked := errors.New("account locked") userContext := new(model.UserContext) - locked, remaining := m.auth.IsAccountLocked(username) - if locked { - m.log.App.Debug().Msgf("Account for user %s is locked for %d seconds, denying auth", username, remaining) - headers["x-tinyauth-lock-locked"] = "true" - headers["x-tinyauth-lock-reset"] = time.Now().Add(time.Duration(remaining) * time.Second).Format(time.RFC3339) - return nil, headers, nil - } + var search *model.UserSearch - search, err := m.auth.SearchUser(username) + err := m.auth.WithLoginAttempt(func(actions cache.CacheStoreActions[service.LoginAttempt]) error { + locked, remaining := m.auth.IsAccountLocked(username, actions) - if err != nil { - if errors.Is(err, service.ErrUserNotFound) { - m.auth.DummyPasswordCheck(password) + if locked { + m.log.App.Debug().Msgf("Account for user %s is locked for %d seconds, denying auth", username, remaining) + headers["x-tinyauth-lock-locked"] = "true" + headers["x-tinyauth-lock-reset"] = time.Now().Add(time.Duration(remaining) * time.Second).Format(time.RFC3339) + return errLocked + } + + var err error + search, err = m.auth.SearchUser(username) + + if err != nil { + if errors.Is(err, service.ErrUserNotFound) { + m.auth.DummyPasswordCheck(password) + } + return fmt.Errorf("error searching for user: %w", err) } - return nil, nil, fmt.Errorf("error searching for user: %w", err) - } - err = m.auth.CheckUserPassword(*search, password) + err = m.auth.CheckUserPassword(*search, password) + + if err != nil { + m.auth.RecordLoginAttempt(username, false, actions) + return fmt.Errorf("invalid password for basic auth user: %w", err) + } + + m.auth.RecordLoginAttempt(username, true, actions) + + return nil + }) if err != nil { - m.auth.RecordLoginAttempt(username, false) - return nil, nil, fmt.Errorf("invalid password for basic auth user: %w", err) + if errors.Is(err, errLocked) { + return nil, headers, nil + } + return nil, nil, err } - m.auth.RecordLoginAttempt(username, true) - switch search.Type { case model.UserLocal: user := m.auth.GetLocalUser(username) diff --git a/internal/service/auth_service.go b/internal/service/auth_service.go index a4cef1e1..47390cb6 100644 --- a/internal/service/auth_service.go +++ b/internal/service/auth_service.go @@ -267,12 +267,16 @@ func (auth *AuthService) GetLDAPUser(userDN string) (*model.LDAPUser, error) { }, nil } -func (auth *AuthService) IsAccountLocked(identifier string) (bool, int) { - if auth.config.Auth.LoginMaxRetries <= 0 || auth.config.Auth.LoginTimeout <= 0 { +func (auth *AuthService) rateLimitEnabled() bool { + return auth.config.Auth.LoginMaxRetries > 0 && auth.config.Auth.LoginTimeout > 0 +} + +func (auth *AuthService) IsAccountLocked(identifier string, actions cache.CacheStoreActions[LoginAttempt]) (bool, int) { + if !auth.rateLimitEnabled() { return false, 0 } - attempt, exists := auth.caches.login.Get(identifier) + attempt, exists := actions.Get(identifier) if !exists { return false, 0 } @@ -285,46 +289,51 @@ func (auth *AuthService) IsAccountLocked(identifier string) (bool, int) { return false, 0 } -func (auth *AuthService) RecordLoginAttempt(identifier string, success bool) { - if auth.config.Auth.LoginMaxRetries <= 0 || auth.config.Auth.LoginTimeout <= 0 { +func (auth *AuthService) RecordLoginAttempt(identifier string, success bool, actions cache.CacheStoreActions[LoginAttempt]) { + if !auth.rateLimitEnabled() { return } - auth.caches.login.WithLock(func(actions cache.CacheStoreActions[LoginAttempt]) { - entry, ok := actions.Get(identifier) + entry, ok := actions.Get(identifier) - if !ok { - attempt := LoginAttempt{ - LastAttempt: time.Now(), - } - if !success { - attempt.FailedAttempts = 1 - if attempt.FailedAttempts >= auth.config.Auth.LoginMaxRetries { - attempt.LockedUntil = time.Now().Add(time.Duration(auth.config.Auth.LoginTimeout) * time.Second) - auth.log.App.Warn().Str("identifier", identifier).Int("failedAttempts", attempt.FailedAttempts).Msg("Account locked due to too many failed login attempts") - } + if !ok { + attempt := LoginAttempt{ + LastAttempt: time.Now(), + } + if !success { + attempt.FailedAttempts = 1 + if attempt.FailedAttempts >= auth.config.Auth.LoginMaxRetries { + attempt.LockedUntil = time.Now().Add(time.Duration(auth.config.Auth.LoginTimeout) * time.Second) + auth.log.App.Warn().Str("identifier", identifier).Int("failed_attempts", attempt.FailedAttempts).Msg("Account locked due to too many failed login attempts") } - // match current tinyauth behavior which doesn't expire rate limits - actions.Set(identifier, attempt, 0) - return } + actions.Set(identifier, attempt, time.Duration(auth.config.Auth.LoginTimeout)*time.Second) + return + } - entry.LastAttempt = time.Now() + entry.LastAttempt = time.Now() - if success { - entry.FailedAttempts = 0 - entry.LockedUntil = time.Time{} - } else { - entry.FailedAttempts++ + if success { + entry.FailedAttempts = 0 + entry.LockedUntil = time.Time{} + } else { + entry.FailedAttempts++ - if entry.FailedAttempts >= auth.config.Auth.LoginMaxRetries { - entry.LockedUntil = time.Now().Add(time.Duration(auth.config.Auth.LoginTimeout) * time.Second) - auth.log.App.Warn().Str("identifier", identifier).Int("failedAttempts", entry.FailedAttempts).Msg("Account locked due to too many failed login attempts") - } + if entry.FailedAttempts >= auth.config.Auth.LoginMaxRetries { + entry.LockedUntil = time.Now().Add(time.Duration(auth.config.Auth.LoginTimeout) * time.Second) + auth.log.App.Warn().Str("identifier", identifier).Int("failed_attempts", entry.FailedAttempts).Msg("Account locked due to too many failed login attempts") } + } + + actions.Set(identifier, entry, time.Duration(auth.config.Auth.LoginTimeout)*time.Second) +} - actions.Set(identifier, entry, 0) +func (auth *AuthService) WithLoginAttempt(f func(actions cache.CacheStoreActions[LoginAttempt]) error) error { + var err error + auth.caches.login.WithLock(func(actions cache.CacheStoreActions[LoginAttempt]) { + err = f(actions) }) + return err } // We could also directly access the policyEngine.effectToAccess but @@ -337,7 +346,7 @@ func (auth *AuthService) IsEmailWhitelisted(provider string, email string) bool } match, err := utils.CheckFilter(strings.Join(whitelist, ","), email) if err != nil { - if err == utils.ErrFilterEmpty { + if errors.Is(err, utils.ErrFilterEmpty) { return EffectAbstain } auth.log.App.Error().Err(err).Str("email", email).Msg("Failed to evaluate email whitelist filter, defaulting to deny")