From cf14ea0d608c7767b0ee57ccd809236aadf1599f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 30 May 2026 19:36:42 +0200 Subject: [PATCH 1/6] 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 | 93 ++++++++++++++++++++++++++-- internal/api/router.go | 2 +- internal/api/router_authuser_test.go | 22 +++++-- 3 files changed, 104 insertions(+), 13 deletions(-) diff --git a/internal/api/middleware.go b/internal/api/middleware.go index c89797d9c..41857929e 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -43,6 +43,33 @@ func (h *Handler) isPublicEndpoint(path string) bool { return false } +// 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) @@ -58,6 +85,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 == "" { @@ -242,12 +323,12 @@ func logMissingCSRFToken(req *events.LambdaFunctionURLRequest, csrfToken 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 e41f9e406..f83ad15ed 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -373,7 +373,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 7e5571a85..7b76da6e2 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 f26fe5e3bd567bf514f8a343ad002a4016cc45eb Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 1 Jun 2026 19:25:26 +0200 Subject: [PATCH 2/6] 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 41857929e..54e3a14b2 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -100,45 +100,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 f15285ff99abe8eef4021f28da528f13f942ae5b Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 5 Jun 2026 16:25:29 +0200 Subject: [PATCH 3/6] 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 54e3a14b2..eae6dac9e 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -126,17 +126,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 7b76da6e2..119dafa15 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 54ddb10a8e725a191e62817d12dec36a872b6264 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sun, 7 Jun 2026 14:44:08 -0700 Subject: [PATCH 4/6] 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 eae6dac9e..35ead9bfc 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -163,7 +163,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 119dafa15..d6989c897 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 c4c2ce1e4e3073bb414e2408b0ac4830b2397b04 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 19 Jun 2026 23:53:53 +0200 Subject: [PATCH 5/6] 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 35ead9bfc..a26279914 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -66,7 +66,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 } @@ -126,26 +126,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 d6989c897..756c1fa9b 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 569efd7f40717ad55ae257c2b58f8bc65e80ed47 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 20 Jun 2026 02:14:21 +0200 Subject: [PATCH 6/6] 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 a26279914..7dbcc3512 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -66,7 +66,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 } @@ -93,7 +92,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 756c1fa9b..c010e9c8f 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