From 37e1069404cc80d425c57f329ad87d427d89b6a9 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Thu, 18 Jun 2026 16:26:02 +0100 Subject: [PATCH] feat(backend): update auth middleware Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- backend/mw/auth.go | 18 +- backend/mw/auth_test.go | 469 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 484 insertions(+), 3 deletions(-) create mode 100644 backend/mw/auth_test.go diff --git a/backend/mw/auth.go b/backend/mw/auth.go index 3540c6a..18c32e6 100644 --- a/backend/mw/auth.go +++ b/backend/mw/auth.go @@ -2,6 +2,7 @@ package mw import ( "context" + "log" "net/http" "strings" @@ -50,7 +51,9 @@ func OptionalAuth(next http.Handler) http.Handler { tokenString := strings.TrimPrefix(authHeader, "Bearer ") userID, role, jti, err := auth.VerifyToken(tokenString, r.Context()) - if err == nil { + if err != nil { + log.Printf("OptionalAuth: invalid token: %v", err) + } else { ctx := context.WithValue(r.Context(), UserIDKey, userID) ctx = context.WithValue(ctx, UserRoleKey, role) ctx = context.WithValue(ctx, JTIKey, jti) @@ -97,9 +100,18 @@ func RequireVerified(next http.Handler) http.Handler { return RequireRole("verified_email", "admin")(next) } -// RequireAdmin middleware - only allows admin +// RequireAdmin middleware - only allows admin and validates the user ID is present +// in context, so handlers don't need to re-check. The user ID is always set by +// RequireAuth before this runs, but this adds a defensive safety net. func RequireAdmin(next http.Handler) http.Handler { - return RequireRole("admin")(next) + return RequireRole("admin")(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uid, ok := r.Context().Value(UserIDKey).(string) + if !ok || uid == "" { + http.Error(w, "Authentication required", http.StatusUnauthorized) + return + } + next.ServeHTTP(w, r) + })) } // Helper functions to get user info from context diff --git a/backend/mw/auth_test.go b/backend/mw/auth_test.go new file mode 100644 index 0000000..f0ccf1d --- /dev/null +++ b/backend/mw/auth_test.go @@ -0,0 +1,469 @@ +//go:build test +// +build test + +package mw + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "crussell/testutils/jwt" +) + +func testHandler() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + userID, _ := GetUserID(r.Context()) + role, _ := GetUserRole(r.Context()) + jti, _ := GetJTI(r.Context()) + w.WriteHeader(http.StatusOK) + w.Write([]byte(userID + "|" + role + "|" + jti)) + }) +} + +// ============================================================================= +// RequireAuth +// ============================================================================= + +func TestRequireAuth_NoHeader(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + w := httptest.NewRecorder() + RequireAuth(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401, got %d", w.Code) + } +} + +func TestRequireAuth_MalformedHeader(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "NotBearer token123") + w := httptest.NewRecorder() + RequireAuth(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401, got %d", w.Code) + } +} + +func TestRequireAuth_EmptyToken(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Bearer ") + w := httptest.NewRecorder() + RequireAuth(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401, got %d", w.Code) + } +} + +func TestRequireAuth_InvalidToken(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Bearer this.is.not.a.valid.jwt") + w := httptest.NewRecorder() + RequireAuth(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401, got %d", w.Code) + } +} + +func TestRequireAuth_ValidToken(t *testing.T) { + token := jwt.GenerateTestToken("testuser123", "verified_email") + + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Bearer "+token) + w := httptest.NewRecorder() + RequireAuth(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + + body := w.Body.String() + if body == "" || body == "|" { + t.Errorf("expected user info in response, got %q", body) + } +} + +func TestRequireAuth_AdminToken(t *testing.T) { + token := jwt.GenerateAdminToken() + + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Bearer "+token) + w := httptest.NewRecorder() + RequireAuth(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestRequireAuth_SetsAllContextFields(t *testing.T) { + token := jwt.GenerateTestToken("testuser123", "admin") + + var capturedID, capturedRole, capturedJTI string + collector := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedID, _ = GetUserID(r.Context()) + capturedRole, _ = GetUserRole(r.Context()) + capturedJTI, _ = GetJTI(r.Context()) + w.WriteHeader(http.StatusOK) + }) + + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Bearer "+token) + w := httptest.NewRecorder() + RequireAuth(collector).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", w.Code) + } + if capturedID != "testuser123" { + t.Errorf("expected userID 'testuser123', got %q", capturedID) + } + if capturedRole != "admin" { + t.Errorf("expected role 'admin', got %q", capturedRole) + } + if capturedJTI == "" { + t.Error("expected non-empty JTI") + } +} + +// ============================================================================= +// OptionalAuth +// ============================================================================= + +func TestOptionalAuth_NoHeader(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + w := httptest.NewRecorder() + OptionalAuth(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200 (pass-through), got %d", w.Code) + } + + body := w.Body.String() + if body != "||" { + t.Errorf("expected empty context (||), got %q", body) + } +} + +func TestOptionalAuth_ValidToken(t *testing.T) { + token := jwt.GenerateTestToken("testuser123", "verified_email") + + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Bearer "+token) + w := httptest.NewRecorder() + OptionalAuth(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } + + body := w.Body.String() + if body == "||" { + t.Error("expected context to be set with valid token") + } +} + +func TestOptionalAuth_InvalidToken(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Bearer obviously.invalid.token") + w := httptest.NewRecorder() + OptionalAuth(testHandler()).ServeHTTP(w, req) + + // Should pass through without setting context + if w.Code != http.StatusOK { + t.Errorf("expected 200 (pass-through on invalid token), got %d", w.Code) + } + + body := w.Body.String() + if body != "||" { + t.Errorf("expected empty context on invalid token, got %q", body) + } +} + +func TestOptionalAuth_NoBearer(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Basic somecreds") + w := httptest.NewRecorder() + OptionalAuth(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200 (pass-through without Bearer prefix), got %d", w.Code) + } +} + +// ============================================================================= +// RequireRole +// ============================================================================= + +func TestRequireRole_Allowed(t *testing.T) { + handler := RequireRole("admin")(testHandler()) + + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "admin") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200 for admin role, got %d", w.Code) + } +} + +func TestRequireRole_MultipleRoles(t *testing.T) { + handler := RequireRole("admin", "verified_email")(testHandler()) + + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "verified_email") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200 for verified_email in allowed list, got %d", w.Code) + } +} + +func TestRequireRole_Forbidden(t *testing.T) { + handler := RequireRole("admin")(testHandler()) + + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "verified_email") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for non-admin role, got %d", w.Code) + } +} + +func TestRequireRole_NoRoleInContext(t *testing.T) { + handler := RequireRole("admin")(testHandler()) + + req := httptest.NewRequest("GET", "/", nil) + // No UserRoleKey in context + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401 when no role in context, got %d", w.Code) + } +} + +func TestRequireRole_EmptyAllowedList(t *testing.T) { + handler := RequireRole()(testHandler()) + + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "admin") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 with empty allowed list, got %d", w.Code) + } +} + +// ============================================================================= +// RequireVerified (wraps RequireRole("verified_email", "admin")) +// ============================================================================= + +func TestRequireVerified_VerifiedEmail(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "verified_email") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + RequireVerified(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200 for verified_email, got %d", w.Code) + } +} + +func TestRequireVerified_Admin(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "admin") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + RequireVerified(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200 for admin, got %d", w.Code) + } +} + +func TestRequireVerified_Unverified(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "unverified_email") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + RequireVerified(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for unverified, got %d", w.Code) + } +} + +// ============================================================================= +// RequireAdmin (RequireRole("admin") + UserIDKey guard) +// ============================================================================= + +func TestRequireAdmin_AdminRole(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "admin") + ctx = context.WithValue(ctx, UserIDKey, "adminuser001") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + RequireAdmin(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200 for admin, got %d", w.Code) + } +} + +func TestRequireAdmin_NonAdmin(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "verified_email") + ctx = context.WithValue(ctx, UserIDKey, "someuser001") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + RequireAdmin(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for non-admin, got %d", w.Code) + } +} + +func TestRequireAdmin_NoUserIDInContext(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "admin") + // No UserIDKey set + req = req.WithContext(ctx) + w := httptest.NewRecorder() + RequireAdmin(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401 when UserID missing from context, got %d", w.Code) + } +} + +func TestRequireAdmin_EmptyUserID(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + ctx := context.WithValue(req.Context(), UserRoleKey, "admin") + ctx = context.WithValue(ctx, UserIDKey, "") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + RequireAdmin(testHandler()).ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401 for empty UserID, got %d", w.Code) + } +} + +// ============================================================================= +// Context helpers — GetUserID, GetUserRole, GetJTI +// ============================================================================= + +func TestGetUserID_Present(t *testing.T) { + ctx := context.WithValue(context.Background(), UserIDKey, "abc123") + id, ok := GetUserID(ctx) + if !ok { + t.Error("expected ok=true") + } + if id != "abc123" { + t.Errorf("expected 'abc123', got %q", id) + } +} + +func TestGetUserID_Missing(t *testing.T) { + _, ok := GetUserID(context.Background()) + if ok { + t.Error("expected ok=false") + } +} + +func TestGetUserRole_Present(t *testing.T) { + ctx := context.WithValue(context.Background(), UserRoleKey, "admin") + role, ok := GetUserRole(ctx) + if !ok { + t.Error("expected ok=true") + } + if role != "admin" { + t.Errorf("expected 'admin', got %q", role) + } +} + +func TestGetUserRole_Missing(t *testing.T) { + _, ok := GetUserRole(context.Background()) + if ok { + t.Error("expected ok=false") + } +} + +func TestGetJTI_Present(t *testing.T) { + ctx := context.WithValue(context.Background(), JTIKey, "jti_test_123") + jti, ok := GetJTI(ctx) + if !ok { + t.Error("expected ok=true") + } + if jti != "jti_test_123" { + t.Errorf("expected 'jti_test_123', got %q", jti) + } +} + +func TestGetJTI_Missing(t *testing.T) { + _, ok := GetJTI(context.Background()) + if ok { + t.Error("expected ok=false") + } +} + +// ============================================================================= +// Full middleware chain — RequireAuth → RequireAdmin +// ============================================================================= + +func TestAuthChain_AdminTokenFullFlow(t *testing.T) { + token := jwt.GenerateAdminToken() + + handler := RequireAuth(RequireAdmin(testHandler())) + + req := httptest.NewRequest("GET", "/admin/dashboard", nil) + req.Header.Set("Authorization", "Bearer "+token) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200 for admin token through full chain, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestAuthChain_VerifiedTokenBlockedByRequireAdmin(t *testing.T) { + token := jwt.GenerateTestToken("user123abc", "verified_email") + + handler := RequireAuth(RequireAdmin(testHandler())) + + req := httptest.NewRequest("GET", "/admin/dashboard", nil) + req.Header.Set("Authorization", "Bearer "+token) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for verified_email through RequireAdmin, got %d", w.Code) + } +} + +func TestAuthChain_NoTokenBlockedByRequireAuth(t *testing.T) { + handler := RequireAuth(RequireAdmin(testHandler())) + + req := httptest.NewRequest("GET", "/admin/dashboard", nil) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401 with no token, got %d", w.Code) + } +}