fix(security): defence-in-depth AuthUser check in Router.Route

internal/api/middleware.go

@@ -209,6 +209,21 @@ func logMissingCSRFToken(req *events.LambdaFunctionURLRequest, csrfToken string)
 		req.RequestContext.HTTP.Method, req.RequestContext.HTTP.Path, req.RequestContext.HTTP.SourceIP)
 }
 
+// requireAuth verifies the request carries a valid authentication credential
+// of any kind (admin API key, user API key, or session bearer token).
+//
+// Used as a defence-in-depth check by Router.Route for AuthUser routes:
+// validateSecurity → authenticate already runs before dispatch, but if a
+// future refactor reorders middleware or a new route bypasses
+// validateSecurity, this check still rejects unauthenticated requests at
+// the router level. Returns nil on success, a 401 ClientError otherwise.
+func (h *Handler) requireAuth(ctx context.Context, req *events.LambdaFunctionURLRequest) error {
+	if h.authenticate(ctx, req) {
+		return nil
+	}
+	return NewClientError(401, "authentication required")
+}
+
 // requireAdmin checks if the current user has admin role.
 // Accepts both admin API-key auth and Bearer token auth.
 func (h *Handler) requireAdmin(ctx context.Context, req *events.LambdaFunctionURLRequest) (*Session, error) {

internal/api/router.go

@@ -14,17 +14,26 @@ type RouteHandler func(ctx context.Context, req *events.LambdaFunctionURLRequest
 // AuthLevel controls how Router.Route() enforces authentication.
 // The zero value is AuthAdmin — secure by default.
 // Any Route without an explicit Auth field requires admin access.
+//
+// Router.Route enforces these levels itself as a defence-in-depth check,
+// in addition to the validateSecurity → authenticate middleware that runs
+// earlier in the request pipeline. If middleware ordering ever changes or
+// a new route bypasses validateSecurity, the router-level enforcement
+// still rejects unauthorized requests.
 type AuthLevel int
 
 const (
-	// AuthAdmin requires admin role (API key or admin bearer token).
-	// Zero value — any Route without an explicit Auth field gets this.
+	// AuthAdmin requires admin role (admin API key or admin bearer-token
+	// session). Zero value — any Route without an explicit Auth field gets
+	// this. Enforced by Router.Route via h.requireAdmin.
 	AuthAdmin AuthLevel = iota
-	// AuthUser requires any authenticated user. Use for self-service
-	// endpoints (logout, profile, API key management).
+	// AuthUser requires any authenticated user (admin API key, user API
+	// key, or any valid bearer-token session). Use for self-service
+	// endpoints (logout, profile, API key management). Enforced by
+	// Router.Route via h.requireAuth.
 	AuthUser
 	// AuthPublic requires no authentication. Must also be listed in
-	// isPublicEndpoint() for middleware bypass.
+	// isPublicEndpoint() so the middleware skips its auth/CSRF checks.
 	AuthPublic
 )
 
@@ -214,14 +223,28 @@ func (r *Router) registerRoutes() {
 }
 
 // Route finds and executes the matching route handler.
-// Routes with Auth == AuthAdmin (the default) require admin access before the handler is called.
+//
+// Authentication enforcement is defence-in-depth: validateSecurity →
+// authenticate already runs in the middleware pipeline before dispatch,
+// but Router.Route also enforces the per-route Auth level so routes stay
+// protected even if middleware ordering changes or a new code path
+// bypasses validateSecurity. Routes with Auth == AuthAdmin (the default)
+// require admin access; AuthUser routes require any authenticated user;
+// AuthPublic routes are unauthenticated.
 func (r *Router) Route(ctx context.Context, method, path string, req *events.LambdaFunctionURLRequest) (any, error) {
 	for _, route := range r.routes {
 		if r.matches(route, method, path) {
-			if route.Auth == AuthAdmin {
+			switch route.Auth {
+			case AuthAdmin:
 				if _, err := r.h.requireAdmin(ctx, req); err != nil {
 					return nil, err
 				}
+			case AuthUser:
+				if err := r.h.requireAuth(ctx, req); err != nil {
+					return nil, err
+				}
+			case AuthPublic:
+				// no auth check; relied upon by middleware via isPublicEndpoint
 			}
 			params := r.extractParams(route, path)
 			return route.Handler(ctx, req, params)

internal/api/router_authuser_test.go

@@ -0,0 +1,138 @@
+package api
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/aws/aws-lambda-go/events"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// These tests cover the defence-in-depth AuthUser enforcement added to
+// Router.Route — see issue #60. Before the fix, AuthUser routes (e.g.
+// /api/auth/logout, /api/api-keys, /api/federation/iac) fell through the
+// router with no auth check; only the validateSecurity middleware
+// protected them. These tests pin the router-level enforcement so a
+// future middleware refactor can't silently expose them.
+
+// TestRouterAuthUser_NoCredentials_Rejects verifies that an AuthUser
+// route returns a 401 ClientError when no credential is presented.
+func TestRouterAuthUser_NoCredentials_Rejects(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	h := &Handler{auth: mockAuth}
+	r := NewRouter(h)
+
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{},
+	}
+
+	_, err := r.Route(ctx, "POST", "/api/auth/logout", req)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected ClientError, got %T: %v", err, err)
+	assert.Equal(t, 401, ce.code)
+}
+
+// TestRouterAuthUser_InvalidBearerToken_Rejects verifies that an AuthUser
+// route returns 401 when the bearer token is not recognised by the auth
+// service.
+func TestRouterAuthUser_InvalidBearerToken_Rejects(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	mockAuth.On("ValidateUserAPIKeyAPI", ctx, "").
+		Return(nil, nil, errors.New("empty key"))
+	mockAuth.On("ValidateSession", ctx, "bad-token").
+		Return(nil, errors.New("expired"))
+	h := &Handler{auth: mockAuth}
+	r := NewRouter(h)
+
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"Authorization": "Bearer bad-token"},
+	}
+
+	_, err := r.Route(ctx, "GET", "/api/api-keys", req)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected ClientError, got %T: %v", err, err)
+	assert.Equal(t, 401, ce.code)
+}
+
+// TestRouterAuthUser_ValidUserSession_Accepts verifies that an AuthUser
+// route dispatches to the handler when a valid non-admin user session is
+// present. Before the fix, AuthUser dispatch was unconditional — this
+// test ensures the new check doesn't accidentally reject legitimate
+// users.
+func TestRouterAuthUser_ValidUserSession_Accepts(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	userSession := &Session{UserID: "11111111-1111-1111-1111-111111111111", Role: "user"}
+	mockAuth.On("ValidateSession", ctx, "user-token").Return(userSession, nil)
+	mockAuth.On("Logout", ctx, "user-token").Return(nil)
+	h := &Handler{auth: mockAuth}
+	r := NewRouter(h)
+
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"Authorization": "Bearer user-token"},
+	}
+
+	result, err := r.Route(ctx, "POST", "/api/auth/logout", req)
+	require.NoError(t, err)
+	assert.NotNil(t, result)
+}
+
+// TestRouterAuthPublic_NoCredentials_Accepts verifies that AuthPublic
+// routes still dispatch with no credentials — the new switch in
+// Router.Route must not regress public-endpoint behaviour.
+func TestRouterAuthPublic_NoCredentials_Accepts(t *testing.T) {
+	ctx := context.Background()
+	h := &Handler{}
+	r := NewRouter(h)
+
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{},
+	}
+
+	// /api/health is AuthPublic; healthCheckHandler doesn't depend on auth.
+	_, err := r.Route(ctx, "GET", "/api/health", req)
+	require.NoError(t, err)
+}
+
+// TestRequireAuth_AdminAPIKey verifies the new requireAuth helper accepts
+// the admin API key.
+func TestRequireAuth_AdminAPIKey(t *testing.T) {
+	h := &Handler{apiKey: "admin-secret"}
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"X-API-Key": "admin-secret"},
+	}
+	require.NoError(t, h.requireAuth(context.Background(), req))
+}
+
+// TestRequireAuth_UserSession verifies requireAuth accepts a valid
+// non-admin user session.
+func TestRequireAuth_UserSession(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	userSession := &Session{UserID: "uid", Role: "user"}
+	mockAuth.On("ValidateSession", ctx, "user-token").Return(userSession, nil)
+	h := &Handler{auth: mockAuth}
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"Authorization": "Bearer user-token"},
+	}
+	require.NoError(t, h.requireAuth(ctx, req))
+}
+
+// TestRequireAuth_NoCredential_Rejects verifies requireAuth returns a 401
+// ClientError when no credential is presented.
+func TestRequireAuth_NoCredential_Rejects(t *testing.T) {
+	mockAuth := new(MockAuthService)
+	h := &Handler{auth: mockAuth}
+	req := &events.LambdaFunctionURLRequest{Headers: map[string]string{}}
+	err := h.requireAuth(context.Background(), req)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok)
+	assert.Equal(t, 401, ce.code)
+}