From 45f9c8d9e067229eb41d58853779b8bddb46308b Mon Sep 17 00:00:00 2001 From: roryqi Date: Wed, 9 Oct 2024 11:26:57 +0800 Subject: [PATCH] [#4903] feat(core,server): Support to grant or revoke privileges for a role (#5021) Supports to grant or revoke privileges for a role Fix: #4903 I will add the document later. Add the UT. --- .../authorization/SecurableObjects.java | 4 +- .../exceptions/IllegalPrivilegeException.java | 62 ++++ .../gravitino/client/DTOConverters.java | 25 +- .../gravitino/client/ErrorHandlers.java | 17 +- .../gravitino/client/GravitinoClient.java | 43 +++ .../gravitino/client/GravitinoMetalake.java | 87 +++++ .../gravitino/client/TestPermission.java | 117 ++++++- .../dto/requests/PrivilegeGrantRequest.java | 68 ++++ .../dto/requests/PrivilegeRevokeRequest.java | 68 ++++ .../dto/requests/RoleCreateRequest.java | 5 +- .../dto/responses/ErrorResponse.java | 17 +- .../AccessControlDispatcher.java | 30 ++ .../authorization/AccessControlManager.java | 15 + .../authorization/AuthorizationUtils.java | 32 +- .../authorization/PermissionManager.java | 301 +++++++++++++++++- .../hook/AccessControlHookDispatcher.java | 15 + .../storage/relational/JDBCBackend.java | 2 + .../relational/mapper/RoleMetaMapper.java | 4 + .../mapper/RoleMetaSQLProviderFactory.java | 5 + .../mapper/SecurableObjectMapper.java | 6 + .../SecurableObjectSQLProviderFactory.java | 5 + .../base/RoleMetaBaseSQLProvider.java | 19 ++ .../base/SecurableObjectBaseSQLProvider.java | 17 + .../relational/service/RoleMetaService.java | 90 ++++++ .../relational/utils/POConverters.java | 21 ++ ...estAccessControlManagerForPermissions.java | 116 +++++++ .../service/TestRoleMetaService.java | 176 ++++++++++ .../test/authorization/AccessControlIT.java | 143 +++++++++ .../apache/gravitino/server/web/Utils.java | 8 +- .../mapper/JsonMappingExceptionMapper.java | 2 +- .../web/mapper/JsonParseExceptionMapper.java | 2 +- .../server/web/rest/ExceptionHandlers.java | 18 ++ .../server/web/rest/PermissionOperations.java | 98 ++++++ .../server/web/rest/RoleOperations.java | 2 + .../web/rest/TestPermissionOperations.java | 172 ++++++++++ .../server/web/rest/TestRoleOperations.java | 3 +- 36 files changed, 1777 insertions(+), 38 deletions(-) create mode 100644 api/src/main/java/org/apache/gravitino/exceptions/IllegalPrivilegeException.java create mode 100644 common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeGrantRequest.java create mode 100644 common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeRevokeRequest.java diff --git a/api/src/main/java/org/apache/gravitino/authorization/SecurableObjects.java b/api/src/main/java/org/apache/gravitino/authorization/SecurableObjects.java index 2ec594bcd05..e6ca3a851c7 100644 --- a/api/src/main/java/org/apache/gravitino/authorization/SecurableObjects.java +++ b/api/src/main/java/org/apache/gravitino/authorization/SecurableObjects.java @@ -21,6 +21,7 @@ import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; +import com.google.common.collect.Sets; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; @@ -125,7 +126,8 @@ private static class SecurableObjectImpl extends MetadataObjectImpl implements S SecurableObjectImpl(String parent, String name, Type type, List privileges) { super(parent, name, type); - this.privileges = ImmutableList.copyOf(privileges); + // Remove duplicated privileges + this.privileges = ImmutableList.copyOf(Sets.newHashSet(privileges)); } @Override 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/DTOConverters.java b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java index 6b259405d7f..20aa1931984 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java @@ -20,11 +20,14 @@ import static org.apache.gravitino.dto.util.DTOConverters.toFunctionArg; +import java.util.List; +import java.util.stream.Collectors; import org.apache.gravitino.Catalog; import org.apache.gravitino.CatalogChange; import org.apache.gravitino.MetalakeChange; import org.apache.gravitino.Namespace; import org.apache.gravitino.SchemaChange; +import org.apache.gravitino.authorization.Privilege; import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.dto.AuditDTO; import org.apache.gravitino.dto.CatalogDTO; @@ -302,19 +305,21 @@ static SecurableObjectDTO toSecurableObject(SecurableObject securableObject) { return SecurableObjectDTO.builder() .withFullName(securableObject.fullName()) .withType(securableObject.type()) - .withPrivileges( - securableObject.privileges().stream() - .map( - privilege -> { - return PrivilegeDTO.builder() - .withCondition(privilege.condition()) - .withName(privilege.name()) - .build(); - }) - .toArray(PrivilegeDTO[]::new)) + .withPrivileges(toPrivileges(securableObject.privileges()).toArray(new PrivilegeDTO[0])) .build(); } + static List toPrivileges(List privileges) { + return privileges.stream() + .map( + privilege -> + PrivilegeDTO.builder() + .withCondition(privilege.condition()) + .withName(privilege.name()) + .build()) + .collect(Collectors.toList()); + } + static TagUpdateRequest toTagUpdateRequest(TagChange change) { if (change instanceof TagChange.RenameTag) { return new TagUpdateRequest.RenameTagRequest(((TagChange.RenameTag) change).getNewName()); 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 7414f4c4e88..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())) { @@ -680,6 +689,10 @@ public void accept(ErrorResponse errorResponse) { throw new NoSuchGroupException(errorMessage); } else if (errorResponse.getType().equals(NoSuchRoleException.class.getSimpleName())) { throw new NoSuchRoleException(errorMessage); + } else if (errorResponse + .getType() + .equals(NoSuchMetadataObjectException.class.getSimpleName())) { + throw new NoSuchMetadataObjectException(errorMessage); } else { throw new NotFoundException(errorMessage); } 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 a8a46ff8f47..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 @@ -29,11 +29,13 @@ import org.apache.gravitino.SupportsCatalogs; import org.apache.gravitino.authorization.Group; import org.apache.gravitino.authorization.Owner; +import org.apache.gravitino.authorization.Privilege; import org.apache.gravitino.authorization.Role; import org.apache.gravitino.authorization.SecurableObject; 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; @@ -389,6 +391,47 @@ public String[] listRoleNames() throws NoSuchMetalakeException { return getMetalake().listRoleNames(); } + /** + * Grant privileges to a role. + * + * @param role The name of the role. + * @param privileges The privileges to grant. + * @param object The object is associated with privileges to grant. + * @return The role after granted. + * @throws NoSuchRoleException If the role with the given name does not exist. + * @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 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, + IllegalPrivilegeException { + return getMetalake().grantPrivilegesToRole(role, object, privileges); + } + + /** + * Revoke privileges from a role. + * + * @param role The name of the role. + * @param privileges The privileges to revoke. + * @param object The object is associated with privileges to revoke. + * @return The role after revoked. + * @throws NoSuchRoleException If the role with the given name does not exist. + * @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 IllegalPrivilegeException If any privilege can't be bind to the metadata object. + * @throws RuntimeException If revoking privileges from a role encounters storage issues. + */ + public Role revokePrivilegesFromRole( + String role, MetadataObject object, List privileges) + throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException, + IllegalPrivilegeException { + return getMetalake().revokePrivilegesFromRole(role, object, privileges); + } + /** * Creates a new builder for constructing a GravitinoClient. * 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 4905681b7e5..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 @@ -37,6 +37,7 @@ import org.apache.gravitino.SupportsCatalogs; import org.apache.gravitino.authorization.Group; import org.apache.gravitino.authorization.Owner; +import org.apache.gravitino.authorization.Privilege; import org.apache.gravitino.authorization.Role; import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.authorization.SupportsRoles; @@ -49,6 +50,8 @@ import org.apache.gravitino.dto.requests.CatalogUpdatesRequest; import org.apache.gravitino.dto.requests.GroupAddRequest; import org.apache.gravitino.dto.requests.OwnerSetRequest; +import org.apache.gravitino.dto.requests.PrivilegeGrantRequest; +import org.apache.gravitino.dto.requests.PrivilegeRevokeRequest; import org.apache.gravitino.dto.requests.RoleCreateRequest; import org.apache.gravitino.dto.requests.RoleGrantRequest; import org.apache.gravitino.dto.requests.RoleRevokeRequest; @@ -75,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; @@ -894,6 +898,89 @@ public Group revokeRolesFromGroup(List roles, String group) return resp.getGroup(); } + /** + * Grant privileges to a role. + * + * @param role The name of the role. + * @param privileges The privileges to grant. + * @param object The object is associated with privileges to grant. + * @return The role after granted. + * @throws NoSuchRoleException If the role with the given name does not exist. + * @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 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, + IllegalPrivilegeException { + PrivilegeGrantRequest request = + new PrivilegeGrantRequest(DTOConverters.toPrivileges(privileges)); + request.validate(); + + RoleResponse resp = + restClient.put( + String.format( + API_PERMISSION_PATH, + this.name(), + String.format( + "roles/%s/%s/%s/grant", + RESTUtils.encodeString(role), + object.type().name().toLowerCase(Locale.ROOT), + object.fullName())), + request, + RoleResponse.class, + Collections.emptyMap(), + ErrorHandlers.permissionOperationErrorHandler()); + + resp.validate(); + + return resp.getRole(); + } + + /** + * Revoke privileges from a role. + * + * @param role The name of the role. + * @param privileges The privileges to revoke. + * @param object The object is associated with privileges to revoke. + * @return The role after revoked. + * @throws NoSuchRoleException If the role with the given name does not exist. + * @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 IllegalPrivilegeException If any privilege can't be bind to the metadata object. + * @throws RuntimeException If revoking privileges from a role encounters storage issues. + */ + public Role revokePrivilegesFromRole( + String role, MetadataObject object, List privileges) + throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException, + IllegalPrivilegeException { + PrivilegeRevokeRequest request = + new PrivilegeRevokeRequest(DTOConverters.toPrivileges(privileges)); + request.validate(); + + RoleResponse resp = + restClient.put( + String.format( + API_PERMISSION_PATH, + this.name(), + String.format( + "roles/%s/%s/%s/revoke", + RESTUtils.encodeString(role), + object.type().name().toLowerCase(Locale.ROOT), + object.fullName())), + request, + RoleResponse.class, + Collections.emptyMap(), + ErrorHandlers.permissionOperationErrorHandler()); + + resp.validate(); + + return resp.getRole(); + } + /** * Get the owner of a metadata object. * diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/TestPermission.java b/clients/client-java/src/test/java/org/apache/gravitino/client/TestPermission.java index 70912e9b591..006a47b43ca 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/TestPermission.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/TestPermission.java @@ -24,17 +24,28 @@ import com.google.common.collect.Lists; import java.time.Instant; import java.util.List; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.MetadataObjects; import org.apache.gravitino.authorization.Group; +import org.apache.gravitino.authorization.Privilege; +import org.apache.gravitino.authorization.Privileges; +import org.apache.gravitino.authorization.Role; +import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.authorization.User; import org.apache.gravitino.dto.AuditDTO; import org.apache.gravitino.dto.MetalakeDTO; import org.apache.gravitino.dto.authorization.GroupDTO; +import org.apache.gravitino.dto.authorization.PrivilegeDTO; +import org.apache.gravitino.dto.authorization.RoleDTO; +import org.apache.gravitino.dto.authorization.SecurableObjectDTO; import org.apache.gravitino.dto.authorization.UserDTO; +import org.apache.gravitino.dto.requests.PrivilegeRevokeRequest; import org.apache.gravitino.dto.requests.RoleGrantRequest; import org.apache.gravitino.dto.requests.RoleRevokeRequest; import org.apache.gravitino.dto.responses.ErrorResponse; import org.apache.gravitino.dto.responses.GroupResponse; import org.apache.gravitino.dto.responses.MetalakeResponse; +import org.apache.gravitino.dto.responses.RoleResponse; import org.apache.gravitino.dto.responses.UserResponse; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.Method; @@ -174,8 +185,112 @@ public void testRevokeRoleFromGroup() throws Exception { // test Exception ErrorResponse errResp = ErrorResponse.internalError("internal error"); - buildMockResource(Method.DELETE, groupPath, null, errResp, SC_SERVER_ERROR); + buildMockResource(Method.PUT, groupPath, null, errResp, SC_SERVER_ERROR); Assertions.assertThrows( RuntimeException.class, () -> gravitinoClient.revokeRolesFromGroup(roles, group)); } + + @Test + public void testGrantPrivilegeToRole() throws Exception { + String role = "role"; + String rolePath = + String.format( + API_PERMISSION_PATH, + metalakeName, + String.format("roles/%s/%s/%s/grant", role, "metalake", metalakeName)); + RoleDTO roleDTO = + RoleDTO.builder() + .withName(role) + .withSecurableObjects( + new SecurableObjectDTO[] { + SecurableObjectDTO.builder() + .withFullName(metalakeName) + .withType(MetadataObject.Type.METALAKE) + .withPrivileges( + new PrivilegeDTO[] { + PrivilegeDTO.builder() + .withName(Privilege.Name.CREATE_TABLE) + .withCondition(Privilege.Condition.ALLOW) + .build() + }) + .build() + }) + .withAudit(AuditDTO.builder().withCreator("test").withCreateTime(Instant.now()).build()) + .build(); + RoleResponse response = new RoleResponse(roleDTO); + + PrivilegeRevokeRequest request = + new PrivilegeRevokeRequest( + Lists.newArrayList( + PrivilegeDTO.builder() + .withName(Privilege.Name.CREATE_TABLE) + .withCondition(Privilege.Condition.ALLOW) + .build())); + + buildMockResource(Method.PUT, rolePath, request, response, SC_OK); + MetadataObject object = MetadataObjects.of(null, metalakeName, MetadataObject.Type.METALAKE); + Role grantedRole = + gravitinoClient.grantPrivilegesToRole( + role, object, Lists.newArrayList(Privileges.CreateTable.allow())); + Assertions.assertEquals(grantedRole.name(), role); + Assertions.assertEquals(1, grantedRole.securableObjects().size()); + SecurableObject securableObject = grantedRole.securableObjects().get(0); + Assertions.assertEquals(metalakeName, securableObject.name()); + Assertions.assertEquals(MetadataObject.Type.METALAKE, securableObject.type()); + Assertions.assertEquals( + Privilege.Name.CREATE_TABLE, securableObject.privileges().get(0).name()); + Assertions.assertEquals( + Privilege.Condition.ALLOW, securableObject.privileges().get(0).condition()); + + // test Exception + ErrorResponse errResp = ErrorResponse.internalError("internal error"); + buildMockResource(Method.PUT, rolePath, null, errResp, SC_SERVER_ERROR); + Assertions.assertThrows( + RuntimeException.class, + () -> + gravitinoClient.grantPrivilegesToRole( + role, object, Lists.newArrayList(Privileges.CreateTable.allow()))); + } + + @Test + public void testRevokePrivilegeFromRole() throws Exception { + String role = "role"; + String rolePath = + String.format( + API_PERMISSION_PATH, + metalakeName, + String.format("roles/%s/%s/%s/revoke", role, "metalake", metalakeName)); + RoleDTO roleDTO = + RoleDTO.builder() + .withName(role) + .withAudit(AuditDTO.builder().withCreator("test").withCreateTime(Instant.now()).build()) + .withSecurableObjects(new SecurableObjectDTO[0]) + .build(); + RoleResponse response = new RoleResponse(roleDTO); + + PrivilegeRevokeRequest request = + new PrivilegeRevokeRequest( + Lists.newArrayList( + PrivilegeDTO.builder() + .withName(Privilege.Name.CREATE_TABLE) + .withCondition(Privilege.Condition.ALLOW) + .build())); + + buildMockResource(Method.PUT, rolePath, request, response, SC_OK); + MetadataObject object = MetadataObjects.of(null, metalakeName, MetadataObject.Type.METALAKE); + Role revokedRole = + gravitinoClient.revokePrivilegesFromRole( + role, object, Lists.newArrayList(Privileges.CreateTable.allow())); + Assertions.assertEquals(revokedRole.name(), role); + Assertions.assertTrue(revokedRole.securableObjects().isEmpty()); + + // test Exception + ErrorResponse errResp = ErrorResponse.internalError("internal error"); + buildMockResource(Method.PUT, rolePath, null, errResp, SC_SERVER_ERROR); + Assertions.assertThrows( + RuntimeException.class, + () -> + gravitinoClient.revokePrivilegesFromRole( + role, object, Lists.newArrayList(Privileges.CreateTable.allow()))); + } } diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeGrantRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeGrantRequest.java new file mode 100644 index 00000000000..26122ae51d7 --- /dev/null +++ b/common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeGrantRequest.java @@ -0,0 +1,68 @@ +/* + * 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.dto.requests; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; +import java.util.List; +import lombok.Builder; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import lombok.extern.jackson.Jacksonized; +import org.apache.gravitino.dto.authorization.PrivilegeDTO; +import org.apache.gravitino.rest.RESTRequest; + +/** Request to grant a privilege to a role. */ +@Getter +@EqualsAndHashCode +@ToString +@Builder +@Jacksonized +public class PrivilegeGrantRequest implements RESTRequest { + + @JsonProperty("privileges") + private final List privileges; + + /** + * Constructor for privilegeGrantRequest. + * + * @param privileges The privileges for the PrivilegeGrantRequest. + */ + public PrivilegeGrantRequest(List privileges) { + this.privileges = privileges; + } + + /** Default constructor for PrivilegeGrantRequest. */ + public PrivilegeGrantRequest() { + this(null); + } + + /** + * Validates the fields of the request. + * + * @throws IllegalArgumentException if the privileges field is not set or empty. + */ + @Override + public void validate() throws IllegalArgumentException { + Preconditions.checkArgument( + privileges != null && !privileges.isEmpty(), + "\"privileges\" field is required and cannot be empty"); + } +} diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeRevokeRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeRevokeRequest.java new file mode 100644 index 00000000000..10da859d0cb --- /dev/null +++ b/common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeRevokeRequest.java @@ -0,0 +1,68 @@ +/* + * 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.dto.requests; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; +import java.util.List; +import lombok.Builder; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import lombok.extern.jackson.Jacksonized; +import org.apache.gravitino.dto.authorization.PrivilegeDTO; +import org.apache.gravitino.rest.RESTRequest; + +/** Request to revoke a privilege from a role. */ +@Getter +@EqualsAndHashCode +@ToString +@Builder +@Jacksonized +public class PrivilegeRevokeRequest implements RESTRequest { + + @JsonProperty("privileges") + private final List privileges; + + /** + * Constructor for privilegeRevokeRequest. + * + * @param privileges The privileges for the PrivilegeRevokeRequest. + */ + public PrivilegeRevokeRequest(List privileges) { + this.privileges = privileges; + } + + /** Default constructor for PrivilegeRevokeRequest. */ + public PrivilegeRevokeRequest() { + this(null); + } + + /** + * Validates the fields of the request. + * + * @throws IllegalArgumentException if the privileges field is not set or empty. + */ + @Override + public void validate() throws IllegalArgumentException { + Preconditions.checkArgument( + privileges != null && !privileges.isEmpty(), + "\"privileges\" field is required and cannot be empty"); + } +} diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/RoleCreateRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/RoleCreateRequest.java index b4c3f24aee7..9d85c0c6e14 100644 --- a/common/src/main/java/org/apache/gravitino/dto/requests/RoleCreateRequest.java +++ b/common/src/main/java/org/apache/gravitino/dto/requests/RoleCreateRequest.java @@ -78,9 +78,6 @@ public RoleCreateRequest( public void validate() throws IllegalArgumentException { Preconditions.checkArgument( StringUtils.isNotBlank(name), "\"name\" field is required and cannot be empty"); - - Preconditions.checkArgument( - securableObjects != null && securableObjects.length != 0, - "\"securableObjects\" can't null or empty"); + Preconditions.checkArgument(securableObjects != null, "\"securableObjects\" can't null "); } } 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/AccessControlDispatcher.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java index a3919512b87..73004280b7e 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java @@ -278,4 +278,34 @@ Role createRole( */ String[] listRoleNamesByObject(String metalake, MetadataObject object) throws NoSuchMetalakeException, NoSuchMetadataObjectException; + + /** + * Grant privileges to a role. + * + * @param role The name of the role. + * @param privileges The privileges to grant. + * @param object The object is associated with privileges to grant. + * @return The role after granted. + * @throws NoSuchRoleException If the role with the given name does not exist. + * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. + * @throws RuntimeException If granting roles to a role encounters storage issues. + */ + Role grantPrivilegeToRole( + String metalake, String role, MetadataObject object, List privileges) + throws NoSuchGroupException, NoSuchRoleException; + + /** + * Revoke privileges from a role. + * + * @param role The name of the role. + * @param privileges The privileges to revoke. + * @param object The object is associated with privileges to revoke. + * @return The role after revoked. + * @throws NoSuchRoleException If the role with the given name does not exist. + * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. + * @throws RuntimeException If revoking privileges from a role encounters storage issues. + */ + Role revokePrivilegesFromRole( + String metalake, String role, MetadataObject object, List privileges) + throws NoSuchMetalakeException, NoSuchRoleException; } diff --git a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java index 75b0f9f1e53..c9adf314a87 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java @@ -150,6 +150,7 @@ public Role getRole(String metalake, String role) return roleManager.getRole(metalake, role); } + @Override public boolean deleteRole(String metalake, String role) throws NoSuchMetalakeException { return roleManager.deleteRole(metalake, role); } @@ -164,4 +165,18 @@ public String[] listRoleNamesByObject(String metalake, MetadataObject object) throws NoSuchMetalakeException, NoSuchMetadataObjectException { return roleManager.listRoleNamesByObject(metalake, object); } + + @Override + public Role grantPrivilegeToRole( + String metalake, String role, MetadataObject object, List privileges) + throws NoSuchRoleException, NoSuchMetalakeException { + return permissionManager.grantPrivilegesToRole(metalake, role, object, privileges); + } + + @Override + public Role revokePrivilegesFromRole( + String metalake, String role, MetadataObject object, List privileges) + throws NoSuchRoleException, NoSuchMetalakeException { + return permissionManager.revokePrivilegesFromRole(metalake, role, object, privileges); + } } 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 5a4a62cd60e..66019ee0403 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; @@ -54,6 +56,7 @@ public class AuthorizationUtils { static final String ROLE_DOES_NOT_EXIST_MSG = "Role %s does not exist in th metalake %s"; private static final Logger LOG = LoggerFactory.getLogger(AuthorizationUtils.class); private static final String METALAKE_DOES_NOT_EXIST_MSG = "Metalake %s does not exist"; + private static final Set FILESET_PRIVILEGES = Sets.immutableEnumSet( Privilege.Name.CREATE_FILESET, Privilege.Name.WRITE_FILESET, Privilege.Name.READ_FILESET); @@ -220,7 +223,7 @@ public static void checkSecurableObject(String metalake, MetadataObject object) Supplier exceptionToThrowSupplier = () -> new NoSuchMetadataObjectException( - "Securable object %s doesn't exist", object.fullName()); + "Securable object %s type %s doesn't exist", object.fullName(), object.type()); switch (object.type()) { case METALAKE: @@ -265,14 +268,26 @@ 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 with different condition", + 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 @@ -309,10 +324,9 @@ 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 doesn'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 edb02cdcec5..056b18f4045 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java @@ -19,6 +19,7 @@ package org.apache.gravitino.authorization; import static org.apache.gravitino.authorization.AuthorizationUtils.GROUP_DOES_NOT_EXIST_MSG; +import static org.apache.gravitino.authorization.AuthorizationUtils.ROLE_DOES_NOT_EXIST_MSG; import static org.apache.gravitino.authorization.AuthorizationUtils.USER_DOES_NOT_EXIST_MSG; import com.google.common.collect.Lists; @@ -26,12 +27,15 @@ import java.time.Instant; import java.util.List; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityStore; +import org.apache.gravitino.MetadataObject; import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NoSuchGroupException; +import org.apache.gravitino.exceptions.NoSuchRoleException; import org.apache.gravitino.exceptions.NoSuchUserException; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.GroupEntity; @@ -373,11 +377,304 @@ User revokeRolesFromUser(String metalake, List roles, String user) { } } - private List toRoleNames(List roleEntities) { - return roleEntities.stream().map(RoleEntity::name).collect(Collectors.toList()); + Role grantPrivilegesToRole( + String metalake, String role, MetadataObject object, List privileges) { + try { + AuthorizationPluginCallbackWrapper authorizationPluginCallbackWrapper = + new AuthorizationPluginCallbackWrapper(); + + Role updatedRole = + store.update( + AuthorizationUtils.ofRole(metalake, role), + RoleEntity.class, + Entity.EntityType.ROLE, + roleEntity -> { + List grantedSecurableObjects = + generateNewSecurableObjects( + roleEntity.securableObjects(), + object, + targetObject -> { + if (targetObject == null) { + return createNewSecurableObject( + metalake, + role, + object, + privileges, + roleEntity, + authorizationPluginCallbackWrapper); + } else { + return updateGrantedSecurableObject( + metalake, + role, + object, + privileges, + roleEntity, + targetObject, + authorizationPluginCallbackWrapper); + } + }); + + AuditInfo auditInfo = + AuditInfo.builder() + .withCreator(roleEntity.auditInfo().creator()) + .withCreateTime(roleEntity.auditInfo().createTime()) + .withLastModifier(PrincipalUtils.getCurrentPrincipal().getName()) + .withLastModifiedTime(Instant.now()) + .build(); + + return RoleEntity.builder() + .withId(roleEntity.id()) + .withName(roleEntity.name()) + .withNamespace(roleEntity.namespace()) + .withProperties(roleEntity.properties()) + .withAuditInfo(auditInfo) + .withSecurableObjects(grantedSecurableObjects) + .build(); + }); + + // Execute the authorization plugin callback + authorizationPluginCallbackWrapper.execute(); + return updatedRole; + } catch (NoSuchEntityException nse) { + LOG.error("Failed to grant, role {} does not exist in the metalake {}", role, metalake, nse); + throw new NoSuchRoleException(ROLE_DOES_NOT_EXIST_MSG, role, metalake); + } catch (IOException ioe) { + LOG.error("Grant privileges to {} failed due to storage issues", role, ioe); + throw new RuntimeException(ioe); + } + } + + 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 doesn't + // need to update. + 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; + } + } + + Role revokePrivilegesFromRole( + String metalake, String role, MetadataObject object, List privileges) { + try { + AuthorizationPluginCallbackWrapper authorizationCallbackWrapper = + new AuthorizationPluginCallbackWrapper(); + + RoleEntity updatedRole = + store.update( + AuthorizationUtils.ofRole(metalake, role), + RoleEntity.class, + Entity.EntityType.ROLE, + roleEntity -> { + List revokedSecurableObjects = + generateNewSecurableObjects( + roleEntity.securableObjects(), + object, + targetObject -> { + // If the securable object doesn't exist, we do nothing except for + // logging. + if (targetObject == null) { + LOG.warn( + "Securable object {} type {} doesn't exist in the role {}", + object.fullName(), + object.type(), + role); + return null; + } else { + // If the securable object exists, we remove the privileges of the + // securable object and will remove duplicated privileges at the same + // time. + return updateRevokedSecurableObject( + metalake, + role, + object, + privileges, + roleEntity, + targetObject, + authorizationCallbackWrapper); + } + }); + + AuditInfo auditInfo = + AuditInfo.builder() + .withCreator(roleEntity.auditInfo().creator()) + .withCreateTime(roleEntity.auditInfo().createTime()) + .withLastModifier(PrincipalUtils.getCurrentPrincipal().getName()) + .withLastModifiedTime(Instant.now()) + .build(); + + return RoleEntity.builder() + .withId(roleEntity.id()) + .withName(roleEntity.name()) + .withNamespace(roleEntity.namespace()) + .withProperties(roleEntity.properties()) + .withAuditInfo(auditInfo) + .withSecurableObjects(revokedSecurableObjects) + .build(); + }); + + authorizationCallbackWrapper.execute(); + + return updatedRole; + } catch (NoSuchEntityException nse) { + LOG.error("Failed to revoke, role {} does not exist in the metalake {}", role, metalake, nse); + throw new NoSuchRoleException(ROLE_DOES_NOT_EXIST_MSG, role, metalake); + } catch (IOException ioe) { + LOG.error("Revoke privileges from {} failed due to storage issues", role, ioe); + throw new RuntimeException(ioe); + } + } + + private static SecurableObject createNewSecurableObject( + String metalake, + String role, + MetadataObject object, + List privileges, + RoleEntity roleEntity, + AuthorizationPluginCallbackWrapper authorizationPluginCallbackWrapper) { + // Add a new securable object if there doesn't exist 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; + } + + private static SecurableObject updateRevokedSecurableObject( + String metalake, + String role, + MetadataObject object, + List privileges, + RoleEntity roleEntity, + SecurableObject targetObject, + AuthorizationPluginCallbackWrapper authorizationCallbackWrapper) { + // Use set to deduplicate the 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)); + + // 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. + 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. + // 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. + authorizationCallbackWrapper.setCallback( + () -> + AuthorizationUtils.callAuthorizationPluginForMetadataObject( + metalake, + object, + authorizationPlugin -> { + authorizationPlugin.onRoleUpdated( + roleEntity, RoleChange.removeSecurableObject(role, targetObject)); + })); + // If we return null, the newly generated objects won't contain this object, the storage will + // delete this object. + return null; + } + } + + // This method will generate all the securable objects after granting or revoking privileges + private List generateNewSecurableObjects( + List securableObjects, + MetadataObject targetObject, + Function objectUpdater) { + + // Find a matched securable object to update privileges + List updateSecurableObjects = Lists.newArrayList(securableObjects); + SecurableObject matchedSecurableObject = + securableObjects.stream().filter(targetObject::equals).findFirst().orElse(null); + if (matchedSecurableObject != null) { + updateSecurableObjects.remove(matchedSecurableObject); + } + + // Apply the updates for the matched object + SecurableObject newSecurableObject = objectUpdater.apply(matchedSecurableObject); + if (newSecurableObject != null) { + updateSecurableObjects.add(newSecurableObject); + } + return updateSecurableObjects; + } + + private static class AuthorizationPluginCallbackWrapper { + private Runnable callback; + + public void setCallback(Runnable callback) { + this.callback = callback; + } + + public void execute() { + if (callback != null) { + callback.run(); + } + } } private List toRoleIds(List roleEntities) { return roleEntities.stream().map(RoleEntity::id).collect(Collectors.toList()); } + + private List toRoleNames(List roleEntities) { + return roleEntities.stream().map(RoleEntity::name).collect(Collectors.toList()); + } } diff --git a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java index e16974764b1..36f75cfad15 100644 --- a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java @@ -28,6 +28,7 @@ import org.apache.gravitino.authorization.Group; import org.apache.gravitino.authorization.Owner; import org.apache.gravitino.authorization.OwnerManager; +import org.apache.gravitino.authorization.Privilege; import org.apache.gravitino.authorization.Role; import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.authorization.User; @@ -180,4 +181,18 @@ public String[] listRoleNamesByObject(String metalake, MetadataObject object) throws NoSuchMetalakeException, NoSuchMetadataObjectException { return dispatcher.listRoleNamesByObject(metalake, object); } + + @Override + public Role grantPrivilegeToRole( + String metalake, String role, MetadataObject object, List privileges) + throws NoSuchMetalakeException, NoSuchRoleException { + return dispatcher.grantPrivilegeToRole(metalake, role, object, privileges); + } + + @Override + public Role revokePrivilegesFromRole( + String metalake, String role, MetadataObject object, List privileges) + throws NoSuchMetalakeException, NoSuchRoleException { + return dispatcher.revokePrivilegesFromRole(metalake, role, object, privileges); + } } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java b/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java index c1f72c36098..1ed13855572 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java @@ -176,6 +176,8 @@ public E update( return (E) UserMetaService.getInstance().updateUser(ident, updater); case GROUP: return (E) GroupMetaService.getInstance().updateGroup(ident, updater); + case ROLE: + return (E) RoleMetaService.getInstance().updateRole(ident, updater); case TAG: return (E) TagMetaService.getInstance().updateTag(ident, updater); default: diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/RoleMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/RoleMetaMapper.java index 6b155f49856..8f02201310c 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/RoleMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/RoleMetaMapper.java @@ -76,6 +76,10 @@ List listRolesByMetadataObjectIdAndType( method = "insertRoleMetaOnDuplicateKeyUpdate") void insertRoleMetaOnDuplicateKeyUpdate(@Param("roleMeta") RolePO rolePO); + @UpdateProvider(type = RoleMetaSQLProviderFactory.class, method = "updateRoleMeta") + Integer updateRoleMeta( + @Param("newRoleMeta") RolePO newRolePO, @Param("oldRoleMeta") RolePO oldRolePO); + @UpdateProvider(type = RoleMetaSQLProviderFactory.class, method = "softDeleteRoleMetaByRoleId") void softDeleteRoleMetaByRoleId(Long roleId); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/RoleMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/RoleMetaSQLProviderFactory.java index 2aedd02eb3e..61daab396bc 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/RoleMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/RoleMetaSQLProviderFactory.java @@ -83,6 +83,11 @@ public static String insertRoleMetaOnDuplicateKeyUpdate(@Param("roleMeta") RoleP return getProvider().insertRoleMetaOnDuplicateKeyUpdate(rolePO); } + public static String updateRoleMeta( + @Param("newRoleMeta") RolePO newRolePO, @Param("oldRoleMeta") RolePO oldRolePO) { + return getProvider().updateRoleMeta(newRolePO, oldRolePO); + } + public static String softDeleteRoleMetaByRoleId(Long roleId) { return getProvider().softDeleteRoleMetaByRoleId(roleId); } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SecurableObjectMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SecurableObjectMapper.java index e5fa90cc914..a5160f905eb 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SecurableObjectMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SecurableObjectMapper.java @@ -45,6 +45,12 @@ public interface SecurableObjectMapper { void batchInsertSecurableObjects( @Param("securableObjects") List securableObjectPOs); + @UpdateProvider( + type = SecurableObjectSQLProviderFactory.class, + method = "batchSoftDeleteSecurableObjects") + void batchSoftDeleteSecurableObjects( + @Param("securableObjects") List securableObjectPOs); + @UpdateProvider( type = SecurableObjectSQLProviderFactory.class, method = "softDeleteSecurableObjectsByRoleId") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SecurableObjectSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SecurableObjectSQLProviderFactory.java index 46a6f47f272..2777f2e9bad 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SecurableObjectSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SecurableObjectSQLProviderFactory.java @@ -55,6 +55,11 @@ public static String batchInsertSecurableObjects( return getProvider().batchInsertSecurableObjects(securableObjectPOs); } + public static String batchSoftDeleteSecurableObjects( + @Param("securableObjects") List securableObjectPOs) { + return getProvider().batchSoftDeleteSecurableObjects(securableObjectPOs); + } + public static String softDeleteSecurableObjectsByRoleId(@Param("roleId") Long roleId) { return getProvider().softDeleteSecurableObjectsByRoleId(roleId); } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/RoleMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/RoleMetaBaseSQLProvider.java index b2dc1213765..d28e26f3a1e 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/RoleMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/RoleMetaBaseSQLProvider.java @@ -151,6 +151,25 @@ public String insertRoleMetaOnDuplicateKeyUpdate(@Param("roleMeta") RolePO roleP + " deleted_at = #{roleMeta.deletedAt}"; } + public String updateRoleMeta( + @Param("newRoleMeta") RolePO newRolePO, @Param("oldRoleMeta") RolePO oldRolePO) { + return "UPDATE " + + ROLE_TABLE_NAME + + " SET role_name = #{newRoleMeta.roleName}," + + " metalake_id = #{newRoleMeta.metalakeId}," + + " properties = #{newRoleMeta.properties}," + + " audit_info = #{newRoleMeta.auditInfo}," + + " current_version = #{newRoleMeta.currentVersion}," + + " last_version = #{newRoleMeta.lastVersion}," + + " deleted_at = #{newRoleMeta.deletedAt}" + + " WHERE role_id = #{oldRoleMeta.roleId}" + + " AND role_name = #{oldRoleMeta.roleName}" + + " AND metalake_id = #{oldRoleMeta.metalakeId}" + + " AND current_version = #{oldRoleMeta.currentVersion}" + + " AND last_version = #{oldRoleMeta.lastVersion}" + + " AND deleted_at = 0"; + } + public String softDeleteRoleMetaByRoleId(Long roleId) { return "UPDATE " + ROLE_TABLE_NAME diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SecurableObjectBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SecurableObjectBaseSQLProvider.java index 012d9ed95d8..5561f7cbb47 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SecurableObjectBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SecurableObjectBaseSQLProvider.java @@ -48,6 +48,23 @@ public String batchInsertSecurableObjects( + ""; } + public String batchSoftDeleteSecurableObjects( + @Param("securableObjects") List securableObjectPOs) { + return ""; + } + public String softDeleteSecurableObjectsByRoleId(@Param("roleId") Long roleId) { return "UPDATE " + SECURABLE_OBJECT_TABLE_NAME diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java index 915a1495022..0236b01fa3f 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java @@ -18,12 +18,19 @@ */ package org.apache.gravitino.storage.relational.service; +import com.google.common.base.Preconditions; import com.google.common.collect.Lists; +import com.google.common.collect.Sets; import java.io.IOException; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import org.apache.gravitino.Entity; +import org.apache.gravitino.HasIdentifier; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; @@ -160,6 +167,89 @@ public void insertRole(RoleEntity roleEntity, boolean overwritten) throws IOExce } } + public RoleEntity updateRole( + NameIdentifier identifier, Function updater) throws IOException { + AuthorizationUtils.checkRole(identifier); + + try { + String metalake = identifier.namespace().level(0); + Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(metalake); + + RolePO rolePO = getRolePOByMetalakeIdAndName(metalakeId, identifier.name()); + RoleEntity oldRoleEntity = + POConverters.fromRolePO( + rolePO, listSecurableObjects(rolePO), AuthorizationUtils.ofRoleNamespace(metalake)); + + RoleEntity newRoleEntity = (RoleEntity) updater.apply((E) oldRoleEntity); + + Preconditions.checkArgument( + Objects.equals(oldRoleEntity.id(), newRoleEntity.id()), + "The updated role entity id: %s should be same with the role entity id before: %s", + newRoleEntity.id(), + oldRoleEntity.id()); + + Set oldObjects = new HashSet<>(oldRoleEntity.securableObjects()); + Set newObjects = new HashSet<>(newRoleEntity.securableObjects()); + Set insertObjects = Sets.difference(newObjects, oldObjects); + Set deleteObjects = Sets.difference(oldObjects, newObjects); + + if (insertObjects.isEmpty() && deleteObjects.isEmpty()) { + return newRoleEntity; + } + + List deleteSecurableObjectPOs = + toSecurableObjectPOs(deleteObjects, oldRoleEntity, metalakeId); + + List insertSecurableObjectPOs = + toSecurableObjectPOs(insertObjects, oldRoleEntity, metalakeId); + + SessionUtils.doMultipleWithCommit( + () -> + SessionUtils.doWithoutCommit( + RoleMetaMapper.class, + mapper -> + mapper.updateRoleMeta( + POConverters.updateRolePOWithVersion(rolePO, newRoleEntity), rolePO)), + () -> { + if (deleteSecurableObjectPOs.isEmpty()) { + return; + } + + SessionUtils.doWithoutCommit( + SecurableObjectMapper.class, + mapper -> mapper.batchSoftDeleteSecurableObjects(deleteSecurableObjectPOs)); + }, + () -> { + if (insertSecurableObjectPOs.isEmpty()) { + return; + } + + SessionUtils.doWithoutCommit( + SecurableObjectMapper.class, + mapper -> mapper.batchInsertSecurableObjects(insertSecurableObjectPOs)); + }); + + return newRoleEntity; + } catch (RuntimeException re) { + ExceptionUtils.checkSQLException(re, Entity.EntityType.ROLE, identifier.toString()); + throw re; + } + } + + private List toSecurableObjectPOs( + Set deleteObjects, RoleEntity oldRoleEntity, Long metalakeId) { + List securableObjectPOs = Lists.newArrayList(); + for (SecurableObject object : deleteObjects) { + SecurableObjectPO.Builder objectBuilder = + POConverters.initializeSecurablePOBuilderWithVersion( + oldRoleEntity.id(), object, getEntityType(object)); + objectBuilder.withMetadataObjectId( + MetadataObjectService.getMetadataObjectId(metalakeId, object.fullName(), object.type())); + securableObjectPOs.add(objectBuilder.build()); + } + return securableObjectPOs; + } + public RoleEntity getRoleByIdentifier(NameIdentifier identifier) { AuthorizationUtils.checkRole(identifier); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java b/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java index f6392127b36..f09f6751cf5 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java @@ -1069,6 +1069,27 @@ public static SecurableObjectPO.Builder initializeSecurablePOBuilderWithVersion( } } + public static RolePO updateRolePOWithVersion(RolePO oldRolePO, RoleEntity newRole) { + Long lastVersion = oldRolePO.getLastVersion(); + // TODO: set the version to the last version + 1 when having some fields need be multiple + // version + Long nextVersion = lastVersion; + try { + return RolePO.builder() + .withRoleId(oldRolePO.getRoleId()) + .withRoleName(newRole.name()) + .withMetalakeId(oldRolePO.getMetalakeId()) + .withProperties(JsonUtils.anyFieldMapper().writeValueAsString(newRole.properties())) + .withAuditInfo(JsonUtils.anyFieldMapper().writeValueAsString(newRole.auditInfo())) + .withCurrentVersion(nextVersion) + .withLastVersion(nextVersion) + .withDeletedAt(DEFAULT_DELETED_AT) + .build(); + } catch (JsonProcessingException e) { + throw new RuntimeException("Failed to serialize json object:", e); + } + } + public static TagEntity fromTagPO(TagPO tagPO, Namespace namespace) { try { return TagEntity.builder() diff --git a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java index ce3c4e90c9d..e7e792536d2 100644 --- a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java +++ b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java @@ -28,11 +28,14 @@ import java.time.Instant; import java.util.List; import org.apache.commons.lang3.reflect.FieldUtils; +import org.apache.gravitino.Catalog; import org.apache.gravitino.Config; import org.apache.gravitino.Configs; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityStore; import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.MetadataObjects; import org.apache.gravitino.Namespace; import org.apache.gravitino.catalog.CatalogManager; import org.apache.gravitino.connector.BaseCatalog; @@ -117,6 +120,34 @@ public class TestAccessControlManagerForPermissions { .withAuditInfo(auditInfo) .build(); + private static RoleEntity grantedRoleEntity = + RoleEntity.builder() + .withNamespace( + Namespace.of(METALAKE, Entity.SYSTEM_CATALOG_RESERVED_NAME, Entity.ROLE_SCHEMA_NAME)) + .withId(1L) + .withName("grantedRole") + .withProperties(Maps.newHashMap()) + .withSecurableObjects( + Lists.newArrayList( + SecurableObjects.ofCatalog( + CATALOG, Lists.newArrayList(Privileges.UseCatalog.allow())))) + .withAuditInfo(auditInfo) + .build(); + + private static RoleEntity revokedRoleEntity = + RoleEntity.builder() + .withNamespace( + Namespace.of(METALAKE, Entity.SYSTEM_CATALOG_RESERVED_NAME, Entity.ROLE_SCHEMA_NAME)) + .withId(1L) + .withName("revokedRole") + .withProperties(Maps.newHashMap()) + .withSecurableObjects( + Lists.newArrayList( + SecurableObjects.ofCatalog( + CATALOG, Lists.newArrayList(Privileges.UseCatalog.allow())))) + .withAuditInfo(auditInfo) + .build(); + @BeforeAll public static void setUp() throws Exception { config = new Config(false) {}; @@ -130,6 +161,8 @@ public static void setUp() throws Exception { entityStore.put(userEntity, true); entityStore.put(groupEntity, true); entityStore.put(roleEntity, true); + entityStore.put(grantedRoleEntity, true); + entityStore.put(revokedRoleEntity, true); accessControlManager = new AccessControlManager(entityStore, new RandomIdGenerator(), config); @@ -139,6 +172,8 @@ public static void setUp() throws Exception { FieldUtils.writeField(GravitinoEnv.getInstance(), "catalogManager", catalogManager, true); BaseCatalog catalog = Mockito.mock(BaseCatalog.class); Mockito.when(catalogManager.loadCatalog(any())).thenReturn(catalog); + Mockito.when(catalogManager.listCatalogsInfo(Mockito.any())) + .thenReturn(new Catalog[] {catalog}); authorizationPlugin = Mockito.mock(AuthorizationPlugin.class); Mockito.when(catalog.getAuthorizationPlugin()).thenReturn(authorizationPlugin); } @@ -309,4 +344,85 @@ public void testRevokeRoleFormGroup() { NoSuchGroupException.class, () -> accessControlManager.revokeRolesFromGroup(METALAKE, ROLE, notExist)); } + + @Test + public void testGrantPrivilegeToRole() { + reset(authorizationPlugin); + String notExist = "not-exist"; + + Role role = + accessControlManager.grantPrivilegeToRole( + METALAKE, + "grantedRole", + MetadataObjects.of(null, METALAKE, MetadataObject.Type.METALAKE), + Lists.newArrayList(Privileges.CreateTable.allow())); + + List objects = role.securableObjects(); + + // Test authorization plugin + verify(authorizationPlugin).onRoleUpdated(any(), any()); + + Assertions.assertEquals(2, objects.size()); + + // Repeat to grant + role = + accessControlManager.grantPrivilegeToRole( + METALAKE, + "grantedRole", + MetadataObjects.of(null, METALAKE, MetadataObject.Type.METALAKE), + Lists.newArrayList(Privileges.CreateTable.allow())); + objects = role.securableObjects(); + + Assertions.assertEquals(2, objects.size()); + + // Throw NoSuchRoleException + Assertions.assertThrows( + NoSuchRoleException.class, + () -> + accessControlManager.grantPrivilegeToRole( + METALAKE, + notExist, + MetadataObjects.of(null, METALAKE, MetadataObject.Type.METALAKE), + Lists.newArrayList(Privileges.CreateTable.allow()))); + } + + @Test + public void testRevokePrivilegeFromRole() { + reset(authorizationPlugin); + String notExist = "not-exist"; + + Role role = + accessControlManager.revokePrivilegesFromRole( + METALAKE, + "revokedRole", + MetadataObjects.of(null, CATALOG, MetadataObject.Type.CATALOG), + Lists.newArrayList(Privileges.UseCatalog.allow())); + + // Test authorization plugin + verify(authorizationPlugin).onRoleUpdated(any(), any()); + + List objects = role.securableObjects(); + + Assertions.assertTrue(objects.isEmpty()); + + // repeat to revoke + role = + accessControlManager.revokePrivilegesFromRole( + METALAKE, + "revokedRole", + MetadataObjects.of(null, CATALOG, MetadataObject.Type.CATALOG), + Lists.newArrayList(Privileges.UseCatalog.allow())); + objects = role.securableObjects(); + Assertions.assertTrue(objects.isEmpty()); + + // Throw NoSuchRoleException + Assertions.assertThrows( + NoSuchRoleException.class, + () -> + accessControlManager.revokePrivilegesFromRole( + METALAKE, + notExist, + MetadataObjects.of(null, METALAKE, MetadataObject.Type.METALAKE), + Lists.newArrayList(Privileges.CreateTable.allow()))); + } } diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java index 1f818b11253..14ba3254d78 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java @@ -27,8 +27,10 @@ import java.sql.SQLException; import java.sql.Statement; import java.time.Instant; +import java.util.Collections; import java.util.Comparator; import java.util.List; +import java.util.function.Function; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.AuthorizationUtils; @@ -559,6 +561,180 @@ void deleteMetalake() throws IOException { .isEmpty()); } + @Test + void testUpdateRole() throws IOException { + AuditInfo auditInfo = + AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build(); + BaseMetalake metalake = + createBaseMakeLake(RandomIdGenerator.INSTANCE.nextId(), metalakeName, auditInfo); + backend.insert(metalake, false); + + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), Namespace.of("metalake"), "catalog", auditInfo); + backend.insert(catalog, false); + + RoleMetaService roleMetaService = RoleMetaService.getInstance(); + RoleEntity roleEntity = + createRoleEntity( + RandomIdGenerator.INSTANCE.nextId(), + AuthorizationUtils.ofRoleNamespace(metalakeName), + "role1", + auditInfo, + "catalog"); + roleMetaService.insertRole(roleEntity, false); + + // grant privileges to the role + Function grantUpdater = + role -> { + AuditInfo updateAuditInfo = + AuditInfo.builder() + .withCreator(role.auditInfo().creator()) + .withCreateTime(role.auditInfo().createTime()) + .withLastModifier("grantRole") + .withLastModifiedTime(Instant.now()) + .build(); + + List securableObjects = Lists.newArrayList(role.securableObjects()); + securableObjects.add( + SecurableObjects.ofMetalake( + metalakeName, Lists.newArrayList(Privileges.CreateTable.allow()))); + + return RoleEntity.builder() + .withId(role.id()) + .withName(role.name()) + .withNamespace(role.namespace()) + .withProperties(ImmutableMap.of("k1", "v1")) + .withSecurableObjects(securableObjects) + .withAuditInfo(updateAuditInfo) + .build(); + }; + + Assertions.assertNotNull(roleMetaService.updateRole(roleEntity.nameIdentifier(), grantUpdater)); + RoleEntity grantRole = roleMetaService.getRoleByIdentifier(roleEntity.nameIdentifier()); + + Assertions.assertEquals(grantRole.id(), roleEntity.id()); + Assertions.assertEquals(grantRole.name(), roleEntity.name()); + Assertions.assertEquals("creator", grantRole.auditInfo().creator()); + Assertions.assertEquals("grantRole", grantRole.auditInfo().lastModifier()); + Assertions.assertEquals( + Lists.newArrayList( + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.UseCatalog.allow())), + SecurableObjects.ofMetalake( + metalakeName, Lists.newArrayList(Privileges.CreateTable.allow()))), + grantRole.securableObjects()); + + // revoke privileges from the role + Function revokeUpdater = + role -> { + AuditInfo updateAuditInfo = + AuditInfo.builder() + .withCreator(role.auditInfo().creator()) + .withCreateTime(role.auditInfo().createTime()) + .withLastModifier("revokeRole") + .withLastModifiedTime(Instant.now()) + .build(); + + List securableObjects = Lists.newArrayList(role.securableObjects()); + securableObjects.remove(0); + + return RoleEntity.builder() + .withId(role.id()) + .withName(role.name()) + .withNamespace(role.namespace()) + .withAuditInfo(updateAuditInfo) + .withProperties(role.properties()) + .withSecurableObjects(securableObjects) + .withAuditInfo(updateAuditInfo) + .build(); + }; + roleMetaService.updateRole(roleEntity.nameIdentifier(), revokeUpdater); + + RoleEntity revokeRole = roleMetaService.getRoleByIdentifier(roleEntity.nameIdentifier()); + Assertions.assertEquals(revokeRole.id(), roleEntity.id()); + Assertions.assertEquals(revokeRole.name(), roleEntity.name()); + Assertions.assertEquals("creator", revokeRole.auditInfo().creator()); + Assertions.assertEquals("revokeRole", revokeRole.auditInfo().lastModifier()); + Assertions.assertEquals( + Lists.newArrayList( + SecurableObjects.ofMetalake( + metalakeName, Lists.newArrayList(Privileges.CreateTable.allow()))), + revokeRole.securableObjects()); + + // grant and revoke privileges for the role + Function grantRevokeUpdater = + role -> { + AuditInfo updateAuditInfo = + AuditInfo.builder() + .withCreator(role.auditInfo().creator()) + .withCreateTime(role.auditInfo().createTime()) + .withLastModifier("grantRevokeRole") + .withLastModifiedTime(Instant.now()) + .build(); + + List securableObjects = Lists.newArrayList(role.securableObjects()); + securableObjects.remove(0); + securableObjects.add( + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.CreateTable.allow()))); + + return RoleEntity.builder() + .withId(role.id()) + .withName(role.name()) + .withNamespace(role.namespace()) + .withAuditInfo(updateAuditInfo) + .withProperties(role.properties()) + .withSecurableObjects(securableObjects) + .withAuditInfo(updateAuditInfo) + .build(); + }; + roleMetaService.updateRole(roleEntity.nameIdentifier(), grantRevokeUpdater); + + RoleEntity grantRevokeRole = roleMetaService.getRoleByIdentifier(roleEntity.nameIdentifier()); + Assertions.assertEquals(grantRevokeRole.id(), roleEntity.id()); + Assertions.assertEquals(grantRevokeRole.name(), roleEntity.name()); + Assertions.assertEquals("creator", grantRevokeRole.auditInfo().creator()); + Assertions.assertEquals("grantRevokeRole", grantRevokeRole.auditInfo().lastModifier()); + Assertions.assertEquals( + Lists.newArrayList( + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.CreateTable.allow()))), + grantRevokeRole.securableObjects()); + + // revoke multiple securable objects + roleMetaService.updateRole(roleEntity.nameIdentifier(), grantUpdater); + Function revokeMultipleUpdater = + role -> { + AuditInfo updateAuditInfo = + AuditInfo.builder() + .withCreator(role.auditInfo().creator()) + .withCreateTime(role.auditInfo().createTime()) + .withLastModifier("revokeMultiple") + .withLastModifiedTime(Instant.now()) + .build(); + + return RoleEntity.builder() + .withId(role.id()) + .withName(role.name()) + .withNamespace(role.namespace()) + .withAuditInfo(updateAuditInfo) + .withProperties(role.properties()) + .withSecurableObjects(Collections.emptyList()) + .withAuditInfo(updateAuditInfo) + .build(); + }; + + roleMetaService.updateRole(roleEntity.nameIdentifier(), revokeMultipleUpdater); + RoleEntity revokeMultipleRole = + roleMetaService.getRoleByIdentifier(roleEntity.nameIdentifier()); + Assertions.assertEquals(revokeMultipleRole.id(), roleEntity.id()); + Assertions.assertEquals(revokeMultipleRole.name(), roleEntity.name()); + Assertions.assertEquals("creator", revokeMultipleRole.auditInfo().creator()); + Assertions.assertEquals("revokeMultiple", revokeMultipleRole.auditInfo().lastModifier()); + Assertions.assertTrue(revokeMultipleRole.securableObjects().isEmpty()); + } + @Test void deleteMetalakeCascade() throws IOException { AuditInfo auditInfo = diff --git a/integration-test/src/test/java/org/apache/gravitino/integration/test/authorization/AccessControlIT.java b/integration-test/src/test/java/org/apache/gravitino/integration/test/authorization/AccessControlIT.java index 6bd8cd0b159..68ab1f19a0d 100644 --- a/integration-test/src/test/java/org/apache/gravitino/integration/test/authorization/AccessControlIT.java +++ b/integration-test/src/test/java/org/apache/gravitino/integration/test/authorization/AccessControlIT.java @@ -28,6 +28,8 @@ import java.util.stream.Collectors; import org.apache.gravitino.Catalog; import org.apache.gravitino.Configs; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.MetadataObjects; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Schema; import org.apache.gravitino.auth.AuthConstants; @@ -40,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; @@ -214,6 +217,47 @@ void testManageRoles() { NoSuchMetadataObjectException.class, () -> metalake.createRole("not-existed", properties, Lists.newArrayList(catalogObject))); + // Create a role with duplicated securable objects + SecurableObject duplicatedMetalakeObject = + SecurableObjects.ofMetalake( + metalakeName, Lists.newArrayList(Privileges.CreateCatalog.allow())); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + metalake.createRole( + roleName, + properties, + Lists.newArrayList(metalakeObject, duplicatedMetalakeObject))); + + // Create a role with wrong privilege + SecurableObject wrongPrivilegeObject = + SecurableObjects.ofCatalog("wrong", Lists.newArrayList(Privileges.CreateCatalog.allow())); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + metalake.createRole( + "not-existed", properties, Lists.newArrayList(wrongPrivilegeObject))); + + // Create a role with wrong privilege + SecurableObject wrongCatalogObject = + SecurableObjects.ofCatalog( + "fileset_catalog", Lists.newArrayList(Privileges.SelectTable.allow())); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + metalake.createRole("not-existed", properties, Lists.newArrayList(wrongCatalogObject))); + + // Create a role with duplicated privilege + SecurableObject duplicatedCatalogObject = + SecurableObjects.ofCatalog( + "fileset_catalog", + Lists.newArrayList(Privileges.SelectTable.allow(), Privileges.SelectTable.deny())); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + metalake.createRole( + "not-existed", properties, Lists.newArrayList(duplicatedCatalogObject))); + // Get a role role = metalake.getRole(roleName); @@ -282,9 +326,15 @@ void testManageRoles() { Assertions.assertEquals(createdPrivilege.name(), Privilege.Name.CREATE_CATALOG); Assertions.assertEquals(createdPrivilege.condition(), Privilege.Condition.ALLOW); + // create empty objects role + Role emptyObjectsRole = metalake.createRole("empty", properties, Collections.emptyList()); + Assertions.assertEquals("empty", emptyObjectsRole.name()); + Assertions.assertEquals(properties, role.properties()); + // Delete a role Assertions.assertTrue(metalake.deleteRole(roleName)); Assertions.assertFalse(metalake.deleteRole(roleName)); + metalake.deleteRole(emptyObjectsRole.name()); } @Test @@ -397,6 +447,99 @@ void testManageGroupPermissions() { metalake.deleteRole(roleName); } + @Test + void testManageRolePermissions() { + String roleName = "role#123"; + Map properties = Maps.newHashMap(); + properties.put("k1", "v1"); + metalake.createRole(roleName, properties, Lists.newArrayList()); + MetadataObject metadataObject = + MetadataObjects.of(null, metalakeName, MetadataObject.Type.METALAKE); + + // grant a privilege + Role role = + metalake.grantPrivilegesToRole( + roleName, metadataObject, Lists.newArrayList(Privileges.CreateCatalog.allow())); + Assertions.assertEquals(1, role.securableObjects().size()); + + // grant a wrong privilege + MetadataObject catalog = MetadataObjects.of(null, "catalog", MetadataObject.Type.CATALOG); + Assertions.assertThrows( + IllegalPrivilegeException.class, + () -> + metalake.grantPrivilegesToRole( + roleName, catalog, Lists.newArrayList(Privileges.CreateCatalog.allow()))); + + // grant a wrong catalog type privilege + MetadataObject wrongCatalog = + MetadataObjects.of(null, "fileset_catalog", MetadataObject.Type.CATALOG); + Assertions.assertThrows( + IllegalPrivilegeException.class, + () -> + metalake.grantPrivilegesToRole( + roleName, wrongCatalog, Lists.newArrayList(Privileges.SelectTable.allow()))); + + // grant a duplicated privilege + MetadataObject duplicatedCatalog = + MetadataObjects.of(null, "fileset_catalog", MetadataObject.Type.CATALOG); + Assertions.assertThrows( + IllegalPrivilegeException.class, + () -> + metalake.grantPrivilegesToRole( + roleName, + duplicatedCatalog, + Lists.newArrayList(Privileges.SelectTable.allow(), Privileges.SelectTable.deny()))); + + // repeat to grant a privilege + role = + metalake.grantPrivilegesToRole( + roleName, metadataObject, Lists.newArrayList(Privileges.CreateCatalog.allow())); + Assertions.assertEquals(1, role.securableObjects().size()); + + // grant a not-existing role + Assertions.assertThrows( + NoSuchRoleException.class, + () -> + metalake.grantPrivilegesToRole( + "not-exist", metadataObject, Lists.newArrayList(Privileges.CreateCatalog.allow()))); + + // revoke a privilege + role = + metalake.revokePrivilegesFromRole( + roleName, metadataObject, Lists.newArrayList(Privileges.CreateCatalog.allow())); + Assertions.assertTrue(role.securableObjects().isEmpty()); + + // revoke a wrong privilege + Assertions.assertThrows( + IllegalPrivilegeException.class, + () -> + metalake.revokePrivilegesFromRole( + roleName, catalog, Lists.newArrayList(Privileges.CreateCatalog.allow()))); + + // revoke a wrong catalog type privilege + Assertions.assertThrows( + IllegalPrivilegeException.class, + () -> + metalake.revokePrivilegesFromRole( + roleName, wrongCatalog, Lists.newArrayList(Privileges.SelectTable.allow()))); + + // repeat to revoke a privilege + role = + metalake.revokePrivilegesFromRole( + roleName, metadataObject, Lists.newArrayList(Privileges.CreateCatalog.allow())); + Assertions.assertTrue(role.securableObjects().isEmpty()); + + // revoke a not-existing role + Assertions.assertThrows( + NoSuchRoleException.class, + () -> + metalake.revokePrivilegesFromRole( + "not-exist", metadataObject, Lists.newArrayList(Privileges.CreateCatalog.allow()))); + + // Cleanup + metalake.deleteRole(roleName); + } + private static void assertSecurableObjects( List expect, List actual) { Assertions.assertEquals(expect.size(), actual.size()); 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 e2405a6adff..ec270fb825d 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 @@ -47,12 +47,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 { + for (PrivilegeDTO privilegeDTO : privilegeGrantRequest.getPrivileges()) { + AuthorizationUtils.checkPrivilege(privilegeDTO, object, metalake); + } + + AuthorizationUtils.checkSecurableObject(metalake, object); + return TreeLockUtils.doWithTreeLock( + AuthorizationUtils.ofRole(metalake, role), + LockType.WRITE, + () -> + Utils.ok( + new RoleResponse( + DTOConverters.toDTO( + accessControlManager.grantPrivilegeToRole( + metalake, + role, + object, + privilegeGrantRequest.getPrivileges().stream() + .map(DTOConverters::fromPrivilegeDTO) + .collect(Collectors.toList())))))); + }); + } catch (Exception e) { + return ExceptionHandlers.handleRolePermissionOperationException( + OperationType.GRANT, fullName, role, e); + } + } + + @PUT + @Path("roles/{role}/{type}/{fullName}/revoke/") + @Produces("application/vnd.gravitino.v1+json") + @Timed(name = "revoke-privilege-from-role." + MetricNames.HTTP_PROCESS_DURATION, absolute = true) + @ResponseMetered(name = "revoke-privilege-from-role", absolute = true) + public Response revokePrivilegeFromRole( + @PathParam("metalake") String metalake, + @PathParam("role") String role, + @PathParam("type") String type, + @PathParam("fullName") String fullName, + PrivilegeRevokeRequest privilegeRevokeRequest) { + try { + MetadataObject object = + MetadataObjects.parse( + fullName, MetadataObject.Type.valueOf(type.toUpperCase(Locale.ROOT))); + + return Utils.doAs( + httpRequest, + () -> { + for (PrivilegeDTO privilegeDTO : privilegeRevokeRequest.getPrivileges()) { + AuthorizationUtils.checkPrivilege(privilegeDTO, object, metalake); + } + + AuthorizationUtils.checkSecurableObject(metalake, object); + return TreeLockUtils.doWithTreeLock( + AuthorizationUtils.ofRole(metalake, role), + LockType.WRITE, + () -> + Utils.ok( + new RoleResponse( + DTOConverters.toDTO( + accessControlManager.revokePrivilegesFromRole( + metalake, + role, + object, + privilegeRevokeRequest.getPrivileges().stream() + .map(DTOConverters::fromPrivilegeDTO) + .collect(Collectors.toList())))))); + }); + } catch (Exception e) { + return ExceptionHandlers.handleRolePermissionOperationException( + OperationType.REVOKE, fullName, role, e); + } + } } diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java index 9810ad759e3..e20f6b4510a 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java @@ -137,6 +137,8 @@ public Response createRole(@PathParam("metalake") String metalake, RoleCreateReq metadataObjects.add(metadataObject); } + Set privileges = Sets.newHashSet(object.privileges()); + AuthorizationUtils.checkDuplicatedNamePrivilege(privileges); for (Privilege privilege : object.privileges()) { AuthorizationUtils.checkPrivilege((PrivilegeDTO) privilege, object, metalake); } diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java index 76dba3edd4b..e927a0a4e96 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java @@ -39,20 +39,31 @@ import org.apache.gravitino.GravitinoEnv; import org.apache.gravitino.authorization.AccessControlManager; import org.apache.gravitino.authorization.Group; +import org.apache.gravitino.authorization.Privilege; +import org.apache.gravitino.authorization.Privileges; +import org.apache.gravitino.authorization.Role; +import org.apache.gravitino.authorization.SecurableObjects; import org.apache.gravitino.authorization.User; +import org.apache.gravitino.dto.authorization.PrivilegeDTO; +import org.apache.gravitino.dto.requests.PrivilegeGrantRequest; +import org.apache.gravitino.dto.requests.PrivilegeRevokeRequest; import org.apache.gravitino.dto.requests.RoleGrantRequest; import org.apache.gravitino.dto.requests.RoleRevokeRequest; import org.apache.gravitino.dto.responses.ErrorConstants; import org.apache.gravitino.dto.responses.ErrorResponse; import org.apache.gravitino.dto.responses.GroupResponse; +import org.apache.gravitino.dto.responses.RoleResponse; import org.apache.gravitino.dto.responses.UserResponse; +import org.apache.gravitino.exceptions.IllegalPrivilegeException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.exceptions.NoSuchRoleException; import org.apache.gravitino.exceptions.NoSuchUserException; import org.apache.gravitino.lock.LockManager; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.GroupEntity; +import org.apache.gravitino.meta.RoleEntity; import org.apache.gravitino.meta.UserEntity; +import org.apache.gravitino.metalake.MetalakeDispatcher; import org.apache.gravitino.rest.RESTUtils; import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; @@ -66,6 +77,7 @@ public class TestPermissionOperations extends JerseyTest { private static final AccessControlManager manager = mock(AccessControlManager.class); + private static final MetalakeDispatcher metalakeDispatcher = mock(MetalakeDispatcher.class); private static class MockServletRequestFactory extends ServletRequestFactoryBase { @Override @@ -84,6 +96,8 @@ public static void setup() throws IllegalAccessException { Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); FieldUtils.writeField(GravitinoEnv.getInstance(), "accessControlDispatcher", manager, true); + FieldUtils.writeField( + GravitinoEnv.getInstance(), "metalakeDispatcher", metalakeDispatcher, true); } @Override @@ -394,4 +408,162 @@ public void testRevokeRolesFromGroup() { Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse.getCode()); Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse.getType()); } + + @Test + public void testGrantPrivilegesToRole() { + RoleEntity roleEntity = + RoleEntity.builder() + .withId(1L) + .withName("role") + .withSecurableObjects( + Lists.newArrayList( + SecurableObjects.ofMetalake( + "metalake1", Lists.newArrayList(Privileges.CreateTable.allow())))) + .withAuditInfo( + AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build()) + .build(); + when(manager.grantPrivilegeToRole(any(), any(), any(), any())).thenReturn(roleEntity); + when(metalakeDispatcher.metalakeExists(any())).thenReturn(true); + PrivilegeGrantRequest request = + new PrivilegeGrantRequest( + Lists.newArrayList( + PrivilegeDTO.builder() + .withName(Privilege.Name.CREATE_TABLE) + .withCondition(Privilege.Condition.ALLOW) + .build())); + + Response resp = + target("/metalakes/metalake1/permissions/roles/role1/metalake/metalake1/grant") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.OK.getStatusCode(), resp.getStatus()); + RoleResponse grantResponse = resp.readEntity(RoleResponse.class); + Assertions.assertEquals(0, grantResponse.getCode()); + + Role role = grantResponse.getRole(); + Assertions.assertEquals(roleEntity.name(), role.name()); + Assertions.assertEquals(1, role.securableObjects().size()); + Assertions.assertEquals("metalake1", role.securableObjects().get(0).name()); + Assertions.assertEquals( + Privilege.Name.CREATE_TABLE, role.securableObjects().get(0).privileges().get(0).name()); + Assertions.assertEquals( + Privilege.Condition.ALLOW, role.securableObjects().get(0).privileges().get(0).condition()); + Assertions.assertEquals(roleEntity.properties(), role.properties()); + + doThrow(new RuntimeException("mock error")) + .when(manager) + .grantPrivilegeToRole(any(), any(), any(), any()); + Response resp3 = + target("/metalakes/metalake1/permissions/roles/role1/metalake/metalake1/grant") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals( + Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp3.getStatus()); + + ErrorResponse errorResponse = resp3.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse.getCode()); + Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse.getType()); + + // Test with wrong binding privileges + PrivilegeGrantRequest wrongPriRequest = + new PrivilegeGrantRequest( + Lists.newArrayList( + PrivilegeDTO.builder() + .withName(Privilege.Name.CREATE_CATALOG) + .withCondition(Privilege.Condition.ALLOW) + .build())); + + Response wrongPrivilegeResp = + target("/metalakes/metalake1/permissions/roles/role1/catalog/catalog1/grant") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(wrongPriRequest, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals( + Response.Status.BAD_REQUEST.getStatusCode(), wrongPrivilegeResp.getStatus()); + + ErrorResponse wrongPriErrorResp = wrongPrivilegeResp.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, wrongPriErrorResp.getCode()); + Assertions.assertEquals( + IllegalPrivilegeException.class.getSimpleName(), wrongPriErrorResp.getType()); + } + + @Test + public void testRevokePrivilegesFromRole() { + RoleEntity roleEntity = + RoleEntity.builder() + .withId(1L) + .withName("role") + .withSecurableObjects(Lists.newArrayList()) + .withAuditInfo( + AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build()) + .build(); + when(manager.revokePrivilegesFromRole(any(), any(), any(), any())).thenReturn(roleEntity); + when(metalakeDispatcher.metalakeExists(any())).thenReturn(true); + PrivilegeRevokeRequest request = + new PrivilegeRevokeRequest( + Lists.newArrayList( + PrivilegeDTO.builder() + .withName(Privilege.Name.CREATE_TABLE) + .withCondition(Privilege.Condition.ALLOW) + .build())); + + Response resp = + target("/metalakes/metalake1/permissions/roles/role1/metalake/metalake1/revoke") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.OK.getStatusCode(), resp.getStatus()); + RoleResponse revokeResponse = resp.readEntity(RoleResponse.class); + Assertions.assertEquals(0, revokeResponse.getCode()); + + Role role = revokeResponse.getRole(); + Assertions.assertEquals(roleEntity.name(), role.name()); + Assertions.assertEquals(roleEntity.securableObjects(), role.securableObjects()); + Assertions.assertEquals(roleEntity.properties(), role.properties()); + + doThrow(new RuntimeException("mock error")) + .when(manager) + .revokePrivilegesFromRole(any(), any(), any(), any()); + Response resp3 = + target("/metalakes/metalake1/permissions/roles/role1/metalake/metalake1/revoke") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals( + Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp3.getStatus()); + + ErrorResponse errorResponse = resp3.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse.getCode()); + Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse.getType()); + + // Test with wrong binding privileges + PrivilegeRevokeRequest wrongPriRequest = + new PrivilegeRevokeRequest( + Lists.newArrayList( + PrivilegeDTO.builder() + .withName(Privilege.Name.CREATE_CATALOG) + .withCondition(Privilege.Condition.ALLOW) + .build())); + + Response wrongPrivilegeResp = + target("/metalakes/metalake1/permissions/roles/role1/catalog/catalog1/revoke") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(wrongPriRequest, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals( + Response.Status.BAD_REQUEST.getStatusCode(), wrongPrivilegeResp.getStatus()); + + ErrorResponse wrongPriErrorResp = wrongPrivilegeResp.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, wrongPriErrorResp.getCode()); + Assertions.assertEquals( + IllegalPrivilegeException.class.getSimpleName(), wrongPriErrorResp.getType()); + } } diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java index 5767464894a..265fb607353 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java @@ -59,6 +59,7 @@ import org.apache.gravitino.dto.responses.NameListResponse; import org.apache.gravitino.dto.responses.RoleResponse; import org.apache.gravitino.dto.util.DTOConverters; +import org.apache.gravitino.exceptions.IllegalPrivilegeException; import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.exceptions.NoSuchRoleException; @@ -281,7 +282,7 @@ public void testCreateRole() { ErrorResponse wrongPriErrorResp = wrongPrivilegeResp.readEntity(ErrorResponse.class); Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, wrongPriErrorResp.getCode()); Assertions.assertEquals( - IllegalArgumentException.class.getSimpleName(), wrongPriErrorResp.getType()); + IllegalPrivilegeException.class.getSimpleName(), wrongPriErrorResp.getType()); // Test with empty securable objects request RoleCreateRequest emptyObjectRequest =