From b6802de21d61cddd9065a2c64b420cc4fb16a81b Mon Sep 17 00:00:00 2001 From: Rory Date: Tue, 8 Oct 2024 16:14:24 +0800 Subject: [PATCH] address comment --- .../exceptions/IllegalPrivilegeException.java | 62 +++++ .../gravitino/client/ErrorHandlers.java | 13 +- .../gravitino/client/GravitinoClient.java | 11 +- .../gravitino/client/GravitinoMetalake.java | 11 +- .../test/authorization/AccessControlIT.java | 9 +- .../dto/responses/ErrorResponse.java | 17 +- .../authorization/AuthorizationUtils.java | 26 +- .../authorization/PermissionManager.java | 237 +++++++++++------- .../apache/gravitino/server/web/Utils.java | 8 +- .../mapper/JsonMappingExceptionMapper.java | 2 +- .../web/mapper/JsonParseExceptionMapper.java | 2 +- .../server/web/rest/RoleOperations.java | 1 + .../web/rest/TestPermissionOperations.java | 5 +- .../server/web/rest/TestRoleOperations.java | 3 +- 14 files changed, 280 insertions(+), 127 deletions(-) create mode 100644 api/src/main/java/org/apache/gravitino/exceptions/IllegalPrivilegeException.java diff --git a/api/src/main/java/org/apache/gravitino/exceptions/IllegalPrivilegeException.java b/api/src/main/java/org/apache/gravitino/exceptions/IllegalPrivilegeException.java new file mode 100644 index 00000000000..0fead4d964d --- /dev/null +++ b/api/src/main/java/org/apache/gravitino/exceptions/IllegalPrivilegeException.java @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.exceptions; + +import com.google.errorprone.annotations.FormatMethod; +import com.google.errorprone.annotations.FormatString; + +/** An exception thrown when a privilege is invalid. */ +public class IllegalPrivilegeException extends IllegalArgumentException { + /** + * Constructs a new exception with the specified detail message. + * + * @param message the detail message. + * @param args the arguments to the message. + */ + @FormatMethod + public IllegalPrivilegeException(@FormatString String message, Object... args) { + super(String.format(message, args)); + } + + /** + * Constructs a new exception with the specified detail message and cause. + * + * @param cause the cause. + * @param message the detail message. + * @param args the arguments to the message. + */ + @FormatMethod + public IllegalPrivilegeException(Throwable cause, @FormatString String message, Object... args) { + super(String.format(message, args), cause); + } + + /** + * Constructs a new exception with the specified cause. + * + * @param cause the cause. + */ + public IllegalPrivilegeException(Throwable cause) { + super(cause); + } + + /** Constructs a new exception with the specified detail message and cause. */ + public IllegalPrivilegeException() { + super(); + } +} diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java b/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java index 18ad7e55d95..5ab440092ad 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java @@ -31,6 +31,7 @@ import org.apache.gravitino.exceptions.ConnectionFailedException; import org.apache.gravitino.exceptions.FilesetAlreadyExistsException; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; +import org.apache.gravitino.exceptions.IllegalPrivilegeException; import org.apache.gravitino.exceptions.MetalakeAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchCatalogException; import org.apache.gravitino.exceptions.NoSuchFilesetException; @@ -626,7 +627,11 @@ public void accept(ErrorResponse errorResponse) { switch (errorResponse.getCode()) { case ErrorConstants.ILLEGAL_ARGUMENTS_CODE: - throw new IllegalArgumentException(errorMessage); + if (errorResponse.getType().equals(IllegalPrivilegeException.class.getSimpleName())) { + throw new IllegalPrivilegeException(errorMessage); + } else { + throw new IllegalArgumentException(errorMessage); + } case ErrorConstants.NOT_FOUND_CODE: if (errorResponse.getType().equals(NoSuchMetalakeException.class.getSimpleName())) { @@ -669,7 +674,11 @@ public void accept(ErrorResponse errorResponse) { switch (errorResponse.getCode()) { case ErrorConstants.ILLEGAL_ARGUMENTS_CODE: - throw new IllegalArgumentException(errorMessage); + if (errorResponse.getType().equals(IllegalPrivilegeException.class.getSimpleName())) { + throw new IllegalPrivilegeException(errorMessage); + } else { + throw new IllegalArgumentException(errorMessage); + } case ErrorConstants.NOT_FOUND_CODE: if (errorResponse.getType().equals(NoSuchMetalakeException.class.getSimpleName())) { diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java index 239d82ea906..217b8b35ab6 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java @@ -35,6 +35,7 @@ import org.apache.gravitino.authorization.User; import org.apache.gravitino.exceptions.CatalogAlreadyExistsException; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; +import org.apache.gravitino.exceptions.IllegalPrivilegeException; import org.apache.gravitino.exceptions.NoSuchCatalogException; import org.apache.gravitino.exceptions.NoSuchGroupException; import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; @@ -401,11 +402,12 @@ public String[] listRoleNames() throws NoSuchMetalakeException { * @throws NoSuchMetadataObjectException If the metadata object with the given name does not * exist. * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. - * @throws IllegalArgumentException If any privilege can't be bind to the metadata object. + * @throws IllegalPrivilegeException If any privilege can't be bind to the metadata object. * @throws RuntimeException If granting roles to a role encounters storage issues. */ public Role grantPrivilegesToRole(String role, MetadataObject object, List privileges) - throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException { + throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException, + IllegalPrivilegeException { return getMetalake().grantPrivilegesToRole(role, object, privileges); } @@ -420,12 +422,13 @@ public Role grantPrivilegesToRole(String role, MetadataObject object, List privileges) - throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException { + throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException, + IllegalPrivilegeException { return getMetalake().revokePrivilegesFromRole(role, object, privileges); } diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java index 1c5b871ebf9..46f75aa018b 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java @@ -78,6 +78,7 @@ import org.apache.gravitino.dto.responses.UserResponse; import org.apache.gravitino.exceptions.CatalogAlreadyExistsException; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; +import org.apache.gravitino.exceptions.IllegalPrivilegeException; import org.apache.gravitino.exceptions.NoSuchCatalogException; import org.apache.gravitino.exceptions.NoSuchGroupException; import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; @@ -908,11 +909,12 @@ public Group revokeRolesFromGroup(List roles, String group) * @throws NoSuchMetadataObjectException If the metadata object with the given name does not * exist. * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. - * @throws IllegalArgumentException If any privilege can't be bind to the metadata object. + * @throws IllegalPrivilegeException If any privilege can't be bind to the metadata object. * @throws RuntimeException If granting privileges to a role encounters storage issues. */ public Role grantPrivilegesToRole(String role, MetadataObject object, List privileges) - throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException { + throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException, + IllegalPrivilegeException { PrivilegeGrantRequest request = new PrivilegeGrantRequest(DTOConverters.toPrivileges(privileges)); request.validate(); @@ -948,12 +950,13 @@ public Role grantPrivilegesToRole(String role, MetadataObject object, List privileges) - throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException { + throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException, + IllegalPrivilegeException { PrivilegeRevokeRequest request = new PrivilegeRevokeRequest(DTOConverters.toPrivileges(privileges)); request.validate(); diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java index 7946f363ac4..b0f0c0c108d 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java @@ -42,6 +42,7 @@ import org.apache.gravitino.authorization.User; import org.apache.gravitino.client.GravitinoMetalake; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; +import org.apache.gravitino.exceptions.IllegalPrivilegeException; import org.apache.gravitino.exceptions.NoSuchGroupException; import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; import org.apache.gravitino.exceptions.NoSuchRoleException; @@ -453,7 +454,7 @@ void testManageRolePermissions() { // grant a wrong privilege MetadataObject catalog = MetadataObjects.of(null, "catalog", MetadataObject.Type.CATALOG); Assertions.assertThrows( - IllegalArgumentException.class, + IllegalPrivilegeException.class, () -> metalake.grantPrivilegesToRole( roleName, catalog, Lists.newArrayList(Privileges.CreateCatalog.allow()))); @@ -462,7 +463,7 @@ void testManageRolePermissions() { MetadataObject wrongCatalog = MetadataObjects.of(null, "fileset_catalog", MetadataObject.Type.CATALOG); Assertions.assertThrows( - IllegalArgumentException.class, + IllegalPrivilegeException.class, () -> metalake.grantPrivilegesToRole( roleName, wrongCatalog, Lists.newArrayList(Privileges.SelectTable.allow()))); @@ -488,14 +489,14 @@ void testManageRolePermissions() { // revoke a wrong privilege Assertions.assertThrows( - IllegalArgumentException.class, + IllegalPrivilegeException.class, () -> metalake.revokePrivilegesFromRole( roleName, catalog, Lists.newArrayList(Privileges.CreateCatalog.allow()))); // revoke a wrong catalog type privilege Assertions.assertThrows( - IllegalArgumentException.class, + IllegalPrivilegeException.class, () -> metalake.revokePrivilegesFromRole( roleName, wrongCatalog, Lists.newArrayList(Privileges.SelectTable.allow()))); diff --git a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java index f38fabc81b5..86e619bc786 100644 --- a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java +++ b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java @@ -119,11 +119,20 @@ public static ErrorResponse illegalArguments(String message) { * @return The new instance. */ public static ErrorResponse illegalArguments(String message, Throwable throwable) { + return illegalArguments(IllegalArgumentException.class.getSimpleName(), message, throwable); + } + + /** + * Create a new illegal arguments error instance of {@link ErrorResponse}. + * + * @param type The type of the error. + * @param message The message of the error. + * @param throwable The throwable that caused the error. + * @return The new instance. + */ + public static ErrorResponse illegalArguments(String type, String message, Throwable throwable) { return new ErrorResponse( - ErrorConstants.ILLEGAL_ARGUMENTS_CODE, - IllegalArgumentException.class.getSimpleName(), - message, - getStackTrace(throwable)); + ErrorConstants.ILLEGAL_ARGUMENTS_CODE, type, message, getStackTrace(throwable)); } /** diff --git a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java index 876def372f0..b8113ad2d40 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java @@ -22,6 +22,7 @@ import com.google.common.collect.Sets; import java.io.IOException; +import java.util.Collection; import java.util.List; import java.util.Set; import java.util.function.Consumer; @@ -38,6 +39,7 @@ import org.apache.gravitino.connector.authorization.AuthorizationPlugin; import org.apache.gravitino.dto.authorization.PrivilegeDTO; import org.apache.gravitino.dto.util.DTOConverters; +import org.apache.gravitino.exceptions.IllegalPrivilegeException; import org.apache.gravitino.exceptions.NoSuchCatalogException; import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; @@ -266,15 +268,25 @@ public static void checkSecurableObject(String metalake, MetadataObject object) } } + public static void checkDuplicatedNamePrivilege(Collection privileges) { + Set privilegeNameSet = Sets.newHashSet(); + for (Privilege privilege : privileges) { + if (privilegeNameSet.contains(privilege.name())) { + throw new IllegalPrivilegeException( + "Doesn't support duplicated privilege name %s", privilege.name()); + } + privilegeNameSet.add(privilege.name()); + } + } + public static void checkPrivilege( PrivilegeDTO privilegeDTO, MetadataObject object, String metalake) { Privilege privilege = DTOConverters.fromPrivilegeDTO(privilegeDTO); if (!privilege.canBindTo(object.type())) { - throw new IllegalArgumentException( - String.format( - "Securable object %s type %s don't support binding privilege %s", - object.fullName(), object.type(), privilege)); + throw new IllegalPrivilegeException( + "Securable object %s type %s don't support binding privilege %s", + object.fullName(), object.type(), privilege); } if (object.type() == MetadataObject.Type.CATALOG @@ -311,10 +323,8 @@ private static void checkCatalogType( NameIdentifier catalogIdent, Catalog.Type type, Privilege privilege) { Catalog catalog = GravitinoEnv.getInstance().catalogDispatcher().loadCatalog(catalogIdent); if (catalog.type() != type) { - throw new IllegalArgumentException( - String.format( - "Catalog %s type %s don't support privilege %s", - catalogIdent, catalog.type(), privilege)); + throw new IllegalPrivilegeException( + "Catalog %s type %s don't support privilege %s", catalogIdent, catalog.type(), privilege); } } diff --git a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java index 394dea1ed83..5c4836d830e 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java @@ -382,6 +382,7 @@ Role grantPrivilegesToRole( try { AuthorizationPluginCallbackWrapper authorizationPluginCallbackWrapper = new AuthorizationPluginCallbackWrapper(); + Role updatedRole = store.update( AuthorizationUtils.ofRole(metalake, role), @@ -392,57 +393,24 @@ Role grantPrivilegesToRole( updateSecurableObjects( roleEntity.securableObjects(), object, - oldObject -> { - if (oldObject == null) { - // Add a new securable object if there not exists the object in the role - SecurableObject securableObject = - SecurableObjects.parse( - object.fullName(), - object.type(), - Lists.newArrayList(privileges)); - - authorizationPluginCallbackWrapper.setCallBack( - () -> - AuthorizationUtils.callAuthorizationPluginForMetadataObject( - metalake, - object, - authorizationPlugin -> { - authorizationPlugin.onRoleUpdated( - roleEntity, - RoleChange.addSecurableObject(role, securableObject)); - })); - - return securableObject; + targetObject -> { + if (targetObject == null) { + return createNewSecurableObject( + metalake, + role, + object, + privileges, + roleEntity, + authorizationPluginCallbackWrapper); } else { - // Removed duplicated privileges by set - Set updatePrivileges = Sets.newHashSet(); - updatePrivileges.addAll(oldObject.privileges()); - // If old object contains all the privileges to grant, the object don't - // need to change. - if (updatePrivileges.containsAll(privileges)) { - return oldObject; - } else { - updatePrivileges.addAll(privileges); - SecurableObject newSecurableObject = - SecurableObjects.parse( - oldObject.fullName(), - oldObject.type(), - Lists.newArrayList(updatePrivileges)); - - authorizationPluginCallbackWrapper.setCallBack( - () -> - AuthorizationUtils.callAuthorizationPluginForMetadataObject( - metalake, - object, - authorizationPlugin -> { - authorizationPlugin.onRoleUpdated( - roleEntity, - RoleChange.updateSecurableObject( - role, oldObject, newSecurableObject)); - })); - - return newSecurableObject; - } + return updateGrantedSecurableObject( + metalake, + role, + object, + privileges, + roleEntity, + targetObject, + authorizationPluginCallbackWrapper); } }); @@ -464,6 +432,7 @@ Role grantPrivilegesToRole( .build(); }); + // Execute the authorization plugin callback authorizationPluginCallbackWrapper.execute(); return updatedRole; } catch (NoSuchEntityException nse) { @@ -474,6 +443,72 @@ Role grantPrivilegesToRole( } } + private static SecurableObject updateGrantedSecurableObject( + String metalake, + String role, + MetadataObject object, + List privileges, + RoleEntity roleEntity, + SecurableObject targetObject, + AuthorizationPluginCallbackWrapper authorizationPluginCallbackWrapper) { + // Removed duplicated privileges by set + Set updatePrivileges = Sets.newHashSet(); + updatePrivileges.addAll(targetObject.privileges()); + // If old object contains all the privileges to grant, the object don't + // need to change. + if (updatePrivileges.containsAll(privileges)) { + return targetObject; + } else { + updatePrivileges.addAll(privileges); + AuthorizationUtils.checkDuplicatedNamePrivilege(privileges); + + SecurableObject newSecurableObject = + SecurableObjects.parse( + targetObject.fullName(), targetObject.type(), Lists.newArrayList(updatePrivileges)); + + // We set authorization callback here, we won't execute this callback in this place. + // We will execute the callback after we execute the SQL transaction. + authorizationPluginCallbackWrapper.setCallBack( + () -> + AuthorizationUtils.callAuthorizationPluginForMetadataObject( + metalake, + object, + authorizationPlugin -> { + authorizationPlugin.onRoleUpdated( + roleEntity, + RoleChange.updateSecurableObject(role, targetObject, newSecurableObject)); + })); + + return newSecurableObject; + } + } + + private static SecurableObject createNewSecurableObject( + String metalake, + String role, + MetadataObject object, + List privileges, + RoleEntity roleEntity, + AuthorizationPluginCallbackWrapper authorizationPluginCallbackWrapper) { + // Add a new securable object if there not exists the object in the role + SecurableObject securableObject = + SecurableObjects.parse(object.fullName(), object.type(), Lists.newArrayList(privileges)); + + // We set authorization callback here, we won't execute this callback in this place. + // We will execute the callback after we execute the SQL transaction. + authorizationPluginCallbackWrapper.setCallBack( + () -> + AuthorizationUtils.callAuthorizationPluginForMetadataObject( + metalake, + object, + authorizationPlugin -> { + authorizationPlugin.onRoleUpdated( + roleEntity, RoleChange.addSecurableObject(role, securableObject)); + })); + + return securableObject; + } + Role revokePrivilegesFromRole( String metalake, String role, MetadataObject object, List privileges) { try { @@ -490,10 +525,10 @@ Role revokePrivilegesFromRole( updateSecurableObjects( roleEntity.securableObjects(), object, - oldObject -> { + targetObject -> { // If the securable object doesn't exist, we do nothing except for // logging. - if (oldObject == null) { + if (targetObject == null) { LOG.warn( "Securable object {} type {} doesn't exist in the role {}", object.fullName(), @@ -501,48 +536,14 @@ Role revokePrivilegesFromRole( role); return null; } else { - // If the securable object exists, we remove the privileges of the - // securable object. Remove duplicated privileges - Set updatePrivileges = Sets.newHashSet(); - updatePrivileges.addAll(oldObject.privileges()); - privileges.forEach(updatePrivileges::remove); - - // If the object still contains privilege, we should update the object - // with new privileges - if (!updatePrivileges.isEmpty()) { - SecurableObject newSecurableObject = - SecurableObjects.parse( - oldObject.fullName(), - oldObject.type(), - Lists.newArrayList(updatePrivileges)); - authorizationCallbackWrapper.setCallBack( - () -> - AuthorizationUtils.callAuthorizationPluginForMetadataObject( - metalake, - object, - authorizationPlugin -> { - authorizationPlugin.onRoleUpdated( - roleEntity, - RoleChange.updateSecurableObject( - role, oldObject, newSecurableObject)); - })); - - return newSecurableObject; - } else { - // If the object doesn't contain any privilege, we remove this object. - authorizationCallbackWrapper.setCallBack( - () -> - AuthorizationUtils.callAuthorizationPluginForMetadataObject( - metalake, - object, - authorizationPlugin -> { - authorizationPlugin.onRoleUpdated( - roleEntity, - RoleChange.removeSecurableObject(role, oldObject)); - })); - - return null; - } + return updateRevokedSecurableObject( + metalake, + role, + object, + privileges, + roleEntity, + targetObject, + authorizationCallbackWrapper); } }); @@ -575,6 +576,54 @@ Role revokePrivilegesFromRole( } } + private static SecurableObject updateRevokedSecurableObject( + String metalake, + String role, + MetadataObject object, + List privileges, + RoleEntity roleEntity, + SecurableObject targetObject, + AuthorizationPluginCallbackWrapper authorizationCallbackWrapper) { + // If the securable object exists, we remove the privileges of the + // securable object. Remove duplicated privileges + Set updatePrivileges = Sets.newHashSet(); + updatePrivileges.addAll(targetObject.privileges()); + privileges.forEach(updatePrivileges::remove); + + // If the object still contains privilege, we should update the object + // with new privileges + if (!updatePrivileges.isEmpty()) { + SecurableObject newSecurableObject = + SecurableObjects.parse( + targetObject.fullName(), targetObject.type(), Lists.newArrayList(updatePrivileges)); + authorizationCallbackWrapper.setCallBack( + () -> + AuthorizationUtils.callAuthorizationPluginForMetadataObject( + metalake, + object, + authorizationPlugin -> { + authorizationPlugin.onRoleUpdated( + roleEntity, + RoleChange.updateSecurableObject(role, targetObject, newSecurableObject)); + })); + + return newSecurableObject; + } else { + // If the object doesn't contain any privilege, we remove this object. + authorizationCallbackWrapper.setCallBack( + () -> + AuthorizationUtils.callAuthorizationPluginForMetadataObject( + metalake, + object, + authorizationPlugin -> { + authorizationPlugin.onRoleUpdated( + roleEntity, RoleChange.removeSecurableObject(role, targetObject)); + })); + + return null; + } + } + private List updateSecurableObjects( List securableObjects, MetadataObject targetObject, diff --git a/server/src/main/java/org/apache/gravitino/server/web/Utils.java b/server/src/main/java/org/apache/gravitino/server/web/Utils.java index 5a0ece3324d..7226eb363e3 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/Utils.java +++ b/server/src/main/java/org/apache/gravitino/server/web/Utils.java @@ -53,12 +53,16 @@ public static Response ok() { } public static Response illegalArguments(String message) { - return illegalArguments(message, null); + return illegalArguments(IllegalArgumentException.class.getSimpleName(), message, null); } public static Response illegalArguments(String message, Throwable throwable) { + return illegalArguments(throwable.getClass().getSimpleName(), message, throwable); + } + + public static Response illegalArguments(String type, String message, Throwable throwable) { return Response.status(Response.Status.BAD_REQUEST) - .entity(ErrorResponse.illegalArguments(message, throwable)) + .entity(ErrorResponse.illegalArguments(type, message, throwable)) .type(MediaType.APPLICATION_JSON) .build(); } diff --git a/server/src/main/java/org/apache/gravitino/server/web/mapper/JsonMappingExceptionMapper.java b/server/src/main/java/org/apache/gravitino/server/web/mapper/JsonMappingExceptionMapper.java index d2151385d04..df972d0ffa9 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/mapper/JsonMappingExceptionMapper.java +++ b/server/src/main/java/org/apache/gravitino/server/web/mapper/JsonMappingExceptionMapper.java @@ -34,6 +34,6 @@ public class JsonMappingExceptionMapper implements ExceptionMapper