From 54c4a98d2a8b7d261766cba8b1891847f2876410 Mon Sep 17 00:00:00 2001 From: Rory Date: Wed, 18 Dec 2024 15:52:16 +0800 Subject: [PATCH 01/10] [#5661] feat(auth): Add JDBC authorization plugin interface --- .../access-control-integration-test.yml | 3 + .../authorization-jdbc/build.gradle.kts | 97 ++++ .../jdbc/JdbcAuthorizationObject.java | 93 ++++ .../jdbc/JdbcAuthorizationSQL.java | 117 +++++ .../authorization/jdbc/JdbcPrivilege.java | 103 ++++ .../jdbc/JdbcSQLBasedAuthorizationPlugin.java | 456 ++++++++++++++++++ .../JdbcSecurableObjectMappingProvider.java | 218 +++++++++ .../authorization/jdbc/TemporaryGroup.java | 51 ++ .../authorization/jdbc/TemporaryUser.java | 51 ++ .../JdbcSQLBasedAuthorizationPluginTest.java | 331 +++++++++++++ settings.gradle.kts | 2 +- 11 files changed, 1521 insertions(+), 1 deletion(-) create mode 100644 authorizations/authorization-jdbc/build.gradle.kts create mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java create mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationSQL.java create mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java create mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java create mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java create mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryGroup.java create mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryUser.java create mode 100644 authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPluginTest.java diff --git a/.github/workflows/access-control-integration-test.yml b/.github/workflows/access-control-integration-test.yml index 54ffde2ee82..6997eaf9a4c 100644 --- a/.github/workflows/access-control-integration-test.yml +++ b/.github/workflows/access-control-integration-test.yml @@ -90,6 +90,9 @@ jobs: ./gradlew -PtestMode=embedded -PjdbcBackend=h2 -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false :authorizations:authorization-ranger:test ./gradlew -PtestMode=deploy -PjdbcBackend=mysql -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false :authorizations:authorization-ranger:test ./gradlew -PtestMode=deploy -PjdbcBackend=postgresql -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false :authorizations:authorization-ranger:test + ./gradlew -PtestMode=embedded -PjdbcBackend=h2 -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false :authorizations:authorization-jdbc:test + ./gradlew -PtestMode=deploy -PjdbcBackend=mysql -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false :authorizations:authorization-jdbc:test + ./gradlew -PtestMode=deploy -PjdbcBackend=postgresql -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false :authorizations:authorization-jdbc:test - name: Upload integrate tests reports uses: actions/upload-artifact@v3 diff --git a/authorizations/authorization-jdbc/build.gradle.kts b/authorizations/authorization-jdbc/build.gradle.kts new file mode 100644 index 00000000000..2649370d8cf --- /dev/null +++ b/authorizations/authorization-jdbc/build.gradle.kts @@ -0,0 +1,97 @@ +/* + * 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. + */ +description = "authorization-jdbc" + +plugins { + `maven-publish` + id("java") + id("idea") +} + +dependencies { + implementation(project(":api")) { + exclude(group = "*") + } + implementation(project(":core")) { + exclude(group = "*") + } + + implementation(libs.bundles.log4j) + implementation(libs.commons.lang3) + implementation(libs.guava) + implementation(libs.javax.jaxb.api) { + exclude("*") + } + implementation(libs.javax.ws.rs.api) + implementation(libs.jettison) + compileOnly(libs.lombok) + implementation(libs.mail) + implementation(libs.rome) + implementation(libs.commons.dbcp2) + + testImplementation(project(":common")) + testImplementation(project(":clients:client-java")) + testImplementation(project(":server")) + testImplementation(project(":catalogs:catalog-common")) + testImplementation(project(":integration-test-common", "testArtifacts")) + testImplementation(libs.junit.jupiter.api) + testImplementation(libs.mockito.core) + testImplementation(libs.testcontainers) + testRuntimeOnly(libs.junit.jupiter.engine) +} + +tasks { + val runtimeJars by registering(Copy::class) { + from(configurations.runtimeClasspath) + into("build/libs") + } + + val copyAuthorizationLibs by registering(Copy::class) { + dependsOn("jar", runtimeJars) + from("build/libs") { + exclude("guava-*.jar") + exclude("log4j-*.jar") + exclude("slf4j-*.jar") + } + into("$rootDir/distribution/package/authorizations/ranger/libs") + } + + register("copyLibAndConfig", Copy::class) { + dependsOn(copyAuthorizationLibs) + } + + jar { + dependsOn(runtimeJars) + } +} + +tasks.test { + doFirst { + environment("HADOOP_USER_NAME", "gravitino") + } + dependsOn(":catalogs:catalog-hive:jar", ":catalogs:catalog-hive:runtimeJars") + + val skipITs = project.hasProperty("skipITs") + if (skipITs) { + // Exclude integration tests + exclude("**/integration/test/**") + } else { + dependsOn(tasks.jar) + } +} diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java new file mode 100644 index 00000000000..37e64093225 --- /dev/null +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java @@ -0,0 +1,93 @@ +/* + * 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.authorization.jdbc; + +import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; +import java.util.List; +import javax.annotation.Nullable; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.authorization.AuthorizationPrivilege; +import org.apache.gravitino.authorization.AuthorizationSecurableObject; + +/** + * JdbcAuthorizationObject is used for translating securable object to authorization securable + * object. JdbcAuthorizationObject has the database and table name. When table name is null, the + * object represents a database. The database can't be null. + */ +public class JdbcAuthorizationObject implements AuthorizationSecurableObject { + + public static final String ALL = "*"; + private String database; + private String table; + + List privileges; + + JdbcAuthorizationObject(String database, String table, List privileges) { + Preconditions.checkNotNull(database, "Jdbc authorization object database can't null"); + this.database = database; + this.table = table; + this.privileges = privileges; + } + + @Nullable + @Override + public String parent() { + if (table != null) { + return database; + } + + return null; + } + + @Override + public String name() { + if (table != null) { + return table; + } + + return database; + } + + @Override + public List names() { + List names = Lists.newArrayList(); + names.add(database); + if (table != null) { + names.add(table); + } + return names; + } + + @Override + public Type type() { + if (table != null) { + return () -> MetadataObject.Type.TABLE; + } + return () -> MetadataObject.Type.SCHEMA; + } + + @Override + public void validateAuthorizationMetadataObject() throws IllegalArgumentException {} + + @Override + public List privileges() { + return privileges; + } +} diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationSQL.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationSQL.java new file mode 100644 index 00000000000..d9783d35c4b --- /dev/null +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationSQL.java @@ -0,0 +1,117 @@ +/* + * 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.authorization.jdbc; + +import java.util.List; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.annotation.Unstable; +import org.apache.gravitino.authorization.Owner; + +/** Interface for SQL operations of the underlying access control system. */ +@Unstable +interface JdbcAuthorizationSQL { + + /** + * Get SQL statements for creating a user. + * + * @param username the username to create + * @return a SQL statement to create a user + */ + String getCreateUserSQL(String username); + + /** + * Get SQL statements for creating a group. + * + * @param username the username to drop + * @return a SQL statement to drop a user + */ + String getDropUserSQL(String username); + + /** + * Get SQL statements for creating a role. + * + * @param roleName the role name to create + * @return a SQL statement to create a role + */ + String getCreateRoleSQL(String roleName); + + /** + * Get SQL statements for dropping a role. + * + * @param roleName the role name to drop + * @return a SQL statement to drop a role + */ + String getDropRoleSQL(String roleName); + + /** + * Get SQL statements for granting privileges. + * + * @param privilege the privilege to grant + * @param objectType the object type in the database system + * @param objectName the object name in the database system + * @param roleName the role name to grant + * @return a sql statement to grant privilege + */ + String getGrantPrivilegeSQL( + String privilege, String objectType, String objectName, String roleName); + + /** + * Get SQL statements for revoking privileges. + * + * @param privilege the privilege to revoke + * @param objectType the object type in the database system + * @param objectName the object name in the database system + * @param roleName the role name to revoke + * @return a sql statement to revoke privilege + */ + String getRevokePrivilegeSQL( + String privilege, String objectType, String objectName, String roleName); + + /** + * Get SQL statements for granting role. + * + * @param roleName the role name to grant + * @param grantorType the grantor type, usually USER or ROLE + * @param grantorName the grantor name + * @return a sql statement to grant role + */ + String getGrantRoleSQL(String roleName, String grantorType, String grantorName); + + /** + * Get SQL statements for revoking roles. + * + * @param roleName the role name to revoke + * @param revokerType the revoker type, usually USER or ROLE + * @param revokerName the revoker name + * @return a sql statement to revoke role + */ + String getRevokeRoleSQL(String roleName, String revokerType, String revokerName); + + /** + * Get SQL statements for setting owner. + * + * @param type The metadata object type + * @param objectName the object name in the database system + * @param preOwner the previous owner of the object + * @param newOwner the new owner of the object + * @return the sql statement list to set owner + */ + List getSetOwnerSQL( + MetadataObject.Type type, String objectName, Owner preOwner, Owner newOwner); +} diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java new file mode 100644 index 00000000000..118d61f09de --- /dev/null +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java @@ -0,0 +1,103 @@ +/* + * 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.authorization.jdbc; + +import org.apache.gravitino.authorization.AuthorizationPrivilege; +import org.apache.gravitino.authorization.Privilege; + +public class JdbcPrivilege implements AuthorizationPrivilege { + + private static final JdbcPrivilege SELECT_PRI = new JdbcPrivilege(Type.SELECT); + private static final JdbcPrivilege INSERT_PRI = new JdbcPrivilege(Type.INSERT); + private static final JdbcPrivilege UPDATE_PRI = new JdbcPrivilege(Type.UPDATE); + private static final JdbcPrivilege ALTER_PRI = new JdbcPrivilege(Type.ALTER); + private static final JdbcPrivilege DELETE_PRI = new JdbcPrivilege(Type.DELETE); + private static final JdbcPrivilege ALL_PRI = new JdbcPrivilege(Type.ALL); + private static final JdbcPrivilege CREATE_PRI = new JdbcPrivilege(Type.CREATE); + private static final JdbcPrivilege DROP_PRI = new JdbcPrivilege(Type.DROP); + private static final JdbcPrivilege USAGE_PRI = new JdbcPrivilege(Type.USAGE); + + private final Type type; + + private JdbcPrivilege(Type type) { + this.type = type; + } + + @Override + public String getName() { + return type.getName(); + } + + @Override + public Privilege.Condition condition() { + return Privilege.Condition.ALLOW; + } + + @Override + public boolean equalsTo(String value) { + return false; + } + + static JdbcPrivilege valueOf(Type type) { + switch (type) { + case CREATE: + return CREATE_PRI; + case DELETE: + return DELETE_PRI; + case ALL: + return ALL_PRI; + case DROP: + return DROP_PRI; + case ALTER: + return ALTER_PRI; + case INSERT: + return INSERT_PRI; + case UPDATE: + return UPDATE_PRI; + case SELECT: + return SELECT_PRI; + case USAGE: + return USAGE_PRI; + default: + throw new IllegalArgumentException(String.format("Unsupported parameter type %s", type)); + } + } + + public enum Type { + SELECT("SELECT"), + INSERT("INSERT"), + UPDATE("UPDATE"), + ALTER("ALTER"), + DELETE("DELETE"), + ALL("ALL PRIVILEGES"), + CREATE("CREATE"), + DROP("DROP"), + USAGE("USAGE"); + + private final String name; + + Type(String name) { + this.name = name; + } + + public String getName() { + return name; + } + } +} diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java new file mode 100644 index 00000000000..0aff510e8e2 --- /dev/null +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java @@ -0,0 +1,456 @@ +/* + * 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.authorization.jdbc; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Lists; +import java.io.IOException; +import java.sql.Connection; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.commons.dbcp2.BasicDataSource; +import org.apache.commons.pool2.impl.BaseObjectPoolConfig; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.annotation.Unstable; +import org.apache.gravitino.authorization.AuthorizationPrivilege; +import org.apache.gravitino.authorization.AuthorizationSecurableObject; +import org.apache.gravitino.authorization.Group; +import org.apache.gravitino.authorization.MetadataObjectChange; +import org.apache.gravitino.authorization.Owner; +import org.apache.gravitino.authorization.Role; +import org.apache.gravitino.authorization.RoleChange; +import org.apache.gravitino.authorization.SecurableObject; +import org.apache.gravitino.authorization.User; +import org.apache.gravitino.connector.authorization.AuthorizationPlugin; +import org.apache.gravitino.exceptions.AuthorizationPluginException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * JdbcSQLBasedAuthorizationPlugin is the base class for all JDBC-based authorization plugins. For + * example, JdbcHiveAuthorizationPlugin is the JDBC-based authorization plugin for Hive. Different + * JDBC-based authorization plugins can inherit this class and implement their own SQL statements. + */ +@Unstable +abstract class JdbcSQLBasedAuthorizationPlugin + implements AuthorizationPlugin, JdbcAuthorizationSQL { + + private static final String CONFIG_PREFIX = "authorization.jdbc."; + public static final String JDBC_DRIVER = CONFIG_PREFIX + "driver"; + public static final String JDBC_URL = CONFIG_PREFIX + "url"; + public static final String JDBC_USERNAME = CONFIG_PREFIX + "username"; + public static final String JDBC_PASSWORD = CONFIG_PREFIX + "password"; + + private static final String GROUP_PREFIX = "GRAVITINO_GROUP_"; + private static final Logger LOG = LoggerFactory.getLogger(JdbcSQLBasedAuthorizationPlugin.class); + + protected BasicDataSource dataSource; + protected JdbcSecurableObjectMappingProvider mappingProvider; + + public JdbcSQLBasedAuthorizationPlugin(Map config) { + // Initialize the data source + dataSource = new BasicDataSource(); + String jdbcUrl = config.get(JDBC_URL); + dataSource.setUrl(jdbcUrl); + dataSource.setDriverClassName(config.get(JDBC_DRIVER)); + dataSource.setUsername(config.get(JDBC_USERNAME)); + dataSource.setPassword(config.get(JDBC_PASSWORD)); + dataSource.setDefaultAutoCommit(true); + dataSource.setMaxTotal(20); + dataSource.setMaxIdle(5); + dataSource.setMinIdle(0); + dataSource.setLogAbandoned(true); + dataSource.setRemoveAbandonedOnBorrow(true); + dataSource.setTestOnBorrow(BaseObjectPoolConfig.DEFAULT_TEST_ON_BORROW); + dataSource.setTestWhileIdle(BaseObjectPoolConfig.DEFAULT_TEST_WHILE_IDLE); + dataSource.setNumTestsPerEvictionRun(BaseObjectPoolConfig.DEFAULT_NUM_TESTS_PER_EVICTION_RUN); + dataSource.setTestOnReturn(BaseObjectPoolConfig.DEFAULT_TEST_ON_RETURN); + dataSource.setLifo(BaseObjectPoolConfig.DEFAULT_LIFO); + mappingProvider = new JdbcSecurableObjectMappingProvider(); + } + + @Override + public void close() throws IOException { + if (dataSource != null) { + try { + dataSource.close(); + } catch (SQLException e) { + throw new RuntimeException(e); + } + } + } + + @Override + public Boolean onMetadataUpdated(MetadataObjectChange... changes) throws RuntimeException { + // This interface mainly handles the metadata object rename change and delete change. + // The privilege for JdbcSQLBasedAuthorizationPlugin will be renamed or deleted automatically. + // We don't need to do any other things. + return true; + } + + @Override + public Boolean onRoleCreated(Role role) throws AuthorizationPluginException { + beforeExecuteSQL(); + + String sql = getCreateRoleSQL(role.name()); + executeUpdateSQL(sql, "already exists"); + + if (role.securableObjects() != null) { + for (SecurableObject object : role.securableObjects()) { + onRoleUpdated(role, RoleChange.addSecurableObject(role.name(), object)); + } + } + + return true; + } + + @Override + public Boolean onRoleAcquired(Role role) throws AuthorizationPluginException { + throw new UnsupportedOperationException("Doesn't support to acquired a role"); + } + + @Override + public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException { + beforeExecuteSQL(); + + String sql = getDropRoleSQL(role.name()); + executeUpdateSQL(sql); + return null; + } + + @Override + public Boolean onRoleUpdated(Role role, RoleChange... changes) + throws AuthorizationPluginException { + beforeExecuteSQL(); + + onRoleCreated(role); + for (RoleChange change : changes) { + if (change instanceof RoleChange.AddSecurableObject) { + SecurableObject object = ((RoleChange.AddSecurableObject) change).getSecurableObject(); + grantObjectPrivileges(role, object); + } else if (change instanceof RoleChange.RemoveSecurableObject) { + SecurableObject object = ((RoleChange.RemoveSecurableObject) change).getSecurableObject(); + revokeObjectPrivileges(role, object); + } else if (change instanceof RoleChange.UpdateSecurableObject) { + RoleChange.UpdateSecurableObject updateChange = (RoleChange.UpdateSecurableObject) change; + SecurableObject addObject = updateChange.getNewSecurableObject(); + SecurableObject removeObject = updateChange.getSecurableObject(); + revokeObjectPrivileges(role, removeObject); + grantObjectPrivileges(role, addObject); + } else { + throw new IllegalArgumentException(String.format("Don't support RoleChange %s", change)); + } + } + return true; + } + + @Override + public Boolean onGrantedRolesToUser(List roles, User user) + throws AuthorizationPluginException { + beforeExecuteSQL(); + + onUserAdded(user); + + for (Role role : roles) { + onRoleCreated(role); + String sql = getGrantRoleSQL(role.name(), "USER", user.name()); + executeUpdateSQL(sql); + } + return true; + } + + @Override + public Boolean onRevokedRolesFromUser(List roles, User user) + throws AuthorizationPluginException { + beforeExecuteSQL(); + + onUserAdded(user); + + for (Role role : roles) { + onRoleCreated(role); + String sql = getRevokeRoleSQL(role.name(), "USER", user.name()); + executeUpdateSQL(sql); + } + return true; + } + + @Override + public Boolean onGrantedRolesToGroup(List roles, Group group) + throws AuthorizationPluginException { + beforeExecuteSQL(); + + onGroupAdded(group); + + for (Role role : roles) { + onRoleCreated(role); + String sql = + getGrantRoleSQL(role.name(), "USER", String.format("%s%s", GROUP_PREFIX, group.name())); + executeUpdateSQL(sql); + } + return true; + } + + @Override + public Boolean onRevokedRolesFromGroup(List roles, Group group) + throws AuthorizationPluginException { + beforeExecuteSQL(); + + onGroupAdded(group); + + for (Role role : roles) { + onRoleCreated(role); + String sql = + getRevokeRoleSQL(role.name(), "USER", String.format("%s%s", GROUP_PREFIX, group.name())); + executeUpdateSQL(sql); + } + return true; + } + + @Override + public Boolean onUserAdded(User user) throws AuthorizationPluginException { + beforeExecuteSQL(); + + String sql = getCreateUserSQL(user.name()); + executeUpdateSQL(sql); + return true; + } + + @Override + public Boolean onUserRemoved(User user) throws AuthorizationPluginException { + beforeExecuteSQL(); + + String sql = getDropUserSQL(user.name()); + executeUpdateSQL(sql); + return true; + } + + @Override + public Boolean onUserAcquired(User user) throws AuthorizationPluginException { + throw new UnsupportedOperationException("Doesn't support to acquired a user"); + } + + @Override + public Boolean onGroupAdded(Group group) throws AuthorizationPluginException { + beforeExecuteSQL(); + + String name = String.format("%s%s", GROUP_PREFIX, group.name()); + String sql = getCreateUserSQL(name); + executeUpdateSQL(sql); + return true; + } + + @Override + public Boolean onGroupRemoved(Group group) throws AuthorizationPluginException { + beforeExecuteSQL(); + + String name = String.format("%s%s", GROUP_PREFIX, group.name()); + String sql = getDropUserSQL(name); + executeUpdateSQL(sql); + return true; + } + + @Override + public Boolean onGroupAcquired(Group group) throws AuthorizationPluginException { + throw new UnsupportedOperationException("Doesn't support to acquired a group"); + } + + @Override + public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner newOwner) + throws AuthorizationPluginException { + beforeExecuteSQL(); + + if (newOwner.type() == Owner.Type.USER) { + onUserAdded(new TemporaryUser(newOwner.name())); + } else if (newOwner.type() == Owner.Type.GROUP) { + onGroupAdded(new TemporaryGroup(newOwner.name())); + } else { + throw new IllegalArgumentException( + String.format("Don't support owner type %s", newOwner.type())); + } + + List authObjects = mappingProvider.translateOwner(metadataObject); + for (AuthorizationSecurableObject authObject : authObjects) { + List sqls = + getSetOwnerSQL( + authObject.type().metadataObjectType(), authObject.fullName(), preOwner, newOwner); + for (String sql : sqls) { + executeUpdateSQL(sql); + } + } + return true; + } + + @Override + public String getCreateUserSQL(String username) { + return String.format("CREATE USER %s", username); + } + + @Override + public String getDropUserSQL(String username) { + return String.format("DROP USER %s", username); + } + + @Override + public String getCreateRoleSQL(String roleName) { + return String.format("CREATE ROLE %s", roleName); + } + + @Override + public String getDropRoleSQL(String roleName) { + return String.format("DROP ROLE %s", roleName); + } + + @Override + public String getGrantPrivilegeSQL( + String privilege, String objectType, String objectName, String roleName) { + return String.format( + "GRANT %s ON %s %s TO ROLE %s", privilege, objectType, objectName, roleName); + } + + @Override + public String getRevokePrivilegeSQL( + String privilege, String objectType, String objectName, String roleName) { + return String.format( + "REVOKE %s ON %s %s FROM ROLE %s", privilege, objectType, objectName, roleName); + } + + @Override + public String getGrantRoleSQL(String roleName, String grantorType, String grantorName) { + return String.format("GRANT ROLE %s TO %s %s", roleName, grantorType, grantorName); + } + + @Override + public String getRevokeRoleSQL(String roleName, String revokerType, String revokerName) { + return String.format("REVOKE ROLE %s FROM %s %s", roleName, revokerType, revokerName); + } + + protected void beforeExecuteSQL() { + // Do nothing by default. + } + + @VisibleForTesting + Connection getConnection() throws SQLException { + return dataSource.getConnection(); + } + + protected void executeUpdateSQL(String sql) { + executeUpdateSQL(sql, null); + } + + /** + * Convert the object name contains `*` to a list of AuthorizationSecurableObject. + * + * @param object The + * @return + */ + protected List convertResourceAll( + AuthorizationSecurableObject object) { + List authObjects = Lists.newArrayList(); + authObjects.add(object); + return authObjects; + } + + protected List filterUnsupportedPrivileges( + List privileges) { + return privileges; + } + + protected AuthorizationPluginException toAuthorizationPluginException(SQLException se) { + return new AuthorizationPluginException( + "Jdbc authorization plugin fail to execute SQL, error code: %d", se.getErrorCode()); + } + + void executeUpdateSQL(String sql, String ignoreErrorMsg) { + try (final Connection connection = getConnection()) { + try (final Statement statement = connection.createStatement()) { + statement.executeUpdate(sql); + } + } catch (SQLException se) { + if (ignoreErrorMsg != null && se.getMessage().contains(ignoreErrorMsg)) { + return; + } + LOG.error("Jdbc authorization plugin exception: ", se); + throw toAuthorizationPluginException(se); + } + } + + private void grantObjectPrivileges(Role role, SecurableObject object) { + List authObjects = mappingProvider.translatePrivilege(object); + for (AuthorizationSecurableObject authObject : authObjects) { + List convertedObjects = Lists.newArrayList(); + if (authObject.name().equals(JdbcAuthorizationObject.ALL)) { + convertedObjects.addAll(convertResourceAll(authObject)); + } else { + convertedObjects.add(authObject); + } + + for (AuthorizationSecurableObject convertedObject : convertedObjects) { + List privileges = + filterUnsupportedPrivileges(authObject.privileges()).stream() + .map(AuthorizationPrivilege::getName) + .collect(Collectors.toList()); + // We don't grant the privileges in one SQL, because some privilege has been granted, it + // will cause the failure of the SQL. So we grant the privileges one by one. + for (String privilege : privileges) { + String sql = + getGrantPrivilegeSQL( + privilege, + convertedObject.metadataObjectType().name(), + convertedObject.fullName(), + role.name()); + executeUpdateSQL(sql, "is already granted"); + } + } + } + } + + private void revokeObjectPrivileges(Role role, SecurableObject removeObject) { + List authObjects = + mappingProvider.translatePrivilege(removeObject); + for (AuthorizationSecurableObject authObject : authObjects) { + List convertedObjects = Lists.newArrayList(); + if (authObject.name().equals(JdbcAuthorizationObject.ALL)) { + convertedObjects.addAll(convertResourceAll(authObject)); + } else { + convertedObjects.add(authObject); + } + + for (AuthorizationSecurableObject convertedObject : convertedObjects) { + List privileges = + filterUnsupportedPrivileges(authObject.privileges()).stream() + .map(AuthorizationPrivilege::getName) + .collect(Collectors.toList()); + for (String privilege : privileges) { + // We don't revoke the privileges in one SQL, because some privilege has been revoked, it + // will cause the failure of the SQL. So we revoke the privileges one by one. + String sql = + getRevokePrivilegeSQL( + privilege, + convertedObject.metadataObjectType().name(), + convertedObject.fullName(), + role.name()); + executeUpdateSQL(sql, "Cannot find privilege Privilege"); + } + } + } + } +} diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java new file mode 100644 index 00000000000..b998f9a3bf1 --- /dev/null +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java @@ -0,0 +1,218 @@ +/* + * 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.authorization.jdbc; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.MetadataObjects; +import org.apache.gravitino.authorization.AuthorizationMetadataObject; +import org.apache.gravitino.authorization.AuthorizationPrivilege; +import org.apache.gravitino.authorization.AuthorizationPrivilegesMappingProvider; +import org.apache.gravitino.authorization.AuthorizationSecurableObject; +import org.apache.gravitino.authorization.Privilege; +import org.apache.gravitino.authorization.SecurableObject; + +/** + * JdbcSecurableObjectMappingProvider is used for translating securable object to authorization + * securable object. + */ +public class JdbcSecurableObjectMappingProvider implements AuthorizationPrivilegesMappingProvider { + + private final Map> privilegeMapping = + ImmutableMap.of( + Privilege.Name.CREATE_TABLE, + Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.CREATE)), + Privilege.Name.CREATE_SCHEMA, + Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.CREATE)), + Privilege.Name.SELECT_TABLE, + Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.SELECT)), + Privilege.Name.MODIFY_TABLE, + Sets.newHashSet( + JdbcPrivilege.valueOf(JdbcPrivilege.Type.SELECT), + JdbcPrivilege.valueOf(JdbcPrivilege.Type.UPDATE), + JdbcPrivilege.valueOf(JdbcPrivilege.Type.DELETE), + JdbcPrivilege.valueOf(JdbcPrivilege.Type.INSERT), + JdbcPrivilege.valueOf(JdbcPrivilege.Type.ALTER)), + Privilege.Name.USE_SCHEMA, + Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.USAGE))); + + private final Map privilegeScopeMapping = + ImmutableMap.of( + Privilege.Name.CREATE_TABLE, PrivilegeScope.TABLE, + Privilege.Name.CREATE_SCHEMA, PrivilegeScope.DATABASE, + Privilege.Name.SELECT_TABLE, PrivilegeScope.TABLE, + Privilege.Name.MODIFY_TABLE, PrivilegeScope.TABLE, + Privilege.Name.USE_SCHEMA, PrivilegeScope.DATABASE); + + private final Set ownerPrivileges = ImmutableSet.of(); + + private final Set allowObjectTypes = + ImmutableSet.of( + MetadataObject.Type.METALAKE, + MetadataObject.Type.CATALOG, + MetadataObject.Type.SCHEMA, + MetadataObject.Type.TABLE); + + @Override + public Map> privilegesMappingRule() { + return privilegeMapping; + } + + @Override + public Set ownerMappingRule() { + return ownerPrivileges; + } + + @Override + public Set allowPrivilegesRule() { + return privilegeMapping.keySet(); + } + + @Override + public Set allowMetadataObjectTypesRule() { + return allowObjectTypes; + } + + @Override + public List translatePrivilege(SecurableObject securableObject) { + List authObjects = Lists.newArrayList(); + List databasePrivileges = Lists.newArrayList(); + List tablePrivileges = Lists.newArrayList(); + JdbcAuthorizationObject databaseObject; + JdbcAuthorizationObject tableObject; + switch (securableObject.type()) { + case METALAKE: + case CATALOG: + convertPluginPrivileges(securableObject, databasePrivileges, tablePrivileges); + + if (!databasePrivileges.isEmpty()) { + databaseObject = + new JdbcAuthorizationObject(JdbcAuthorizationObject.ALL, null, databasePrivileges); + authObjects.add(databaseObject); + } + + if (!tablePrivileges.isEmpty()) { + tableObject = + new JdbcAuthorizationObject( + JdbcAuthorizationObject.ALL, JdbcAuthorizationObject.ALL, tablePrivileges); + authObjects.add(tableObject); + } + break; + + case SCHEMA: + convertPluginPrivileges(securableObject, databasePrivileges, tablePrivileges); + if (!databasePrivileges.isEmpty()) { + databaseObject = + new JdbcAuthorizationObject(securableObject.name(), null, databasePrivileges); + authObjects.add(databaseObject); + } + + if (!tablePrivileges.isEmpty()) { + tableObject = + new JdbcAuthorizationObject( + securableObject.name(), JdbcAuthorizationObject.ALL, tablePrivileges); + authObjects.add(tableObject); + } + break; + + case TABLE: + convertPluginPrivileges(securableObject, databasePrivileges, tablePrivileges); + if (!tablePrivileges.isEmpty()) { + MetadataObject metadataObject = + MetadataObjects.parse(securableObject.parent(), MetadataObject.Type.SCHEMA); + tableObject = + new JdbcAuthorizationObject( + metadataObject.name(), securableObject.name(), tablePrivileges); + authObjects.add(tableObject); + } + break; + + default: + throw new IllegalArgumentException( + String.format("Don't support metadata object type %s", securableObject.type())); + } + + return authObjects; + } + + @Override + public List translateOwner(MetadataObject metadataObject) { + List objects = Lists.newArrayList(); + JdbcPrivilege allPri = JdbcPrivilege.valueOf(JdbcPrivilege.Type.ALL); + switch (metadataObject.type()) { + case METALAKE: + case CATALOG: + objects.add( + new JdbcAuthorizationObject( + JdbcAuthorizationObject.ALL, null, Lists.newArrayList(allPri))); + objects.add( + new JdbcAuthorizationObject( + JdbcAuthorizationObject.ALL, + JdbcAuthorizationObject.ALL, + Lists.newArrayList(allPri))); + break; + case SCHEMA: + objects.add( + new JdbcAuthorizationObject(metadataObject.name(), null, Lists.newArrayList(allPri))); + objects.add( + new JdbcAuthorizationObject( + metadataObject.name(), JdbcAuthorizationObject.ALL, Lists.newArrayList(allPri))); + break; + case TABLE: + MetadataObject schema = + MetadataObjects.parse(metadataObject.parent(), MetadataObject.Type.SCHEMA); + objects.add( + new JdbcAuthorizationObject( + schema.name(), metadataObject.name(), Lists.newArrayList(allPri))); + break; + default: + throw new IllegalArgumentException(""); + } + return objects; + } + + @Override + public AuthorizationMetadataObject translateMetadataObject(MetadataObject metadataObject) { + throw new UnsupportedOperationException("Not supported"); + } + + private void convertPluginPrivileges( + SecurableObject securableObject, + List databasePrivileges, + List tablePrivileges) { + for (Privilege privilege : securableObject.privileges()) { + if (privilegeScopeMapping.get(privilege.name()) == PrivilegeScope.DATABASE) { + databasePrivileges.addAll(privilegeMapping.get(privilege.name())); + } else if (privilegeScopeMapping.get(privilege.name()) == PrivilegeScope.TABLE) { + tablePrivileges.addAll(privilegeMapping.get(privilege.name())); + } + } + } + + enum PrivilegeScope { + DATABASE, + TABLE + } +} diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryGroup.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryGroup.java new file mode 100644 index 00000000000..98f72354fa2 --- /dev/null +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryGroup.java @@ -0,0 +1,51 @@ +/* + * 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.authorization.jdbc; + +import com.google.common.collect.ImmutableList; +import java.util.List; +import org.apache.gravitino.Audit; +import org.apache.gravitino.authorization.Group; +import org.apache.gravitino.meta.AuditInfo; + +/* + * TemporaryGroup is used for translating the owner to the SQL. + */ +class TemporaryGroup implements Group { + private final String name; + + TemporaryGroup(String name) { + this.name = name; + } + + @Override + public String name() { + return name; + } + + @Override + public List roles() { + return ImmutableList.of(); + } + + @Override + public Audit auditInfo() { + return AuditInfo.EMPTY; + } +} diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryUser.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryUser.java new file mode 100644 index 00000000000..aa47710bf75 --- /dev/null +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryUser.java @@ -0,0 +1,51 @@ +/* + * 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.authorization.jdbc; + +import com.google.common.collect.ImmutableList; +import java.util.List; +import org.apache.gravitino.Audit; +import org.apache.gravitino.authorization.User; +import org.apache.gravitino.meta.AuditInfo; + +/* + * TemporaryUser is used for translating the owner to the SQL. + */ +class TemporaryUser implements User { + private final String name; + + TemporaryUser(String name) { + this.name = name; + } + + @Override + public String name() { + return name; + } + + @Override + public List roles() { + return ImmutableList.of(); + } + + @Override + public Audit auditInfo() { + return AuditInfo.EMPTY; + } +} diff --git a/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPluginTest.java b/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPluginTest.java new file mode 100644 index 00000000000..58e335284a0 --- /dev/null +++ b/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPluginTest.java @@ -0,0 +1,331 @@ +/* + * 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.authorization.jdbc; + +import com.google.common.collect.Lists; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.apache.gravitino.Audit; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.MetadataObjects; +import org.apache.gravitino.authorization.Group; +import org.apache.gravitino.authorization.Owner; +import org.apache.gravitino.authorization.Privileges; +import org.apache.gravitino.authorization.Role; +import org.apache.gravitino.authorization.RoleChange; +import org.apache.gravitino.authorization.SecurableObject; +import org.apache.gravitino.authorization.SecurableObjects; +import org.apache.gravitino.authorization.User; +import org.apache.gravitino.meta.AuditInfo; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class JdbcSQLBasedAuthorizationPluginTest { + private static List expectSQLs = Lists.newArrayList(); + private static List expectTypes = Lists.newArrayList(); + private static List expectObjectNames = Lists.newArrayList(); + private static List> expectPreOwners = Lists.newArrayList(); + private static List expectNewOwners = Lists.newArrayList(); + private static int currentSQLIndex = 0; + private static int currentIndex = 0; + + private static final JdbcSQLBasedAuthorizationPlugin plugin = + new JdbcSQLBasedAuthorizationPlugin(Collections.emptyMap()) { + + @Override + public List getSetOwnerSQL( + MetadataObject.Type type, String objectName, Owner preOwner, Owner newOwner) { + Assertions.assertEquals(expectTypes.get(currentIndex), type); + Assertions.assertEquals(expectObjectNames.get(currentIndex), objectName); + Assertions.assertEquals(expectPreOwners.get(currentIndex), Optional.ofNullable(preOwner)); + Assertions.assertEquals(expectNewOwners.get(currentIndex), newOwner); + currentIndex++; + return Collections.emptyList(); + } + + void executeUpdateSQL(String sql, String ignoreErrorMsg) { + Assertions.assertEquals(expectSQLs.get(currentSQLIndex), sql); + currentSQLIndex++; + } + }; + + @Test + public void testUserManagement() { + expectSQLs = Lists.newArrayList("CREATE USER tmp"); + currentSQLIndex = 0; + plugin.onUserAdded(new TemporaryUser("tmp")); + + Assertions.assertThrows( + UnsupportedOperationException.class, () -> plugin.onUserAcquired(new TemporaryUser("tmp"))); + + expectSQLs = Lists.newArrayList("DROP USER tmp"); + currentSQLIndex = 0; + plugin.onUserRemoved(new TemporaryUser("tmp")); + } + + @Test + public void testGroupManagement() { + expectSQLs = Lists.newArrayList("CREATE USER GRAVITINO_GROUP_tmp"); + currentSQLIndex = 0; + plugin.onGroupAdded(new TemporaryGroup("tmp")); + + Assertions.assertThrows( + UnsupportedOperationException.class, + () -> plugin.onGroupAcquired(new TemporaryGroup("tmp"))); + + expectSQLs = Lists.newArrayList("DROP USER GRAVITINO_GROUP_tmp"); + currentSQLIndex = 0; + plugin.onGroupRemoved(new TemporaryGroup("tmp")); + } + + @Test + public void testRoleManagement() { + expectSQLs = Lists.newArrayList("CREATE ROLE tmp"); + currentSQLIndex = 0; + Role role = new TemporaryRole("tmp"); + plugin.onRoleCreated(role); + + Assertions.assertThrows(UnsupportedOperationException.class, () -> plugin.onRoleAcquired(role)); + + currentSQLIndex = 0; + expectSQLs = Lists.newArrayList("DROP ROLE tmp"); + plugin.onRoleDeleted(role); + } + + @Test + public void testPermissionManagement() { + Role role = new TemporaryRole("tmp"); + Group group = new TemporaryGroup("tmp"); + User user = new TemporaryUser("tmp"); + + currentSQLIndex = 0; + expectSQLs = + Lists.newArrayList( + "CREATE USER GRAVITINO_GROUP_tmp", + "CREATE ROLE tmp", + "GRANT ROLE tmp TO USER GRAVITINO_GROUP_tmp"); + plugin.onGrantedRolesToGroup(Lists.newArrayList(role), group); + + currentSQLIndex = 0; + expectSQLs = + Lists.newArrayList("CREATE USER tmp", "CREATE ROLE tmp", "GRANT ROLE tmp TO USER tmp"); + plugin.onGrantedRolesToUser(Lists.newArrayList(role), user); + + currentSQLIndex = 0; + expectSQLs = + Lists.newArrayList( + "CREATE USER GRAVITINO_GROUP_tmp", + "CREATE ROLE tmp", + "REVOKE ROLE tmp FROM USER GRAVITINO_GROUP_tmp"); + plugin.onRevokedRolesFromGroup(Lists.newArrayList(role), group); + + currentSQLIndex = 0; + expectSQLs = + Lists.newArrayList("CREATE USER tmp", "CREATE ROLE tmp", "REVOKE ROLE tmp FROM USER tmp"); + plugin.onRevokedRolesFromUser(Lists.newArrayList(role), user); + + // Test metalake object and different role change + currentSQLIndex = 0; + expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE *.* TO ROLE tmp"); + SecurableObject metalakeObject = + SecurableObjects.ofMetalake("metalake", Lists.newArrayList(Privileges.SelectTable.allow())); + RoleChange roleChange = RoleChange.addSecurableObject("tmp", metalakeObject); + plugin.onRoleUpdated(role, roleChange); + + currentSQLIndex = 0; + expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "REVOKE SELECT ON TABLE *.* FROM ROLE tmp"); + roleChange = RoleChange.removeSecurableObject("tmp", metalakeObject); + plugin.onRoleUpdated(role, roleChange); + + currentSQLIndex = 0; + expectSQLs = + Lists.newArrayList( + "CREATE ROLE tmp", + "REVOKE SELECT ON TABLE *.* FROM ROLE tmp", + "GRANT CREATE ON TABLE *.* TO ROLE tmp"); + SecurableObject newMetalakeObject = + SecurableObjects.ofMetalake("metalake", Lists.newArrayList(Privileges.CreateTable.allow())); + roleChange = RoleChange.updateSecurableObject("tmp", metalakeObject, newMetalakeObject); + plugin.onRoleUpdated(role, roleChange); + + // Test catalog object + currentSQLIndex = 0; + SecurableObject catalogObject = + SecurableObjects.ofCatalog("catalog", Lists.newArrayList(Privileges.SelectTable.allow())); + roleChange = RoleChange.addSecurableObject("tmp", catalogObject); + expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE *.* TO ROLE tmp"); + plugin.onRoleUpdated(role, roleChange); + + // Test schema object + currentSQLIndex = 0; + SecurableObject schemaObject = + SecurableObjects.ofSchema( + catalogObject, "schema", Lists.newArrayList(Privileges.SelectTable.allow())); + roleChange = RoleChange.addSecurableObject("tmp", schemaObject); + expectSQLs = + Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE schema.* TO ROLE tmp"); + plugin.onRoleUpdated(role, roleChange); + + // Test table object + currentSQLIndex = 0; + SecurableObject tableObject = + SecurableObjects.ofTable( + schemaObject, "table", Lists.newArrayList(Privileges.SelectTable.allow())); + roleChange = RoleChange.addSecurableObject("tmp", tableObject); + expectSQLs = + Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE schema.table TO ROLE tmp"); + plugin.onRoleUpdated(role, roleChange); + } + + @Test + public void testOwnerManagement() { + + // Test metalake object + Owner owner = new TemporaryOwner("tmp", Owner.Type.USER); + MetadataObject metalakeObject = + MetadataObjects.of(null, "metalake", MetadataObject.Type.METALAKE); + expectSQLs = Lists.newArrayList("CREATE USER tmp"); + currentSQLIndex = 0; + expectTypes.add(MetadataObject.Type.SCHEMA); + expectObjectNames.add("*"); + expectPreOwners.add(Optional.empty()); + expectNewOwners.add(owner); + + expectTypes.add(MetadataObject.Type.TABLE); + expectObjectNames.add("*.*"); + expectPreOwners.add(Optional.empty()); + expectNewOwners.add(owner); + plugin.onOwnerSet(metalakeObject, null, owner); + + // clean up + expectTypes.clear(); + expectObjectNames.clear(); + expectPreOwners.clear(); + expectNewOwners.clear(); + currentIndex = 0; + expectSQLs = Lists.newArrayList("CREATE USER tmp"); + currentSQLIndex = 0; + + // Test catalog object + MetadataObject catalogObject = MetadataObjects.of(null, "catalog", MetadataObject.Type.CATALOG); + expectTypes.add(MetadataObject.Type.SCHEMA); + expectObjectNames.add("*"); + expectPreOwners.add(Optional.empty()); + expectNewOwners.add(owner); + + expectTypes.add(MetadataObject.Type.TABLE); + expectObjectNames.add("*.*"); + expectPreOwners.add(Optional.empty()); + expectNewOwners.add(owner); + plugin.onOwnerSet(catalogObject, null, owner); + + // clean up + expectTypes.clear(); + expectObjectNames.clear(); + expectPreOwners.clear(); + expectNewOwners.clear(); + currentIndex = 0; + expectSQLs = Lists.newArrayList("CREATE USER tmp"); + currentSQLIndex = 0; + + // Test schema object + MetadataObject schemaObject = + MetadataObjects.of("catalog", "schema", MetadataObject.Type.SCHEMA); + expectTypes.add(MetadataObject.Type.SCHEMA); + expectObjectNames.add("schema"); + expectPreOwners.add(Optional.empty()); + expectNewOwners.add(owner); + + expectTypes.add(MetadataObject.Type.TABLE); + expectObjectNames.add("schema.*"); + expectPreOwners.add(Optional.empty()); + expectNewOwners.add(owner); + plugin.onOwnerSet(schemaObject, null, owner); + + // clean up + expectTypes.clear(); + expectObjectNames.clear(); + expectPreOwners.clear(); + expectNewOwners.clear(); + currentIndex = 0; + expectSQLs = Lists.newArrayList("CREATE USER tmp"); + currentSQLIndex = 0; + + // Test table object + MetadataObject tableObject = + MetadataObjects.of( + Lists.newArrayList("catalog", "schema", "table"), MetadataObject.Type.TABLE); + + expectTypes.add(MetadataObject.Type.TABLE); + expectObjectNames.add("schema.table"); + expectPreOwners.add(Optional.empty()); + expectNewOwners.add(owner); + plugin.onOwnerSet(tableObject, null, owner); + } + + private static class TemporaryRole implements Role { + private final String name; + + public TemporaryRole(String name) { + this.name = name; + } + + @Override + public Audit auditInfo() { + return AuditInfo.EMPTY; + } + + @Override + public String name() { + return name; + } + + @Override + public Map properties() { + return Collections.emptyMap(); + } + + @Override + public List securableObjects() { + return Collections.emptyList(); + } + } + + private static class TemporaryOwner implements Owner { + private final String name; + private final Type type; + + public TemporaryOwner(String name, Type type) { + this.name = name; + this.type = type; + } + + @Override + public String name() { + return name; + } + + @Override + public Type type() { + return type; + } + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 150acdb00ce..b3eb56578aa 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -57,7 +57,7 @@ if (gradle.startParameter.projectProperties["enableFuse"]?.toBoolean() == true) } include("iceberg:iceberg-common") include("iceberg:iceberg-rest-server") -include("authorizations:authorization-ranger") +include("authorizations:authorization-ranger", "authorizations:authorization-jdbc") include("trino-connector:trino-connector", "trino-connector:integration-test") include("spark-connector:spark-common") // kyuubi hive connector doesn't support 2.13 for Spark3.3 From 86bd49a313aed35562e3dbd71c70847204b25df4 Mon Sep 17 00:00:00 2001 From: Rory Date: Thu, 19 Dec 2024 17:03:57 +0800 Subject: [PATCH 02/10] address comments --- .../authorization/jdbc/JdbcPrivilege.java | 36 +++++++++---------- .../jdbc/JdbcSQLBasedAuthorizationPlugin.java | 4 +-- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java index 118d61f09de..dce14161dbd 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java @@ -23,15 +23,15 @@ public class JdbcPrivilege implements AuthorizationPrivilege { - private static final JdbcPrivilege SELECT_PRI = new JdbcPrivilege(Type.SELECT); - private static final JdbcPrivilege INSERT_PRI = new JdbcPrivilege(Type.INSERT); - private static final JdbcPrivilege UPDATE_PRI = new JdbcPrivilege(Type.UPDATE); - private static final JdbcPrivilege ALTER_PRI = new JdbcPrivilege(Type.ALTER); - private static final JdbcPrivilege DELETE_PRI = new JdbcPrivilege(Type.DELETE); - private static final JdbcPrivilege ALL_PRI = new JdbcPrivilege(Type.ALL); - private static final JdbcPrivilege CREATE_PRI = new JdbcPrivilege(Type.CREATE); - private static final JdbcPrivilege DROP_PRI = new JdbcPrivilege(Type.DROP); - private static final JdbcPrivilege USAGE_PRI = new JdbcPrivilege(Type.USAGE); + private static final JdbcPrivilege SELECT_PRIVILEGE = new JdbcPrivilege(Type.SELECT); + private static final JdbcPrivilege INSERT_PRIVILEGE = new JdbcPrivilege(Type.INSERT); + private static final JdbcPrivilege UPDATE_PRIVILEGE = new JdbcPrivilege(Type.UPDATE); + private static final JdbcPrivilege ALTER_PRIVILEGE = new JdbcPrivilege(Type.ALTER); + private static final JdbcPrivilege DELETE_PRIVILEGE = new JdbcPrivilege(Type.DELETE); + private static final JdbcPrivilege ALL_PRIVILEGE = new JdbcPrivilege(Type.ALL); + private static final JdbcPrivilege CREATE_PRIVILEGE = new JdbcPrivilege(Type.CREATE); + private static final JdbcPrivilege DROP_PRIVILEGE = new JdbcPrivilege(Type.DROP); + private static final JdbcPrivilege USAGE_PRIVILEGE = new JdbcPrivilege(Type.USAGE); private final Type type; @@ -57,23 +57,23 @@ public boolean equalsTo(String value) { static JdbcPrivilege valueOf(Type type) { switch (type) { case CREATE: - return CREATE_PRI; + return CREATE_PRIVILEGE; case DELETE: - return DELETE_PRI; + return DELETE_PRIVILEGE; case ALL: - return ALL_PRI; + return ALL_PRIVILEGE; case DROP: - return DROP_PRI; + return DROP_PRIVILEGE; case ALTER: - return ALTER_PRI; + return ALTER_PRIVILEGE; case INSERT: - return INSERT_PRI; + return INSERT_PRIVILEGE; case UPDATE: - return UPDATE_PRI; + return UPDATE_PRIVILEGE; case SELECT: - return SELECT_PRI; + return SELECT_PRIVILEGE; case USAGE: - return USAGE_PRI; + return USAGE_PRIVILEGE; default: throw new IllegalArgumentException(String.format("Unsupported parameter type %s", type)); } diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java index 0aff510e8e2..50e5dc1a2f8 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java @@ -359,8 +359,8 @@ protected void executeUpdateSQL(String sql) { /** * Convert the object name contains `*` to a list of AuthorizationSecurableObject. * - * @param object The - * @return + * @param object The object contains the name with `*` to be converted + * @return The list of AuthorizationSecurableObject */ protected List convertResourceAll( AuthorizationSecurableObject object) { From 73e900d0f0e5d90c741796a97311e416db3ced8d Mon Sep 17 00:00:00 2001 From: Rory Date: Thu, 19 Dec 2024 20:40:51 +0800 Subject: [PATCH 03/10] address comment --- .../authorization-jdbc/build.gradle.kts | 3 -- ...ugin.java => JdbcAuthorizationPlugin.java} | 24 ++++++--- .../JdbcSecurableObjectMappingProvider.java | 29 +++++------ .../authorization/jdbc/TemporaryGroup.java | 51 ------------------- .../authorization/jdbc/TemporaryUser.java | 51 ------------------- ....java => JdbcAuthorizationPluginTest.java} | 33 +++++++----- 6 files changed, 51 insertions(+), 140 deletions(-) rename authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/{JdbcSQLBasedAuthorizationPlugin.java => JdbcAuthorizationPlugin.java} (95%) delete mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryGroup.java delete mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryUser.java rename authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/{JdbcSQLBasedAuthorizationPluginTest.java => JdbcAuthorizationPluginTest.java} (92%) diff --git a/authorizations/authorization-jdbc/build.gradle.kts b/authorizations/authorization-jdbc/build.gradle.kts index 2649370d8cf..8b105908c26 100644 --- a/authorizations/authorization-jdbc/build.gradle.kts +++ b/authorizations/authorization-jdbc/build.gradle.kts @@ -82,9 +82,6 @@ tasks { } tasks.test { - doFirst { - environment("HADOOP_USER_NAME", "gravitino") - } dependsOn(":catalogs:catalog-hive:jar", ":catalogs:catalog-hive:runtimeJars") val skipITs = project.hasProperty("skipITs") diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java similarity index 95% rename from authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java rename to authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java index 50e5dc1a2f8..f445d1107af 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPlugin.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java @@ -42,6 +42,9 @@ import org.apache.gravitino.authorization.User; import org.apache.gravitino.connector.authorization.AuthorizationPlugin; import org.apache.gravitino.exceptions.AuthorizationPluginException; +import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.GroupEntity; +import org.apache.gravitino.meta.UserEntity; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,8 +54,7 @@ * JDBC-based authorization plugins can inherit this class and implement their own SQL statements. */ @Unstable -abstract class JdbcSQLBasedAuthorizationPlugin - implements AuthorizationPlugin, JdbcAuthorizationSQL { +abstract class JdbcAuthorizationPlugin implements AuthorizationPlugin, JdbcAuthorizationSQL { private static final String CONFIG_PREFIX = "authorization.jdbc."; public static final String JDBC_DRIVER = CONFIG_PREFIX + "driver"; @@ -61,12 +63,12 @@ abstract class JdbcSQLBasedAuthorizationPlugin public static final String JDBC_PASSWORD = CONFIG_PREFIX + "password"; private static final String GROUP_PREFIX = "GRAVITINO_GROUP_"; - private static final Logger LOG = LoggerFactory.getLogger(JdbcSQLBasedAuthorizationPlugin.class); + private static final Logger LOG = LoggerFactory.getLogger(JdbcAuthorizationPlugin.class); protected BasicDataSource dataSource; protected JdbcSecurableObjectMappingProvider mappingProvider; - public JdbcSQLBasedAuthorizationPlugin(Map config) { + public JdbcAuthorizationPlugin(Map config) { // Initialize the data source dataSource = new BasicDataSource(); String jdbcUrl = config.get(JDBC_URL); @@ -279,9 +281,19 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n beforeExecuteSQL(); if (newOwner.type() == Owner.Type.USER) { - onUserAdded(new TemporaryUser(newOwner.name())); + onUserAdded( + UserEntity.builder() + .withName(newOwner.name()) + .withId(0L) + .withAuditInfo(AuditInfo.EMPTY) + .build()); } else if (newOwner.type() == Owner.Type.GROUP) { - onGroupAdded(new TemporaryGroup(newOwner.name())); + onGroupAdded( + GroupEntity.builder() + .withName(newOwner.name()) + .withId(0L) + .withAuditInfo(AuditInfo.EMPTY) + .build()); } else { throw new IllegalArgumentException( String.format("Don't support owner type %s", newOwner.type())); diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java index b998f9a3bf1..9610c902d7f 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java @@ -58,13 +58,13 @@ public class JdbcSecurableObjectMappingProvider implements AuthorizationPrivileg Privilege.Name.USE_SCHEMA, Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.USAGE))); - private final Map privilegeScopeMapping = + private final Map privilegeScopeMapping = ImmutableMap.of( - Privilege.Name.CREATE_TABLE, PrivilegeScope.TABLE, - Privilege.Name.CREATE_SCHEMA, PrivilegeScope.DATABASE, - Privilege.Name.SELECT_TABLE, PrivilegeScope.TABLE, - Privilege.Name.MODIFY_TABLE, PrivilegeScope.TABLE, - Privilege.Name.USE_SCHEMA, PrivilegeScope.DATABASE); + Privilege.Name.CREATE_TABLE, MetadataObject.Type.TABLE, + Privilege.Name.CREATE_SCHEMA, MetadataObject.Type.SCHEMA, + Privilege.Name.SELECT_TABLE, MetadataObject.Type.TABLE, + Privilege.Name.MODIFY_TABLE, MetadataObject.Type.TABLE, + Privilege.Name.USE_SCHEMA, MetadataObject.Type.SCHEMA); private final Set ownerPrivileges = ImmutableSet.of(); @@ -105,7 +105,7 @@ public List translatePrivilege(SecurableObject sec switch (securableObject.type()) { case METALAKE: case CATALOG: - convertPluginPrivileges(securableObject, databasePrivileges, tablePrivileges); + convertJdbcPrivileges(securableObject, databasePrivileges, tablePrivileges); if (!databasePrivileges.isEmpty()) { databaseObject = @@ -122,7 +122,7 @@ public List translatePrivilege(SecurableObject sec break; case SCHEMA: - convertPluginPrivileges(securableObject, databasePrivileges, tablePrivileges); + convertJdbcPrivileges(securableObject, databasePrivileges, tablePrivileges); if (!databasePrivileges.isEmpty()) { databaseObject = new JdbcAuthorizationObject(securableObject.name(), null, databasePrivileges); @@ -138,7 +138,7 @@ public List translatePrivilege(SecurableObject sec break; case TABLE: - convertPluginPrivileges(securableObject, databasePrivileges, tablePrivileges); + convertJdbcPrivileges(securableObject, databasePrivileges, tablePrivileges); if (!tablePrivileges.isEmpty()) { MetadataObject metadataObject = MetadataObjects.parse(securableObject.parent(), MetadataObject.Type.SCHEMA); @@ -198,21 +198,16 @@ public AuthorizationMetadataObject translateMetadataObject(MetadataObject metada throw new UnsupportedOperationException("Not supported"); } - private void convertPluginPrivileges( + private void convertJdbcPrivileges( SecurableObject securableObject, List databasePrivileges, List tablePrivileges) { for (Privilege privilege : securableObject.privileges()) { - if (privilegeScopeMapping.get(privilege.name()) == PrivilegeScope.DATABASE) { + if (privilegeScopeMapping.get(privilege.name()) == MetadataObject.Type.SCHEMA) { databasePrivileges.addAll(privilegeMapping.get(privilege.name())); - } else if (privilegeScopeMapping.get(privilege.name()) == PrivilegeScope.TABLE) { + } else if (privilegeScopeMapping.get(privilege.name()) == MetadataObject.Type.TABLE) { tablePrivileges.addAll(privilegeMapping.get(privilege.name())); } } } - - enum PrivilegeScope { - DATABASE, - TABLE - } } diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryGroup.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryGroup.java deleted file mode 100644 index 98f72354fa2..00000000000 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryGroup.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * 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.authorization.jdbc; - -import com.google.common.collect.ImmutableList; -import java.util.List; -import org.apache.gravitino.Audit; -import org.apache.gravitino.authorization.Group; -import org.apache.gravitino.meta.AuditInfo; - -/* - * TemporaryGroup is used for translating the owner to the SQL. - */ -class TemporaryGroup implements Group { - private final String name; - - TemporaryGroup(String name) { - this.name = name; - } - - @Override - public String name() { - return name; - } - - @Override - public List roles() { - return ImmutableList.of(); - } - - @Override - public Audit auditInfo() { - return AuditInfo.EMPTY; - } -} diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryUser.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryUser.java deleted file mode 100644 index aa47710bf75..00000000000 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/TemporaryUser.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * 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.authorization.jdbc; - -import com.google.common.collect.ImmutableList; -import java.util.List; -import org.apache.gravitino.Audit; -import org.apache.gravitino.authorization.User; -import org.apache.gravitino.meta.AuditInfo; - -/* - * TemporaryUser is used for translating the owner to the SQL. - */ -class TemporaryUser implements User { - private final String name; - - TemporaryUser(String name) { - this.name = name; - } - - @Override - public String name() { - return name; - } - - @Override - public List roles() { - return ImmutableList.of(); - } - - @Override - public Audit auditInfo() { - return AuditInfo.EMPTY; - } -} diff --git a/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPluginTest.java b/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java similarity index 92% rename from authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPluginTest.java rename to authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java index 58e335284a0..f4ecb1c0dd4 100644 --- a/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcSQLBasedAuthorizationPluginTest.java +++ b/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java @@ -35,10 +35,12 @@ import org.apache.gravitino.authorization.SecurableObjects; import org.apache.gravitino.authorization.User; import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.GroupEntity; +import org.apache.gravitino.meta.UserEntity; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -public class JdbcSQLBasedAuthorizationPluginTest { +public class JdbcAuthorizationPluginTest { private static List expectSQLs = Lists.newArrayList(); private static List expectTypes = Lists.newArrayList(); private static List expectObjectNames = Lists.newArrayList(); @@ -47,8 +49,8 @@ public class JdbcSQLBasedAuthorizationPluginTest { private static int currentSQLIndex = 0; private static int currentIndex = 0; - private static final JdbcSQLBasedAuthorizationPlugin plugin = - new JdbcSQLBasedAuthorizationPlugin(Collections.emptyMap()) { + private static final JdbcAuthorizationPlugin plugin = + new JdbcAuthorizationPlugin(Collections.emptyMap()) { @Override public List getSetOwnerSQL( @@ -71,29 +73,28 @@ void executeUpdateSQL(String sql, String ignoreErrorMsg) { public void testUserManagement() { expectSQLs = Lists.newArrayList("CREATE USER tmp"); currentSQLIndex = 0; - plugin.onUserAdded(new TemporaryUser("tmp")); + plugin.onUserAdded(createUser("tmp")); Assertions.assertThrows( - UnsupportedOperationException.class, () -> plugin.onUserAcquired(new TemporaryUser("tmp"))); + UnsupportedOperationException.class, () -> plugin.onUserAcquired(createUser("tmp"))); expectSQLs = Lists.newArrayList("DROP USER tmp"); currentSQLIndex = 0; - plugin.onUserRemoved(new TemporaryUser("tmp")); + plugin.onUserRemoved(createUser("tmp")); } @Test public void testGroupManagement() { expectSQLs = Lists.newArrayList("CREATE USER GRAVITINO_GROUP_tmp"); currentSQLIndex = 0; - plugin.onGroupAdded(new TemporaryGroup("tmp")); + plugin.onGroupAdded(createGroup("tmp")); Assertions.assertThrows( - UnsupportedOperationException.class, - () -> plugin.onGroupAcquired(new TemporaryGroup("tmp"))); + UnsupportedOperationException.class, () -> plugin.onGroupAcquired(createGroup("tmp"))); expectSQLs = Lists.newArrayList("DROP USER GRAVITINO_GROUP_tmp"); currentSQLIndex = 0; - plugin.onGroupRemoved(new TemporaryGroup("tmp")); + plugin.onGroupRemoved(createGroup("tmp")); } @Test @@ -113,8 +114,8 @@ public void testRoleManagement() { @Test public void testPermissionManagement() { Role role = new TemporaryRole("tmp"); - Group group = new TemporaryGroup("tmp"); - User user = new TemporaryUser("tmp"); + Group group = createGroup("tmp"); + User user = createUser("tmp"); currentSQLIndex = 0; expectSQLs = @@ -328,4 +329,12 @@ public Type type() { return type; } } + + private static Group createGroup(String name) { + return GroupEntity.builder().withId(0L).withName(name).withAuditInfo(AuditInfo.EMPTY).build(); + } + + private static User createUser(String name) { + return UserEntity.builder().withId(0L).withName(name).withAuditInfo(AuditInfo.EMPTY).build(); + } } From 04f9e3c565824dbe5ec848a8a28a7152fa33c062 Mon Sep 17 00:00:00 2001 From: Rory Date: Mon, 23 Dec 2024 12:21:21 +0800 Subject: [PATCH 04/10] address comment --- .../jdbc/JdbcAuthorizationObject.java | 2 +- .../jdbc/JdbcAuthorizationPlugin.java | 163 +++++++++--------- .../jdbc/JdbcAuthorizationProperties.java | 45 +++++ .../jdbc/JdbcAuthorizationSQL.java | 32 ++-- .../JdbcSecurableObjectMappingProvider.java | 15 +- .../jdbc/JdbcAuthorizationPluginTest.java | 107 +++++------- 6 files changed, 197 insertions(+), 167 deletions(-) create mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationProperties.java diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java index 37e64093225..c5bbdd0d622 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java @@ -40,7 +40,7 @@ public class JdbcAuthorizationObject implements AuthorizationSecurableObject { List privileges; JdbcAuthorizationObject(String database, String table, List privileges) { - Preconditions.checkNotNull(database, "Jdbc authorization object database can't null"); + Preconditions.checkNotNull(database, "JDBC authorization object database can't null"); this.database = database; this.table = table; this.privileges = privileges; diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java index f445d1107af..d209a6eb8fc 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java @@ -56,12 +56,6 @@ @Unstable abstract class JdbcAuthorizationPlugin implements AuthorizationPlugin, JdbcAuthorizationSQL { - private static final String CONFIG_PREFIX = "authorization.jdbc."; - public static final String JDBC_DRIVER = CONFIG_PREFIX + "driver"; - public static final String JDBC_URL = CONFIG_PREFIX + "url"; - public static final String JDBC_USERNAME = CONFIG_PREFIX + "username"; - public static final String JDBC_PASSWORD = CONFIG_PREFIX + "password"; - private static final String GROUP_PREFIX = "GRAVITINO_GROUP_"; private static final Logger LOG = LoggerFactory.getLogger(JdbcAuthorizationPlugin.class); @@ -71,11 +65,13 @@ abstract class JdbcAuthorizationPlugin implements AuthorizationPlugin, JdbcAutho public JdbcAuthorizationPlugin(Map config) { // Initialize the data source dataSource = new BasicDataSource(); - String jdbcUrl = config.get(JDBC_URL); + JdbcAuthorizationProperties.validate(config); + + String jdbcUrl = config.get(JdbcAuthorizationProperties.JDBC_URL); dataSource.setUrl(jdbcUrl); - dataSource.setDriverClassName(config.get(JDBC_DRIVER)); - dataSource.setUsername(config.get(JDBC_USERNAME)); - dataSource.setPassword(config.get(JDBC_PASSWORD)); + dataSource.setDriverClassName(config.get(JdbcAuthorizationProperties.JDBC_DRIVER)); + dataSource.setUsername(config.get(JdbcAuthorizationProperties.JDBC_USERNAME)); + dataSource.setPassword(config.get(JdbcAuthorizationProperties.JDBC_PASSWORD)); dataSource.setDefaultAutoCommit(true); dataSource.setMaxTotal(20); dataSource.setMaxIdle(5); @@ -95,6 +91,7 @@ public void close() throws IOException { if (dataSource != null) { try { dataSource.close(); + dataSource = null; } catch (SQLException e) { throw new RuntimeException(e); } @@ -111,10 +108,10 @@ public Boolean onMetadataUpdated(MetadataObjectChange... changes) throws Runtime @Override public Boolean onRoleCreated(Role role) throws AuthorizationPluginException { - beforeExecuteSQL(); - - String sql = getCreateRoleSQL(role.name()); - executeUpdateSQL(sql, "already exists"); + List sqls = getCreateRoleSQL(role.name()); + for (String sql : sqls) { + executeUpdateSQL(sql, "already exists"); + } if (role.securableObjects() != null) { for (SecurableObject object : role.securableObjects()) { @@ -132,18 +129,16 @@ public Boolean onRoleAcquired(Role role) throws AuthorizationPluginException { @Override public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException { - beforeExecuteSQL(); - - String sql = getDropRoleSQL(role.name()); - executeUpdateSQL(sql); + List sqls = getDropRoleSQL(role.name()); + for (String sql : sqls) { + executeUpdateSQL(sql); + } return null; } @Override public Boolean onRoleUpdated(Role role, RoleChange... changes) throws AuthorizationPluginException { - beforeExecuteSQL(); - onRoleCreated(role); for (RoleChange change : changes) { if (change instanceof RoleChange.AddSecurableObject) { @@ -159,7 +154,8 @@ public Boolean onRoleUpdated(Role role, RoleChange... changes) revokeObjectPrivileges(role, removeObject); grantObjectPrivileges(role, addObject); } else { - throw new IllegalArgumentException(String.format("Don't support RoleChange %s", change)); + throw new IllegalArgumentException( + String.format("RoleChange is not supported - %s", change)); } } return true; @@ -168,14 +164,14 @@ public Boolean onRoleUpdated(Role role, RoleChange... changes) @Override public Boolean onGrantedRolesToUser(List roles, User user) throws AuthorizationPluginException { - beforeExecuteSQL(); - onUserAdded(user); for (Role role : roles) { onRoleCreated(role); - String sql = getGrantRoleSQL(role.name(), "USER", user.name()); - executeUpdateSQL(sql); + List sqls = getGrantRoleSQL(role.name(), "USER", user.name()); + for (String sql : sqls) { + executeUpdateSQL(sql); + } } return true; } @@ -183,14 +179,14 @@ public Boolean onGrantedRolesToUser(List roles, User user) @Override public Boolean onRevokedRolesFromUser(List roles, User user) throws AuthorizationPluginException { - beforeExecuteSQL(); - onUserAdded(user); for (Role role : roles) { onRoleCreated(role); - String sql = getRevokeRoleSQL(role.name(), "USER", user.name()); - executeUpdateSQL(sql); + List sqls = getRevokeRoleSQL(role.name(), "USER", user.name()); + for (String sql : sqls) { + executeUpdateSQL(sql); + } } return true; } @@ -198,15 +194,15 @@ public Boolean onRevokedRolesFromUser(List roles, User user) @Override public Boolean onGrantedRolesToGroup(List roles, Group group) throws AuthorizationPluginException { - beforeExecuteSQL(); - onGroupAdded(group); for (Role role : roles) { onRoleCreated(role); - String sql = + List sqls = getGrantRoleSQL(role.name(), "USER", String.format("%s%s", GROUP_PREFIX, group.name())); - executeUpdateSQL(sql); + for (String sql : sqls) { + executeUpdateSQL(sql); + } } return true; } @@ -214,34 +210,34 @@ public Boolean onGrantedRolesToGroup(List roles, Group group) @Override public Boolean onRevokedRolesFromGroup(List roles, Group group) throws AuthorizationPluginException { - beforeExecuteSQL(); - onGroupAdded(group); for (Role role : roles) { onRoleCreated(role); - String sql = + List sqls = getRevokeRoleSQL(role.name(), "USER", String.format("%s%s", GROUP_PREFIX, group.name())); - executeUpdateSQL(sql); + for (String sql : sqls) { + executeUpdateSQL(sql); + } } return true; } @Override public Boolean onUserAdded(User user) throws AuthorizationPluginException { - beforeExecuteSQL(); - - String sql = getCreateUserSQL(user.name()); - executeUpdateSQL(sql); + List sqls = getCreateUserSQL(user.name()); + for (String sql : sqls) { + executeUpdateSQL(sql); + } return true; } @Override public Boolean onUserRemoved(User user) throws AuthorizationPluginException { - beforeExecuteSQL(); - - String sql = getDropUserSQL(user.name()); - executeUpdateSQL(sql); + List sqls = getDropUserSQL(user.name()); + for (String sql : sqls) { + executeUpdateSQL(sql); + } return true; } @@ -252,21 +248,21 @@ public Boolean onUserAcquired(User user) throws AuthorizationPluginException { @Override public Boolean onGroupAdded(Group group) throws AuthorizationPluginException { - beforeExecuteSQL(); - String name = String.format("%s%s", GROUP_PREFIX, group.name()); - String sql = getCreateUserSQL(name); - executeUpdateSQL(sql); + List sqls = getCreateUserSQL(name); + for (String sql : sqls) { + executeUpdateSQL(sql); + } return true; } @Override public Boolean onGroupRemoved(Group group) throws AuthorizationPluginException { - beforeExecuteSQL(); - String name = String.format("%s%s", GROUP_PREFIX, group.name()); - String sql = getDropUserSQL(name); - executeUpdateSQL(sql); + List sqls = getDropUserSQL(name); + for (String sql : sqls) { + executeUpdateSQL(sql); + } return true; } @@ -278,8 +274,6 @@ public Boolean onGroupAcquired(Group group) throws AuthorizationPluginException @Override public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner newOwner) throws AuthorizationPluginException { - beforeExecuteSQL(); - if (newOwner.type() == Owner.Type.USER) { onUserAdded( UserEntity.builder() @@ -312,51 +306,50 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n } @Override - public String getCreateUserSQL(String username) { - return String.format("CREATE USER %s", username); + public List getCreateUserSQL(String username) { + return Lists.newArrayList(String.format("CREATE USER %s", username)); } @Override - public String getDropUserSQL(String username) { - return String.format("DROP USER %s", username); + public List getDropUserSQL(String username) { + return Lists.newArrayList(String.format("DROP USER %s", username)); } @Override - public String getCreateRoleSQL(String roleName) { - return String.format("CREATE ROLE %s", roleName); + public List getCreateRoleSQL(String roleName) { + return Lists.newArrayList(String.format("CREATE ROLE %s", roleName)); } @Override - public String getDropRoleSQL(String roleName) { - return String.format("DROP ROLE %s", roleName); + public List getDropRoleSQL(String roleName) { + return Lists.newArrayList(String.format("DROP ROLE %s", roleName)); } @Override - public String getGrantPrivilegeSQL( + public List getGrantPrivilegeSQL( String privilege, String objectType, String objectName, String roleName) { - return String.format( - "GRANT %s ON %s %s TO ROLE %s", privilege, objectType, objectName, roleName); + return Lists.newArrayList( + String.format("GRANT %s ON %s %s TO ROLE %s", privilege, objectType, objectName, roleName)); } @Override - public String getRevokePrivilegeSQL( + public List getRevokePrivilegeSQL( String privilege, String objectType, String objectName, String roleName) { - return String.format( - "REVOKE %s ON %s %s FROM ROLE %s", privilege, objectType, objectName, roleName); + return Lists.newArrayList( + String.format( + "REVOKE %s ON %s %s FROM ROLE %s", privilege, objectType, objectName, roleName)); } @Override - public String getGrantRoleSQL(String roleName, String grantorType, String grantorName) { - return String.format("GRANT ROLE %s TO %s %s", roleName, grantorType, grantorName); + public List getGrantRoleSQL(String roleName, String grantorType, String grantorName) { + return Lists.newArrayList( + String.format("GRANT ROLE %s TO %s %s", roleName, grantorType, grantorName)); } @Override - public String getRevokeRoleSQL(String roleName, String revokerType, String revokerName) { - return String.format("REVOKE ROLE %s FROM %s %s", roleName, revokerType, revokerName); - } - - protected void beforeExecuteSQL() { - // Do nothing by default. + public List getRevokeRoleSQL(String roleName, String revokerType, String revokerName) { + return Lists.newArrayList( + String.format("REVOKE ROLE %s FROM %s %s", roleName, revokerType, revokerName)); } @VisibleForTesting @@ -388,7 +381,7 @@ protected List filterUnsupportedPrivileges( protected AuthorizationPluginException toAuthorizationPluginException(SQLException se) { return new AuthorizationPluginException( - "Jdbc authorization plugin fail to execute SQL, error code: %d", se.getErrorCode()); + "JDBC authorization plugin fail to execute SQL, error code: %d", se.getErrorCode()); } void executeUpdateSQL(String sql, String ignoreErrorMsg) { @@ -400,7 +393,7 @@ void executeUpdateSQL(String sql, String ignoreErrorMsg) { if (ignoreErrorMsg != null && se.getMessage().contains(ignoreErrorMsg)) { return; } - LOG.error("Jdbc authorization plugin exception: ", se); + LOG.error("JDBC authorization plugin exception: ", se); throw toAuthorizationPluginException(se); } } @@ -423,13 +416,15 @@ private void grantObjectPrivileges(Role role, SecurableObject object) { // We don't grant the privileges in one SQL, because some privilege has been granted, it // will cause the failure of the SQL. So we grant the privileges one by one. for (String privilege : privileges) { - String sql = + List sqls = getGrantPrivilegeSQL( privilege, convertedObject.metadataObjectType().name(), convertedObject.fullName(), role.name()); - executeUpdateSQL(sql, "is already granted"); + for (String sql : sqls) { + executeUpdateSQL(sql, "is already granted"); + } } } } @@ -454,13 +449,15 @@ private void revokeObjectPrivileges(Role role, SecurableObject removeObject) { for (String privilege : privileges) { // We don't revoke the privileges in one SQL, because some privilege has been revoked, it // will cause the failure of the SQL. So we revoke the privileges one by one. - String sql = + List sqls = getRevokePrivilegeSQL( privilege, convertedObject.metadataObjectType().name(), convertedObject.fullName(), role.name()); - executeUpdateSQL(sql, "Cannot find privilege Privilege"); + for (String sql : sqls) { + executeUpdateSQL(sql, "Cannot find privilege Privilege"); + } } } } diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationProperties.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationProperties.java new file mode 100644 index 00000000000..4d325c6812e --- /dev/null +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationProperties.java @@ -0,0 +1,45 @@ +/* + * 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.authorization.jdbc; + +import java.util.Map; + +/** The properties for JDBC authorization plugin. */ +public class JdbcAuthorizationProperties { + private static final String CONFIG_PREFIX = "authorization.jdbc."; + public static final String JDBC_PASSWORD = CONFIG_PREFIX + "password"; + public static final String JDBC_USERNAME = CONFIG_PREFIX + "username"; + public static final String JDBC_URL = CONFIG_PREFIX + "url"; + public static final String JDBC_DRIVER = CONFIG_PREFIX + "driver"; + + public static void validate(Map properties) { + if (!properties.containsKey(JDBC_URL)) { + throw new IllegalArgumentException("JDBC URL is required"); + } + if (!properties.containsKey(JDBC_USERNAME)) { + throw new IllegalArgumentException("JDBC username is required"); + } + if (!properties.containsKey(JDBC_PASSWORD)) { + throw new IllegalArgumentException("JDBC password is required"); + } + if (!properties.containsKey(JDBC_DRIVER)) { + throw new IllegalArgumentException("JDBC driver is required"); + } + } +} diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationSQL.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationSQL.java index d9783d35c4b..f7171ff354a 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationSQL.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationSQL.java @@ -31,33 +31,33 @@ interface JdbcAuthorizationSQL { * Get SQL statements for creating a user. * * @param username the username to create - * @return a SQL statement to create a user + * @return the SQL statement list to create a user */ - String getCreateUserSQL(String username); + List getCreateUserSQL(String username); /** * Get SQL statements for creating a group. * * @param username the username to drop - * @return a SQL statement to drop a user + * @return the SQL statement list to drop a user */ - String getDropUserSQL(String username); + List getDropUserSQL(String username); /** * Get SQL statements for creating a role. * * @param roleName the role name to create - * @return a SQL statement to create a role + * @return the SQL statement list to create a role */ - String getCreateRoleSQL(String roleName); + List getCreateRoleSQL(String roleName); /** * Get SQL statements for dropping a role. * * @param roleName the role name to drop - * @return a SQL statement to drop a role + * @return the SQL statement list to drop a role */ - String getDropRoleSQL(String roleName); + List getDropRoleSQL(String roleName); /** * Get SQL statements for granting privileges. @@ -66,9 +66,9 @@ interface JdbcAuthorizationSQL { * @param objectType the object type in the database system * @param objectName the object name in the database system * @param roleName the role name to grant - * @return a sql statement to grant privilege + * @return the sql statement list to grant privilege */ - String getGrantPrivilegeSQL( + List getGrantPrivilegeSQL( String privilege, String objectType, String objectName, String roleName); /** @@ -78,9 +78,9 @@ String getGrantPrivilegeSQL( * @param objectType the object type in the database system * @param objectName the object name in the database system * @param roleName the role name to revoke - * @return a sql statement to revoke privilege + * @return the sql statement list to revoke privilege */ - String getRevokePrivilegeSQL( + List getRevokePrivilegeSQL( String privilege, String objectType, String objectName, String roleName); /** @@ -89,9 +89,9 @@ String getRevokePrivilegeSQL( * @param roleName the role name to grant * @param grantorType the grantor type, usually USER or ROLE * @param grantorName the grantor name - * @return a sql statement to grant role + * @return the sql statement list to grant role */ - String getGrantRoleSQL(String roleName, String grantorType, String grantorName); + List getGrantRoleSQL(String roleName, String grantorType, String grantorName); /** * Get SQL statements for revoking roles. @@ -99,9 +99,9 @@ String getRevokePrivilegeSQL( * @param roleName the role name to revoke * @param revokerType the revoker type, usually USER or ROLE * @param revokerName the revoker name - * @return a sql statement to revoke role + * @return the sql statement list to revoke role */ - String getRevokeRoleSQL(String roleName, String revokerType, String revokerName); + List getRevokeRoleSQL(String roleName, String revokerType, String revokerName); /** * Get SQL statements for setting owner. diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java index 9610c902d7f..ed1f9a9bd15 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java @@ -160,32 +160,35 @@ public List translatePrivilege(SecurableObject sec @Override public List translateOwner(MetadataObject metadataObject) { List objects = Lists.newArrayList(); - JdbcPrivilege allPri = JdbcPrivilege.valueOf(JdbcPrivilege.Type.ALL); + JdbcPrivilege allPrivilege = JdbcPrivilege.valueOf(JdbcPrivilege.Type.ALL); switch (metadataObject.type()) { case METALAKE: case CATALOG: objects.add( new JdbcAuthorizationObject( - JdbcAuthorizationObject.ALL, null, Lists.newArrayList(allPri))); + JdbcAuthorizationObject.ALL, null, Lists.newArrayList(allPrivilege))); objects.add( new JdbcAuthorizationObject( JdbcAuthorizationObject.ALL, JdbcAuthorizationObject.ALL, - Lists.newArrayList(allPri))); + Lists.newArrayList(allPrivilege))); break; case SCHEMA: objects.add( - new JdbcAuthorizationObject(metadataObject.name(), null, Lists.newArrayList(allPri))); + new JdbcAuthorizationObject( + metadataObject.name(), null, Lists.newArrayList(allPrivilege))); objects.add( new JdbcAuthorizationObject( - metadataObject.name(), JdbcAuthorizationObject.ALL, Lists.newArrayList(allPri))); + metadataObject.name(), + JdbcAuthorizationObject.ALL, + Lists.newArrayList(allPrivilege))); break; case TABLE: MetadataObject schema = MetadataObjects.parse(metadataObject.parent(), MetadataObject.Type.SCHEMA); objects.add( new JdbcAuthorizationObject( - schema.name(), metadataObject.name(), Lists.newArrayList(allPri))); + schema.name(), metadataObject.name(), Lists.newArrayList(allPrivilege))); break; default: throw new IllegalArgumentException(""); diff --git a/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java b/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java index f4ecb1c0dd4..69f13dcf8e4 100644 --- a/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java +++ b/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java @@ -18,12 +18,12 @@ */ package org.apache.gravitino.authorization.jdbc; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; -import org.apache.gravitino.Audit; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.MetadataObjects; import org.apache.gravitino.authorization.Group; @@ -36,6 +36,7 @@ import org.apache.gravitino.authorization.User; 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.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -48,9 +49,19 @@ public class JdbcAuthorizationPluginTest { private static List expectNewOwners = Lists.newArrayList(); private static int currentSQLIndex = 0; private static int currentIndex = 0; + private static final Map properties = + ImmutableMap.of( + JdbcAuthorizationProperties.JDBC_URL, + "xx", + JdbcAuthorizationProperties.JDBC_USERNAME, + "xx", + JdbcAuthorizationProperties.JDBC_PASSWORD, + "xx", + JdbcAuthorizationProperties.JDBC_DRIVER, + "xx"); private static final JdbcAuthorizationPlugin plugin = - new JdbcAuthorizationPlugin(Collections.emptyMap()) { + new JdbcAuthorizationPlugin(properties) { @Override public List getSetOwnerSQL( @@ -86,38 +97,38 @@ public void testUserManagement() { @Test public void testGroupManagement() { expectSQLs = Lists.newArrayList("CREATE USER GRAVITINO_GROUP_tmp"); - currentSQLIndex = 0; + resetSQLIndex(); plugin.onGroupAdded(createGroup("tmp")); Assertions.assertThrows( UnsupportedOperationException.class, () -> plugin.onGroupAcquired(createGroup("tmp"))); expectSQLs = Lists.newArrayList("DROP USER GRAVITINO_GROUP_tmp"); - currentSQLIndex = 0; + resetSQLIndex(); plugin.onGroupRemoved(createGroup("tmp")); } @Test public void testRoleManagement() { expectSQLs = Lists.newArrayList("CREATE ROLE tmp"); - currentSQLIndex = 0; - Role role = new TemporaryRole("tmp"); + resetSQLIndex(); + Role role = createRole("tmp"); plugin.onRoleCreated(role); Assertions.assertThrows(UnsupportedOperationException.class, () -> plugin.onRoleAcquired(role)); - currentSQLIndex = 0; + resetSQLIndex(); expectSQLs = Lists.newArrayList("DROP ROLE tmp"); plugin.onRoleDeleted(role); } @Test public void testPermissionManagement() { - Role role = new TemporaryRole("tmp"); + Role role = createRole("tmp"); Group group = createGroup("tmp"); User user = createUser("tmp"); - currentSQLIndex = 0; + resetSQLIndex(); expectSQLs = Lists.newArrayList( "CREATE USER GRAVITINO_GROUP_tmp", @@ -125,12 +136,12 @@ public void testPermissionManagement() { "GRANT ROLE tmp TO USER GRAVITINO_GROUP_tmp"); plugin.onGrantedRolesToGroup(Lists.newArrayList(role), group); - currentSQLIndex = 0; + resetSQLIndex(); expectSQLs = Lists.newArrayList("CREATE USER tmp", "CREATE ROLE tmp", "GRANT ROLE tmp TO USER tmp"); plugin.onGrantedRolesToUser(Lists.newArrayList(role), user); - currentSQLIndex = 0; + resetSQLIndex(); expectSQLs = Lists.newArrayList( "CREATE USER GRAVITINO_GROUP_tmp", @@ -138,25 +149,25 @@ public void testPermissionManagement() { "REVOKE ROLE tmp FROM USER GRAVITINO_GROUP_tmp"); plugin.onRevokedRolesFromGroup(Lists.newArrayList(role), group); - currentSQLIndex = 0; + resetSQLIndex(); expectSQLs = Lists.newArrayList("CREATE USER tmp", "CREATE ROLE tmp", "REVOKE ROLE tmp FROM USER tmp"); plugin.onRevokedRolesFromUser(Lists.newArrayList(role), user); // Test metalake object and different role change - currentSQLIndex = 0; + resetSQLIndex(); expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE *.* TO ROLE tmp"); SecurableObject metalakeObject = SecurableObjects.ofMetalake("metalake", Lists.newArrayList(Privileges.SelectTable.allow())); RoleChange roleChange = RoleChange.addSecurableObject("tmp", metalakeObject); plugin.onRoleUpdated(role, roleChange); - currentSQLIndex = 0; + resetSQLIndex(); expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "REVOKE SELECT ON TABLE *.* FROM ROLE tmp"); roleChange = RoleChange.removeSecurableObject("tmp", metalakeObject); plugin.onRoleUpdated(role, roleChange); - currentSQLIndex = 0; + resetSQLIndex(); expectSQLs = Lists.newArrayList( "CREATE ROLE tmp", @@ -168,7 +179,7 @@ public void testPermissionManagement() { plugin.onRoleUpdated(role, roleChange); // Test catalog object - currentSQLIndex = 0; + resetSQLIndex(); SecurableObject catalogObject = SecurableObjects.ofCatalog("catalog", Lists.newArrayList(Privileges.SelectTable.allow())); roleChange = RoleChange.addSecurableObject("tmp", catalogObject); @@ -176,7 +187,7 @@ public void testPermissionManagement() { plugin.onRoleUpdated(role, roleChange); // Test schema object - currentSQLIndex = 0; + resetSQLIndex(); SecurableObject schemaObject = SecurableObjects.ofSchema( catalogObject, "schema", Lists.newArrayList(Privileges.SelectTable.allow())); @@ -186,7 +197,7 @@ public void testPermissionManagement() { plugin.onRoleUpdated(role, roleChange); // Test table object - currentSQLIndex = 0; + resetSQLIndex(); SecurableObject tableObject = SecurableObjects.ofTable( schemaObject, "table", Lists.newArrayList(Privileges.SelectTable.allow())); @@ -217,13 +228,8 @@ public void testOwnerManagement() { plugin.onOwnerSet(metalakeObject, null, owner); // clean up - expectTypes.clear(); - expectObjectNames.clear(); - expectPreOwners.clear(); - expectNewOwners.clear(); - currentIndex = 0; + cleanup(); expectSQLs = Lists.newArrayList("CREATE USER tmp"); - currentSQLIndex = 0; // Test catalog object MetadataObject catalogObject = MetadataObjects.of(null, "catalog", MetadataObject.Type.CATALOG); @@ -239,13 +245,8 @@ public void testOwnerManagement() { plugin.onOwnerSet(catalogObject, null, owner); // clean up - expectTypes.clear(); - expectObjectNames.clear(); - expectPreOwners.clear(); - expectNewOwners.clear(); - currentIndex = 0; + cleanup(); expectSQLs = Lists.newArrayList("CREATE USER tmp"); - currentSQLIndex = 0; // Test schema object MetadataObject schemaObject = @@ -262,13 +263,8 @@ public void testOwnerManagement() { plugin.onOwnerSet(schemaObject, null, owner); // clean up - expectTypes.clear(); - expectObjectNames.clear(); - expectPreOwners.clear(); - expectNewOwners.clear(); - currentIndex = 0; + cleanup(); expectSQLs = Lists.newArrayList("CREATE USER tmp"); - currentSQLIndex = 0; // Test table object MetadataObject tableObject = @@ -282,32 +278,17 @@ public void testOwnerManagement() { plugin.onOwnerSet(tableObject, null, owner); } - private static class TemporaryRole implements Role { - private final String name; - - public TemporaryRole(String name) { - this.name = name; - } - - @Override - public Audit auditInfo() { - return AuditInfo.EMPTY; - } - - @Override - public String name() { - return name; - } - - @Override - public Map properties() { - return Collections.emptyMap(); - } + private static void resetSQLIndex() { + currentSQLIndex = 0; + } - @Override - public List securableObjects() { - return Collections.emptyList(); - } + private static void cleanup() { + expectTypes.clear(); + expectObjectNames.clear(); + expectPreOwners.clear(); + expectNewOwners.clear(); + currentIndex = 0; + currentSQLIndex = 0; } private static class TemporaryOwner implements Owner { @@ -330,6 +311,10 @@ public Type type() { } } + private static Role createRole(String name) { + return RoleEntity.builder().withId(0L).withName(name).withAuditInfo(AuditInfo.EMPTY).build(); + } + private static Group createGroup(String name) { return GroupEntity.builder().withId(0L).withName(name).withAuditInfo(AuditInfo.EMPTY).build(); } From 3b5c9499200633c25a1decf2a7a61e1e31ba6b8b Mon Sep 17 00:00:00 2001 From: Rory Date: Mon, 23 Dec 2024 20:44:26 +0800 Subject: [PATCH 05/10] address comments --- .../jdbc/JdbcAuthorizationPlugin.java | 4 +- .../jdbc/JdbcMetadataObject.java | 78 ++++++++++++++++++ .../authorization/jdbc/JdbcPrivilege.java | 82 ++++--------------- ...onObject.java => JdbcSecurableObject.java} | 54 +----------- .../JdbcSecurableObjectMappingProvider.java | 67 +++++++-------- 5 files changed, 132 insertions(+), 153 deletions(-) create mode 100644 authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java rename authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/{JdbcAuthorizationObject.java => JdbcSecurableObject.java} (56%) diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java index d209a6eb8fc..66916e71088 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java @@ -402,7 +402,7 @@ private void grantObjectPrivileges(Role role, SecurableObject object) { List authObjects = mappingProvider.translatePrivilege(object); for (AuthorizationSecurableObject authObject : authObjects) { List convertedObjects = Lists.newArrayList(); - if (authObject.name().equals(JdbcAuthorizationObject.ALL)) { + if (authObject.name().equals(JdbcSecurableObject.ALL)) { convertedObjects.addAll(convertResourceAll(authObject)); } else { convertedObjects.add(authObject); @@ -435,7 +435,7 @@ private void revokeObjectPrivileges(Role role, SecurableObject removeObject) { mappingProvider.translatePrivilege(removeObject); for (AuthorizationSecurableObject authObject : authObjects) { List convertedObjects = Lists.newArrayList(); - if (authObject.name().equals(JdbcAuthorizationObject.ALL)) { + if (authObject.name().equals(JdbcSecurableObject.ALL)) { convertedObjects.addAll(convertResourceAll(authObject)); } else { convertedObjects.add(authObject); diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java new file mode 100644 index 00000000000..b813ba53772 --- /dev/null +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java @@ -0,0 +1,78 @@ +/* + * 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.authorization.jdbc; + +import com.google.common.base.Preconditions; +import java.util.List; +import javax.annotation.Nullable; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.authorization.AuthorizationMetadataObject; + +public class JdbcMetadataObject implements AuthorizationMetadataObject { + + private final String parent; + private final String name; + private final MetadataObject.Type type; + + public JdbcMetadataObject(String parent, String name, MetadataObject.Type type) { + this.parent = parent; + this.name = name; + this.type = type; + } + + @Nullable + @Override + public String parent() { + return parent; + } + + @Override + public String name() { + return name; + } + + @Override + public List names() { + return DOT_SPLITTER.splitToList(fullName()); + } + + @Override + public Type type() { + return () -> type; + } + + @Override + public void validateAuthorizationMetadataObject() throws IllegalArgumentException { + List names = names(); + Preconditions.checkArgument( + names != null && !names.isEmpty(), "The name of the object is empty."); + Preconditions.checkArgument( + names.size() <= 2, "The name of the object is not in the format of 'database.table'."); + Preconditions.checkArgument(type != null, "The type of the object is null."); + Preconditions.checkArgument( + names.size() == 1 || type == MetadataObject.Type.SCHEMA, + "The type of the object is not SCHEMA."); + Preconditions.checkArgument( + names.size() == 2 || type == MetadataObject.Type.TABLE, + "The type of the object is not TABLE."); + for (String name : names) { + Preconditions.checkArgument(name != null, "Cannot create a metadata object with null name"); + } + } +} diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java index dce14161dbd..845b31a5b59 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcPrivilege.java @@ -21,27 +21,26 @@ import org.apache.gravitino.authorization.AuthorizationPrivilege; import org.apache.gravitino.authorization.Privilege; -public class JdbcPrivilege implements AuthorizationPrivilege { - - private static final JdbcPrivilege SELECT_PRIVILEGE = new JdbcPrivilege(Type.SELECT); - private static final JdbcPrivilege INSERT_PRIVILEGE = new JdbcPrivilege(Type.INSERT); - private static final JdbcPrivilege UPDATE_PRIVILEGE = new JdbcPrivilege(Type.UPDATE); - private static final JdbcPrivilege ALTER_PRIVILEGE = new JdbcPrivilege(Type.ALTER); - private static final JdbcPrivilege DELETE_PRIVILEGE = new JdbcPrivilege(Type.DELETE); - private static final JdbcPrivilege ALL_PRIVILEGE = new JdbcPrivilege(Type.ALL); - private static final JdbcPrivilege CREATE_PRIVILEGE = new JdbcPrivilege(Type.CREATE); - private static final JdbcPrivilege DROP_PRIVILEGE = new JdbcPrivilege(Type.DROP); - private static final JdbcPrivilege USAGE_PRIVILEGE = new JdbcPrivilege(Type.USAGE); - - private final Type type; - - private JdbcPrivilege(Type type) { - this.type = type; +public enum JdbcPrivilege implements AuthorizationPrivilege { + SELECT("SELECT"), + INSERT("INSERT"), + UPDATE("UPDATE"), + ALTER("ALTER"), + DELETE("DELETE"), + ALL("ALL PRIVILEGES"), + CREATE("CREATE"), + DROP("DROP"), + USAGE("USAGE"); + + private final String name; + + JdbcPrivilege(String name) { + this.name = name; } @Override public String getName() { - return type.getName(); + return name; } @Override @@ -51,53 +50,6 @@ public Privilege.Condition condition() { @Override public boolean equalsTo(String value) { - return false; - } - - static JdbcPrivilege valueOf(Type type) { - switch (type) { - case CREATE: - return CREATE_PRIVILEGE; - case DELETE: - return DELETE_PRIVILEGE; - case ALL: - return ALL_PRIVILEGE; - case DROP: - return DROP_PRIVILEGE; - case ALTER: - return ALTER_PRIVILEGE; - case INSERT: - return INSERT_PRIVILEGE; - case UPDATE: - return UPDATE_PRIVILEGE; - case SELECT: - return SELECT_PRIVILEGE; - case USAGE: - return USAGE_PRIVILEGE; - default: - throw new IllegalArgumentException(String.format("Unsupported parameter type %s", type)); - } - } - - public enum Type { - SELECT("SELECT"), - INSERT("INSERT"), - UPDATE("UPDATE"), - ALTER("ALTER"), - DELETE("DELETE"), - ALL("ALL PRIVILEGES"), - CREATE("CREATE"), - DROP("DROP"), - USAGE("USAGE"); - - private final String name; - - Type(String name) { - this.name = name; - } - - public String getName() { - return name; - } + return name.equals(value); } } diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java similarity index 56% rename from authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java rename to authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java index c5bbdd0d622..13d145a6e5c 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationObject.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java @@ -18,10 +18,7 @@ */ package org.apache.gravitino.authorization.jdbc; -import com.google.common.base.Preconditions; -import com.google.common.collect.Lists; import java.util.List; -import javax.annotation.Nullable; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.authorization.AuthorizationPrivilege; import org.apache.gravitino.authorization.AuthorizationSecurableObject; @@ -31,61 +28,18 @@ * object. JdbcAuthorizationObject has the database and table name. When table name is null, the * object represents a database. The database can't be null. */ -public class JdbcAuthorizationObject implements AuthorizationSecurableObject { +public class JdbcSecurableObject extends JdbcMetadataObject + implements AuthorizationSecurableObject { public static final String ALL = "*"; - private String database; - private String table; List privileges; - JdbcAuthorizationObject(String database, String table, List privileges) { - Preconditions.checkNotNull(database, "JDBC authorization object database can't null"); - this.database = database; - this.table = table; + JdbcSecurableObject(String database, String table, List privileges) { + super(database, table, table == null ? MetadataObject.Type.SCHEMA : MetadataObject.Type.TABLE); this.privileges = privileges; } - @Nullable - @Override - public String parent() { - if (table != null) { - return database; - } - - return null; - } - - @Override - public String name() { - if (table != null) { - return table; - } - - return database; - } - - @Override - public List names() { - List names = Lists.newArrayList(); - names.add(database); - if (table != null) { - names.add(table); - } - return names; - } - - @Override - public Type type() { - if (table != null) { - return () -> MetadataObject.Type.TABLE; - } - return () -> MetadataObject.Type.SCHEMA; - } - - @Override - public void validateAuthorizationMetadataObject() throws IllegalArgumentException {} - @Override public List privileges() { return privileges; diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java index ed1f9a9bd15..49d86fe3436 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java @@ -42,21 +42,17 @@ public class JdbcSecurableObjectMappingProvider implements AuthorizationPrivileg private final Map> privilegeMapping = ImmutableMap.of( - Privilege.Name.CREATE_TABLE, - Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.CREATE)), - Privilege.Name.CREATE_SCHEMA, - Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.CREATE)), - Privilege.Name.SELECT_TABLE, - Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.SELECT)), + Privilege.Name.CREATE_TABLE, Sets.newHashSet(JdbcPrivilege.CREATE), + Privilege.Name.CREATE_SCHEMA, Sets.newHashSet(JdbcPrivilege.CREATE), + Privilege.Name.SELECT_TABLE, Sets.newHashSet(JdbcPrivilege.SELECT), Privilege.Name.MODIFY_TABLE, Sets.newHashSet( - JdbcPrivilege.valueOf(JdbcPrivilege.Type.SELECT), - JdbcPrivilege.valueOf(JdbcPrivilege.Type.UPDATE), - JdbcPrivilege.valueOf(JdbcPrivilege.Type.DELETE), - JdbcPrivilege.valueOf(JdbcPrivilege.Type.INSERT), - JdbcPrivilege.valueOf(JdbcPrivilege.Type.ALTER)), - Privilege.Name.USE_SCHEMA, - Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.USAGE))); + JdbcPrivilege.SELECT, + JdbcPrivilege.UPDATE, + JdbcPrivilege.DELETE, + JdbcPrivilege.INSERT, + JdbcPrivilege.ALTER), + Privilege.Name.USE_SCHEMA, Sets.newHashSet(JdbcPrivilege.USAGE)); private final Map privilegeScopeMapping = ImmutableMap.of( @@ -100,8 +96,8 @@ public List translatePrivilege(SecurableObject sec List authObjects = Lists.newArrayList(); List databasePrivileges = Lists.newArrayList(); List tablePrivileges = Lists.newArrayList(); - JdbcAuthorizationObject databaseObject; - JdbcAuthorizationObject tableObject; + JdbcSecurableObject databaseObject; + JdbcSecurableObject tableObject; switch (securableObject.type()) { case METALAKE: case CATALOG: @@ -109,14 +105,14 @@ public List translatePrivilege(SecurableObject sec if (!databasePrivileges.isEmpty()) { databaseObject = - new JdbcAuthorizationObject(JdbcAuthorizationObject.ALL, null, databasePrivileges); + new JdbcSecurableObject(JdbcSecurableObject.ALL, null, databasePrivileges); authObjects.add(databaseObject); } if (!tablePrivileges.isEmpty()) { tableObject = - new JdbcAuthorizationObject( - JdbcAuthorizationObject.ALL, JdbcAuthorizationObject.ALL, tablePrivileges); + new JdbcSecurableObject( + JdbcSecurableObject.ALL, JdbcSecurableObject.ALL, tablePrivileges); authObjects.add(tableObject); } break; @@ -125,14 +121,14 @@ public List translatePrivilege(SecurableObject sec convertJdbcPrivileges(securableObject, databasePrivileges, tablePrivileges); if (!databasePrivileges.isEmpty()) { databaseObject = - new JdbcAuthorizationObject(securableObject.name(), null, databasePrivileges); + new JdbcSecurableObject(securableObject.name(), null, databasePrivileges); authObjects.add(databaseObject); } if (!tablePrivileges.isEmpty()) { tableObject = - new JdbcAuthorizationObject( - securableObject.name(), JdbcAuthorizationObject.ALL, tablePrivileges); + new JdbcSecurableObject( + securableObject.name(), JdbcSecurableObject.ALL, tablePrivileges); authObjects.add(tableObject); } break; @@ -143,7 +139,7 @@ public List translatePrivilege(SecurableObject sec MetadataObject metadataObject = MetadataObjects.parse(securableObject.parent(), MetadataObject.Type.SCHEMA); tableObject = - new JdbcAuthorizationObject( + new JdbcSecurableObject( metadataObject.name(), securableObject.name(), tablePrivileges); authObjects.add(tableObject); } @@ -160,35 +156,34 @@ public List translatePrivilege(SecurableObject sec @Override public List translateOwner(MetadataObject metadataObject) { List objects = Lists.newArrayList(); - JdbcPrivilege allPrivilege = JdbcPrivilege.valueOf(JdbcPrivilege.Type.ALL); switch (metadataObject.type()) { case METALAKE: case CATALOG: objects.add( - new JdbcAuthorizationObject( - JdbcAuthorizationObject.ALL, null, Lists.newArrayList(allPrivilege))); + new JdbcSecurableObject( + JdbcSecurableObject.ALL, null, Lists.newArrayList(JdbcPrivilege.ALL))); objects.add( - new JdbcAuthorizationObject( - JdbcAuthorizationObject.ALL, - JdbcAuthorizationObject.ALL, - Lists.newArrayList(allPrivilege))); + new JdbcSecurableObject( + JdbcSecurableObject.ALL, + JdbcSecurableObject.ALL, + Lists.newArrayList(JdbcPrivilege.ALL))); break; case SCHEMA: objects.add( - new JdbcAuthorizationObject( - metadataObject.name(), null, Lists.newArrayList(allPrivilege))); + new JdbcSecurableObject( + metadataObject.name(), null, Lists.newArrayList(JdbcPrivilege.ALL))); objects.add( - new JdbcAuthorizationObject( + new JdbcSecurableObject( metadataObject.name(), - JdbcAuthorizationObject.ALL, - Lists.newArrayList(allPrivilege))); + JdbcSecurableObject.ALL, + Lists.newArrayList(JdbcPrivilege.ALL))); break; case TABLE: MetadataObject schema = MetadataObjects.parse(metadataObject.parent(), MetadataObject.Type.SCHEMA); objects.add( - new JdbcAuthorizationObject( - schema.name(), metadataObject.name(), Lists.newArrayList(allPrivilege))); + new JdbcSecurableObject( + schema.name(), metadataObject.name(), Lists.newArrayList(JdbcPrivilege.ALL))); break; default: throw new IllegalArgumentException(""); From 9f410f8ab2f519788114cc7a3b808a2aaeb2a2ea Mon Sep 17 00:00:00 2001 From: Rory Date: Mon, 23 Dec 2024 21:05:16 +0800 Subject: [PATCH 06/10] Address comments --- .../jdbc/JdbcAuthorizationPlugin.java | 4 ---- .../jdbc/JdbcSecurableObject.java | 18 +++++++++++++-- .../JdbcSecurableObjectMappingProvider.java | 23 ++++++++++--------- .../jdbc/JdbcAuthorizationPluginTest.java | 16 ++++--------- 4 files changed, 32 insertions(+), 29 deletions(-) diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java index 66916e71088..f889cee2240 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java @@ -164,7 +164,6 @@ public Boolean onRoleUpdated(Role role, RoleChange... changes) @Override public Boolean onGrantedRolesToUser(List roles, User user) throws AuthorizationPluginException { - onUserAdded(user); for (Role role : roles) { onRoleCreated(role); @@ -179,7 +178,6 @@ public Boolean onGrantedRolesToUser(List roles, User user) @Override public Boolean onRevokedRolesFromUser(List roles, User user) throws AuthorizationPluginException { - onUserAdded(user); for (Role role : roles) { onRoleCreated(role); @@ -194,7 +192,6 @@ public Boolean onRevokedRolesFromUser(List roles, User user) @Override public Boolean onGrantedRolesToGroup(List roles, Group group) throws AuthorizationPluginException { - onGroupAdded(group); for (Role role : roles) { onRoleCreated(role); @@ -210,7 +207,6 @@ public Boolean onGrantedRolesToGroup(List roles, Group group) @Override public Boolean onRevokedRolesFromGroup(List roles, Group group) throws AuthorizationPluginException { - onGroupAdded(group); for (Role role : roles) { onRoleCreated(role); diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java index 13d145a6e5c..70ce0e246ee 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java @@ -35,11 +35,25 @@ public class JdbcSecurableObject extends JdbcMetadataObject List privileges; - JdbcSecurableObject(String database, String table, List privileges) { - super(database, table, table == null ? MetadataObject.Type.SCHEMA : MetadataObject.Type.TABLE); + private JdbcSecurableObject( + String parent, + String name, + MetadataObject.Type type, + List privileges) { + super(parent, name, type); this.privileges = privileges; } + static JdbcSecurableObject create( + String schema, String table, List privileges) { + String parent = table == null ? null : schema; + String name = table == null ? schema : table; + MetadataObject.Type type = + table == null ? MetadataObject.Type.SCHEMA : MetadataObject.Type.TABLE; + + return new JdbcSecurableObject(parent, name, type, privileges); + } + @Override public List privileges() { return privileges; diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java index 49d86fe3436..70b2d10e39c 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java @@ -105,13 +105,13 @@ public List translatePrivilege(SecurableObject sec if (!databasePrivileges.isEmpty()) { databaseObject = - new JdbcSecurableObject(JdbcSecurableObject.ALL, null, databasePrivileges); + JdbcSecurableObject.create(JdbcSecurableObject.ALL, null, databasePrivileges); authObjects.add(databaseObject); } if (!tablePrivileges.isEmpty()) { tableObject = - new JdbcSecurableObject( + JdbcSecurableObject.create( JdbcSecurableObject.ALL, JdbcSecurableObject.ALL, tablePrivileges); authObjects.add(tableObject); } @@ -121,13 +121,13 @@ public List translatePrivilege(SecurableObject sec convertJdbcPrivileges(securableObject, databasePrivileges, tablePrivileges); if (!databasePrivileges.isEmpty()) { databaseObject = - new JdbcSecurableObject(securableObject.name(), null, databasePrivileges); + JdbcSecurableObject.create(securableObject.name(), null, databasePrivileges); authObjects.add(databaseObject); } if (!tablePrivileges.isEmpty()) { tableObject = - new JdbcSecurableObject( + JdbcSecurableObject.create( securableObject.name(), JdbcSecurableObject.ALL, tablePrivileges); authObjects.add(tableObject); } @@ -139,7 +139,7 @@ public List translatePrivilege(SecurableObject sec MetadataObject metadataObject = MetadataObjects.parse(securableObject.parent(), MetadataObject.Type.SCHEMA); tableObject = - new JdbcSecurableObject( + JdbcSecurableObject.create( metadataObject.name(), securableObject.name(), tablePrivileges); authObjects.add(tableObject); } @@ -160,20 +160,20 @@ public List translateOwner(MetadataObject metadata case METALAKE: case CATALOG: objects.add( - new JdbcSecurableObject( + JdbcSecurableObject.create( JdbcSecurableObject.ALL, null, Lists.newArrayList(JdbcPrivilege.ALL))); objects.add( - new JdbcSecurableObject( + JdbcSecurableObject.create( JdbcSecurableObject.ALL, JdbcSecurableObject.ALL, Lists.newArrayList(JdbcPrivilege.ALL))); break; case SCHEMA: objects.add( - new JdbcSecurableObject( + JdbcSecurableObject.create( metadataObject.name(), null, Lists.newArrayList(JdbcPrivilege.ALL))); objects.add( - new JdbcSecurableObject( + JdbcSecurableObject.create( metadataObject.name(), JdbcSecurableObject.ALL, Lists.newArrayList(JdbcPrivilege.ALL))); @@ -182,11 +182,12 @@ public List translateOwner(MetadataObject metadata MetadataObject schema = MetadataObjects.parse(metadataObject.parent(), MetadataObject.Type.SCHEMA); objects.add( - new JdbcSecurableObject( + JdbcSecurableObject.create( schema.name(), metadataObject.name(), Lists.newArrayList(JdbcPrivilege.ALL))); break; default: - throw new IllegalArgumentException(""); + throw new IllegalArgumentException( + "Don't support metadata object type " + metadataObject.type()); } return objects; } diff --git a/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java b/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java index 69f13dcf8e4..b72392a6cd8 100644 --- a/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java +++ b/authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java @@ -130,28 +130,20 @@ public void testPermissionManagement() { resetSQLIndex(); expectSQLs = - Lists.newArrayList( - "CREATE USER GRAVITINO_GROUP_tmp", - "CREATE ROLE tmp", - "GRANT ROLE tmp TO USER GRAVITINO_GROUP_tmp"); + Lists.newArrayList("CREATE ROLE tmp", "GRANT ROLE tmp TO USER GRAVITINO_GROUP_tmp"); plugin.onGrantedRolesToGroup(Lists.newArrayList(role), group); resetSQLIndex(); - expectSQLs = - Lists.newArrayList("CREATE USER tmp", "CREATE ROLE tmp", "GRANT ROLE tmp TO USER tmp"); + expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "GRANT ROLE tmp TO USER tmp"); plugin.onGrantedRolesToUser(Lists.newArrayList(role), user); resetSQLIndex(); expectSQLs = - Lists.newArrayList( - "CREATE USER GRAVITINO_GROUP_tmp", - "CREATE ROLE tmp", - "REVOKE ROLE tmp FROM USER GRAVITINO_GROUP_tmp"); + Lists.newArrayList("CREATE ROLE tmp", "REVOKE ROLE tmp FROM USER GRAVITINO_GROUP_tmp"); plugin.onRevokedRolesFromGroup(Lists.newArrayList(role), group); resetSQLIndex(); - expectSQLs = - Lists.newArrayList("CREATE USER tmp", "CREATE ROLE tmp", "REVOKE ROLE tmp FROM USER tmp"); + expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "REVOKE ROLE tmp FROM USER tmp"); plugin.onRevokedRolesFromUser(Lists.newArrayList(role), user); // Test metalake object and different role change From 36e9a91042914d92b018043230bba41a152a6796 Mon Sep 17 00:00:00 2001 From: Rory Date: Tue, 24 Dec 2024 10:57:05 +0800 Subject: [PATCH 07/10] address comment --- .../jdbc/JdbcAuthorizationProperties.java | 21 ++++++----- .../jdbc/JdbcMetadataObject.java | 35 ++++++++++++++++--- .../jdbc/JdbcSecurableObject.java | 8 +++-- 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationProperties.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationProperties.java index 4d325c6812e..b13504fd2fd 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationProperties.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationProperties.java @@ -29,17 +29,16 @@ public class JdbcAuthorizationProperties { public static final String JDBC_DRIVER = CONFIG_PREFIX + "driver"; public static void validate(Map properties) { - if (!properties.containsKey(JDBC_URL)) { - throw new IllegalArgumentException("JDBC URL is required"); - } - if (!properties.containsKey(JDBC_USERNAME)) { - throw new IllegalArgumentException("JDBC username is required"); - } - if (!properties.containsKey(JDBC_PASSWORD)) { - throw new IllegalArgumentException("JDBC password is required"); - } - if (!properties.containsKey(JDBC_DRIVER)) { - throw new IllegalArgumentException("JDBC driver is required"); + String errorMsg = "%s is required"; + check(properties, JDBC_URL, errorMsg); + check(properties, JDBC_USERNAME, errorMsg); + check(properties, JDBC_PASSWORD, errorMsg); + check(properties, JDBC_DRIVER, errorMsg); + } + + private static void check(Map properties, String key, String errorMsg) { + if (!properties.containsKey(key) && properties.get(key) != null) { + throw new IllegalArgumentException(String.format(errorMsg, key)); } } } diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java index b813ba53772..f7a577aeaee 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java @@ -28,9 +28,9 @@ public class JdbcMetadataObject implements AuthorizationMetadataObject { private final String parent; private final String name; - private final MetadataObject.Type type; + private final Type type; - public JdbcMetadataObject(String parent, String name, MetadataObject.Type type) { + public JdbcMetadataObject(String parent, String name, Type type) { this.parent = parent; this.name = name; this.type = type; @@ -54,7 +54,7 @@ public List names() { @Override public Type type() { - return () -> type; + return type; } @Override @@ -66,13 +66,38 @@ public void validateAuthorizationMetadataObject() throws IllegalArgumentExceptio names.size() <= 2, "The name of the object is not in the format of 'database.table'."); Preconditions.checkArgument(type != null, "The type of the object is null."); Preconditions.checkArgument( - names.size() == 1 || type == MetadataObject.Type.SCHEMA, + names.size() == 1 || type.metadataObjectType() == MetadataObject.Type.SCHEMA, "The type of the object is not SCHEMA."); Preconditions.checkArgument( - names.size() == 2 || type == MetadataObject.Type.TABLE, + names.size() == 2 || type.metadataObjectType() == MetadataObject.Type.TABLE, "The type of the object is not TABLE."); for (String name : names) { Preconditions.checkArgument(name != null, "Cannot create a metadata object with null name"); } } + + public enum Type implements AuthorizationMetadataObject.Type { + SCHEMA(MetadataObject.Type.SCHEMA), + TABLE(MetadataObject.Type.TABLE); + + private final MetadataObject.Type metadataType; + + Type(MetadataObject.Type type) { + this.metadataType = type; + } + + public MetadataObject.Type metadataObjectType() { + return metadataType; + } + + public static Type fromMetadataType(MetadataObject.Type metadataType) { + for (Type type : Type.values()) { + if (type.metadataObjectType() == metadataType) { + return type; + } + } + throw new IllegalArgumentException( + "No matching RangerMetadataObject.Type for " + metadataType); + } + } } diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java index 70ce0e246ee..ab07fd2c2e1 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java @@ -38,7 +38,7 @@ public class JdbcSecurableObject extends JdbcMetadataObject private JdbcSecurableObject( String parent, String name, - MetadataObject.Type type, + JdbcMetadataObject.Type type, List privileges) { super(parent, name, type); this.privileges = privileges; @@ -48,8 +48,10 @@ static JdbcSecurableObject create( String schema, String table, List privileges) { String parent = table == null ? null : schema; String name = table == null ? schema : table; - MetadataObject.Type type = - table == null ? MetadataObject.Type.SCHEMA : MetadataObject.Type.TABLE; + JdbcMetadataObject.Type type = + table == null + ? JdbcMetadataObject.Type.fromMetadataType(MetadataObject.Type.SCHEMA) + : JdbcMetadataObject.Type.fromMetadataType(MetadataObject.Type.TABLE); return new JdbcSecurableObject(parent, name, type, privileges); } From fc0373974f401488cd5e1f73ee1f31ba01b48c46 Mon Sep 17 00:00:00 2001 From: Rory Date: Tue, 24 Dec 2024 14:47:58 +0800 Subject: [PATCH 08/10] polish --- .../authorization/jdbc/JdbcMetadataObject.java | 16 ++++++++++------ .../authorization/jdbc/JdbcSecurableObject.java | 4 +++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java index f7a577aeaee..3f2f227823a 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java @@ -65,12 +65,16 @@ public void validateAuthorizationMetadataObject() throws IllegalArgumentExceptio Preconditions.checkArgument( names.size() <= 2, "The name of the object is not in the format of 'database.table'."); Preconditions.checkArgument(type != null, "The type of the object is null."); - Preconditions.checkArgument( - names.size() == 1 || type.metadataObjectType() == MetadataObject.Type.SCHEMA, - "The type of the object is not SCHEMA."); - Preconditions.checkArgument( - names.size() == 2 || type.metadataObjectType() == MetadataObject.Type.TABLE, - "The type of the object is not TABLE."); + if (names.size() == 1) { + Preconditions.checkArgument( + type.metadataObjectType() == MetadataObject.Type.SCHEMA, + "The type of the object is not SCHEMA."); + } else { + Preconditions.checkArgument( + type.metadataObjectType() == MetadataObject.Type.TABLE, + "The type of the object is not TABLE."); + } + for (String name : names) { Preconditions.checkArgument(name != null, "Cannot create a metadata object with null name"); } diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java index ab07fd2c2e1..78b82e2a8da 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObject.java @@ -53,7 +53,9 @@ static JdbcSecurableObject create( ? JdbcMetadataObject.Type.fromMetadataType(MetadataObject.Type.SCHEMA) : JdbcMetadataObject.Type.fromMetadataType(MetadataObject.Type.TABLE); - return new JdbcSecurableObject(parent, name, type, privileges); + JdbcSecurableObject object = new JdbcSecurableObject(parent, name, type, privileges); + object.validateAuthorizationMetadataObject(); + return object; } @Override From 8dcf09ed7b05338bdfc8473bba2b5ff883e9cef1 Mon Sep 17 00:00:00 2001 From: Rory Date: Tue, 24 Dec 2024 14:49:14 +0800 Subject: [PATCH 09/10] polish the comment --- .../apache/gravitino/authorization/jdbc/JdbcMetadataObject.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java index 3f2f227823a..21963285516 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java @@ -101,7 +101,7 @@ public static Type fromMetadataType(MetadataObject.Type metadataType) { } } throw new IllegalArgumentException( - "No matching RangerMetadataObject.Type for " + metadataType); + "No matching JdbcMetadataObject.Type for " + metadataType); } } } From 74ae88baa0a0fd3d10df6cfd7846105746c17041 Mon Sep 17 00:00:00 2001 From: Rory Date: Tue, 24 Dec 2024 15:15:09 +0800 Subject: [PATCH 10/10] fix style --- .../gravitino/authorization/jdbc/JdbcMetadataObject.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java index 21963285516..c74c7ae6093 100644 --- a/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java +++ b/authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcMetadataObject.java @@ -100,8 +100,7 @@ public static Type fromMetadataType(MetadataObject.Type metadataType) { return type; } } - throw new IllegalArgumentException( - "No matching JdbcMetadataObject.Type for " + metadataType); + throw new IllegalArgumentException("No matching JdbcMetadataObject.Type for " + metadataType); } } }