Tomas Jelinek has uploaded a new change for review. Change subject: core: disable everyone for login ......................................................................
core: disable everyone for login If the "everyone" has assignd a role which contains login, from this point everyone is allowed to log in. In this specific bug the NetworkUser has been assigned to everyone which contained the login. This caused that everyone was allowed to log in. This issue did not happened the first time so this patch solves it in a more generic way - ignores the everyone when checking if the user is allowed to log in. Change-Id: I7bba37a450efdd9e3475470e4ed8f49347c51a2e Bug-Url: https://bugzilla.redhat.com/916328 Signed-off-by: Tomas Jelinek <tjeli...@redhat.com> --- M backend/manager/dbscripts/multi_level_administration_sp.sql M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbFacadeDAOTest.java 5 files changed, 12 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/30/12830/1 diff --git a/backend/manager/dbscripts/multi_level_administration_sp.sql b/backend/manager/dbscripts/multi_level_administration_sp.sql index a80174a..985b92a 100644 --- a/backend/manager/dbscripts/multi_level_administration_sp.sql +++ b/backend/manager/dbscripts/multi_level_administration_sp.sql @@ -424,7 +424,7 @@ -- gets entity permissions given the user id, groups, action group id and the object type and object id Create or replace FUNCTION get_entity_permissions_for_user_and_groups(v_user_id UUID,v_group_ids text,v_action_group_id INTEGER,v_object_id UUID,v_object_type_id INTEGER, -OUT v_permission_id UUID) +v_ignore_everyone BOOLEAN, OUT v_permission_id UUID) -- Add the parameters for the stored procedure here AS $procedure$ DECLARE @@ -437,7 +437,7 @@ -- get allparents of object and (object_id in(select id from fn_get_entity_parents(v_object_id,v_object_type_id))) -- get user and his groups - and (ad_element_id = v_everyone_object_id or + and ((NOT v_ignore_everyone and ad_element_id = v_everyone_object_id) or ad_element_id = v_user_id or ad_element_id in(select * from fnsplitteruuid(v_group_ids))) LIMIT 1; END; $procedure$ LANGUAGE plpgsql; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java index 0c0ea89..4d51086 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java @@ -840,16 +840,19 @@ * the object to check * @param type * the type of the object to check + * @param ignoreEveryone + * if true, the "everyone" will not be considered * @return <code>true</code> if the current user is authorized to run the action, <code>false</code> otherwise */ protected boolean checkUserAndGroupsAuthorization(Guid userId, String groupIds, final ActionGroup actionGroup, final Guid object, - final VdcObjectType type) { + final VdcObjectType type, + final boolean ignoreEveryone) { // Grant if there is matching permission in the database: final NGuid permId = - getDbFacade().getEntityPermissionsForUserAndGroups(userId, groupIds, actionGroup, object, type); + getDbFacade().getEntityPermissionsForUserAndGroups(userId, groupIds, actionGroup, object, type, ignoreEveryone); if (permId != null) { if (log.isDebugEnabled()) { log.debugFormat("Found permission {0} for user when running {1}, on {2} with id {3}", diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java index e1ac856..735f5ef 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java @@ -177,7 +177,7 @@ * the system in order to perform this check. The user is indeed logged in when running every command * except the login command */ - if (!checkUserAndGroupsAuthorization(ldapUser.getUserId(), ldapUser.getGroupIds(), getActionType().getActionGroup(), MultiLevelAdministrationHandler.BOTTOM_OBJECT_ID, VdcObjectType.Bottom)) { + if (!checkUserAndGroupsAuthorization(ldapUser.getUserId(), ldapUser.getGroupIds(), getActionType().getActionGroup(), MultiLevelAdministrationHandler.BOTTOM_OBJECT_ID, VdcObjectType.Bottom, true)) { addCanDoActionMessage(VdcBllMessages.USER_NOT_AUTHORIZED_TO_PERFORM_ACTION); return false; } diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java index 2038484..e4e73b8 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java @@ -231,10 +231,10 @@ } public NGuid getEntityPermissionsForUserAndGroups(Guid userId, String groupIds, ActionGroup actionGroup, Guid objectId, - VdcObjectType vdcObjectType) { + VdcObjectType vdcObjectType, boolean ignoreEveryone) { MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource().addValue("user_id", userId) .addValue("group_ids", groupIds).addValue("action_group_id", actionGroup.getId()).addValue("object_id", objectId).addValue( - "object_type_id", vdcObjectType.getValue()); + "object_type_id", vdcObjectType.getValue()).addValue("ignore_everyone", ignoreEveryone); String resultKey = "permission_id"; Map<String, Object> dbResults = diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbFacadeDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbFacadeDAOTest.java index c4c067c..61a0222 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbFacadeDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbFacadeDAOTest.java @@ -518,11 +518,11 @@ public void testGetEntityPermissionsByUserAndGroups(){ // Should not return null since the user has the relevant permission assertNotNull(dbFacade.getEntityPermissionsForUserAndGroups(Guid.NewGuid(), DIRECTORY_ELEMENT_ID_WITH_BASIC_PERMISSIONS.toString(), ActionGroup.VM_BASIC_OPERATIONS, - VMT_ID, VdcObjectType.VM)); + VMT_ID, VdcObjectType.VM, false)); // Should return null since the user does not has the relevant permission assertNull(dbFacade.getEntityPermissionsForUserAndGroups(Guid.NewGuid(), DIRECTORY_ELEMENT_ID_WITH_BASIC_PERMISSIONS.toString(), ActionGroup.CREATE_TEMPLATE, - VMT_ID, VdcObjectType.VM)); + VMT_ID, VdcObjectType.VM, false)); } } -- To view, visit http://gerrit.ovirt.org/12830 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7bba37a450efdd9e3475470e4ed8f49347c51a2e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches