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.

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

Introduction of Glob Patterns in extension of API related to URL paths #417

Merged
merged 15 commits into from
Aug 19, 2024
Merged
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
### Changed
- Add JWT failure reason to metadata and use it in jwt-status field on denied requests

## [0.21.0]
### Changed
- Added `paths` field in API to support Glob Patterns

## [0.20.15]
### Changed
- Java-control-plane update to 1.0.45
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,11 @@ fun Value.toIncomingEndpoint(properties: SnapshotProperties): IncomingEndpoint {
val pathPrefix = this.field("pathPrefix")?.stringValue
val path = this.field("path")?.stringValue
val pathRegex = this.field("pathRegex")?.stringValue
val paths = this.field("paths")?.list().orEmpty().map { it.stringValue }.toSet()

if (isMoreThanOnePropertyDefined(path, pathPrefix, pathRegex)) {
throw NodeMetadataValidationException("Precisely one of 'path', 'pathPrefix' or 'pathRegex' field is allowed")
if (isMoreThanOnePropertyDefined(paths, path, pathPrefix, pathRegex)) {
throw NodeMetadataValidationException(
"Precisely one of 'paths', 'path', 'pathPrefix' or 'pathRegex' field is allowed")
}

val methods = this.field("methods")?.list().orEmpty().map { it.stringValue }.toSet()
Expand All @@ -407,16 +409,20 @@ fun Value.toIncomingEndpoint(properties: SnapshotProperties): IncomingEndpoint {
val oauth = properties.let { this.field("oauth")?.toOAuth(it) }

return when {
path != null -> IncomingEndpoint(path, PathMatchingType.PATH, methods, clients, unlistedClientsPolicy, oauth)
paths.isNotEmpty() -> IncomingEndpoint(
paths, "", PathMatchingType.PATH, methods, clients, unlistedClientsPolicy, oauth)
path != null -> IncomingEndpoint(
paths, path, PathMatchingType.PATH, methods, clients, unlistedClientsPolicy, oauth)
pathPrefix != null -> IncomingEndpoint(
pathPrefix, PathMatchingType.PATH_PREFIX, methods, clients, unlistedClientsPolicy, oauth
paths, pathPrefix, PathMatchingType.PATH_PREFIX, methods, clients, unlistedClientsPolicy, oauth
)

pathRegex != null -> IncomingEndpoint(
pathRegex, PathMatchingType.PATH_REGEX, methods, clients, unlistedClientsPolicy, oauth
paths, pathRegex, PathMatchingType.PATH_REGEX, methods, clients, unlistedClientsPolicy, oauth
)

else -> throw NodeMetadataValidationException("One of 'path', 'pathPrefix' or 'pathRegex' field is required")
else -> throw NodeMetadataValidationException(
"One of 'paths', 'path', 'pathPrefix' or 'pathRegex' field is required")
}
}

Expand Down Expand Up @@ -447,7 +453,16 @@ fun Value.toIncomingRateLimitEndpoint(): IncomingRateLimitEndpoint {
}
}

fun isMoreThanOnePropertyDefined(vararg properties: String?): Boolean = properties.filterNotNull().count() > 1
fun isMoreThanOnePropertyDefined(vararg properties: Any?): Boolean =
countNonNullAndNotEmptyProperties(properties.toList()) > 1

private fun countNonNullAndNotEmptyProperties(props: List<Any?>): Int = props.filterNotNull().count {
if (it is Set<*>) {
it.isNotEmpty()
} else {
true
}
}

private fun Value?.toIncomingTimeoutPolicy(): Incoming.TimeoutPolicy {
val idleTimeout: Duration? = this?.field("idleTimeout")?.toDuration()
Expand Down Expand Up @@ -784,6 +799,7 @@ data class ClientWithSelector private constructor(
}

data class IncomingEndpoint(
override val paths: Set<String> = emptySet(),
override val path: String = "",
override val pathMatchingType: PathMatchingType = PathMatchingType.PATH,
override val methods: Set<String> = emptySet(),
Expand Down Expand Up @@ -824,6 +840,7 @@ data class OAuth(
}

interface EndpointBase {
val paths: Set<String>
val path: String
val pathMatchingType: PathMatchingType
val methods: Set<String>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.google.protobuf.Any
import com.google.protobuf.Empty
import com.google.protobuf.util.Durations
import io.envoyproxy.envoy.config.core.v3.HttpUri
import io.envoyproxy.envoy.config.core.v3.TypedExtensionConfig
import io.envoyproxy.envoy.config.route.v3.HeaderMatcher
import io.envoyproxy.envoy.config.route.v3.RouteMatch
import io.envoyproxy.envoy.extensions.filters.http.jwt_authn.v3.JwtAuthentication
Expand All @@ -13,6 +14,7 @@ import io.envoyproxy.envoy.extensions.filters.http.jwt_authn.v3.JwtRequirementOr
import io.envoyproxy.envoy.extensions.filters.http.jwt_authn.v3.RemoteJwks
import io.envoyproxy.envoy.extensions.filters.http.jwt_authn.v3.RequirementRule
import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpFilter
import io.envoyproxy.envoy.extensions.path.match.uri_template.v3.UriTemplateMatchConfig
import io.envoyproxy.envoy.type.matcher.v3.RegexMatcher
import pl.allegro.tech.servicemesh.envoycontrol.groups.Group
import pl.allegro.tech.servicemesh.envoycontrol.groups.IncomingEndpoint
Expand Down Expand Up @@ -98,10 +100,10 @@ class JwtFilterFactory(
}

private fun createRules(endpoints: List<IncomingEndpoint>): Set<RequirementRule> {
return endpoints.mapNotNull(this::createRuleForEndpoint).toSet()
return endpoints.flatMap(this::createRulesForEndpoint).toSet()
}

private fun createRuleForEndpoint(endpoint: IncomingEndpoint): RequirementRule? {
private fun createRulesForEndpoint(endpoint: IncomingEndpoint): Set<RequirementRule> {
val providers = mutableSetOf<String>()

if (endpoint.oauth != null) {
Expand All @@ -111,11 +113,44 @@ class JwtFilterFactory(
providers.addAll(endpoint.clients.filter { it.name in clientToOAuthProviderName.keys }
.mapNotNull { clientToOAuthProviderName[it.name] })

return if (providers.isNotEmpty()) {
requirementRuleWithPathMatching(endpoint.path, endpoint.pathMatchingType, endpoint.methods, providers)
if (providers.isEmpty()) {
return emptySet()
}

return if (endpoint.paths.isNotEmpty()) {
endpoint.paths.map {
requirementRuleWithURITemplateMatching(it, endpoint.methods, providers)
}.toSet()
} else {
null
setOf(requirementRuleWithPathMatching(
endpoint.path, endpoint.pathMatchingType, endpoint.methods, providers))
}
}

private fun requirementRuleWithURITemplateMatching(
pathGlobPattern: String,
methods: Set<String>,
providers: MutableSet<String>
): RequirementRule {
val pathMatching = RouteMatch.newBuilder().setPathMatchPolicy(
TypedExtensionConfig.newBuilder()
.setName("envoy.path.match.uri_template.uri_template_matcher")
.setTypedConfig(
Any.pack(
UriTemplateMatchConfig.newBuilder()
.setPathTemplate(pathGlobPattern)
.build()
)
).build()
)
if (methods.isNotEmpty()) {
pathMatching.addHeaders(createHeaderMatcherBuilder(methods))
}

return RequirementRule.newBuilder()
.setMatch(pathMatching)
.setRequires(createJwtRequirement(providers))
.build()
}

private fun requirementRuleWithPathMatching(
Expand All @@ -135,22 +170,25 @@ class JwtFilterFactory(
)
}
if (methods.isNotEmpty()) {
val methodsRegexp = methods.joinToString("|")
val headerMatcher = HeaderMatcher.newBuilder()
.setName(":method")
.setSafeRegexMatch(
RegexMatcher.newBuilder().setRegex(methodsRegexp).setGoogleRe2(
RegexMatcher.GoogleRE2.getDefaultInstance()
).build()
)
pathMatching.addHeaders(headerMatcher)
pathMatching.addHeaders(createHeaderMatcherBuilder(methods))
}

return RequirementRule.newBuilder()
.setMatch(pathMatching)
.setRequires(createJwtRequirement(providers))
.build()
}

private fun createHeaderMatcherBuilder(methods: Set<String>): HeaderMatcher.Builder {
return HeaderMatcher.newBuilder()
.setName(":method")
.setSafeRegexMatch(
RegexMatcher.newBuilder().setRegex(methods.joinToString("|")).setGoogleRe2(
RegexMatcher.GoogleRE2.getDefaultInstance()
).build()
)
}

private val requirementsForProviders: Map<ProviderName, JwtRequirement> =
jwtProviders.keys.associateWith { JwtRequirement.newBuilder().setProviderName(it).build() }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters

import com.google.protobuf.Any
import io.envoyproxy.envoy.config.core.v3.TypedExtensionConfig
import io.envoyproxy.envoy.config.rbac.v3.Permission
import io.envoyproxy.envoy.config.route.v3.HeaderMatcher
import io.envoyproxy.envoy.extensions.path.match.uri_template.v3.UriTemplateMatchConfig
import io.envoyproxy.envoy.type.matcher.v3.PathMatcher
import io.envoyproxy.envoy.type.matcher.v3.RegexMatcher
import io.envoyproxy.envoy.type.matcher.v3.StringMatcher
Expand All @@ -11,8 +14,12 @@ import pl.allegro.tech.servicemesh.envoycontrol.groups.PathMatchingType
class RBACFilterPermissions {
fun createCombinedPermissions(incomingEndpoint: IncomingEndpoint): Permission.Builder {
val permissions = listOfNotNull(
createPathPermissionForEndpoint(incomingEndpoint),
createMethodPermissions(incomingEndpoint)
if (incomingEndpoint.paths.isNotEmpty()) {
createPathTemplatesPermissionForEndpoint(incomingEndpoint)
} else {
createPathPermissionForEndpoint(incomingEndpoint)
},
createMethodPermissions(incomingEndpoint),
)
.map { it.build() }

Expand All @@ -21,6 +28,22 @@ class RBACFilterPermissions {
)
}

private fun createPathTemplatesPermissionForEndpoint(incomingEndpoint: IncomingEndpoint): Permission.Builder {
return permission()
.setOrRules(Permission.Set.newBuilder().addAllRules(
incomingEndpoint.paths.map(this::createPathTemplate)))
}

private fun createPathTemplate(path: String): Permission {
return permission().setUriTemplate(TypedExtensionConfig.newBuilder()
.setName("envoy.path.match.uri_template.uri_template_matcher")
.setTypedConfig(Any.pack(
UriTemplateMatchConfig.newBuilder()
.setPathTemplate(path)
.build()
))).build()
}

fun createPathPermission(path: String, matchingType: PathMatchingType): Permission.Builder {
return permission().setUrlPath(createPathMatcher(path, matchingType))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class NodeMetadataTest {

// expects
val exception = assertThrows<NodeMetadataValidationException> { proto.toIncomingEndpoint(snapshotProperties()) }
assertThat(exception.status.description).isEqualTo("Precisely one of 'path', 'pathPrefix' or 'pathRegex' field is allowed")
assertThat(exception.status.description).isEqualTo("Precisely one of 'paths', 'path', 'pathPrefix' or 'pathRegex' field is allowed")
assertThat(exception.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT)
}

Expand All @@ -135,18 +135,18 @@ class NodeMetadataTest {

// expects
val exception = assertThrows<NodeMetadataValidationException> { proto.toIncomingEndpoint(snapshotProperties()) }
assertThat(exception.status.description).isEqualTo("Precisely one of 'path', 'pathPrefix' or 'pathRegex' field is allowed")
assertThat(exception.status.description).isEqualTo("Precisely one of 'paths', 'path', 'pathPrefix' or 'pathRegex' field is allowed")
assertThat(exception.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT)
}

@Test
fun `should reject endpoint with no path or pathPrefix or pathRegex defined`() {
fun `should reject endpoint with empty paths or without path or pathPrefix or pathRegex defined`() {
// given
val proto = incomingEndpointProto(path = null, pathPrefix = null, pathRegex = null)

// expects
val exception = assertThrows<NodeMetadataValidationException> { proto.toIncomingEndpoint(snapshotProperties()) }
assertThat(exception.status.description).isEqualTo("One of 'path', 'pathPrefix' or 'pathRegex' field is required")
assertThat(exception.status.description).isEqualTo("One of 'paths', 'path', 'pathPrefix' or 'pathRegex' field is required")
assertThat(exception.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ internal class JwtFilterFactoryTest {
Incoming(
pathToProvider.map { (path, provider) ->
IncomingEndpoint(
path,
path = path,
oauth = OAuth(provider, policy = policy)
)
}
Expand All @@ -182,7 +182,7 @@ internal class JwtFilterFactoryTest {
Incoming(
pathToProvider.map { (path, _) ->
IncomingEndpoint(
path,
path = path,
clients = setOf(ClientWithSelector.create("oauth", "client")),
oauth = null
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
permissionsEnabled = true,
endpoints = listOf(
IncomingEndpoint(
emptySet(),
"/oauth-protected",
PathMatchingType.PATH,
setOf("GET"),
Expand Down Expand Up @@ -110,6 +111,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
permissionsEnabled = true,
endpoints = listOf(
IncomingEndpoint(
emptySet(),
"/oauth-protected",
PathMatchingType.PATH,
setOf("GET"),
Expand Down Expand Up @@ -152,6 +154,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
permissionsEnabled = true,
endpoints = listOf(
IncomingEndpoint(
emptySet(),
"/oauth-protected",
PathMatchingType.PATH,
setOf("GET"),
Expand All @@ -178,6 +181,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
permissionsEnabled = true,
endpoints = listOf(
IncomingEndpoint(
emptySet(),
"/oauth-protected",
PathMatchingType.PATH,
setOf("GET"),
Expand Down Expand Up @@ -219,6 +223,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
permissionsEnabled = true,
endpoints = listOf(
IncomingEndpoint(
emptySet(),
"/oauth-protected",
PathMatchingType.PATH,
setOf("GET"),
Expand Down Expand Up @@ -264,6 +269,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
permissionsEnabled = true,
endpoints = listOf(
IncomingEndpoint(
emptySet(),
"/oauth-protected",
PathMatchingType.PATH,
setOf("GET"),
Expand Down Expand Up @@ -336,7 +342,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
) = """
{
"policies": {
"IncomingEndpoint(path=/oauth-protected, pathMatchingType=PATH, methods=[GET], clients=[$clientsWithSelector], unlistedClientsPolicy=$unlistedClientsPolicy, oauth=$oauth)": {
"IncomingEndpoint(paths=[], path=/oauth-protected, pathMatchingType=PATH, methods=[GET], clients=[$clientsWithSelector], unlistedClientsPolicy=$unlistedClientsPolicy, oauth=$oauth)": {
"permissions": [
{
"and_rules": {
Expand Down
Loading
Loading