From 5f5a997c9703e5216f27b0df91275f0b6ae3b47d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 30 May 2026 19:36:42 +0200 Subject: [PATCH 1/8] refactor(api): requireAuth returns resolved Principal (closes #194) authenticatePrincipal resolves the caller's identity once across all three credential kinds (admin API key, user API key, bearer session) and returns a *Principal. requireAuth now delegates to it instead of calling authenticate() which threw the resolved identity away. Router updated to _, err := r.h.requireAuth(...). Nil-auth guard moved before the API-key path so a missing auth service fails closed (401) rather than falling through. Dead userIface/userFields locals removed. TestRequireAuth_* tests extended to assert the returned Principal (Kind, Role, UserID, Session pointer) for each credential type. --- internal/api/middleware.go | 95 ++++++++++++++++++++++++++-- internal/api/router.go | 2 +- internal/api/router_authuser_test.go | 22 +++++-- 3 files changed, 105 insertions(+), 14 deletions(-) diff --git a/internal/api/middleware.go b/internal/api/middleware.go index ee28a9d3a..ea38263ef 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -50,7 +50,34 @@ func (h *Handler) isPublicEndpoint(path string) bool { return false } -// authenticate checks authentication via admin API key, user API key, or Bearer token. +// PrincipalKind identifies which credential type was used to authenticate. +type PrincipalKind string + +const ( + // PrincipalAdminAPIKey is set when the request authenticated with the + // shared admin API key (X-API-Key header matching h.apiKey). + PrincipalAdminAPIKey PrincipalKind = "admin-api-key" + // PrincipalUserAPIKey is set when the request authenticated with a + // per-user API key issued via /api/api-keys. + PrincipalUserAPIKey PrincipalKind = "user-api-key" + // PrincipalSession is set when the request authenticated with a + // bearer-token session (X-Authorization / Authorization header). + PrincipalSession PrincipalKind = "session" +) + +// Principal carries the resolved caller identity returned by +// authenticatePrincipal and requireAuth. Handlers that need the caller's +// identity read it from here rather than re-resolving it through a second +// ValidateSession / ValidateUserAPIKeyAPI call. +type Principal struct { + Kind PrincipalKind + UserID string // empty for PrincipalAdminAPIKey + Email string // empty for PrincipalAdminAPIKey; populated for session/user-api-key + Role string // "admin" for admin-api-key; user's role otherwise + Session *Session // non-nil only for PrincipalSession +} + +// authenticate checks authentication via admin API key, user API key, or Bearer token func (h *Handler) authenticate(ctx context.Context, req *events.LambdaFunctionURLRequest) bool { apiKey := extractAPIKey(req) @@ -65,6 +92,60 @@ func (h *Handler) authenticate(ctx context.Context, req *events.LambdaFunctionUR return h.checkBearerToken(ctx, req) } +// authenticatePrincipal performs the same three-path credential check as +// authenticate but returns the fully resolved Principal so callers do not +// need to repeat the lookup. Returns a non-nil Principal on success; returns +// nil and a 401 ClientError when no valid credential is present. +func (h *Handler) authenticatePrincipal(ctx context.Context, req *events.LambdaFunctionURLRequest) (*Principal, error) { + apiKey := extractAPIKey(req) + + if h.checkAdminAPIKey(apiKey) { + return &Principal{Kind: PrincipalAdminAPIKey, Role: "admin"}, nil + } + + if h.auth == nil { + return nil, NewClientError(401, "authentication required") + } + + if apiKey != "" { + _, userRaw, err := h.auth.ValidateUserAPIKeyAPI(ctx, apiKey) + if err == nil && userRaw != nil { + p := &Principal{Kind: PrincipalUserAPIKey, Role: "user"} + // userRaw is returned as any from the interface. Extract fields + // via a locally-scoped interface to avoid an import cycle. + if uf, ok := userRaw.(interface { + GetID() string + GetEmail() string + GetRole() string + }); ok { + p.UserID = uf.GetID() + p.Email = uf.GetEmail() + p.Role = uf.GetRole() + } + return p, nil + } + if err != nil { + logging.Debugf("User API key validation failed: %v", err) + } + } + + token := h.extractBearerToken(req) + if token != "" { + session, err := h.auth.ValidateSession(ctx, token) + if err == nil && session != nil { + return &Principal{ + Kind: PrincipalSession, + UserID: session.UserID, + Email: session.Email, + Role: session.Role, + Session: session, + }, nil + } + } + + return nil, NewClientError(401, "authentication required") +} + func extractAPIKey(req *events.LambdaFunctionURLRequest) string { apiKey := req.Headers["x-api-key"] if apiKey == "" { @@ -289,12 +370,12 @@ func redactQueryParam(u, param string) string { // 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") +// the router level. Returns the resolved Principal on success, a 401 +// ClientError otherwise. Callers should use the returned Principal rather +// than re-resolving the caller's identity through a second ValidateSession +// or ValidateUserAPIKeyAPI call. +func (h *Handler) requireAuth(ctx context.Context, req *events.LambdaFunctionURLRequest) (*Principal, error) { + return h.authenticatePrincipal(ctx, req) } // requireAdmin gates the coarse admin-only routes (AuthAdmin). "Admin" is now diff --git a/internal/api/router.go b/internal/api/router.go index 81e5c5ffc..7e152cd45 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -385,7 +385,7 @@ func (r *Router) Route(ctx context.Context, method, path string, req *events.Lam return nil, err } case AuthUser: - if err := r.h.requireAuth(ctx, req); err != nil { + if _, err := r.h.requireAuth(ctx, req); err != nil { return nil, err } case AuthPublic: diff --git a/internal/api/router_authuser_test.go b/internal/api/router_authuser_test.go index 39368d5e6..fc5c6ba67 100644 --- a/internal/api/router_authuser_test.go +++ b/internal/api/router_authuser_test.go @@ -100,18 +100,22 @@ func TestRouterAuthPublic_NoCredentials_Accepts(t *testing.T) { require.NoError(t, err) } -// TestRequireAuth_AdminAPIKey verifies the new requireAuth helper accepts -// the admin API key. +// TestRequireAuth_AdminAPIKey verifies that requireAuth accepts the admin +// API key and returns a Principal of kind PrincipalAdminAPIKey. 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)) + p, err := h.requireAuth(context.Background(), req) + require.NoError(t, err) + require.NotNil(t, p) + assert.Equal(t, PrincipalAdminAPIKey, p.Kind) + assert.Equal(t, "admin", p.Role) } // TestRequireAuth_UserSession verifies requireAuth accepts a valid -// non-admin user session. +// non-admin user session and returns a populated Principal. func TestRequireAuth_UserSession(t *testing.T) { ctx := context.Background() mockAuth := new(MockAuthService) @@ -121,7 +125,13 @@ func TestRequireAuth_UserSession(t *testing.T) { req := &events.LambdaFunctionURLRequest{ Headers: map[string]string{"Authorization": "Bearer user-token"}, } - require.NoError(t, h.requireAuth(ctx, req)) + p, err := h.requireAuth(ctx, req) + require.NoError(t, err) + require.NotNil(t, p) + assert.Equal(t, PrincipalSession, p.Kind) + assert.Equal(t, "uid", p.UserID) + assert.Equal(t, "user", p.Role) + assert.Equal(t, userSession, p.Session) } // TestRequireAuth_NoCredential_Rejects verifies requireAuth returns a 401 @@ -130,7 +140,7 @@ 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) + _, err := h.requireAuth(context.Background(), req) require.Error(t, err) ce, ok := IsClientError(err) require.True(t, ok) From a849063825bc79eac73f1d0035a5eca627d833d0 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 1 Jun 2026 19:25:26 +0200 Subject: [PATCH 2/8] fix(ci): resolve failing gocyclo check on PR #864 Extract principalFromUserAPIKey and principalFromBearerToken helpers from authenticatePrincipal to bring its cyclomatic complexity from 11 down to 4, passing the gocyclo <=10 gate. --- internal/api/middleware.go | 86 ++++++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 32 deletions(-) diff --git a/internal/api/middleware.go b/internal/api/middleware.go index ea38263ef..34a637b7a 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -107,45 +107,67 @@ func (h *Handler) authenticatePrincipal(ctx context.Context, req *events.LambdaF return nil, NewClientError(401, "authentication required") } - if apiKey != "" { - _, userRaw, err := h.auth.ValidateUserAPIKeyAPI(ctx, apiKey) - if err == nil && userRaw != nil { - p := &Principal{Kind: PrincipalUserAPIKey, Role: "user"} - // userRaw is returned as any from the interface. Extract fields - // via a locally-scoped interface to avoid an import cycle. - if uf, ok := userRaw.(interface { - GetID() string - GetEmail() string - GetRole() string - }); ok { - p.UserID = uf.GetID() - p.Email = uf.GetEmail() - p.Role = uf.GetRole() - } - return p, nil - } - if err != nil { - logging.Debugf("User API key validation failed: %v", err) - } + if p := h.principalFromUserAPIKey(ctx, apiKey); p != nil { + return p, nil } - token := h.extractBearerToken(req) - if token != "" { - session, err := h.auth.ValidateSession(ctx, token) - if err == nil && session != nil { - return &Principal{ - Kind: PrincipalSession, - UserID: session.UserID, - Email: session.Email, - Role: session.Role, - Session: session, - }, nil - } + if p := h.principalFromBearerToken(ctx, req); p != nil { + return p, nil } return nil, NewClientError(401, "authentication required") } +// principalFromUserAPIKey resolves a Principal from a user API key. +// Returns nil when the key is empty, validation fails, or the user record +// cannot be retrieved. +func (h *Handler) principalFromUserAPIKey(ctx context.Context, apiKey string) *Principal { + if apiKey == "" { + return nil + } + _, userRaw, err := h.auth.ValidateUserAPIKeyAPI(ctx, apiKey) + if err != nil { + logging.Debugf("User API key validation failed: %v", err) + return nil + } + if userRaw == nil { + return nil + } + p := &Principal{Kind: PrincipalUserAPIKey, Role: "user"} + // userRaw is returned as any from the interface. Extract fields + // via a locally-scoped interface to avoid an import cycle. + if uf, ok := userRaw.(interface { + GetID() string + GetEmail() string + GetRole() string + }); ok { + p.UserID = uf.GetID() + p.Email = uf.GetEmail() + p.Role = uf.GetRole() + } + return p +} + +// principalFromBearerToken resolves a Principal from a session bearer token. +// Returns nil when no token is present or the session is invalid. +func (h *Handler) principalFromBearerToken(ctx context.Context, req *events.LambdaFunctionURLRequest) *Principal { + token := h.extractBearerToken(req) + if token == "" { + return nil + } + session, err := h.auth.ValidateSession(ctx, token) + if err != nil || session == nil { + return nil + } + return &Principal{ + Kind: PrincipalSession, + UserID: session.UserID, + Email: session.Email, + Role: session.Role, + Session: session, + } +} + func extractAPIKey(req *events.LambdaFunctionURLRequest) string { apiKey := req.Headers["x-api-key"] if apiKey == "" { From 06a6755671781282eb29c88f558b478c7fe3771b Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 5 Jun 2026 16:25:29 +0200 Subject: [PATCH 3/8] fix(api/auth): fail closed in principalFromUserAPIKey + cover user-API-key and nil-auth paths (refs #194) When ValidateUserAPIKeyAPI returns a userRaw value whose concrete type does not satisfy the GetID/GetEmail/GetRole interface, principalFromUserAPIKey previously returned a partially-populated Principal (Role:"user", empty UserID/Email). Any handler trusting the Principal would have granted access to an unresolved identity. Fix: return nil (deny) when the assertion fails. Adds three tests: - TestRequireAuth_UserAPIKey: valid user API key yields PrincipalUserAPIKey with populated UserID/Email/Role (zero coverage on the #178 hot-spot). - TestRequireAuth_UserAPIKey_BadRecord: unexpected userRaw type yields 401; fails pre-fix (Principal returned), passes post-fix (nil returned). - TestRequireAuth_NilAuth_Rejects: Handler with auth==nil returns 401 for a non-admin key, confirming the fail-closed nil-auth branch. --- internal/api/middleware.go | 19 +++++-- internal/api/router_authuser_test.go | 84 ++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 6 deletions(-) diff --git a/internal/api/middleware.go b/internal/api/middleware.go index 34a637b7a..7911ffb86 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -133,17 +133,24 @@ func (h *Handler) principalFromUserAPIKey(ctx context.Context, apiKey string) *P if userRaw == nil { return nil } - p := &Principal{Kind: PrincipalUserAPIKey, Role: "user"} // userRaw is returned as any from the interface. Extract fields // via a locally-scoped interface to avoid an import cycle. - if uf, ok := userRaw.(interface { + // If the assertion fails (unexpected concrete type) we deny rather than + // returning a partially-populated Principal: fail closed. + uf, ok := userRaw.(interface { GetID() string GetEmail() string GetRole() string - }); ok { - p.UserID = uf.GetID() - p.Email = uf.GetEmail() - p.Role = uf.GetRole() + }) + if !ok { + logging.Debugf("User API key: userRaw does not implement expected interface (%T); denying", userRaw) + return nil + } + p := &Principal{ + Kind: PrincipalUserAPIKey, + UserID: uf.GetID(), + Email: uf.GetEmail(), + Role: uf.GetRole(), } return p } diff --git a/internal/api/router_authuser_test.go b/internal/api/router_authuser_test.go index fc5c6ba67..071683169 100644 --- a/internal/api/router_authuser_test.go +++ b/internal/api/router_authuser_test.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-lambda-go/events" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -146,3 +147,86 @@ func TestRequireAuth_NoCredential_Rejects(t *testing.T) { require.True(t, ok) assert.Equal(t, 401, ce.code) } + +// testUserRecord is a minimal local type that satisfies the GetID/GetEmail/GetRole +// interface checked inside principalFromUserAPIKey. It represents a concrete +// user value returned by ValidateUserAPIKeyAPI on the happy path. +type testUserRecord struct { + id string + email string + role string +} + +func (u *testUserRecord) GetID() string { return u.id } +func (u *testUserRecord) GetEmail() string { return u.email } +func (u *testUserRecord) GetRole() string { return u.role } + +// TestRequireAuth_UserAPIKey verifies that a valid user API key yields a +// Principal with Kind == PrincipalUserAPIKey and populated UserID/Email/Role. +// This covers the #178 regression hot-spot that previously had zero coverage. +func TestRequireAuth_UserAPIKey(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + userRec := &testUserRecord{id: "uid-123", email: "alice@example.com", role: "user"} + mockAuth.On("ValidateUserAPIKeyAPI", ctx, "valid-user-key"). + Return(nil, userRec, nil) + + h := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"X-API-Key": "valid-user-key"}, + } + p, err := h.requireAuth(ctx, req) + require.NoError(t, err) + require.NotNil(t, p) + assert.Equal(t, PrincipalUserAPIKey, p.Kind) + assert.Equal(t, "uid-123", p.UserID) + assert.Equal(t, "alice@example.com", p.Email) + assert.Equal(t, "user", p.Role) +} + +// TestRequireAuth_UserAPIKey_BadRecord verifies that principalFromUserAPIKey +// fails closed when ValidateUserAPIKeyAPI succeeds but returns a userRaw +// value that does NOT satisfy the GetID/GetEmail/GetRole interface (unexpected +// concrete type). Pre-fix this returned a Role:"user" Principal; post-fix it +// must return nil and the overall requireAuth must return a 401 ClientError. +func TestRequireAuth_UserAPIKey_BadRecord(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + // Return a value that does NOT implement GetID/GetEmail/GetRole. + type unexpectedType struct{ Name string } + mockAuth.On("ValidateUserAPIKeyAPI", ctx, "bad-record-key"). + Return(nil, &unexpectedType{Name: "oops"}, nil) + // Bearer and session path must also return nothing. + mockAuth.On("ValidateSession", ctx, mock.Anything). + Return(nil, errors.New("no session")).Maybe() + + h := &Handler{auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"X-API-Key": "bad-record-key"}, + } + _, err := h.requireAuth(ctx, req) + require.Error(t, err, "expected denial when user record has unexpected type") + ce, ok := IsClientError(err) + require.True(t, ok, "expected ClientError, got %T: %v", err, err) + assert.Equal(t, 401, ce.code) +} + +// TestRequireAuth_NilAuth_Rejects verifies that a Handler constructed with +// auth == nil returns a 401 ClientError for a non-admin credential, confirming +// the h.auth == nil branch fails closed. +func TestRequireAuth_NilAuth_Rejects(t *testing.T) { + // No admin key configured, auth is nil. + h := &Handler{auth: nil} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"X-API-Key": "some-key"}, + } + _, err := h.requireAuth(context.Background(), 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) +} From 8dce565e6139b525ab4c18ad7ccd68cfb612b727 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sun, 7 Jun 2026 14:44:08 -0700 Subject: [PATCH 4/8] fix(api): drop session.Role after base removed the field Session no longer carries Role (removed upstream when the auth model moved to group-based permissions via HasPermissionAPI). Remove the stale session.Role reference from principalFromBearerToken and drop the corresponding test assertion that expected a role value the session mock never set. Closes the go vet compile error blocking pre-commit on PR #864. --- internal/api/middleware.go | 1 - internal/api/router_authuser_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/internal/api/middleware.go b/internal/api/middleware.go index 7911ffb86..96a706ce7 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -170,7 +170,6 @@ func (h *Handler) principalFromBearerToken(ctx context.Context, req *events.Lamb Kind: PrincipalSession, UserID: session.UserID, Email: session.Email, - Role: session.Role, Session: session, } } diff --git a/internal/api/router_authuser_test.go b/internal/api/router_authuser_test.go index 071683169..fb5446137 100644 --- a/internal/api/router_authuser_test.go +++ b/internal/api/router_authuser_test.go @@ -131,7 +131,6 @@ func TestRequireAuth_UserSession(t *testing.T) { require.NotNil(t, p) assert.Equal(t, PrincipalSession, p.Kind) assert.Equal(t, "uid", p.UserID) - assert.Equal(t, "user", p.Role) assert.Equal(t, userSession, p.Session) } From a440c4ec991bb7803be1f77b6f52484447e85ab1 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 19 Jun 2026 23:53:53 +0200 Subject: [PATCH 5/8] fix(api): assert *auth.User directly in principalFromUserAPIKey The duck-typed interface assertion (GetID/GetEmail/GetRole) always fails at runtime because *auth.User exposes only struct fields, not methods. Every user-API-key request was silently 401ing. Fix: assert userRaw to *auth.User directly (middleware.go already imports auth). Update TestRequireAuth_UserAPIKey to use the real *auth.User concrete type instead of the testUserRecord stand-in, so the test now exercises the same type assertion as production code. Drop the stale Role assertion (role system removed in #907) and the testUserRecord helper. --- internal/api/middleware.go | 25 +++++++++---------------- internal/api/router_authuser_test.go | 28 ++++++++-------------------- 2 files changed, 17 insertions(+), 36 deletions(-) diff --git a/internal/api/middleware.go b/internal/api/middleware.go index 96a706ce7..5a5a5dfb5 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -73,7 +73,7 @@ type Principal struct { Kind PrincipalKind UserID string // empty for PrincipalAdminAPIKey Email string // empty for PrincipalAdminAPIKey; populated for session/user-api-key - Role string // "admin" for admin-api-key; user's role otherwise + Role string // "admin" for PrincipalAdminAPIKey; empty for session/user-api-key (role system removed in #907) Session *Session // non-nil only for PrincipalSession } @@ -133,26 +133,19 @@ func (h *Handler) principalFromUserAPIKey(ctx context.Context, apiKey string) *P if userRaw == nil { return nil } - // userRaw is returned as any from the interface. Extract fields - // via a locally-scoped interface to avoid an import cycle. - // If the assertion fails (unexpected concrete type) we deny rather than - // returning a partially-populated Principal: fail closed. - uf, ok := userRaw.(interface { - GetID() string - GetEmail() string - GetRole() string - }) + // ValidateUserAPIKeyAPI returns *auth.User as any via the AuthServiceInterface. + // Assert to the concrete type; if validation succeeded but the concrete type is + // unexpected we deny rather than returning a partially-populated Principal: fail closed. + user, ok := userRaw.(*auth.User) if !ok { - logging.Debugf("User API key: userRaw does not implement expected interface (%T); denying", userRaw) + logging.Debugf("User API key: userRaw has unexpected type (%T); denying", userRaw) return nil } - p := &Principal{ + return &Principal{ Kind: PrincipalUserAPIKey, - UserID: uf.GetID(), - Email: uf.GetEmail(), - Role: uf.GetRole(), + UserID: user.ID, + Email: user.Email, } - return p } // principalFromBearerToken resolves a Principal from a session bearer token. diff --git a/internal/api/router_authuser_test.go b/internal/api/router_authuser_test.go index fb5446137..b7cabc15f 100644 --- a/internal/api/router_authuser_test.go +++ b/internal/api/router_authuser_test.go @@ -5,6 +5,7 @@ import ( "errors" "testing" + "github.com/LeanerCloud/CUDly/internal/auth" "github.com/aws/aws-lambda-go/events" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -147,28 +148,17 @@ func TestRequireAuth_NoCredential_Rejects(t *testing.T) { assert.Equal(t, 401, ce.code) } -// testUserRecord is a minimal local type that satisfies the GetID/GetEmail/GetRole -// interface checked inside principalFromUserAPIKey. It represents a concrete -// user value returned by ValidateUserAPIKeyAPI on the happy path. -type testUserRecord struct { - id string - email string - role string -} - -func (u *testUserRecord) GetID() string { return u.id } -func (u *testUserRecord) GetEmail() string { return u.email } -func (u *testUserRecord) GetRole() string { return u.role } - // TestRequireAuth_UserAPIKey verifies that a valid user API key yields a -// Principal with Kind == PrincipalUserAPIKey and populated UserID/Email/Role. -// This covers the #178 regression hot-spot that previously had zero coverage. +// Principal with Kind == PrincipalUserAPIKey and populated UserID/Email. +// The mock returns the real *auth.User concrete type — the same type that +// auth.Service.ValidateUserAPIKeyAPI returns in production — so this test +// exercises the same type assertion that principalFromUserAPIKey performs. func TestRequireAuth_UserAPIKey(t *testing.T) { ctx := context.Background() mockAuth := new(MockAuthService) t.Cleanup(func() { mockAuth.AssertExpectations(t) }) - userRec := &testUserRecord{id: "uid-123", email: "alice@example.com", role: "user"} + userRec := &auth.User{ID: "uid-123", Email: "alice@example.com"} mockAuth.On("ValidateUserAPIKeyAPI", ctx, "valid-user-key"). Return(nil, userRec, nil) @@ -182,20 +172,18 @@ func TestRequireAuth_UserAPIKey(t *testing.T) { assert.Equal(t, PrincipalUserAPIKey, p.Kind) assert.Equal(t, "uid-123", p.UserID) assert.Equal(t, "alice@example.com", p.Email) - assert.Equal(t, "user", p.Role) } // TestRequireAuth_UserAPIKey_BadRecord verifies that principalFromUserAPIKey // fails closed when ValidateUserAPIKeyAPI succeeds but returns a userRaw -// value that does NOT satisfy the GetID/GetEmail/GetRole interface (unexpected -// concrete type). Pre-fix this returned a Role:"user" Principal; post-fix it +// value that is not *auth.User (unexpected concrete type). Post-fix it // must return nil and the overall requireAuth must return a 401 ClientError. func TestRequireAuth_UserAPIKey_BadRecord(t *testing.T) { ctx := context.Background() mockAuth := new(MockAuthService) t.Cleanup(func() { mockAuth.AssertExpectations(t) }) - // Return a value that does NOT implement GetID/GetEmail/GetRole. + // Return a value that is NOT *auth.User — the concrete type principalFromUserAPIKey asserts. type unexpectedType struct{ Name string } mockAuth.On("ValidateUserAPIKeyAPI", ctx, "bad-record-key"). Return(nil, &unexpectedType{Name: "oops"}, nil) From 17d3bd8e4797ffcb87e43914649fbcf9c122e763 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 20 Jun 2026 02:14:21 +0200 Subject: [PATCH 6/8] refactor(api): drop unused Principal.Role role-semantics field Principal.Role encoded deprecated role-based authorization semantics that the group-based model (issue #907) made obsolete. The only non-test reader of requireAuth's returned Principal is Router.Route, which discards it and checks only the error; admin is enforced via the stateless admin-key check and HasPermissionAPI(ActionAdmin), never via Principal.Role. Remove the field, its lone "admin" assignment on the admin-key path, and the stale test assertion. --- internal/api/middleware.go | 3 +-- internal/api/router_authuser_test.go | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/api/middleware.go b/internal/api/middleware.go index 5a5a5dfb5..12717a850 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -73,7 +73,6 @@ type Principal struct { Kind PrincipalKind UserID string // empty for PrincipalAdminAPIKey Email string // empty for PrincipalAdminAPIKey; populated for session/user-api-key - Role string // "admin" for PrincipalAdminAPIKey; empty for session/user-api-key (role system removed in #907) Session *Session // non-nil only for PrincipalSession } @@ -100,7 +99,7 @@ func (h *Handler) authenticatePrincipal(ctx context.Context, req *events.LambdaF apiKey := extractAPIKey(req) if h.checkAdminAPIKey(apiKey) { - return &Principal{Kind: PrincipalAdminAPIKey, Role: "admin"}, nil + return &Principal{Kind: PrincipalAdminAPIKey}, nil } if h.auth == nil { diff --git a/internal/api/router_authuser_test.go b/internal/api/router_authuser_test.go index b7cabc15f..f1d36a01f 100644 --- a/internal/api/router_authuser_test.go +++ b/internal/api/router_authuser_test.go @@ -113,7 +113,6 @@ func TestRequireAuth_AdminAPIKey(t *testing.T) { require.NoError(t, err) require.NotNil(t, p) assert.Equal(t, PrincipalAdminAPIKey, p.Kind) - assert.Equal(t, "admin", p.Role) } // TestRequireAuth_UserSession verifies requireAuth accepts a valid From 43a9680ad7228f49108d99bd34fe4a1fb8a90ab0 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 10 Jul 2026 17:38:45 +0200 Subject: [PATCH 7/8] style(api): reorder Principal fields for govet fieldalignment The Principal struct introduced by this refactor triggered the govet fieldalignment check (56 pointer bytes could be 48). Move the Session pointer ahead of the trailing Email string so a string field sits last, shrinking the GC pointer-scan region. Behaviour is unchanged; all struct literals reference fields by name. --- internal/api/middleware.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/api/middleware.go b/internal/api/middleware.go index 12717a850..6f9b08079 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -70,10 +70,12 @@ const ( // identity read it from here rather than re-resolving it through a second // ValidateSession / ValidateUserAPIKeyAPI call. type Principal struct { + // Field order groups the pointer-bearing Session ahead of the trailing + // string so the struct satisfies govet fieldalignment. Kind PrincipalKind + Session *Session // non-nil only for PrincipalSession UserID string // empty for PrincipalAdminAPIKey Email string // empty for PrincipalAdminAPIKey; populated for session/user-api-key - Session *Session // non-nil only for PrincipalSession } // authenticate checks authentication via admin API key, user API key, or Bearer token From b6fd257a579f25a920d9f1abe5250f4625debfd7 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sun, 19 Jul 2026 20:09:57 +0200 Subject: [PATCH 8/8] fix(lint): godot period + gosec G101 suppression in middleware Add missing terminal period to authenticate's godoc comment so godot passes. Annotate PrincipalUserAPIKey with #nosec G101 to suppress the false-positive hardcoded-credential finding; the string is a credential-type label, not a credential value. --- internal/api/middleware.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/api/middleware.go b/internal/api/middleware.go index 6f9b08079..bb56a5c05 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -59,7 +59,7 @@ const ( PrincipalAdminAPIKey PrincipalKind = "admin-api-key" // PrincipalUserAPIKey is set when the request authenticated with a // per-user API key issued via /api/api-keys. - PrincipalUserAPIKey PrincipalKind = "user-api-key" + PrincipalUserAPIKey PrincipalKind = "user-api-key" // #nosec G101 -- credential-type label, not a credential value // PrincipalSession is set when the request authenticated with a // bearer-token session (X-Authorization / Authorization header). PrincipalSession PrincipalKind = "session" @@ -78,7 +78,7 @@ type Principal struct { Email string // empty for PrincipalAdminAPIKey; populated for session/user-api-key } -// authenticate checks authentication via admin API key, user API key, or Bearer token +// authenticate checks authentication via admin API key, user API key, or Bearer token. func (h *Handler) authenticate(ctx context.Context, req *events.LambdaFunctionURLRequest) bool { apiKey := extractAPIKey(req)