Skip to content

Commit

Permalink
Merge pull request #825 from roxspring/feature/gh-814-SecureAllEndpoi…
Browse files Browse the repository at this point in the history
…ntsWithScopesRule-whitelist

SecureAllEndpointsWithScopesRule Context & whitelist support
  • Loading branch information
maxim-tschumak committed Sep 6, 2018
2 parents 9138b05 + 4836693 commit 9643b42
Show file tree
Hide file tree
Showing 11 changed files with 275 additions and 553 deletions.
7 changes: 3 additions & 4 deletions cli/zally/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,12 @@ func TestIntegrationWithLocalYamlFile(t *testing.T) {
t.Run("integrationWithLocalYamlFile", func(t *testing.T) {
out, e := RunAppAndCaptureOutput([]string{"", "lint", "../../server/src/test/resources/fixtures/api_spa.yaml"})

assert.Contains(t, out, "MUST violations: 24")
assert.Contains(t, out, "MUST violations: 29")
assert.Contains(t, out, "SHOULD violations: 1")
assert.Contains(t, out, "MAY violations: 6")
assert.Contains(t, out, "HINT violations: 0")

assert.NotNil(t, e)
assert.Equal(t, "Failing because: 24 must violation(s) found", e.Error())
})
}

Expand All @@ -68,7 +67,7 @@ func TestIntegrationWithSomeOtherLocalYamlFile(t *testing.T) {
out, e := RunAppAndCaptureOutput([]string{"", "lint", "../../server/src/test/resources/fixtures/api_tinbox.yaml"})

assert.Contains(t, out, "Provide API Specification using OpenAPI")
assert.Contains(t, out, "MUST violations: 39")
assert.Contains(t, out, "MUST violations: 48")
assert.Contains(t, out, "SHOULD violations: 23")
assert.Contains(t, out, "MAY violations: 11")
assert.Contains(t, out, "HINT violations: 0")
Expand Down Expand Up @@ -96,7 +95,7 @@ func TestIntegrationWithRemoteYamlFile(t *testing.T) {
defer ts.Close()
out, e := RunAppAndCaptureOutput([]string{"", "lint", ts.URL + "/api_spa.yaml"})

assert.Contains(t, out, "MUST violations: 24")
assert.Contains(t, out, "MUST violations: 29")
assert.Contains(t, out, "SHOULD violations: 1")
assert.Contains(t, out, "MAY violations: 6")
assert.Contains(t, out, "HINT violations: 0")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,90 +1,91 @@
package de.zalando.zally.rule.zalando

import com.google.common.collect.Sets
import com.typesafe.config.Config
import de.zalando.zally.rule.api.Check
import de.zalando.zally.rule.api.Context
import de.zalando.zally.rule.api.Rule
import de.zalando.zally.rule.api.Severity
import de.zalando.zally.rule.api.Violation
import io.swagger.models.Operation
import io.swagger.models.Swagger
import io.swagger.models.auth.OAuth2Definition
import io.swagger.v3.oas.models.OpenAPI
import io.swagger.v3.oas.models.PathItem
import io.swagger.v3.oas.models.security.SecurityScheme
import org.springframework.beans.factory.annotation.Autowired
import java.util.SortedSet

@Rule(
ruleSet = ZalandoRuleSet::class,
id = "105",
severity = Severity.MUST,
title = "Secure All Endpoints With Scopes"
ruleSet = ZalandoRuleSet::class,
id = "105",
severity = Severity.MUST,
title = "Secure All Endpoints With Scopes"
)
class SecureAllEndpointsWithScopesRule(@Autowired rulesConfig: Config) {

private val scopeRegex = Regex(rulesConfig.getString(
"${SecureAllEndpointsWithScopesRule::class.java.simpleName}.scope_regex"))
"${SecureAllEndpointsWithScopesRule::class.java.simpleName}.scope_regex"))

private val pathWhitelist = rulesConfig.getStringList(
"${SecureAllEndpointsWithScopesRule::class.java.simpleName}.path_whitelist")
.map { it.toRegex() }

@Check(severity = Severity.MUST)
fun checkDefinedScopeFormats(swagger: Swagger): Violation? {
return swagger.securityDefinitions.orEmpty().flatMap { (schemeKey, scheme) ->
when (scheme) {
is OAuth2Definition -> {
scheme.scopes.orEmpty().flatMap { (scope, _) ->
checkDefinedScopeFormat(scope)?.let {
listOf("securityDefinitions $schemeKey $scope: $it")
} ?: emptyList()
}
fun checkDefinedScopeFormats(context: Context): List<Violation> =
context.api.components?.securitySchemes?.values.orEmpty()
.filter { it.type == SecurityScheme.Type.OAUTH2 }
.flatMap { it.allFlows() }
.flatMap { flow ->
flow.scopes.orEmpty().keys.filterNot { scope ->
scopeRegex.matches(scope)
}
.map { scope ->
context.violation("scope '$scope' does not match regex '$scopeRegex'", flow.scopes)
}
else -> emptyList()
}
}.takeIf { it.isNotEmpty() }?.let { Violation("Defined scopes should match an expected format", it) }
}

private fun checkDefinedScopeFormat(scope: String): String? {
return when {
scopeRegex.matches(scope) -> null
else -> "scope '$scope' does not match regex '$scopeRegex'"
}
}

@Check(severity = Severity.MUST)
fun checkOperationsAreScoped(swagger: Swagger): Violation? {
val definedScopes = getDefinedScopes(swagger)
val hasTopLevelScope = hasTopLevelScope(swagger, definedScopes)
val paths = swagger.paths.orEmpty().entries.flatMap { (pathKey, path) ->
path.operationMap.orEmpty().entries.map { (method, operation) ->
val actualScopes = extractAppliedScopes(operation)
val undefinedScopes = Sets.difference(actualScopes, definedScopes)
val unsecured = undefinedScopes.size == actualScopes.size && !hasTopLevelScope
val msg = when {
unsecured ->
"no valid OAuth2 scope"
else -> null
}
if (msg != null) "$pathKey $method has $msg" else null
}.filterNotNull()
fun checkOperationsAreScoped(context: Context): List<Violation> {
val defined = defined(context.api)
return context.validateOperations(pathFilter = this::pathFilter) { (_, op) ->
val requested = requested(context.api, op, defined)
val undefined = undefined(requested, defined)
when {
requested.isEmpty() -> context.violations("Endpoint not secured by OAuth2 scope(s)", op.security ?: op)
undefined.isNotEmpty() -> context.violations("Endpoint secured by undefined OAuth2 scope(s): ${undefined.joinToString()}", op.security ?: op)
else -> emptyList()
}
}
return if (!paths.isEmpty()) {
Violation("Every endpoint must be secured by some scope(s)", paths)
} else null
}

// get the scopes from security definition
private fun getDefinedScopes(swagger: Swagger): Set<Pair<String, String>> =
swagger.securityDefinitions.orEmpty().entries.flatMap { (group, def) ->
(def as? OAuth2Definition)?.scopes.orEmpty().keys.map { scope -> group to scope }
}.toSet()
private fun pathFilter(entry: Map.Entry<String, PathItem>): Boolean = pathWhitelist.none { it.containsMatchIn(entry.key) }

// Extract all oauth2 scopes applied to the given operation into a simple list
private fun extractAppliedScopes(operation: Operation): Set<Pair<String, String>> =
operation.security?.flatMap { groupDefinition ->
groupDefinition.entries.flatMap { (group, scopes) ->
scopes.map { group to it }
}
}.orEmpty().toSet()
private fun SecurityScheme?.allFlows() = listOfNotNull(
this?.flows?.implicit,
this?.flows?.password,
this?.flows?.clientCredentials,
this?.flows?.authorizationCode
)

private fun hasTopLevelScope(swagger: Swagger, definedScopes: Set<Pair<String, String>>): Boolean =
swagger.security?.any { securityRequirement ->
securityRequirement.requirements.entries.any { (group, scopes) ->
scopes.any { scope -> (group to scope) in definedScopes }
}
} ?: false
private fun defined(api: OpenAPI): Map<String, Set<String>> = api.components?.securitySchemes.orEmpty()
.filterValues { scheme -> scheme.type == SecurityScheme.Type.OAUTH2 }
.mapValues { it.value.allFlows().flatMap { it.scopes.keys }.toSet() }

private fun requested(
api: OpenAPI,
op: io.swagger.v3.oas.models.Operation,
defined: Map<String, Set<String>>
): List<Pair<String, String>> = (op.security ?: api.security ?: emptyList())
.flatMap { requirement ->
requirement
.filterKeys { name -> defined.containsKey(name) }
.flatMap { (name, scopes) -> scopes.map { name to it } }
}

private fun undefined(
requested: List<Pair<String, String>>,
defined: Map<String, Set<String>>
): SortedSet<String> = requested
.filterNot { (name, scope) ->
defined[name].orEmpty().contains(scope)
}
.map { "${it.first}:${it.second}" }
.toSortedSet()
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ object JsonPointers {
"^/components/schemas/(.*)$" to "/definitions/$1",
"^/components/responses/(.*)$" to "/responses/$1",
"^/components/parameters/(.*)$" to "/parameters/$1",
"^/components/securitySchemes/(.*?)/flows/(implicit|password|clientCredentials|authorizationCode)/(.*)$" to "/securityDefinitions/$1/$3",
"^/components/securitySchemes/(.*)$" to "/securityDefinitions/$1",
"^/paths/(.+/responses/.+)/content/.+/(schema.*)$" to "/paths/$1/$2",

Expand Down
1 change: 1 addition & 0 deletions server/src/main/resources/rules-config.conf
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ ApiAudienceRule {

SecureAllEndpointsWithScopesRule {
scope_regex: "^(uid)|(([a-z-]+\\.){1,2}(read|write))$"
path_whitelist: []
}

ProprietaryHeadersRule {
Expand Down
3 changes: 3 additions & 0 deletions server/src/test/java/de/zalando/zally/TestUtil.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import io.swagger.v3.oas.models.PathItem
import io.swagger.v3.oas.models.Paths
import io.swagger.v3.oas.models.responses.ApiResponse
import io.swagger.v3.oas.models.responses.ApiResponses
import java.io.StringReader

val testConfig: Config by lazy {
ConfigFactory.load("rules-config.conf")
Expand Down Expand Up @@ -81,6 +82,8 @@ fun getSwaggerContextFromContent(content: String): Context {
}
}

fun getConfigFromContent(content: String): Config = ConfigFactory.parseReader(StringReader(content))

fun getResourceContent(fileName: String): String = ClasspathHelper.loadFileFromClasspath("fixtures/$fileName")

fun getResourceJson(fileName: String): JsonNode = ObjectTreeReader().read(getResourceContent(fileName))
Expand Down

0 comments on commit 9643b42

Please sign in to comment.