Skip to content

Commit

Permalink
Update fcrepo-module-auth-rbacl due to upstream changes.
Browse files Browse the repository at this point in the history 
- Add logging
- Update spring XML
- Change classes where implementations have been replaced

Partial resolution of: https://www.pivotaltracker.com/story/show/80568076
  • Loading branch information
Andrew Woods committed Oct 21, 2014
1 parent 325402c commit d33556b
Show file tree
Hide file tree
Showing 14 changed files with 125 additions and 63 deletions.
5 changes: 5 additions & 0 deletions fcrepo-auth-roles-basic/pom.xml
View file
Expand Up @@ -90,6 +90,11 @@
<artifactId>fcrepo-http-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.springframework</groupId>
Expand Down
20 changes: 20 additions & 0 deletions ...les-basic/src/test/java/org/fcrepo/auth/roles/basic/integration/AbstractBasicRolesIT.java
View file
Expand Up @@ -16,19 +16,23 @@
package org.fcrepo.auth.roles.basic.integration;

import static org.fcrepo.auth.common.ServletContainerAuthenticationProvider.EVERYONE_NAME;
import static org.slf4j.LoggerFactory.getLogger;

import java.util.ArrayList;
import java.util.List;
import java.util.UUID;

import org.fcrepo.auth.roles.common.integration.AbstractRolesIT;
import org.fcrepo.auth.roles.common.integration.RolesFadTestObjectBean;
import org.slf4j.Logger;

/**
* @author Scott Prater
*/
public abstract class AbstractBasicRolesIT extends AbstractRolesIT {

private static Logger logger = getLogger(AbstractBasicRolesIT.class);

protected final static String testParent1 = getRandomPid();
protected final static String testParent2 = getRandomPid();
protected final static String testParent3 = getRandomPid();
Expand All @@ -48,6 +52,22 @@ public abstract class AbstractBasicRolesIT extends AbstractRolesIT {
defineTestObjects();

private static List<RolesFadTestObjectBean> defineTestObjects() {

logger.debug("testParent1: {}", testParent1);
logger.debug("testParent2: {}", testParent2);
logger.debug("testParent3: {}", testParent3);
logger.debug("testParent4: {}", testParent4);
logger.debug("testChild1NoACL: {}", testChild1NoACL);
logger.debug("testChild2WithACL: {}", testChild2WithACL);
logger.debug("testChild3A: {}", testChild3A);
logger.debug("testChild3B: {}", testChild3B);
logger.debug("testChild4WithACL: {}", testChild4WithACL);
logger.debug("testChild5WithACL: {}", testChild5WithACL);
logger.debug("tsp1Data: {}", tsp1Data);
logger.debug("tsp2Data: {}", tsp2Data);
logger.debug("tsc1Data: {}", tsc1Data);
logger.debug("tsc2Data: {}", tsc2Data);

final List<RolesFadTestObjectBean> test_objs = new ArrayList<>();
/* public object with public datastream */
final RolesFadTestObjectBean objA = new RolesFadTestObjectBean();
Expand Down
33 changes: 22 additions & 11 deletions ...-roles-basic/src/test/java/org/fcrepo/auth/roles/basic/integration/BasicRolesAdminIT.java
View file
Expand Up @@ -21,6 +21,7 @@
import static javax.ws.rs.core.Response.Status.NO_CONTENT;
import static javax.ws.rs.core.Response.Status.OK;
import static org.junit.Assert.assertEquals;
import static org.slf4j.LoggerFactory.getLogger;

import java.io.IOException;
import java.util.List;
Expand All @@ -29,6 +30,7 @@

import org.apache.http.client.ClientProtocolException;
import org.junit.Test;
import org.slf4j.Logger;

/**
* Verifies that role for admins is properly enforced.
Expand All @@ -38,6 +40,8 @@
*/
public class BasicRolesAdminIT extends AbstractBasicRolesIT {

private static Logger logger = getLogger(BasicRolesAdminIT.class);

private final static String TESTDS = "admintestds";

@Override
Expand Down Expand Up @@ -481,30 +485,37 @@ public void testAdminCannotAddACLToAdminObjPublicDatastream()

/* Deletions */
@Test
public void testAdminCanDeleteOpenObjAndItsDescendants()
throws ClientProtocolException, IOException {
public void testAdminCanDeleteOpenObjAndItsDescendants() throws Exception {
logger.debug("Running testAdminCanDeleteOpenObjAndItsDescendants()");

// "exampleadmin" must be able to write to '/' (tombstones added on delete)
final RolesFadTestObjectBean root = new RolesFadTestObjectBean();
root.setPath("/");
root.addACL("exampleadmin", "writer");
addObjectACLs(root);

assertEquals("Admin cannot delete object testparent3!", NO_CONTENT
.getStatusCode(),
canDelete("exampleadmin", testParent3, true));
canDelete("exampleadmin", testParent3, true));

assertEquals(
"Admin should not have permission to try to read deleted datastream testparent3/tsp1_data!",
FORBIDDEN.getStatusCode(), canDelete("exampleadmin",
"Admin should not be able to read deleted datastream testparent3/tsp1_data!",
NOT_FOUND.getStatusCode(), canDelete("exampleadmin",
testParent3 + "/" + tsp1Data, true));

assertEquals(
"Admin should not have permission to try to read deleted datastream testparent3/tsp2_data!",
FORBIDDEN.getStatusCode(), canDelete("exampleadmin",
"Admin should not be able to read deleted datastream testparent3/tsp2_data!",
NOT_FOUND.getStatusCode(), canDelete("exampleadmin",
testParent3 + "/" + tsp2Data, true));

assertEquals(
"Admin should not have permission to try to read deleted object testparent3/testchild3a!",
FORBIDDEN.getStatusCode(), canDelete("exampleadmin",
"Admin should not be able to read deleted object testparent3/testchild3a!",
NOT_FOUND.getStatusCode(), canDelete("exampleadmin",
testParent3 + "/" + testChild3A, true));

assertEquals(
"Admin should not have permission to try to read deleted object testparent3/testchild3b!",
FORBIDDEN.getStatusCode(), canDelete("exampleadmin",
"Admin should not be able to read deleted object testparent3/testchild3b!",
NOT_FOUND.getStatusCode(), canDelete("exampleadmin",
testParent3 + "/" + testChild3B, true));

assertEquals(
Expand Down
5 changes: 3 additions & 2 deletions fcrepo-auth-roles-basic/src/test/resources/spring-test/repo.xml
View file
Expand Up @@ -9,8 +9,6 @@

<context:annotation-config />

<context:component-scan base-package="org.fcrepo" />

<bean name="modeshapeRepofactory" class="org.fcrepo.kernel.impl.spring.ModeShapeRepositoryFactoryBean"
depends-on="authenticationProvider">
<property name="repositoryConfiguration" value="${fcrepo.modeshape.configuration:repository.json}" />
Expand All @@ -24,6 +22,9 @@

<bean class="org.modeshape.jcr.ModeShapeEngine" init-method="start"/>

<!-- For the time being, load annotation config here too -->
<bean class="org.fcrepo.metrics.MetricsConfig"/>

<bean id="connectionManager" class="org.apache.http.impl.conn.PoolingHttpClientConnectionManager" />

</beans>
15 changes: 12 additions & 3 deletions fcrepo-auth-roles-basic/src/test/resources/spring-test/rest.xml
View file
Expand Up @@ -6,15 +6,24 @@
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd
http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.0.xsd http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util.xsd">

<context:property-placeholder/>

<!-- Context that houses JAX-RS Resources that compose the API
as well as some utility gear. -->

<context:annotation-config/>

<bean class="org.fcrepo.http.commons.session.SessionFactory"/>

<!-- Mints PIDs-->
<bean class="org.fcrepo.kernel.impl.identifiers.UUIDPidMinter"/>

<!-- Identifier translation chain -->
<util:list id="translationChain" value-type="org.fcrepo.kernel.identifiers.InternalIdentifierConverter">
<bean class="org.fcrepo.kernel.impl.identifiers.HashConverter"/>
<bean class="org.fcrepo.kernel.impl.identifiers.NamespaceConverter"/>
</util:list>

<context:component-scan base-package="org.fcrepo" />
<context:component-scan base-package="org.fcrepo"/>

</beans>
5 changes: 5 additions & 0 deletions fcrepo-auth-roles-common/pom.xml
View file
Expand Up @@ -39,6 +39,11 @@
<artifactId>infinispan-cachestore-leveldb</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.glassfish.grizzly</groupId>
Expand Down
13 changes: 10 additions & 3 deletions ...common/src/main/java/org/fcrepo/auth/roles/common/AbstractRolesAuthorizationDelegate.java
View file
Expand Up @@ -76,8 +76,15 @@ public abstract class AbstractRolesAuthorizationDelegate implements FedoraAuthor
}

@Override
public boolean hasPermission(final Session session, final Path absPath,
final String[] actions) {
public boolean hasPermission(final Session session, final Path absPath, final String[] actions) {
LOGGER.debug("Does user have permission for actions: {}, on path: {}", actions, absPath);
final boolean permission = doHasPermission(session, absPath, actions);

LOGGER.debug("Permission for actions: {}, on: {} = {}", actions, absPath, permission);
return permission;
}

private boolean doHasPermission(final Session session, final Path absPath, final String[] actions) {
final Set<String> roles;

final Principal userPrincipal = getUserPrincipal(session);
Expand All @@ -103,7 +110,7 @@ public boolean hasPermission(final Session session, final Path absPath,
}

if (LOGGER.isDebugEnabled()) {
LOGGER.debug("{}\t{}\t{}", roles, actions, absPath);
LOGGER.debug("roles: {}, actions: {}, path: {}", roles, actions, absPath);
if (actions.length > 1) { // have yet to see more than one
LOGGER.debug("FOUND MULTIPLE ACTIONS: {}", Arrays
.toString(actions));
Expand Down
33 changes: 5 additions & 28 deletions fcrepo-auth-roles-common/src/main/java/org/fcrepo/auth/roles/common/AccessRoles.java
View file
Expand Up @@ -16,7 +16,6 @@
package org.fcrepo.auth.roles.common;

import static javax.ws.rs.core.MediaType.APPLICATION_JSON;
import static org.fcrepo.jcr.FedoraJcrTypes.FCR_METADATA;

import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -45,13 +44,10 @@
import com.google.common.annotations.VisibleForTesting;
import com.hp.hpl.jena.rdf.model.Resource;
import org.fcrepo.http.commons.AbstractResource;
import org.fcrepo.http.commons.api.rdf.UriAwareIdentifierConverter;
import org.fcrepo.http.commons.api.rdf.HttpResourceConverter;
import org.fcrepo.kernel.FedoraBinary;
import org.fcrepo.kernel.FedoraResource;
import org.fcrepo.kernel.identifiers.IdentifierConverter;
import org.fcrepo.kernel.impl.DatastreamImpl;
import org.fcrepo.kernel.impl.FedoraBinaryImpl;
import org.fcrepo.kernel.impl.FedoraObjectImpl;
import org.jvnet.hk2.annotations.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -72,7 +68,7 @@ public class AccessRoles extends AbstractResource {
private static final Logger LOGGER = LoggerFactory
.getLogger(AccessRoles.class);

protected IdentifierConverter<Resource,Node> identifierTranslator;
protected IdentifierConverter<Resource,FedoraResource> identifierTranslator;


@Inject
Expand Down Expand Up @@ -255,36 +251,17 @@ protected FedoraResource resource() {
}


protected IdentifierConverter<Resource,Node> translator() {
protected IdentifierConverter<Resource,FedoraResource> translator() {
if (identifierTranslator == null) {
identifierTranslator = new UriAwareIdentifierConverter(session,
identifierTranslator = new HttpResourceConverter(session,
uriInfo.getBaseUriBuilder().clone().path("{path: .*}"));
}

return identifierTranslator;
}

private FedoraResource getResourceFromPath(final String externalPath) {
final FedoraResource resource;
final boolean metadata = externalPath != null
&& externalPath.endsWith(FCR_METADATA);

final Node node = translator().convert(translator().toDomain(externalPath));

if (DatastreamImpl.hasMixin(node)) {
final DatastreamImpl datastream = new DatastreamImpl(node);

if (metadata) {
resource = datastream;
} else {
resource = datastream.getBinary();
}
} else if (FedoraBinaryImpl.hasMixin(node)) {
resource = new FedoraBinaryImpl(node);
} else {
resource = new FedoraObjectImpl(node);
}
return resource;
return translator().convert(translator().toDomain(externalPath));
}

}
8 changes: 7 additions & 1 deletion fcrepo-auth-roles-common/src/main/java/org/fcrepo/auth/roles/common/AccessRolesProvider.java
View file
Expand Up @@ -68,6 +68,12 @@ public class AccessRolesProvider {
* @return a set of roles for each principal
*/
public Map<String, List<String>> getRoles(final Node node, final boolean effective) {
try {
LOGGER.debug("Finding roles for: {}, effective={}", node.getPath(), effective);
} catch (RepositoryException e) {
LOGGER.debug("Unable to get path! {}", e.getMessage());
}

final Map<String, List<String>> data = new HashMap<>();
try {

Expand All @@ -94,7 +100,7 @@ public Map<String, List<String>> getRoles(final Node node, final boolean effecti
}
}
} catch (final ItemNotFoundException e) {
LOGGER.trace("Subject not found, using default access roles", e);
LOGGER.debug("Subject not found, using default access roles: {}", e.getMessage());
return DEFAULT_ACCESS_ROLES;
}
}
Expand Down
5 changes: 2 additions & 3 deletions ...po-auth-roles-common/src/main/java/org/fcrepo/auth/roles/common/AccessRolesResources.java
View file
Expand Up @@ -19,7 +19,6 @@

import java.util.Map;

import javax.jcr.Node;
import javax.jcr.RepositoryException;
import javax.ws.rs.core.UriInfo;

Expand Down Expand Up @@ -51,11 +50,11 @@ public class AccessRolesResources implements UriAwareResourceModelFactory {
*/
@Override
public Model createModelForResource(final FedoraResource resource,
final UriInfo uriInfo, final IdentifierConverter<Resource, Node> graphSubjects) {
final UriInfo uriInfo, final IdentifierConverter<Resource, FedoraResource> graphSubjects) {
final Model model = ModelFactory.createDefaultModel();

try {
final Resource s = graphSubjects.reverse().convert(resource.getNode());
final Resource s = graphSubjects.reverse().convert(resource);

if (resource.getNode().isNodeType(
FedoraJcrTypes.FEDORA_RESOURCE)) {
Expand Down
3 changes: 2 additions & 1 deletion ...uth-roles-common/src/test/java/org/fcrepo/auth/roles/common/AccessRolesResourcesTest.java
View file
Expand Up @@ -48,7 +48,7 @@
*/
public class AccessRolesResourcesTest {

private IdentifierConverter<Resource, Node> graphSubjects;
private IdentifierConverter<Resource, FedoraResource> graphSubjects;

@Mock
private FedoraResource fedoraResource;
Expand Down Expand Up @@ -87,6 +87,7 @@ public void setUp() throws RepositoryException {
public void testCreateModelForNonFedoraResource()
throws RepositoryException {

when(fedoraResource.getPath()).thenReturn("/" + pathString);
when(resourceNode.isNodeType(eq(FedoraJcrTypes.FEDORA_RESOURCE)))
.thenReturn(false);

Expand Down

0 comments on commit d33556b

Please sign in to comment.