This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push:
new 32d94cdca24 [fix](auth)unified workload and resource permission logic
#32907 (#33923)
32d94cdca24 is described below
commit 32d94cdca24d503885ce45b6a7ed43539681da9f
Author: Mingyu Chen <[email protected]>
AuthorDate: Sat Apr 20 17:20:04 2024 +0800
[fix](auth)unified workload and resource permission logic #32907 (#33923)
---
.../org/apache/doris/analysis/ResourcePattern.java | 27 +++++++----
.../doris/mysql/privilege/ResourcePrivEntry.java | 9 +---
.../doris/mysql/privilege/ResourcePrivTable.java | 19 ++------
.../org/apache/doris/mysql/privilege/Role.java | 17 ++-----
.../mysql/privilege/WorkloadGroupPrivTable.java | 15 ++----
.../org/apache/doris/analysis/GrantStmtTest.java | 4 +-
.../org/apache/doris/mysql/privilege/AuthTest.java | 55 +++++++++++++++++++---
7 files changed, 81 insertions(+), 65 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/ResourcePattern.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/ResourcePattern.java
index 2e9b203c170..92ec507f88d 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ResourcePattern.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ResourcePattern.java
@@ -22,6 +22,7 @@ import org.apache.doris.common.FeNameFormat;
import org.apache.doris.common.io.Text;
import org.apache.doris.common.io.Writable;
import org.apache.doris.mysql.privilege.Auth.PrivLevel;
+import org.apache.doris.persist.gson.GsonPostProcessable;
import org.apache.doris.persist.gson.GsonUtils;
import com.google.common.base.Strings;
@@ -34,14 +35,14 @@ import java.io.IOException;
// only the following 2 formats are allowed
// *
// resource
-public class ResourcePattern implements Writable {
+public class ResourcePattern implements Writable, GsonPostProcessable {
@SerializedName(value = "resourceName")
private String resourceName;
public static ResourcePattern ALL;
static {
- ALL = new ResourcePattern("*");
+ ALL = new ResourcePattern("%");
try {
ALL.analyze();
} catch (AnalysisException e) {
@@ -53,7 +54,11 @@ public class ResourcePattern implements Writable {
}
public ResourcePattern(String resourceName) {
- this.resourceName = Strings.isNullOrEmpty(resourceName) ? "*" :
resourceName;
+ // To be compatible with previous syntax
+ if ("*".equals(resourceName)) {
+ resourceName = "%";
+ }
+ this.resourceName = Strings.isNullOrEmpty(resourceName) ? "%" :
resourceName;
}
public String getResourceName() {
@@ -61,15 +66,11 @@ public class ResourcePattern implements Writable {
}
public PrivLevel getPrivLevel() {
- if (resourceName.equals("*")) {
- return PrivLevel.GLOBAL;
- } else {
- return PrivLevel.RESOURCE;
- }
+ return PrivLevel.RESOURCE;
}
public void analyze() throws AnalysisException {
- if (!resourceName.equals("*")) {
+ if (!resourceName.equals("%")) {
FeNameFormat.checkResourceName(resourceName);
}
}
@@ -105,4 +106,12 @@ public class ResourcePattern implements Writable {
String json = Text.readString(in);
return GsonUtils.GSON.fromJson(json, ResourcePattern.class);
}
+
+ @Override
+ public void gsonPostProcess() throws IOException {
+ // // To be compatible with previous syntax
+ if ("*".equals(resourceName)) {
+ resourceName = "%";
+ }
+ }
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/ResourcePrivEntry.java
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/ResourcePrivEntry.java
index a41f3803088..65546e49ebc 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/ResourcePrivEntry.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/ResourcePrivEntry.java
@@ -27,11 +27,8 @@ import java.io.DataInput;
import java.io.IOException;
public class ResourcePrivEntry extends PrivEntry {
- protected static final String ANY_RESOURCE = "*";
-
protected PatternMatcher resourcePattern;
protected String origResource;
- protected boolean isAnyResource;
protected ResourcePrivEntry() {
}
@@ -41,15 +38,12 @@ public class ResourcePrivEntry extends PrivEntry {
super(privSet);
this.resourcePattern = resourcePattern;
this.origResource = origResource;
- if (origResource.equals(ANY_RESOURCE)) {
- isAnyResource = true;
- }
}
public static ResourcePrivEntry create(String resourceName, PrivBitSet
privs)
throws AnalysisException, PatternMatcherException {
PatternMatcher resourcePattern = PatternMatcher.createMysqlPattern(
- resourceName.equals(ANY_RESOURCE) ? "%" : resourceName,
+ resourceName,
CaseSensibility.RESOURCE.getCaseSensibility());
if (privs.containsNodePriv() || privs.containsDbTablePriv()) {
throw new AnalysisException("Resource privilege can not contains
node or db table privileges: " + privs);
@@ -112,6 +106,5 @@ public class ResourcePrivEntry extends PrivEntry {
} catch (PatternMatcherException e) {
throw new IOException(e);
}
- isAnyResource = origResource.equals(ANY_RESOURCE);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/ResourcePrivTable.java
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/ResourcePrivTable.java
index 010d1026c95..93f8bb20eb9 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/ResourcePrivTable.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/ResourcePrivTable.java
@@ -26,26 +26,15 @@ import org.apache.logging.log4j.Logger;
public class ResourcePrivTable extends PrivTable {
private static final Logger LOG =
LogManager.getLogger(ResourcePrivTable.class);
- /*
- * Return first priv which match the user@host on resourceName The
returned priv will be
- * saved in 'savedPrivs'.
- */
public void getPrivs(String resourceName, PrivBitSet savedPrivs) {
- ResourcePrivEntry matchedEntry = null;
+ // need check all entries, because may have 2 entries match
resourceName,
+ // For example, if the resourceName is g1, there are two entry `%` and
`g1` compound requirements
for (PrivEntry entry : entries) {
ResourcePrivEntry resourcePrivEntry = (ResourcePrivEntry) entry;
// check resource
- if (!resourcePrivEntry.getResourcePattern().match(resourceName)) {
- continue;
+ if (resourcePrivEntry.getResourcePattern().match(resourceName)) {
+ savedPrivs.or(resourcePrivEntry.getPrivSet());
}
-
- matchedEntry = resourcePrivEntry;
- break;
- }
- if (matchedEntry == null) {
- return;
}
-
- savedPrivs.or(matchedEntry.getPrivSet());
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java
index 8724331eb0f..fcb899e09a6 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java
@@ -424,8 +424,8 @@ public class Role implements Writable, GsonPostProcessable {
return true;
}
PrivBitSet savedPrivs = PrivBitSet.of();
- // Workload groups do not support global usage_priv, so only global
admin_priv and usage_priv are checked.
- if (checkGlobalInternal(PrivPredicate.ADMIN, savedPrivs)
+ // usage priv not in global, but grant_priv may in global
+ if (checkGlobalInternal(wanted, savedPrivs)
|| checkWorkloadGroupInternal(workloadGroupName, wanted,
savedPrivs)) {
return true;
}
@@ -522,18 +522,7 @@ public class Role implements Writable, GsonPostProcessable
{
if (privs.isEmpty()) {
return;
}
- // grant privs to user
- switch (resourcePattern.getPrivLevel()) {
- case GLOBAL:
- grantGlobalPrivs(privs);
- break;
- case RESOURCE:
- grantResourcePrivs(resourcePattern.getResourceName(), privs);
- break;
- default:
- Preconditions.checkNotNull(null,
resourcePattern.getPrivLevel());
- }
-
+ grantResourcePrivs(resourcePattern.getResourceName(), privs);
}
private void grantPrivs(WorkloadGroupPattern workloadGroupPattern,
PrivBitSet privs) throws DdlException {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/WorkloadGroupPrivTable.java
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/WorkloadGroupPrivTable.java
index 994118c7431..7b35b41f01f 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/WorkloadGroupPrivTable.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/WorkloadGroupPrivTable.java
@@ -20,20 +20,13 @@ package org.apache.doris.mysql.privilege;
public class WorkloadGroupPrivTable extends PrivTable {
public void getPrivs(String workloadGroupName, PrivBitSet savedPrivs) {
- WorkloadGroupPrivEntry matchedEntry = null;
+ // need check all entries, because may have 2 entries match
workloadGroupName,
+ // For example, if the workloadGroupName is g1, there are two entry
`%` and `g1` compound requirements
for (PrivEntry entry : entries) {
WorkloadGroupPrivEntry workloadGroupPrivEntry =
(WorkloadGroupPrivEntry) entry;
- if
(!workloadGroupPrivEntry.getWorkloadGroupPattern().match(workloadGroupName)) {
- continue;
+ if
(workloadGroupPrivEntry.getWorkloadGroupPattern().match(workloadGroupName)) {
+ savedPrivs.or(workloadGroupPrivEntry.getPrivSet());
}
-
- matchedEntry = workloadGroupPrivEntry;
- break;
- }
- if (matchedEntry == null) {
- return;
}
-
- savedPrivs.or(matchedEntry.getPrivSet());
}
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/analysis/GrantStmtTest.java
b/fe/fe-core/src/test/java/org/apache/doris/analysis/GrantStmtTest.java
index 985cce34686..71273c8ea26 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/analysis/GrantStmtTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/GrantStmtTest.java
@@ -106,8 +106,8 @@ public class GrantStmtTest {
stmt = new GrantStmt(new UserIdentity("testUser", "%"), null, new
ResourcePattern("*"), privileges);
stmt.analyze(analyzer);
- Assert.assertEquals(Auth.PrivLevel.GLOBAL,
stmt.getResourcePattern().getPrivLevel());
- Assert.assertEquals("GRANT Usage_priv ON RESOURCE '*' TO
'testUser'@'%'", stmt.toSql());
+ Assert.assertEquals(Auth.PrivLevel.RESOURCE,
stmt.getResourcePattern().getPrivLevel());
+ Assert.assertEquals("GRANT Usage_priv ON RESOURCE '%' TO
'testUser'@'%'", stmt.toSql());
}
@Test(expected = AnalysisException.class)
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java
b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java
index bf52ea76225..a96586c1fa9 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java
@@ -18,6 +18,7 @@
package org.apache.doris.mysql.privilege;
import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.CompoundPredicate.Operator;
import org.apache.doris.analysis.CreateRoleStmt;
import org.apache.doris.analysis.CreateUserStmt;
import org.apache.doris.analysis.DropRoleStmt;
@@ -1676,15 +1677,17 @@ public class AuthTest {
}
@Test
- public void testResource() {
+ public void testResource() throws UserException {
UserIdentity userIdentity = new UserIdentity("testUser", "%");
String role = "role0";
String resourceName = "spark0";
ResourcePattern resourcePattern = new ResourcePattern(resourceName);
- String anyResource = "*";
+ String anyResource = "%";
ResourcePattern anyResourcePattern = new ResourcePattern(anyResource);
List<AccessPrivilegeWithCols> usagePrivileges = Lists
.newArrayList(new
AccessPrivilegeWithCols(AccessPrivilege.USAGE_PRIV));
+ List<AccessPrivilegeWithCols> grantPrivileges = Lists
+ .newArrayList(new
AccessPrivilegeWithCols(AccessPrivilege.GRANT_PRIV));
UserDesc userDesc = new UserDesc(userIdentity, "12345", true);
// ------ grant|revoke resource to|from user ------
@@ -1829,8 +1832,9 @@ public class AuthTest {
Assert.fail();
}
Assert.assertTrue(accessManager.checkResourcePriv(userIdentity,
resourceName, PrivPredicate.USAGE));
- Assert.assertTrue(accessManager.checkGlobalPriv(userIdentity,
PrivPredicate.USAGE));
- Assert.assertTrue(accessManager.checkGlobalPriv(userIdentity,
PrivPredicate.SHOW_RESOURCES));
+ // anyResource not belong to global auth
+ Assert.assertFalse(accessManager.checkGlobalPriv(userIdentity,
PrivPredicate.USAGE));
+ Assert.assertFalse(accessManager.checkGlobalPriv(userIdentity,
PrivPredicate.SHOW_RESOURCES));
Assert.assertFalse(accessManager.checkGlobalPriv(userIdentity,
PrivPredicate.SHOW));
// 3. revoke usage_priv on resource '*' from 'testUser'@'%'
@@ -1887,7 +1891,7 @@ public class AuthTest {
Assert.fail();
}
Assert.assertTrue(accessManager.checkResourcePriv(userIdentity,
resourceName, PrivPredicate.USAGE));
- Assert.assertTrue(accessManager.checkGlobalPriv(userIdentity,
PrivPredicate.USAGE));
+ Assert.assertFalse(accessManager.checkGlobalPriv(userIdentity,
PrivPredicate.USAGE));
// 3. revoke usage_priv on resource '*' from role 'role0'
revokeStmt = new RevokeStmt(null, role, anyResourcePattern,
usagePrivileges);
@@ -1956,10 +1960,26 @@ public class AuthTest {
GrantStmt grantStmt3 = new GrantStmt(userIdentity, "test_role",
tablePattern, usagePrivileges);
ExceptionChecker.expectThrowsWithMsg(AnalysisException.class,
"Can not grant/revoke USAGE_PRIV to/from database or table",
() -> grantStmt3.analyze(analyzer));
+
+ // 4.drop user
+ dropUser(userIdentity);
+
+ // -------- test anyPattern has usage_priv and g1 has grant_priv
----------
+ // 1. create user with no role
+ createUser(userIdentity);
+ // 2. grant usage_priv on workload group '%' to user
+ grant(new GrantStmt(userIdentity, null, anyResourcePattern,
usagePrivileges));
+ // 3.grant grant_priv on workload group g1
+ grant(new GrantStmt(userIdentity, null, resourcePattern,
grantPrivileges));
+ Assert.assertTrue(accessManager.checkResourcePriv(userIdentity,
resourceName,
+ PrivPredicate.of(PrivBitSet.of(Privilege.USAGE_PRIV,
Privilege.GRANT_PRIV),
+ Operator.AND)));
+ // 4. drop user
+ dropUser(userIdentity);
}
@Test
- public void testWorkloadGroupPriv() {
+ public void testWorkloadGroupPriv() throws UserException {
UserIdentity userIdentity = new UserIdentity("testUser", "%");
String role = "role0";
String workloadGroupName = "g1";
@@ -1968,6 +1988,8 @@ public class AuthTest {
WorkloadGroupPattern anyWorkloadGroupPattern = new
WorkloadGroupPattern(anyWorkloadGroup);
List<AccessPrivilegeWithCols> usagePrivileges = Lists
.newArrayList(new
AccessPrivilegeWithCols(AccessPrivilege.USAGE_PRIV));
+ List<AccessPrivilegeWithCols> grantPrivileges = Lists
+ .newArrayList(new
AccessPrivilegeWithCols(AccessPrivilege.GRANT_PRIV));
UserDesc userDesc = new UserDesc(userIdentity, "12345", true);
// ------ grant|revoke workload group to|from user ------
@@ -2240,6 +2262,27 @@ public class AuthTest {
GrantStmt grantStmt3 = new GrantStmt(userIdentity, "test_role",
tablePattern, usagePrivileges);
ExceptionChecker.expectThrowsWithMsg(AnalysisException.class,
"Can not grant/revoke USAGE_PRIV to/from database or table",
() -> grantStmt3.analyze(analyzer));
+ // 4.drop user
+ dropUser(userIdentity);
+
+ // -------- test anyPattern has usage_priv and g1 has grant_priv
----------
+ // 1. create user with no role
+ createUser(userIdentity);
+ // 2. grant usage_priv on workload group '%' to user
+ grant(new GrantStmt(userIdentity, null, anyWorkloadGroupPattern,
usagePrivileges));
+ // 3.grant grant_priv on workload group g1
+ grant(new GrantStmt(userIdentity, null, workloadGroupPattern,
grantPrivileges));
+ Assert.assertTrue(accessManager.checkWorkloadGroupPriv(userIdentity,
workloadGroupName,
+ PrivPredicate.of(PrivBitSet.of(Privilege.USAGE_PRIV,
Privilege.GRANT_PRIV),
+ Operator.AND)));
+ // 4. drop user
+ dropUser(userIdentity);
+ }
+
+ private void dropUser(UserIdentity userIdentity) throws UserException {
+ DropUserStmt dropUserStmt = new DropUserStmt(userIdentity);
+ dropUserStmt.analyze(analyzer);
+ auth.dropUser(dropUserStmt);
}
private void createUser(UserIdentity userIdentity) throws UserException {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
