Support cookie-backed API key security inference (#3910)

* Fix cookie-backed API key security inference

* Deduplicate security-bound OpenAPI parameters

* Address OpenAPI review feedback

Author: CaliLuke

AGENTS.md

@@ -57,6 +57,8 @@ No commented-out code—delete dead code.
 
 - **Use NameScope helpers** for type references: `GoTypeRef`, `GoFullTypeRef`, `GoTypeName`. Never concatenate strings for types.
 - Let Goa decide pointer/value semantics. Do not force `pointer=true` except in transport validation.
+- **Keep helper visibility minimal**: If logic is shared only inside one codegen area, keep it package-private or move it under an `internal` package. Do not export helpers from a parent package just to share them across sibling generators.
+- **Avoid pass-through wrappers**: When two helper functions differ only by forwarding arguments or hard-coding `nil`, collapse them into a single implementation instead of adding an extra layer.
 
 ### Documentation
 

codegen/sections_test.go

@@ -71,11 +71,23 @@ import (
 package testpackage
 
 `
-		imprt            = []*ImportSpec{{Path: "test"}}
-		imports          = append(imprt, &ImportSpec{Path: "other"})
-		pathImport       = []*ImportSpec{{Path: "import/with/slashes"}}
-		pathImports      = append(pathImport, &ImportSpec{Path: "other/import/with/slashes"})
-		pathNamedImport  = []*ImportSpec{{Name: "myname", Path: "import/with/slashes"}}
+		imprt = func() []*ImportSpec {
+			imports := make([]*ImportSpec, 0, 2)
+			imports = append(imports, &ImportSpec{Path: "test"})
+			return imports
+		}()
+		imports    = append(imprt, &ImportSpec{Path: "other"})
+		pathImport = func() []*ImportSpec {
+			imports := make([]*ImportSpec, 0, 2)
+			imports = append(imports, &ImportSpec{Path: "import/with/slashes"})
+			return imports
+		}()
+		pathImports     = append(pathImport, &ImportSpec{Path: "other/import/with/slashes"})
+		pathNamedImport = func() []*ImportSpec {
+			imports := make([]*ImportSpec, 0, 2)
+			imports = append(imports, &ImportSpec{Name: "myname", Path: "import/with/slashes"})
+			return imports
+		}()
 		pathNamedImports = append(pathNamedImport, &ImportSpec{Name: "myothername", Path: "other/import/with/slashes"})
 	)
 	cases := map[string]struct {

expr/helpers.go

@@ -12,15 +12,17 @@ func Title(s string) string {
 }
 
 // findKey finds the given key in the endpoint expression and returns the
-// transport element name and the position (header, query, or body for HTTP or
-// message, metadata for gRPC endpoint).
+// transport element name and the position (header, query, cookie, or body for
+// HTTP or message, metadata for gRPC endpoint).
 func findKey(exp eval.Expression, keyAtt string) (string, string) {
 	switch e := exp.(type) {
 	case *HTTPEndpointExpr:
 		if n, exists := e.Params.FindKey(keyAtt); exists {
 			return n, "query"
 		} else if n, exists := e.Headers.FindKey(keyAtt); exists {
 			return n, "header"
+		} else if n, exists := e.Cookies.FindKey(keyAtt); exists {
+			return n, "cookie"
 		} else if e.Body == nil {
 			return "", "header"
 		}

http/codegen/cookie_security_test.go

@@ -0,0 +1,156 @@
+package codegen
+
+import (
+	"bytes"
+	"context"
+	"path/filepath"
+	"testing"
+	"text/template"
+
+	"github.com/getkin/kin-openapi/openapi2"
+	"github.com/getkin/kin-openapi/openapi3"
+	"github.com/stretchr/testify/require"
+
+	"goa.design/goa/v3/codegen"
+	"goa.design/goa/v3/dsl"
+	"goa.design/goa/v3/expr"
+	"goa.design/goa/v3/http/codegen/openapi"
+	openapiv2 "goa.design/goa/v3/http/codegen/openapi/v2"
+	openapiv3 "goa.design/goa/v3/http/codegen/openapi/v3"
+)
+
+func TestCookieAPIKeySecurity(t *testing.T) {
+	t.Run("endpoint requirement uses cookie transport", func(t *testing.T) {
+		root := RunHTTPDSL(t, cookieAPIKeySecurityDSL)
+		endpoint := root.API.HTTP.Services[0].HTTPEndpoints[0]
+		require.Len(t, endpoint.Requirements, 1)
+		require.Len(t, endpoint.Requirements[0].Schemes, 1)
+
+		scheme := endpoint.Requirements[0].Schemes[0]
+		require.Equal(t, "cookie", scheme.In)
+		require.Equal(t, "__Host-ak_session", scheme.Name)
+
+		headers := expr.AsObject(endpoint.Headers.Type)
+		require.Zero(t, len(*headers), "cookie-backed api key must not synthesize an Authorization header")
+	})
+
+	t.Run("openapi uses cookie security scheme", func(t *testing.T) {
+		root := RunHTTPDSL(t, cookieAPIKeySecurityDSL)
+		openapi.Definitions = make(map[string]*openapi.Schema)
+
+		v2JSON := renderOpenAPIJSON(t, openapiv2.Files, root)
+		var swagger openapi2.T
+		require.NoError(t, swagger.UnmarshalJSON(v2JSON))
+		require.Len(t, swagger.SecurityDefinitions, 1)
+		require.Len(t, swagger.Paths, 1)
+		require.Contains(t, swagger.Paths, "/auth/profile")
+		require.NotNil(t, swagger.Paths["/auth/profile"].Get.Security)
+		require.Len(t, *swagger.Paths["/auth/profile"].Get.Security, 1)
+		for name, def := range swagger.SecurityDefinitions {
+			require.Equal(t, "apiKey", def.Type, name)
+			require.Equal(t, "cookie", def.In, name)
+			require.Equal(t, "__Host-ak_session", def.Name, name)
+			require.Contains(t, (*swagger.Paths["/auth/profile"].Get.Security)[0], name)
+		}
+
+		openapi.Definitions = make(map[string]*openapi.Schema)
+		v3JSON := renderOpenAPIJSON(t, openapiv3.Files, root)
+		loader := openapi3.NewLoader()
+		doc, err := loader.LoadFromData(v3JSON)
+		require.NoError(t, err)
+		require.NoError(t, doc.Validate(context.Background()))
+		require.Len(t, doc.Components.SecuritySchemes, 1)
+		require.NotNil(t, doc.Paths.Find("/auth/profile"))
+		require.NotNil(t, doc.Paths.Find("/auth/profile").Get.Security)
+		require.Len(t, *doc.Paths.Find("/auth/profile").Get.Security, 1)
+		for name, ref := range doc.Components.SecuritySchemes {
+			require.NotNil(t, ref.Value, name)
+			require.Equal(t, "apiKey", ref.Value.Type, name)
+			require.Equal(t, "cookie", ref.Value.In, name)
+			require.Equal(t, "__Host-ak_session", ref.Value.Name, name)
+			require.Contains(t, (*doc.Paths.Find("/auth/profile").Get.Security)[0], name)
+		}
+	})
+
+	t.Run("http codegen does not duplicate cookie-backed auth fields", func(t *testing.T) {
+		root := RunHTTPDSL(t, cookieAPIKeySecurityDSL)
+		services := CreateHTTPServices(root)
+
+		serverTypes := serverType("gen", root.API.HTTP.Services[0], services)
+		var serverTypesBuf bytes.Buffer
+		for _, section := range serverTypes.SectionTemplates[1:] {
+			require.NoError(t, section.Write(&serverTypesBuf))
+		}
+		serverTypesCode := codegen.FormatTestCode(t, "package foo\n"+serverTypesBuf.String())
+		require.Contains(t, serverTypesCode, "func NewProfilePayload(browserSession string)")
+		require.NotContains(t, serverTypesCode, "browserSession *string, browserSession *string")
+		require.NotContains(t, serverTypesCode, "browserSession string, browserSession string")
+
+		serverFiles := ServerFiles("", services)
+		require.Len(t, serverFiles, 2)
+		serverDecode := codegen.SectionCode(t, serverFiles[1].SectionTemplates[2])
+		require.Contains(t, serverDecode, `r.Cookie("__Host-ak_session")`)
+		require.NotContains(t, serverDecode, "Authorization")
+		require.NotContains(t, serverDecode, "browserSession *string, browserSession *string")
+		require.NotContains(t, serverDecode, "browserSession string, browserSession string")
+
+		clientFiles := ClientFiles("", services)
+		require.Len(t, clientFiles, 2)
+		clientEncode := codegen.SectionCode(t, clientFiles[1].SectionTemplates[2])
+		require.Contains(t, clientEncode, `req.AddCookie(&http.Cookie{`)
+		require.Contains(t, clientEncode, `Name:  "__Host-ak_session"`)
+		require.NotContains(t, clientEncode, "Authorization")
+	})
+}
+
+func renderOpenAPIJSON(
+	t *testing.T,
+	build func(*expr.RootExpr) ([]*codegen.File, error),
+	root *expr.RootExpr,
+) []byte {
+	t.Helper()
+
+	files, err := build(root)
+	require.NoError(t, err)
+	for _, f := range files {
+		if filepath.Ext(f.Path) != ".json" {
+			continue
+		}
+		require.Len(t, f.SectionTemplates, 1)
+		section := f.SectionTemplates[0]
+		require.NotEmpty(t, section.Source)
+		require.NotNil(t, section.Data)
+
+		var buf bytes.Buffer
+		tmpl := template.Must(template.New("openapi").Funcs(section.FuncMap).Parse(section.Source))
+		require.NoError(t, tmpl.Execute(&buf, section.Data))
+		return buf.Bytes()
+	}
+
+	t.Fatalf("no JSON OpenAPI file generated")
+	return nil
+}
+
+var cookieAPIKeySecurityDSL = func() {
+	var browserSessionCookie = dsl.APIKeySecurity("browser_session_cookie", func() {
+		dsl.Description("Browser session cookie")
+	})
+
+	dsl.Service("cookieSecurity", func() {
+		dsl.Method("profile", func() {
+			dsl.Security(browserSessionCookie)
+			dsl.Payload(func() {
+				dsl.APIKey("browser_session_cookie", "browser_session", dsl.String, func() {
+					dsl.Description("Opaque browser session cookie")
+				})
+				dsl.Required("browser_session")
+			})
+			dsl.Result(dsl.Empty)
+			dsl.HTTP(func() {
+				dsl.GET("/auth/profile")
+				dsl.Cookie("browser_session:__Host-ak_session")
+				dsl.Response(dsl.StatusOK)
+			})
+		})
+	})
+}

http/codegen/openapi/internal/security.go

@@ -0,0 +1,33 @@
+package internal
+
+import (
+	"strings"
+
+	"goa.design/goa/v3/expr"
+)
+
+// IsSecurityParameter returns true if the given HTTP transport element is used
+// by one of the endpoint security schemes and should therefore not be emitted
+// again as a regular OpenAPI parameter.
+func IsSecurityParameter(endpoint *expr.HTTPEndpointExpr, in, name string) bool {
+	if endpoint == nil {
+		return false
+	}
+	for _, req := range endpoint.Requirements {
+		for _, scheme := range req.Schemes {
+			if scheme.In != in {
+				continue
+			}
+			if in == "header" {
+				if strings.EqualFold(scheme.Name, name) {
+					return true
+				}
+				continue
+			}
+			if scheme.Name == name {
+				return true
+			}
+		}
+	}
+	return false
+}

http/codegen/openapi/v2/builder.go

@@ -12,6 +12,7 @@ import (
 	"goa.design/goa/v3/codegen"
 	"goa.design/goa/v3/expr"
 	"goa.design/goa/v3/http/codegen/openapi"
+	openapiinternal "goa.design/goa/v3/http/codegen/openapi/internal"
 )
 
 // NewV2 returns the OpenAPI v2 specification for the given API.
@@ -35,7 +36,7 @@ func NewV2(root *expr.RootExpr, h *expr.HostExpr) (*V2, error) {
 	if hasAbsoluteRoutes(root) {
 		basePath = ""
 	}
-	params := paramsFromExpr(root.API.HTTP.Params, basePath)
+	params := paramsFromExpr(nil, root.API.HTTP.Params, basePath)
 	var paramMap map[string]*Parameter
 	if len(params) > 0 {
 		paramMap = make(map[string]*Parameter, len(params))
@@ -269,7 +270,7 @@ func summaryFromMeta(name string, meta expr.MetaExpr) string {
 	return name
 }
 
-func paramsFromExpr(params *expr.MappedAttributeExpr, path string) []*Parameter {
+func paramsFromExpr(endpoint *expr.HTTPEndpointExpr, params *expr.MappedAttributeExpr, path string) []*Parameter {
 	if params == nil {
 		return nil
 	}
@@ -283,6 +284,9 @@ func paramsFromExpr(params *expr.MappedAttributeExpr, path string) []*Parameter
 			in = "path"
 			required = true
 		}
+		if endpoint != nil && in != "path" && openapiinternal.IsSecurityParameter(endpoint, in, pn) {
+			return nil
+		}
 		param := paramFor(at, pn, in, required)
 		res = append(res, param)
 		return nil
@@ -294,24 +298,14 @@ func paramsFromHeaders(endpoint *expr.HTTPEndpointExpr) []*Parameter {
 	var params []*Parameter
 
 	expr.WalkMappedAttr(endpoint.Headers, func(name, elem string, att *expr.AttributeExpr) error { // nolint: errcheck
+		if openapiinternal.IsSecurityParameter(endpoint, "header", elem) {
+			return nil
+		}
 		required := endpoint.Headers.IsRequiredNoDefault(name)
 		params = append(params, paramFor(att, elem, "header", required))
 		return nil
 	})
 
-	// Add basic auth to headers
-	if att := expr.TaggedAttribute(endpoint.MethodExpr.Payload, "security:username"); att != "" {
-		// Basic Auth is always encoded in the Authorization header
-		// https://golang.org/pkg/net/http/#Request.SetBasicAuth
-		params = append(params, &Parameter{
-			In:          "header",
-			Name:        "Authorization",
-			Required:    endpoint.MethodExpr.Payload.IsRequired(att),
-			Description: "Basic Auth security using Basic scheme (https://tools.ietf.org/html/rfc7617)",
-			Type:        "string",
-		})
-	}
-
 	return params
 }
 
@@ -502,7 +496,7 @@ func buildPathFromExpr(s *V2, root *expr.RootExpr, h *expr.HostExpr, route *expr
 		// Remove any wildcards that is defined in path as a workaround to
 		// https://github.com/OAI/OpenAPI-Specification/issues/291
 		key = expr.HTTPWildcardRegex.ReplaceAllString(key, "/{$1}")
-		params := paramsFromExpr(endpoint.Params, key)
+		params := paramsFromExpr(endpoint, endpoint.Params, key)
 		params = append(params, paramsFromHeaders(endpoint)...)
 		var produces []string
 

http/codegen/openapi/v2/testdata/TestSections/security_file0.golden

@@ -14,33 +14,6 @@
       "get": {
         "description": "\n**Required security scopes for basic**:\n  * `api:read`\n\n**Required security scopes for jwt**:\n  * `api:read`\n\n**Required security scopes for api_key**:\n  * `api:read`",
         "operationId": "testService#testEndpointA",
-        "parameters": [
-          {
-            "in": "query",
-            "name": "k",
-            "required": true,
-            "type": "string"
-          },
-          {
-            "in": "header",
-            "name": "Token",
-            "required": true,
-            "type": "string"
-          },
-          {
-            "in": "header",
-            "name": "X-Authorization",
-            "required": true,
-            "type": "string"
-          },
-          {
-            "description": "Basic Auth security using Basic scheme (https://tools.ietf.org/html/rfc7617)",
-            "in": "header",
-            "name": "Authorization",
-            "required": true,
-            "type": "string"
-          }
-        ],
         "responses": {
           "204": {
             "description": "No Content response."
@@ -66,20 +39,6 @@
       },
       "post": {
         "operationId": "testService#testEndpointB",
-        "parameters": [
-          {
-            "in": "query",
-            "name": "auth",
-            "required": true,
-            "type": "string"
-          },
-          {
-            "in": "header",
-            "name": "Authorization",
-            "required": true,
-            "type": "string"
-          }
-        ],
         "responses": {
           "204": {
             "description": "No Content response."