> On May 10, 2016, 10:28 p.m., Richard Zang wrote:
> > ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/clusters/UserAccessListCtrl.js,
> >  line 181
> > <https://reviews.apache.org/r/46808/diff/2/?file=1378440#file1378440line181>
> >
> >     loadUser() needs to be called strictly after delete/add operation to 
> > have the up-to-date roles loaded. It's better to put it in the then block. 
> > This can be tested in the case I mentioned earlier: mygroup has "cluster 
> > user" role and aaa bbb ccc are in mygroup. Change aaa to "cluster 
> > administrator", confirm, then change aaa back to None, aaa stays at 
> > "cluster administrator" because loadUser() happens before delete finishes.
> Keta Patel wrote:
>     As per your suggestion, I have added "addPrivilege(user)" to the then 
> clause of the deletePrivilege calls. Since I use individual delete calls for 
> the various privilege IDs, I make use of "Prromise.all().then()" to ensure 
> that addPrivilege() gets called only after all the delete calls are complete. 
> The patch "AMBARI-15552-May-11.patch" contains these changes.
>     I couldn't figure out using "deletePrivileges()" to delete a set of 
> privileges and so made use of these individual calls. If you think using the 
> individual "deletePrivileges()" call is workable in this case, could you 
> please provide me with the information on the valid attributes for the body.
>     Thank you!

Please try DELETE 

- Richard

This is an automatically generated e-mail. To reply, visit:

On May 11, 2016, 10:08 p.m., Keta Patel wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46808/
> -----------------------------------------------------------
> (Updated May 11, 2016, 10:08 p.m.)
> Review request for Ambari, Di Li and Richard Zang.
> Bugs: AMBARI-15552
>     https://issues.apache.org/jira/browse/AMBARI-15552
> Repository: ambari
> Description
> -------
> Reproduction Steps:
> 1. Go to Admin->Manage Ambari
> 2. Create a group with a few users belonging to it. 
>     (I have created "mygroup" with "user1", "user2", "user3") 
>     (attachments "user1.tiff", "mygroup.tiff" shows samples)
> 3. Go to Clusters->Roles on the left navigation menu.
> 4. The default view is the "Block" view for the roles. Assign "mygroup" a 
> role, say "Cluster User". 
>     (attachment "block_view_original.tiff")
> 5. Click on "List" view, it will show Users by default. It correctly shows 
> the role "Cluster User" for each user in "mygroup". 
>     (attachment "list_view_users.tiff")
> 6. Now, try adding a new Role, say "Service Operator", to one of the users, 
> say "user3". 
>     (attachments "list_view_add_role_to_user_step1.tiff", 
> "list_view_add_role_to_user_step2.tiff")
> 7. After making this change, the role gets added for that user (in our case 
> "user3"), but the roles from other users in its group gets removed. Also, the 
> previous role for the user ("user3") is replaced by the new Role.
>     (attachment "list_view_add_role_to_user_step3.tiff")
> 8. You can confirm this from the the "Block" view. 
>     (attachment "block_view_after_step3.tiff")
> So, the problem here lies with the List view where it is not able to process 
> the changes in the Roles correctly. A change in the Role of a user causes the 
> following:
> CASE-1: The displayed role (effective privilege) comes from an explicitly 
> assigned role to the user.
> 1.1) The new selected role correctly replaces the existing privilege that was 
> explicitly assigned to the user.
> 1.2) But if the user was assigned multiple roles explicilty (before the fix 
> for AMBARI-16102 got pushed in), then all the other roles, which are of lower 
> privilege than the role that got replaced, are still displayed in the Block 
> view (because those roles are still in the database). So, if the new selected 
> role happened to be of a lower privilege than and existing role of the user, 
> then even though the user sees a success Alert message, the effective 
> privileg he sees is different. For the Ambari user, this behavior is not 
> easily understandable.
> CASE-2: The displayed role (effective privilege) comes from a group the user 
> belongs to.
> 2.1) If the new selected privilege is higher than the effective privilege 
> coming from the user's group(s), then the newly selected role replaces this 
> "group" privilege in the database, insetad of creating a new entry.
> 2.2) As a result of losing the group privilege, all the group members also 
> lose their privileges and they show "None" as their effective privilege.
> 2.3) If the newly selected privilege is lower than effective group privilege, 
> the Alert message shows a success of role change but the effective privilge 
> is still not the one that the Ambari user selected.
> Expected results:
> 1. Updating a Role of a user must replace any/all of the explicit roles it 
> has been assigned through the Block View. (this addresses 1.2)
> Note: Even though AMBARI-16102 has attempted to fix the Block view by 
> allowing only a user to have just one role assigned to it, there could be 
> cases where the earlier version of Block view has already allowed users to 
> have multiple roles. So, taking this into consideration, the fix must address 
> removing any or all of the roles the user was assigned explicitly.
> 2. Adding a Role to a user must not affect the Roles of other users in its 
> group. (addressing 2.1 and 2.2)
> 3. Selecting a "NONE" for a user role shows the Alert "User's role chnaged to 
> None". This  may not reflect the correct privilege status as the user might 
> have some effective privilege coming from its group(s). In the fix, the Alert 
> must show the relevant message.
> 4. Alert messages must show more informative messages of what is happening 
> with the user's privileges and why. (addressing 1.2 and 2.3)
> Diffs
> -----
> ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/clusters/UserAccessListCtrl.js
>  11a73f6 
> ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/Cluster.js 
> ff388cd 
> ambari-admin/src/main/resources/ui/admin-web/test/unit/controllers/clusters/UserAccessListCtrl_test.js
>  edf16be 
> Diff: https://reviews.apache.org/r/46808/diff/
> Testing
> -------
> The testing done mainly checks the logic used to update the privileges of the 
> user/group which is done after a REST call to retrieve the privileges.
> The test cases have mocks setup for server calls. The response from the 
> server calls are also mocked to work with a particular set of users and 
> groups.
> The logic in the .then() clause following the server calls is added in the 
> mock promises and tweaked slightly to work locally.
> The role selection for Users is tested for:
> 1. the new selected role has the same privilege as the user's effective 
> privilege coming from its gruop(s)
> 2. the new selected role has greater privilege than the user's effective 
> privilege coming from its group(s)
> 3. the new selected role has lower privilege tha n the user's effective 
> privilege coming from its group(s)
> The role selection for Groups is tested for:
> 1. the new selected role has the same privilege as the group's effective 
> privilege.
> 2. the new selected role has greater privilege than the group's effective 
> privilege.
> 3. the new selected role has lower privilege than the group's effective 
> privilege.
> File Attachments
> ----------------
> AMBARI-15552-May-05.patch
> https://reviews.apache.org/media/uploaded/files/2016/05/05/18b572fb-d55b-470a-ab3e-64fb05165e35__AMBARI-15552-May-05.patch
> AMBARI-15552-May-10.patch
> https://reviews.apache.org/media/uploaded/files/2016/05/10/3103f74e-e4a5-4814-8539-cd82a842af88__AMBARI-15552-May-10.patch
> AMBARI-15552-May-11.patch
> https://reviews.apache.org/media/uploaded/files/2016/05/11/67e503d1-4c68-467d-a587-d0af24a2ff3d__AMBARI-15552-May-11.patch
> Thanks,
> Keta Patel

Reply via email to