Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a Bearer security schema support #1227

Merged
merged 3 commits into from
May 27, 2021
Merged

Add a Bearer security schema support #1227

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c6151e
Add a `Bearer` security schema support
vadeg Apr 23, 2021
294f8a4
Fix rule 105 and add more unit tests
vadeg May 20, 2021
68418ef
Improve formating
vadeg May 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions server/zally-core/src/main/kotlin/org/zalando/zally/core/util/OpenApiUtil.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import io.swagger.v3.oas.models.OpenAPI
import io.swagger.v3.oas.models.media.Schema
import io.swagger.v3.oas.models.parameters.Parameter
import io.swagger.v3.oas.models.responses.ApiResponse
import io.swagger.v3.oas.models.security.SecurityScheme
import java.util.Objects

data class HeaderElement(
Expand Down Expand Up @@ -118,6 +119,28 @@ fun OpenAPI.getAllParameters(): Map<String, Parameter> = this.components?.parame
it?.readOperations().orEmpty().flatMap { it?.parameters.orEmpty().mapNotNull { it.name to it } }
}

fun OpenAPI.getAllSecuritySchemes(): Map<String, SecurityScheme> = this.components?.securitySchemes.orEmpty()

/**
* Checks if the SecurityScheme is a Bearer security scheme
*/
fun SecurityScheme.isBearer(): Boolean = this.scheme == "bearer" && this.type == SecurityScheme.Type.HTTP

/**
* Checks if the SecurityScheme is an OAuth2 security scheme
*/
fun SecurityScheme.isOAuth2(): Boolean = this.type == SecurityScheme.Type.OAUTH2

fun SecurityScheme.allFlows() = listOfNotNull(
this.flows?.implicit,
this.flows?.password,
this.flows?.clientCredentials,
this.flows?.authorizationCode
)

fun SecurityScheme.allScopes(): List<String> =
this.allFlows().flatMap { flow -> flow.scopes?.keys.orEmpty() }.toSet().filterNotNull()

/**
* Calculates custom hash to avoid calling the hash of the parent schema.
* E.g. io.swagger.v3.oas.models.media.ArraySchema#hashCode() calls super#hashCode()
Expand Down
119 changes: 73 additions & 46 deletions ...ndo/src/main/kotlin/org/zalando/zally/ruleset/zalando/SecureAllEndpointsWithScopesRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
package org.zalando.zally.ruleset.zalando

import com.typesafe.config.Config
import io.swagger.v3.oas.models.OpenAPI
import io.swagger.v3.oas.models.Operation
import io.swagger.v3.oas.models.PathItem
import io.swagger.v3.oas.models.security.SecurityRequirement
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
import org.zalando.zally.rule.api.Rule
import org.zalando.zally.rule.api.Severity
import org.zalando.zally.rule.api.Violation
import io.swagger.v3.oas.models.OpenAPI
import io.swagger.v3.oas.models.PathItem
import io.swagger.v3.oas.models.security.SecurityScheme
import java.util.SortedSet

@Rule(
ruleSet = ZalandoRuleSet::class,
Expand All @@ -33,7 +39,8 @@ class SecureAllEndpointsWithScopesRule(rulesConfig: Config) {
@Check(severity = Severity.MUST)
fun checkDefinedScopeFormats(context: Context): List<Violation> =
context.api.components?.securitySchemes?.values.orEmpty()
.filter { it.type == SecurityScheme.Type.OAUTH2 }
.filter { it.isOAuth2() }
.filterNotNull()
.flatMap { it.allFlows() }
.flatMap { flow ->
flow.scopes.orEmpty().keys.filterNot { scope ->
Expand All @@ -46,56 +53,76 @@ class SecureAllEndpointsWithScopesRule(rulesConfig: Config) {

@Check(severity = Severity.MUST)
fun checkOperationsAreScoped(context: Context): List<Violation> {
val defined = defined(context.api)
val securitySchemes = context.api.getAllSecuritySchemes()

// val definedScopes = definedScopes(context.api)
return context.validateOperations(pathFilter = this::pathFilter) { (_, op) ->
op?.let {
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

val requirements = definedSecurityRequirements(op, context.api)

if (requirements.isEmpty()) {
context.violations(
"Endpoint is not secured by scope(s)", op.security ?: op
)
else -> emptyList()
} else {
requirements.flatMap {
it.map { (schemaName, operationScopes) ->
securitySchemes[schemaName]?.let { schema ->
when {
schema.isOAuth2() -> {
validateOAuth2Schema(
context,
op,
operationScopes,
schema,
schemaName
)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The brackets are not necessary - which may improve readability here.

schema.isBearer() -> validateBearerSchema(context, op, schemaName)
else -> null
}
}
}
}
}
}.orEmpty()
}
}

private fun pathFilter(entry: Map.Entry<String, PathItem?>): Boolean =
pathWhitelist.none { it.containsMatchIn(entry.key) }

private fun SecurityScheme?.allFlows() = listOfNotNull(
this?.flows?.implicit,
this?.flows?.password,
this?.flows?.clientCredentials,
this?.flows?.authorizationCode
)
private fun definedSecurityRequirements(operation: Operation, api: OpenAPI): List<SecurityRequirement> =
api.security.orEmpty() + operation.security.orEmpty()

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.orEmpty() }.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 validateOAuth2Schema(
context: Context,
op: Operation,
requestedScopes: List<String?>,
scheme: 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 undefined =
requestedScopes.filterNotNull().filterNot { sc -> definedScopes.contains(sc) }
return if (undefined.isNotEmpty()) {
context.violation(
"Endpoint is secured by undefined OAuth2 scope(s): $schemeName:${undefined.joinToString()}",
op.security ?: op
)
} else null
}

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()
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) }
}
81 changes: 74 additions & 7 deletions ...src/test/kotlin/org/zalando/zally/ruleset/zalando/SecureAllEndpointsWithScopesRuleTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import org.zalando.zally.ruleset.zalando.util.getConfigFromContent
import org.zalando.zally.test.ZallyAssertions
import org.intellij.lang.annotations.Language
import org.junit.Test
import org.zalando.zally.test.ZallyAssertions.assertThat

/**
* Tests for SecureAllEndpointsWithScopesRule
Expand Down Expand Up @@ -140,8 +141,8 @@ class SecureAllEndpointsWithScopesRuleTest {

val violations = rule.checkOperationsAreScoped(context)

ZallyAssertions.assertThat(violations)
.descriptionsEqualTo("Endpoint not secured by OAuth2 scope(s)")
assertThat(violations)
.descriptionsEqualTo("Endpoint is not secured by scope(s)")
.pointersEqualTo("/paths/~1things/get")
}

Expand Down Expand Up @@ -200,8 +201,8 @@ class SecureAllEndpointsWithScopesRuleTest {

val violations = rule.checkOperationsAreScoped(context)

ZallyAssertions.assertThat(violations)
.descriptionsEqualTo("Endpoint secured by undefined OAuth2 scope(s): oauth2:undefined-scope")
assertThat(violations)
.descriptionsEqualTo("Endpoint is secured by undefined OAuth2 scope(s): oauth2:undefined-scope")
.pointersEqualTo("/paths/~1things/get/security")
}

Expand Down Expand Up @@ -231,7 +232,7 @@ class SecureAllEndpointsWithScopesRuleTest {

val violations = rule.checkOperationsAreScoped(context)

ZallyAssertions.assertThat(violations).isEmpty()
assertThat(violations).isEmpty()
}

@Test
Expand Down Expand Up @@ -291,8 +292,74 @@ class SecureAllEndpointsWithScopesRuleTest {

val violations = rule.checkOperationsAreScoped(context)

ZallyAssertions.assertThat(violations)
.descriptionsEqualTo("Endpoint not secured by OAuth2 scope(s)")
assertThat(violations)
.descriptionsEqualTo("Endpoint is not secured by scope(s)")
.pointersEqualTo("/paths/~1things/get")
}

@Test
fun `Rule supports Bearer security scheme`() {
@Language("YAML")
val yaml = """
openapi: 3.0.1

paths:
'/things':
get:
responses:
200:
description: OK
security:
- BearerAuth: ['scope.execute']
'/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).isEmpty()
}

@Test
fun `Unsecured path is detected using Bearer security scheme`() {
@Language("YAML")
val yaml = """
openapi: 3.0.1

paths:
'/unsecured-path':
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)
}
}