github-actions[bot] commented on code in PR #67206:
URL: https://github.com/apache/doris/pull/67206#discussion_r3870906655
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/authorizer/ranger/hive/RangerHivePlugin.java:
##########
@@ -17,17 +17,39 @@
package org.apache.doris.catalog.authorizer.ranger.hive;
+import org.apache.ranger.authorization.hadoop.config.RangerPluginConfig;
import org.apache.ranger.plugin.service.RangerAuthContextListener;
import org.apache.ranger.plugin.service.RangerBasePlugin;
+import java.util.Map;
+import java.util.Properties;
+
public class RangerHivePlugin extends RangerBasePlugin {
+ static final String POLICY_REST_URL_PROPERTY =
"ranger.plugin.hive.policy.rest.url";
+
public RangerHivePlugin(String serviceName) {
super(serviceName, null);
}
public RangerHivePlugin(String serviceName, RangerAuthContextListener
rangerAuthContextListener) {
- super(serviceName, null, null);
+ this(serviceName, null, rangerAuthContextListener);
+ }
+
+ public RangerHivePlugin(String serviceName, Map<String, String> properties,
+ RangerAuthContextListener rangerAuthContextListener) {
+ super(createPluginConfig(serviceName, properties));
super.init();
super.registerAuthContextEventListener(rangerAuthContextListener);
}
+
+ static RangerPluginConfig createPluginConfig(String serviceName,
Map<String, String> properties) {
+ if (properties != null &&
properties.containsKey(POLICY_REST_URL_PROPERTY)) {
+ Properties rangerProperties = new Properties();
+ rangerProperties.putAll(properties);
Review Comment:
[P1] Mask Ranger credentials stored in catalog properties
Ranger 2.8 consumes ranger.plugin.hive.policy.rest.client.password as an
HTTP Basic-auth secret, and this putAll makes that key usable from catalog
properties. The persisted key is
access_controller.properties.ranger.plugin.hive.policy.rest.client.password,
but both catalog printers mask only exact entries in
DatasourcePrintableMap.SENSITIVE_KEY, so a user with catalog SHOW receives the
password verbatim. Please register the stored Ranger credential key(s) as
sensitive and test both SHOW CREATE CATALOG and catalog-property output.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/authorizer/ranger/RangerAccessController.java:
##########
@@ -167,6 +165,8 @@ public Optional<DataMaskPolicy>
evalDataMaskPolicy(UserIdentity currentUser, Str
protected abstract RangerAccessRequestImpl createRequest(UserIdentity
currentUser);
+ protected abstract String getSelectAccessType();
Review Comment:
[P2] Preserve precompiled RangerAccessController subclasses
Authorization factories can be loaded from external plugin JARs. A subclass
compiled against the previous public base has no implementation of this new
abstract method; it can still load, but inherited row-filter/data-mask
evaluation now invokes the missing method and fails with AbstractMethodError
during otherwise valid SELECT planning. Please keep a concrete legacy uppercase
SELECT default here and override only Hive, so existing plugins remain
binary-compatible.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/authorizer/ranger/hive/RangerHivePlugin.java:
##########
@@ -17,17 +17,39 @@
package org.apache.doris.catalog.authorizer.ranger.hive;
+import org.apache.ranger.authorization.hadoop.config.RangerPluginConfig;
import org.apache.ranger.plugin.service.RangerAuthContextListener;
import org.apache.ranger.plugin.service.RangerBasePlugin;
+import java.util.Map;
+import java.util.Properties;
+
public class RangerHivePlugin extends RangerBasePlugin {
+ static final String POLICY_REST_URL_PROPERTY =
"ranger.plugin.hive.policy.rest.url";
+
public RangerHivePlugin(String serviceName) {
super(serviceName, null);
}
public RangerHivePlugin(String serviceName, RangerAuthContextListener
rangerAuthContextListener) {
- super(serviceName, null, null);
+ this(serviceName, null, rangerAuthContextListener);
+ }
+
+ public RangerHivePlugin(String serviceName, Map<String, String> properties,
+ RangerAuthContextListener rangerAuthContextListener) {
+ super(createPluginConfig(serviceName, properties));
Review Comment:
[P1] Keep catalog dry runs from mutating process-global Ranger/Hadoop state
This constructor is executed during CREATE CATALOG validation, but
RangerBasePlugin consumes the unfiltered map with process-wide effects: its
constructor can honor ranger.plugin.hive.ugi.* and replace Hadoop's JVM login
user, while init() freezes the singleton AuditProviderFactory from the first
plugin's properties. Controller cleanup restores neither, so a temporary
catalog can change unrelated HMS/HDFS identity and suppress or redirect later
Ranger services' audit events. Please allowlist catalog-safe policy/client
keys, keep UGI/audit settings at process scope, and add an ordering test that
proves both global states survive a properties-backed dry run unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
