This is an automated email from the ASF dual-hosted git repository.
czweng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git
The following commit(s) were added to refs/heads/master by this push:
new 347ae2104 [core] Fix snapshot privilege check logic (#4376)
347ae2104 is described below
commit 347ae2104101c2a3d37832fabac5e5a5b7c20b48
Author: yunfengzhou-hub <[email protected]>
AuthorDate: Mon Oct 28 15:19:45 2024 +0800
[core] Fix snapshot privilege check logic (#4376)
This closes #4376.
---
.../paimon/privilege/NoPrivilegeException.java | 18 +++++
.../apache/paimon/privilege/PrivilegeChecker.java | 16 ++++
.../paimon/privilege/PrivilegedFileStore.java | 2 +-
.../paimon/privilege/PrivilegedFileStoreTable.java | 21 +++++
.../paimon/catalog/FileSystemCatalogTest.java | 3 +-
.../paimon/privilege/PrivilegedCatalogTest.java | 91 ++++++++++++++++++++++
6 files changed, 149 insertions(+), 2 deletions(-)
diff --git
a/paimon-core/src/main/java/org/apache/paimon/privilege/NoPrivilegeException.java
b/paimon-core/src/main/java/org/apache/paimon/privilege/NoPrivilegeException.java
index c868aebe7..5feb78058 100644
---
a/paimon-core/src/main/java/org/apache/paimon/privilege/NoPrivilegeException.java
+++
b/paimon-core/src/main/java/org/apache/paimon/privilege/NoPrivilegeException.java
@@ -23,6 +23,9 @@ import java.util.stream.Collectors;
/** Thrown when tries to perform an operation but the current user does not
have the privilege. */
public class NoPrivilegeException extends RuntimeException {
+ private final String user;
+ private final String objectType;
+ private final String identifier;
public NoPrivilegeException(
String user, String objectType, String identifier,
PrivilegeType... privilege) {
@@ -35,5 +38,20 @@ public class NoPrivilegeException extends RuntimeException {
.collect(Collectors.joining(" or ")),
objectType,
identifier));
+ this.user = user;
+ this.objectType = objectType;
+ this.identifier = identifier;
+ }
+
+ String getUser() {
+ return user;
+ }
+
+ String getObjectType() {
+ return objectType;
+ }
+
+ String getIdentifier() {
+ return identifier;
}
}
diff --git
a/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegeChecker.java
b/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegeChecker.java
index ebd579bdb..1771d40f4 100644
---
a/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegeChecker.java
+++
b/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegeChecker.java
@@ -24,6 +24,22 @@ import java.io.Serializable;
/** Check if current user has privilege to perform related operations. */
public interface PrivilegeChecker extends Serializable {
+ default void assertCanSelectOrInsert(Identifier identifier) {
+ try {
+ assertCanSelect(identifier);
+ } catch (NoPrivilegeException e) {
+ try {
+ assertCanInsert(identifier);
+ } catch (NoPrivilegeException e1) {
+ throw new NoPrivilegeException(
+ e1.getUser(),
+ e1.getObjectType(),
+ e1.getIdentifier(),
+ PrivilegeType.SELECT,
+ PrivilegeType.INSERT);
+ }
+ }
+ }
void assertCanSelect(Identifier identifier);
diff --git
a/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegedFileStore.java
b/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegedFileStore.java
index ec068b25a..59243a535 100644
---
a/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegedFileStore.java
+++
b/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegedFileStore.java
@@ -72,7 +72,7 @@ public class PrivilegedFileStore<T> implements FileStore<T> {
@Override
public SnapshotManager snapshotManager() {
- privilegeChecker.assertCanSelect(identifier);
+ privilegeChecker.assertCanSelectOrInsert(identifier);
return wrapped.snapshotManager();
}
diff --git
a/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegedFileStoreTable.java
b/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegedFileStoreTable.java
index 4eb1a1a90..37990ed5a 100644
---
a/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegedFileStoreTable.java
+++
b/paimon-core/src/main/java/org/apache/paimon/privilege/PrivilegedFileStoreTable.java
@@ -19,6 +19,7 @@
package org.apache.paimon.privilege;
import org.apache.paimon.FileStore;
+import org.apache.paimon.Snapshot;
import org.apache.paimon.catalog.Identifier;
import org.apache.paimon.manifest.ManifestCacheFilter;
import org.apache.paimon.schema.TableSchema;
@@ -35,12 +36,14 @@ import org.apache.paimon.table.source.InnerTableRead;
import org.apache.paimon.table.source.StreamDataTableScan;
import org.apache.paimon.table.source.snapshot.SnapshotReader;
import org.apache.paimon.utils.BranchManager;
+import org.apache.paimon.utils.SnapshotManager;
import org.apache.paimon.utils.TagManager;
import java.time.Duration;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
+import java.util.OptionalLong;
/** {@link FileStoreTable} with privilege checks. */
public class PrivilegedFileStoreTable extends DelegatedFileStoreTable {
@@ -55,6 +58,24 @@ public class PrivilegedFileStoreTable extends
DelegatedFileStoreTable {
this.identifier = identifier;
}
+ @Override
+ public SnapshotManager snapshotManager() {
+ privilegeChecker.assertCanSelectOrInsert(identifier);
+ return wrapped.snapshotManager();
+ }
+
+ @Override
+ public OptionalLong latestSnapshotId() {
+ privilegeChecker.assertCanSelectOrInsert(identifier);
+ return wrapped.latestSnapshotId();
+ }
+
+ @Override
+ public Snapshot snapshot(long snapshotId) {
+ privilegeChecker.assertCanSelectOrInsert(identifier);
+ return wrapped.snapshot(snapshotId);
+ }
+
@Override
public SnapshotReader newSnapshotReader() {
privilegeChecker.assertCanSelect(identifier);
diff --git
a/paimon-core/src/test/java/org/apache/paimon/catalog/FileSystemCatalogTest.java
b/paimon-core/src/test/java/org/apache/paimon/catalog/FileSystemCatalogTest.java
index 67fbd2718..35a5c06ff 100644
---
a/paimon-core/src/test/java/org/apache/paimon/catalog/FileSystemCatalogTest.java
+++
b/paimon-core/src/test/java/org/apache/paimon/catalog/FileSystemCatalogTest.java
@@ -22,7 +22,8 @@ import org.apache.paimon.fs.Path;
import org.junit.jupiter.api.BeforeEach;
-class FileSystemCatalogTest extends CatalogTestBase {
+/** Tests for {@link FileSystemCatalog}. */
+public class FileSystemCatalogTest extends CatalogTestBase {
@BeforeEach
public void setUp() throws Exception {
diff --git
a/paimon-core/src/test/java/org/apache/paimon/privilege/PrivilegedCatalogTest.java
b/paimon-core/src/test/java/org/apache/paimon/privilege/PrivilegedCatalogTest.java
new file mode 100644
index 000000000..019707108
--- /dev/null
+++
b/paimon-core/src/test/java/org/apache/paimon/privilege/PrivilegedCatalogTest.java
@@ -0,0 +1,91 @@
+/*
+ * 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.paimon.privilege;
+
+import org.apache.paimon.catalog.Catalog;
+import org.apache.paimon.catalog.FileSystemCatalogTest;
+import org.apache.paimon.catalog.Identifier;
+import org.apache.paimon.table.FileStoreTable;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.function.Executable;
+
+import java.util.OptionalLong;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/** Tests for {@link PrivilegedCatalog}. */
+public class PrivilegedCatalogTest extends FileSystemCatalogTest {
+ private static final String PASSWORD_ROOT = "123456";
+ private static final String USERNAME_TEST_USER = "test_user";
+ private static final String PASSWORD_TEST_USER = "test_password";
+
+ @BeforeEach
+ public void setUp() throws Exception {
+ super.setUp();
+ getPrivilegeManager("anonymous",
"anonymous").initializePrivilege(PASSWORD_ROOT);
+ catalog = new PrivilegedCatalog(catalog, getPrivilegeManager("root",
PASSWORD_ROOT));
+ }
+
+ @Override
+ @Test
+ public void testGetTable() throws Exception {
+ super.testGetTable();
+ Identifier identifier = Identifier.create("test_db", "test_table");
+
+ PrivilegedCatalog rootCatalog = ((PrivilegedCatalog) catalog);
+ rootCatalog.createPrivilegedUser(USERNAME_TEST_USER,
PASSWORD_TEST_USER);
+ Catalog userCatalog =
+ new PrivilegedCatalog(
+ rootCatalog.wrapped(),
+ getPrivilegeManager(USERNAME_TEST_USER,
PASSWORD_TEST_USER));
+ FileStoreTable dataTable = (FileStoreTable)
userCatalog.getTable(identifier);
+
+ assertNoPrivilege(dataTable::snapshotManager);
+ assertNoPrivilege(dataTable::latestSnapshotId);
+ assertNoPrivilege(() -> dataTable.snapshot(0));
+
+ rootCatalog.grantPrivilegeOnTable(USERNAME_TEST_USER, identifier,
PrivilegeType.SELECT);
+ userCatalog =
+ new PrivilegedCatalog(
+ rootCatalog.wrapped(),
+ getPrivilegeManager(USERNAME_TEST_USER,
PASSWORD_TEST_USER));
+ FileStoreTable dataTable2 = (FileStoreTable)
userCatalog.getTable(identifier);
+
+ assertThat(dataTable2.snapshotManager().latestSnapshotId()).isNull();
+
assertThat(dataTable2.latestSnapshotId()).isEqualTo(OptionalLong.empty());
+ assertThatThrownBy(() -> dataTable2.snapshot(0)).isNotNull();
+ }
+
+ private FileBasedPrivilegeManager getPrivilegeManager(String user, String
password) {
+ return new FileBasedPrivilegeManager(warehouse, fileIO, user,
password);
+ }
+
+ private void assertNoPrivilege(Executable executable) {
+ Exception e = assertThrows(Exception.class, executable);
+ if (e.getCause() != null) {
+ assertThat(e).hasRootCauseInstanceOf(NoPrivilegeException.class);
+ } else {
+ assertThat(e).isInstanceOf(NoPrivilegeException.class);
+ }
+ }
+}
