Fix rule 105 and add more unit tests

zalando · May 20, 2021 · 294f8a4 · 294f8a4
1 parent 0c6151e
commit 294f8a4
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 29 deletions.
diff --git a/...ndo/src/main/kotlin/org/zalando/zally/ruleset/zalando/SecureAllEndpointsWithScopesRule.kt b/...ndo/src/main/kotlin/org/zalando/zally/ruleset/zalando/SecureAllEndpointsWithScopesRule.kt
@@ -9,7 +9,6 @@ import io.swagger.v3.oas.models.security.SecurityScheme
 import org.zalando.zally.core.util.allFlows
 import org.zalando.zally.core.util.allScopes
 import org.zalando.zally.core.util.getAllSecuritySchemes
-import org.zalando.zally.core.util.isBearer
 import org.zalando.zally.core.util.isOAuth2
 import org.zalando.zally.rule.api.Check
 import org.zalando.zally.rule.api.Context
@@ -59,28 +58,30 @@ class SecureAllEndpointsWithScopesRule(rulesConfig: Config) {
         return context.validateOperations(pathFilter = this::pathFilter) { (_, op) ->
             op?.let {
 
-                val requirements = definedSecurityRequirements(op, context.api)
+                val definedOpSecurityRequirements = definedSecurityRequirements(op, context.api)
 
-                if (requirements.isEmpty()) {
+                if (definedOpSecurityRequirements.isEmpty()) {
                     context.violations(
                         "Endpoint is not secured by scope(s)", op.security ?: op
                     )
                 } else {
-                    requirements.flatMap {
-                        it.map { (schemaName, operationScopes) ->
-                            securitySchemes[schemaName]?.let { schema ->
-                                when {
-                                    schema.isOAuth2() -> {
-                                        validateOAuth2Schema(
-                                            context,
-                                            op,
-                                            operationScopes,
-                                            schema,
-                                            schemaName
-                                        )
-                                    }
-                                    schema.isBearer() -> validateBearerSchema(context, op, schemaName)
-                                    else -> null
+                    definedOpSecurityRequirements.flatMap {
+                        it.map { (opSchemeName, opScopes) ->
+                            val matchingScheme = securitySchemes[opSchemeName]
+                            if (matchingScheme == null) {
+                                context.violation("Security scheme $opSchemeName not found", op)
+                            } else {
+                                if (matchingScheme.isOAuth2()) {
+                                    validateOAuth2Schema(
+                                        context,
+                                        op,
+                                        opScopes,
+                                        matchingScheme,
+                                        opSchemeName
+                                    )
+                                } else {
+                                    // Scopes are only used with OAuth 2 and OpenID Connect
+                                    null
                                 }
                             }
                         }
@@ -90,22 +91,27 @@ class SecureAllEndpointsWithScopesRule(rulesConfig: Config) {
         }
     }
 
-    private fun definedSecurityRequirements(operation: Operation, api: OpenAPI): List<SecurityRequirement> =
-        api.security.orEmpty() + operation.security.orEmpty()
+    private fun definedSecurityRequirements(operation: Operation, api: OpenAPI): List<SecurityRequirement> {
+        val operationSecurity = operation.security.orEmpty()
+        if (operationSecurity.isEmpty()) {
+            return api.security.orEmpty()
+        }
+        return operationSecurity
+    }
 
     private fun validateOAuth2Schema(
         context: Context,
         op: Operation,
         requestedScopes: List<String?>,
-        scheme: SecurityScheme,
+        definedScheme: SecurityScheme,
         schemeName: String
     ): Violation? {
         if (requestedScopes.isEmpty()) {
             return context.violation(
                 "Endpoint is not secured by OAuth2 scope(s)", op.security ?: op
             )
         }
-        val definedScopes = scheme.allScopes()
+        val definedScopes = definedScheme.allScopes()
         val undefined =
             requestedScopes.filterNotNull().filterNot { sc -> definedScopes.contains(sc) }
         return if (undefined.isNotEmpty()) {
@@ -116,13 +122,6 @@ class SecureAllEndpointsWithScopesRule(rulesConfig: Config) {
         } else null
     }
 
-    private fun validateBearerSchema(context: Context, op: Operation, schemeName: String): Violation? {
-        val requirement = op.security?.find { it[schemeName] != null }
-        return if (requirement == null) {
-            context.violation("Endpoint is not secured by scope(s)", op.security)
-        } else null
-    }
-
     private fun pathFilter(entry: Map.Entry<String, PathItem?>): Boolean =
         pathWhitelist.none { it.containsMatchIn(entry.key) }
 }
diff --git a/...src/test/kotlin/org/zalando/zally/ruleset/zalando/SecureAllEndpointsWithScopesRuleTest.kt b/...src/test/kotlin/org/zalando/zally/ruleset/zalando/SecureAllEndpointsWithScopesRuleTest.kt
@@ -362,4 +362,68 @@ class SecureAllEndpointsWithScopesRuleTest {
         val violations = rule.checkOperationsAreScoped(context)
         assertThat(violations).hasSize(1)
     }
+
+    @Test
+    fun `Rule supports Bearer global security scheme`() {
+        @Language("YAML")
+        val yaml = """
+            openapi: 3.0.1
+            security:
+              - BearerAuth: ['scope.execute']
+            paths:
+              '/things':
+                get:
+                  responses:
+                    200:
+                      description: OK
+              '/other-things':
+                get:
+                  responses:
+                    200:
+                      description: OK
+            components:      
+              securitySchemes:
+                BearerAuth:
+                  type: http
+                  scheme: bearer
+            """.trimIndent()
+
+        val context = DefaultContextFactory().getOpenApiContext(yaml)
+
+        val violations = rule.checkOperationsAreScoped(context)
+        assertThat(violations).isEmpty()
+    }
+
+    @Test
+    fun `Security scheme names match`() {
+        @Language("YAML")
+        val yaml = """
+            openapi: 3.0.1
+            security:
+              - AnotherBearerAuth: ['scope.execute']
+            paths:
+              '/things':
+                get:
+                  responses:
+                    200:
+                      description: OK
+              '/other-things':
+                get:
+                  responses:
+                    200:
+                      description: OK
+                  security:
+                    - BearerAuth: []                      
+            components:      
+              securitySchemes:
+                BearerAuth:
+                  type: http
+                  scheme: bearer
+            """.trimIndent()
+
+        val context = DefaultContextFactory().getOpenApiContext(yaml)
+
+        val violations = rule.checkOperationsAreScoped(context)
+        assertThat(violations).hasSize(1)
+    }
 }