From 72fb161f567e7de2c5ceb25bc5789447932fbe69 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Wed, 27 May 2026 15:29:46 -0700 Subject: [PATCH 1/5] refactor(compose): demote PrincipalAuthenticatorSpec to a function type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Spec interface had exactly one production implementation (accessJWTAuthenticatorSpec) — a premature port by the rule established in PRs #61 and #62 (no port without a second implementation). Replace with a function type so AccessJWT returns a closure and the accessJWTAuthenticatorSpec struct + BuildPrincipalAuthenticator method disappear. buildPrincipalAuthenticators now calls spec() instead of spec.BuildPrincipalAuthenticator(). Consumers (testkit/internal/authflow/runtime.go) compile unchanged: []compose.PrincipalAuthenticatorSpec{compose.AccessJWT(...)} works identically whether PrincipalAuthenticatorSpec is an interface or a function type. The test fixture principalAuthenticatorSpecFunc, which existed solely to wrap a func into the interface, is removed; tests now pass inline closures. --- http/compose/authenticators.go | 23 ++++++----------------- http/compose/http.go | 2 +- http/compose/http_test.go | 34 ++++++++++++++-------------------- 3 files changed, 21 insertions(+), 38 deletions(-) diff --git a/http/compose/authenticators.go b/http/compose/authenticators.go index 1350dc0..f7df6d4 100644 --- a/http/compose/authenticators.go +++ b/http/compose/authenticators.go @@ -7,27 +7,16 @@ import ( ) // PrincipalAuthenticatorSpec builds one authkit principal authenticator for a composed service. -type PrincipalAuthenticatorSpec interface { - // BuildPrincipalAuthenticator constructs the authenticator represented by the spec. - BuildPrincipalAuthenticator() (authkit.PrincipalAuthenticator, error) -} - -type accessJWTAuthenticatorSpec struct { - verifier *jwt.Verifier - principalFinder authkit.PrincipalFinder -} +// NewHTTP calls each spec exactly once during composition and propagates any error verbatim. +type PrincipalAuthenticatorSpec func() (authkit.PrincipalAuthenticator, error) -// AccessJWT configures an access JWT authenticator from verifier and principalFinder. +// AccessJWT configures an access JWT bearer-token principal authenticator from verifier and +// principalFinder. The returned spec calls middleware.NewAuthenticator at composition time. func AccessJWT( verifier *jwt.Verifier, principalFinder authkit.PrincipalFinder, ) PrincipalAuthenticatorSpec { - return accessJWTAuthenticatorSpec{ - verifier: verifier, - principalFinder: principalFinder, + return func() (authkit.PrincipalAuthenticator, error) { + return middleware.NewAuthenticator(verifier, principalFinder) } } - -func (s accessJWTAuthenticatorSpec) BuildPrincipalAuthenticator() (authkit.PrincipalAuthenticator, error) { - return middleware.NewAuthenticator(s.verifier, s.principalFinder) -} diff --git a/http/compose/http.go b/http/compose/http.go index 27930a5..8b8a8c8 100644 --- a/http/compose/http.go +++ b/http/compose/http.go @@ -67,7 +67,7 @@ func buildPrincipalAuthenticators( return nil, fmt.Errorf("compose: principal authenticator spec %d is required", i) } - authenticator, err := spec.BuildPrincipalAuthenticator() + authenticator, err := spec() if err != nil { return nil, fmt.Errorf("compose: build principal authenticator %d: %w", i, err) } diff --git a/http/compose/http_test.go b/http/compose/http_test.go index 7cc1d52..1b976ac 100644 --- a/http/compose/http_test.go +++ b/http/compose/http_test.go @@ -45,9 +45,9 @@ func TestNewHTTPValidatesInputs(t *testing.T) { name: "wraps principal authenticator build errors", options: compose.HTTPOptions{ PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ - principalAuthenticatorSpecFunc(func() (authkit.PrincipalAuthenticator, error) { + func() (authkit.PrincipalAuthenticator, error) { return nil, boom - }), + }, }, Authorizer: testAuthorizer{}, }, @@ -58,11 +58,11 @@ func TestNewHTTPValidatesInputs(t *testing.T) { name: "rejects nil built principal authenticator", options: compose.HTTPOptions{ PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ - principalAuthenticatorSpecFunc(func() (authkit.PrincipalAuthenticator, error) { + func() (authkit.PrincipalAuthenticator, error) { var authenticator authkit.PrincipalAuthenticator return authenticator, nil - }), + }, }, Authorizer: testAuthorizer{}, }, @@ -72,9 +72,9 @@ func TestNewHTTPValidatesInputs(t *testing.T) { name: "wraps missing authorizer error", options: compose.HTTPOptions{ PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ - principalAuthenticatorSpecFunc(func() (authkit.PrincipalAuthenticator, error) { + func() (authkit.PrincipalAuthenticator, error) { return newTestPrincipalAuthenticator("test", true), nil - }), + }, }, }, want: "compose: create pipeline: authkit: authorizer is required", @@ -126,12 +126,12 @@ func TestNewHTTPPreservesPrincipalAuthenticatorOrder(t *testing.T) { second := newTestPrincipalAuthenticator("second", true) kit, err := compose.NewHTTP(compose.HTTPOptions{ PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ - principalAuthenticatorSpecFunc(func() (authkit.PrincipalAuthenticator, error) { + func() (authkit.PrincipalAuthenticator, error) { return first, nil - }), - principalAuthenticatorSpecFunc(func() (authkit.PrincipalAuthenticator, error) { + }, + func() (authkit.PrincipalAuthenticator, error) { return second, nil - }), + }, }, Authorizer: testAuthorizer{}, }) @@ -152,9 +152,9 @@ func TestNewHTTPPreservesPrincipalAuthenticatorOrder(t *testing.T) { func TestNewHTTPReturnsUsableMiddleware(t *testing.T) { kit, err := compose.NewHTTP(compose.HTTPOptions{ PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ - principalAuthenticatorSpecFunc(func() (authkit.PrincipalAuthenticator, error) { + func() (authkit.PrincipalAuthenticator, error) { return newTestPrincipalAuthenticator("test", true), nil - }), + }, }, Authorizer: testAuthorizer{}, }) @@ -179,9 +179,9 @@ func TestNewHTTPReturnsUsableMiddleware(t *testing.T) { func TestNewHTTPAppliesMiddlewareOptions(t *testing.T) { kit, err := compose.NewHTTP(compose.HTTPOptions{ PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ - principalAuthenticatorSpecFunc(func() (authkit.PrincipalAuthenticator, error) { + func() (authkit.PrincipalAuthenticator, error) { return newTestPrincipalAuthenticator("test", false), nil - }), + }, }, Authorizer: testAuthorizer{}, MiddlewareOptions: []auth.Option{ @@ -237,12 +237,6 @@ func TestAccessJWTSpecWrapsConstructorErrors(t *testing.T) { } } -type principalAuthenticatorSpecFunc func() (authkit.PrincipalAuthenticator, error) - -func (f principalAuthenticatorSpecFunc) BuildPrincipalAuthenticator() (authkit.PrincipalAuthenticator, error) { - return f() -} - type testPrincipalAuthenticator struct { name string accepted bool From 5428cc943e1bbdc6a1cd3662294eb0c25f7edf71 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Wed, 27 May 2026 15:30:42 -0700 Subject: [PATCH 2/5] refactor(http): add godocs and security/defensive inline comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Godocs: - http/auth/options.go: options (private struct) and defaultOptions (private function), matching the PR #62 bar. - http/compose/http.go: buildPrincipalAuthenticators (private function). Inline comments — match the access/middleware/authenticator.go:76-88 style. Each comment names what the check defends against and why the error envelope is what it is, not what the code does: - http/auth/middleware.go::RequireAuthorization: annotate the extract == nil branch and the wrapped extractor-failure branch. Both report ErrInternal because they are caller-side construction bugs (nil extractor) or caller-supplied function bugs (extraction error), not authorization denials. ErrInternal maps to 500 in the default renderer; ErrUnauthorized maps to 403. Choosing one over the other affects the public API contract; the choice is worth spelling out at the call site. - http/compose/http.go::buildPrincipalAuthenticators: annotate the two nil branches (nil spec, nil-authenticator-from-non-erroring-spec) as contract violations that abort composition. No new comments where the code already reads obviously (defaultErrorRenderer status mapping, empty-struct context key, per-helper nil-request guards in http/facts/facts.go). Package godocs already cover those. --- http/auth/middleware.go | 4 ++++ http/auth/options.go | 2 ++ http/compose/http.go | 7 +++++++ 3 files changed, 13 insertions(+) diff --git a/http/auth/middleware.go b/http/auth/middleware.go index d8482b2..909e82e 100644 --- a/http/auth/middleware.go +++ b/http/auth/middleware.go @@ -108,6 +108,8 @@ func (m *Middleware) RequireAuthorization( } authenticatedReq := req.WithContext(contextWithAuthentication(req.Context(), authentication)) + // A nil extractor is a caller-construction bug, not an authorization decision; report + // as ErrInternal so the renderer maps it to 500 rather than 403. if extract == nil { m.renderer( w, @@ -119,6 +121,8 @@ func (m *Middleware) RequireAuthorization( } authorizationRequest, err := extract(authenticatedReq) + // Extractor failures are caller-side bugs (e.g. malformed routing data) rather than + // authorization denials; wrap as ErrInternal to keep the 401/403 envelope honest. if err != nil { m.renderer( w, diff --git a/http/auth/options.go b/http/auth/options.go index 59c20fc..75fb74c 100644 --- a/http/auth/options.go +++ b/http/auth/options.go @@ -1,5 +1,6 @@ package auth +// options holds the resolved configuration applied to a Middleware at construction time. type options struct { renderer ErrorRenderer } @@ -7,6 +8,7 @@ type options struct { // Option configures Middleware. type Option func(*options) +// defaultOptions returns the baseline configuration applied before user options are evaluated. func defaultOptions() options { return options{ renderer: defaultErrorRenderer, diff --git a/http/compose/http.go b/http/compose/http.go index 8b8a8c8..e658a16 100644 --- a/http/compose/http.go +++ b/http/compose/http.go @@ -58,11 +58,16 @@ func NewHTTP(opts HTTPOptions) (*HTTP, error) { }, nil } +// buildPrincipalAuthenticators invokes each spec in order and returns the materialised +// authenticators. A nil spec or a spec that builds a nil authenticator is treated as a caller +// contract violation and aborts composition with an error identifying the offending index. func buildPrincipalAuthenticators( specs []PrincipalAuthenticatorSpec, ) ([]authkit.PrincipalAuthenticator, error) { authenticators := make([]authkit.PrincipalAuthenticator, 0, len(specs)) for i, spec := range specs { + // Caller contract violation: HTTPOptions.PrincipalAuthenticators must not contain a nil + // spec, otherwise the composition order would be silently corrupted. if spec == nil { return nil, fmt.Errorf("compose: principal authenticator spec %d is required", i) } @@ -71,6 +76,8 @@ func buildPrincipalAuthenticators( if err != nil { return nil, fmt.Errorf("compose: build principal authenticator %d: %w", i, err) } + // A nil authenticator from a non-erroring spec is a builder bug, not a runtime decision; + // fail composition rather than letting a nil entry slip into the pipeline. if authenticator == nil { return nil, fmt.Errorf("compose: principal authenticator %d built nil authenticator", i) } From 8752eb236035bb78a923ba874db2643391d81f16 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Wed, 27 May 2026 15:32:34 -0700 Subject: [PATCH 3/5] test(http): split shared helpers into helpers_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mechanical lift only — no test logic changes. http/auth/helpers_test.go: newMiddleware, newTestPipeline, newTestPipelineWithAuthorizer, newTestPipelineWithPrincipalAuthenticator, newTestPipelineWithOptions, assertAuthenticationContext, testAuthentication, testPrincipal, testResource. http/compose/helpers_test.go: newTestPrincipalAuthenticator, requestWithBearer, newAccessJWTIssuerAndVerifier. The hand-rolled testPrincipalAuthenticator, testAuthorizer, and the allow/deny/failingPrincipalAuthenticator + allowAuthorizer constructors intentionally stay in middleware_test.go and http_test.go for this commit. They get replaced by mockery in the next commit and the lift would be churn. Matches the helpers_test.go precedent from PRs #61 (access/jwt) and #63 (exchange). --- http/auth/helpers_test.go | 96 ++++++++++++++++++++++++++++++++++++ http/auth/middleware_test.go | 84 ------------------------------- http/compose/helpers_test.go | 31 ++++++++++++ http/compose/http_test.go | 22 --------- 4 files changed, 127 insertions(+), 106 deletions(-) create mode 100644 http/auth/helpers_test.go create mode 100644 http/compose/helpers_test.go diff --git a/http/auth/helpers_test.go b/http/auth/helpers_test.go new file mode 100644 index 0000000..716c592 --- /dev/null +++ b/http/auth/helpers_test.go @@ -0,0 +1,96 @@ +package auth_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/meigma/authkit" + "github.com/meigma/authkit/http/auth" +) + +func newMiddleware(t *testing.T, pipeline *authkit.Pipeline) *auth.Middleware { + t.Helper() + + middleware, err := auth.NewMiddleware(pipeline) + require.NoError(t, err) + + return middleware +} + +func newTestPipeline(t *testing.T, decision authkit.Decision) *authkit.Pipeline { + t.Helper() + + return newTestPipelineWithAuthorizer(t, testAuthorizer{ + can: func(context.Context, authkit.AuthorizationCheck) (authkit.Decision, error) { + return decision, nil + }, + }) +} + +func newTestPipelineWithAuthorizer(t *testing.T, authorizer authkit.Authorizer) *authkit.Pipeline { + t.Helper() + + return newTestPipelineWithOptions(t, authkit.PipelineOptions{ + PrincipalAuthenticators: []authkit.PrincipalAuthenticator{allowPrincipalAuthenticator()}, + Authorizer: authorizer, + }) +} + +func newTestPipelineWithPrincipalAuthenticator( + t *testing.T, + authenticator authkit.PrincipalAuthenticator, + authorizer authkit.Authorizer, +) *authkit.Pipeline { + t.Helper() + + return newTestPipelineWithOptions(t, authkit.PipelineOptions{ + PrincipalAuthenticators: []authkit.PrincipalAuthenticator{authenticator}, + Authorizer: authorizer, + }) +} + +func newTestPipelineWithOptions(t *testing.T, opts authkit.PipelineOptions) *authkit.Pipeline { + t.Helper() + + pipeline, err := authkit.NewPipeline(opts) + require.NoError(t, err) + + return pipeline +} + +func assertAuthenticationContext(ctx context.Context, t *testing.T) { + t.Helper() + + authentication, ok := auth.AuthenticationFromContext(ctx) + require.True(t, ok) + assert.Equal(t, testAuthentication(), authentication) + + principal, ok := auth.PrincipalFromContext(ctx) + require.True(t, ok) + assert.Equal(t, testPrincipal(), principal) +} + +func testAuthentication() authkit.Authentication { + return authkit.Authentication{ + AuthenticatorName: "test", + Principal: testPrincipal(), + } +} + +func testPrincipal() authkit.Principal { + return authkit.Principal{ + ID: "principal_1", + Kind: authkit.PrincipalKindUser, + DisplayName: "Ada Lovelace", + } +} + +func testResource(id string) authkit.Resource { + return authkit.Resource{ + Type: "note", + ID: id, + } +} diff --git a/http/auth/middleware_test.go b/http/auth/middleware_test.go index 189facc..14e1c76 100644 --- a/http/auth/middleware_test.go +++ b/http/auth/middleware_test.go @@ -274,90 +274,6 @@ func TestMiddlewareUsesCustomRenderer(t *testing.T) { assert.Equal(t, "custom\n", recorder.Body.String()) } -func newMiddleware(t *testing.T, pipeline *authkit.Pipeline) *auth.Middleware { - t.Helper() - - middleware, err := auth.NewMiddleware(pipeline) - require.NoError(t, err) - - return middleware -} - -func newTestPipeline(t *testing.T, decision authkit.Decision) *authkit.Pipeline { - t.Helper() - - return newTestPipelineWithAuthorizer(t, testAuthorizer{ - can: func(context.Context, authkit.AuthorizationCheck) (authkit.Decision, error) { - return decision, nil - }, - }) -} - -func newTestPipelineWithAuthorizer(t *testing.T, authorizer authkit.Authorizer) *authkit.Pipeline { - t.Helper() - - return newTestPipelineWithOptions(t, authkit.PipelineOptions{ - PrincipalAuthenticators: []authkit.PrincipalAuthenticator{allowPrincipalAuthenticator()}, - Authorizer: authorizer, - }) -} - -func newTestPipelineWithPrincipalAuthenticator( - t *testing.T, - authenticator authkit.PrincipalAuthenticator, - authorizer authkit.Authorizer, -) *authkit.Pipeline { - t.Helper() - - return newTestPipelineWithOptions(t, authkit.PipelineOptions{ - PrincipalAuthenticators: []authkit.PrincipalAuthenticator{authenticator}, - Authorizer: authorizer, - }) -} - -func newTestPipelineWithOptions(t *testing.T, opts authkit.PipelineOptions) *authkit.Pipeline { - t.Helper() - - pipeline, err := authkit.NewPipeline(opts) - require.NoError(t, err) - - return pipeline -} - -func assertAuthenticationContext(ctx context.Context, t *testing.T) { - t.Helper() - - authentication, ok := auth.AuthenticationFromContext(ctx) - require.True(t, ok) - assert.Equal(t, testAuthentication(), authentication) - - principal, ok := auth.PrincipalFromContext(ctx) - require.True(t, ok) - assert.Equal(t, testPrincipal(), principal) -} - -func testAuthentication() authkit.Authentication { - return authkit.Authentication{ - AuthenticatorName: "test", - Principal: testPrincipal(), - } -} - -func testPrincipal() authkit.Principal { - return authkit.Principal{ - ID: "principal_1", - Kind: authkit.PrincipalKindUser, - DisplayName: "Ada Lovelace", - } -} - -func testResource(id string) authkit.Resource { - return authkit.Resource{ - Type: "note", - ID: id, - } -} - type testPrincipalAuthenticator struct { name string authenticate func(context.Context, *http.Request) (*authkit.PrincipalAuthentication, error) diff --git a/http/compose/helpers_test.go b/http/compose/helpers_test.go new file mode 100644 index 0000000..af9c328 --- /dev/null +++ b/http/compose/helpers_test.go @@ -0,0 +1,31 @@ +package compose_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/meigma/authkit/access/jwt" + "github.com/meigma/authkit/internal/authtest" +) + +func newTestPrincipalAuthenticator(name string, accepted bool) *testPrincipalAuthenticator { + return &testPrincipalAuthenticator{ + name: name, + accepted: accepted, + principalID: "principal_" + name, + } +} + +func requestWithBearer(token string) *http.Request { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("Authorization", "Bearer "+token) + + return req +} + +func newAccessJWTIssuerAndVerifier(t *testing.T) (*jwt.Issuer, *jwt.Verifier) { + t.Helper() + + return authtest.NewAccessJWTIssuerAndVerifier(t) +} diff --git a/http/compose/http_test.go b/http/compose/http_test.go index 1b976ac..dd5220e 100644 --- a/http/compose/http_test.go +++ b/http/compose/http_test.go @@ -14,7 +14,6 @@ import ( "github.com/meigma/authkit/access/jwt" "github.com/meigma/authkit/http/auth" "github.com/meigma/authkit/http/compose" - "github.com/meigma/authkit/internal/authtest" "github.com/meigma/authkit/store/memory" ) @@ -244,14 +243,6 @@ type testPrincipalAuthenticator struct { principalID string } -func newTestPrincipalAuthenticator(name string, accepted bool) *testPrincipalAuthenticator { - return &testPrincipalAuthenticator{ - name: name, - accepted: accepted, - principalID: "principal_" + name, - } -} - func (a *testPrincipalAuthenticator) Name() string { return a.name } @@ -278,16 +269,3 @@ type testAuthorizer struct{} func (a testAuthorizer) Can(_ context.Context, _ authkit.AuthorizationCheck) (authkit.Decision, error) { return authkit.Decision{Allowed: true}, nil } - -func requestWithBearer(token string) *http.Request { - req := httptest.NewRequest(http.MethodGet, "/", nil) - req.Header.Set("Authorization", "Bearer "+token) - - return req -} - -func newAccessJWTIssuerAndVerifier(t *testing.T) (*jwt.Issuer, *jwt.Verifier) { - t.Helper() - - return authtest.NewAccessJWTIssuerAndVerifier(t) -} From fd350cc35e8ccb0b718a43f6dad82ea0ca91322a Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Wed, 27 May 2026 15:38:31 -0700 Subject: [PATCH 4/5] test(http): migrate root-port fakes to mockery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add authkit.PrincipalAuthenticator and authkit.Authorizer to .mockery.yaml. The generated mocks land at mocks/authkit/principal_authenticator.go and mocks/authkit/authorizer.go in package authkitmocks. Both are root-package ports that http/auth and http/compose tests faked by hand — twice over. http/auth/middleware_test.go and http/auth/helpers_test.go: - Delete hand-rolled testPrincipalAuthenticator and testAuthorizer types. - Delete allow/deny/failingPrincipalAuthenticator and allowAuthorizer constructor helpers. - Replace per-test with authkitmocks.PrincipalAuthenticator and authkitmocks.Authorizer; keep typed helpers (newAllowPrincipalAuthenticator, newDenyPrincipalAuthenticator, newFailingPrincipalAuthenticator) in helpers_test.go so the Name()/AuthenticatePrincipal expectation patterns stay in one place. - Tests whose short-circuit path never reaches the authorizer now pass an unused authkitmocks.NewAuthorizer(t) (no expectations set); a stray Can() call would panic the test. http/compose/http_test.go and http/compose/helpers_test.go: - Delete hand-rolled testPrincipalAuthenticator and testAuthorizer. - Delete newTestPrincipalAuthenticator factory. - Migrate the five tests that drove behaviour through the hand-rolled fakes to authkitmocks; the two AccessJWT integration tests keep their real-implementation chain and gain an unused authkitmocks.NewAuthorizer for the slot the pipeline does not exercise. Matches the access/middleware (PR #61), authz/role (PR #62), and exchange (PR #63) migration shape. --- .mockery.yaml | 14 ++ http/auth/helpers_test.go | 72 +++++++--- http/auth/middleware_test.go | 162 ++++++---------------- http/compose/helpers_test.go | 8 -- http/compose/http_test.go | 163 +++++++++++++---------- mocks/authkit/authorizer.go | 105 +++++++++++++++ mocks/authkit/principal_authenticator.go | 152 +++++++++++++++++++++ 7 files changed, 456 insertions(+), 220 deletions(-) create mode 100644 mocks/authkit/authorizer.go create mode 100644 mocks/authkit/principal_authenticator.go diff --git a/.mockery.yaml b/.mockery.yaml index b2b6a5c..1202bf2 100644 --- a/.mockery.yaml +++ b/.mockery.yaml @@ -22,3 +22,17 @@ packages: pkgname: authkitmocks dir: mocks/authkit filename: principal_resolver.go + PrincipalAuthenticator: + config: + template: testify + structname: PrincipalAuthenticator + pkgname: authkitmocks + dir: mocks/authkit + filename: principal_authenticator.go + Authorizer: + config: + template: testify + structname: Authorizer + pkgname: authkitmocks + dir: mocks/authkit + filename: authorizer.go diff --git a/http/auth/helpers_test.go b/http/auth/helpers_test.go index 716c592..b4cb1db 100644 --- a/http/auth/helpers_test.go +++ b/http/auth/helpers_test.go @@ -2,15 +2,20 @@ package auth_test import ( "context" + "fmt" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/meigma/authkit" "github.com/meigma/authkit/http/auth" + authkitmocks "github.com/meigma/authkit/mocks/authkit" ) +const testAuthenticatorName = "test" + func newMiddleware(t *testing.T, pipeline *authkit.Pipeline) *auth.Middleware { t.Helper() @@ -20,25 +25,16 @@ func newMiddleware(t *testing.T, pipeline *authkit.Pipeline) *auth.Middleware { return middleware } -func newTestPipeline(t *testing.T, decision authkit.Decision) *authkit.Pipeline { - t.Helper() - - return newTestPipelineWithAuthorizer(t, testAuthorizer{ - can: func(context.Context, authkit.AuthorizationCheck) (authkit.Decision, error) { - return decision, nil - }, - }) -} - +// newTestPipelineWithAuthorizer composes a pipeline with an allow-everything principal +// authenticator and the supplied authorizer. func newTestPipelineWithAuthorizer(t *testing.T, authorizer authkit.Authorizer) *authkit.Pipeline { t.Helper() - return newTestPipelineWithOptions(t, authkit.PipelineOptions{ - PrincipalAuthenticators: []authkit.PrincipalAuthenticator{allowPrincipalAuthenticator()}, - Authorizer: authorizer, - }) + return newTestPipelineWithPrincipalAuthenticator(t, newAllowPrincipalAuthenticator(t), authorizer) } +// newTestPipelineWithPrincipalAuthenticator composes a pipeline from the supplied authenticator +// and authorizer. func newTestPipelineWithPrincipalAuthenticator( t *testing.T, authenticator authkit.PrincipalAuthenticator, @@ -46,19 +42,55 @@ func newTestPipelineWithPrincipalAuthenticator( ) *authkit.Pipeline { t.Helper() - return newTestPipelineWithOptions(t, authkit.PipelineOptions{ + pipeline, err := authkit.NewPipeline(authkit.PipelineOptions{ PrincipalAuthenticators: []authkit.PrincipalAuthenticator{authenticator}, Authorizer: authorizer, }) + require.NoError(t, err) + + return pipeline } -func newTestPipelineWithOptions(t *testing.T, opts authkit.PipelineOptions) *authkit.Pipeline { +// newAllowPrincipalAuthenticator returns a mock authenticator that accepts every request and +// returns testPrincipal(). +func newAllowPrincipalAuthenticator(t *testing.T) *authkitmocks.PrincipalAuthenticator { t.Helper() - pipeline, err := authkit.NewPipeline(opts) - require.NoError(t, err) + authenticator := authkitmocks.NewPrincipalAuthenticator(t) + authenticator.EXPECT().Name().Return(testAuthenticatorName) + authenticator.EXPECT(). + AuthenticatePrincipal(mock.Anything, mock.Anything). + Return(&authkit.PrincipalAuthentication{Principal: testPrincipal()}, nil) - return pipeline + return authenticator +} + +// newDenyPrincipalAuthenticator returns a mock authenticator that wraps ErrUnauthenticated. The +// pipeline does not call Name() on the ErrUnauthenticated path, so Name() is not expected. +func newDenyPrincipalAuthenticator(t *testing.T) *authkitmocks.PrincipalAuthenticator { + t.Helper() + + authenticator := authkitmocks.NewPrincipalAuthenticator(t) + authenticator.EXPECT(). + AuthenticatePrincipal(mock.Anything, mock.Anything). + Return(nil, fmt.Errorf("%w: credential missing", authkit.ErrUnauthenticated)) + + return authenticator +} + +// newFailingPrincipalAuthenticator returns a mock authenticator whose AuthenticatePrincipal call +// returns err. The pipeline wraps the failure with Name() in the ErrInternal message, so Name() +// is expected to be called. +func newFailingPrincipalAuthenticator(t *testing.T, err error) *authkitmocks.PrincipalAuthenticator { + t.Helper() + + authenticator := authkitmocks.NewPrincipalAuthenticator(t) + authenticator.EXPECT().Name().Return(testAuthenticatorName) + authenticator.EXPECT(). + AuthenticatePrincipal(mock.Anything, mock.Anything). + Return(nil, err) + + return authenticator } func assertAuthenticationContext(ctx context.Context, t *testing.T) { @@ -75,7 +107,7 @@ func assertAuthenticationContext(ctx context.Context, t *testing.T) { func testAuthentication() authkit.Authentication { return authkit.Authentication{ - AuthenticatorName: "test", + AuthenticatorName: testAuthenticatorName, Principal: testPrincipal(), } } diff --git a/http/auth/middleware_test.go b/http/auth/middleware_test.go index 14e1c76..0f84949 100644 --- a/http/auth/middleware_test.go +++ b/http/auth/middleware_test.go @@ -3,16 +3,17 @@ package auth_test import ( "context" "errors" - "fmt" "net/http" "net/http/httptest" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/meigma/authkit" "github.com/meigma/authkit/http/auth" + authkitmocks "github.com/meigma/authkit/mocks/authkit" ) func TestNewMiddlewareValidatesPipeline(t *testing.T) { @@ -33,7 +34,7 @@ func TestContextHelpersReturnFalseWhenMissing(t *testing.T) { } func TestMiddlewareAuthenticatePopulatesContext(t *testing.T) { - pipeline := newTestPipeline(t, authkit.Decision{Allowed: true}) + pipeline := newTestPipelineWithPrincipalAuthenticator(t, newAllowPrincipalAuthenticator(t), authkitmocks.NewAuthorizer(t)) middleware := newMiddleware(t, pipeline) handlerCalled := false handler := middleware.Authenticate(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { @@ -51,7 +52,8 @@ func TestMiddlewareAuthenticatePopulatesContext(t *testing.T) { } func TestMiddlewareAuthenticateRendersUnauthenticated(t *testing.T) { - pipeline := newTestPipelineWithPrincipalAuthenticator(t, denyPrincipalAuthenticator("test"), allowAuthorizer()) + authenticator := newDenyPrincipalAuthenticator(t) + pipeline := newTestPipelineWithPrincipalAuthenticator(t, authenticator, authkitmocks.NewAuthorizer(t)) middleware := newMiddleware(t, pipeline) handler := middleware.Authenticate(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNoContent) @@ -65,16 +67,15 @@ func TestMiddlewareAuthenticateRendersUnauthenticated(t *testing.T) { } func TestMiddlewareRequireAllowsRequest(t *testing.T) { - authorizer := testAuthorizer{ - can: func(_ context.Context, check authkit.AuthorizationCheck) (authkit.Decision, error) { - assert.Equal(t, testPrincipal(), check.Principal) - assert.Equal(t, "note:update", check.Action) - assert.Equal(t, testResource("note-1"), check.Resource) - assert.Empty(t, check.Facts) - - return authkit.Decision{Allowed: true}, nil - }, - } + authorizer := authkitmocks.NewAuthorizer(t) + authorizer.EXPECT(). + Can(mock.Anything, mock.MatchedBy(func(check authkit.AuthorizationCheck) bool { + return assert.Equal(t, testPrincipal(), check.Principal) && + assert.Equal(t, "note:update", check.Action) && + assert.Equal(t, testResource("note-1"), check.Resource) && + assert.Empty(t, check.Facts) + })). + Return(authkit.Decision{Allowed: true}, nil) pipeline := newTestPipelineWithAuthorizer(t, authorizer) middleware := newMiddleware(t, pipeline) handlerCalled := false @@ -95,10 +96,11 @@ func TestMiddlewareRequireAllowsRequest(t *testing.T) { } func TestMiddlewareRequireRendersDeniedDecision(t *testing.T) { - pipeline := newTestPipeline(t, authkit.Decision{ - Allowed: false, - Reason: "policy denied", - }) + authorizer := authkitmocks.NewAuthorizer(t) + authorizer.EXPECT(). + Can(mock.Anything, mock.Anything). + Return(authkit.Decision{Allowed: false, Reason: "policy denied"}, nil) + pipeline := newTestPipelineWithAuthorizer(t, authorizer) middleware := newMiddleware(t, pipeline) handler := middleware.Require("note:update", testResource("note-1"))( http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -114,15 +116,14 @@ func TestMiddlewareRequireRendersDeniedDecision(t *testing.T) { } func TestMiddlewareRequireFuncExtractsResource(t *testing.T) { - authorizer := testAuthorizer{ - can: func(_ context.Context, check authkit.AuthorizationCheck) (authkit.Decision, error) { - assert.Equal(t, "note:read", check.Action) - assert.Equal(t, testResource("42"), check.Resource) - assert.Empty(t, check.Facts) - - return authkit.Decision{Allowed: true}, nil - }, - } + authorizer := authkitmocks.NewAuthorizer(t) + authorizer.EXPECT(). + Can(mock.Anything, mock.MatchedBy(func(check authkit.AuthorizationCheck) bool { + return assert.Equal(t, "note:read", check.Action) && + assert.Equal(t, testResource("42"), check.Resource) && + assert.Empty(t, check.Facts) + })). + Return(authkit.Decision{Allowed: true}, nil) pipeline := newTestPipelineWithAuthorizer(t, authorizer) middleware := newMiddleware(t, pipeline) handler := middleware.RequireFunc("note:read", func(req *http.Request) (authkit.Resource, error) { @@ -140,33 +141,18 @@ func TestMiddlewareRequireFuncExtractsResource(t *testing.T) { } func TestMiddlewareRequireAuthorizationExtractsFacts(t *testing.T) { - authenticatorCalls := 0 - authorizer := testAuthorizer{ - can: func(_ context.Context, check authkit.AuthorizationCheck) (authkit.Decision, error) { - assert.Equal(t, "note:read", check.Action) - assert.Equal(t, testResource("42"), check.Resource) - assert.Equal(t, authkit.Facts{ - "request_method": "GET", - "tenant_id": "tenant-1", - }, check.Facts) - - return authkit.Decision{Allowed: true}, nil - }, - } - pipeline := newTestPipelineWithPrincipalAuthenticator( - t, - testPrincipalAuthenticator{ - name: "test", - authenticate: func(context.Context, *http.Request) (*authkit.PrincipalAuthentication, error) { - authenticatorCalls++ - - return &authkit.PrincipalAuthentication{ - Principal: testPrincipal(), - }, nil - }, - }, - authorizer, - ) + authorizer := authkitmocks.NewAuthorizer(t) + authorizer.EXPECT(). + Can(mock.Anything, mock.MatchedBy(func(check authkit.AuthorizationCheck) bool { + return assert.Equal(t, "note:read", check.Action) && + assert.Equal(t, testResource("42"), check.Resource) && + assert.Equal(t, authkit.Facts{ + "request_method": "GET", + "tenant_id": "tenant-1", + }, check.Facts) + })). + Return(authkit.Decision{Allowed: true}, nil) + pipeline := newTestPipelineWithPrincipalAuthenticator(t, newAllowPrincipalAuthenticator(t), authorizer) middleware := newMiddleware(t, pipeline) handler := middleware.RequireAuthorization(func(req *http.Request) (authkit.AuthorizationRequest, error) { assertAuthenticationContext(req.Context(), t) @@ -191,12 +177,11 @@ func TestMiddlewareRequireAuthorizationExtractsFacts(t *testing.T) { mux.ServeHTTP(recorder, req) assert.Equal(t, http.StatusOK, recorder.Code) - assert.Equal(t, 1, authenticatorCalls) } func TestMiddlewareRequireAuthorizationDoesNotExtractWhenUnauthenticated(t *testing.T) { extractorCalls := 0 - pipeline := newTestPipelineWithPrincipalAuthenticator(t, denyPrincipalAuthenticator("test"), allowAuthorizer()) + pipeline := newTestPipelineWithPrincipalAuthenticator(t, newDenyPrincipalAuthenticator(t), authkitmocks.NewAuthorizer(t)) middleware := newMiddleware(t, pipeline) handler := middleware.RequireAuthorization(func(*http.Request) (authkit.AuthorizationRequest, error) { extractorCalls++ @@ -218,7 +203,7 @@ func TestMiddlewareRequireAuthorizationDoesNotExtractWhenUnauthenticated(t *test func TestMiddlewareRequireFuncRendersExtractorFailureAsInternal(t *testing.T) { extractErr := errors.New("missing resource") - pipeline := newTestPipeline(t, authkit.Decision{Allowed: true}) + pipeline := newTestPipelineWithPrincipalAuthenticator(t, newAllowPrincipalAuthenticator(t), authkitmocks.NewAuthorizer(t)) middleware := newMiddleware(t, pipeline) handler := middleware.RequireFunc("note:read", func(*http.Request) (authkit.Resource, error) { return authkit.Resource{}, extractErr @@ -234,11 +219,8 @@ func TestMiddlewareRequireFuncRendersExtractorFailureAsInternal(t *testing.T) { } func TestMiddlewareRendersInternalFailures(t *testing.T) { - pipeline := newTestPipelineWithPrincipalAuthenticator( - t, - failingPrincipalAuthenticator("test", errors.New("provider failed")), - allowAuthorizer(), - ) + authenticator := newFailingPrincipalAuthenticator(t, errors.New("provider failed")) + pipeline := newTestPipelineWithPrincipalAuthenticator(t, authenticator, authkitmocks.NewAuthorizer(t)) middleware := newMiddleware(t, pipeline) handler := middleware.Authenticate(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) @@ -253,7 +235,7 @@ func TestMiddlewareRendersInternalFailures(t *testing.T) { func TestMiddlewareUsesCustomRenderer(t *testing.T) { var renderedErr error - pipeline := newTestPipelineWithPrincipalAuthenticator(t, denyPrincipalAuthenticator("test"), allowAuthorizer()) + pipeline := newTestPipelineWithPrincipalAuthenticator(t, newDenyPrincipalAuthenticator(t), authkitmocks.NewAuthorizer(t)) middleware, err := auth.NewMiddleware( pipeline, auth.WithErrorRenderer(func(w http.ResponseWriter, _ *http.Request, err error) { @@ -273,59 +255,3 @@ func TestMiddlewareUsesCustomRenderer(t *testing.T) { assert.Equal(t, http.StatusTeapot, recorder.Code) assert.Equal(t, "custom\n", recorder.Body.String()) } - -type testPrincipalAuthenticator struct { - name string - authenticate func(context.Context, *http.Request) (*authkit.PrincipalAuthentication, error) -} - -func (a testPrincipalAuthenticator) Name() string { - return a.name -} - -func (a testPrincipalAuthenticator) AuthenticatePrincipal( - ctx context.Context, - req *http.Request, -) (*authkit.PrincipalAuthentication, error) { - return a.authenticate(ctx, req) -} - -type testAuthorizer struct { - can func(context.Context, authkit.AuthorizationCheck) (authkit.Decision, error) -} - -func (a testAuthorizer) Can(ctx context.Context, check authkit.AuthorizationCheck) (authkit.Decision, error) { - return a.can(ctx, check) -} - -func allowPrincipalAuthenticator() testPrincipalAuthenticator { - return testPrincipalAuthenticator{ - name: "test", - authenticate: func(context.Context, *http.Request) (*authkit.PrincipalAuthentication, error) { - return &authkit.PrincipalAuthentication{ - Principal: testPrincipal(), - }, nil - }, - } -} - -func denyPrincipalAuthenticator(name string) testPrincipalAuthenticator { - return failingPrincipalAuthenticator(name, fmt.Errorf("%w: credential missing", authkit.ErrUnauthenticated)) -} - -func failingPrincipalAuthenticator(name string, err error) testPrincipalAuthenticator { - return testPrincipalAuthenticator{ - name: name, - authenticate: func(context.Context, *http.Request) (*authkit.PrincipalAuthentication, error) { - return nil, err - }, - } -} - -func allowAuthorizer() testAuthorizer { - return testAuthorizer{ - can: func(context.Context, authkit.AuthorizationCheck) (authkit.Decision, error) { - return authkit.Decision{Allowed: true}, nil - }, - } -} diff --git a/http/compose/helpers_test.go b/http/compose/helpers_test.go index af9c328..36fe381 100644 --- a/http/compose/helpers_test.go +++ b/http/compose/helpers_test.go @@ -9,14 +9,6 @@ import ( "github.com/meigma/authkit/internal/authtest" ) -func newTestPrincipalAuthenticator(name string, accepted bool) *testPrincipalAuthenticator { - return &testPrincipalAuthenticator{ - name: name, - accepted: accepted, - principalID: "principal_" + name, - } -} - func requestWithBearer(token string) *http.Request { req := httptest.NewRequest(http.MethodGet, "/", nil) req.Header.Set("Authorization", "Bearer "+token) diff --git a/http/compose/http_test.go b/http/compose/http_test.go index dd5220e..42edf33 100644 --- a/http/compose/http_test.go +++ b/http/compose/http_test.go @@ -3,78 +3,97 @@ package compose_test import ( "context" "errors" + "fmt" "net/http" "net/http/httptest" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/meigma/authkit" "github.com/meigma/authkit/access/jwt" "github.com/meigma/authkit/http/auth" "github.com/meigma/authkit/http/compose" + authkitmocks "github.com/meigma/authkit/mocks/authkit" "github.com/meigma/authkit/store/memory" ) func TestNewHTTPValidatesInputs(t *testing.T) { boom := errors.New("boom") tests := []struct { - name string - options compose.HTTPOptions - want string - wantIs error + name string + setupOpts func(t *testing.T) compose.HTTPOptions + want string + wantIs error }{ { name: "requires at least one principal authenticator", - options: compose.HTTPOptions{ - Authorizer: testAuthorizer{}, + setupOpts: func(t *testing.T) compose.HTTPOptions { + t.Helper() + return compose.HTTPOptions{ + Authorizer: authkitmocks.NewAuthorizer(t), + } }, want: "compose: at least one principal authenticator is required", }, { name: "rejects nil principal authenticator spec", - options: compose.HTTPOptions{ - PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{nil}, - Authorizer: testAuthorizer{}, + setupOpts: func(t *testing.T) compose.HTTPOptions { + t.Helper() + return compose.HTTPOptions{ + PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{nil}, + Authorizer: authkitmocks.NewAuthorizer(t), + } }, want: "compose: principal authenticator spec 0 is required", }, { name: "wraps principal authenticator build errors", - options: compose.HTTPOptions{ - PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ - func() (authkit.PrincipalAuthenticator, error) { - return nil, boom + setupOpts: func(t *testing.T) compose.HTTPOptions { + t.Helper() + return compose.HTTPOptions{ + PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ + func() (authkit.PrincipalAuthenticator, error) { + return nil, boom + }, }, - }, - Authorizer: testAuthorizer{}, + Authorizer: authkitmocks.NewAuthorizer(t), + } }, want: "compose: build principal authenticator 0: boom", wantIs: boom, }, { name: "rejects nil built principal authenticator", - options: compose.HTTPOptions{ - PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ - func() (authkit.PrincipalAuthenticator, error) { - var authenticator authkit.PrincipalAuthenticator - - return authenticator, nil + setupOpts: func(t *testing.T) compose.HTTPOptions { + t.Helper() + return compose.HTTPOptions{ + PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ + func() (authkit.PrincipalAuthenticator, error) { + var authenticator authkit.PrincipalAuthenticator + + return authenticator, nil + }, }, - }, - Authorizer: testAuthorizer{}, + Authorizer: authkitmocks.NewAuthorizer(t), + } }, want: "compose: principal authenticator 0 built nil authenticator", }, { name: "wraps missing authorizer error", - options: compose.HTTPOptions{ - PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ - func() (authkit.PrincipalAuthenticator, error) { - return newTestPrincipalAuthenticator("test", true), nil + setupOpts: func(t *testing.T) compose.HTTPOptions { + t.Helper() + authenticator := authkitmocks.NewPrincipalAuthenticator(t) + return compose.HTTPOptions{ + PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ + func() (authkit.PrincipalAuthenticator, error) { + return authenticator, nil + }, }, - }, + } }, want: "compose: create pipeline: authkit: authorizer is required", }, @@ -82,7 +101,7 @@ func TestNewHTTPValidatesInputs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := compose.NewHTTP(tt.options) + _, err := compose.NewHTTP(tt.setupOpts(t)) require.Error(t, err) assert.Contains(t, err.Error(), tt.want) @@ -110,7 +129,7 @@ func TestNewHTTPWithAccessJWTAuthenticatesPrincipals(t *testing.T) { PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ compose.AccessJWT(verifier, store), }, - Authorizer: testAuthorizer{}, + Authorizer: authkitmocks.NewAuthorizer(t), }) require.NoError(t, err) @@ -121,8 +140,24 @@ func TestNewHTTPWithAccessJWTAuthenticatesPrincipals(t *testing.T) { } func TestNewHTTPPreservesPrincipalAuthenticatorOrder(t *testing.T) { - first := newTestPrincipalAuthenticator("first", false) - second := newTestPrincipalAuthenticator("second", true) + first := authkitmocks.NewPrincipalAuthenticator(t) + first.EXPECT(). + AuthenticatePrincipal(mock.Anything, mock.Anything). + Return(nil, fmt.Errorf("%w: first denies", authkit.ErrUnauthenticated)). + Once() + + second := authkitmocks.NewPrincipalAuthenticator(t) + second.EXPECT().Name().Return("second") + second.EXPECT(). + AuthenticatePrincipal(mock.Anything, mock.Anything). + Return(&authkit.PrincipalAuthentication{ + Principal: authkit.Principal{ + ID: "principal_second", + Kind: authkit.PrincipalKindService, + }, + }, nil). + Once() + kit, err := compose.NewHTTP(compose.HTTPOptions{ PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ func() (authkit.PrincipalAuthenticator, error) { @@ -132,7 +167,7 @@ func TestNewHTTPPreservesPrincipalAuthenticatorOrder(t *testing.T) { return second, nil }, }, - Authorizer: testAuthorizer{}, + Authorizer: authkitmocks.NewAuthorizer(t), }) require.NoError(t, err) @@ -144,18 +179,27 @@ func TestNewHTTPPreservesPrincipalAuthenticatorOrder(t *testing.T) { require.NoError(t, err) assert.Equal(t, "second", authentication.AuthenticatorName) assert.Equal(t, "principal_second", authentication.Principal.ID) - assert.Equal(t, 1, first.calls) - assert.Equal(t, 1, second.calls) } func TestNewHTTPReturnsUsableMiddleware(t *testing.T) { + authenticator := authkitmocks.NewPrincipalAuthenticator(t) + authenticator.EXPECT().Name().Return("test") + authenticator.EXPECT(). + AuthenticatePrincipal(mock.Anything, mock.Anything). + Return(&authkit.PrincipalAuthentication{ + Principal: authkit.Principal{ + ID: "principal_test", + Kind: authkit.PrincipalKindService, + }, + }, nil) + kit, err := compose.NewHTTP(compose.HTTPOptions{ PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ func() (authkit.PrincipalAuthenticator, error) { - return newTestPrincipalAuthenticator("test", true), nil + return authenticator, nil }, }, - Authorizer: testAuthorizer{}, + Authorizer: authkitmocks.NewAuthorizer(t), }) require.NoError(t, err) @@ -176,13 +220,18 @@ func TestNewHTTPReturnsUsableMiddleware(t *testing.T) { } func TestNewHTTPAppliesMiddlewareOptions(t *testing.T) { + authenticator := authkitmocks.NewPrincipalAuthenticator(t) + authenticator.EXPECT(). + AuthenticatePrincipal(mock.Anything, mock.Anything). + Return(nil, fmt.Errorf("%w: deny", authkit.ErrUnauthenticated)) + kit, err := compose.NewHTTP(compose.HTTPOptions{ PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{ func() (authkit.PrincipalAuthenticator, error) { - return newTestPrincipalAuthenticator("test", false), nil + return authenticator, nil }, }, - Authorizer: testAuthorizer{}, + Authorizer: authkitmocks.NewAuthorizer(t), MiddlewareOptions: []auth.Option{ auth.WithErrorRenderer(func(w http.ResponseWriter, _ *http.Request, _ error) { http.Error(w, "custom auth error", http.StatusTeapot) @@ -227,7 +276,7 @@ func TestAccessJWTSpecWrapsConstructorErrors(t *testing.T) { t.Run(tt.name, func(t *testing.T) { _, err := compose.NewHTTP(compose.HTTPOptions{ PrincipalAuthenticators: []compose.PrincipalAuthenticatorSpec{tt.spec}, - Authorizer: testAuthorizer{}, + Authorizer: authkitmocks.NewAuthorizer(t), }) require.Error(t, err) @@ -235,37 +284,3 @@ func TestAccessJWTSpecWrapsConstructorErrors(t *testing.T) { }) } } - -type testPrincipalAuthenticator struct { - name string - accepted bool - calls int - principalID string -} - -func (a *testPrincipalAuthenticator) Name() string { - return a.name -} - -func (a *testPrincipalAuthenticator) AuthenticatePrincipal( - _ context.Context, - _ *http.Request, -) (*authkit.PrincipalAuthentication, error) { - a.calls++ - if !a.accepted { - return nil, authkit.ErrUnauthenticated - } - - return &authkit.PrincipalAuthentication{ - Principal: authkit.Principal{ - ID: a.principalID, - Kind: authkit.PrincipalKindService, - }, - }, nil -} - -type testAuthorizer struct{} - -func (a testAuthorizer) Can(_ context.Context, _ authkit.AuthorizationCheck) (authkit.Decision, error) { - return authkit.Decision{Allowed: true}, nil -} diff --git a/mocks/authkit/authorizer.go b/mocks/authkit/authorizer.go new file mode 100644 index 0000000..7f1cc01 --- /dev/null +++ b/mocks/authkit/authorizer.go @@ -0,0 +1,105 @@ +// Code generated by mockery; DO NOT EDIT. +// github.com/vektra/mockery +// template: testify + +package authkitmocks + +import ( + "context" + + "github.com/meigma/authkit" + mock "github.com/stretchr/testify/mock" +) + +// NewAuthorizer creates a new instance of Authorizer. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewAuthorizer(t interface { + mock.TestingT + Cleanup(func()) +}) *Authorizer { + mock := &Authorizer{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} + +// Authorizer is an autogenerated mock type for the Authorizer type +type Authorizer struct { + mock.Mock +} + +type Authorizer_Expecter struct { + mock *mock.Mock +} + +func (_m *Authorizer) EXPECT() *Authorizer_Expecter { + return &Authorizer_Expecter{mock: &_m.Mock} +} + +// Can provides a mock function for the type Authorizer +func (_mock *Authorizer) Can(ctx context.Context, check authkit.AuthorizationCheck) (authkit.Decision, error) { + ret := _mock.Called(ctx, check) + + if len(ret) == 0 { + panic("no return value specified for Can") + } + + var r0 authkit.Decision + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, authkit.AuthorizationCheck) (authkit.Decision, error)); ok { + return returnFunc(ctx, check) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, authkit.AuthorizationCheck) authkit.Decision); ok { + r0 = returnFunc(ctx, check) + } else { + r0 = ret.Get(0).(authkit.Decision) + } + if returnFunc, ok := ret.Get(1).(func(context.Context, authkit.AuthorizationCheck) error); ok { + r1 = returnFunc(ctx, check) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// Authorizer_Can_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Can' +type Authorizer_Can_Call struct { + *mock.Call +} + +// Can is a helper method to define mock.On call +// - ctx context.Context +// - check authkit.AuthorizationCheck +func (_e *Authorizer_Expecter) Can(ctx interface{}, check interface{}) *Authorizer_Can_Call { + return &Authorizer_Can_Call{Call: _e.mock.On("Can", ctx, check)} +} + +func (_c *Authorizer_Can_Call) Run(run func(ctx context.Context, check authkit.AuthorizationCheck)) *Authorizer_Can_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 authkit.AuthorizationCheck + if args[1] != nil { + arg1 = args[1].(authkit.AuthorizationCheck) + } + run( + arg0, + arg1, + ) + }) + return _c +} + +func (_c *Authorizer_Can_Call) Return(decision authkit.Decision, err error) *Authorizer_Can_Call { + _c.Call.Return(decision, err) + return _c +} + +func (_c *Authorizer_Can_Call) RunAndReturn(run func(ctx context.Context, check authkit.AuthorizationCheck) (authkit.Decision, error)) *Authorizer_Can_Call { + _c.Call.Return(run) + return _c +} diff --git a/mocks/authkit/principal_authenticator.go b/mocks/authkit/principal_authenticator.go new file mode 100644 index 0000000..122517c --- /dev/null +++ b/mocks/authkit/principal_authenticator.go @@ -0,0 +1,152 @@ +// Code generated by mockery; DO NOT EDIT. +// github.com/vektra/mockery +// template: testify + +package authkitmocks + +import ( + "context" + "net/http" + + "github.com/meigma/authkit" + mock "github.com/stretchr/testify/mock" +) + +// NewPrincipalAuthenticator creates a new instance of PrincipalAuthenticator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewPrincipalAuthenticator(t interface { + mock.TestingT + Cleanup(func()) +}) *PrincipalAuthenticator { + mock := &PrincipalAuthenticator{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} + +// PrincipalAuthenticator is an autogenerated mock type for the PrincipalAuthenticator type +type PrincipalAuthenticator struct { + mock.Mock +} + +type PrincipalAuthenticator_Expecter struct { + mock *mock.Mock +} + +func (_m *PrincipalAuthenticator) EXPECT() *PrincipalAuthenticator_Expecter { + return &PrincipalAuthenticator_Expecter{mock: &_m.Mock} +} + +// AuthenticatePrincipal provides a mock function for the type PrincipalAuthenticator +func (_mock *PrincipalAuthenticator) AuthenticatePrincipal(ctx context.Context, r *http.Request) (*authkit.PrincipalAuthentication, error) { + ret := _mock.Called(ctx, r) + + if len(ret) == 0 { + panic("no return value specified for AuthenticatePrincipal") + } + + var r0 *authkit.PrincipalAuthentication + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, *http.Request) (*authkit.PrincipalAuthentication, error)); ok { + return returnFunc(ctx, r) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, *http.Request) *authkit.PrincipalAuthentication); ok { + r0 = returnFunc(ctx, r) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*authkit.PrincipalAuthentication) + } + } + if returnFunc, ok := ret.Get(1).(func(context.Context, *http.Request) error); ok { + r1 = returnFunc(ctx, r) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// PrincipalAuthenticator_AuthenticatePrincipal_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AuthenticatePrincipal' +type PrincipalAuthenticator_AuthenticatePrincipal_Call struct { + *mock.Call +} + +// AuthenticatePrincipal is a helper method to define mock.On call +// - ctx context.Context +// - r *http.Request +func (_e *PrincipalAuthenticator_Expecter) AuthenticatePrincipal(ctx interface{}, r interface{}) *PrincipalAuthenticator_AuthenticatePrincipal_Call { + return &PrincipalAuthenticator_AuthenticatePrincipal_Call{Call: _e.mock.On("AuthenticatePrincipal", ctx, r)} +} + +func (_c *PrincipalAuthenticator_AuthenticatePrincipal_Call) Run(run func(ctx context.Context, r *http.Request)) *PrincipalAuthenticator_AuthenticatePrincipal_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 *http.Request + if args[1] != nil { + arg1 = args[1].(*http.Request) + } + run( + arg0, + arg1, + ) + }) + return _c +} + +func (_c *PrincipalAuthenticator_AuthenticatePrincipal_Call) Return(principalAuthentication *authkit.PrincipalAuthentication, err error) *PrincipalAuthenticator_AuthenticatePrincipal_Call { + _c.Call.Return(principalAuthentication, err) + return _c +} + +func (_c *PrincipalAuthenticator_AuthenticatePrincipal_Call) RunAndReturn(run func(ctx context.Context, r *http.Request) (*authkit.PrincipalAuthentication, error)) *PrincipalAuthenticator_AuthenticatePrincipal_Call { + _c.Call.Return(run) + return _c +} + +// Name provides a mock function for the type PrincipalAuthenticator +func (_mock *PrincipalAuthenticator) Name() string { + ret := _mock.Called() + + if len(ret) == 0 { + panic("no return value specified for Name") + } + + var r0 string + if returnFunc, ok := ret.Get(0).(func() string); ok { + r0 = returnFunc() + } else { + r0 = ret.Get(0).(string) + } + return r0 +} + +// PrincipalAuthenticator_Name_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Name' +type PrincipalAuthenticator_Name_Call struct { + *mock.Call +} + +// Name is a helper method to define mock.On call +func (_e *PrincipalAuthenticator_Expecter) Name() *PrincipalAuthenticator_Name_Call { + return &PrincipalAuthenticator_Name_Call{Call: _e.mock.On("Name")} +} + +func (_c *PrincipalAuthenticator_Name_Call) Run(run func()) *PrincipalAuthenticator_Name_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *PrincipalAuthenticator_Name_Call) Return(s string) *PrincipalAuthenticator_Name_Call { + _c.Call.Return(s) + return _c +} + +func (_c *PrincipalAuthenticator_Name_Call) RunAndReturn(run func() string) *PrincipalAuthenticator_Name_Call { + _c.Call.Return(run) + return _c +} From fd62910e7488f9fee7e1053cd078143853910c2a Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Wed, 27 May 2026 15:39:36 -0700 Subject: [PATCH 5/5] chore(http): apply golines wrapping to long test lines golangci-lint's golines formatter (configured in .golangci.yml) wraps a few long lines that the mockery migration introduced. Pure formatting; no logic change. --- http/auth/middleware_test.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/http/auth/middleware_test.go b/http/auth/middleware_test.go index 0f84949..8db4016 100644 --- a/http/auth/middleware_test.go +++ b/http/auth/middleware_test.go @@ -34,7 +34,11 @@ func TestContextHelpersReturnFalseWhenMissing(t *testing.T) { } func TestMiddlewareAuthenticatePopulatesContext(t *testing.T) { - pipeline := newTestPipelineWithPrincipalAuthenticator(t, newAllowPrincipalAuthenticator(t), authkitmocks.NewAuthorizer(t)) + pipeline := newTestPipelineWithPrincipalAuthenticator( + t, + newAllowPrincipalAuthenticator(t), + authkitmocks.NewAuthorizer(t), + ) middleware := newMiddleware(t, pipeline) handlerCalled := false handler := middleware.Authenticate(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { @@ -181,7 +185,11 @@ func TestMiddlewareRequireAuthorizationExtractsFacts(t *testing.T) { func TestMiddlewareRequireAuthorizationDoesNotExtractWhenUnauthenticated(t *testing.T) { extractorCalls := 0 - pipeline := newTestPipelineWithPrincipalAuthenticator(t, newDenyPrincipalAuthenticator(t), authkitmocks.NewAuthorizer(t)) + pipeline := newTestPipelineWithPrincipalAuthenticator( + t, + newDenyPrincipalAuthenticator(t), + authkitmocks.NewAuthorizer(t), + ) middleware := newMiddleware(t, pipeline) handler := middleware.RequireAuthorization(func(*http.Request) (authkit.AuthorizationRequest, error) { extractorCalls++ @@ -203,7 +211,11 @@ func TestMiddlewareRequireAuthorizationDoesNotExtractWhenUnauthenticated(t *test func TestMiddlewareRequireFuncRendersExtractorFailureAsInternal(t *testing.T) { extractErr := errors.New("missing resource") - pipeline := newTestPipelineWithPrincipalAuthenticator(t, newAllowPrincipalAuthenticator(t), authkitmocks.NewAuthorizer(t)) + pipeline := newTestPipelineWithPrincipalAuthenticator( + t, + newAllowPrincipalAuthenticator(t), + authkitmocks.NewAuthorizer(t), + ) middleware := newMiddleware(t, pipeline) handler := middleware.RequireFunc("note:read", func(*http.Request) (authkit.Resource, error) { return authkit.Resource{}, extractErr @@ -235,7 +247,11 @@ func TestMiddlewareRendersInternalFailures(t *testing.T) { func TestMiddlewareUsesCustomRenderer(t *testing.T) { var renderedErr error - pipeline := newTestPipelineWithPrincipalAuthenticator(t, newDenyPrincipalAuthenticator(t), authkitmocks.NewAuthorizer(t)) + pipeline := newTestPipelineWithPrincipalAuthenticator( + t, + newDenyPrincipalAuthenticator(t), + authkitmocks.NewAuthorizer(t), + ) middleware, err := auth.NewMiddleware( pipeline, auth.WithErrorRenderer(func(w http.ResponseWriter, _ *http.Request, err error) {