mneethiraj commented on code in PR #584:
URL: https://github.com/apache/ranger/pull/584#discussion_r2231326464
##########
agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPluginContext.java:
##########
@@ -40,6 +41,10 @@ public class RangerPluginContext {
private final RangerPluginConfig
config;
private final Map<String, Map<RangerPolicy.RangerPolicyResource,
RangerResourceMatcher>> resourceMatchers = new HashMap<>();
private final ReentrantReadWriteLock
lock = new ReentrantReadWriteLock(true); // fair lock
+ private Mapper
userNameTransformInst;
Review Comment:
I suggest moving new members to `RangerAuthContext` class.
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerDefaultRequestProcessor.java:
##########
@@ -157,6 +174,52 @@ public void enrich(RangerAccessRequest request) {
}
}
+ private String getTransformedUser(PolicyEngine policyEngine,
RangerAccessRequest request) {
+ String user = request.getUser();
+ String userNameCaseConversion =
policyEngine.getPluginContext().getUserNameCaseConversion();
+
+ if
(UgsyncCommonConstants.UGSYNC_LOWER_CASE_CONVERSION_VALUE.equalsIgnoreCase(userNameCaseConversion))
{
Review Comment:
Consider avoiding string comparision on every authz call; instead have the
configured parsed to a boolean in RangerAuthContext and access boolean from
here.
##########
security-admin/src/main/java/org/apache/ranger/common/PropertiesUtil.java:
##########
@@ -526,4 +544,72 @@ protected void
processProperties(ConfigurableListableBeanFactory beanFactory, Pr
super.processProperties(beanFactory, props);
}
+
+ private void updateRangerPluginsPropertiesForUserGroup(Properties props) {
+ if (propertiesMap != null) {
+ String userCaseConv =
StringUtils.isEmpty(propertiesMap.get(RangerCommonConstants.PLUGINS_CONF_USERNAME_CASE_CONVERSION_PARAM))
+ ?
UgsyncCommonConstants.DEFAULT_UGSYNC_USERNAME_CASE_CONVERSION_VALUE
+ :
propertiesMap.get(RangerCommonConstants.PLUGINS_CONF_USERNAME_CASE_CONVERSION_PARAM);
+
propertiesMap.put(RangerCommonConstants.PLUGINS_CONF_USERNAME_CASE_CONVERSION_PARAM,
userCaseConv);
+
props.put(RangerCommonConstants.PLUGINS_CONF_USERNAME_CASE_CONVERSION_PARAM,
userCaseConv);
+
+ String groupCaseConv =
StringUtils.isEmpty(propertiesMap.get(RangerCommonConstants.PLUGINS_CONF_GROUPNAME_CASE_CONVERSION_PARAM))
+ ?
UgsyncCommonConstants.DEFAULT_UGSYNC_GROUPNAME_CASE_CONVERSION_VALUE
+ :
propertiesMap.get(RangerCommonConstants.PLUGINS_CONF_GROUPNAME_CASE_CONVERSION_PARAM);
+
propertiesMap.put(RangerCommonConstants.PLUGINS_CONF_GROUPNAME_CASE_CONVERSION_PARAM,
groupCaseConv);
+
props.put(RangerCommonConstants.PLUGINS_CONF_GROUPNAME_CASE_CONVERSION_PARAM,
groupCaseConv);
+
+ String userHandler =
StringUtils.isEmpty(propertiesMap.get(RangerCommonConstants.PLUGINS_CONF_MAPPING_USERNAME_HANDLER))
+ ?
UgsyncCommonConstants.DEFAULT_SYNC_MAPPING_USERNAME_HANDLER
+ :
propertiesMap.get(RangerCommonConstants.PLUGINS_CONF_MAPPING_USERNAME_HANDLER);
+
propertiesMap.put(RangerCommonConstants.PLUGINS_CONF_MAPPING_USERNAME_HANDLER,
userHandler);
+
props.put(RangerCommonConstants.PLUGINS_CONF_MAPPING_USERNAME_HANDLER,
userHandler);
+
+ String groupHandler =
StringUtils.isEmpty(propertiesMap.get(RangerCommonConstants.PLUGINS_CONF_MAPPING_GROUPNAME_HANDLER))
+ ?
UgsyncCommonConstants.DEFAULT_SYNC_MAPPING_GROUPNAME_HANDLER
+ :
propertiesMap.get(RangerCommonConstants.PLUGINS_CONF_MAPPING_GROUPNAME_HANDLER);
+
propertiesMap.put(RangerCommonConstants.PLUGINS_CONF_MAPPING_GROUPNAME_HANDLER,
groupHandler);
+
props.put(RangerCommonConstants.PLUGINS_CONF_MAPPING_GROUPNAME_HANDLER,
groupHandler);
+
+
propertiesMap.put(RangerCommonConstants.PLUGINS_CONF_MAPPING_SEPARATOR,
getRegexSeparator());
+ props.put(RangerCommonConstants.PLUGINS_CONF_MAPPING_SEPARATOR,
getRegexSeparator());
+
+
propertiesMap.putAll(getAllRegexPatternsConfig(RangerCommonConstants.PLUGINS_CONF_MAPPING_USERNAME));
+
props.putAll(getAllRegexPatternsConfig(RangerCommonConstants.PLUGINS_CONF_MAPPING_USERNAME));
+
+
propertiesMap.putAll(getAllRegexPatternsConfig(RangerCommonConstants.PLUGINS_CONF_MAPPING_GROUPNAME));
+
props.putAll(getAllRegexPatternsConfig(RangerCommonConstants.PLUGINS_CONF_MAPPING_GROUPNAME));
+ }
+ }
+
+ private static String getRegexSeparator() {
+ String ret = UgsyncCommonConstants.DEFAULT_MAPPING_SEPARATOR;
+ String val =
PropertiesUtil.getProperty(RangerCommonConstants.PLUGINS_CONF_MAPPING_SEPARATOR);
+ if (StringUtils.isNotEmpty(val)) {
+ if (val.length() == 1) {
+ ret = val;
+ } else {
+ LOG.warn("More than one character found in RegEx Separator,
using default RegEx Separator");
+ }
+ }
+ LOG.info(String.format("Using %s as the RegEx Separator", ret));
Review Comment:
Replace use of `String.format` with parameterized log message.
##########
ugsync-util/src/main/java/org/apache/ranger/ugsyncutil/transform/RegEx.java:
##########
@@ -69,7 +62,7 @@ public String transform(String attrValue) {
return result;
}
- protected void populateReplacementPatterns(String baseProperty,
List<String> regexPatterns, String regexSeparator) {
+ public void populateReplacementPatterns(String baseProperty, List<String>
regexPatterns, String regexSeparator) {
Review Comment:
This is referenced from this class, and its test class `TestRegEx`. I
suggest changing visibility to package-private i.e. remove `public`. Btw, there
is a typo in the test class package name:
`org.apache.ranger.ugsynutil.transform` =>
`org.apache.ranger.ugsyncutil.transform` (missing `c` in `ugsyncutil`).
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerDefaultRequestProcessor.java:
##########
@@ -157,6 +174,52 @@ public void enrich(RangerAccessRequest request) {
}
}
+ private String getTransformedUser(PolicyEngine policyEngine,
RangerAccessRequest request) {
+ String user = request.getUser();
+ String userNameCaseConversion =
policyEngine.getPluginContext().getUserNameCaseConversion();
+
+ if
(UgsyncCommonConstants.UGSYNC_LOWER_CASE_CONVERSION_VALUE.equalsIgnoreCase(userNameCaseConversion))
{
+ user = user.toLowerCase();
+ } else if
(UgsyncCommonConstants.UGSYNC_UPPER_CASE_CONVERSION_VALUE.equalsIgnoreCase(userNameCaseConversion))
{
+ user = user.toUpperCase();
+ }
+
+ Mapper userNameTransformInst =
policyEngine.getPluginContext().getUserNameTransformInst();
+ if (userNameTransformInst != null) {
+ String transformedUser = userNameTransformInst.transform(user);
+ LOG.debug("Original username = {}, Transformed username = {}",
request.getUser(), transformedUser);
+ return transformedUser;
+ }
+
+ return user;
+ }
+
+ private Set<String> getTransformedGroups(PolicyEngine policyEngine,
RangerAccessRequest request) {
+ Mapper groupNameTransformInst =
policyEngine.getPluginContext().getGroupNameTransformInst();
+ String caseConversion =
policyEngine.getPluginContext().getGroupNameCaseConversion();
+
+ if (groupNameTransformInst == null || request.getUserGroups() == null)
{
+ return request.getUserGroups();
+ }
+
+ return request.getUserGroups().stream()
+ .filter(Objects::nonNull)
+ .map(group -> {
+ String originalGroup = group;
+
+ if
(UgsyncCommonConstants.UGSYNC_LOWER_CASE_CONVERSION_VALUE.equalsIgnoreCase(caseConversion))
{
Review Comment:
Consider avoiding string comparision on every authz call, and for each
group; instead have the configured parsed to a boolean in RangerAuthContext and
access boolean from here.
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java:
##########
@@ -1435,6 +1396,37 @@ public boolean isSyncSourceValidationEnabled() {
return isSyncSourceValidationEnabled;
}
+ public Map<String, String> getNameTransformationRules() {
Review Comment:
Methods `getNameTransformationRules()` and `getAllRegexPatternsConfig()`
don't seem to be used. Please review and remove.
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerDefaultRequestProcessor.java:
##########
@@ -157,6 +174,52 @@ public void enrich(RangerAccessRequest request) {
}
}
+ private String getTransformedUser(PolicyEngine policyEngine,
RangerAccessRequest request) {
+ String user = request.getUser();
+ String userNameCaseConversion =
policyEngine.getPluginContext().getUserNameCaseConversion();
+
+ if
(UgsyncCommonConstants.UGSYNC_LOWER_CASE_CONVERSION_VALUE.equalsIgnoreCase(userNameCaseConversion))
{
+ user = user.toLowerCase();
+ } else if
(UgsyncCommonConstants.UGSYNC_UPPER_CASE_CONVERSION_VALUE.equalsIgnoreCase(userNameCaseConversion))
{
+ user = user.toUpperCase();
+ }
+
+ Mapper userNameTransformInst =
policyEngine.getPluginContext().getUserNameTransformInst();
+ if (userNameTransformInst != null) {
+ String transformedUser = userNameTransformInst.transform(user);
+ LOG.debug("Original username = {}, Transformed username = {}",
request.getUser(), transformedUser);
+ return transformedUser;
+ }
+
+ return user;
+ }
+
+ private Set<String> getTransformedGroups(PolicyEngine policyEngine,
RangerAccessRequest request) {
+ Mapper groupNameTransformInst =
policyEngine.getPluginContext().getGroupNameTransformInst();
+ String caseConversion =
policyEngine.getPluginContext().getGroupNameCaseConversion();
+
+ if (groupNameTransformInst == null || request.getUserGroups() == null)
{
Review Comment:
When `groupNameTransformInst` is null, `return` at line 202 will result in
case-conversion to be not applied. Please review and update.
##########
ugsync-util/pom.xml:
##########
@@ -47,6 +47,28 @@
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
Review Comment:
Are libraries `log4j-api` and `log4j-over-slf4j` needed in ugsync-util
module? If not, please remove these dependencies.
--
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]
