mneethiraj commented on code in PR #1120:
URL: https://github.com/apache/ranger/pull/1120#discussion_r3708172771


##########
security-admin/src/main/java/org/apache/ranger/common/RangerConstants.java:
##########
@@ -46,6 +46,9 @@ public class RangerConstants extends RangerCommonEnums {
     public static final String RANGER_ADMIN_SUPER_USERS  = 
"ranger.admin.super.users";
     public static final String RANGER_ADMIN_SUPER_GROUPS = 
"ranger.admin.super.groups";
 
+    // login id used by the Ranger Admin health readiness endpoint
+    public static final String HEALTH_CHECK_USERNAME = "healthcheck";

Review Comment:
   I suggest reading the name of healthcheck user from configuration - perhaps 
in RangerBizUtil.
   
   ```
   public static final String HEALTHCHECK_USERNAME = 
RangerAdminConfig.getInstance().get("ranger.admin.healthcheck.username", 
"healthcheck");
   ```



##########
security-admin/src/main/java/org/apache/ranger/util/RangerServerHealthUtil.java:
##########
@@ -98,4 +136,44 @@ private Map<String, Object> getDbStatus(String dbVersion) {
 
         return ret;
     }
+
+    public List<String> getServiceDefNames() {
+        LOG.debug("==> RangerServerHealthUtil.getServiceDefNames()");
+
+        if (!bizUtil.isHealthCheckUser(resolveAuthenticatedLoginId())) {

Review Comment:
   Is this `if` necessary? I suggest removing this.



##########
security-admin/src/main/java/org/apache/ranger/util/RangerServerHealthUtil.java:
##########
@@ -98,4 +136,44 @@ private Map<String, Object> getDbStatus(String dbVersion) {
 
         return ret;
     }
+
+    public List<String> getServiceDefNames() {
+        LOG.debug("==> RangerServerHealthUtil.getServiceDefNames()");
+
+        if (!bizUtil.isHealthCheckUser(resolveAuthenticatedLoginId())) {
+            throw 
restErrorUtil.createRESTException(HttpServletResponse.SC_FORBIDDEN,
+                    "Only the healthcheck user may query service-def names via 
this path.", true);
+        }
+
+        List<String> ret = serviceDefService.getAllServiceDefNames();
+
+        LOG.debug("<== RangerServerHealthUtil.getServiceDefNames(): count={}", 
(ret == null ? 0 : ret.size()));
+
+        return ret;
+    }
+
+    /**
+     * Header-based auth sets {@link RangerAuthenticationToken} before Ranger
+     * {@link UserSessionBase} is available. Fall back to that token when the 
Ranger session
+     * has not been materialized yet (e.g. healthcheck user not in DB).
+     */
+    private String resolveAuthenticatedLoginId() {

Review Comment:
   To be consistent with rest of Ranger code base, I suggest having a single 
return statement in a method:
   
   ```
   if (loginId == null) {
     Object authentication = 
SecurityContextHolder.getContext().getAuthentication();
   
     if (authentication instanceof RangerAuthenticationToken) {
       RangerAuthenticationToken token = (RangerAuthenticationToken) 
authentication;
   
       if (token.getAuthType() == XXAuthSession.AUTH_TYPE_TRUSTED_PROXY) {
         loginId = token.getName();;
       }
     }
   }
   
   return loginId;
   ```



-- 
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]

Reply via email to