Skip to content

[#9254,#9168] improvement(authz): Optimize metadata authorization in the list metalake and list role. #9255

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

Merged
merged 8 commits into from
Nov 27, 2025
Merged

[#9254,#9168] improvement(authz): Optimize metadata authorization in the list metalake and list role. #9255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
121784a
Optimize metadata authorization in the list metalake.
hdygxsj Nov 25, 2025
0c41108
fix ci
hdygxsj Nov 25, 2025
7e0aed5
Update server-common/src/main/java/org/apache/gravitino/server/author…
hdygxsj Nov 26, 2025
c4fd592
Update server-common/src/main/java/org/apache/gravitino/server/author…
hdygxsj Nov 26, 2025
4d5b3ac
fix comments
hdygxsj Nov 26, 2025
5471796
fix ai comments
hdygxsj Nov 26, 2025
be3f6f7
fix list role
hdygxsj Nov 27, 2025
8a877fd
Merge remote-tracking branch 'origin/impr-9254' into impr-9254
hdygxsj Nov 27, 2025
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
88 changes: 43 additions & 45 deletions ...r-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.apache.gravitino.Configs;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.Metalake;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.authorization.GravitinoAuthorizer;
Expand Down Expand Up @@ -88,6 +89,25 @@ public static NameIdentifier[] filterByPrivilege(
.toArray(NameIdentifier[]::new);
}

public static Metalake[] filterMetalakes(Metalake[] metalakes, String expression) {
Copy link
Contributor

@jerqi jerqi Nov 26, 2025

Choose a reason for hiding this comment

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

Why do we need to create a new method instead of reusing the method filterByExpression?

Copy link
Collaborator Author

@hdygxsj hdygxsj Nov 26, 2025

Choose a reason for hiding this comment

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

Because this method requires passing a metalake.

image

Copy link
Contributor

@jerqi jerqi Nov 26, 2025

Choose a reason for hiding this comment

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

We can extract common part and reuse them instead of producing duplicated code.

Copy link
Collaborator Author

@hdygxsj hdygxsj Nov 27, 2025

Choose a reason for hiding this comment

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

fixed

if (!enableAuthorization()) {
return metalakes;
}
checkExecutor();
AuthorizationRequestContext authorizationRequestContext = new AuthorizationRequestContext();
return doFilter(
expression,
metalakes,
GravitinoAuthorizerProvider.getInstance().getGravitinoAuthorizer(),
authorizationRequestContext,
metalake -> {
String metalakeName = metalake.name();
return splitMetadataNames(
metalakeName,
Entity.EntityType.METALAKE,
NameIdentifierUtil.ofMetalake(metalakeName));
});
}
/**
* Call {@link AuthorizationExpressionEvaluator} to filter the metadata list
*
Expand All @@ -102,41 +122,7 @@ public static NameIdentifier[] filterByExpression(
String expression,
Entity.EntityType entityType,
NameIdentifier[] nameIdentifiers) {
if (!enableAuthorization()) {
return nameIdentifiers;
}
checkExecutor();
AuthorizationRequestContext authorizationRequestContext = new AuthorizationRequestContext();
List<CompletableFuture<NameIdentifier>> futures = new ArrayList<>();
for (NameIdentifier nameIdentifier : nameIdentifiers) {
Principal currentPrincipal = PrincipalUtils.getCurrentPrincipal();
futures.add(
CompletableFuture.supplyAsync(
() -> {
try {
return PrincipalUtils.doAs(
currentPrincipal,
() -> {
Map<Entity.EntityType, NameIdentifier> nameIdentifierMap =
spiltMetadataNames(metalake, entityType, nameIdentifier);
AuthorizationExpressionEvaluator authorizationExpressionEvaluator =
new AuthorizationExpressionEvaluator(expression);
return authorizationExpressionEvaluator.evaluate(
nameIdentifierMap, authorizationRequestContext)
? nameIdentifier
: null;
});
} catch (Exception e) {
LOG.error("GravitinoAuthorize error:{}", e.getMessage(), e);
return null;
}
},
executor));
}
return futures.stream()
.map(CompletableFuture::join)
.filter(Objects::nonNull)
.toArray(NameIdentifier[]::new);
return filterByExpression(metalake, expression, entityType, nameIdentifiers, e -> e);
}

/**
Expand All @@ -151,7 +137,7 @@ public static boolean checkAccess(
NameIdentifier identifier, Entity.EntityType entityType, String expression) {
String metalake = NameIdentifierUtil.getMetalake(identifier);
Map<Entity.EntityType, NameIdentifier> nameIdentifierMap =
spiltMetadataNames(metalake, entityType, identifier);
splitMetadataNames(metalake, entityType, identifier);
AuthorizationExpressionEvaluator authorizationExpressionEvaluator =
new AuthorizationExpressionEvaluator(expression);
return authorizationExpressionEvaluator.evaluate(
Expand All @@ -177,9 +163,8 @@ public static <E> E[] filterByExpression(
Function<E, NameIdentifier> toNameIdentifier) {
GravitinoAuthorizer authorizer =
GravitinoAuthorizerProvider.getInstance().getGravitinoAuthorizer();
Principal currentPrincipal = PrincipalUtils.getCurrentPrincipal();
return filterByExpression(
metalake, expression, entityType, entities, toNameIdentifier, currentPrincipal, authorizer);
metalake, expression, entityType, entities, toNameIdentifier, authorizer);
}

/**
Expand All @@ -191,7 +176,6 @@ public static <E> E[] filterByExpression(
* @param entityType entity type
* @param entities metadata entities
* @param toNameIdentifier function to convert entity to NameIdentifier
* @param currentPrincipal current principal
* @param authorizer authorizer to filter metadata
* @return Filtered Metadata Entity
* @param <E> Entity class
Expand All @@ -202,13 +186,30 @@ public static <E> E[] filterByExpression(
Entity.EntityType entityType,
E[] entities,
Function<E, NameIdentifier> toNameIdentifier,
Principal currentPrincipal,
GravitinoAuthorizer authorizer) {
if (!enableAuthorization()) {
return entities;
}
checkExecutor();
AuthorizationRequestContext authorizationRequestContext = new AuthorizationRequestContext();
return doFilter(
expression,
entities,
authorizer,
authorizationRequestContext,
(entity) -> {
NameIdentifier nameIdentifier = toNameIdentifier.apply(entity);
return splitMetadataNames(metalake, entityType, nameIdentifier);
});
}

private static <E> E[] doFilter(
String expression,
E[] entities,
GravitinoAuthorizer authorizer,
AuthorizationRequestContext authorizationRequestContext,
Function<E, Map<Entity.EntityType, NameIdentifier>> extractMetadataNamesMap) {
Principal currentPrincipal = PrincipalUtils.getCurrentPrincipal();
List<CompletableFuture<E>> futures = new ArrayList<>();
for (E entity : entities) {
futures.add(
Expand All @@ -220,11 +221,8 @@ public static <E> E[] filterByExpression(
() -> {
AuthorizationExpressionEvaluator authorizationExpressionEvaluator =
new AuthorizationExpressionEvaluator(expression, authorizer);
NameIdentifier nameIdentifier = toNameIdentifier.apply(entity);
Map<Entity.EntityType, NameIdentifier> nameIdentifierMap =
spiltMetadataNames(metalake, entityType, nameIdentifier);
return authorizationExpressionEvaluator.evaluate(
nameIdentifierMap, authorizationRequestContext)
extractMetadataNamesMap.apply(entity), authorizationRequestContext)
? entity
: null;
});
Expand All @@ -251,7 +249,7 @@ public static <E> E[] filterByExpression(
* @param nameIdentifier metadata name
* @return A map containing the metadata object and all its parent objects, keyed by their types
*/
public static Map<Entity.EntityType, NameIdentifier> spiltMetadataNames(
public static Map<Entity.EntityType, NameIdentifier> splitMetadataNames(
String metalake, Entity.EntityType entityType, NameIdentifier nameIdentifier) {
Map<Entity.EntityType, NameIdentifier> nameIdentifierMap = new HashMap<>();
nameIdentifierMap.put(Entity.EntityType.METALAKE, NameIdentifierUtil.ofMetalake(metalake));
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public static Map<Entity.EntityType, NameIdentifier> extractNameIdentifierFromPa
NameIdentifier nameIdentifier =
MetadataObjectUtil.toEntityIdent(metalake, MetadataObjects.parse(fullName, type));
nameIdentifierMap.putAll(
MetadataAuthzHelper.spiltMetadataNames(
MetadataAuthzHelper.splitMetadataNames(
metalake, MetadataObjectUtil.toEntityType(type), nameIdentifier));
}

Expand Down
17 changes: 1 addition & 16 deletions server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,22 +89,7 @@ public Response listMetalakes() {
httpRequest,
() -> {
Metalake[] metalakes = metalakeDispatcher.listMetalakes();
metalakes =
Arrays.stream(metalakes)
.filter(
metalake -> {
NameIdentifier[] nameIdentifiers =
new NameIdentifier[] {NameIdentifierUtil.ofMetalake(metalake.name())};
return MetadataAuthzHelper.filterByExpression(
metalake.name(),
"METALAKE_USER",
Entity.EntityType.METALAKE,
nameIdentifiers)
.length
> 0;
})
.toList()
.toArray(new Metalake[0]);
metalakes = MetadataAuthzHelper.filterMetalakes(metalakes, "METALAKE_USER");
MetalakeDTO[] metalakeDTOs =
Arrays.stream(metalakes).map(DTOConverters::toDTO).toArray(MetalakeDTO[]::new);
Response response = Utils.ok(new MetalakeListResponse(metalakeDTOs));
Expand Down
23 changes: 6 additions & 17 deletions server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AccessControlDispatcher;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Privilege;
Expand Down Expand Up @@ -89,22 +88,12 @@ public Response listRoles(@PathParam("metalake") String metalake) {
() -> {
String[] names = accessControlManager.listRoleNames(metalake);
names =
Arrays.stream(names)
.filter(
role -> {
NameIdentifier[] nameIdentifiers =
new NameIdentifier[] {NameIdentifierUtil.ofRole(metalake, role)};
return MetadataAuthzHelper.filterByExpression(
metalake,
AuthorizationExpressionConstants
.loadRoleAuthorizationExpression,
Entity.EntityType.ROLE,
nameIdentifiers)
.length
> 0;
})
.collect(Collectors.toList())
.toArray(new String[0]);
MetadataAuthzHelper.filterByExpression(
metalake,
AuthorizationExpressionConstants.loadRoleAuthorizationExpression,
Entity.EntityType.ROLE,
names,
roleName -> NameIdentifierUtil.ofRole(metalake, roleName));
return Utils.ok(new NameListResponse(names));
});
} catch (Exception e) {
Expand Down