Repository: goadesign/goa (go)

Issue:
v3: Using BasicAuth with custom scopes
As mentioned in #1959, we kinda 'misuse' BasicAuth in v1 by supplying scopes to it. This allows us to extract the scopes via goa.ContextRequiredScopes(ctx) in the middleware, authenticate the user against LDAP and match the scopes against LDAP groups. It even generates correct swagger, which contains the required scope, e.g.: BasicAuth (users:list). As far as I can see, v3 does not allow scopes to be defined on Security(BasicAuth) and also requires to have username and password payload defined on every method, instead of handling everything in middleware only.

Is there any approach to move this to v2/v3 or any idea how we could solve this? :smiley:

Pull request description:
Allow security scopes on BasicAuth and APIKey authentication
This PR resolves #2105 and allows to define `Scope` on `BasicAuth` and `APIKeyAuth` definitions.

Description on endpoint now contains the `Security` name to distinguish between the methods. Additionally consistent formatting is applied to descriptions containing security scopes.

Patch:
diff --git a/codegen/service/endpoint.go b/codegen/service/endpoint.go
index f2e9be69..8bb6e7b9 100644
--- a/codegen/service/endpoint.go
+++ b/codegen/service/endpoint.go
@@ -207,6 +207,8 @@ func New{{ .VarName }}Endpoint(s {{ .ServiceVarName }}{{ range .Schemes }}, auth
 			{{- if eq .Type "Basic" }}
 				sc := security.BasicScheme{
 					Name: {{ printf "%q" .SchemeName }},
+					Scopes: []string{ {{- range .Scopes }}{{ printf "%q" . }}, {{ end }} },
+					RequiredScopes: []string{ {{- range $r.Scopes }}{{ printf "%q" . }}, {{ end }} },
 				}
 				{{- if .UsernamePointer }}
 				var user string
@@ -226,6 +228,8 @@ func New{{ .VarName }}Endpoint(s {{ .ServiceVarName }}{{ range .Schemes }}, auth
 			{{- else if eq .Type "APIKey" }}
 				sc := security.APIKeyScheme{
 					Name: {{ printf "%q" .SchemeName }},
+					Scopes: []string{ {{- range .Scopes }}{{ printf "%q" . }}, {{ end }} },
+					RequiredScopes: []string{ {{- range $r.Scopes }}{{ printf "%q" . }}, {{ end }} },
 				}
 				{{- if $s.CredPointer }}
 				var key string
diff --git a/codegen/service/service_data.go b/codegen/service/service_data.go
index 54ce71e1..c4409fac 100644
--- a/codegen/service/service_data.go
+++ b/codegen/service/service_data.go
@@ -854,6 +854,13 @@ func buildSchemeData(s *expr.SchemeExpr, m *expr.MethodExpr) *SchemeData {
 		user := codegen.Goify(userAtt, true)
 		passAtt := expr.TaggedAttribute(m.Payload, "security:password")
 		pass := codegen.Goify(passAtt, true)
+		var scopes []string
+		if len(s.Scopes) > 0 {
+			scopes = make([]string, len(s.Scopes))
+			for i, s := range s.Scopes {
+				scopes[i] = s.Name
+			}
+		}
 		return &SchemeData{
 			Type:             s.Kind.String(),
 			SchemeName:       s.SchemeName,
@@ -865,10 +872,18 @@ func buildSchemeData(s *expr.SchemeExpr, m *expr.MethodExpr) *SchemeData {
 			PasswordField:    pass,
 			PasswordPointer:  m.Payload.IsPrimitivePointer(passAtt, true),
 			PasswordRequired: m.Payload.IsRequired(passAtt),
+			Scopes:           scopes,
 		}
 	case expr.APIKeyKind:
 		if keyAtt := expr.TaggedAttribute(m.Payload, "security:apikey:"+s.SchemeName); keyAtt != "" {
 			key := codegen.Goify(keyAtt, true)
+			var scopes []string
+			if len(s.Scopes) > 0 {
+				scopes = make([]string, len(s.Scopes))
+				for i, s := range s.Scopes {
+					scopes[i] = s.Name
+				}
+			}
 			return &SchemeData{
 				Type:         s.Kind.String(),
 				Name:         s.Name,
@@ -877,6 +892,7 @@ func buildSchemeData(s *expr.SchemeExpr, m *expr.MethodExpr) *SchemeData {
 				CredPointer:  m.Payload.IsPrimitivePointer(keyAtt, true),
 				CredRequired: m.Payload.IsRequired(keyAtt),
 				KeyAttr:      keyAtt,
+				Scopes:       scopes,
 				In:           s.In,
 			}
 		}
diff --git a/dsl/security.go b/dsl/security.go
index f063bad5..dd33ba94 100644
--- a/dsl/security.go
+++ b/dsl/security.go
@@ -519,7 +519,7 @@ func TokenField(tag interface{}, name string, args ...interface{}) {
 // Scope has two uses: in JWTSecurity or OAuth2Security it defines a scope
 // supported by the scheme. In Security it lists required scopes.
 //
-// Scope must appear in Security, JWTSecurity or OAuth2Security.
+// Scope must appear in Security, BasicSecurity, APIKeySecurity, JWTSecurity or OAuth2Security.
 //
 // Scope accepts one or two arguments: the first argument is the scope name and
 // when used in JWTSecurity or OAuth2Security the second argument is a
diff --git a/expr/method.go b/expr/method.go
index 6b7c8e98..d4a16744 100644
--- a/expr/method.go
+++ b/expr/method.go
@@ -136,7 +136,7 @@ func (m *MethodExpr) Validate() error {
 			for _, scope := range r.Scopes {
 				found := false
 				for _, s := range r.Schemes {
-					if s.Kind == OAuth2Kind || s.Kind == JWTKind {
+					if s.Kind == BasicAuthKind || s.Kind == APIKeyKind || s.Kind == OAuth2Kind || s.Kind == JWTKind {
 						for _, se := range s.Scopes {
 							if se.Name == scope {
 								found = true
diff --git a/expr/security.go b/expr/security.go
index e518e60e..3a3029d5 100644
--- a/expr/security.go
+++ b/expr/security.go
@@ -65,7 +65,7 @@ type (
 		// Name refers to a header or parameter name, based on In's
 		// value.
 		Name string
-		// Scopes lists the JWT or OAuth2 scopes.
+		// Scopes lists the Basic, APIKey, JWT or OAuth2 scopes.
 		Scopes []*ScopeExpr
 		// Flows determine the oauth2 flows supported by this scheme.
 		Flows []*FlowExpr
diff --git a/http/codegen/openapi/openapi_v2_builder.go b/http/codegen/openapi/openapi_v2_builder.go
index 0d8e4965..b5356fad 100644
--- a/http/codegen/openapi/openapi_v2_builder.go
+++ b/http/codegen/openapi/openapi_v2_builder.go
@@ -143,6 +143,22 @@ func mustGenerate(meta expr.MetaExpr) bool {
 	return true
 }
 
+// addScopeDescription generates and adds required scopes to the scheme's description.
+func addScopeDescription(scopes []*expr.ScopeExpr, sd *SecurityDefinition) {
+	// Generate scopes to add to description
+	lines := []string{}
+	for _, scope := range scopes {
+		lines = append(lines, fmt.Sprintf("  * `%s`: %s", scope.Name, scope.Description))
+	}
+	// Add scope description only if scopes are defined
+	if len(lines) > 0 {
+		if sd.Description != "" {
+			sd.Description += "\n"
+		}
+		sd.Description += fmt.Sprintf("\n**Security Scopes**:\n%s", strings.Join(lines, "\n"))
+	}
+}
+
 // securitySpecFromExpr generates the OpenAPI security definitions from the
 // security design.
 func securitySpecFromExpr(root *expr.RootExpr) map[string]*SecurityDefinition {
@@ -159,21 +175,19 @@ func securitySpecFromExpr(root *expr.RootExpr) map[string]*SecurityDefinition {
 					switch s.Kind {
 					case expr.BasicAuthKind:
 						sd.Type = "basic"
+						addScopeDescription(s.Scopes, &sd)
 					case expr.APIKeyKind:
 						sd.Type = "apiKey"
 						sd.In = s.In
 						sd.Name = s.Name
+						addScopeDescription(s.Scopes, &sd)
 					case expr.JWTKind:
 						sd.Type = "apiKey"
 						// OpenAPI V2 spec does not support JWT scheme. Hence we add the scheme
 						// information to the description.
-						lines := []string{}
-						for _, scope := range s.Scopes {
-							lines = append(lines, fmt.Sprintf("  * `%s`: %s", scope.Name, scope.Description))
-						}
+						addScopeDescription(s.Scopes, &sd)
 						sd.In = s.In
 						sd.Name = s.Name
-						sd.Description += fmt.Sprintf("\n**Security Scopes**:\n%s", strings.Join(lines, "\n"))
 					case expr.OAuth2Kind:
 						sd.Type = "oauth2"
 						if scopesLen := len(s.Scopes); scopesLen > 0 {
@@ -648,15 +662,18 @@ func buildPathFromExpr(s *V2, root *expr.RootExpr, h *expr.HostExpr, route *expr
 				switch s.Kind {
 				case expr.OAuth2Kind:
 					requirement[s.Hash()] = append(requirement[s.Hash()], req.Scopes...)
-				case expr.JWTKind:
+				case expr.BasicAuthKind, expr.APIKeyKind, expr.JWTKind:
 					lines := make([]string, 0, len(req.Scopes))
 					for _, scope := range req.Scopes {
 						lines = append(lines, fmt.Sprintf("  * `%s`", scope))
 					}
-					if description != "" {
-						description += "\n"
+					// List scopes only if they are defined
+					if len(lines) > 0 {
+						if description != "" {
+							description += "\n"
+						}
+						description += fmt.Sprintf("\n**Required security scopes for %s**:\n%s", s.SchemeName, strings.Join(lines, "\n"))
 					}
-					description += fmt.Sprintf("\nRequired security scopes:\n%s", strings.Join(lines, "\n"))
 				}
 			}
 			requirements[i] = requirement
diff --git a/security/scheme.go b/security/scheme.go
index eea35d93..222d555f 100644
--- a/security/scheme.go
+++ b/security/scheme.go
@@ -20,6 +20,11 @@ type (
 	BasicScheme struct {
 		// Name is the scheme name defined in the design.
 		Name string
+		// Scopes holds a list of scopes for the scheme.
+		Scopes []string
+		// RequiredScopes holds a list of scopes which are required
+		// by the scheme. It is a subset of Scopes field.
+		RequiredScopes []string
 	}
 
 	// APIKeyScheme represents the API key security scheme.
@@ -27,6 +32,11 @@ type (
 	APIKeyScheme struct {
 		// Name is the scheme name defined in the design.
 		Name string
+		// Scopes holds a list of scopes for the scheme.
+		Scopes []string
+		// RequiredScopes holds a list of scopes which are required
+		// by the scheme. It is a subset of Scopes field.
+		RequiredScopes []string
 	}
 
 	// JWTScheme represents an API key based scheme with support
@@ -83,6 +93,18 @@ type (
 	AuthJWTFunc func(ctx context.Context, token string, s *JWTScheme) (context.Context, error)
 )
 
+// Validate returns a non-nil error if scopes does not contain all of
+// Basic scheme's required scopes.
+func (s *BasicScheme) Validate(scopes []string) error {
+	return validateScopes(s.RequiredScopes, scopes)
+}
+
+// Validate returns a non-nil error if scopes does not contain all of
+// APIKey scheme's required scopes.
+func (s *APIKeyScheme) Validate(scopes []string) error {
+	return validateScopes(s.RequiredScopes, scopes)
+}
+
 // Validate returns a non-nil error if scopes does not contain all of
 // OAuth2 scheme's required scopes.
 func (s *OAuth2Scheme) Validate(scopes []string) error {


Test patch:
diff --git a/codegen/service/security_test.go b/codegen/service/security_test.go
index cc576f81..1d544903 100644
--- a/codegen/service/security_test.go
+++ b/codegen/service/security_test.go
@@ -48,6 +48,7 @@ func TestSecureEndpoint(t *testing.T) {
 		Code string
 	}{
 		{"with-required-scopes", testdata.EndpointWithRequiredScopesDSL, testdata.EndpointWithRequiredScopesCode},
+		{"with-optional-required-scopes", testdata.EndpointWithOptionalRequiredScopesDSL, testdata.EndpointWithOptionalRequiredScopesCode},
 		{"with-api-key-override", testdata.EndpointWithAPIKeyOverrideDSL, testdata.EndpointWithAPIKeyOverrideCode},
 		{"with-oauth2", testdata.EndpointWithOAuth2DSL, testdata.EndpointWithOAuth2Code},
 	}
diff --git a/codegen/service/testdata/security_dsls.go b/codegen/service/testdata/security_dsls.go
index 984512ea..2127b6b3 100644
--- a/codegen/service/testdata/security_dsls.go
+++ b/codegen/service/testdata/security_dsls.go
@@ -4,7 +4,11 @@ import (
 	. "goa.design/goa/v3/dsl"
 )
 
-var BasicAuth = BasicAuthSecurity("basic")
+var BasicAuth = BasicAuthSecurity("basic", func() {
+	Scope("api:read", "Read-only access")
+	Scope("api:write", "Read and write access")
+	Scope("api:admin", "Admin access")
+})
 
 var JWTAuth = JWTSecurity("jwt", func() {
 	Scope("api:read", "Read-only access")
@@ -12,7 +16,11 @@ var JWTAuth = JWTSecurity("jwt", func() {
 	Scope("api:admin", "Admin access")
 })
 
-var APIKeyAuth = APIKeySecurity("api_key")
+var APIKeyAuth = APIKeySecurity("api_key", func() {
+	Scope("api:read", "Read-only access")
+	Scope("api:write", "Read and write access")
+	Scope("api:admin", "Admin access")
+})
 
 var OAuth2AuthorizationCode = OAuth2Security("authCode", func() {
 	AuthorizationCodeFlow("/authorization", "/token", "/refresh")
@@ -109,6 +117,24 @@ var EndpointWithRequiredScopesDSL = func() {
 	})
 }
 
+var EndpointWithOptionalRequiredScopesDSL = func() {
+	Service("EndpointWithOptionalRequiredScopes", func() {
+		Method("SecureWithOptionalRequiredScopes", func() {
+			Security(BasicAuth, func() {
+				Scope("api:read")
+				Scope("api:write")
+			})
+			Payload(func() {
+				Username("user", String)
+				Password("pass", String)
+			})
+			HTTP(func() {
+				GET("/")
+			})
+		})
+	})
+}
+
 var EndpointWithAPIKeyOverrideDSL = func() {
 	Service("EndpointWithAPIKeyOverride", func() {
 		Security(BasicAuth)
diff --git a/codegen/service/testdata/security_functions.go b/codegen/service/testdata/security_functions.go
index 4844eb40..03e57db3 100644
--- a/codegen/service/testdata/security_functions.go
+++ b/codegen/service/testdata/security_functions.go
@@ -67,6 +67,35 @@ func NewSecureWithRequiredScopesEndpoint(s Service, authJWTFn security.AuthJWTFu
 }
 `
 
+var EndpointWithOptionalRequiredScopesCode = `// NewSecureWithOptionalRequiredScopesEndpoint returns an endpoint function
+// that calls the method "SecureWithOptionalRequiredScopes" of service
+// "EndpointWithOptionalRequiredScopes".
+func NewSecureWithOptionalRequiredScopesEndpoint(s Service, authBasicFn security.AuthBasicFunc) goa.Endpoint {
+	return func(ctx context.Context, req interface{}) (interface{}, error) {
+		p := req.(*SecureWithOptionalRequiredScopesPayload)
+		var err error
+		sc := security.BasicScheme{
+			Name:           "basic",
+			Scopes:         []string{"api:read", "api:write", "api:admin"},
+			RequiredScopes: []string{"api:read", "api:write"},
+		}
+		var user string
+		if p.User != nil {
+			user = *p.User
+		}
+		var pass string
+		if p.Pass != nil {
+			pass = *p.Pass
+		}
+		ctx, err = authBasicFn(ctx, user, pass, &sc)
+		if err != nil {
+			return nil, err
+		}
+		return nil, s.SecureWithOptionalRequiredScopes(ctx, p)
+	}
+}
+`
+
 var EndpointWithAPIKeyOverrideCode = `// NewSecureWithAPIKeyOverrideEndpoint returns an endpoint function that calls
 // the method "SecureWithAPIKeyOverride" of service
 // "EndpointWithAPIKeyOverride".
@@ -75,7 +104,9 @@ func NewSecureWithAPIKeyOverrideEndpoint(s Service, authAPIKeyFn security.AuthAP
 		p := req.(*SecureWithAPIKeyOverridePayload)
 		var err error
 		sc := security.APIKeyScheme{
-			Name: "api_key",
+			Name:           "api_key",
+			Scopes:         []string{"api:read", "api:write", "api:admin"},
+			RequiredScopes: []string{},
 		}
 		var key string
 		if p.Key != nil {
diff --git a/http/codegen/testdata/openapi_v2/TestSections/security_file0.golden b/http/codegen/testdata/openapi_v2/TestSections/security_file0.golden
index 3c99b629..e4b2902b 100644
--- a/http/codegen/testdata/openapi_v2/TestSections/security_file0.golden
+++ b/http/codegen/testdata/openapi_v2/TestSections/security_file0.golden
@@ -1,1 +1,1 @@
-{"swagger":"2.0","info":{"title":"","version":""},"host":"localhost:80","consumes":["application/json","application/xml","application/gob"],"produces":["application/json","application/xml","application/gob"],"paths":{"/":{"get":{"tags":["testService"],"summary":"testEndpointA testService","description":"\nRequired security scopes:\n  * `api:read`","operationId":"testService#testEndpointA","parameters":[{"name":"k","in":"query","required":true,"type":"string"},{"name":"Token","in":"header","required":true,"type":"string"},{"name":"X-Authorization","in":"header","required":true,"type":"string"},{"name":"Authorization","in":"header","description":"Basic Auth security using Basic scheme (https://tools.ietf.org/html/rfc7617)","required":true,"type":"string"}],"responses":{"200":{"description":"OK response."}},"schemes":["http"],"security":[{"api_key_query_k":[],"basic_header_Authorization":[],"jwt_header_X-Authorization":[],"oauth2_header_Token":["api:read"]}]},"post":{"tags":["testService"],"summary":"testEndpointB testService","operationId":"testService#testEndpointB","parameters":[{"name":"auth","in":"query","required":true,"type":"string"},{"name":"Authorization","in":"header","required":true,"type":"string"}],"responses":{"200":{"description":"OK response."}},"schemes":["http"],"security":[{"api_key_header_Authorization":[]},{"oauth2_query_auth":["api:read","api:write"]}]}}},"securityDefinitions":{"api_key_header_Authorization":{"type":"apiKey","description":"Secures endpoint by requiring an API key.","name":"Authorization","in":"header"},"api_key_query_k":{"type":"apiKey","description":"Secures endpoint by requiring an API key.","name":"k","in":"query"},"basic_header_Authorization":{"type":"basic","description":"Basic authentication used to authenticate security principal during signin"},"jwt_header_X-Authorization":{"type":"apiKey","description":"Secures endpoint by requiring a valid JWT token retrieved via the signin endpoint. Supports scopes \"api:read\" and \"api:write\".\n**Security Scopes**:\n  * `api:read`: Read-only access\n  * `api:write`: Read and write access","name":"X-Authorization","in":"header"},"oauth2_header_Token":{"type":"oauth2","description":"Secures endpoint by requiring a valid OAuth2 token retrieved via the signin endpoint. Supports scopes \"api:read\" and \"api:write\".","flow":"accessCode","authorizationUrl":"http://goa.design/authorization","tokenUrl":"http://goa.design/token","scopes":{"api:read":"Read-only access","api:write":"Read and write access"}},"oauth2_query_auth":{"type":"oauth2","description":"Secures endpoint by requiring a valid OAuth2 token retrieved via the signin endpoint. Supports scopes \"api:read\" and \"api:write\".","flow":"accessCode","authorizationUrl":"http://goa.design/authorization","tokenUrl":"http://goa.design/token","scopes":{"api:read":"Read-only access","api:write":"Read and write access"}}}}
\ No newline at end of file
+{"swagger":"2.0","info":{"title":"","version":""},"host":"localhost:80","consumes":["application/json","application/xml","application/gob"],"produces":["application/json","application/xml","application/gob"],"paths":{"/":{"get":{"tags":["testService"],"summary":"testEndpointA testService","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":[{"name":"k","in":"query","required":true,"type":"string"},{"name":"Token","in":"header","required":true,"type":"string"},{"name":"X-Authorization","in":"header","required":true,"type":"string"},{"name":"Authorization","in":"header","description":"Basic Auth security using Basic scheme (https://tools.ietf.org/html/rfc7617)","required":true,"type":"string"}],"responses":{"200":{"description":"OK response."}},"schemes":["http"],"security":[{"api_key_query_k":[],"basic_header_Authorization":[],"jwt_header_X-Authorization":[],"oauth2_header_Token":["api:read"]}]},"post":{"tags":["testService"],"summary":"testEndpointB testService","operationId":"testService#testEndpointB","parameters":[{"name":"auth","in":"query","required":true,"type":"string"},{"name":"Authorization","in":"header","required":true,"type":"string"}],"responses":{"200":{"description":"OK response."}},"schemes":["http"],"security":[{"api_key_header_Authorization":[]},{"oauth2_query_auth":["api:read","api:write"]}]}}},"securityDefinitions":{"api_key_header_Authorization":{"type":"apiKey","description":"Secures endpoint by requiring an API key.","name":"Authorization","in":"header"},"api_key_query_k":{"type":"apiKey","description":"Secures endpoint by requiring an API key.","name":"k","in":"query"},"basic_header_Authorization":{"type":"basic","description":"Basic authentication used to authenticate security principal during signin"},"jwt_header_X-Authorization":{"type":"apiKey","description":"Secures endpoint by requiring a valid JWT token retrieved via the signin endpoint. Supports scopes \"api:read\" and \"api:write\".\n\n**Security Scopes**:\n  * `api:read`: Read-only access\n  * `api:write`: Read and write access","name":"X-Authorization","in":"header"},"oauth2_header_Token":{"type":"oauth2","description":"Secures endpoint by requiring a valid OAuth2 token retrieved via the signin endpoint. Supports scopes \"api:read\" and \"api:write\".","flow":"accessCode","authorizationUrl":"http://goa.design/authorization","tokenUrl":"http://goa.design/token","scopes":{"api:read":"Read-only access","api:write":"Read and write access"}},"oauth2_query_auth":{"type":"oauth2","description":"Secures endpoint by requiring a valid OAuth2 token retrieved via the signin endpoint. Supports scopes \"api:read\" and \"api:write\".","flow":"accessCode","authorizationUrl":"http://goa.design/authorization","tokenUrl":"http://goa.design/token","scopes":{"api:read":"Read-only access","api:write":"Read and write access"}}}}
\ No newline at end of file
diff --git a/http/codegen/testdata/openapi_v2/TestSections/security_file1.golden b/http/codegen/testdata/openapi_v2/TestSections/security_file1.golden
index 13be03bb..39ff21a7 100644
--- a/http/codegen/testdata/openapi_v2/TestSections/security_file1.golden
+++ b/http/codegen/testdata/openapi_v2/TestSections/security_file1.golden
@@ -19,7 +19,13 @@ paths:
       summary: testEndpointA testService
       description: |2-
 
-        Required security scopes:
+        **Required security scopes for basic**:
+          * `api:read`
+
+        **Required security scopes for jwt**:
+          * `api:read`
+
+        **Required security scopes for api_key**:
           * `api:read`
       operationId: testService#testEndpointA
       parameters:
@@ -94,6 +100,7 @@ securityDefinitions:
     type: apiKey
     description: |-
       Secures endpoint by requiring a valid JWT token retrieved via the signin endpoint. Supports scopes "api:read" and "api:write".
+
       **Security Scopes**:
         * `api:read`: Read-only access
         * `api:write`: Read and write access