You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Gautam Borad <gb...@gmail.com> on 2015/10/08 15:05:25 UTC

Review Request 39122: RANGER-688 : Handle scenario where ids of XUser and XPortalUser are not in sync

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

Review request for ranger, Alok Lal, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.


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


Repository: ranger


Description
-------

Patch takes care of managing the users permissions even if ids (XUser and XPortalUser ids) are not in sync


Diffs
-----

  security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 5f43bc020df6842cc04708113fa46c4b370ac854 
  security-admin/src/main/java/org/apache/ranger/common/UserSessionBase.java 59e55f3e1db02f1fdd2d03dd22183929b1173027 
  security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java d3467f86aefccca9b0387e382628b95e4466eecd 
  security-admin/src/main/java/org/apache/ranger/db/XXUserDao.java 088759469608ab45e3ad3341ad2bf6558a30f5d9 
  security-admin/src/main/java/org/apache/ranger/db/XXUserPermissionDao.java e10dc14fdcb58265467aa02d0a9b9b2aea1040ad 
  security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java f0aa938dd918124e050ee54a3dcc64e86e3f5236 
  security-admin/src/main/java/org/apache/ranger/service/XUserPermissionService.java 3ff9c8d0ac070032089bcbc9f5c3c33c401b30b3 
  security-admin/src/main/java/org/apache/ranger/service/XUserPermissionServiceBase.java 59c082d2760acc8bd296b29497ab763cbb97c407 
  security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0370e9abe60582152f6d71ac8e0ed8312e4f2e7f 

Diff: https://reviews.apache.org/r/39122/diff/


Testing
-------

Tested by creating a XPOrtalUser user using curl command and then for a new user, assigned permissions to check if permissions are not messed up.


Thanks,

Gautam Borad


Re: Review Request 39122: RANGER-688 : Handle scenario where ids of XUser and XPortalUser are not in sync

Posted by Gautam Borad <gb...@gmail.com>.

> On Oct. 8, 2015, 4:49 p.m., Madhan Neethiraj wrote:
> > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java, line 270
> > <https://reviews.apache.org/r/39122/diff/1/?file=1092890#file1092890line270>
> >
> >     In this case, vxUserPermission.userId will be null. Will this userpermission object be useful? Would this cause failure in persisting in the db?

Here we are passing username as x_user is null so in service class, we will find x_portal_user using username and in database create permission for that user. 

Though such permission will not cause an issue and also not make sense to have in database, that's why it would be better we dont create such permissions, updating code to ignore if x_user does not exist.


> On Oct. 8, 2015, 4:49 p.m., Madhan Neethiraj wrote:
> > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java, line 272
> > <https://reviews.apache.org/r/39122/diff/1/?file=1092890#file1092890line272>
> >
> >     XXUserPermission.userId now has XXUser.id, instead of XXPortalUser.id. Please review all uses of XXUserPermission.userId to make sure that XXUser.id is used to store/compare. For example in  XXModuleDefDao.findAccessibleModulesByUserId().
> >     
> >     Also, what is the plan to update existing entries in XXUserPermission table?

XXUserPermission.userId has XXPortalUser.id only. XXUser.id will be used for UI only. 
For example when server returns object to UI at that time UI will get userId from XXUser table; and when UI will send object to server it will be considered as xUserId and will be mapped to xPortalUser.id and then persistence will be done.


Also, what is the plan to update existing entries in XXUserPermission table?
>>> As per discussion with Vel, we have created JIRA but right now it is not set as priority task.


> On Oct. 8, 2015, 4:49 p.m., Madhan Neethiraj wrote:
> > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java, line 283
> > <https://reviews.apache.org/r/39122/diff/1/?file=1092890#file1092890line283>
> >
> >     What does 'isCreate' flag mean here? It seems the code updates existing object if this flag is set..

Here we check for existing object in database, if it does not exist then we'll create new. 

If object exist in db and isCreate flag is false then we ignore. 

However, if object exists in db and isCreate flag is set, then we update permission object to allow that user.

It might be possible that userPermission object is there in db but `isAllowed` is set to `No` so in this case if isCreate flag is set then we are kind of assigning/creating permission to that user.


- Gautam


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


On Oct. 9, 2015, 9:15 a.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39122/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 9:15 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-688
>     https://issues.apache.org/jira/browse/RANGER-688
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> *Current Implementation:*
> 
> x_user_module_perm.userId refers to x_portal_user.id but while assigning/revoking permissions using permission model, UI sends userId of x_user table.
> So this is a bug in current implementation because UI sends x_user.id and server assumes that it's x_portal_user.id and will create permission.
> When IDs of x_user and x_portal_user will not be in sync at that time this may cause a serious issue.
> 
> *Fix provided in patch:*
> 
> UI will always have x_user/s but on server side x_portal_user/s required. So now we can't expect UI to send x_portal_user.id in userPermission object so in patch what we have done is:
> Let UI send x_user.id in userPermission object but on server side it will be mapped to x_portal_user.id and same when UI is reading permissions, let database return x_portal_user.id that will be mapped to x_user.id. 
> Due to this UI remains as it is and also, we don't need to change table structure of x_user_module_perm.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 5f43bc0 
>   security-admin/src/main/java/org/apache/ranger/common/UserSessionBase.java 59e55f3 
>   security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java d3467f8 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserDao.java 0887594 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserPermissionDao.java e10dc14 
>   security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java f0aa938 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionService.java 3ff9c8d 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionServiceBase.java 59c082d 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0370e9a 
> 
> Diff: https://reviews.apache.org/r/39122/diff/
> 
> 
> Testing
> -------
> 
> Tested by creating a XPOrtalUser user using curl command and then for a new user, assigned permissions to check if permissions are not messed up.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>


Re: Review Request 39122: RANGER-688 : Handle scenario where ids of XUser and XPortalUser are not in sync

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39122/#review101928
-----------------------------------------------------------



security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java (line 270)
<https://reviews.apache.org/r/39122/#comment159431>

    In this case, vxUserPermission.userId will be null. Will this userpermission object be useful? Would this cause failure in persisting in the db?



security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java (line 272)
<https://reviews.apache.org/r/39122/#comment159434>

    XXUserPermission.userId now has XXUser.id, instead of XXPortalUser.id. Please review all uses of XXUserPermission.userId to make sure that XXUser.id is used to store/compare. For example in  XXModuleDefDao.findAccessibleModulesByUserId().
    
    Also, what is the plan to update existing entries in XXUserPermission table?



security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java (line 283)
<https://reviews.apache.org/r/39122/#comment159432>

    What does 'isCreate' flag mean here? It seems the code updates existing object if this flag is set..


- Madhan Neethiraj


On Oct. 8, 2015, 1:05 p.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39122/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2015, 1:05 p.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-688
>     https://issues.apache.org/jira/browse/RANGER-688
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Patch takes care of managing the users permissions even if ids (XUser and XPortalUser ids) are not in sync
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 5f43bc020df6842cc04708113fa46c4b370ac854 
>   security-admin/src/main/java/org/apache/ranger/common/UserSessionBase.java 59e55f3e1db02f1fdd2d03dd22183929b1173027 
>   security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java d3467f86aefccca9b0387e382628b95e4466eecd 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserDao.java 088759469608ab45e3ad3341ad2bf6558a30f5d9 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserPermissionDao.java e10dc14fdcb58265467aa02d0a9b9b2aea1040ad 
>   security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java f0aa938dd918124e050ee54a3dcc64e86e3f5236 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionService.java 3ff9c8d0ac070032089bcbc9f5c3c33c401b30b3 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionServiceBase.java 59c082d2760acc8bd296b29497ab763cbb97c407 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0370e9abe60582152f6d71ac8e0ed8312e4f2e7f 
> 
> Diff: https://reviews.apache.org/r/39122/diff/
> 
> 
> Testing
> -------
> 
> Tested by creating a XPOrtalUser user using curl command and then for a new user, assigned permissions to check if permissions are not messed up.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>


Re: Review Request 39122: RANGER-688 : Handle scenario where ids of XUser and XPortalUser are not in sync

Posted by Madhan Neethiraj <ma...@apache.org>.

> On Oct. 9, 2015, 10:24 p.m., Madhan Neethiraj wrote:
> > Followig two places seem to be inconsistent in assigning to XXUserPermission.userId: the first one assigns XXUser.id and the second one assigns XXPortalUser.id. Please review.
> > 
> > XUserMgr.java: #272
> >   vXUserPermission.setUserId(xUser.getId());
> > 
> > XUserPermissionServiceBase.java: #53
> >   vObj.setUserId(portalUser.getId());
> 
> Gautam Borad wrote:
>     XUserMgr.java: #272
>       vXUserPermission.setUserId(xUser.getId());
>     >>> From here object is getting created using createResource method of service class, so as per the implementation while creating object, client/UI/internal code will send xUserId and at service layer it will be mapped to xPortalUserId.
>     So this seems correct.
>     
>     XUserPermissionServiceBase.java: #53
>       vObj.setUserId(portalUser.getId());
>     >>> This is the place where the mapping from xUserId to xPortalUserId happens. As I mentioned above while creating/updating, UI/client will send xUserId which will be mapped here to xPortalUserId.

VXUserPermission.userId is assigned two different values: XUser.Id in XUserMgr.java: #273 and XPortalUserId.id in XUserPermissionServiceBase.java: #53. Hence this concern.

It looks like the value assigned in XUserPermissionServiceBase.mapViewToEntityBean() (line #53) may not be necessary. This method is to populate the entity object, XXUserPermission, from the contents of the view object, VXUserPermission, - right? Not assigning  "VXUserPermission.userId" with "portalUser.id" will help reduce the confusion - and also make sure that subsequent references to VXUserPermission.userId, like in validateXUserPermForCreate()/validateXUserPermForUpdate() will expect "user.Id". If the assignment (line #53) is for optimization (to avoid query to covert userId ==> portaUserId), I would suggest going with one of the following:
 - adding a field "VXUserPermission.portalUserId" and have it populated from the userId field at the point of entry
 - use a query that finds XXUserPermission from the given user.id and moduleId (instead of portalUser.id and moduleId)


- Madhan


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


On Oct. 9, 2015, 9:15 a.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39122/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 9:15 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-688
>     https://issues.apache.org/jira/browse/RANGER-688
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> *Current Implementation:*
> 
> x_user_module_perm.userId refers to x_portal_user.id but while assigning/revoking permissions using permission model, UI sends userId of x_user table.
> So this is a bug in current implementation because UI sends x_user.id and server assumes that it's x_portal_user.id and will create permission.
> When IDs of x_user and x_portal_user will not be in sync at that time this may cause a serious issue.
> 
> *Fix provided in patch:*
> 
> UI will always have x_user/s but on server side x_portal_user/s required. So now we can't expect UI to send x_portal_user.id in userPermission object so in patch what we have done is:
> Let UI send x_user.id in userPermission object but on server side it will be mapped to x_portal_user.id and same when UI is reading permissions, let database return x_portal_user.id that will be mapped to x_user.id. 
> Due to this UI remains as it is and also, we don't need to change table structure of x_user_module_perm.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 5f43bc0 
>   security-admin/src/main/java/org/apache/ranger/common/UserSessionBase.java 59e55f3 
>   security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java d3467f8 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserDao.java 0887594 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserPermissionDao.java e10dc14 
>   security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java f0aa938 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionService.java 3ff9c8d 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionServiceBase.java 59c082d 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0370e9a 
> 
> Diff: https://reviews.apache.org/r/39122/diff/
> 
> 
> Testing
> -------
> 
> Tested by creating a XPOrtalUser user using curl command and then for a new user, assigned permissions to check if permissions are not messed up.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>


Re: Review Request 39122: RANGER-688 : Handle scenario where ids of XUser and XPortalUser are not in sync

Posted by Gautam Borad <gb...@gmail.com>.

> On Oct. 9, 2015, 10:24 p.m., Madhan Neethiraj wrote:
> > Followig two places seem to be inconsistent in assigning to XXUserPermission.userId: the first one assigns XXUser.id and the second one assigns XXPortalUser.id. Please review.
> > 
> > XUserMgr.java: #272
> >   vXUserPermission.setUserId(xUser.getId());
> > 
> > XUserPermissionServiceBase.java: #53
> >   vObj.setUserId(portalUser.getId());

XUserMgr.java: #272
  vXUserPermission.setUserId(xUser.getId());
>>> From here object is getting created using createResource method of service class, so as per the implementation while creating object, client/UI/internal code will send xUserId and at service layer it will be mapped to xPortalUserId.
So this seems correct.

XUserPermissionServiceBase.java: #53
  vObj.setUserId(portalUser.getId());
>>> This is the place where the mapping from xUserId to xPortalUserId happens. As I mentioned above while creating/updating, UI/client will send xUserId which will be mapped here to xPortalUserId.


- Gautam


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


On Oct. 9, 2015, 9:15 a.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39122/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 9:15 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-688
>     https://issues.apache.org/jira/browse/RANGER-688
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> *Current Implementation:*
> 
> x_user_module_perm.userId refers to x_portal_user.id but while assigning/revoking permissions using permission model, UI sends userId of x_user table.
> So this is a bug in current implementation because UI sends x_user.id and server assumes that it's x_portal_user.id and will create permission.
> When IDs of x_user and x_portal_user will not be in sync at that time this may cause a serious issue.
> 
> *Fix provided in patch:*
> 
> UI will always have x_user/s but on server side x_portal_user/s required. So now we can't expect UI to send x_portal_user.id in userPermission object so in patch what we have done is:
> Let UI send x_user.id in userPermission object but on server side it will be mapped to x_portal_user.id and same when UI is reading permissions, let database return x_portal_user.id that will be mapped to x_user.id. 
> Due to this UI remains as it is and also, we don't need to change table structure of x_user_module_perm.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 5f43bc0 
>   security-admin/src/main/java/org/apache/ranger/common/UserSessionBase.java 59e55f3 
>   security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java d3467f8 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserDao.java 0887594 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserPermissionDao.java e10dc14 
>   security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java f0aa938 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionService.java 3ff9c8d 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionServiceBase.java 59c082d 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0370e9a 
> 
> Diff: https://reviews.apache.org/r/39122/diff/
> 
> 
> Testing
> -------
> 
> Tested by creating a XPOrtalUser user using curl command and then for a new user, assigned permissions to check if permissions are not messed up.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>


Re: Review Request 39122: RANGER-688 : Handle scenario where ids of XUser and XPortalUser are not in sync

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39122/#review102121
-----------------------------------------------------------


Followig two places seem to be inconsistent in assigning to XXUserPermission.userId: the first one assigns XXUser.id and the second one assigns XXPortalUser.id. Please review.

XUserMgr.java: #272
  vXUserPermission.setUserId(xUser.getId());

XUserPermissionServiceBase.java: #53
  vObj.setUserId(portalUser.getId());

- Madhan Neethiraj


On Oct. 9, 2015, 9:15 a.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39122/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 9:15 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-688
>     https://issues.apache.org/jira/browse/RANGER-688
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> *Current Implementation:*
> 
> x_user_module_perm.userId refers to x_portal_user.id but while assigning/revoking permissions using permission model, UI sends userId of x_user table.
> So this is a bug in current implementation because UI sends x_user.id and server assumes that it's x_portal_user.id and will create permission.
> When IDs of x_user and x_portal_user will not be in sync at that time this may cause a serious issue.
> 
> *Fix provided in patch:*
> 
> UI will always have x_user/s but on server side x_portal_user/s required. So now we can't expect UI to send x_portal_user.id in userPermission object so in patch what we have done is:
> Let UI send x_user.id in userPermission object but on server side it will be mapped to x_portal_user.id and same when UI is reading permissions, let database return x_portal_user.id that will be mapped to x_user.id. 
> Due to this UI remains as it is and also, we don't need to change table structure of x_user_module_perm.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 5f43bc0 
>   security-admin/src/main/java/org/apache/ranger/common/UserSessionBase.java 59e55f3 
>   security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java d3467f8 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserDao.java 0887594 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserPermissionDao.java e10dc14 
>   security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java f0aa938 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionService.java 3ff9c8d 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionServiceBase.java 59c082d 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0370e9a 
> 
> Diff: https://reviews.apache.org/r/39122/diff/
> 
> 
> Testing
> -------
> 
> Tested by creating a XPOrtalUser user using curl command and then for a new user, assigned permissions to check if permissions are not messed up.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>


Re: Review Request 39122: RANGER-688 : Handle scenario where ids of XUser and XPortalUser are not in sync

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39122/#review102634
-----------------------------------------------------------

Ship it!


Ship It!

- Madhan Neethiraj


On Oct. 14, 2015, 10:27 a.m., Gautam Borad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39122/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2015, 10:27 a.m.)
> 
> 
> Review request for ranger, Alok Lal, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-688
>     https://issues.apache.org/jira/browse/RANGER-688
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> *Current Implementation:*
> 
> x_user_module_perm.userId refers to x_portal_user.id but while assigning/revoking permissions using permission model, UI sends userId of x_user table.
> So this is a bug in current implementation because UI sends x_user.id and server assumes that it's x_portal_user.id and will create permission.
> When IDs of x_user and x_portal_user will not be in sync at that time this may cause a serious issue.
> 
> *Fix provided in patch:*
> 
> UI will always have x_user/s but on server side x_portal_user/s required. So now we can't expect UI to send x_portal_user.id in userPermission object so in patch what we have done is:
> Let UI send x_user.id in userPermission object but on server side it will be mapped to x_portal_user.id and same when UI is reading permissions, let database return x_portal_user.id that will be mapped to x_user.id. 
> Due to this UI remains as it is and also, we don't need to change table structure of x_user_module_perm.
> 
> 
> Diffs
> -----
> 
>   security-admin/scripts/setup.sh 9710706ed5147d572dd988eaf7db5e757a8338f2 
>   security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java b86087773c97770a96ee62fa817b10615be08bab 
>   security-admin/src/main/java/org/apache/ranger/common/UserSessionBase.java 59e55f3e1db02f1fdd2d03dd22183929b1173027 
>   security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java d3467f86aefccca9b0387e382628b95e4466eecd 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserDao.java 088759469608ab45e3ad3341ad2bf6558a30f5d9 
>   security-admin/src/main/java/org/apache/ranger/db/XXUserPermissionDao.java e10dc14fdcb58265467aa02d0a9b9b2aea1040ad 
>   security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java f0aa938dd918124e050ee54a3dcc64e86e3f5236 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionService.java 3ff9c8d0ac070032089bcbc9f5c3c33c401b30b3 
>   security-admin/src/main/java/org/apache/ranger/service/XUserPermissionServiceBase.java 59c082d2760acc8bd296b29497ab763cbb97c407 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0370e9abe60582152f6d71ac8e0ed8312e4f2e7f 
> 
> Diff: https://reviews.apache.org/r/39122/diff/
> 
> 
> Testing
> -------
> 
> Tested by creating a XPOrtalUser user using curl command and then for a new user, assigned permissions to check if permissions are not messed up.
> 
> 
> Thanks,
> 
> Gautam Borad
> 
>


Re: Review Request 39122: RANGER-688 : Handle scenario where ids of XUser and XPortalUser are not in sync

Posted by Gautam Borad <gb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39122/
-----------------------------------------------------------

(Updated Oct. 14, 2015, 10:27 a.m.)


Review request for ranger, Alok Lal, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.


Changes
-------

Updated patch as per Madhan's recommendation


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


Repository: ranger


Description
-------

*Current Implementation:*

x_user_module_perm.userId refers to x_portal_user.id but while assigning/revoking permissions using permission model, UI sends userId of x_user table.
So this is a bug in current implementation because UI sends x_user.id and server assumes that it's x_portal_user.id and will create permission.
When IDs of x_user and x_portal_user will not be in sync at that time this may cause a serious issue.

*Fix provided in patch:*

UI will always have x_user/s but on server side x_portal_user/s required. So now we can't expect UI to send x_portal_user.id in userPermission object so in patch what we have done is:
Let UI send x_user.id in userPermission object but on server side it will be mapped to x_portal_user.id and same when UI is reading permissions, let database return x_portal_user.id that will be mapped to x_user.id. 
Due to this UI remains as it is and also, we don't need to change table structure of x_user_module_perm.


Diffs (updated)
-----

  security-admin/scripts/setup.sh 9710706ed5147d572dd988eaf7db5e757a8338f2 
  security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java b86087773c97770a96ee62fa817b10615be08bab 
  security-admin/src/main/java/org/apache/ranger/common/UserSessionBase.java 59e55f3e1db02f1fdd2d03dd22183929b1173027 
  security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java d3467f86aefccca9b0387e382628b95e4466eecd 
  security-admin/src/main/java/org/apache/ranger/db/XXUserDao.java 088759469608ab45e3ad3341ad2bf6558a30f5d9 
  security-admin/src/main/java/org/apache/ranger/db/XXUserPermissionDao.java e10dc14fdcb58265467aa02d0a9b9b2aea1040ad 
  security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java f0aa938dd918124e050ee54a3dcc64e86e3f5236 
  security-admin/src/main/java/org/apache/ranger/service/XUserPermissionService.java 3ff9c8d0ac070032089bcbc9f5c3c33c401b30b3 
  security-admin/src/main/java/org/apache/ranger/service/XUserPermissionServiceBase.java 59c082d2760acc8bd296b29497ab763cbb97c407 
  security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0370e9abe60582152f6d71ac8e0ed8312e4f2e7f 

Diff: https://reviews.apache.org/r/39122/diff/


Testing
-------

Tested by creating a XPOrtalUser user using curl command and then for a new user, assigned permissions to check if permissions are not messed up.


Thanks,

Gautam Borad


Re: Review Request 39122: RANGER-688 : Handle scenario where ids of XUser and XPortalUser are not in sync

Posted by Gautam Borad <gb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39122/
-----------------------------------------------------------

(Updated Oct. 9, 2015, 9:15 a.m.)


Review request for ranger, Alok Lal, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.


Changes
-------

Updated patch to cater to Madhan's comment.


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


Repository: ranger


Description (updated)
-------

*Current Implementation:*

x_user_module_perm.userId refers to x_portal_user.id but while assigning/revoking permissions using permission model, UI sends userId of x_user table.
So this is a bug in current implementation because UI sends x_user.id and server assumes that it's x_portal_user.id and will create permission.
When IDs of x_user and x_portal_user will not be in sync at that time this may cause a serious issue.

*Fix provided in patch:*

UI will always have x_user/s but on server side x_portal_user/s required. So now we can't expect UI to send x_portal_user.id in userPermission object so in patch what we have done is:
Let UI send x_user.id in userPermission object but on server side it will be mapped to x_portal_user.id and same when UI is reading permissions, let database return x_portal_user.id that will be mapped to x_user.id. 
Due to this UI remains as it is and also, we don't need to change table structure of x_user_module_perm.


Diffs (updated)
-----

  security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 5f43bc0 
  security-admin/src/main/java/org/apache/ranger/common/UserSessionBase.java 59e55f3 
  security-admin/src/main/java/org/apache/ranger/db/XXPortalUserDao.java d3467f8 
  security-admin/src/main/java/org/apache/ranger/db/XXUserDao.java 0887594 
  security-admin/src/main/java/org/apache/ranger/db/XXUserPermissionDao.java e10dc14 
  security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java f0aa938 
  security-admin/src/main/java/org/apache/ranger/service/XUserPermissionService.java 3ff9c8d 
  security-admin/src/main/java/org/apache/ranger/service/XUserPermissionServiceBase.java 59c082d 
  security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0370e9a 

Diff: https://reviews.apache.org/r/39122/diff/


Testing
-------

Tested by creating a XPOrtalUser user using curl command and then for a new user, assigned permissions to check if permissions are not messed up.


Thanks,

Gautam Borad