feat: auth system hardening with token revocation, password management, and error sanitization
- Add token revocation with in-memory blacklist and periodic cleanup (SEC-M1)
- Add POST /api/auth/logout endpoint
- Fix OIDC auth_token cookie to HttpOnly with exchange endpoint (SEC-H3)
- Add password complexity validation (min 8 chars) (SEC-M2)
- Prevent admin self-deletion (SEC-M3)
- Add PUT /api/auth/users/{uid} for role/email updates (FUNC-M1)
- Add PUT /api/auth/users/{uid}/password for password changes (FUNC-H1)
- Sanitize error messages in auth handlers (SEC-M4)
This commit is contained in:
+181
-16
@@ -4,6 +4,7 @@ import (
|
||||
"crypto/rand"
|
||||
"encoding/hex"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
|
||||
@@ -63,7 +64,8 @@ func (s *Server) login(w http.ResponseWriter, r *http.Request) {
|
||||
Role: user.Role,
|
||||
})
|
||||
if err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to generate token: "+err.Error())
|
||||
slog.Error("failed to generate token", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -80,7 +82,8 @@ func (s *Server) currentUser(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
user, err := s.store.GetUserByID(claims.UserID)
|
||||
if err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to get user: "+err.Error())
|
||||
slog.Error("failed to get user", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -175,11 +178,13 @@ func (s *Server) oidcCallback(w http.ResponseWriter, r *http.Request) {
|
||||
Role: "viewer", // OIDC users default to viewer; admin promotes via settings
|
||||
})
|
||||
if err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to create user: "+err.Error())
|
||||
slog.Error("failed to create user", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
} else {
|
||||
respondError(w, http.StatusInternalServerError, "failed to get user: "+err.Error())
|
||||
slog.Error("failed to get user", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
}
|
||||
@@ -190,28 +195,53 @@ func (s *Server) oidcCallback(w http.ResponseWriter, r *http.Request) {
|
||||
Role: user.Role,
|
||||
})
|
||||
if err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to generate token: "+err.Error())
|
||||
slog.Error("failed to generate token", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
// Set the token in a short-lived cookie the frontend can read once.
|
||||
// Pass token via short-lived httpOnly cookie. The frontend reads it via
|
||||
// a dedicated /api/auth/oidc/token endpoint and then the cookie is cleared.
|
||||
http.SetCookie(w, &http.Cookie{
|
||||
Name: "auth_token",
|
||||
Value: token.Token,
|
||||
Path: "/",
|
||||
MaxAge: 60, // 1 minute — frontend reads it immediately
|
||||
HttpOnly: false,
|
||||
Path: "/api/auth/oidc",
|
||||
MaxAge: 60,
|
||||
HttpOnly: true,
|
||||
Secure: true,
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
})
|
||||
http.Redirect(w, r, "/?oidc=success", http.StatusFound)
|
||||
}
|
||||
|
||||
// oidcExchangeToken handles POST /api/auth/oidc/token — exchanges the httpOnly cookie for a JSON token.
|
||||
func (s *Server) oidcExchangeToken(w http.ResponseWriter, r *http.Request) {
|
||||
cookie, err := r.Cookie("auth_token")
|
||||
if err != nil || cookie.Value == "" {
|
||||
respondError(w, http.StatusUnauthorized, "no OIDC token available")
|
||||
return
|
||||
}
|
||||
|
||||
// Clear the cookie immediately.
|
||||
http.SetCookie(w, &http.Cookie{
|
||||
Name: "auth_token",
|
||||
Value: "",
|
||||
Path: "/api/auth/oidc",
|
||||
MaxAge: -1,
|
||||
HttpOnly: true,
|
||||
Secure: true,
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
})
|
||||
|
||||
respondJSON(w, http.StatusOK, map[string]string{"token": cookie.Value})
|
||||
}
|
||||
|
||||
// getAuthSettings handles GET /api/auth/settings.
|
||||
func (s *Server) getAuthSettings(w http.ResponseWriter, r *http.Request) {
|
||||
as, err := s.store.GetAuthSettings()
|
||||
if err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to get auth settings: "+err.Error())
|
||||
slog.Error("failed to get auth settings", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
// Mask the client secret for the response.
|
||||
@@ -253,7 +283,8 @@ func (s *Server) updateAuthSettings(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
if err := s.store.UpdateAuthSettings(req); err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to update auth settings: "+err.Error())
|
||||
slog.Error("failed to update auth settings", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -269,7 +300,8 @@ func (s *Server) updateAuthSettings(w http.ResponseWriter, r *http.Request) {
|
||||
func (s *Server) listUsers(w http.ResponseWriter, r *http.Request) {
|
||||
users, err := s.store.GetAllUsers()
|
||||
if err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to list users: "+err.Error())
|
||||
slog.Error("failed to list users", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
respondJSON(w, http.StatusOK, users)
|
||||
@@ -291,6 +323,10 @@ func (s *Server) createUser(w http.ResponseWriter, r *http.Request) {
|
||||
respondError(w, http.StatusBadRequest, "username and password are required")
|
||||
return
|
||||
}
|
||||
if err := validatePassword(req.Password); err != nil {
|
||||
respondError(w, http.StatusBadRequest, err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
if req.Role == "" {
|
||||
req.Role = "viewer"
|
||||
@@ -302,7 +338,8 @@ func (s *Server) createUser(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
hash, err := auth.HashPassword(req.Password)
|
||||
if err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to hash password: "+err.Error())
|
||||
slog.Error("failed to hash password", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -313,7 +350,8 @@ func (s *Server) createUser(w http.ResponseWriter, r *http.Request) {
|
||||
Role: req.Role,
|
||||
})
|
||||
if err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to create user: "+err.Error())
|
||||
slog.Error("failed to create user", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -324,6 +362,13 @@ func (s *Server) createUser(w http.ResponseWriter, r *http.Request) {
|
||||
func (s *Server) deleteUser(w http.ResponseWriter, r *http.Request) {
|
||||
id := chi.URLParam(r, "uid")
|
||||
|
||||
// Prevent deleting your own account.
|
||||
claims, ok := auth.ClaimsFromContext(r.Context())
|
||||
if ok && claims.UserID == id {
|
||||
respondError(w, http.StatusBadRequest, "cannot delete your own account")
|
||||
return
|
||||
}
|
||||
|
||||
// Prevent deleting the last admin.
|
||||
user, err := s.store.GetUserByID(id)
|
||||
if err != nil {
|
||||
@@ -331,7 +376,8 @@ func (s *Server) deleteUser(w http.ResponseWriter, r *http.Request) {
|
||||
respondNotFound(w, "user")
|
||||
return
|
||||
}
|
||||
respondError(w, http.StatusInternalServerError, "failed to get user: "+err.Error())
|
||||
slog.Error("failed to get user", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -352,9 +398,128 @@ func (s *Server) deleteUser(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
if err := s.store.DeleteUser(id); err != nil {
|
||||
respondError(w, http.StatusInternalServerError, "failed to delete user: "+err.Error())
|
||||
slog.Error("failed to delete user", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
respondJSON(w, http.StatusOK, map[string]string{"deleted": id})
|
||||
}
|
||||
|
||||
// validatePassword checks that a password meets minimum complexity requirements.
|
||||
func validatePassword(password string) error {
|
||||
if len(password) < 8 {
|
||||
return fmt.Errorf("password must be at least 8 characters long")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// logout handles POST /api/auth/logout — revokes the current token.
|
||||
func (s *Server) logout(w http.ResponseWriter, r *http.Request) {
|
||||
tokenStr := auth.ExtractToken(r)
|
||||
if tokenStr != "" {
|
||||
s.localAuth.RevokeToken(tokenStr)
|
||||
}
|
||||
respondJSON(w, http.StatusOK, map[string]string{"status": "logged out"})
|
||||
}
|
||||
|
||||
// changePassword handles PUT /api/auth/users/{uid}/password.
|
||||
func (s *Server) changePassword(w http.ResponseWriter, r *http.Request) {
|
||||
uid := chi.URLParam(r, "uid")
|
||||
|
||||
var req struct {
|
||||
Password string `json:"password"`
|
||||
}
|
||||
if !decodeJSON(w, r, &req) {
|
||||
return
|
||||
}
|
||||
|
||||
if req.Password == "" {
|
||||
respondError(w, http.StatusBadRequest, "password is required")
|
||||
return
|
||||
}
|
||||
if err := validatePassword(req.Password); err != nil {
|
||||
respondError(w, http.StatusBadRequest, err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
hash, err := auth.HashPassword(req.Password)
|
||||
if err != nil {
|
||||
slog.Error("failed to hash password", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
if err := s.store.UpdateUserPassword(uid, hash); err != nil {
|
||||
if errors.Is(err, store.ErrNotFound) {
|
||||
respondNotFound(w, "user")
|
||||
return
|
||||
}
|
||||
slog.Error("failed to update password", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
respondJSON(w, http.StatusOK, map[string]string{"status": "password updated"})
|
||||
}
|
||||
|
||||
// updateUser handles PUT /api/auth/users/{uid}.
|
||||
func (s *Server) updateUser(w http.ResponseWriter, r *http.Request) {
|
||||
uid := chi.URLParam(r, "uid")
|
||||
|
||||
var req struct {
|
||||
Email string `json:"email"`
|
||||
Role string `json:"role"`
|
||||
}
|
||||
if !decodeJSON(w, r, &req) {
|
||||
return
|
||||
}
|
||||
|
||||
if req.Role != "" && req.Role != "admin" && req.Role != "viewer" {
|
||||
respondError(w, http.StatusBadRequest, "role must be 'admin' or 'viewer'")
|
||||
return
|
||||
}
|
||||
|
||||
existing, err := s.store.GetUserByID(uid)
|
||||
if err != nil {
|
||||
if errors.Is(err, store.ErrNotFound) {
|
||||
respondNotFound(w, "user")
|
||||
return
|
||||
}
|
||||
slog.Error("failed to get user", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
// If demoting from admin, check we're not removing the last admin.
|
||||
if existing.Role == "admin" && req.Role == "viewer" {
|
||||
users, err := s.store.GetAllUsers()
|
||||
if err == nil {
|
||||
adminCount := 0
|
||||
for _, u := range users {
|
||||
if u.Role == "admin" {
|
||||
adminCount++
|
||||
}
|
||||
}
|
||||
if adminCount <= 1 {
|
||||
respondError(w, http.StatusBadRequest, "cannot demote the last admin user")
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if req.Email != "" {
|
||||
existing.Email = req.Email
|
||||
}
|
||||
if req.Role != "" {
|
||||
existing.Role = req.Role
|
||||
}
|
||||
|
||||
if err := s.store.UpdateUser(existing); err != nil {
|
||||
slog.Error("failed to update user", "error", err)
|
||||
respondError(w, http.StatusInternalServerError, "internal server error")
|
||||
return
|
||||
}
|
||||
|
||||
respondJSON(w, http.StatusOK, existing)
|
||||
}
|
||||
|
||||
@@ -105,6 +105,7 @@ func (s *Server) Router() chi.Router {
|
||||
r.Post("/auth/login", s.rateLimitedLogin(loginLimiter))
|
||||
r.Get("/auth/oidc/login", s.oidcLogin)
|
||||
r.Get("/auth/oidc/callback", s.oidcCallback)
|
||||
r.Post("/auth/oidc/token", s.oidcExchangeToken)
|
||||
|
||||
// Webhook handler (uses its own secret-based auth).
|
||||
r.Mount("/webhook", s.webhook.Route())
|
||||
@@ -115,6 +116,7 @@ func (s *Server) Router() chi.Router {
|
||||
|
||||
// Read-only endpoints (any authenticated user).
|
||||
r.Get("/auth/me", s.currentUser)
|
||||
r.Post("/auth/logout", s.logout)
|
||||
r.Get("/projects", s.listProjects)
|
||||
r.Route("/projects/{id}", func(r chi.Router) {
|
||||
r.Get("/", s.getProject)
|
||||
@@ -145,6 +147,8 @@ func (s *Server) Router() chi.Router {
|
||||
r.Put("/auth/settings", s.updateAuthSettings)
|
||||
r.Get("/auth/users", s.listUsers)
|
||||
r.Post("/auth/users", s.createUser)
|
||||
r.Put("/auth/users/{uid}", s.updateUser)
|
||||
r.Put("/auth/users/{uid}/password", s.changePassword)
|
||||
r.Delete("/auth/users/{uid}", s.deleteUser)
|
||||
|
||||
// Project mutation endpoints.
|
||||
|
||||
Reference in New Issue
Block a user