From 4117211bb7becf5a849000bf6945896e122d2159 Mon Sep 17 00:00:00 2001 From: Ahoo Wang Date: Sun, 8 Jan 2023 00:58:00 +0800 Subject: [PATCH] Refactor: Remove `Statement.conditions` (#63) * Refactor: Remove `Statement.conditions` --- README.md | 130 ++++++++-------- README.zh-CN.md | 130 ++++++++-------- .../me/ahoo/cosec/api/policy/Statement.kt | 8 - .../cosec/policy/DefaultPolicyEvaluator.kt | 4 +- .../me/ahoo/cosec/policy/StatementData.kt | 3 +- .../serialization/JsonStatementSerializer.kt | 12 -- .../me/ahoo/cosec/policy/StatementDataTest.kt | 13 +- .../serialization/CoSecJsonSerializerTest.kt | 139 ++++++++++-------- document/cosec-policy.schema.json | 6 - document/design/assets/Modeling.svg | 18 +-- document/design/uml/modeling.puml | 2 +- gradle.properties | 2 +- 12 files changed, 212 insertions(+), 255 deletions(-) diff --git a/README.md b/README.md index a01f96fc..254b21e5 100644 --- a/README.md +++ b/README.md @@ -81,11 +81,9 @@ RBAC-based And Policy-based Multi-Tenant Reactive Security Framework. "pattern": "/user/#{principal.id}/*" } ], - "conditions": [ - { - "type": "authenticated" - } - ] + "condition": { + "type": "authenticated" + } }, { "name": "Developer", @@ -95,15 +93,13 @@ RBAC-based And Policy-based Multi-Tenant Reactive Security Framework. "type": "all" } ], - "conditions": [ - { - "type": "in", - "part": "context.principal.id", - "in": [ - "developerId" - ] - } - ] + "condition": { + "type": "in", + "part": "context.principal.id", + "in": [ + "developerId" + ] + } }, { "name": "RequestOriginDeny", @@ -113,14 +109,12 @@ RBAC-based And Policy-based Multi-Tenant Reactive Security Framework. "type": "all" } ], - "conditions": [ - { - "type": "reg", - "negate": true, - "part": "request.origin", - "pattern": "^(http|https)://github.com" - } - ] + "condition": { + "type": "reg", + "negate": true, + "part": "request.origin", + "pattern": "^(http|https)://github.com" + } }, { "name": "IpBlacklist", @@ -130,18 +124,16 @@ RBAC-based And Policy-based Multi-Tenant Reactive Security Framework. "type": "all" } ], - "conditions": [ - { - "type": "path", - "part": "request.remoteIp", - "path": { - "caseSensitive": false, - "separator": ".", - "decodeAndParseSegments": false - }, - "pattern": "192.168.0.*" - } - ] + "condition": { + "type": "path", + "part": "request.remoteIp", + "path": { + "caseSensitive": false, + "separator": ".", + "decodeAndParseSegments": false + }, + "pattern": "192.168.0.*" + } }, { "name": "RegionWhitelist", @@ -151,14 +143,12 @@ RBAC-based And Policy-based Multi-Tenant Reactive Security Framework. "type": "all" } ], - "conditions": [ - { - "negate": true, - "type": "reg", - "part": "request.attributes.ipRegion", - "pattern": "^中国\\|0\\|(上海|广东省)\\|.*" - } - ] + "condition": { + "negate": true, + "type": "reg", + "part": "request.attributes.ipRegion", + "pattern": "^中国\\|0\\|(上海|广东省)\\|.*" + } }, { "name": "AllowDeveloperOrIpRange", @@ -168,37 +158,35 @@ RBAC-based And Policy-based Multi-Tenant Reactive Security Framework. "type": "all" } ], - "conditions": [ - { - "type": "bool", - "bool": { - "and": [ - { - "type": "authenticated" - } - ], - "or": [ - { - "type": "in", - "part": "context.principal.id", - "in": [ - "developerId" - ] + "condition": { + "type": "bool", + "bool": { + "and": [ + { + "type": "authenticated" + } + ], + "or": [ + { + "type": "in", + "part": "context.principal.id", + "in": [ + "developerId" + ] + }, + { + "type": "path", + "part": "request.remoteIp", + "path": { + "caseSensitive": false, + "separator": ".", + "decodeAndParseSegments": false }, - { - "type": "path", - "part": "request.remoteIp", - "path": { - "caseSensitive": false, - "separator": ".", - "decodeAndParseSegments": false - }, - "pattern": "192.168.0.*" - } - ] - } + "pattern": "192.168.0.*" + } + ] } - ] + } } ] } diff --git a/README.zh-CN.md b/README.zh-CN.md index 5c0b9109..5f507d62 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -81,11 +81,9 @@ "pattern": "/user/#{principal.id}/*" } ], - "conditions": [ - { - "type": "authenticated" - } - ] + "condition": { + "type": "authenticated" + } }, { "name": "Developer", @@ -95,15 +93,13 @@ "type": "all" } ], - "conditions": [ - { - "type": "in", - "part": "context.principal.id", - "in": [ - "developerId" - ] - } - ] + "condition": { + "type": "in", + "part": "context.principal.id", + "in": [ + "developerId" + ] + } }, { "name": "RequestOriginDeny", @@ -113,14 +109,12 @@ "type": "all" } ], - "conditions": [ - { - "type": "reg", - "negate": true, - "part": "request.origin", - "pattern": "^(http|https)://github.com" - } - ] + "condition": { + "type": "reg", + "negate": true, + "part": "request.origin", + "pattern": "^(http|https)://github.com" + } }, { "name": "IpBlacklist", @@ -130,18 +124,16 @@ "type": "all" } ], - "conditions": [ - { - "type": "path", - "part": "request.remoteIp", - "path": { - "caseSensitive": false, - "separator": ".", - "decodeAndParseSegments": false - }, - "pattern": "192.168.0.*" - } - ] + "condition": { + "type": "path", + "part": "request.remoteIp", + "path": { + "caseSensitive": false, + "separator": ".", + "decodeAndParseSegments": false + }, + "pattern": "192.168.0.*" + } }, { "name": "RegionWhitelist", @@ -151,14 +143,12 @@ "type": "all" } ], - "conditions": [ - { - "negate": true, - "type": "reg", - "part": "request.attributes.ipRegion", - "pattern": "^中国\\|0\\|(上海|广东省)\\|.*" - } - ] + "condition": { + "negate": true, + "type": "reg", + "part": "request.attributes.ipRegion", + "pattern": "^中国\\|0\\|(上海|广东省)\\|.*" + } }, { "name": "AllowDeveloperOrIpRange", @@ -168,37 +158,35 @@ "type": "all" } ], - "conditions": [ - { - "type": "bool", - "bool": { - "and": [ - { - "type": "authenticated" - } - ], - "or": [ - { - "type": "in", - "part": "context.principal.id", - "in": [ - "developerId" - ] + "condition": { + "type": "bool", + "bool": { + "and": [ + { + "type": "authenticated" + } + ], + "or": [ + { + "type": "in", + "part": "context.principal.id", + "in": [ + "developerId" + ] + }, + { + "type": "path", + "part": "request.remoteIp", + "path": { + "caseSensitive": false, + "separator": ".", + "decodeAndParseSegments": false }, - { - "type": "path", - "part": "request.remoteIp", - "path": { - "caseSensitive": false, - "separator": ".", - "decodeAndParseSegments": false - }, - "pattern": "192.168.0.*" - } - ] - } + "pattern": "192.168.0.*" + } + ] } - ] + } } ] } diff --git a/cosec-api/src/main/kotlin/me/ahoo/cosec/api/policy/Statement.kt b/cosec-api/src/main/kotlin/me/ahoo/cosec/api/policy/Statement.kt index e3152193..497bbaa8 100644 --- a/cosec-api/src/main/kotlin/me/ahoo/cosec/api/policy/Statement.kt +++ b/cosec-api/src/main/kotlin/me/ahoo/cosec/api/policy/Statement.kt @@ -22,19 +22,11 @@ interface Statement : Named, PermissionVerifier { val effect: Effect val actions: List val condition: ConditionMatcher - val conditions: List override fun verify(request: Request, securityContext: SecurityContext): VerifyResult { if (!condition.match(request, securityContext)) { return VerifyResult.IMPLICIT_DENY } - conditions.any { - !it.match(request, securityContext) - }.let { anyNotMatched -> - if (anyNotMatched) { - return VerifyResult.IMPLICIT_DENY - } - } actions.any { it.match(request, securityContext) }.let { anyMatched -> diff --git a/cosec-core/src/main/kotlin/me/ahoo/cosec/policy/DefaultPolicyEvaluator.kt b/cosec-core/src/main/kotlin/me/ahoo/cosec/policy/DefaultPolicyEvaluator.kt index 318b6a50..c2854971 100644 --- a/cosec-core/src/main/kotlin/me/ahoo/cosec/policy/DefaultPolicyEvaluator.kt +++ b/cosec-core/src/main/kotlin/me/ahoo/cosec/policy/DefaultPolicyEvaluator.kt @@ -53,9 +53,7 @@ object DefaultPolicyEvaluator : PolicyEvaluator { it.match(mockRequest, mockContext) } - statement.conditions.forEach { - it.match(mockRequest, mockContext) - } + statement.condition.match(mockRequest, mockContext) } } } diff --git a/cosec-core/src/main/kotlin/me/ahoo/cosec/policy/StatementData.kt b/cosec-core/src/main/kotlin/me/ahoo/cosec/policy/StatementData.kt index 902a6f71..dec54236 100644 --- a/cosec-core/src/main/kotlin/me/ahoo/cosec/policy/StatementData.kt +++ b/cosec-core/src/main/kotlin/me/ahoo/cosec/policy/StatementData.kt @@ -23,6 +23,5 @@ data class StatementData( override val name: String = "", override val effect: Effect = Effect.ALLOW, override val actions: List = listOf(), - override val condition: ConditionMatcher = AllConditionMatcher.INSTANCE, - override val conditions: List = listOf(), + override val condition: ConditionMatcher = AllConditionMatcher.INSTANCE ) : Statement diff --git a/cosec-core/src/main/kotlin/me/ahoo/cosec/serialization/JsonStatementSerializer.kt b/cosec-core/src/main/kotlin/me/ahoo/cosec/serialization/JsonStatementSerializer.kt index 3f74e49b..becca8b7 100644 --- a/cosec-core/src/main/kotlin/me/ahoo/cosec/serialization/JsonStatementSerializer.kt +++ b/cosec-core/src/main/kotlin/me/ahoo/cosec/serialization/JsonStatementSerializer.kt @@ -31,7 +31,6 @@ const val STATEMENT_NAME = "name" const val STATEMENT_EFFECT_KEY = "effect" const val STATEMENT_ACTIONS_KEY = "actions" const val STATEMENT_CONDITION_KEY = "condition" -const val STATEMENT_CONDITIONS_KEY = "conditions" object JsonStatementSerializer : StdSerializer(Statement::class.java) { override fun serialize(value: Statement, gen: JsonGenerator, provider: SerializerProvider) { @@ -46,13 +45,6 @@ object JsonStatementSerializer : StdSerializer(Statement::class.java) gen.writeEndArray() } gen.writePOJOField(STATEMENT_CONDITION_KEY, value.condition) - if (value.conditions.isNotEmpty()) { - gen.writeArrayFieldStart(STATEMENT_CONDITIONS_KEY) - value.conditions.forEach { - gen.writeObject(it) - } - gen.writeEndArray() - } gen.writeEndObject() } } @@ -66,9 +58,6 @@ object JsonStatementDeserializer : StdDeserializer(Statement::class.j val condition = jsonNode.get(STATEMENT_CONDITION_KEY)?.traverse(p.codec)?.readValueAs(ConditionMatcher::class.java) ?: AllConditionMatcher.INSTANCE - val conditions = jsonNode.get(STATEMENT_CONDITIONS_KEY)?.map { - it.traverse(p.codec).readValueAs(ConditionMatcher::class.java) - }.orEmpty() return StatementData( name = jsonNode.get(STATEMENT_NAME)?.asText().orEmpty(), @@ -77,7 +66,6 @@ object JsonStatementDeserializer : StdDeserializer(Statement::class.j }.traverse(p.codec).readValueAs(Effect::class.java), actions = actions, condition = condition, - conditions = conditions ) } } diff --git a/cosec-core/src/test/kotlin/me/ahoo/cosec/policy/StatementDataTest.kt b/cosec-core/src/test/kotlin/me/ahoo/cosec/policy/StatementDataTest.kt index 23cf4a56..bede4070 100644 --- a/cosec-core/src/test/kotlin/me/ahoo/cosec/policy/StatementDataTest.kt +++ b/cosec-core/src/test/kotlin/me/ahoo/cosec/policy/StatementDataTest.kt @@ -23,6 +23,7 @@ import me.ahoo.cosec.configuration.JsonConfiguration import me.ahoo.cosec.configuration.JsonConfiguration.Companion.asConfiguration import me.ahoo.cosec.policy.action.AllActionMatcher import me.ahoo.cosec.policy.action.PathActionMatcherFactory +import me.ahoo.cosec.policy.condition.AllConditionMatcher import me.ahoo.cosec.policy.condition.SpelConditionMatcherFactory import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.* @@ -36,7 +37,7 @@ internal class StatementDataTest { assertThat(statementData.name, equalTo("")) assertThat(statementData.effect, equalTo(Effect.ALLOW)) assertThat(statementData.actions, equalTo(listOf())) - assertThat(statementData.conditions, equalTo(listOf())) + assertThat(statementData.condition, instanceOf(AllConditionMatcher::class.java)) assertThat(statementData.verify(mockk(), mockk()), `is`(VerifyResult.IMPLICIT_DENY)) } @@ -67,12 +68,10 @@ internal class StatementDataTest { ).asConfiguration() ) ), - conditions = listOf( - SpelConditionMatcherFactory().create( - mapOf( - MATCHER_PATTERN_KEY to "context.principal.authenticated()" - ).asConfiguration() - ) + condition = SpelConditionMatcherFactory().create( + mapOf( + MATCHER_PATTERN_KEY to "context.principal.authenticated()" + ).asConfiguration() ) ) val request = mockk { diff --git a/cosec-core/src/test/kotlin/me/ahoo/cosec/serialization/CoSecJsonSerializerTest.kt b/cosec-core/src/test/kotlin/me/ahoo/cosec/serialization/CoSecJsonSerializerTest.kt index b0b3e20e..9c68132c 100644 --- a/cosec-core/src/test/kotlin/me/ahoo/cosec/serialization/CoSecJsonSerializerTest.kt +++ b/cosec-core/src/test/kotlin/me/ahoo/cosec/serialization/CoSecJsonSerializerTest.kt @@ -30,6 +30,9 @@ import me.ahoo.cosec.policy.action.NoneActionMatcherFactory import me.ahoo.cosec.policy.action.PathActionMatcherFactory import me.ahoo.cosec.policy.action.RegularActionMatcherFactory import me.ahoo.cosec.policy.condition.AllConditionMatcherFactory +import me.ahoo.cosec.policy.condition.BOOL_CONDITION_MATCHER_AND_KEY +import me.ahoo.cosec.policy.condition.BoolConditionMatcher +import me.ahoo.cosec.policy.condition.BoolConditionMatcherFactory import me.ahoo.cosec.policy.condition.NoneConditionMatcherFactory import me.ahoo.cosec.policy.condition.OgnlConditionMatcherFactory import me.ahoo.cosec.policy.condition.SpelConditionMatcherFactory @@ -73,16 +76,17 @@ internal class CoSecJsonSerializerTest { assertThat(input.type, `is`(actionMatcher.type)) } - @ParameterizedTest - @MethodSource("serializeConditionMatcherProvider") - fun serializeConditionMatcher(conditionMatcher: ConditionMatcher) { + @Test + fun serializeConditionMatcher() { + val conditionMatcher = serializeConditionMatcherProvider() val output = CoSecJsonSerializer.writeValueAsString(conditionMatcher) val input = CoSecJsonSerializer.readValue( output, ConditionMatcher::class.java ) - assertThat(input, instanceOf(conditionMatcher.javaClass)) + assertThat(input, instanceOf(BoolConditionMatcher::class.java)) assertThat(input.type, `is`(conditionMatcher.type)) + assertThat((input as BoolConditionMatcher).and.size, equalTo(10)) } @ParameterizedTest @@ -96,7 +100,7 @@ internal class CoSecJsonSerializerTest { assertThat(input, instanceOf(statement.javaClass)) assertThat(input.effect, `is`(statement.effect)) assertThat(input.actions, hasSize(statement.actions.size)) - assertThat(input.conditions, hasSize(statement.conditions.size)) + assertThat(input.condition, notNullValue()) } @ParameterizedTest @@ -195,64 +199,71 @@ internal class CoSecJsonSerializerTest { } @JvmStatic - fun serializeConditionMatcherProvider(): Stream { - return Stream.of( - AllConditionMatcherFactory().create( - mapOf( - MATCHER_TYPE_KEY to AllConditionMatcherFactory.TYPE - ).asConfiguration() - ), - NoneConditionMatcherFactory().create( - mapOf( - MATCHER_TYPE_KEY to NoneConditionMatcherFactory.TYPE - ).asConfiguration() - ), - AuthenticatedConditionMatcherFactory().create( - mapOf( - MATCHER_TYPE_KEY to AuthenticatedConditionMatcherFactory.TYPE - ).asConfiguration() - ), - InDefaultTenantConditionMatcherFactory().create( - mapOf( - MATCHER_TYPE_KEY to InDefaultTenantConditionMatcherFactory.TYPE - ).asConfiguration() - ), - InPlatformTenantConditionMatcherFactory().create( - mapOf( - MATCHER_TYPE_KEY to InPlatformTenantConditionMatcherFactory.TYPE - ).asConfiguration() - ), - InUserTenantConditionMatcherFactory().create( - mapOf( - MATCHER_TYPE_KEY to InUserTenantConditionMatcherFactory.TYPE - ).asConfiguration() - ), - InConditionMatcherFactory().create( - mapOf( - MATCHER_TYPE_KEY to InConditionMatcherFactory.TYPE, - CONDITION_MATCHER_PART_KEY to RequestParts.REMOTE_IP, - "in" to setOf("remoteIp", "remoteIp1") - ).asConfiguration() - ), - RegularConditionMatcherFactory().create( - mapOf( - MATCHER_TYPE_KEY to RegularConditionMatcherFactory.TYPE, - CONDITION_MATCHER_PART_KEY to RequestParts.REMOTE_IP, - MATCHER_PATTERN_KEY to "192\\.168\\.0\\.[0-9]*" - ).asConfiguration() - ), - SpelConditionMatcherFactory().create( - mapOf( - MATCHER_TYPE_KEY to SpelConditionMatcherFactory.TYPE, - MATCHER_PATTERN_KEY to "context.principal.id=='1'" - ).asConfiguration() - ), - OgnlConditionMatcherFactory().create( - mapOf( - MATCHER_TYPE_KEY to OgnlConditionMatcherFactory.TYPE, - MATCHER_PATTERN_KEY to "path == \"auth/login\"" - ).asConfiguration() - ) + fun serializeConditionMatcherProvider(): ConditionMatcher { + return BoolConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to BoolConditionMatcherFactory.TYPE, + BoolConditionMatcherFactory.TYPE to mapOf( + BOOL_CONDITION_MATCHER_AND_KEY to listOf( + AllConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to AllConditionMatcherFactory.TYPE + ).asConfiguration() + ), + NoneConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to NoneConditionMatcherFactory.TYPE + ).asConfiguration() + ), + AuthenticatedConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to AuthenticatedConditionMatcherFactory.TYPE + ).asConfiguration() + ), + InDefaultTenantConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to InDefaultTenantConditionMatcherFactory.TYPE + ).asConfiguration() + ), + InPlatformTenantConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to InPlatformTenantConditionMatcherFactory.TYPE + ).asConfiguration() + ), + InUserTenantConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to InUserTenantConditionMatcherFactory.TYPE + ).asConfiguration() + ), + InConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to InConditionMatcherFactory.TYPE, + CONDITION_MATCHER_PART_KEY to RequestParts.REMOTE_IP, + "in" to setOf("remoteIp", "remoteIp1") + ).asConfiguration() + ), + RegularConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to RegularConditionMatcherFactory.TYPE, + CONDITION_MATCHER_PART_KEY to RequestParts.REMOTE_IP, + MATCHER_PATTERN_KEY to "192\\.168\\.0\\.[0-9]*" + ).asConfiguration() + ), + SpelConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to SpelConditionMatcherFactory.TYPE, + MATCHER_PATTERN_KEY to "context.principal.id=='1'" + ).asConfiguration() + ), + OgnlConditionMatcherFactory().create( + mapOf( + MATCHER_TYPE_KEY to OgnlConditionMatcherFactory.TYPE, + MATCHER_PATTERN_KEY to "path == \"auth/login\"" + ).asConfiguration() + ) + ) + ) + ).asConfiguration() ) } @@ -263,7 +274,7 @@ internal class CoSecJsonSerializerTest { StatementData( effect = Effect.DENY, actions = serializeActionMatcherProvider().toList(), - conditions = serializeConditionMatcherProvider().toList() + condition = serializeConditionMatcherProvider() ) ) } diff --git a/document/cosec-policy.schema.json b/document/cosec-policy.schema.json index 28e0fbd6..bdf86dd2 100644 --- a/document/cosec-policy.schema.json +++ b/document/cosec-policy.schema.json @@ -249,12 +249,6 @@ }, "condition": { "$ref": "#/definitions/conditionMatcher" - }, - "conditions": { - "type": "array", - "items": { - "$ref": "#/definitions/conditionMatcher" - } } } } diff --git a/document/design/assets/Modeling.svg b/document/design/assets/Modeling.svg index eb7c1713..c3212994 100644 --- a/document/design/assets/Modeling.svg +++ b/document/design/assets/Modeling.svg @@ -21,14 +21,14 @@ cluster authorization-->authenticationTenantval tenantId: Stringval isPlatformTenant: Booleanval isDefaultTenant: Booleanval isUserTenant: BooleanTenantCapableval tenant: TenantIs it a root platform tenant?PolicyTypeSYSTEMCUSTOMGLOBALPolicyval id: Stringval name: Stringval type: PolicyTypeval description: Stringval tenantId:Stringval statements: Set<Statement>PolicyTypeSYSTEMCUSTOMGLOBALPolicyval id: Stringval name: Stringval type: PolicyTypeval description: Stringval tenantId:Stringval statements: Set<Statement>EffectALLOWDENYRequestMatchermatch(Request,SecurityContext): BooleanActionMatcherConditionMatcherVerifyResultALLOWEXPLICIT_DENYIMPLICIT_DENYStatementval name: Stringval effect: Effectval actions: Set<ActionMatcher>val conditions: Set<ConditionMatcher>Statementval name: Stringval effect: Effectval actions: Set<ActionMatcher>val condition: ConditionMatcherPermissionVerifierverify(Request,SecurityContext): VerifyResultPolicyEvaluatorevaluate(Policy)PolicyCapableval policies: Set<String>Used to evaluate the effectiveness of the Policyhttps://github.com/Ahoo-Wang/CoSechttps://github.com/Ahoo-Wang/CoSec