Juan Hernandez has uploaded a new change for review. Change subject: restapi: Don't add permissions via user/roles ......................................................................
restapi: Don't add permissions via user/roles Currently permissions can be added using the /api/users/{user:id}/roles collection and doing it results in adding system permissions. The support for adding system permissions has been moved to /api/permissions in a previous patch. This patch makes the /api/users/{user:id}/roles read only so that the only way to add system permissions will be the /api/permissions resource. Change-Id: Id0acfd16d14965cfb7988556ed1b0ec1d3605c1d Bug-Url: https://bugzilla.redhat.com/1018552 Signed-off-by: Juan Hernandez <juan.hernan...@redhat.com> --- M backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/AssignedRolesResource.java M backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedRolesResource.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendAssignedRolesResourceTest.java 4 files changed, 2 insertions(+), 246 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/58/20358/1 diff --git a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/AssignedRolesResource.java b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/AssignedRolesResource.java index 21f2a0f..0fc15a4 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/AssignedRolesResource.java +++ b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/AssignedRolesResource.java @@ -16,17 +16,12 @@ package org.ovirt.engine.api.resource; -import javax.ws.rs.Consumes; -import javax.ws.rs.DELETE; import javax.ws.rs.GET; import javax.ws.rs.Path; import javax.ws.rs.PathParam; -import javax.ws.rs.POST; import javax.ws.rs.Produces; -import javax.ws.rs.core.Response; import org.jboss.resteasy.annotations.providers.jaxb.Formatted; -import org.ovirt.engine.api.model.Role; import org.ovirt.engine.api.model.Roles; /** @@ -38,15 +33,6 @@ @GET @Formatted public Roles list(); - - @POST - @Formatted - @Consumes({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) - public Response add(Role role); - - @DELETE - @Path("{id}") - public Response remove(@PathParam("id") String id); /** * Sub-resource locator method, returns individual RoleResource on which the diff --git a/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml b/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml index abc89a1..9433026 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml +++ b/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml @@ -2104,28 +2104,6 @@ signatures: [] urlparams: {} headers: {} -- name: /api/groups/{group:id}/roles/{role:id}|rel=delete - description: delete the specified role assigned to a group - request: - body: - parameterType: null - signatures: [] - urlparams: - async: {context: matrix, type: 'xs:boolean', value: true|false, required: false} - headers: - Correlation-Id: {value: 'any string', required: false} -- name: /api/groups/{group:id}/roles|rel=add - description: add a new role to the specified group - request: - body: - parameterType: Role - signatures: - - mandatoryArguments: {role.id: 'xs:string'} - urlparams: {} - headers: - Content-Type: {value: application/xml|json, required: true} - Expect: {value: 201-created, required: false} - Correlation-Id: {value: 'any string', required: false} - name: /api/groups/{group:id}/roles/{role:id}/permits|rel=get description: get the permits for the specified role in a group request: @@ -3489,29 +3467,6 @@ signatures: [] urlparams: {} headers: {} -- name: /api/users/{user:id}/roles/{role:id}|rel=delete - description: delete the specified role assigned to the user - request: - body: - parameterType: null - signatures: [] - urlparams: - async: {context: matrix, type: 'xs:boolean', value: true|false, required: false} - headers: - Correlation-Id: {value: 'any string', required: false} -- name: /api/users/{user:id}/roles|rel=add - description: add a new role to the user - request: - body: - parameterType: Role - signatures: - - mandatoryArguments: {role.id: 'xs:string'} - description: add a new role to the user - urlparams: {} - headers: - Content-Type: {value: application/xml|json, required: true} - Expect: {value: 201-created, required: false} - Correlation-Id: {value: 'any string', required: false} - name: /api/users/{user:id}/roles/{role:id}/permits|rel=get description: get the list of permits for the role assigned to the user request: diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedRolesResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedRolesResource.java index 6ed365c..732d5ed 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedRolesResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedRolesResource.java @@ -1,21 +1,18 @@ package org.ovirt.engine.api.restapi.resource; -import java.util.ArrayList; import java.util.List; import javax.ws.rs.core.Response; +import org.apache.commons.lang.NotImplementedException; import org.ovirt.engine.api.model.Role; import org.ovirt.engine.api.model.Roles; import org.ovirt.engine.api.model.User; import org.ovirt.engine.api.resource.AssignedRolesResource; import org.ovirt.engine.api.resource.RoleResource; import org.ovirt.engine.core.common.VdcObjectType; -import org.ovirt.engine.core.common.action.PermissionsOperationsParametes; -import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.businessentities.permissions; import org.ovirt.engine.core.common.queries.IdQueryParameters; -import org.ovirt.engine.core.common.queries.NameQueryParameters; import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.compat.Guid; @@ -34,24 +31,6 @@ } @Override - public Response add(Role role) { - validateParameters(role, "id|name"); - validateEnums(Role.class, role); - if (!role.isSetId()) { - org.ovirt.engine.core.common.businessentities.Role entity = getEntity( - org.ovirt.engine.core.common.businessentities.Role.class, - VdcQueryType.GetRoleByName, - new NameQueryParameters(role.getName()), - role.getName()); - role.setId(entity.getId().toString()); - } - return performCreate(VdcActionType.AddSystemPermission, - new PermissionsOperationsParametes(newPermission(role.getId())), - new QueryIdResolver<Guid>(VdcQueryType.GetPermissionById, - IdQueryParameters.class)); - } - - @Override @SingleEntityResource public RoleResource getRoleSubResource(String id) { return inject(new BackendRoleResource(id, principalId)); @@ -65,8 +44,7 @@ @Override public Response performRemove(String id) { - return performAction(VdcActionType.RemovePermission, - new PermissionsOperationsParametes(getPermission(id))); + throw new NotImplementedException(); } protected Roles mapCollection(List<permissions> entities) { @@ -84,29 +62,6 @@ role.setUser(new User()); role.getUser().setId(principalId.toString()); return role; - } - - protected permissions newPermission(String roleId) { - permissions permission = new permissions(); - permission.setad_element_id(principalId); - permission.setrole_id(new Guid(roleId)); - return permission; - } - - protected permissions getPermission(String roleId) { - List<permissions> permissions = - asCollection(getEntity(ArrayList.class, - VdcQueryType.GetPermissionsByAdElementId, - new IdQueryParameters(principalId), - principalId.toString())); - for (permissions p : permissions) { - if (principalId.equals(p.getad_element_id()) - && roleId.equals(p.getrole_id().toString()) - && p.getObjectType() == VdcObjectType.System) { - return p; - } - } - return handleError(new EntityNotFoundException(roleId), true); } @Override diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendAssignedRolesResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendAssignedRolesResourceTest.java index 2253699..75fa666 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendAssignedRolesResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendAssignedRolesResourceTest.java @@ -3,20 +3,13 @@ import java.util.ArrayList; import java.util.List; -import javax.ws.rs.WebApplicationException; -import javax.ws.rs.core.Response; - import org.junit.Ignore; import org.junit.Test; import org.ovirt.engine.api.model.Role; import org.ovirt.engine.core.common.VdcObjectType; -import org.ovirt.engine.core.common.action.PermissionsOperationsParametes; -import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.businessentities.permissions; import org.ovirt.engine.core.common.queries.IdQueryParameters; -import org.ovirt.engine.core.common.queries.NameQueryParameters; import org.ovirt.engine.core.common.queries.VdcQueryType; -import org.ovirt.engine.core.compat.Guid; public class BackendAssignedRolesResourceTest extends AbstractBackendCollectionResourceTest<Role, permissions, BackendAssignedRolesResource> { @@ -29,139 +22,6 @@ @Ignore @Override public void testQuery() throws Exception { - } - - @Test - public void testRemove() throws Exception { - setUpGetEntityExpectations(GUIDS[1], false); - setUpGetEntityExpectations(VdcQueryType.GetPermissionsByAdElementId, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { GUIDS[0] }, - setUpPermissions()); - setUriInfo(setUpActionExpectations(VdcActionType.RemovePermission, - PermissionsOperationsParametes.class, - new String[] { "Permission.ad_element_id", "Permission.role_id" }, - new Object[] { GUIDS[0], GUIDS[1] }, - true, - true)); - verifyRemove(collection.remove(GUIDS[1].toString())); - } - - @Test - public void testRemoveNonExistant() throws Exception{ - setUpGetEntityExpectations(NON_EXISTANT_GUID, true); - control.replay(); - try { - collection.remove(NON_EXISTANT_GUID.toString()); - fail("expected WebApplicationException"); - } catch (WebApplicationException wae) { - assertNotNull(wae.getResponse()); - assertEquals(wae.getResponse().getStatus(), 404); - } - } - - private void setUpGetEntityExpectations(Guid entityId, Boolean returnNull) throws Exception { - org.ovirt.engine.core.common.businessentities.Role role = null; - if (!returnNull) { - role = new org.ovirt.engine.core.common.businessentities.Role(); - role.setId(entityId); - } - setUpGetEntityExpectations(VdcQueryType.GetRoleById, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { entityId }, - role); - } - - @Test - public void testRemoveCantDo() throws Exception { - doTestBadRemove(false, true, CANT_DO); - } - - @Test - public void testRemoveFailed() throws Exception { - doTestBadRemove(true, false, FAILURE); - } - - protected void doTestBadRemove(boolean canDo, boolean success, String detail) throws Exception { - setUpGetEntityExpectations(GUIDS[1], false); - setUpGetEntityExpectations(VdcQueryType.GetPermissionsByAdElementId, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { GUIDS[0] }, - setUpPermissions()); - setUriInfo(setUpActionExpectations(VdcActionType.RemovePermission, - PermissionsOperationsParametes.class, - new String[] { "Permission.ad_element_id", "Permission.role_id" }, - new Object[] { GUIDS[0], GUIDS[1] }, - canDo, - success)); - try { - collection.remove(GUIDS[1].toString()); - fail("expected WebApplicationException"); - } catch (WebApplicationException wae) { - verifyFault(wae, detail); - } - } - - @Test - public void testAddAssignedRole() throws Exception { - setUriInfo(setUpBasicUriExpectations()); - setUpCreationExpectations(VdcActionType.AddSystemPermission, - PermissionsOperationsParametes.class, - new String[] { "Permission.ad_element_id", "Permission.role_id" }, - new Object[] { GUIDS[0], GUIDS[1] }, - true, - true, - GUIDS[2], - VdcQueryType.GetPermissionById, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { GUIDS[2] }, - getEntity(1)); - Role model = new Role(); - model.setId(GUIDS[1].toString()); - - Response response = collection.add(model); - assertEquals(201, response.getStatus()); - assertTrue(response.getEntity() instanceof Role); - verifyModel((Role) response.getEntity(), 1); - } - - @Test - public void testAddAssignedRoleByName() throws Exception { - setUriInfo(setUpBasicUriExpectations()); - setUpGetEntityExpectations(VdcQueryType.GetRoleByName, - NameQueryParameters.class, - new String[] { "Name" }, - new Object[] { NAMES[1] }, - getRole()); - setUpCreationExpectations(VdcActionType.AddSystemPermission, - PermissionsOperationsParametes.class, - new String[] { "Permission.ad_element_id", "Permission.role_id" }, - new Object[] { GUIDS[0], GUIDS[1] }, - true, - true, - GUIDS[2], - VdcQueryType.GetPermissionById, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { GUIDS[2] }, - getEntity(1)); - Role model = new Role(); - model.setName(NAMES[1]); - - Response response = collection.add(model); - assertEquals(201, response.getStatus()); - assertTrue(response.getEntity() instanceof Role); - verifyModel((Role) response.getEntity(), 1); - } - - private org.ovirt.engine.core.common.businessentities.Role getRole() { - org.ovirt.engine.core.common.businessentities.Role role = new org.ovirt.engine.core.common.businessentities.Role(); - role.setId(GUIDS[1]); - return role; } @Override -- To view, visit http://gerrit.ovirt.org/20358 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id0acfd16d14965cfb7988556ed1b0ec1d3605c1d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches