From a6f9d23eee00068b8b429a3eb4c080ba2e520d5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20BIDON?= Date: Mon, 4 May 2026 22:04:46 +0200 Subject: [PATCH] fix(auth): detect nil interface vs nil interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fixes #147 Signed-off-by: Frédéric BIDON --- client-middleware/opentracing/go.sum | 3 +-- go.mod | 2 +- middleware/context.go | 3 ++- middleware/route_authenticator_test.go | 35 ++++++++++++++++++++++++++ middleware/router.go | 3 ++- 5 files changed, 41 insertions(+), 5 deletions(-) diff --git a/client-middleware/opentracing/go.sum b/client-middleware/opentracing/go.sum index ad3ed94c..93c05819 100644 --- a/client-middleware/opentracing/go.sum +++ b/client-middleware/opentracing/go.sum @@ -42,8 +42,7 @@ github.com/go-openapi/swag/typeutils v0.26.0 h1:2kdEwdiNWy+JJdOvu5MA2IIg2SylWAFu github.com/go-openapi/swag/typeutils v0.26.0/go.mod h1:oovDuIUvTrEHVMqWilQzKzV4YlSKgyZmFh7AlfABNVE= github.com/go-openapi/swag/yamlutils v0.25.5 h1:kASCIS+oIeoc55j28T4o8KwlV2S4ZLPT6G0iq2SSbVQ= github.com/go-openapi/swag/yamlutils v0.25.5/go.mod h1:Gek1/SjjfbYvM+Iq4QGwa/2lEXde9n2j4a3wI3pNuOQ= -github.com/go-openapi/testify/enable/yaml/v2 v2.4.2 h1:5zRca5jw7lzVREKCZVNBpysDNBjj74rBh0N2BGQbSR0= -github.com/go-openapi/testify/enable/yaml/v2 v2.4.2/go.mod h1:XVevPw5hUXuV+5AkI1u1PeAm27EQVrhXTTCPAF85LmE= +github.com/go-openapi/testify/enable/yaml/v2 v2.5.0 h1:3hZD1fwydvCx/cc1R2uYNQirHqf2s6lqpKV3FcNTURA= github.com/go-openapi/testify/v2 v2.5.0 h1:UOCr63aAsMIDydZbZGqo5Ev01D4eydItRbekDuZMJLw= github.com/go-openapi/testify/v2 v2.5.0/go.mod h1:SgsVHtfooshd0tublTtJ50FPKhujf47YRqauXXOUxfw= github.com/go-openapi/validate v0.25.2 h1:12NsfLAwGegqbGWr2CnvT65X/Q2USJipmJ9b7xDJZz0= diff --git a/go.mod b/go.mod index 043658ce..eb648c6d 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/go-openapi/swag/fileutils v0.26.0 github.com/go-openapi/swag/jsonutils v0.26.0 github.com/go-openapi/swag/stringutils v0.26.0 + github.com/go-openapi/swag/typeutils v0.26.0 github.com/go-openapi/testify/enable/yaml/v2 v2.5.0 github.com/go-openapi/testify/v2 v2.5.0 github.com/go-openapi/validate v0.25.2 @@ -30,7 +31,6 @@ require ( github.com/go-openapi/swag/jsonname v0.25.5 // indirect github.com/go-openapi/swag/loading v0.25.5 // indirect github.com/go-openapi/swag/mangling v0.25.5 // indirect - github.com/go-openapi/swag/typeutils v0.26.0 // indirect github.com/go-openapi/swag/yamlutils v0.25.5 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/google/uuid v1.6.0 // indirect diff --git a/middleware/context.go b/middleware/context.go index 1f85e86b..728c7330 100644 --- a/middleware/context.go +++ b/middleware/context.go @@ -17,6 +17,7 @@ import ( "github.com/go-openapi/loads" "github.com/go-openapi/spec" "github.com/go-openapi/strfmt" + "github.com/go-openapi/swag/typeutils" "github.com/go-openapi/runtime" "github.com/go-openapi/runtime/logger" @@ -468,7 +469,7 @@ func (c *Context) Authorize(request *http.Request, route *MatchedRoute) (any, *h } applies, usr, err := route.Authenticators.Authenticate(request, route) - if !applies || err != nil || !route.Authenticators.AllowsAnonymous() && usr == nil { + if !applies || err != nil || !route.Authenticators.AllowsAnonymous() && typeutils.IsZero(usr) { if err != nil { return nil, nil, err } diff --git a/middleware/route_authenticator_test.go b/middleware/route_authenticator_test.go index 17f83cd5..181bcc3b 100644 --- a/middleware/route_authenticator_test.go +++ b/middleware/route_authenticator_test.go @@ -194,6 +194,41 @@ func TestAuthenticateLogicalAnd(t *testing.T) { require.EqualT(t, 5, count) } +// TestAuthenticateTypedNilPrincipal is a regression test for +// https://github.com/go-openapi/runtime/issues/147. +// +// When an authenticator is written with a concrete principal type (e.g. *myPrincipal), +// returning (true, nil, nil) packs a typed-nil into the any return. A naive +// `usr == nil` check would miss it and treat the typed-nil as a successful auth. +// The Authenticate path must detect that case so the unauthenticated branch +// (a 401 in Context.Authorize) is taken. +func TestAuthenticateTypedNilPrincipal(t *testing.T) { + type myPrincipal struct{} + + typedNilAuth := runtime.AuthenticatorFunc(func(_ any) (bool, any, error) { + var p *myPrincipal // typed nil + return true, p, nil + }) + + ra := RouteAuthenticator{ + Authenticator: map[string]runtime.Authenticator{ + "auth1": typedNilAuth, + }, + Schemes: []string{"auth1"}, + Scopes: map[string][]string{"auth1": nil}, + } + ras := RouteAuthenticators([]RouteAuthenticator{ra}) + + req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/", nil) + route := &MatchedRoute{} + ok, prin, err := ras.Authenticate(req, route) + require.NoError(t, err) + // applies=true was returned but principal is (typed) nil: treated as + // not-authenticated, so the aggregator must report no successful auth. + require.FalseT(t, ok) + require.Nil(t, prin) +} + func TestAuthenticateOptional(t *testing.T) { ra1 := RouteAuthenticator{ Authenticator: map[string]runtime.Authenticator{ diff --git a/middleware/router.go b/middleware/router.go index c0b093b2..d375fd77 100644 --- a/middleware/router.go +++ b/middleware/router.go @@ -21,6 +21,7 @@ import ( "github.com/go-openapi/spec" "github.com/go-openapi/strfmt" "github.com/go-openapi/swag/stringutils" + "github.com/go-openapi/swag/typeutils" ) // RouteParam is a object to capture route params in a framework agnostic way. @@ -292,7 +293,7 @@ func (ras RouteAuthenticators) Authenticate(req *http.Request, route *MatchedRou continue } applies, usr, err := ra.Authenticate(req, route) - if !applies || err != nil || usr == nil { + if !applies || err != nil || typeutils.IsZero(usr) { if err != nil { lastError = err }