Martin Peřina has posted comments on this change. Change subject: fix handling of admin user while login ......................................................................
Patch Set 2: (3 comments) http://gerrit.ovirt.org/#/c/34551/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultiLevelAdministrationHandler.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultiLevelAdministrationHandler.java: Line 53: * @return True if user is admin Line 54: */ Line 55: public static boolean isAdminUser(DbUser user) { Line 56: List<Role> userRoles = Line 57: getRoleDAO().getAnyAdminRoleForUserAndGroups(user.getId(), StringUtils.join(user.getGroupIds(), ",")); I still think, that conversion from List to comma separated string should be part of DAO, but if others are OK with that Line 58: if (!userRoles.isEmpty()) { Line 59: log.debug("LoginAdminUser: User logged to admin using role '{}'", userRoles.get(0).getname()); Line 60: return true; Line 61: } http://gerrit.ovirt.org/#/c/34551/2/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/RoleDAOTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/RoleDAOTest.java: Line 93: assertEquals(ROLE_COUNT, result.size()); Line 94: } Line 95: Line 96: /** Line 97: * Ensures the right collection of roles is returned Please update also method comment Line 98: */ Line 99: @Test Line 100: public void testAnyAdminRoleForUserAndGroups() { Line 101: List<Role> result = dao.getAnyAdminRoleForUserAndGroups(USER_ID, Line 96: /** Line 97: * Ensures the right collection of roles is returned Line 98: */ Line 99: @Test Line 100: public void testAnyAdminRoleForUserAndGroups() { It would be nice to have also negative test (no admin role contained in groupsId param) Line 101: List<Role> result = dao.getAnyAdminRoleForUserAndGroups(USER_ID, Line 102: GROUP_IDS, ApplicationMode.AllModes.getValue()); Line 103: assertNotNull(result); Line 104: assertFalse(result.isEmpty()); -- To view, visit http://gerrit.ovirt.org/34551 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6aa489199c904008e46a650f11877091931ee5de Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
