Permalink
Browse files

[HHQ-4133] Perform permission checking on ResourceApi.

  • Loading branch information...
1 parent beee14a commit a9086745f73c9f0d16282697cd1d27312d69da87 Ryan Morgan committed Jul 8, 2010
View
@@ -1,6 +1,8 @@
Changes in HQApi 2.5
+ *) [HHQ-4133] Perform permission checking on ResourceApi.
+
*) Back port AlertApi to 2.x branch. As a part of this change portions of
the new MetricDataApi were also backported to aid in testing.
@@ -51,6 +51,24 @@ class ApiController extends BaseController {
}
/**
+ * Checks view permission for the passed in resource.
+ * @throws PermissionException if permission is not granted, otherwise
+ * the passed in Resource is returned.
+ */
+ protected checkViewPermission(resource) {
+ if (resource.isPlatform()) {
+ return resource.toPlatform().checkPerms(operation: 'view', user:user)
+ } else if (resource.isServer()) {
+ return resource.toServer().checkPerms(operation: 'view', user:user)
+ } else if (resource.isService()) {
+ return resource.toService().checkPerms(operation: 'view', user:user)
+ } else {
+ log.error("Unhandled resource type " + resource.prototype)
+ return null
+ }
+ }
+
+ /**
* Get the resource based on the given id. If the resource is not found,
* null is returned.
*/
@@ -69,10 +87,12 @@ class ApiController extends BaseController {
try {
resource.name // Check the object really exists
resource.entityId // Check the object is an appdef object
- return resource
} catch (Throwable t) {
return null
}
+
+ // ResourceHelper does not check permissions
+ return checkViewPermission(resource)
}
}
@@ -365,20 +365,24 @@ class ResourceController extends ApiController {
if (!id && !platformName) {
failureXml = getFailureXML(ErrorCode.INVALID_PARAMETERS)
} else {
- if (id) {
- resource = getResource(id)
- if (!resource) {
- failureXml = getFailureXML(ErrorCode.OBJECT_NOT_FOUND,
- "Resource id=" + id +
- " not found")
- }
- } else if (platformName) {
- resource = resourceHelper.find('platform':platformName)
- if (!resource) {
- failureXml = getFailureXML(ErrorCode.OBJECT_NOT_FOUND,
- "Platform '" + platformName +
- "' not found")
+ try {
+ if (id) {
+ resource = getResource(id)
+ if (!resource) {
+ failureXml = getFailureXML(ErrorCode.OBJECT_NOT_FOUND,
+ "Resource id=" + id +
+ " not found")
+ }
+ } else if (platformName) {
+ resource = resourceHelper.find('platform': platformName)
+ if (!resource) {
+ failureXml = getFailureXML(ErrorCode.OBJECT_NOT_FOUND,
+ "Platform '" + platformName +
+ "' not found")
+ }
}
+ } catch (PermissionException e) {
+ failureXml = getFailureXML(ErrorCode.PERMISSION_DENIED)
}
}
@@ -415,19 +419,42 @@ class ResourceController extends ApiController {
" not found")
} else {
def platforms = agent.platforms
- resources = platforms*.resource
+ for (platform in platforms) {
+ try {
+ resources.add(platform.checkPerms(operation: 'view', user:user))
+ } catch (PermissionException e) {
+ log.debug("Ignoring platform " + platform.name + " due to permissions.")
+ }
+ }
}
} else if (prototype) {
- resources = resourceHelper.find('byPrototype': prototype)
+ def matching = resourceHelper.find('byPrototype': prototype)
+
+ for (resource in matching) {
+ try {
+ resources.add(checkViewPermission(resource))
+ } catch (PermissionException e) {
+ log.debug("Ignoring resource " + resource.name + " due to permissions")
+ }
+ }
} else if (description) {
// TODO: Move into HQ.
+ def matching = []
def session = DAOFactory.getDAOFactory().currentSession
- resources.addAll(session.createQuery(
+ matching.addAll(session.createQuery(
"select p.resource from Platform p where p.description like '%${description}%'").list())
- resources.addAll(session.createQuery(
+ matching.addAll(session.createQuery(
"select s.resource from Server s where s.description like '%${description}%'").list())
- resources.addAll(session.createQuery(
+ matching.addAll(session.createQuery(
"select s.resource from Service s where s.description like '%${description}%'").list())
+
+ for (resource in matching) {
+ try {
+ resources.add(checkViewPermission(resource))
+ } catch (PermissionException e) {
+ log.debug("Ignoring resource " + resource.name + " due to permissions")
+ }
+ }
} else {
// Shouldn't happen
failureXml = getFailureXML(ErrorCode.INVALID_PARAMETERS)
@@ -32,7 +32,11 @@
import org.hyperic.hq.hqapi1.types.Resource;
import org.hyperic.hq.hqapi1.types.ResourcePrototype;
import org.hyperic.hq.hqapi1.types.ResourcePrototypeResponse;
+import org.hyperic.hq.hqapi1.types.ResourceResponse;
import org.hyperic.hq.hqapi1.types.ResourcesResponse;
+import org.hyperic.hq.hqapi1.types.User;
+
+import java.util.List;
public class ResourceFind_test extends ResourceTestBase {
@@ -132,4 +136,56 @@ public void testFindByDescriptionNoMatches() throws Exception {
assertTrue("Found matches for '" + DESC + "'", response.getResource().size() == 0);
}
+
+ public void testFindResourceByAgentUnauthorized() throws Exception {
+ List<User> users = createTestUsers(1);
+ User user = users.get(0);
+ ResourceApi api = getApi(user.getName(), TESTUSER_PASSWORD).getResourceApi();
+
+ // Use admin user to get local agent..
+ Agent agent = getRunningAgent();
+
+ // Test find by agent
+ ResourcesResponse response = api.getResources(agent, false, false);
+ hqAssertSuccess(response);
+
+ assertTrue("Found resources with unauthorized user", response.getResource().size() == 0);
+
+ deleteTestUsers(users);
+ }
+
+ public void testFindResourceByPrototypeUnauthorized() throws Exception {
+ List<User> users = createTestUsers(1);
+ User user = users.get(0);
+ ResourceApi api = getApi(user.getName(), TESTUSER_PASSWORD).getResourceApi();
+
+ // Use admin user to get local platform..
+ Resource localPlatform = getLocalPlatformResource(false, false);
+
+ // Test find by prototype
+ ResourcesResponse response = api.getResources(localPlatform.getResourcePrototype(), false, false);
+ hqAssertSuccess(response);
+
+ assertTrue("Found resources with unauthorized user", response.getResource().size() == 0);
+
+ deleteTestUsers(users);
+ }
+
+ public void testFindResourceByDescriptionUnauthorized() throws Exception {
+ final String DESC = "Hyperic HQ monitor Agent";
+ List<User> users = createTestUsers(1);
+ User user = users.get(0);
+ ResourceApi api = getApi(user.getName(), TESTUSER_PASSWORD).getResourceApi();
+
+ // Use admin user to get local platform..
+ Resource localPlatform = getLocalPlatformResource(false, false);
+
+ // Test find by prototype
+ ResourcesResponse response = api.getResources(DESC, false, false);
+ hqAssertSuccess(response);
+
+ assertTrue("Found resources with unauthorized user", response.getResource().size() == 0);
+
+ deleteTestUsers(users);
+ }
}
@@ -32,6 +32,11 @@
import org.hyperic.hq.hqapi1.types.Resource;
import org.hyperic.hq.hqapi1.types.ResourceResponse;
import org.hyperic.hq.hqapi1.types.ResourcesResponse;
+import org.hyperic.hq.hqapi1.types.Role;
+import org.hyperic.hq.hqapi1.types.User;
+
+import java.util.Collections;
+import java.util.List;
public class ResourceGet_test extends ResourceTestBase {
@@ -200,4 +205,34 @@ public void testGetInvalidPlatformResource() throws Exception {
false, false);
hqAssertFailureObjectNotFound(getResponse);
}
+
+ public void testGetResourceByIdUnauthorized() throws Exception {
+ List<User> users = createTestUsers(1);
+ User user = users.get(0);
+ ResourceApi api = getApi(user.getName(), TESTUSER_PASSWORD).getResourceApi();
+
+ // Use admin user to get local platform..
+ Resource localPlatform = getLocalPlatformResource(false, false);
+
+ // Test find by ID
+ ResourceResponse response = api.getResource(localPlatform.getId(), false, false);
+ hqAssertFailurePermissionDenied(response);
+
+ deleteTestUsers(users);
+ }
+
+ public void testGetResourceByNameUnauthorized() throws Exception {
+ List<User> users = createTestUsers(1);
+ User user = users.get(0);
+ ResourceApi api = getApi(user.getName(), TESTUSER_PASSWORD).getResourceApi();
+
+ // Use admin user to get local platform..
+ Resource localPlatform = getLocalPlatformResource(false, false);
+
+ // Test find by name
+ ResourceResponse response = api.getPlatformResource(localPlatform.getName(), false, false);
+ hqAssertFailurePermissionDenied(response);
+
+ deleteTestUsers(users);
+ }
}

0 comments on commit a908674

Please sign in to comment.