From b62253558b2827b40f1d7c3cd0c6c4c4d72b00ae Mon Sep 17 00:00:00 2001 From: Richard Millet Date: Fri, 5 Nov 2010 06:38:19 +0000 Subject: [PATCH] CSPACE-3070: Fixed several existing boundary condition errors that were revealed by the previous commit -see http://fisheye.collectionspace.org/changelog/collectionspace/?cs=3415 --- .../storage/AuthorizationDelegate.java | 76 ++++++++++--------- .../storage/PermissionRoleUtil.java | 30 ++++---- 2 files changed, 58 insertions(+), 48 deletions(-) diff --git a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java index 2a3806f14..90302c9f7 100644 --- a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java +++ b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java @@ -118,42 +118,48 @@ public class AuthorizationDelegate { SubjectType subject = PermissionRoleUtil.getRelationSubject(ctx, pr); AuthZ authz = AuthZ.get(); if (subject.equals(SubjectType.ROLE)) { - PermissionValue pv = pr.getPermissions().get(0); - Permission p = getPermission(pv.getPermissionId()); - if (p == null) { - String msg = "deletePermissions: No permission found for id=" + pv.getPermissionId(); - logger.error(msg); - throw new DocumentNotFoundException(msg); - } - CSpaceResource[] resources = getResources(p); - String[] roles = getRoles(pr.getRoles()); - for (CSpaceResource res : resources) { - authz.deletePermissions(res, roles); - } + List permissionValues = pr.getPermissions(); + if (permissionValues != null & permissionValues.size() > 0) { + PermissionValue pv = permissionValues.get(0); + Permission p = getPermission(pv.getPermissionId()); + if (p == null) { + String msg = "deletePermissions: No permission found for id=" + pv.getPermissionId(); + logger.error(msg); + throw new DocumentNotFoundException(msg); + } + CSpaceResource[] resources = getResources(p); + String[] roles = getRoles(pr.getRoles()); + for (CSpaceResource res : resources) { + authz.deletePermissions(res, roles); + } + } } else if (SubjectType.PERMISSION.equals(subject)) { - RoleValue rv = pr.getRoles().get(0); - Role r = getRole(rv.getRoleId()); - if (r == null) { - String msg = "deletePermissions: No role found for id=" + rv.getRoleId(); - logger.error(msg); - throw new DocumentNotFoundException(msg); - } - //using r not rv ensures we're getting the "ROLE" prefix/qualified name - // This needs to use the qualified name, not the display name - String[] roles = {r.getRoleName()}; - for (PermissionValue pv : pr.getPermissions()) { - Permission p = getPermission(pv.getPermissionId()); - if (p == null) { - String msg = "deletePermissions: No permission found for id=" + pv.getPermissionId(); - logger.error(msg); - //TODO: would be nice contiue to still send 400 back - continue; - } - CSpaceResource[] resources = getResources(p); - for (CSpaceResource res : resources) { - authz.deletePermissions(res, roles); - } - } + List roleValues = pr.getRoles(); + if (roleValues != null && roleValues.size() > 0) { + RoleValue rv = roleValues.get(0); + Role r = getRole(rv.getRoleId()); + if (r == null) { + String msg = "deletePermissions: No role found for id=" + rv.getRoleId(); + logger.error(msg); + throw new DocumentNotFoundException(msg); + } + //using r not rv ensures we're getting the "ROLE" prefix/qualified name + // This needs to use the qualified name, not the display name + String[] roles = {r.getRoleName()}; + for (PermissionValue pv : pr.getPermissions()) { + Permission p = getPermission(pv.getPermissionId()); + if (p == null) { + String msg = "deletePermissions: No permission found for id=" + pv.getPermissionId(); + logger.error(msg); + //TODO: would be nice contiue to still send 400 back + continue; + } + CSpaceResource[] resources = getResources(p); + for (CSpaceResource res : resources) { + authz.deletePermissions(res, roles); + } + } + } } } diff --git a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleUtil.java b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleUtil.java index 071f2a1a7..7022a8c61 100644 --- a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleUtil.java +++ b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleUtil.java @@ -101,19 +101,23 @@ public class PermissionRoleUtil { boolean handleDelete) throws DocumentNotFoundException { if (subject.equals(SubjectType.ROLE)) { - //FIXME: potential index out of bounds exception...negative test needed - PermissionValue pv = pr.getPermissions().get(0); - for (RoleValue rv : pr.getRoles()) { - PermissionRoleRel prr = buildPermissonRoleRel(pv, rv, subject, handleDelete); - prrl.add(prr); - } - } else if (SubjectType.PERMISSION.equals(subject)) { - //FIXME: potential index out of bounds exception...negative test needed - RoleValue rv = pr.getRoles().get(0); - for (PermissionValue pv : pr.getPermissions()) { - PermissionRoleRel prr = buildPermissonRoleRel(pv, rv, subject, handleDelete); - prrl.add(prr); - } + List permissionValues = pr.getPermissions(); + if (permissionValues != null && permissionValues.size() > 0) { + PermissionValue pv = permissionValues.get(0); + for (RoleValue rv : pr.getRoles()) { + PermissionRoleRel prr = buildPermissonRoleRel(pv, rv, subject, handleDelete); + prrl.add(prr); + } + } + } else if (subject.equals(SubjectType.PERMISSION)) { + List roleValues = pr.getRoles(); + if (roleValues != null && roleValues.size() > 0) { + RoleValue rv = roleValues.get(0); + for (PermissionValue pv : pr.getPermissions()) { + PermissionRoleRel prr = buildPermissonRoleRel(pv, rv, subject, handleDelete); + prrl.add(prr); + } + } } } -- 2.47.3