Copilot commented on code in PR #1302:
URL: https://github.com/apache/ranger/pull/1302#discussion_r4230104965
##########
agents-common/src/main/java/org/apache/ranger/authorization/utils/JsonUtils.java:
##########
@@ -47,6 +48,7 @@ public class JsonUtils {
private static final Logger LOG = LoggerFactory.getLogger(JsonUtils.class);
private static final TypeReference<Map<String, String>>
TYPE_MAP_STRING_STRING = new TypeReference<Map<String, String>>()
{};
+ private static final TypeReference<Map<String, Object>>
TYPE_MAP_STRING_OBJECT = new TypeReference<>() {};
Review Comment:
Using the diamond operator with an anonymous TypeReference (`new
TypeReference<>() {}`) can break compilation when building with Java 8 (and is
inconsistent with the explicit generic style used in the surrounding
constants). Prefer `new TypeReference<Map<String, Object>>() {}` for
compatibility and consistency.
##########
agents-common/src/test/java/org/apache/ranger/plugin/conditionevaluator/RangerRequestScriptEvaluatorTest.java:
##########
@@ -615,6 +653,14 @@ private static String tempFilePath(String tag, long nonce)
{
return new File(System.getProperty("java.io.tmpdir"), "ranger_test_" +
tag + "_" + nonce).getAbsolutePath();
}
+ RangerAccessRequest createRequest(String userName, String attributes) {
+ RangerAccessRequestImpl ret = (RangerAccessRequestImpl)
createRequest(userName, Collections.emptySet(), Collections.emptySet(),
Collections.emptyList());
+
+ RangerAccessRequestUtil.setContextAttrJson(ret.getContext(),
attributes);
+
+ return ret;
+ }
Review Comment:
The cast to `RangerAccessRequestImpl` makes this helper brittle (it will
fail with a `ClassCastException` if the underlying factory method ever returns
a different implementation). If the `RangerAccessRequest` interface exposes
`getContext()`, use it directly to avoid the cast; otherwise, consider changing
the underlying `createRequest(...)` helper to return `RangerAccessRequestImpl`
within the test.
##########
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java:
##########
@@ -60,11 +60,36 @@ public class RangerAccessRequestUtil {
public static final String KEY_CONTEXT_BATCH_EVAL_CONTEXT =
"BATCH_EVAL_CONTEXT";
public static final String KEY_CONTEXT_RESOURCE_ZONE_NAMES =
"RESOURCE_ZONE_NAMES";
public static final String KEY_CONTEXT_ACL_ENFORCER =
"_ACL_ENFORCER";
+ public static final String KEY_CONTEXT_ATTR_JSON =
"_ATTR_JSON";
private RangerAccessRequestUtil() {
// to avoid instantiation
}
+ public static void setContextAttrJson(Map<String, Object> context, String
attrJson) {
+ if (context != null) {
+ if (attrJson == null) {
+ context.remove(KEY_CONTEXT_ATTR_JSON);
+ } else {
+ context.put(KEY_CONTEXT_ATTR_JSON, attrJson);
+ }
+ }
+ }
+
+ public static String getContextAttrJson(Map<String, Object> context) {
+ Object ret = context != null ? context.get(KEY_CONTEXT_ATTR_JSON) :
null;
+
+ if (ret != null) {
+ if (ret instanceof String) {
+ return (String) ret;
+ } else {
+ LOG.error("getContextAttrJson(): failed to get {} from
context", KEY_CONTEXT_ATTR_JSON);
+ }
+ }
+
+ return null;
+ }
Review Comment:
If `_ATTR_JSON` is present but not a `String`, this will emit an error
without indicating the offending type/value and will keep the bad value in the
context, potentially causing repeated log noise. Consider including
`ret.getClass()` in the log and removing the invalid entry from the context to
self-heal.
##########
agents-common/src/main/java/org/apache/ranger/authorization/utils/JsonUtils.java:
##########
@@ -197,6 +199,20 @@ public static Map<String, String>
jsonToMapStringString(String jsonStr) {
return ret;
}
+ public static Map<String, Object> jsonToMapStringObject(String jsonStr) {
+ Map<String, Object> ret = null;
+
+ if (StringUtils.isNotEmpty(jsonStr)) {
+ try {
+ ret = getMapper().readValue(jsonStr, TYPE_MAP_STRING_OBJECT);
+ } catch (Exception excp) {
+ LOG.warn("jsonToMapStringObject() failed to convert json to
object: {}", jsonStr, excp);
+ }
Review Comment:
This warning log prints the full `jsonStr`, which may contain sensitive
context attributes (PII/financial data). Consider logging only metadata (e.g.,
length) and/or truncating/masking the value, or moving the raw payload to
debug-level under a feature flag.
--
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]
