diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java index e2a1182bd228ac..c1c353e499c659 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java +++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java @@ -118,6 +118,10 @@ public class Auth implements Writable { private PasswordPolicyManager passwdPolicyManager = new PasswordPolicyManager(); + // Prepared point-query plans use this process-local epoch to avoid repeating built-in + // privilege checks while no authorization state has changed. + private transient volatile long authorizationVersion; + private ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); private void readLock() { @@ -136,6 +140,19 @@ private void writeUnlock() { lock.writeLock().unlock(); } + private void markAuthorizationChanged() { + authorizationVersion++; + } + + public long getAuthorizationVersion() { + return authorizationVersion; + } + + /** Whether this version covers every role which can affect authorization decisions. */ + public boolean isAuthorizationVersionReliable() { + return !isLdapAuthEnabled(); + } + public enum PrivLevel { GLOBAL, CATALOG, DATABASE, TABLE, RESOURCE, WORKLOAD_GROUP, CLUSTER, STAGE, STORAGE_VAULT } @@ -587,6 +604,7 @@ private void createUserInternal(UserIdentity userIdent, String roleName, byte[] if (role != null) { userRoleManager.addUserRole(userIdent, roleName); } + markAuthorizationChanged(); // other user properties propertyMgr.addUserResource(userIdent.getQualifiedUser()); MetricRepo.updateUserConnectionMaxMetric(this, userIdent.getQualifiedUser(), @@ -641,6 +659,7 @@ private void dropUserInternal(UserIdentity userIdent, boolean ignoreIfNonExists, roleManager.removeDefaultRole(userIdent); // drop user role userRoleManager.dropUser(userIdent); + markAuthorizationChanged(); passwdPolicyManager.dropUser(userIdent); userManager.removeUser(userIdent); if (CollectionUtils.isEmpty(userManager.getUserByName(userIdent.getQualifiedUser()))) { @@ -755,6 +774,7 @@ private void grantInternal(UserIdentity userIdent, String role, TablePattern tbl } Role newRole = new Role(role, tblPattern, privs, colPrivileges); roleManager.addOrMergeRole(newRole, false /* err on exist */); + markAuthorizationChanged(); if (!isReplay) { PrivInfo info = new PrivInfo(userIdent, tblPattern, privs, null, role, colPrivileges); Env.getCurrentEnv().getEditLog().logGrantPriv(info); @@ -806,6 +826,7 @@ private void grantInternal(UserIdentity userIdent, String role, ResourcePattern Role newRole = new Role(role, resourcePattern, privs); roleManager.addOrMergeRole(newRole, false /* err on exist */); + markAuthorizationChanged(); if (!isReplay) { PrivInfo info = new PrivInfo(userIdent, resourcePattern, privs, null, role); @@ -837,6 +858,7 @@ private void grantInternal(UserIdentity userIdent, String role, WorkloadGroupPat Role newRole = new Role(role, workloadGroupPattern, privs); roleManager.addOrMergeRole(newRole, false /* err on exist */); + markAuthorizationChanged(); if (!isReplay) { PrivInfo info = new PrivInfo(userIdent, workloadGroupPattern, privs, null, role); @@ -862,6 +884,7 @@ private void grantInternal(UserIdentity userIdent, List roles, boolean i } } userRoleManager.addUserRoles(userIdent, roles); + markAuthorizationChanged(); if (!isReplay) { PrivInfo info = new PrivInfo(userIdent, roles); Env.getCurrentEnv().getEditLog().logGrantPriv(info); @@ -952,6 +975,7 @@ private void revokeInternal(UserIdentity userIdent, String role, TablePattern tb } // revoke privs from role roleManager.revokePrivs(role, tblPattern, privs, colPrivileges, errOnNonExist); + markAuthorizationChanged(); if (!isReplay) { PrivInfo info = new PrivInfo(userIdent, tblPattern, privs, null, role, colPrivileges); @@ -973,6 +997,7 @@ private void revokeInternal(UserIdentity userIdent, String role, ResourcePattern // revoke privs from role roleManager.revokePrivs(role, resourcePattern, privs, errOnNonExist); + markAuthorizationChanged(); if (!isReplay) { PrivInfo info = new PrivInfo(userIdent, resourcePattern, privs, null, role); @@ -994,6 +1019,7 @@ private void revokeInternal(UserIdentity userIdent, String role, WorkloadGroupPa // revoke privs from role roleManager.revokePrivs(role, workloadGroupPattern, privs, errOnNonExist); + markAuthorizationChanged(); if (!isReplay) { PrivInfo info = new PrivInfo(userIdent, workloadGroupPattern, privs, null, role); @@ -1019,6 +1045,7 @@ private void revokeInternal(UserIdentity userIdent, List roles, boolean } } userRoleManager.removeUserRoles(userIdent, roles); + markAuthorizationChanged(); if (!isReplay) { PrivInfo info = new PrivInfo(userIdent, roles); Env.getCurrentEnv().getEditLog().logRevokePriv(info); @@ -1134,6 +1161,7 @@ private void createRoleInternal(String role, boolean ignoreIfExists, String comm } roleManager.addOrMergeRole(emptyPrivsRole, true /* err on exist */); + markAuthorizationChanged(); if (!isReplay) { PrivInfo info = new PrivInfo(role, comment); @@ -1167,6 +1195,7 @@ private void dropRoleInternal(String role, boolean ignoreIfNonExists, boolean is roleManager.dropRole(role, true /* err on non exist */); userRoleManager.dropRole(role); + markAuthorizationChanged(); if (!isReplay) { PrivInfo info = new PrivInfo(null, null, null, role, null, null, ""); Env.getCurrentEnv().getEditLog().logDropRole(info); @@ -1996,6 +2025,7 @@ private void setRoleToUser(UserIdentity userIdent, String role) throws DdlExcept userRoleManager.dropUser(userIdent); userRoleManager.addUserRole(userIdent, role); userRoleManager.addUserRole(userIdent, roleManager.getUserDefaultRoleName(userIdent)); + markAuthorizationChanged(); } private void updateUserTlsRequirements(UserIdentity userIdent) throws DdlException { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/SecurityDependencyContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/SecurityDependencyContext.java new file mode 100644 index 00000000000000..52cc9f23b35dba --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/SecurityDependencyContext.java @@ -0,0 +1,303 @@ +// 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.doris.nereids; + +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.authorization.DataMaskSpec; +import org.apache.doris.authorization.RowFilterSpec; +import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.TableIf; +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.datasource.InternalCatalog; +import org.apache.doris.mysql.privilege.Auth; +import org.apache.doris.mysql.privilege.InternalAuthorizationPlugin; +import org.apache.doris.nereids.SqlCacheContext.FullColumnName; +import org.apache.doris.nereids.SqlCacheContext.FullTableName; +import org.apache.doris.nereids.rules.analysis.UserAuthentication; +import org.apache.doris.policy.PolicyMgr; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.SessionVariable; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import org.apache.commons.collections4.CollectionUtils; + +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; + +/** + * Security decisions which an analyzed plan depends on. + * + *

Unlike {@link SqlCacheContext}, this context exists independently of the SQL result-cache switch. A prepared + * short-circuit plan can otherwise outlive the privilege and data-policy decisions made while it was analyzed. + * Callers record both positive and negative policy answers so that adding a policy invalidates a plan which was + * built before that policy existed. + */ +public class SecurityDependencyContext { + private static final long UNKNOWN_VERSION = -1; + + private final Env planningEnv; + private final long authorizationVersion; + private final long rowPolicyVersion; + private final boolean versionValidationEligible; + private final Map> checkedPrivileges = Maps.newLinkedHashMap(); + private final Map> rowPolicies = Maps.newLinkedHashMap(); + private final Map> dataMaskPolicies = Maps.newLinkedHashMap(); + private final Map> dataMaskColumnsByTable = Maps.newLinkedHashMap(); + private boolean useVersionValidation; + private boolean complete = true; + + /** Create a context which always uses full security revalidation. */ + public SecurityDependencyContext() { + this(null, UNKNOWN_VERSION, UNKNOWN_VERSION, false); + } + + /** Create a context and capture the security versions before analysis starts. */ + public SecurityDependencyContext(ConnectContext connectContext) { + this(connectContext == null ? null : connectContext.getEnv(), usesAuthorizationChecks(connectContext)); + } + + private SecurityDependencyContext(Env env, boolean versionValidationEligible) { + this(env, currentAuthorizationVersion(env), currentRowPolicyVersion(env), versionValidationEligible); + } + + private SecurityDependencyContext(Env planningEnv, long authorizationVersion, long rowPolicyVersion, + boolean versionValidationEligible) { + this.planningEnv = planningEnv; + this.authorizationVersion = authorizationVersion; + this.rowPolicyVersion = rowPolicyVersion; + this.versionValidationEligible = versionValidationEligible; + } + + /** Record the columns whose SELECT privilege was checked while the plan was analyzed. */ + public synchronized void addCheckedPrivilege(TableIf table, Set usedColumns) { + Optional tableName = qualifiedName(table); + if (!tableName.isPresent()) { + complete = false; + return; + } + Set existing = checkedPrivileges.get(tableName.get()); + if (existing == null) { + checkedPrivileges.put(tableName.get(), ImmutableSet.copyOf(usedColumns)); + } else { + checkedPrivileges.put(tableName.get(), ImmutableSet.builder() + .addAll(existing).addAll(usedColumns).build()); + } + } + + /** Record the complete row-filter answer, including an empty answer. */ + public synchronized void setRowPolicies( + String catalog, String database, String table, List policies) { + rowPolicies.put(new FullTableName(catalog, database, table), ImmutableList.copyOf(policies)); + } + + /** Record the mask answer for a column, including the absence of a mask. */ + public synchronized void addDataMask( + String catalog, String database, String table, String column, Optional mask) { + String normalizedColumn = column.toLowerCase(Locale.ROOT); + FullTableName tableName = new FullTableName(catalog, database, table); + dataMaskPolicies.put(new FullColumnName(catalog, database, table, normalizedColumn), mask); + dataMaskColumnsByTable.computeIfAbsent(tableName, ignored -> new LinkedHashSet<>()).add(normalizedColumn); + } + + /** Freeze the decisions used by a completed plan before storing them in a reusable context. */ + public synchronized SecurityDependencyContext snapshot() { + SecurityDependencyContext snapshot = new SecurityDependencyContext( + planningEnv, authorizationVersion, rowPolicyVersion, versionValidationEligible); + snapshot.complete = complete; + for (Map.Entry> entry : checkedPrivileges.entrySet()) { + snapshot.checkedPrivileges.put(entry.getKey(), ImmutableSet.copyOf(entry.getValue())); + } + for (Map.Entry> entry : rowPolicies.entrySet()) { + snapshot.rowPolicies.put(entry.getKey(), ImmutableList.copyOf(entry.getValue())); + } + snapshot.dataMaskPolicies.putAll(dataMaskPolicies); + for (Map.Entry> entry : dataMaskColumnsByTable.entrySet()) { + snapshot.dataMaskColumnsByTable.put(entry.getKey(), ImmutableSet.copyOf(entry.getValue())); + } + snapshot.useVersionValidation = snapshot.canUseVersionValidation(); + return snapshot; + } + + /** Freeze the decisions for a prepared short-circuit plan, failing closed if authorization was not recorded. */ + public synchronized SecurityDependencyContext snapshotForShortCircuit() { + SecurityDependencyContext snapshot = snapshot(); + if (checkedPrivileges.isEmpty()) { + snapshot.complete = false; + } + return snapshot; + } + + /** + * Revalidate every security decision before a cached plan bypasses analysis. + * + *

A false result does not deny the statement itself. It rejects only the cached plan, after which the normal + * planning path performs the authoritative checks and returns the usual user-facing error when access was + * revoked. Authorization-source failures also reject reuse, so this fast path always fails closed. + */ + public boolean isValid(ConnectContext connectContext) { + if (!complete || connectContext == null) { + return false; + } + try { + Env env = connectContext.getEnv(); + if (useVersionValidation) { + return usesAuthorizationChecks(connectContext) && versionsAreCurrent(env); + } + UserIdentity currentUser = connectContext.getCurrentUserIdentity(); + if (currentUser == null) { + return false; + } + for (Map.Entry> entry : checkedPrivileges.entrySet()) { + TableIf table = findTable(env, entry.getKey()); + if (table == null) { + return false; + } + UserAuthentication.checkPermission(table, connectContext, entry.getValue()); + } + for (Map.Entry> entry : rowPolicies.entrySet()) { + FullTableName table = entry.getKey(); + List current = env.getAccessManager().evalRowFilterPolicies( + currentUser, table.catalog, table.db, table.table); + if (!CollectionUtils.isEqualCollection(entry.getValue(), current)) { + return false; + } + } + return dataMasksAreValid(env, currentUser); + } catch (UserException | RuntimeException e) { + return false; + } + } + + private boolean canUseVersionValidation() { + if (!complete || !versionValidationEligible || checkedPrivileges.isEmpty() + || authorizationVersion == UNKNOWN_VERSION || rowPolicyVersion == UNKNOWN_VERSION) { + return false; + } + return allDependenciesUseInternalCatalog() && usesVersionedBuiltInAuthorization(planningEnv); + } + + private boolean versionsAreCurrent(Env env) { + if (env == null || env != planningEnv) { + return false; + } + Auth auth = env.getAuth(); + PolicyMgr policyMgr = env.getPolicyMgr(); + return auth != null && policyMgr != null + && auth.isAuthorizationVersionReliable() + && env.getAccessManager().getAccessControllerOrDefault(InternalCatalog.INTERNAL_CATALOG_NAME) + instanceof InternalAuthorizationPlugin + && auth.getAuthorizationVersion() == authorizationVersion + && policyMgr.getRowPolicyVersion() == rowPolicyVersion; + } + + private static boolean usesVersionedBuiltInAuthorization(Env env) { + if (env == null || env.getAuth() == null || env.getPolicyMgr() == null + || !env.getAuth().isAuthorizationVersionReliable()) { + return false; + } + return env.getAccessManager().getAccessControllerOrDefault(InternalCatalog.INTERNAL_CATALOG_NAME) + instanceof InternalAuthorizationPlugin; + } + + private boolean allDependenciesUseInternalCatalog() { + for (FullTableName table : checkedPrivileges.keySet()) { + if (!InternalCatalog.INTERNAL_CATALOG_NAME.equals(table.catalog)) { + return false; + } + } + for (FullTableName table : rowPolicies.keySet()) { + if (!InternalCatalog.INTERNAL_CATALOG_NAME.equals(table.catalog)) { + return false; + } + } + for (FullTableName table : dataMaskColumnsByTable.keySet()) { + if (!InternalCatalog.INTERNAL_CATALOG_NAME.equals(table.catalog)) { + return false; + } + } + return true; + } + + private static long currentAuthorizationVersion(Env env) { + Auth auth = env == null ? null : env.getAuth(); + return auth == null ? UNKNOWN_VERSION : auth.getAuthorizationVersion(); + } + + private static long currentRowPolicyVersion(Env env) { + PolicyMgr policyMgr = env == null ? null : env.getPolicyMgr(); + return policyMgr == null ? UNKNOWN_VERSION : policyMgr.getRowPolicyVersion(); + } + + private static boolean usesAuthorizationChecks(ConnectContext connectContext) { + if (connectContext == null || connectContext.isSkipAuth()) { + return false; + } + SessionVariable sessionVariable = connectContext.getSessionVariable(); + return sessionVariable != null && !sessionVariable.isPlayNereidsDump(); + } + + private boolean dataMasksAreValid(Env env, UserIdentity currentUser) { + for (Map.Entry> entry : dataMaskColumnsByTable.entrySet()) { + FullTableName table = entry.getKey(); + Map current = env.getAccessManager().evalDataMaskPolicies( + currentUser, table.catalog, table.db, table.table, entry.getValue()); + for (String column : entry.getValue()) { + Optional currentMask = Optional.ofNullable( + current.get(column.toLowerCase(Locale.ROOT))); + if (!Objects.equals(dataMaskPolicies.get( + new FullColumnName(table.catalog, table.db, table.table, column)), currentMask)) { + return false; + } + } + } + return true; + } + + private Optional qualifiedName(TableIf table) { + if (table == null) { + return Optional.empty(); + } + DatabaseIf database = table.getDatabase(); + if (database == null || database.getCatalog() == null) { + return Optional.empty(); + } + return Optional.of(new FullTableName( + database.getCatalog().getName(), database.getFullName(), table.getName())); + } + + private TableIf findTable(Env env, FullTableName fullTableName) { + CatalogIf> catalog = env.getCatalogMgr().getCatalog(fullTableName.catalog); + if (catalog == null) { + return null; + } + Optional> database = catalog.getDb(fullTableName.db); + if (!database.isPresent()) { + return null; + } + return database.get().getTable(fullTableName.table).orElse(null); + } +} diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java index 64670a7398b41a..a8caf670852c8b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java @@ -183,6 +183,7 @@ public enum TableFrom { private final Map rewrittenCteConsumer = new HashMap<>(); private final Set viewDdlSqlSet = Sets.newHashSet(); private final SqlCacheContext sqlCacheContext; + private final SecurityDependencyContext securityDependencyContext; // generate for next id for prepared statement's placeholders, which is // connection level @@ -383,6 +384,7 @@ private StatementContext(ConnectContext connectContext, OriginStatement originSt this.connectContext = connectContext; this.originStatement = originStatement; exprIdGenerator = ExprId.createGenerator(initialId); + this.securityDependencyContext = new SecurityDependencyContext(connectContext); if (connectContext != null && connectContext.getSessionVariable() != null) { if (CacheAnalyzer.canUseSqlCache(connectContext.getSessionVariable())) { // cannot set the queryId here because the queryId for the current query is set @@ -692,6 +694,10 @@ public Optional getSqlCacheContext() { return Optional.ofNullable(sqlCacheContext); } + public SecurityDependencyContext getSecurityDependencyContext() { + return securityDependencyContext; + } + public boolean isDpHyp() { return isDpHyp; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckPrivileges.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckPrivileges.java index bce4862aeac94b..1ea1e217887d68 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckPrivileges.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckPrivileges.java @@ -144,6 +144,7 @@ private void checkColumnPrivileges(TableIf table, Set usedColumns) { throw new AnalysisException(e.getMessage(), e); } StatementContext statementContext = cascadesContext.getStatementContext(); + statementContext.getSecurityDependencyContext().addCheckedPrivilege(table, usedColumns); Optional sqlCacheContext = statementContext.getSqlCacheContext(); if (sqlCacheContext.isPresent()) { sqlCacheContext.get().addCheckPrivilegeTablesOrViews(table, usedColumns); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCheckPolicy.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCheckPolicy.java index 40d013ce37c1a2..5e387bc482ef70 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCheckPolicy.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCheckPolicy.java @@ -25,6 +25,7 @@ import org.apache.doris.datasource.CatalogIf; import org.apache.doris.mysql.privilege.AccessControllerManager; import org.apache.doris.nereids.CascadesContext; +import org.apache.doris.nereids.SecurityDependencyContext; import org.apache.doris.nereids.SqlCacheContext; import org.apache.doris.nereids.StatementContext; import org.apache.doris.nereids.analyzer.UnboundAlias; @@ -188,6 +189,7 @@ public RelatedPolicy findPolicy(LogicalPlan logicalPlan, CascadesContext cascade = ImmutableList.builderWithExpectedSize(logicalPlan.getOutput().size()); StatementContext statementContext = cascadesContext.getStatementContext(); + SecurityDependencyContext securityDependencyContext = statementContext.getSecurityDependencyContext(); Optional sqlCacheContext = statementContext.getSqlCacheContext(); boolean hasDataMask = false; // One question for the whole relation rather than one per column: that is what the contract offers @@ -218,6 +220,8 @@ public RelatedPolicy findPolicy(LogicalPlan logicalPlan, CascadesContext cascade if (sqlCacheContext.isPresent()) { sqlCacheContext.get().addDataMaskPolicy(ctlName, dbName, tableName, slot.getName(), dataMaskPolicy); } + securityDependencyContext.addDataMask( + ctlName, dbName, tableName, slot.getName(), dataMaskPolicy); } List rowPolicies = accessManager.evalRowFilterPolicies( @@ -225,6 +229,7 @@ public RelatedPolicy findPolicy(LogicalPlan logicalPlan, CascadesContext cascade if (sqlCacheContext.isPresent()) { sqlCacheContext.get().setRowFilterPolicy(ctlName, dbName, tableName, rowPolicies); } + securityDependencyContext.setRowPolicies(ctlName, dbName, tableName, rowPolicies); return new RelatedPolicy( Optional.ofNullable(CollectionUtils.isEmpty(rowPolicies) diff --git a/fe/fe-core/src/main/java/org/apache/doris/policy/PolicyMgr.java b/fe/fe-core/src/main/java/org/apache/doris/policy/PolicyMgr.java index 0ecb66853d28e2..4e4f104606a1a2 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/policy/PolicyMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/policy/PolicyMgr.java @@ -73,6 +73,13 @@ public class PolicyMgr implements Writable { // ctlName -> dbName -> tableName -> List private Map>>> tablePolicies = Maps.newConcurrentMap(); + // Process-local epoch used to invalidate prepared point-query plans after row-policy changes. + private transient volatile long rowPolicyVersion; + + public long getRowPolicyVersion() { + return rowPolicyVersion; + } + private void writeLock() { lock.writeLock().lock(); } @@ -301,6 +308,7 @@ private void unprotectedAdd(Policy policy) { typeToPolicyMap.put(policy.getType(), dbPolicies); if (PolicyTypeEnum.ROW == policy.getType()) { addTablePolicies((RowPolicy) policy); + rowPolicyVersion++; } } @@ -336,7 +344,7 @@ public void replayStoragePolicyAlter(StoragePolicy log) { private void unprotectedDrop(DropPolicyLog log) { List policies = getPoliciesByType(log.getType()); - policies.removeIf(policy -> { + boolean removed = policies.removeIf(policy -> { if (policy.matchPolicy(log)) { if (policy instanceof StoragePolicy) { ((StoragePolicy) policy).removeResourceReference(); @@ -352,6 +360,9 @@ private void unprotectedDrop(DropPolicyLog log) { return false; }); typeToPolicyMap.put(log.getType(), policies); + if (removed && log.getType() == PolicyTypeEnum.ROW) { + rowPolicyVersion++; + } } public List getUserPolicies(String ctlName, String dbName, String tableName, UserIdentity user) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/ShortCircuitQueryContext.java b/fe/fe-core/src/main/java/org/apache/doris/qe/ShortCircuitQueryContext.java index 99496be25c9b78..287945f682d8eb 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/ShortCircuitQueryContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/ShortCircuitQueryContext.java @@ -23,6 +23,7 @@ import org.apache.doris.analysis.Queriable; import org.apache.doris.catalog.OlapTable; import org.apache.doris.catalog.Type; +import org.apache.doris.nereids.SecurityDependencyContext; import org.apache.doris.planner.OlapScanNode; import org.apache.doris.planner.Planner; import org.apache.doris.thrift.TExpr; @@ -61,6 +62,7 @@ public class ShortCircuitQueryContext { public final String tableName; private final long fileCacheQueryLimitBytes; private final long partitionTopologyVersion; + private final SecurityDependencyContext securityDependencyContext; public final OlapScanNode scanNode; public final Queriable analzyedQuery; @@ -86,7 +88,8 @@ List getReturnTypes() { return returnTypes; } - public ShortCircuitQueryContext(Planner planner, Queriable analzyedQuery) throws TException { + public ShortCircuitQueryContext(Planner planner, Queriable analzyedQuery, + SecurityDependencyContext securityDependencyContext) throws TException { this.planner = planner; this.serializedDescTable = ByteString.copyFrom( new TSerializer().serialize(DescriptorToThriftConverter.toThrift(planner.getDescTable()))); @@ -118,11 +121,18 @@ public ShortCircuitQueryContext(Planner planner, Queriable analzyedQuery) throws this.schemaVersion = this.tbl.getBaseSchemaVersion(); this.partitionTopologyVersion = this.tbl.getPartitionTopologyVersion(); this.analzyedQuery = analzyedQuery; + this.securityDependencyContext = Objects.requireNonNull(securityDependencyContext).snapshotForShortCircuit(); } @VisibleForTesting ShortCircuitQueryContext(OlapTable tbl, String tableName, int schemaVersion, long fileCacheQueryLimitBytes) { + this(tbl, tableName, schemaVersion, fileCacheQueryLimitBytes, null); + } + + @VisibleForTesting + ShortCircuitQueryContext(OlapTable tbl, String tableName, int schemaVersion, + long fileCacheQueryLimitBytes, SecurityDependencyContext securityDependencyContext) { this.planner = null; this.serializedDescTable = ByteString.EMPTY; this.serializedOutputExpr = ByteString.EMPTY; @@ -135,6 +145,7 @@ public ShortCircuitQueryContext(Planner planner, Queriable analzyedQuery) throws this.partitionTopologyVersion = tbl.getPartitionTopologyVersion(); this.scanNode = null; this.analzyedQuery = null; + this.securityDependencyContext = securityDependencyContext; } public boolean isReusable(ConnectContext ctx) { @@ -142,7 +153,8 @@ public boolean isReusable(ConnectContext ctx) { && this.tbl.getBaseSchemaVersion() == this.schemaVersion && Objects.equals(this.tableName, this.tbl.getName()) && this.fileCacheQueryLimitBytes == ctx.getSessionVariable().fileCacheQueryLimitBytes - && this.tbl.getPartitionTopologyVersion() == this.partitionTopologyVersion; + && this.tbl.getPartitionTopologyVersion() == this.partitionTopologyVersion + && (securityDependencyContext == null || securityDependencyContext.isValid(ctx)); } public void sanitize() { diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java b/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java index ef0c28cbeb59c7..5759f5076c7516 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java @@ -1552,7 +1552,8 @@ public void executeAndSendResult(boolean isOutfileQuery, boolean isSendFields, if (statementContext.isShortCircuitQuery()) { ShortCircuitQueryContext shortCircuitQueryContext = statementContext.getShortCircuitQueryContext(); if (shortCircuitQueryContext == null) { - shortCircuitQueryContext = new ShortCircuitQueryContext(planner, (Queriable) parsedStmt); + shortCircuitQueryContext = new ShortCircuitQueryContext(planner, (Queriable) parsedStmt, + statementContext.getSecurityDependencyContext()); // ExecuteCommand publishes this same context after a successful first prepared execution. statementContext.setShortCircuitQueryContext(shortCircuitQueryContext); } 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 993973a905d8f3..c5abcf7e70c229 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 @@ -87,4 +87,17 @@ public void testCheckDbPrivWithSessionMappedRoleForTempUser() throws Exception { } } + @Test + public void testAuthorizationVersionAdvancesOnMutation() throws Exception { + Auth auth = Env.getCurrentEnv().getAuth(); + long version = auth.getAuthorizationVersion(); + + addUser("authorization_version_user", true); + + Assertions.assertTrue(auth.getAuthorizationVersion() > version); + long changedVersion = auth.getAuthorizationVersion(); + auth.refreshUserPrivEntriesByResovledIPs(Collections.emptyMap()); + Assertions.assertEquals(changedVersion, auth.getAuthorizationVersion()); + } + } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/SecurityDependencyContextTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/SecurityDependencyContextTest.java new file mode 100644 index 00000000000000..20163bd89f4620 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/SecurityDependencyContextTest.java @@ -0,0 +1,219 @@ +// 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.doris.nereids; + +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.authorization.DataMaskSpec; +import org.apache.doris.authorization.RowFilterSpec; +import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.TableIf; +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.datasource.CatalogMgr; +import org.apache.doris.mysql.privilege.AccessControllerManager; +import org.apache.doris.mysql.privilege.Auth; +import org.apache.doris.mysql.privilege.InternalAuthorizationPlugin; +import org.apache.doris.mysql.privilege.PrivPredicate; +import org.apache.doris.policy.PolicyMgr; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.SessionVariable; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; + +import java.util.Optional; + +public class SecurityDependencyContextTest { + private static final UserIdentity USER = UserIdentity.createAnalyzedUserIdentWithIp("reader", "%"); + private static final String CATALOG = "internal"; + private static final String DATABASE = "db"; + private static final String TABLE = "tbl"; + private static final String COLUMN = "value"; + + @Test + public void testUnchangedPoliciesAreValid() { + RowFilterSpec rowFilter = RowFilterSpec.restrictive("row:1", "tenant_id = 1"); + DataMaskSpec dataMask = new DataMaskSpec("mask:1", "null"); + SecurityDependencyContext dependencies = new SecurityDependencyContext(); + dependencies.setRowPolicies(CATALOG, DATABASE, TABLE, ImmutableList.of(rowFilter)); + dependencies.addDataMask(CATALOG, DATABASE, TABLE, COLUMN, Optional.of(dataMask)); + + ConnectContext connectContext = contextWithPolicies( + ImmutableList.of(RowFilterSpec.restrictive("row:1", "tenant_id = 1")), + ImmutableMap.of(COLUMN, new DataMaskSpec("mask:1", "null"))); + + Assertions.assertTrue(dependencies.snapshot().isValid(connectContext)); + } + + @Test + public void testAddedRowPolicyInvalidatesNegativeSnapshot() { + SecurityDependencyContext dependencies = new SecurityDependencyContext(); + dependencies.setRowPolicies(CATALOG, DATABASE, TABLE, ImmutableList.of()); + ConnectContext connectContext = contextWithPolicies( + ImmutableList.of(RowFilterSpec.restrictive("row:1", "tenant_id = 1")), ImmutableMap.of()); + + Assertions.assertFalse(dependencies.snapshot().isValid(connectContext)); + } + + @Test + public void testChangedRowPolicyInvalidatesSnapshot() { + SecurityDependencyContext dependencies = new SecurityDependencyContext(); + dependencies.setRowPolicies(CATALOG, DATABASE, TABLE, + ImmutableList.of(RowFilterSpec.restrictive("row:1", "tenant_id = 1"))); + ConnectContext connectContext = contextWithPolicies( + ImmutableList.of(RowFilterSpec.restrictive("row:2", "tenant_id = 2")), ImmutableMap.of()); + + Assertions.assertFalse(dependencies.snapshot().isValid(connectContext)); + } + + @Test + public void testAddedDataMaskInvalidatesNegativeSnapshot() { + SecurityDependencyContext dependencies = new SecurityDependencyContext(); + dependencies.addDataMask(CATALOG, DATABASE, TABLE, COLUMN, Optional.empty()); + ConnectContext connectContext = contextWithPolicies(ImmutableList.of(), + ImmutableMap.of(COLUMN, new DataMaskSpec("mask:1", "null"))); + + Assertions.assertFalse(dependencies.snapshot().isValid(connectContext)); + } + + @Test + public void testMissingCurrentIdentityFailsClosed() { + SecurityDependencyContext dependencies = new SecurityDependencyContext(); + ConnectContext connectContext = Mockito.mock(ConnectContext.class); + + Assertions.assertFalse(dependencies.snapshot().isValid(connectContext)); + } + + @Test + public void testMissingPrivilegeRecordingDisablesShortCircuitReuse() { + SecurityDependencyContext dependencies = new SecurityDependencyContext(); + ConnectContext connectContext = Mockito.mock(ConnectContext.class); + Mockito.when(connectContext.getCurrentUserIdentity()).thenReturn(USER); + + Assertions.assertFalse(dependencies.snapshotForShortCircuit().isValid(connectContext)); + } + + @Test + @SuppressWarnings({"rawtypes", "unchecked"}) + public void testBuiltInVersionsSkipDependencyRevalidation() throws Exception { + CatalogIf catalog = Mockito.mock(CatalogIf.class); + DatabaseIf database = Mockito.mock(DatabaseIf.class); + TableIf table = Mockito.mock(TableIf.class); + Auth auth = Mockito.mock(Auth.class); + PolicyMgr policyMgr = Mockito.mock(PolicyMgr.class); + AccessControllerManager accessManager = Mockito.mock(AccessControllerManager.class); + Env env = Mockito.mock(Env.class); + ConnectContext connectContext = Mockito.mock(ConnectContext.class); + + Mockito.when(catalog.getName()).thenReturn(CATALOG); + Mockito.when(database.getCatalog()).thenReturn(catalog); + Mockito.when(database.getFullName()).thenReturn(DATABASE); + Mockito.when(table.getDatabase()).thenReturn(database); + Mockito.when(table.getName()).thenReturn(TABLE); + Mockito.when(auth.getAuthorizationVersion()).thenReturn(7L); + Mockito.when(auth.isAuthorizationVersionReliable()).thenReturn(true); + Mockito.when(policyMgr.getRowPolicyVersion()).thenReturn(11L); + Mockito.when(accessManager.getAccessControllerOrDefault(CATALOG)) + .thenReturn(new InternalAuthorizationPlugin(auth)); + Mockito.when(env.getAuth()).thenReturn(auth); + Mockito.when(env.getPolicyMgr()).thenReturn(policyMgr); + Mockito.when(env.getAccessManager()).thenReturn(accessManager); + Mockito.when(connectContext.getCurrentUserIdentity()).thenReturn(USER); + Mockito.when(connectContext.getEnv()).thenReturn(env); + Mockito.when(connectContext.getSessionVariable()).thenReturn(new SessionVariable()); + + SecurityDependencyContext dependencies = new SecurityDependencyContext(connectContext); + dependencies.addCheckedPrivilege(table, ImmutableSet.of(COLUMN)); + dependencies.setRowPolicies(CATALOG, DATABASE, TABLE, ImmutableList.of()); + dependencies.addDataMask(CATALOG, DATABASE, TABLE, COLUMN, Optional.empty()); + SecurityDependencyContext snapshot = dependencies.snapshotForShortCircuit(); + + Assertions.assertTrue(snapshot.isValid(connectContext)); + Mockito.verify(accessManager, Mockito.never()).checkColumnsPriv( + connectContext, CATALOG, DATABASE, TABLE, ImmutableSet.of(COLUMN), PrivPredicate.SELECT); + Mockito.verify(accessManager, Mockito.never()).evalRowFilterPolicies( + ArgumentMatchers.any(), ArgumentMatchers.anyString(), ArgumentMatchers.anyString(), + ArgumentMatchers.anyString()); + Mockito.verify(accessManager, Mockito.never()).evalDataMaskPolicies( + ArgumentMatchers.any(), ArgumentMatchers.anyString(), ArgumentMatchers.anyString(), + ArgumentMatchers.anyString(), ArgumentMatchers.anySet()); + + Mockito.when(auth.getAuthorizationVersion()).thenReturn(8L); + Assertions.assertFalse(snapshot.isValid(connectContext)); + Mockito.when(auth.getAuthorizationVersion()).thenReturn(7L); + Mockito.when(policyMgr.getRowPolicyVersion()).thenReturn(12L); + Assertions.assertFalse(snapshot.isValid(connectContext)); + } + + @Test + @SuppressWarnings({"rawtypes", "unchecked"}) + public void testPrivilegeRevocationOrAuthorizationFailureInvalidatesSnapshot() throws Exception { + CatalogIf catalog = Mockito.mock(CatalogIf.class); + DatabaseIf database = Mockito.mock(DatabaseIf.class); + TableIf table = Mockito.mock(TableIf.class); + CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class); + AccessControllerManager accessManager = Mockito.mock(AccessControllerManager.class); + Env env = Mockito.mock(Env.class); + ConnectContext connectContext = Mockito.mock(ConnectContext.class); + + Mockito.when(catalog.getName()).thenReturn(CATALOG); + Mockito.when(catalog.getDb(DATABASE)).thenReturn(Optional.of(database)); + Mockito.when(database.getCatalog()).thenReturn(catalog); + Mockito.when(database.getFullName()).thenReturn(DATABASE); + Mockito.when(database.getTable(TABLE)).thenReturn(Optional.of(table)); + Mockito.when(table.getDatabase()).thenReturn(database); + Mockito.when(table.getName()).thenReturn(TABLE); + Mockito.when(catalogMgr.getCatalog(CATALOG)).thenReturn(catalog); + Mockito.when(env.getCatalogMgr()).thenReturn(catalogMgr); + Mockito.when(env.getAccessManager()).thenReturn(accessManager); + Mockito.when(connectContext.getCurrentUserIdentity()).thenReturn(USER); + Mockito.when(connectContext.getEnv()).thenReturn(env); + Mockito.when(connectContext.getSessionVariable()).thenReturn(new SessionVariable()); + + SecurityDependencyContext dependencies = new SecurityDependencyContext(); + dependencies.addCheckedPrivilege(table, ImmutableSet.of(COLUMN)); + SecurityDependencyContext snapshot = dependencies.snapshot(); + Assertions.assertTrue(snapshot.isValid(connectContext)); + + Mockito.doThrow(new UserException("SELECT was revoked or authorization is unavailable")) + .when(accessManager).checkColumnsPriv( + connectContext, CATALOG, DATABASE, TABLE, ImmutableSet.of(COLUMN), PrivPredicate.SELECT); + Assertions.assertFalse(snapshot.isValid(connectContext)); + } + + private ConnectContext contextWithPolicies( + ImmutableList rowFilters, ImmutableMap dataMasks) { + AccessControllerManager accessManager = Mockito.mock(AccessControllerManager.class); + Mockito.when(accessManager.evalRowFilterPolicies(USER, CATALOG, DATABASE, TABLE)).thenReturn(rowFilters); + Mockito.when(accessManager.evalDataMaskPolicies( + ArgumentMatchers.eq(USER), ArgumentMatchers.eq(CATALOG), ArgumentMatchers.eq(DATABASE), + ArgumentMatchers.eq(TABLE), ArgumentMatchers.anySet())).thenReturn(dataMasks); + Env env = Mockito.mock(Env.class); + Mockito.when(env.getAccessManager()).thenReturn(accessManager); + ConnectContext connectContext = Mockito.mock(ConnectContext.class); + Mockito.when(connectContext.getCurrentUserIdentity()).thenReturn(USER); + Mockito.when(connectContext.getEnv()).thenReturn(env); + return connectContext; + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/ExecuteCommandTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/ExecuteCommandTest.java index 7905b31b5efeb1..963ea19518ba97 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/ExecuteCommandTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/ExecuteCommandTest.java @@ -20,6 +20,7 @@ import org.apache.doris.analysis.DescriptorTable; import org.apache.doris.analysis.Queriable; import org.apache.doris.analysis.TableScanParams; +import org.apache.doris.analysis.UserIdentity; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.OlapTable; @@ -29,6 +30,7 @@ import org.apache.doris.datasource.CatalogIf; import org.apache.doris.datasource.mvcc.MvccSnapshot; import org.apache.doris.datasource.mvcc.MvccTable; +import org.apache.doris.nereids.SecurityDependencyContext; import org.apache.doris.nereids.StatementContext; import org.apache.doris.nereids.analyzer.UnboundRelation; import org.apache.doris.nereids.parser.NereidsParser; @@ -280,7 +282,11 @@ public void testFastPathInstallsCachedShortCircuitContextAcrossExecutions() thro Mockito.when(scanNode.getTableNameInPlan()).thenReturn("tbl"); Mockito.when(scanNode.getConjuncts()).thenReturn(Collections.emptyList()); Mockito.when(planner.getScanNodes()).thenReturn(Collections.singletonList(scanNode)); - ShortCircuitQueryContext cachedPlan = new ShortCircuitQueryContext(planner, Mockito.mock(Queriable.class)); + SecurityDependencyContext securityDependencies = Mockito.mock(SecurityDependencyContext.class); + Mockito.when(securityDependencies.snapshotForShortCircuit()).thenReturn(securityDependencies); + Mockito.when(securityDependencies.isValid(connectContext)).thenReturn(true); + ShortCircuitQueryContext cachedPlan = new ShortCircuitQueryContext( + planner, Mockito.mock(Queriable.class), securityDependencies); preparedStatement.shortCircuitQueryContext = Optional.of(cachedPlan); StmtExecutor executor = Mockito.mock(StmtExecutor.class); @@ -288,6 +294,7 @@ public void testFastPathInstallsCachedShortCircuitContextAcrossExecutions() thro SessionVariable sessionVariable = new SessionVariable(); sessionVariable.enableGroupCommitFullPrepare = false; Mockito.when(connectContext.getSessionVariable()).thenReturn(sessionVariable); + Mockito.when(connectContext.getCurrentUserIdentity()).thenReturn(UserIdentity.ROOT); Mockito.when(connectContext.getStatementContext()).thenReturn(statementContext); Mockito.when(executor.getContext()).thenReturn(connectContext); @@ -305,6 +312,38 @@ public void testFastPathInstallsCachedShortCircuitContextAcrossExecutions() thro Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); } + @Test + public void testInvalidSecurityDependenciesRefreshInsteadOfDirectReuse() throws Exception { + String sql = "select * from tbl"; + LogicalPlan logicalPlan = new NereidsParser().parseSingle(sql); + + ConnectContext connectContext = Mockito.mock(ConnectContext.class); + StatementContext statementContext = new StatementContext(); + statementContext.setShortCircuitQuery(true); + PrepareCommand prepareCommand = new PrepareCommand( + "stmt", logicalPlan, Collections.emptyList(), new OriginStatement(sql, 0)); + PreparedStatementContext preparedStatement = new PreparedStatementContext( + prepareCommand, connectContext, statementContext, "stmt"); + ShortCircuitQueryContext cachedPlan = Mockito.mock(ShortCircuitQueryContext.class); + Mockito.when(cachedPlan.isReusable(connectContext)).thenReturn(false); + preparedStatement.shortCircuitQueryContext = Optional.of(cachedPlan); + + StmtExecutor executor = Mockito.mock(StmtExecutor.class); + Mockito.when(connectContext.getPreparedStementContext("stmt")).thenReturn(preparedStatement); + Mockito.when(connectContext.getSessionVariable()).thenReturn(new SessionVariable()); + Mockito.when(connectContext.getStatementContext()).thenReturn(statementContext); + Mockito.when(executor.getContext()).thenReturn(connectContext); + + new ExecuteCommand("stmt", prepareCommand, statementContext).run(connectContext, executor); + + Mockito.verify(cachedPlan).isReusable(connectContext); + Mockito.verify(executor).execute(); + Mockito.verify(executor, Mockito.never()).executeAndSendResult(Mockito.anyBoolean(), Mockito.anyBoolean(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Assertions.assertNotSame(prepareCommand, preparedStatement.command, + "rejecting a stale fast-path plan must rebuild the retained prepared command"); + } + private String resolveNextSnapshot(TableScanParams scanParams, AtomicInteger snapshotId) { return scanParams.getOrResolveMapParams(ignored -> ImmutableMap.of( "scan.snapshot-id", String.valueOf(snapshotId.incrementAndGet()))) diff --git a/fe/fe-core/src/test/java/org/apache/doris/policy/PolicyTest.java b/fe/fe-core/src/test/java/org/apache/doris/policy/PolicyTest.java index 33759675ecf192..27fdab19e50836 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/policy/PolicyTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/policy/PolicyTest.java @@ -188,6 +188,20 @@ public void testDropPolicy() throws Exception { () -> dropPolicy("DROP ROW POLICY test_row_policy1 ON test.table1")); } + @Test + public void testRowPolicyVersionAdvancesOnMutation() throws Exception { + PolicyMgr policyMgr = Env.getCurrentEnv().getPolicyMgr(); + long version = policyMgr.getRowPolicyVersion(); + + createPolicy("CREATE ROW POLICY test_row_policy_version ON test.table1 AS PERMISSIVE" + + " TO test_policy USING (k1 = 1)"); + long createdVersion = policyMgr.getRowPolicyVersion(); + Assertions.assertTrue(createdVersion > version); + + dropPolicy("DROP ROW POLICY test_row_policy_version ON test.table1"); + Assertions.assertTrue(policyMgr.getRowPolicyVersion() > createdVersion); + } + @Test public void testMergeFilter() throws Exception { createPolicy("CREATE ROW POLICY test_row_policy1 ON test.table1 AS RESTRICTIVE TO test_policy USING (k1 = 1)"); diff --git a/fe/fe-core/src/test/java/org/apache/doris/qe/ShortCircuitQueryContextTest.java b/fe/fe-core/src/test/java/org/apache/doris/qe/ShortCircuitQueryContextTest.java index b43f0157073f91..72587d5037475c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/qe/ShortCircuitQueryContextTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/qe/ShortCircuitQueryContextTest.java @@ -27,6 +27,7 @@ import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.catalog.RandomDistributionInfo; import org.apache.doris.catalog.SinglePartitionInfo; +import org.apache.doris.nereids.SecurityDependencyContext; import org.apache.doris.planner.OlapScanNode; import org.apache.doris.planner.Planner; import org.apache.doris.thrift.TQueryOptions; @@ -93,6 +94,18 @@ public void testReusableRequiresSamePartitionTopologyVersion() { Assertions.assertFalse(context.isReusable(connectContext(-1))); } + @Test + public void testReusableRequiresCurrentSecurityDependencies() { + ConnectContext connectContext = connectContext(-1); + SecurityDependencyContext securityDependencyContext = Mockito.mock(SecurityDependencyContext.class); + Mockito.when(securityDependencyContext.isValid(connectContext)).thenReturn(false); + ShortCircuitQueryContext context = new ShortCircuitQueryContext( + table("tbl", 10), "tbl", 10, -1, securityDependencyContext); + + Assertions.assertFalse(context.isReusable(connectContext)); + Mockito.verify(securityDependencyContext).isValid(connectContext); + } + @Test public void testSerializedQueryOptionsKeepBitmapOpCountVersion() throws Exception { TQueryOptions queryOptions = new SessionVariable().toThrift(); @@ -110,7 +123,8 @@ public void testSerializedQueryOptionsKeepBitmapOpCountVersion() throws Exceptio Mockito.when(planner.getScanNodes()).thenReturn(Collections.singletonList(scanNode)); ShortCircuitQueryContext context = - new ShortCircuitQueryContext(planner, Mockito.mock(Queriable.class)); + new ShortCircuitQueryContext(planner, Mockito.mock(Queriable.class), + new SecurityDependencyContext()); TQueryOptions serializedQueryOptions = new TQueryOptions(); new TDeserializer().deserialize(serializedQueryOptions, context.serializedQueryOptions.toByteArray()); diff --git a/regression-test/data/prepared_stmt_p0/prepared_short_circuit_security_refresh.out b/regression-test/data/prepared_stmt_p0/prepared_short_circuit_security_refresh.out new file mode 100644 index 00000000000000..0b85864c0774b3 --- /dev/null +++ b/regression-test/data/prepared_stmt_p0/prepared_short_circuit_security_refresh.out @@ -0,0 +1,15 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !before_policy -- +2 20 restricted + +-- !cached_before_policy -- +2 20 restricted + +-- !after_policy_added -- + +-- !after_policy_dropped -- +2 20 restricted + +-- !after_select_granted -- +2 20 restricted + diff --git a/regression-test/suites/prepared_stmt_p0/prepared_short_circuit_security_refresh.groovy b/regression-test/suites/prepared_stmt_p0/prepared_short_circuit_security_refresh.groovy new file mode 100644 index 00000000000000..86ccf8ccdabbbf --- /dev/null +++ b/regression-test/suites/prepared_stmt_p0/prepared_short_circuit_security_refresh.groovy @@ -0,0 +1,131 @@ +// 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. + +import java.sql.DriverManager +import java.sql.SQLException + +suite("prepared_short_circuit_security_refresh", "nonConcurrent") { + def dbName = context.config.getDbNameByFile(context.file) + def policyName = "prepared_short_circuit_security_refresh_policy" + def testUser = "prepared_short_circuit_security_refresh_user" + def testPassword = "PreparedSecurity@123" + def adminUser = context.config.jdbcUser + def adminPassword = context.config.jdbcPassword + String serverPrepareUrl = getServerPrepareJdbcUrl(context.config.jdbcUrl, dbName) + + sql "DROP TABLE IF EXISTS prepared_short_circuit_security_refresh_tbl" + sql "DROP USER IF EXISTS ${testUser}" + sql "CREATE USER ${testUser} IDENTIFIED BY '${testPassword}'" + sql """ + CREATE TABLE prepared_short_circuit_security_refresh_tbl ( + k INT NOT NULL, + tenant_id INT NOT NULL, + payload VARCHAR(32) NULL + ) ENGINE=OLAP + UNIQUE KEY(k) + DISTRIBUTED BY HASH(k) BUCKETS 1 + PROPERTIES ( + "replication_num" = "1", + "enable_unique_key_merge_on_write" = "true", + "store_row_column" = "true" + ) + """ + sql """DROP ROW POLICY IF EXISTS ${policyName} + ON ${dbName}.prepared_short_circuit_security_refresh_tbl FOR ${testUser}""" + sql """INSERT INTO prepared_short_circuit_security_refresh_tbl + VALUES (1, 10, 'allowed'), (2, 20, 'restricted')""" + sql "GRANT SELECT_PRIV ON ${dbName}.prepared_short_circuit_security_refresh_tbl TO ${testUser}" + sql "SET GLOBAL enable_server_side_prepared_statement = true" + sql "SYNC" + + if (isCloudMode()) { + def clusters = sql "SHOW CLUSTERS" + assertTrue(!clusters.isEmpty()) + sql "GRANT USAGE_PRIV ON CLUSTER `${clusters[0][0]}` TO ${testUser}" + } + + def adminConnection = DriverManager.getConnection(context.config.jdbcUrl, adminUser, adminPassword) + def adminExecute = { String statement -> + adminConnection.createStatement().withCloseable { adminStatement -> + adminStatement.execute(statement) + } + } + + try { + explain { + sql """ + SELECT /*+ SET_VAR(enable_nereids_planner=true, + enable_fallback_to_original_planner=false, + enable_short_circuit_query=true) */ + k, tenant_id, payload + FROM prepared_short_circuit_security_refresh_tbl + WHERE k = 2 + """ + contains "SHORT-CIRCUIT" + } + connect(testUser, testPassword, serverPrepareUrl) { + sql "SET enable_fallback_to_original_planner = false" + def prepared = prepareStatement( + """SELECT /*+ SET_VAR(enable_nereids_planner=true, + enable_fallback_to_original_planner=false, + enable_short_circuit_query=true) */ + k, tenant_id, payload + FROM prepared_short_circuit_security_refresh_tbl + WHERE k = ?""") + assertEquals(com.mysql.cj.jdbc.ServerPreparedStatement, prepared.class) + prepared.setInt(1, 2) + + // The second execution must use the cached point-query plan. + qe_before_policy prepared + qe_cached_before_policy prepared + + adminExecute(""" + CREATE ROW POLICY ${policyName} + ON ${dbName}.prepared_short_circuit_security_refresh_tbl + AS RESTRICTIVE TO ${testUser} USING (tenant_id = 10) + """) + qe_after_policy_added prepared + + adminExecute("""DROP ROW POLICY ${policyName} + ON ${dbName}.prepared_short_circuit_security_refresh_tbl FOR ${testUser}""") + qe_after_policy_dropped prepared + + adminExecute("""REVOKE SELECT_PRIV + ON ${dbName}.prepared_short_circuit_security_refresh_tbl FROM ${testUser}""") + boolean denied = false + try { + prepared.executeQuery().close() + } catch (SQLException e) { + denied = true + logger.info("prepared execution was denied after SELECT revoke: ${e.message}") + } + assertTrue(denied, "the cached point-query plan must not survive SELECT revocation") + + adminExecute("""GRANT SELECT_PRIV + ON ${dbName}.prepared_short_circuit_security_refresh_tbl TO ${testUser}""") + qe_after_select_granted prepared + prepared.close() + } + } finally { + adminExecute("""DROP ROW POLICY IF EXISTS ${policyName} + ON ${dbName}.prepared_short_circuit_security_refresh_tbl FOR ${testUser}""") + adminExecute("""GRANT SELECT_PRIV + ON ${dbName}.prepared_short_circuit_security_refresh_tbl TO ${testUser}""") + adminConnection.close() + sql "DROP USER IF EXISTS ${testUser}" + } +}