This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch dev-1.1.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/dev-1.1.2 by this push:
new 555f5412c3 [fix](auth) Forbid grant USAGE_PRIV to database.table
(#11234) (#11259)
555f5412c3 is described below
commit 555f5412c3d7237a1930bff92d701ccb11644568
Author: yiguolei <676222...@qq.com>
AuthorDate: Wed Jul 27 17:36:53 2022 +0800
[fix](auth) Forbid grant USAGE_PRIV to database.table (#11234) (#11259)
Co-authored-by: Mingyu Chen <morningman....@gmail.com>
---
.../java/org/apache/doris/analysis/GrantStmt.java | 25 ++++++-----
.../java/org/apache/doris/analysis/RevokeStmt.java | 4 +-
.../org/apache/doris/mysql/privilege/PaloAuth.java | 49 +++++++++++++---------
.../org/apache/doris/mysql/privilege/AuthTest.java | 20 ++++-----
4 files changed, 57 insertions(+), 41 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
index 0c18564962..76849a6b3d 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
@@ -120,9 +120,9 @@ public class GrantStmt extends DdlStmt {
}
if (tblPattern != null) {
- checkPrivileges(analyzer, privileges, role, tblPattern);
+ checkTablePrivileges(privileges, role, tblPattern);
} else {
- checkPrivileges(analyzer, privileges, role, resourcePattern);
+ checkResourcePrivileges(privileges, role, resourcePattern);
}
}
@@ -135,15 +135,15 @@ public class GrantStmt extends DdlStmt {
* 5.1 User should has GLOBAL level GRANT_PRIV
* 5.2 or user has DATABASE/TABLE level GRANT_PRIV if grant/revoke to/from
certain database or table.
* 5.3 or user should has 'resource' GRANT_PRIV if grant/revoke to/from
certain 'resource'
+ * 6. Can not grant USAGE_PRIV to database or table
*
- * @param analyzer
* @param privileges
* @param role
* @param tblPattern
* @throws AnalysisException
*/
- public static void checkPrivileges(Analyzer analyzer, List<PaloPrivilege>
privileges,
- String role, TablePattern tblPattern)
throws AnalysisException {
+ public static void checkTablePrivileges(List<PaloPrivilege> privileges,
String role, TablePattern tblPattern)
+ throws AnalysisException {
// Rule 1
if (tblPattern.getPrivLevel() != PrivLevel.GLOBAL &&
(privileges.contains(PaloPrivilege.ADMIN_PRIV)
|| privileges.contains(PaloPrivilege.NODE_PRIV))) {
@@ -153,7 +153,7 @@ public class GrantStmt extends DdlStmt {
// Rule 2
if (privileges.contains(PaloPrivilege.NODE_PRIV) &&
!Catalog.getCurrentCatalog().getAuth()
.checkGlobalPriv(ConnectContext.get(),
PrivPredicate.OPERATOR)) {
- throw new AnalysisException("Only the user with NODE_PRIV can
grant NODE_PRIV to other user");
+ throw new AnalysisException("Only user with NODE_PRIV can
grant/revoke NODE_PRIV to other user");
}
if (role != null) {
@@ -178,18 +178,23 @@ public class GrantStmt extends DdlStmt {
}
}
}
+
+ // Rule 6
+ if (privileges.contains(PaloPrivilege.USAGE_PRIV)) {
+ throw new AnalysisException("Can not grant/revoke USAGE_PRIV
to/from database or table");
+ }
}
- public static void checkPrivileges(Analyzer analyzer, List<PaloPrivilege>
privileges,
- String role, ResourcePattern
resourcePattern) throws AnalysisException {
+ public static void checkResourcePrivileges(List<PaloPrivilege> privileges,
String role,
+ ResourcePattern resourcePattern) throws AnalysisException {
// Rule 1
if (privileges.contains(PaloPrivilege.NODE_PRIV)) {
- throw new AnalysisException("Can not grant NODE_PRIV to any other
users or roles");
+ throw new AnalysisException("Can not grant/revoke NODE_PRIV
to/from any other users or roles");
}
// Rule 2
if (resourcePattern.getPrivLevel() != PrivLevel.GLOBAL &&
privileges.contains(PaloPrivilege.ADMIN_PRIV)) {
- throw new AnalysisException("ADMIN_PRIV privilege can only be
granted on resource *");
+ throw new AnalysisException("ADMIN_PRIV privilege can only be
granted/revoked on/from resource *");
}
if (role != null) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
index 9d2ce60c7c..837ff2001b 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
@@ -111,9 +111,9 @@ public class RevokeStmt extends DdlStmt {
// Revoke operation obey the same rule as Grant operation. reuse the
same method
if (tblPattern != null) {
- GrantStmt.checkPrivileges(analyzer, privileges, role, tblPattern);
+ GrantStmt.checkTablePrivileges(privileges, role, tblPattern);
} else {
- GrantStmt.checkPrivileges(analyzer, privileges, role,
resourcePattern);
+ GrantStmt.checkResourcePrivileges(privileges, role,
resourcePattern);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java
index 309d5b50a6..c46663a80f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java
@@ -595,25 +595,36 @@ public class PaloAuth implements Writable {
}
// 3. set password
- setPasswordInternal(userIdent, password, null, false /* err on non
exist */,
- false /* set by resolver */, true /* is replay */);
-
- // 4. grant privs of role to user
- grantPrivsByRole(userIdent, role);
-
- // other user properties
- propertyMgr.addUserResource(userIdent.getQualifiedUser(), false /*
not system user */);
-
- if (!userIdent.getQualifiedUser().equals(ROOT_USER) &&
!userIdent.getQualifiedUser().equals(ADMIN_USER)) {
- // grant read privs to database information_schema
- TablePattern tblPattern = new
TablePattern(InfoSchemaDb.DATABASE_NAME, "*");
- try {
-
tblPattern.analyze(ClusterNamespace.getClusterNameFromFullName(userIdent.getQualifiedUser()));
- } catch (AnalysisException e) {
- LOG.warn("should not happen", e);
- }
- grantInternal(userIdent, null /* role */, tblPattern,
PrivBitSet.of(PaloPrivilege.SELECT_PRIV),
- false /* err on non exist */, true /* is replay */);
+ setPasswordInternal(userIdent, password, null, false /* err on non
exist */, false /* set by resolver */,
+ true /* is replay */);
+ try {
+ // 4. grant privs of role to user
+ grantPrivsByRole(userIdent, role);
+
+ // other user properties
+ propertyMgr.addUserResource(userIdent.getQualifiedUser(),
false /* not system user */);
+
+ if (!userIdent.getQualifiedUser().equals(ROOT_USER) &&
!userIdent.getQualifiedUser().equals(ADMIN_USER)) {
+ // grant read privs to database information_schema
+ TablePattern tblPattern = new
TablePattern(InfoSchemaDb.DATABASE_NAME, "*");
+ try {
+
tblPattern.analyze(ClusterNamespace.getClusterNameFromFullName(userIdent.getQualifiedUser()));
+ } catch (AnalysisException e) {
+ LOG.warn("should not happen", e);
+ }
+ grantInternal(userIdent, null /* role */, tblPattern,
PrivBitSet.of(PaloPrivilege.SELECT_PRIV),
+ false /* err on non exist */, true /* is replay
*/);
+ }
+ } catch (Throwable t) {
+ // This is a temp protection to avoid bug such as described in
+ // https://github.com/apache/doris/issues/11235
+ // Normally, all operations in try..catch block should not fail
+ // Why add try..catch block after "setPasswordInternal"?
+ // Because after calling "setPasswordInternal()", the
in-memory state has been changed,
+ // so we should make sure the following operations not throw
any exception, if it throws,
+ // exit the process because there is no way to rollback
in-memory state.
+ LOG.error("got unexpected exception when creating user. exit",
t);
+ System.exit(-1);
}
if (!isReplay) {
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 1d06a5b276..f6bfadaa0c 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
@@ -34,6 +34,7 @@ import org.apache.doris.catalog.DomainResolver;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.DdlException;
+import org.apache.doris.common.ExceptionChecker;
import org.apache.doris.common.UserException;
import org.apache.doris.persist.EditLog;
import org.apache.doris.persist.PrivInfo;
@@ -1586,15 +1587,14 @@ public class AuthTest {
// 2. grant resource priv to db table
TablePattern tablePattern = new TablePattern("db1", "*");
- grantStmt = new GrantStmt(userIdentity, null, tablePattern,
usagePrivileges);
- hasException = false;
- try {
- grantStmt.analyze(analyzer);
- auth.grant(grantStmt);
- } catch (UserException e) {
- e.printStackTrace();
- hasException = true;
- }
- Assert.assertTrue(hasException);
+ GrantStmt grantStmt2 = new GrantStmt(userIdentity, null, tablePattern,
usagePrivileges);
+ ExceptionChecker.expectThrowsWithMsg(AnalysisException.class,
+ "Can not grant/revoke USAGE_PRIV to/from database or table",
() -> grantStmt2.analyze(analyzer));
+
+ // 3. grant resource prov to role on db.table
+ tablePattern = new TablePattern("db1", "*");
+ 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));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
