Repository: goadesign/goa
Pull request #2338 (2019-10-25 02:29:15)

Validate a method when a security but no payload
This is backport of #2332 to v3.

raphael: Thank you!

Patch:
diff --git a/expr/method.go b/expr/method.go
index d4a16744..49d212a1 100644
--- a/expr/method.go
+++ b/expr/method.go
@@ -99,55 +99,53 @@ func (m *MethodExpr) Prepare() {
 // Validate validates the method payloads, results, and errors (if any).
 func (m *MethodExpr) Validate() error {
 	verr := new(eval.ValidationErrors)
-	if m.Payload.Type != Empty {
-		verr.Merge(m.Payload.Validate("payload", m))
-		// validate security scheme requirements
-		var requirements []*SecurityExpr
-		if len(m.Requirements) > 0 {
-			requirements = m.Requirements
-		} else if len(m.Service.Requirements) > 0 {
-			requirements = m.Service.Requirements
-		}
-		for _, r := range requirements {
-			for _, s := range r.Schemes {
-				verr.Merge(s.Validate())
-				switch s.Kind {
-				case BasicAuthKind:
-					if !hasTag(m.Payload, "security:username") {
-						verr.Add(m, "payload of method %q of service %q does not define a username attribute, use Username to define one", m.Name, m.Service.Name)
-					}
-					if !hasTag(m.Payload, "security:password") {
-						verr.Add(m, "payload of method %q of service %q does not define a password attribute, use Password to define one", m.Name, m.Service.Name)
-					}
-				case APIKeyKind:
-					if !hasTag(m.Payload, "security:apikey:"+s.SchemeName) {
-						verr.Add(m, "payload of method %q of service %q does not define an API key attribute, use APIKey to define one", m.Name, m.Service.Name)
-					}
-				case JWTKind:
-					if !hasTag(m.Payload, "security:token") {
-						verr.Add(m, "payload of method %q of service %q does not define a JWT attribute, use Token to define one", m.Name, m.Service.Name)
-					}
-				case OAuth2Kind:
-					if !hasTag(m.Payload, "security:accesstoken") {
-						verr.Add(m, "payload of method %q of service %q does not define a OAuth2 access token attribute, use AccessToken to define one", m.Name, m.Service.Name)
-					}
+	verr.Merge(m.Payload.Validate("payload", m))
+	// validate security scheme requirements
+	var requirements []*SecurityExpr
+	if len(m.Requirements) > 0 {
+		requirements = m.Requirements
+	} else if len(m.Service.Requirements) > 0 {
+		requirements = m.Service.Requirements
+	}
+	for _, r := range requirements {
+		for _, s := range r.Schemes {
+			verr.Merge(s.Validate())
+			switch s.Kind {
+			case BasicAuthKind:
+				if !hasTag(m.Payload, "security:username") {
+					verr.Add(m, "payload of method %q of service %q does not define a username attribute, use Username to define one", m.Name, m.Service.Name)
+				}
+				if !hasTag(m.Payload, "security:password") {
+					verr.Add(m, "payload of method %q of service %q does not define a password attribute, use Password to define one", m.Name, m.Service.Name)
+				}
+			case APIKeyKind:
+				if !hasTag(m.Payload, "security:apikey:"+s.SchemeName) {
+					verr.Add(m, "payload of method %q of service %q does not define an API key attribute, use APIKey to define one", m.Name, m.Service.Name)
+				}
+			case JWTKind:
+				if !hasTag(m.Payload, "security:token") {
+					verr.Add(m, "payload of method %q of service %q does not define a JWT attribute, use Token to define one", m.Name, m.Service.Name)
+				}
+			case OAuth2Kind:
+				if !hasTag(m.Payload, "security:accesstoken") {
+					verr.Add(m, "payload of method %q of service %q does not define a OAuth2 access token attribute, use AccessToken to define one", m.Name, m.Service.Name)
 				}
 			}
-			for _, scope := range r.Scopes {
-				found := false
-				for _, s := range r.Schemes {
-					if s.Kind == BasicAuthKind || s.Kind == APIKeyKind || s.Kind == OAuth2Kind || s.Kind == JWTKind {
-						for _, se := range s.Scopes {
-							if se.Name == scope {
-								found = true
-								break
-							}
+		}
+		for _, scope := range r.Scopes {
+			found := false
+			for _, s := range r.Schemes {
+				if s.Kind == BasicAuthKind || s.Kind == APIKeyKind || s.Kind == OAuth2Kind || s.Kind == JWTKind {
+					for _, se := range s.Scopes {
+						if se.Name == scope {
+							found = true
+							break
 						}
 					}
 				}
-				if !found {
-					verr.Add(m, "security scope %q not found in any of the security schemes.", scope)
-				}
+			}
+			if !found {
+				verr.Add(m, "security scope %q not found in any of the security schemes.", scope)
 			}
 		}
 	}


Test patch:
diff --git a/expr/http_endpoint_test.go b/expr/http_endpoint_test.go
index f89afc6c..a4fa587d 100644
--- a/expr/http_endpoint_test.go
+++ b/expr/http_endpoint_test.go
@@ -57,6 +57,12 @@ func TestHTTPEndpointValidation(t *testing.T) {
 				"service \"Service\" method \"Method\": payload of method \"Method\" of service \"Service\" does not define a JWT attribute, use Token to define one",
 			},
 		},
+		"endpoint-missing-token-payload": {
+			DSL: testdata.EndpointMissingTokenPayload,
+			Errors: []string{
+				"service \"Service\" method \"Method\": payload of method \"Method\" of service \"Service\" does not define a JWT attribute, use Token to define one",
+			},
+		},
 		"endpoint-missing-token-extend": {
 			DSL: testdata.EndpointExtendToken,
 		},
diff --git a/expr/testdata/endpoint_dsls.go b/expr/testdata/endpoint_dsls.go
index 99f848f2..9765fcae 100644
--- a/expr/testdata/endpoint_dsls.go
+++ b/expr/testdata/endpoint_dsls.go
@@ -125,6 +125,22 @@ var EndpointMissingToken = func() {
 	})
 }
 
+var EndpointMissingTokenPayload = func() {
+	var JWT = JWTSecurity("JWT", func() {
+		Scope("api:read", "Read access")
+	})
+	Service("Service", func() {
+		Security(JWT, func() {
+			Scope("api:read")
+		})
+		Method("Method", func() {
+			HTTP(func() {
+				POST("/")
+			})
+		})
+	})
+}
+
 var EndpointExtendToken = func() {
 	var CommonAttributes = Type("Common", func() {
 		Token("token", String)