You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by YiJi Gao <ky...@gmail.com> on 2023/03/12 08:53:32 UTC

Review Request 74346: RANGER-4122: Reorganize checkAdminAccess() and serveral authority check method.

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

Review request for ranger.


Repository: ranger


Description
-------

I have checked the implementation of checkAdminAccess() in @XAuditMgr, @UserMgr and @XUserMgr, it turns out that these methods are the same so I unify them into @RangerAuthorizationHelper. @RangerAuthoritizationHelper is in Request scope, which means Spring container would bind an instance to each HttpRequest. In this way, Ranger Admin could return Error as soo as possible when the UserSession or LoginId of current request is invalid.
Additionally, @checkAdminAccess in RangerBizUtil seems to be inconsistent with those above and I use isAdmin() instead.


Diffs
-----

  security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java 14a5c83e8 
  security-admin/src/main/java/org/apache/ranger/biz/RangerAuthorizationHelper.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java 07fa3dfd9 
  security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java addfd640d 
  security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java eaaa15a11 
  security-admin/src/main/java/org/apache/ranger/biz/XAuditMgr.java 75371f4b2 
  security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 2955bd513 
  security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java faad41c6c 
  security-admin/src/test/java/org/apache/ranger/biz/TestRangerAuthorizationHelper.java PRE-CREATION 
  security-admin/src/test/java/org/apache/ranger/biz/TestRangerBizUtil.java 22e290a66 
  security-admin/src/test/java/org/apache/ranger/biz/TestUserMgr.java b6c43133b 
  security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java 027c3b103 
  security-admin/src/test/java/org/apache/ranger/rest/TestXUserREST.java 74744e6cf 


Diff: https://reviews.apache.org/r/74346/diff/1/


Testing
-------

Tested Ranger build using below command,
mvn clean compile package -DskipTests -Psecurity-admin-react


Successfully setup Ranger Admin UI with updated react 18.2.0 version.


File Attachments
----------------

0001-RANGER-4122-Reorganize-checkAdminAccess-and-serveral.patch
  https://reviews.apache.org/media/uploaded/files/2023/03/12/97095ba9-00e3-4e95-be7c-9c0ee9133249__0001-RANGER-4122-Reorganize-checkAdminAccess-and-serveral.patch


Thanks,

YiJi Gao


Re: Review Request 74346: RANGER-4122: Reorganize checkAdminAccess() and serveral authority check method.

Posted by YiJi Gao <ky...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74346/
-----------------------------------------------------------

(Updated 三月 19, 2023, 3:27 p.m.)


Review request for ranger, madhan, Madhan Neethiraj, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.


Summary (updated)
-----------------

RANGER-4122: Reorganize checkAdminAccess() and serveral authority check method.


Bugs: RANGER-4122
    https://issues.apache.org/jira/browse/RANGER-4122


Repository: ranger


Description
-------

I have checked the implementation of checkAdminAccess() in @XAuditMgr, @UserMgr and @XUserMgr, it turns out that these methods are the same so I unify them into @RangerAuthorizationHelper. @RangerAuthoritizationHelper is in Request scope, which means Spring container would bind an instance to each HttpRequest. In this way, Ranger Admin could return Error as soo as possible when the UserSession or LoginId of current request is invalid.
Additionally, @checkAdminAccess in RangerBizUtil seems to be inconsistent with those above and I use isAdmin() instead.


Diffs (updated)
-----

  security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java 4581112fe 
  security-admin/src/main/java/org/apache/ranger/biz/RangerAuthorizationHelper.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java f9294c1e1 
  security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java 421b2312d 
  security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java d5393603e 
  security-admin/src/main/java/org/apache/ranger/biz/XAuditMgr.java 75371f4b2 
  security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 49a74cd1e 
  security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java faad41c6c 
  security-admin/src/test/java/org/apache/ranger/biz/TestRangerAuthorizationHelper.java PRE-CREATION 
  security-admin/src/test/java/org/apache/ranger/biz/TestRangerBizUtil.java 22e290a66 
  security-admin/src/test/java/org/apache/ranger/biz/TestUserMgr.java b6c43133b 
  security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java 528f4e511 
  security-admin/src/test/java/org/apache/ranger/rest/TestXUserREST.java 74744e6cf 


Diff: https://reviews.apache.org/r/74346/diff/4/

Changes: https://reviews.apache.org/r/74346/diff/3-4/


Testing
-------

Tested Ranger build using below command,
mvn clean compile package install -Dmaven.wagon.http.ssl.insecure=true -Dmaven.wagon.http.ssl.allowall=true -DskipJSTest
1. Build Successfully, all unit test passed and start ranger-admin successfully.
2. Log in as admin, try to create a user via "/service/users" and get allowed.
3. Log in as user, try to create a user via "/service/users" and get blocked.


File Attachments (updated)
----------------

0001-RANGER-4122-Reorganize-checkAdminAccess-and-serveral.patch
  https://reviews.apache.org/media/uploaded/files/2023/03/19/471b6eb6-c725-487e-ad5e-419717099d76__0001-RANGER-4122-Reorganize-checkAdminAccess-and-serveral.patch


Thanks,

YiJi Gao


Re: Review Request 74346: [WIP]RANGER-4122: Reorganize checkAdminAccess() and serveral authority check method.

Posted by YiJi Gao <ky...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74346/
-----------------------------------------------------------

(Updated 三月 19, 2023, 3:08 p.m.)


Review request for ranger, Kirby Zhou, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.


Bugs: RANGER-4122
    https://issues.apache.org/jira/browse/RANGER-4122


Repository: ranger


Description
-------

I have checked the implementation of checkAdminAccess() in @XAuditMgr, @UserMgr and @XUserMgr, it turns out that these methods are the same so I unify them into @RangerAuthorizationHelper. @RangerAuthoritizationHelper is in Request scope, which means Spring container would bind an instance to each HttpRequest. In this way, Ranger Admin could return Error as soo as possible when the UserSession or LoginId of current request is invalid.
Additionally, @checkAdminAccess in RangerBizUtil seems to be inconsistent with those above and I use isAdmin() instead.


Diffs (updated)
-----

  security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java 4581112fe 
  security-admin/src/main/java/org/apache/ranger/biz/RangerAuthorizationHelper.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java f9294c1e1 
  security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java 421b2312d 
  security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java d5393603e 
  security-admin/src/main/java/org/apache/ranger/biz/XAuditMgr.java 75371f4b2 
  security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 49a74cd1e 
  security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java faad41c6c 
  security-admin/src/test/java/org/apache/ranger/biz/TestRangerAuthorizationHelper.java PRE-CREATION 
  security-admin/src/test/java/org/apache/ranger/biz/TestRangerBizUtil.java 22e290a66 
  security-admin/src/test/java/org/apache/ranger/biz/TestUserMgr.java b6c43133b 
  security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java 528f4e511 
  security-admin/src/test/java/org/apache/ranger/rest/TestXUserREST.java 74744e6cf 


Diff: https://reviews.apache.org/r/74346/diff/3/

Changes: https://reviews.apache.org/r/74346/diff/2-3/


Testing (updated)
-------

Tested Ranger build using below command,
mvn clean compile package install -Dmaven.wagon.http.ssl.insecure=true -Dmaven.wagon.http.ssl.allowall=true -DskipJSTest
1. Build Successfully, all unit test passed and start ranger-admin successfully.
2. Log in as admin, try to create a user via "/service/users" and get allowed.
3. Log in as user, try to create a user via "/service/users" and get blocked.


File Attachments
----------------

0001-RANGER-4122-Reorganize-checkAdminAccess-and-serveral.patch
  https://reviews.apache.org/media/uploaded/files/2023/03/12/97095ba9-00e3-4e95-be7c-9c0ee9133249__0001-RANGER-4122-Reorganize-checkAdminAccess-and-serveral.patch


Thanks,

YiJi Gao


Re: Review Request 74346: RANGER-4122: Reorganize checkAdminAccess() and serveral authority check method.

Posted by YiJi Gao <ky...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74346/
-----------------------------------------------------------

(Updated 三月 13, 2023, 5:49 p.m.)


Review request for ranger, Kirby Zhou, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.


Bugs: RANGER-4122
    https://issues.apache.org/jira/browse/RANGER-4122


Repository: ranger


Description
-------

I have checked the implementation of checkAdminAccess() in @XAuditMgr, @UserMgr and @XUserMgr, it turns out that these methods are the same so I unify them into @RangerAuthorizationHelper. @RangerAuthoritizationHelper is in Request scope, which means Spring container would bind an instance to each HttpRequest. In this way, Ranger Admin could return Error as soo as possible when the UserSession or LoginId of current request is invalid.
Additionally, @checkAdminAccess in RangerBizUtil seems to be inconsistent with those above and I use isAdmin() instead.


Diffs (updated)
-----

  security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java 4581112fe 
  security-admin/src/main/java/org/apache/ranger/biz/RangerAuthorizationHelper.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java f9294c1e1 
  security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java 421b2312d 
  security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java d5393603e 
  security-admin/src/main/java/org/apache/ranger/biz/XAuditMgr.java 75371f4b2 
  security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 49a74cd1e 
  security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java faad41c6c 
  security-admin/src/test/java/org/apache/ranger/biz/TestRangerAuthorizationHelper.java PRE-CREATION 
  security-admin/src/test/java/org/apache/ranger/biz/TestRangerBizUtil.java 22e290a66 
  security-admin/src/test/java/org/apache/ranger/biz/TestUserMgr.java b6c43133b 
  security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java 528f4e511 
  security-admin/src/test/java/org/apache/ranger/rest/TestXUserREST.java 74744e6cf 


Diff: https://reviews.apache.org/r/74346/diff/2/

Changes: https://reviews.apache.org/r/74346/diff/1-2/


Testing
-------

Tested Ranger build using below command,
mvn clean compile package -DskipTests -Psecurity-admin-react


Successfully setup Ranger Admin UI with updated react 18.2.0 version.


File Attachments
----------------

0001-RANGER-4122-Reorganize-checkAdminAccess-and-serveral.patch
  https://reviews.apache.org/media/uploaded/files/2023/03/12/97095ba9-00e3-4e95-be7c-9c0ee9133249__0001-RANGER-4122-Reorganize-checkAdminAccess-and-serveral.patch


Thanks,

YiJi Gao