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

Reply via email to