ranger-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Madhan Neethiraj <mad...@apache.org>
Subject Re: Review Request 68096: RANGER-2168: Add service admin user through service config
Date Wed, 01 Aug 2018 05:35:40 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68096/#review206724
-----------------------------------------------------------




security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
Lines 4796 (patched)
<https://reviews.apache.org/r/68096/#comment289810>

    - instead of following 2 DB calls:
      - 1) retrieve XXService from serviceName
      - 2) retrieve XXServiceConfigMap from serviceId and configName
     consider making one DB call that takes serviceName and configName:
       daoMgr.getXXServiceConfigMap().findByServiceNameAndConfigKey(serviceName, SERVICE_ADMIN_USERS);
     the query should internally join XXService and XXServiceConfigMap tables.



security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
Lines 4802 (patched)
<https://reviews.apache.org/r/68096/#comment289811>

    Consider avoiding creation of userList.
    
    public boolean isServiceAdminUser(String serviceName, String userName) {
      boolean            ret              = false;
      XXServiceConfigMap cfgSvcAdminUsers = daoMgr.getXXServiceConfigMap().findByServiceNameAndConfigKey(serviceName,
SERVICE_ADMIN_USERS);
      String             svcAdminUsers    = cfgSvcAdminUsers != null ? cfgSvcAdminUsers.getConfigValue()
: null;
    
      if (svcAdminUsers != null) {
        for (String svcAdminUser : svcAdminUsers.split(",")) {
          if (userName.equals(svcAdminUser)) {
            ret = true;
    
            break;
          }
        }
      }
    
      return ret;
    }



security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
Line 3033 (original), 3033 (patched)
<https://reviews.apache.org/r/68096/#comment289812>

    Please avoid white-space-only changes. This file has bunch of such instance - like #3030
- #3034, #3040 - #3041, #3055, #3061.



security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
Lines 3081 (patched)
<https://reviews.apache.org/r/68096/#comment289813>

    Instead of separate 'if' block here, consider adding 'isServiceAdminUser' to line #3065
- like:
      if (isAdmin || isServiceAdminUser) {



security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
Line 3107 (original), 3110 (patched)
<https://reviews.apache.org/r/68096/#comment289814>

    Consider the following alternate:
      boolean isAdmin    = bizUtil.isAdmin();
      boolean isKeyAdmin = bizUtil.isKeyAdmin();
      String userName    = bizUtil.getCurrentUserLoginId();
      boolean isSvcAdmin = isAdmin || svcStore.isServiceAdminUser(policy.getService(), userName);
      
      if(!isAdmin && !isKeyAdmin && !isSvcAdmin) {
      
    
    Similar update in ensureAdminAndAuditAccess() as well.


- Madhan Neethiraj


On July 29, 2018, 12:07 p.m., Pradeep Agrawal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68096/
> -----------------------------------------------------------
> 
> (Updated July 29, 2018, 12:07 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, deepak sharma, Gautam Borad, Abhay Kulkarni,
Madhan Neethiraj, Mehul Parikh, suja s, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2168
>     https://issues.apache.org/jira/browse/RANGER-2168
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> **Problem Statement:** Currently only user with admin role or a delegated admin user
can create the policy. We can possibly have a service admin user who can be allowed to create
policy. Such users can be configured in the service config itself and can be removed by admin
anytime.
> 
> **Proposed Solution:** 
> Allow admin/keyadmin role users to add a custom service config property 'service.admin.users'
through service page. 
> Users provided in 'service.admin.users' can be internal or external and can have any
role.
> Users provided in 'service.admin.users' should able to create/update/delete/view policies
of that ranger service.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 8efc950ce 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java e4449df2e 
> 
> 
> Diff: https://reviews.apache.org/r/68096/diff/1/
> 
> 
> Testing
> -------
> 
> **Steps Performed:**
> Created an internal user testuser in the Ranger admin.
> Added a hive service 'hivedev' in Ranger.
> 
> **Action-1**: Logged in from 'testuser' and tried to create a policy 'testpolicy' in
'hivedev' service.
> **Expected Behaviour**: Policy creation should fail.
> **Actual Behaviour**: Policy creation failed.
> 
> **Action-2.1**: Logged in from ranger admin user and added a custom property 'service.admin.users'
in 'hivedev' service and provided value 'testuser' in the given text box. Saved the 'hivedev'
service.
> **Action-2.2**: Logged in from 'testuser' and tried to create a policy 'testpolicy' in
'hivedev' service.
> **Expected Behaviour**: Policy creation should successful.
> **Actual Behaviour**: Policy creation finished successfully.
> 
> Tested Policy updation and deletion which also executed successfully.
> 
> **Action-3.1**: Logged in from ranger admin user and removed custom property 'service.admin.users'
from 'hivedev' service. Saved the 'hivedev' service.
> **Action-3.2**: Logged in from 'testuser' and tried to create a policy 'testpolicy1'
in 'hivedev' service.
> **Expected Behaviour**: Policy creation should fail.
> **Actual Behaviour**: Policy creation failed.
> 
> 
> Thanks,
> 
> Pradeep Agrawal
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message