Hello Yair Zaslavsky,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/25482
to review the following change.
Change subject: engine: Can't add user without system level admin permission
......................................................................
engine: Can't add user without system level admin permission
This patch fixes a regression introduced at
Ib62e1c051bc78b8a9ec0f32e6ba4eb9484242591
1. Introducing a new permission of adding non existing users - this is
required, as the other options are:
a. Using the existing "manipulate users" permission - this will cause a
regression, allowing roles to add users directly without the need to use the
add permissions dialogs.
b. Removing the permission check for non existing users/groups - this will
break https://bugzilla.redhat.com/923100
2. The new permission is added to all the roles that have manipulate
permissions manipulation -
this is done in order to fix the above regression.
Change-Id: I308f9cc5edb53b9633d768fd3d382dc9cf62031c
Bug-Url: https://bugzilla.redhat.com/1070651
Signed-off-by: Yair Zaslavsky <[email protected]>
Signed-off-by: Ravi Nori <[email protected]>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java
M
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/PermitType.java
M
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/PermitMapper.java
M
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/roles_ui/RoleTreeView.java
M
frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/LocalizedEnums.java
M
frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
M
frontend/webadmin/modules/uicompat/src/main/resources/org/ovirt/engine/ui/uicompat/LocalizedEnums.properties
A
packaging/dbscripts/upgrade/03_04_0650_add_missing_manipulate_users_permissions.sql
9 files changed, 44 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/82/25482/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java
index f330c8f..eb40b6a 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java
@@ -10,6 +10,7 @@
import org.ovirt.engine.core.common.action.PermissionsOperationsParameters;
import org.ovirt.engine.core.common.action.VdcActionType;
import org.ovirt.engine.core.common.action.VdcReturnValueBase;
+import org.ovirt.engine.core.common.businessentities.ActionGroup;
import org.ovirt.engine.core.common.businessentities.DbGroup;
import org.ovirt.engine.core.common.businessentities.DbUser;
import org.ovirt.engine.core.common.businessentities.Permissions;
@@ -192,10 +193,10 @@
// if the user does not exist in the database we need to
// check if the logged in user has permissions to add another
// user from the directory service
- if (getParameters().getUser() != null && dbUser == null) {
- permissionsSubject.add(new
PermissionSubject(MultiLevelAdministrationHandler.SYSTEM_OBJECT_ID,
- VdcObjectType.System,
- VdcActionType.AddUser.getActionGroup()));
+ if ((getParameters().getUser() != null && dbUser == null)
+ || (getParameters().getGroup() != null && dbGroup == null)) {
+ permissionsSubject.add(new
PermissionSubject(permission.getObjectId(),
+ permission.getObjectType(),
ActionGroup.ADD_USERS_AND_GROUPS_FROM_DIRECTORY));
}
return permissionsSubject;
}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java
index 484a593..6c13df1 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java
@@ -71,6 +71,7 @@
MANIPULATE_USERS(500, RoleType.ADMIN, VdcObjectType.User, true),
MANIPULATE_ROLES(501, RoleType.ADMIN, VdcObjectType.User, true),
MANIPULATE_PERMISSIONS(502, RoleType.USER, VdcObjectType.User, true),
+ ADD_USERS_AND_GROUPS_FROM_DIRECTORY(503, RoleType.USER,
VdcObjectType.User, true),
// storage domains actions groups
CREATE_STORAGE_DOMAIN(600, RoleType.ADMIN, VdcObjectType.Storage, true,
ApplicationMode.VirtOnly),
EDIT_STORAGE_DOMAIN_CONFIGURATION(601, RoleType.ADMIN,
VdcObjectType.Storage, true, ApplicationMode.VirtOnly),
diff --git
a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/PermitType.java
b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/PermitType.java
index 629700b..67f6b88 100644
---
a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/PermitType.java
+++
b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/PermitType.java
@@ -60,6 +60,8 @@
MANIPULATE_USERS,
MANIPULATE_ROLES,
MANIPULATE_PERMISSIONS,
+ ADD_USERS_AND_GROUPS_FROM_DIRECTORY,
+
// storage domains actions groups
CREATE_STORAGE_DOMAIN,
EDIT_STORAGE_DOMAIN_CONFIGURATION,
diff --git
a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/PermitMapper.java
b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/PermitMapper.java
index e5ddfde..8edd1b5 100644
---
a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/PermitMapper.java
+++
b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/PermitMapper.java
@@ -188,6 +188,8 @@
return PermitType.EVENT_NOTIFICATION_MANAGEMENT;
case MANIPULATE_AFFINITY_GROUPS:
return PermitType.MANIPULATE_AFFINITY_GROUPS;
+ case ADD_USERS_AND_GROUPS_FROM_DIRECTORY:
+ return PermitType.ADD_USERS_AND_GROUPS_FROM_DIRECTORY;
default:
return null;
}
@@ -340,6 +342,8 @@
return ActionGroup.EVENT_NOTIFICATION_MANAGEMENT;
case MANIPULATE_AFFINITY_GROUPS:
return ActionGroup.MANIPULATE_AFFINITY_GROUPS;
+ case ADD_USERS_AND_GROUPS_FROM_DIRECTORY:
+ return ActionGroup.ADD_USERS_AND_GROUPS_FROM_DIRECTORY;
default:
return null;
}
diff --git
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/roles_ui/RoleTreeView.java
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/roles_ui/RoleTreeView.java
index 76c3482..d9c2481 100644
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/roles_ui/RoleTreeView.java
+++
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/roles_ui/RoleTreeView.java
@@ -286,6 +286,8 @@
getConstants().allowToAddRemoveUsersFromTheSystemRoleTreeTooltip()),
new
RoleNode(ActionGroup.MANIPULATE_PERMISSIONS,
getConstants().allowToAddRemovePermissionsForUsersOnObjectsInTheSystemRoleTreeTooltip()),
+ new
RoleNode(ActionGroup.ADD_USERS_AND_GROUPS_FROM_DIRECTORY,
+
getConstants().allowToAddUsersAndGroupsFromDirectoryOnObjectsInTheSystemRoleTreeTooltip()),
new RoleNode(ActionGroup.MANIPULATE_ROLES,
getConstants().allowToDefineConfigureRolesInTheSystemRoleTreeTooltip()),
new RoleNode(ActionGroup.LOGIN,
getConstants().allowToLoginToTheSystemRoleTreeTooltip()),
diff --git
a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/LocalizedEnums.java
b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/LocalizedEnums.java
index 151d996..ae6567b 100644
---
a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/LocalizedEnums.java
+++
b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/LocalizedEnums.java
@@ -144,6 +144,8 @@
String ActionGroup___MANIPULATE_PERMISSIONS();
+ String ActionGroup___ADD_USERS_AND_GROUPS_FROM_DIRECTORY();
+
String ActionGroup___LOGIN();
String ActionGroup___TAG_MANAGEMENT();
diff --git
a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
index bc04564..6f9c629 100644
---
a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
+++
b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
@@ -1080,6 +1080,9 @@
@DefaultStringValue("Allow to add/remove permissions for Users on objects
in the system")
String
allowToAddRemovePermissionsForUsersOnObjectsInTheSystemRoleTreeTooltip();
+ @DefaultStringValue("Add users and groups from directory while adding
permissions")
+ String
allowToAddUsersAndGroupsFromDirectoryOnObjectsInTheSystemRoleTreeTooltip();
+
@DefaultStringValue("Allow to login to the system")
String allowToLoginToTheSystemRoleTreeTooltip();
diff --git
a/frontend/webadmin/modules/uicompat/src/main/resources/org/ovirt/engine/ui/uicompat/LocalizedEnums.properties
b/frontend/webadmin/modules/uicompat/src/main/resources/org/ovirt/engine/ui/uicompat/LocalizedEnums.properties
index 2f1d615..dc0eefa 100644
---
a/frontend/webadmin/modules/uicompat/src/main/resources/org/ovirt/engine/ui/uicompat/LocalizedEnums.properties
+++
b/frontend/webadmin/modules/uicompat/src/main/resources/org/ovirt/engine/ui/uicompat/LocalizedEnums.properties
@@ -68,6 +68,7 @@
ActionGroup___VM_POOL_BASIC_OPERATIONS=Basic Operations
ActionGroup___MANIPULATE_USERS=Manipulate Users
ActionGroup___MANIPULATE_PERMISSIONS=Manipulate Permissions
+ActionGroup___ADD_USERS_AND_GROUPS_FROM_DIRECTORY=Add users and groups from
directory while adding permissions
ActionGroup___LOGIN=Login Permissions
ActionGroup___TAG_MANAGEMENT=Tag management Permissions
ActionGroup___BOOKMARK_MANAGEMENT=Bookmark management Permissions
diff --git
a/packaging/dbscripts/upgrade/03_04_0650_add_missing_manipulate_users_permissions.sql
b/packaging/dbscripts/upgrade/03_04_0650_add_missing_manipulate_users_permissions.sql
new file mode 100644
index 0000000..865da42
--- /dev/null
+++
b/packaging/dbscripts/upgrade/03_04_0650_add_missing_manipulate_users_permissions.sql
@@ -0,0 +1,24 @@
+Create or replace FUNCTION _temp_add_missing_manipulate_users_permissions()
+RETURNS VOID
+ AS $procedure$
+ DECLARE
+ v_ADD_USERS_AND_GROUPS_FROM_DIRECTORY INTEGER;
+ v_MANIPULATE_PERMISSIONS INTEGER;
+BEGIN
+ v_ADD_USERS_AND_GROUPS_FROM_DIRECTORY = 503;
+ v_MANIPULATE_PERMISSIONS = 502;
+ INSERT INTO ROLES_GROUPS(role_id,action_group_id)
+ SELECT rg.role_id, v_ADD_USERS_AND_GROUPS_FROM_DIRECTORY
+ FROM ROLES_GROUPS rg
+ WHERE
+ action_group_id = v_MANIPULATE_PERMISSIONS
+ AND NOT EXISTS (SELECT 1
+ FROM ROLES_GROUPS rg2
+ WHERE rg2.role_id = rg.role_id
+ AND action_group_id =
v_ADD_USERS_AND_GROUPS_FROM_DIRECTORY);
+ RETURN;
+END; $procedure$
+LANGUAGE plpgsql;
+
+SELECT _temp_add_missing_manipulate_users_permissions();
+drop function _temp_add_missing_manipulate_users_permissions();
--
To view, visit http://gerrit.ovirt.org/25482
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I308f9cc5edb53b9633d768fd3d382dc9cf62031c
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
