This is an automated email from the ASF dual-hosted git repository. jshao pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 9e77cc5c232d9f93b27caa566d7538a5be14b6f6 Author: yangyang zhong <[email protected]> AuthorDate: Mon Jul 7 14:16:44 2025 +0800 [#7576] improvement(authz): Improve the authorization expression (#7577) ### What changes were proposed in this pull request? Improve the authorization expression ### Why are the changes needed? close: #7576 ### Does this PR introduce _any_ user-facing change? None ### How was this patch tested? ``` org.apache.gravitino.client.integration.test.authorization.CatalogAuthorizationIT org.apache.gravitino.client.integration.test.authorization.SchemaAuthorizationIT org.apache.gravitino.server.authorization.expression.TestAuthorizationExpressionConverter ``` --- .../AuthorizationExpressionConverter.java | 40 ++++++++++++++++++++ .../TestAuthorizationExpressionConverter.java | 43 ++++++++++++++++++++++ .../server/web/rest/CatalogOperations.java | 13 +++---- .../server/web/rest/SchemaOperations.java | 20 +++++----- 4 files changed, 98 insertions(+), 18 deletions(-) diff --git a/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java b/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java index 858d9b4925..d1fbc22faf 100644 --- a/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java +++ b/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java @@ -32,6 +32,9 @@ public class AuthorizationExpressionConverter { /** Match authorization expressions */ public static final Pattern PATTERN = Pattern.compile("([A-Z_]+)::([A-Z_]+)"); + /** Match ANY expressions */ + public static final Pattern ANY_PATTERN = Pattern.compile("ANY\\(([^)]+)\\)"); + /** * The EXPRESSION_CACHE caches the result of converting authorization expressions into an OGNL * expression. @@ -51,6 +54,7 @@ public class AuthorizationExpressionConverter { * @return an OGNL expression used to call GravitinoAuthorizer */ public static String convertToOgnlExpression(String authorizationExpression) { + authorizationExpression = replaceAnyExpressions(authorizationExpression); return EXPRESSION_CACHE.computeIfAbsent( authorizationExpression, (expression) -> { @@ -78,4 +82,40 @@ public class AuthorizationExpressionConverter { return result.toString(); }); } + + /** + * Replaces any expression. For example, replace ANY(OWNER, METALAKE, CATALOG) to METALAKE::OWNER + * || CATALOG::OWNER. + * + * @param expression The original expression + * @return The modified expression + */ + public static String replaceAnyExpressions(String expression) { + Matcher matcher = ANY_PATTERN.matcher(expression); + StringBuffer result = new StringBuffer(); + + while (matcher.find()) { + String innerContent = matcher.group(1); + String[] parts = innerContent.split(","); + if (parts.length < 2) { + matcher.appendReplacement(result, Matcher.quoteReplacement(matcher.group(0))); + continue; + } + + String function = parts[0].trim(); + StringBuilder replacement = new StringBuilder(); + for (int i = 1; i < parts.length; i++) { + String scope = parts[i].trim(); + if (!scope.isEmpty()) { + if (replacement.length() > 0) { + replacement.append(" || "); + } + replacement.append(scope).append("::").append(function); + } + } + matcher.appendReplacement(result, replacement.toString()); + } + matcher.appendTail(result); + return result.toString(); + } } diff --git a/server-common/src/test/java/org/apache/gravitino/server/authorization/expression/TestAuthorizationExpressionConverter.java b/server-common/src/test/java/org/apache/gravitino/server/authorization/expression/TestAuthorizationExpressionConverter.java index 458c63336e..2bc9b6a51e 100644 --- a/server-common/src/test/java/org/apache/gravitino/server/authorization/expression/TestAuthorizationExpressionConverter.java +++ b/server-common/src/test/java/org/apache/gravitino/server/authorization/expression/TestAuthorizationExpressionConverter.java @@ -17,6 +17,7 @@ package org.apache.gravitino.server.authorization.expression; +import static org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConverter.ANY_PATTERN; import static org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConverter.PATTERN; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -36,6 +37,15 @@ public class TestAuthorizationExpressionConverter { assertTrue(PATTERN.matcher("CATALOG::CREATE_TABLE").matches()); } + @Test + public void testAnyPatter() { + assertFalse(ANY_PATTERN.matcher("ANY").matches()); + assertFalse(ANY_PATTERN.matcher("ANYOWNER,METALAKE,CATALOG").matches()); + assertFalse(ANY_PATTERN.matcher("ANY(OWNER,METALAKE,CATALOG").matches()); + assertTrue(ANY_PATTERN.matcher("ANY(OWNER,METALAKE,CATALOG)").matches()); + assertTrue(ANY_PATTERN.matcher("ANY(USE_CATALOG,METALAKE,CATALOG,SCHEMA)").matches()); + } + @Test public void testConvertToOgnlWithoutOwnerExpression() { String createTableAuthorizationExpression = "CATALOG::CREATE_TABLE || SCHEMA::CREATE_SCHEMA"; @@ -76,5 +86,38 @@ public class TestAuthorizationExpressionConverter { + "@org.apache.gravitino.authorization.Privilege$Name@CREATE_SCHEMA) " + "|| authorizer.isOwner(principal,METALAKE_NAME,SCHEMA)", createTableOgnlExpression); + + String expressionWithOwner2 = "(ANY(OWNER,METALAKE,CATALOG)) && CATALOG::USE_CATALOG)"; + String useCatalogOgnExpression = + AuthorizationExpressionConverter.convertToOgnlExpression(expressionWithOwner2); + Assertions.assertEquals( + "(authorizer.isOwner(principal,METALAKE_NAME,METALAKE) " + + "|| authorizer.isOwner(principal,METALAKE_NAME,CATALOG))" + + " && authorizer.authorize(principal,METALAKE_NAME,CATALOG" + + ",@org.apache.gravitino.authorization.Privilege$Name@USE_CATALOG))", + useCatalogOgnExpression); + } + + @Test + public void testReplaseAnyExpression() { + Assertions.assertEquals( + "METALAKE::USE_CATALOG || CATALOG::USE_CATALOG || CATALOG::OWNER", + AuthorizationExpressionConverter.replaceAnyExpressions( + "ANY(USE_CATALOG,METALAKE,CATALOG) || CATALOG::OWNER")); + + Assertions.assertEquals( + "(METALAKE::USE_CATALOG || CATALOG::USE_CATALOG) || CATALOG::OWNER", + AuthorizationExpressionConverter.replaceAnyExpressions( + "(ANY(USE_CATALOG,METALAKE,CATALOG)) || CATALOG::OWNER")); + + Assertions.assertEquals( + "METALAKE::OWNER || CATALOG::OWNER && CATALOG::OWNER", + AuthorizationExpressionConverter.replaceAnyExpressions( + "ANY(OWNER,METALAKE,CATALOG) && CATALOG::OWNER")); + + Assertions.assertEquals( + "(METALAKE::OWNER || CATALOG::OWNER) && CATALOG::USE_CATALOG", + AuthorizationExpressionConverter.replaceAnyExpressions( + "(ANY(OWNER,METALAKE,CATALOG)) && CATALOG::USE_CATALOG")); } } diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java index 367d3bfa36..25565856c8 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java @@ -109,8 +109,8 @@ public class CatalogOperations { }; return MetadataFilterHelper.filterByExpression( metalake, - "METALAKE::USE_CATALOG || CATALOG::USE_CATALOG " - + "|| METALAKE::OWNER || CATALOG::OWNER", + "ANY(USE_CATALOG,METALAKE,CATALOG) || " + + "ANY(OWNER,METALAKE,CATALOG)", Entity.EntityType.CATALOG, nameIdentifiers) .length @@ -126,8 +126,7 @@ public class CatalogOperations { idents = MetadataFilterHelper.filterByExpression( metalake, - "METALAKE::USE_CATALOG || CATALOG::USE_CATALOG " - + "|| METALAKE::OWNER || CATALOG::OWNER", + "ANY(USE_CATALOG,METALAKE,CATALOG) || " + "ANY(OWNER,METALAKE,CATALOG)", Entity.EntityType.CATALOG, idents); Response response = Utils.ok(new EntityListResponse(idents)); @@ -216,8 +215,7 @@ public class CatalogOperations { @Timed(name = "set-catalog." + MetricNames.HTTP_PROCESS_DURATION, absolute = true) @ResponseMetered(name = "set-catalog", absolute = true) @AuthorizationExpression( - expression = - "METALAKE::USE_CATALOG || CATALOG::USE_CATALOG || METALAKE::OWNER || CATALOG::OWNER", + expression = "ANY(USE_CATALOG,METALAKE,CATALOG) || ANY(OWNER,METALAKE,CATALOG)", accessMetadataType = MetadataObject.Type.CATALOG) public Response setCatalog( @PathParam("metalake") @AuthorizationMetadata(type = MetadataObject.Type.METALAKE) @@ -263,8 +261,7 @@ public class CatalogOperations { @Timed(name = "load-catalog." + MetricNames.HTTP_PROCESS_DURATION, absolute = true) @ResponseMetered(name = "load-catalog", absolute = true) @AuthorizationExpression( - expression = - "METALAKE::USE_CATALOG || CATALOG::USE_CATALOG || METALAKE::OWNER || CATALOG::OWNER", + expression = "ANY(USE_CATALOG,METALAKE,CATALOG) || ANY(OWNER,METALAKE,CATALOG)", accessMetadataType = MetadataObject.Type.CATALOG) public Response loadCatalog( @PathParam("metalake") @AuthorizationMetadata(type = MetadataObject.Type.METALAKE) diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java index 603b2e2c75..17b8fc97eb 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java @@ -91,9 +91,9 @@ public class SchemaOperations { idents = MetadataFilterHelper.filterByExpression( metalake, - " ((METALAKE::USE_CATALOG||CATALOG::USE_CATALOG) && " - + "(SCHEMA::OWNER || METALAKE::USE_SCHEMA || CATALOG::USE_SCHEMA || SCHEMA::USE_SCHEMA)) " - + "|| METALAKE::OWNER || CATALOG::OWNER", + " ((ANY(USE_CATALOG,METALAKE,CATALOG)) && " + + "(SCHEMA::OWNER || ANY(USE_SCHEMA,METALAKE,CATALOG,SCHEMA))) " + + "|| ANY(OWNER,METALAKE,CATALOG) ", Entity.EntityType.SCHEMA, idents); Response response = Utils.ok(new EntityListResponse(idents)); @@ -111,8 +111,8 @@ public class SchemaOperations { @ResponseMetered(name = "create-schema", absolute = true) @AuthorizationExpression( expression = - "( (METALAKE::USE_CATALOG || CATALOG::USE_CATALOG) && (METALAKE::CREATE_SCHEMA || CATALOG::CREATE_SCHEMA) )" - + "|| METALAKE::OWNER || CATALOG::OWNER", + "( (ANY(USE_CATALOG,METALAKE,CATALOG)) && (ANY(CREATE_SCHEMA,METALAKE,CATALOG)) ) " + + "|| ANY(OWNER,METALAKE,CATALOG)", accessMetadataType = MetadataObject.Type.CATALOG) public Response createSchema( @PathParam("metalake") @AuthorizationMetadata(type = MetadataObject.Type.METALAKE) @@ -148,9 +148,9 @@ public class SchemaOperations { @ResponseMetered(name = "load-schema", absolute = true) @AuthorizationExpression( expression = - "( (METALAKE::USE_CATALOG || CATALOG::USE_CATALOG) &&" - + " (METALAKE::USE_SCHEMA || CATALOG::USE_SCHEMA || SCHEMA::USE_SCHEMA || SCHEMA::OWNER) ) " - + " || METALAKE::OWNER || CATALOG::OWNER ", + "( (ANY(USE_CATALOG,METALAKE,CATALOG)) &&" + + " (ANY(USE_SCHEMA,METALAKE,CATALOG,SCHEMA) || SCHEMA::OWNER) ) " + + " || ANY(OWNER,METALAKE,CATALOG)", accessMetadataType = MetadataObject.Type.SCHEMA) public Response loadSchema( @PathParam("metalake") @AuthorizationMetadata(type = MetadataObject.Type.METALAKE) @@ -183,7 +183,7 @@ public class SchemaOperations { @ResponseMetered(name = "alter-schema", absolute = true) @AuthorizationExpression( expression = - "METALAKE::OWNER || CATALOG::OWNER || ((METALAKE::USE_CATALOG||CATALOG::USE_CATALOG) && SCHEMA::OWNER)", + "ANY(OWNER,METALAKE,CATALOG) || ((ANY(USE_CATALOG,METALAKE,CATALOG) && SCHEMA::OWNER))", accessMetadataType = MetadataObject.Type.SCHEMA) public Response alterSchema( @PathParam("metalake") @AuthorizationMetadata(type = MetadataObject.Type.METALAKE) @@ -221,7 +221,7 @@ public class SchemaOperations { @ResponseMetered(name = "drop-schema", absolute = true) @AuthorizationExpression( expression = - "METALAKE::OWNER || CATALOG::OWNER || ((METALAKE::USE_CATALOG||CATALOG::USE_CATALOG) && SCHEMA::OWNER)", + "ANY(OWNER,METALAKE,CATALOG) || ((ANY(USE_CATALOG,METALAKE,CATALOG) && SCHEMA::OWNER))", accessMetadataType = MetadataObject.Type.SCHEMA) public Response dropSchema( @PathParam("metalake") @AuthorizationMetadata(type = MetadataObject.Type.METALAKE)
