[Refactor] Support the AccessControl interface to decouple the logic …

…of permission verification (StarRocks#27489) Support the AccessControl interface to decouple the logic of permission verification Signed-off-by: HangyuanLiu <[email protected]>
HangyuanLiu · Aug 8, 2023 · ac4d5d1 · ac4d5d1
1 parent 5e73f53
commit ac4d5d1
Show file tree

Hide file tree

Showing 41 changed files with 1,736 additions and 1,501 deletions.
diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/ResourceMgr.java b/fe/fe-core/src/main/java/com/starrocks/catalog/ResourceMgr.java
@@ -50,9 +50,10 @@
 import com.starrocks.persist.metablock.SRMetaBlockID;
 import com.starrocks.persist.metablock.SRMetaBlockReader;
 import com.starrocks.persist.metablock.SRMetaBlockWriter;
-import com.starrocks.privilege.PrivilegeActions;
+import com.starrocks.privilege.AccessDeniedException;
 import com.starrocks.qe.ConnectContext;
 import com.starrocks.server.GlobalStateMgr;
+import com.starrocks.sql.analyzer.PrivilegeChecker;
 import com.starrocks.sql.ast.AlterResourceStmt;
 import com.starrocks.sql.ast.CreateResourceStmt;
 import com.starrocks.sql.ast.DropCatalogStmt;
@@ -336,7 +337,10 @@ public ProcResult fetchResult() {
                     continue;
                 }
 
-                if (!PrivilegeActions.checkAnyActionOnResource(ConnectContext.get(), resource.getName())) {
+                try {
+                    PrivilegeChecker.checkAnyActionOnResource(ConnectContext.get().getCurrentUserIdentity(),
+                            ConnectContext.get().getCurrentRoleIds(), resource.getName());
+                } catch (AccessDeniedException e) {
                     continue;
                 }
                 resource.getProcNodeData(result);

diff --git a/fe/fe-core/src/main/java/com/starrocks/common/ErrorCode.java b/fe/fe-core/src/main/java/com/starrocks/common/ErrorCode.java
@@ -110,6 +110,10 @@ public enum ErrorCode {
 
     ERR_NOT_SUPPORTED_AUTH_MODE(1251, new byte[] {'0', '8', '0', '0', '4'},
             "Client does not support authentication protocol requested by server; consider upgrading MySQL client"),
+
+    ERR_ACCESS_DENIED(1252, new byte[] {'4', '2', '0', '0', '0'},
+            "Access denied; you need (at least one of) the %s privilege(s) on %s%s for this operation"),
+
     ERR_UNKNOWN_STORAGE_ENGINE(1286, new byte[] {'4', '2', '0', '0', '0'}, "Unknown storage engine '%s'"),
     ERR_UNKNOWN_TIME_ZONE(1298, new byte[] {'H', 'Y', '0', '0', '0'}, "Unknown or incorrect time zone: '%s'"),
     ERR_WRONG_OBJECT(1347, new byte[] {'H', 'Y', '0', '0', '0'}, "'%s'.'%s' is not '%s'"),
@@ -301,7 +305,7 @@ public enum ErrorCode {
     ERR_PRIVILEGE_DB_NOT_FOUND(5086, new byte[] {'4', '2', '0', '0', '0'},
             "Db [%s] not found when checking privilege"),
     ERR_PRIVILEGE_ACCESS_RESOURCE_DENIED(5087, new byte[] {'4', '2', '0', '0', '0'},
-            "%s denied to user '%s'@'%s' for resoure '%s' when checking privilege"),
+            "%s denied to user '%s'@'%s' for resource '%s' when checking privilege"),
     ERR_PRIVILEGE_ACCESS_TABLE_DENIED(5088, new byte[] {'4', '2', '0', '0', '0'},
             "Access denied for user '%s' to table '%s' when checking privilege"),
     ERR_PRIVILEGE_ROUTINELODE_JOB_NOT_FOUND(5089, new byte[] {'4', '2', '0', '0', '0'},

diff --git a/fe/fe-core/src/main/java/com/starrocks/common/ErrorReport.java b/fe/fe-core/src/main/java/com/starrocks/common/ErrorReport.java
@@ -42,7 +42,7 @@
 // Used to report error happened when execute SQL of user
 public class ErrorReport {
 
-    private static String reportCommon(String pattern, ErrorCode errorCode, Object... objs) {
+    public static String reportCommon(String pattern, ErrorCode errorCode, Object... objs) {
         String errMsg;
         if (pattern == null) {
             errMsg = errorCode.formatErrorMsg(objs);

diff --git a/fe/fe-core/src/main/java/com/starrocks/http/BaseAction.java b/fe/fe-core/src/main/java/com/starrocks/http/BaseAction.java
@@ -37,13 +37,14 @@
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.starrocks.common.DdlException;
+import com.starrocks.privilege.AccessDeniedException;
 import com.starrocks.privilege.AuthorizationMgr;
-import com.starrocks.privilege.PrivilegeActions;
 import com.starrocks.privilege.PrivilegeBuiltinConstants;
 import com.starrocks.privilege.PrivilegeException;
 import com.starrocks.privilege.PrivilegeType;
 import com.starrocks.qe.ConnectContext;
 import com.starrocks.server.GlobalStateMgr;
+import com.starrocks.sql.analyzer.PrivilegeChecker;
 import com.starrocks.sql.ast.UserIdentity;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
@@ -106,7 +107,7 @@ public void handleRequest(BaseRequest request) throws Exception {
             execute(request, response);
         } catch (Exception e) {
             LOG.warn("fail to process url: {}", request.getRequest().uri(), e);
-            if (e instanceof UnauthorizedException) {
+            if (e instanceof AccessDeniedException) {
                 response.updateHeader(HttpHeaderNames.WWW_AUTHENTICATE.toString(), "Basic realm=\"\"");
                 writeResponse(request, response, HttpResponseStatus.UNAUTHORIZED);
             } else {
@@ -289,65 +290,57 @@ public static ActionAuthorizationInfo of(String fullUserName, String password, S
     }
 
     // For new RBAC privilege framework
-    protected void checkActionOnSystem(UserIdentity currentUser, PrivilegeType... systemActions)
-            throws UnauthorizedException {
+    protected void checkActionOnSystem(UserIdentity currentUser, PrivilegeType... systemActions) {
         for (PrivilegeType systemAction : systemActions) {
-            if (!PrivilegeActions.checkSystemAction(currentUser, null, systemAction)) {
-                throw new UnauthorizedException("Access denied; you need (at least one of) the "
-                        + systemAction.name() + " privilege(s) for this operation");
-            }
+            PrivilegeChecker.checkSystemAction(currentUser, null, systemAction);
         }
     }
 
     // We check whether user owns db_admin and user_admin role in new RBAC privilege framework for
     // operation which checks `PrivPredicate.ADMIN` in global table in old Auth framework.
-    protected void checkUserOwnsAdminRole(UserIdentity currentUser) throws UnauthorizedException {
+    protected void checkUserOwnsAdminRole(UserIdentity currentUser) throws AccessDeniedException {
         try {
             Set<Long> userOwnedRoles = AuthorizationMgr.getOwnedRolesByUser(currentUser);
             if (!(currentUser.equals(UserIdentity.ROOT) ||
                     userOwnedRoles.contains(PrivilegeBuiltinConstants.ROOT_ROLE_ID) ||
                     (userOwnedRoles.contains(PrivilegeBuiltinConstants.DB_ADMIN_ROLE_ID) &&
                             userOwnedRoles.contains(PrivilegeBuiltinConstants.USER_ADMIN_ROLE_ID)))) {
-                throw new UnauthorizedException(
+                throw new AccessDeniedException(
                         "Access denied; you need own root role or own db_admin and user_admin roles for this " +
                                 "operation");
             }
         } catch (PrivilegeException e) {
-            UnauthorizedException newException = new UnauthorizedException(
+            AccessDeniedException newException = new AccessDeniedException(
                     "Access denied; you need own db_admin and user_admin roles for this operation");
             newException.initCause(e);
         }
     }
 
-    protected void checkTableAction(ConnectContext context, String db, String tbl,
-                                    PrivilegeType action) throws UnauthorizedException {
-        if (!PrivilegeActions.checkTableAction(context, db, tbl, action)) {
-            throw new UnauthorizedException("Access denied; you need (at least one of) the "
-                    + action.name() + " privilege(s) for this operation");
-        }
+    protected void checkTableAction(ConnectContext context, String db, String tbl, PrivilegeType privType) {
+        PrivilegeChecker.checkTableAction(context.getCurrentUserIdentity(), context.getCurrentRoleIds(), db, tbl, privType);
     }
 
     // return currentUserIdentity from StarRocks auth
     public static UserIdentity checkPassword(ActionAuthorizationInfo authInfo)
-            throws UnauthorizedException {
+            throws AccessDeniedException {
         GlobalStateMgr globalStateMgr = GlobalStateMgr.getCurrentState();
         UserIdentity currentUser =
                 globalStateMgr.getAuthenticationMgr().checkPlainPassword(
                         authInfo.fullUserName, authInfo.remoteIp, authInfo.password);
         if (currentUser == null) {
-            throw new UnauthorizedException("Access denied for "
+            throw new AccessDeniedException("Access denied for "
                     + authInfo.fullUserName + "@" + authInfo.remoteIp);
         }
         return currentUser;
     }
 
     public ActionAuthorizationInfo getAuthorizationInfo(BaseRequest request)
-            throws UnauthorizedException {
+            throws AccessDeniedException {
         ActionAuthorizationInfo authInfo = new ActionAuthorizationInfo();
         if (!parseAuthInfo(request, authInfo)) {
             LOG.info("parse auth info failed, Authorization header {}, url {}",
                     request.getAuthorizationHeader(), request.getRequest().uri());
-            throw new UnauthorizedException("Need auth information.");
+            throw new AccessDeniedException("Need auth information.");
         }
         LOG.debug("get auth info: {}", authInfo);
         return authInfo;

diff --git a/fe/fe-core/src/main/java/com/starrocks/http/UnauthorizedException.java b/fe/fe-core/src/main/java/com/starrocks/http/UnauthorizedException.java
diff --git a/fe/fe-core/src/main/java/com/starrocks/http/action/WebBaseAction.java b/fe/fe-core/src/main/java/com/starrocks/http/action/WebBaseAction.java
@@ -46,8 +46,8 @@
 import com.starrocks.http.BaseResponse;
 import com.starrocks.http.HttpAuthManager;
 import com.starrocks.http.HttpAuthManager.SessionValue;
-import com.starrocks.http.UnauthorizedException;
 import com.starrocks.http.rest.RestBaseResult;
+import com.starrocks.privilege.AccessDeniedException;
 import com.starrocks.privilege.PrivilegeType;
 import com.starrocks.qe.ConnectContext;
 import com.starrocks.server.GlobalStateMgr;
@@ -195,7 +195,7 @@ private boolean checkAuthWithCookie(BaseRequest request, BaseResponse response)
             ctx.setThreadLocalInfo();
 
             return true;
-        } catch (UnauthorizedException e) {
+        } catch (AccessDeniedException e) {
             response.appendContent("Authentication Failed. <br/> " + e.getMessage());
             writeAuthResponse(request, response);
             return false;
@@ -217,7 +217,7 @@ private boolean checkCookie(BaseRequest request, BaseResponse response) {
                 checkUserOwnsAdminRole(sessionValue.currentUser);
                 checkActionOnSystem(sessionValue.currentUser, PrivilegeType.NODE);
                 authorized = true;
-            } catch (UnauthorizedException e) {
+            } catch (AccessDeniedException e) {
                 // ignore
             }
 

diff --git a/fe/fe-core/src/main/java/com/starrocks/http/rest/GetLoadInfoAction.java b/fe/fe-core/src/main/java/com/starrocks/http/rest/GetLoadInfoAction.java
@@ -40,11 +40,10 @@
 import com.starrocks.http.BaseRequest;
 import com.starrocks.http.BaseResponse;
 import com.starrocks.http.IllegalArgException;
-import com.starrocks.http.UnauthorizedException;
 import com.starrocks.load.Load;
-import com.starrocks.privilege.PrivilegeActions;
 import com.starrocks.privilege.PrivilegeType;
 import com.starrocks.qe.ConnectContext;
+import com.starrocks.sql.analyzer.PrivilegeChecker;
 import io.netty.handler.codec.http.HttpMethod;
 
 // Get load information of one load job
@@ -76,10 +75,8 @@ public void executeWithoutPassword(BaseRequest request, BaseResponse response)
         }
 
         if (info.tblNames.isEmpty()) {
-            if (!PrivilegeActions.checkActionInDb(ConnectContext.get(), info.dbName, PrivilegeType.INSERT)) {
-                throw new UnauthorizedException(
-                        "Access denied; you need (at least one of) the INSERT privilege(s) for this operation");
-            }
+            PrivilegeChecker.checkActionInDb(ConnectContext.get().getCurrentUserIdentity(),
+                    ConnectContext.get().getCurrentRoleIds(), info.dbName, PrivilegeType.INSERT);
         } else {
             for (String tblName : info.tblNames) {
                 checkTableAction(ConnectContext.get(), info.dbName, tblName, PrivilegeType.INSERT);

diff --git a/fe/fe-core/src/main/java/com/starrocks/http/rest/MetricsAction.java b/fe/fe-core/src/main/java/com/starrocks/http/rest/MetricsAction.java
@@ -40,12 +40,12 @@
 import com.starrocks.http.BaseRequest;
 import com.starrocks.http.BaseResponse;
 import com.starrocks.http.IllegalArgException;
-import com.starrocks.http.UnauthorizedException;
 import com.starrocks.metric.JsonMetricVisitor;
 import com.starrocks.metric.MetricRepo;
 import com.starrocks.metric.MetricVisitor;
 import com.starrocks.metric.PrometheusMetricVisitor;
 import com.starrocks.metric.SimpleCoreMetricVisitor;
+import com.starrocks.privilege.AccessDeniedException;
 import com.starrocks.sql.ast.UserIdentity;
 import io.netty.handler.codec.http.HttpMethod;
 import org.apache.logging.log4j.LogManager;
@@ -98,7 +98,7 @@ public void execute(BaseRequest request, BaseResponse response) throws DdlExcept
                 if (WITH_TABLE_METRICS_ALL.equalsIgnoreCase(withTableMetrics)) {
                     minifyTableMetrics = false;
                 }
-            } catch (UnauthorizedException e) {
+            } catch (AccessDeniedException e) {
                 LOG.warn("Auth failure when getting table level metrics, current user: {}, error msg: {}",
                         currentUser, e.getMessage(), e);
             }

diff --git a/fe/fe-core/src/main/java/com/starrocks/http/rest/RestBaseAction.java b/fe/fe-core/src/main/java/com/starrocks/http/rest/RestBaseAction.java
@@ -42,6 +42,8 @@
 import com.starrocks.http.BaseAction;
 import com.starrocks.http.BaseRequest;
 import com.starrocks.http.BaseResponse;
+import com.starrocks.http.HttpConnectContext;
+import com.starrocks.privilege.AccessDeniedException;
 import com.starrocks.http.UnauthorizedException;
 import com.starrocks.qe.ConnectContext;
 import com.starrocks.server.GlobalStateMgr;
@@ -71,23 +73,14 @@ public void handleRequest(BaseRequest request) {
         BaseResponse response = new BaseResponse();
         try {
             execute(request, response);
+        } catch (AccessDeniedException accessDeniedException) {
+            LOG.warn("fail to process url: {}", request.getRequest().uri(), accessDeniedException);
+            response.updateHeader(HttpHeaderNames.WWW_AUTHENTICATE.toString(), "Basic realm=\"\"");
+            response.appendContent(new RestBaseResult(accessDeniedException.getMessage()).toJson());
+            writeResponse(request, response, HttpResponseStatus.UNAUTHORIZED);
         } catch (DdlException e) {
             LOG.warn("fail to process url: {}", request.getRequest().uri(), e);
-            if (e instanceof UnauthorizedException) {
-                response.updateHeader(HttpHeaderNames.WWW_AUTHENTICATE.toString(), "Basic realm=\"\"");
-                response.appendContent(new RestBaseResult(e.getMessage()).toJson());
-                writeResponse(request, response, HttpResponseStatus.UNAUTHORIZED);
-            } else {
-                sendResult(request, response, new RestBaseResult(e.getMessage()));
-            }
-        } catch (Exception e) {
-            LOG.warn("fail to process url: {}", request.getRequest().uri(), e);
-            String msg = e.getMessage();
-            if (msg == null) {
-                msg = e.toString();
-            }
-            response.appendContent(new RestBaseResult(msg).toJson());
-            writeResponse(request, response, HttpResponseStatus.INTERNAL_SERVER_ERROR);
+            sendResult(request, response, new RestBaseResult(e.getMessage()));
         }
     }
 

diff --git a/fe/fe-core/src/main/java/com/starrocks/http/rest/StopFeAction.java b/fe/fe-core/src/main/java/com/starrocks/http/rest/StopFeAction.java
@@ -19,7 +19,7 @@
 import com.starrocks.http.BaseRequest;
 import com.starrocks.http.BaseResponse;
 import com.starrocks.http.IllegalArgException;
-import com.starrocks.http.UnauthorizedException;
+import com.starrocks.privilege.AccessDeniedException;
 import com.starrocks.privilege.PrivilegeType;
 import com.starrocks.qe.ConnectContext;
 import com.starrocks.sql.ast.UserIdentity;
@@ -36,7 +36,7 @@ public static void registerAction(ActionController controller)
     }
 
     @Override
-    public void executeWithoutPassword(BaseRequest request, BaseResponse response) throws UnauthorizedException {
+    public void executeWithoutPassword(BaseRequest request, BaseResponse response) throws AccessDeniedException {
         UserIdentity currentUser = ConnectContext.get().getCurrentUserIdentity();
         checkActionOnSystem(currentUser, PrivilegeType.OPERATE);
 

diff --git a/fe/fe-core/src/main/java/com/starrocks/load/ExportMgr.java b/fe/fe-core/src/main/java/com/starrocks/load/ExportMgr.java
@@ -39,6 +39,7 @@
 import com.google.gson.Gson;
 import com.starrocks.analysis.TableName;
 import com.starrocks.catalog.Database;
+import com.starrocks.catalog.InternalCatalog;
 import com.starrocks.common.AnalysisException;
 import com.starrocks.common.Config;
 import com.starrocks.common.DdlException;
@@ -52,9 +53,10 @@
 import com.starrocks.persist.metablock.SRMetaBlockID;
 import com.starrocks.persist.metablock.SRMetaBlockReader;
 import com.starrocks.persist.metablock.SRMetaBlockWriter;
-import com.starrocks.privilege.PrivilegeActions;
+import com.starrocks.privilege.AccessDeniedException;
 import com.starrocks.qe.ConnectContext;
 import com.starrocks.server.GlobalStateMgr;
+import com.starrocks.sql.analyzer.PrivilegeChecker;
 import com.starrocks.sql.ast.CancelExportStmt;
 import com.starrocks.sql.ast.ExportStmt;
 import com.starrocks.sql.common.MetaUtils;
@@ -221,13 +223,20 @@ public List<List<String>> getExportJobInfosByIdOrState(
                     if (db == null) {
                         continue;
                     }
-                    if (!PrivilegeActions.checkAnyActionOnOrInDb(ConnectContext.get(), db.getFullName())) {
+
+                    try {
+                        PrivilegeChecker.checkAnyActionOnOrInDb(ConnectContext.get().getCurrentUserIdentity(),
+                                ConnectContext.get().getCurrentRoleIds(),
+                                InternalCatalog.DEFAULT_INTERNAL_CATALOG_NAME,
+                                db.getFullName());
+                    } catch (AccessDeniedException e) {
                         continue;
                     }
                 } else {
-                    if (!PrivilegeActions.checkAnyActionOnTable(ConnectContext.get(),
-                            tableName.getDb(),
-                            tableName.getTbl())) {
+                    try {
+                        PrivilegeChecker.checkAnyActionOnTable(ConnectContext.get().getCurrentUserIdentity(),
+                                ConnectContext.get().getCurrentRoleIds(), tableName);
+                    } catch (AccessDeniedException e) {
                         continue;
                     }
                 }

diff --git a/...-core/src/main/java/com/starrocks/load/routineload/RoutineLoadFunctionalExprProvider.java b/...-core/src/main/java/com/starrocks/load/routineload/RoutineLoadFunctionalExprProvider.java
@@ -20,9 +20,10 @@
 import com.starrocks.common.MetaNotFoundException;
 import com.starrocks.common.util.LogBuilder;
 import com.starrocks.common.util.LogKey;
-import com.starrocks.privilege.PrivilegeActions;
+import com.starrocks.privilege.AccessDeniedException;
 import com.starrocks.privilege.PrivilegeType;
 import com.starrocks.qe.ConnectContext;
+import com.starrocks.sql.analyzer.PrivilegeChecker;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -246,11 +247,17 @@ protected ImmutableList<ColumnValueSupplier<RoutineLoadJob>> delegateWhereSuppli
     @Override
     protected boolean delegatePostRowFilter(ConnectContext cxt, RoutineLoadJob job) {
         try {
-            return PrivilegeActions.checkTableAction(
-                    cxt,
-                    job.getDbFullName(),
-                    job.getTableName(),
-                    PrivilegeType.INSERT);
+            try {
+                PrivilegeChecker.checkTableAction(
+                        cxt.getCurrentUserIdentity(), cxt.getCurrentRoleIds(),
+                        job.getDbFullName(),
+                        job.getTableName(),
+                        PrivilegeType.INSERT);
+            } catch (AccessDeniedException e) {
+                return false;
+            }
+
+            return true;
         } catch (MetaNotFoundException e) {
             LOG.warn(new LogBuilder(LogKey.ROUTINE_LOAD_JOB, job.getId())
                     .add("error_msg", "The metadata of this job has been changed. "