fix(codegen): dedupe security schemes by type (#3690)

* fix(codegen): dedupe security schemes by type

This commit solves a bug where designs that use multiple security
schemes _of the same type_ break the generated `Auther` interface for a
service as well as endpoint-related init methods due to unnecessary code
duplication from ranging over `.Schemes`.

* chore: add tests for multiple security services

Author: disintegrator

codegen/service/service_data.go

@@ -679,6 +679,21 @@ func (s SchemesData) Append(d *SchemeData) SchemesData {
 	return append(s, d)
 }
 
+// DedupeByType returns a new SchemesData slice that is deduplicated by scheme
+// type.
+func (s SchemesData) DedupeByType() SchemesData {
+	seen := make(map[string]struct{})
+	uniqueSchemes := SchemesData{}
+	for _, s := range s {
+		if _, ok := seen[s.Type]; !ok {
+			seen[s.Type] = struct{}{}
+			uniqueSchemes = append(uniqueSchemes, s)
+		}
+	}
+
+	return uniqueSchemes
+}
+
 // analyze creates the data necessary to render the code of the given service.
 // It records the user types needed by the service definition in userTypes.
 func (d ServicesData) analyze(service *expr.ServiceExpr) *Data {

codegen/service/service_test.go

@@ -56,6 +56,8 @@ func TestService(t *testing.T) {
 		{"service-bidirectional-streaming-no-payload", testdata.BidirectionalStreamingNoPayloadMethodDSL, testdata.BidirectionalStreamingNoPayloadMethod},
 		{"service-bidirectional-streaming-result-with-views", testdata.BidirectionalStreamingResultWithViewsMethodDSL, testdata.BidirectionalStreamingResultWithViewsMethod},
 		{"service-bidirectional-streaming-result-with-explicit-view", testdata.BidirectionalStreamingResultWithExplicitViewMethodDSL, testdata.BidirectionalStreamingResultWithExplicitViewMethod},
+		{"service-multiple-api-key-security", testdata.MultipleAPIKeySecurityDSL, testdata.MultipleAPIKeySecurity},
+		{"service-mixed-and-multiple-api-key-security", testdata.MixedAndMultipleAPIKeySecurityDSL, testdata.MixedAndMultipleAPIKeySecurity},
 	}
 	for _, c := range cases {
 		t.Run(c.Name, func(t *testing.T) {

codegen/service/templates/service.go.tpl

@@ -29,7 +29,7 @@ type Service interface {
 {{- if .Schemes }}
 // Auther defines the authorization functions to be implemented by the service.
 type Auther interface {
-	{{- range .Schemes }}
+	{{- range .Schemes.DedupeByType }}
 	{{ printf "%sAuth implements the authorization logic for the %s security scheme." .Type .Type | comment }}
 	{{ .Type }}Auth(ctx context.Context, {{ if eq .Type "Basic" }}user, pass{{ else if eq .Type "APIKey" }}key{{ else }}token{{ end }} string, schema *security.{{ .Type }}Scheme) (context.Context, error)
 	{{- end }}

codegen/service/templates/service_endpoint_method.go.tpl

@@ -1,7 +1,7 @@
 
 
 {{ printf "New%sEndpoint returns an endpoint function that calls the method %q of service %q." .VarName .Name .ServiceName | comment }}
-func New{{ .VarName }}Endpoint(s {{ .ServiceVarName }}{{ range .Schemes }}, auth{{ .Type }}Fn security.Auth{{ .Type }}Func{{ end }}) goa.Endpoint {
+func New{{ .VarName }}Endpoint(s {{ .ServiceVarName }}{{ range .Schemes.DedupeByType }}, auth{{ .Type }}Fn security.Auth{{ .Type }}Func{{ end }}) goa.Endpoint {
 	return func(ctx context.Context, req any) (any, error) {
 {{- if or .ServerStream }}
 		ep := req.(*{{ .ServerStream.EndpointStruct }})

codegen/service/templates/service_endpoints_init.go.tpl

@@ -11,7 +11,7 @@ func New{{ .VarName }}(s {{ .ServiceVarName }}{{ if .HasServerInterceptors }}, s
 	return &{{ .VarName }}{
 {{- end }}
 {{- range .Methods }}
-		{{ .VarName }}: New{{ .VarName }}Endpoint(s{{ range .Schemes }}, a.{{ .Type }}Auth{{ end }}),
+		{{ .VarName }}: New{{ .VarName }}Endpoint(s{{ range .Schemes.DedupeByType }}, a.{{ .Type }}Auth{{ end }}),
 {{- end }}
 	}
 {{- if .HasServerInterceptors }}

codegen/service/testdata/service_code.go

@@ -3315,3 +3315,79 @@ type Foo struct {
 	IntField *int
 }
 `
+
+const MultipleAPIKeySecurity = `
+// Service is the MultipleAPIKeySecurity service interface.
+type Service interface {
+	// A implements A.
+	A(context.Context, *APayload) (err error)
+}
+
+// Auther defines the authorization functions to be implemented by the service.
+type Auther interface {
+	// APIKeyAuth implements the authorization logic for the APIKey security scheme.
+	APIKeyAuth(ctx context.Context, key string, schema *security.APIKeyScheme) (context.Context, error)
+}
+
+// APIName is the name of the API as defined in the design.
+const APIName = "test api"
+
+// APIVersion is the version of the API as defined in the design.
+const APIVersion = "0.0.1"
+
+// ServiceName is the name of the service as defined in the design. This is the
+// same value that is set in the endpoint request contexts under the ServiceKey
+// key.
+const ServiceName = "MultipleAPIKeySecurity"
+
+// MethodNames lists the service method names as defined in the design. These
+// are the same values that are set in the endpoint request contexts under the
+// MethodKey key.
+var MethodNames = [1]string{"A"}
+
+// APayload is the payload type of the MultipleAPIKeySecurity service A method.
+type APayload struct {
+	APIKey   string
+	TenantID string
+}
+`
+
+const MixedAndMultipleAPIKeySecurity = `
+// Service is the MixedAndMultipleAPIKeySecurity service interface.
+type Service interface {
+	// A implements A.
+	A(context.Context, *APayload) (err error)
+}
+
+// Auther defines the authorization functions to be implemented by the service.
+type Auther interface {
+	// JWTAuth implements the authorization logic for the JWT security scheme.
+	JWTAuth(ctx context.Context, token string, schema *security.JWTScheme) (context.Context, error)
+	// APIKeyAuth implements the authorization logic for the APIKey security scheme.
+	APIKeyAuth(ctx context.Context, key string, schema *security.APIKeyScheme) (context.Context, error)
+}
+
+// APIName is the name of the API as defined in the design.
+const APIName = "test api"
+
+// APIVersion is the version of the API as defined in the design.
+const APIVersion = "0.0.1"
+
+// ServiceName is the name of the service as defined in the design. This is the
+// same value that is set in the endpoint request contexts under the ServiceKey
+// key.
+const ServiceName = "MixedAndMultipleAPIKeySecurity"
+
+// MethodNames lists the service method names as defined in the design. These
+// are the same values that are set in the endpoint request contexts under the
+// MethodKey key.
+var MethodNames = [1]string{"A"}
+
+// APayload is the payload type of the MixedAndMultipleAPIKeySecurity service A
+// method.
+type APayload struct {
+	JWT      *string
+	APIKey   *string
+	TenantID *string
+}
+`

codegen/service/testdata/service_dsls.go

@@ -981,3 +981,55 @@ var EndpointWithClientInterceptorDSL = func() {
 		})
 	})
 }
+
+var MultipleAPIKeySecurityDSL = func() {
+	var APIKeyAuth = APIKeySecurity("api_key")
+	var TenantKeyAuth = APIKeySecurity("tenant")
+
+	Service("MultipleAPIKeySecurity", func() {
+		Security(APIKeyAuth, TenantKeyAuth)
+		Method("A", func() {
+			Payload(func() {
+				APIKey("api_key", "api_key", String)
+				APIKey("tenant", "tenant_id", String)
+				Required("api_key", "tenant_id")
+			})
+
+			HTTP(func() {
+				POST("/")
+				Header("api_key:X-API-Key")
+				Header("tenant_id:X-Tenant")
+			})
+		})
+	})
+}
+
+var MixedAndMultipleAPIKeySecurityDSL = func() {
+	var Signature = JWTSecurity("jwt", func() {
+		Scope("api:read", "Read access to API")
+	})
+	var APIKeyAuth = APIKeySecurity("api_key")
+	var TenantKeyAuth = APIKeySecurity("tenant")
+
+	Service("MixedAndMultipleAPIKeySecurity", func() {
+		Security(Signature, func() {
+			Scope("api:read")
+		})
+		Security(APIKeyAuth, TenantKeyAuth)
+
+		Method("A", func() {
+			Payload(func() {
+				Token("jwt", String)
+				APIKey("api_key", "api_key", String)
+				APIKey("tenant", "tenant_id", String)
+			})
+
+			HTTP(func() {
+				POST("/")
+				Header("api_key:X-API-Key")
+				Header("tenant_id:X-Tenant")
+				Header("jwt:Authorization")
+			})
+		})
+	})
+}