NIHVIVO-3404 Add methods to PolicyHelper to do last-minute authorization checks on additions and retractions. Create tests.

webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper.java

@@ -7,7 +7,10 @@ import javax.servlet.http.HttpServletRequest;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import com.hp.hpl.jena.ontology.OntModel;
+import com.hp.hpl.jena.ontology.OntModelSpec;
 import com.hp.hpl.jena.rdf.model.Model;
+import com.hp.hpl.jena.rdf.model.ModelFactory;
 import com.hp.hpl.jena.rdf.model.Property;
 import com.hp.hpl.jena.rdf.model.RDFNode;
 import com.hp.hpl.jena.rdf.model.Resource;
@@ -23,7 +26,6 @@ import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.propstmt.AddDataPro
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.propstmt.AddObjectPropertyStatement;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.propstmt.DropDataPropertyStatement;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.propstmt.DropObjectPropertyStatement;
-import edu.cornell.mannlib.vitro.webapp.controller.VitroRequest;
 
 /**
  * A collection of static methods to help determine whether requested actions
@@ -61,34 +63,14 @@ public class PolicyHelper {
 	}
 
 	/**
-	 * Do the current policies authorize the current user to add all of the
+	 * Do the current policies authorize the current user to add this statement
-	 * statements in this model?
+	 * to this model?
-	 */
-	public static boolean isAuthorizedToAdd(VitroRequest vreq, Model model) {
-		if ((vreq == null) || (model == null)) {
-			return false;
-		}
-
-		StmtIterator stmts = model.listStatements();
-		try {
-			while (stmts.hasNext()) {
-				if (!isAuthorizedToAdd(vreq, stmts.next())) {
-					return false;
-				}
-			}
-			return true;
-		} finally {
-			stmts.close();
-		}
-	}
-
-	/**
-	 * Do the current policies authorize the current user to add this statement?
 	 * 
 	 * The statement is expected to be fully-populated, with no null fields.
 	 */
-	public static boolean isAuthorizedToAdd(VitroRequest vreq, Statement stmt) {
+	public static boolean isAuthorizedToAdd(HttpServletRequest req,
-		if ((vreq == null) || (stmt == null)) {
+			Statement stmt, OntModel modelToBeModified) {
+		if ((req == null) || (stmt == null) || (modelToBeModified == null)) {
 			return false;
 		}
 
@@ -101,46 +83,25 @@ public class PolicyHelper {
 
 		RequestedAction action;
 		if (objectNode.isResource()) {
-			action = new AddObjectPropertyStatement(vreq.getJenaOntModel(),
+			action = new AddObjectPropertyStatement(modelToBeModified,
 					subject.getURI(), predicate.getURI(), objectNode
 							.asResource().getURI());
 		} else {
-			action = new AddDataPropertyStatement(vreq.getJenaOntModel(),
+			action = new AddDataPropertyStatement(modelToBeModified,
 					subject.getURI(), predicate.getURI());
 		}
-		return isAuthorizedForActions(vreq, action);
+		return isAuthorizedForActions(req, action);
 	}
 
 	/**
-	 * Do the current policies authorize the current user to drop all of the
+	 * Do the current policies authorize the current user to drop this statement
-	 * statements in this model?
+	 * from this model?
-	 */
-	public static boolean isAuthorizedToDrop(VitroRequest vreq, Model model) {
-		if ((vreq == null) || (model == null)) {
-			return false;
-		}
-
-		StmtIterator stmts = model.listStatements();
-		try {
-			while (stmts.hasNext()) {
-				if (!isAuthorizedToDrop(vreq, stmts.next())) {
-					return false;
-				}
-			}
-			return true;
-		} finally {
-			stmts.close();
-		}
-	}
-
-	/**
-	 * Do the current policies authorize the current user to drop this
-	 * statement?
 	 * 
 	 * The statement is expected to be fully-populated, with no null fields.
 	 */
-	public static boolean isAuthorizedToDrop(VitroRequest vreq, Statement stmt) {
+	public static boolean isAuthorizedToDrop(HttpServletRequest req,
-		if ((vreq == null) || (stmt == null)) {
+			Statement stmt, OntModel modelToBeModified) {
+		if ((req == null) || (stmt == null) || (modelToBeModified == null)) {
 			return false;
 		}
 
@@ -153,14 +114,150 @@ public class PolicyHelper {
 
 		RequestedAction action;
 		if (objectNode.isResource()) {
-			action = new DropObjectPropertyStatement(vreq.getJenaOntModel(),
+			action = new DropObjectPropertyStatement(modelToBeModified,
 					subject.getURI(), predicate.getURI(), objectNode
 							.asResource().getURI());
 		} else {
-			action = new DropDataPropertyStatement(vreq.getJenaOntModel(),
+			action = new DropDataPropertyStatement(modelToBeModified,
 					subject.getURI(), predicate.getURI());
 		}
-		return isAuthorizedForActions(vreq, action);
+		return isAuthorizedForActions(req, action);
+	}
+
+	/**
+	 * Do the current policies authorize the current user to modify this model
+	 * by adding all of the statments in the additions model and dropping all of
+	 * the statements in the retractions model?
+	 * 
+	 * This differs from the other calls to "isAuthorized..." because we always
+	 * expect the answer to be true. If the answer is false, it should be logged
+	 * as an error.
+	 * 
+	 * Even if a statement fails the test, continue to test the others, so the
+	 * log will contain a full record of all failures. This is no more expensive
+	 * than if all statements succeeded.
+	 */
+	public static boolean isAuthorizedAsExpected(HttpServletRequest req,
+			Model additions, Model retractions, OntModel modelBeingModified) {
+		if (req == null) {
+			log.warn("Can't evaluate authorization if req is null");
+			return false;
+		}
+		if (additions == null) {
+			log.warn("Can't evaluate authorization if additions model is null");
+			return false;
+		}
+		if (retractions == null) {
+			log.warn("Can't evaluate authorization if retractions model is null");
+			return false;
+		}
+		if (modelBeingModified == null) {
+			log.warn("Can't evaluate authorization if model being modified is null");
+			return false;
+		}
+
+		/*
+		 * The naive way to test the additions is to test each statement against
+		 * the JenaOntModel. However, some of the statements may not be
+		 * authorized unless others are added first. The client code should not
+		 * need to know which sequence will be successful. The client code only
+		 * cares that such a sequence does exist.
+		 * 
+		 * There are 3 obvious ways to test this, ranging from the most rigorous
+		 * (and most costly) to the least costly (and least rigorous).
+		 * 
+		 * 1. Try all sequences to find one that works. First, try to add each
+		 * statement to the modelBeingModified. If any statement succeeds,
+		 * construct a temporary model that joins that statement to the
+		 * modelBeingModified. Now try the remaining statements against that
+		 * temporary model, adding the statement each time we are successful. If
+		 * we eventually find all of the statements authorized, declare success.
+		 * This is logically rigorous, but could become geometrically expensive
+		 * as statements are repeatedly tried against incremented models. O(n!).
+		 * 
+		 * 2. Try each statement on the assumption that all of the others have
+		 * already been added. So for each statement we create a temporary
+		 * modeol that joins the other additions to the JenaOntModel. If all
+		 * statements pass this test, declare success. This is logically flawed
+		 * since it is possible that two statements would circularly authorize
+		 * each other, but that neither statement could be added first. However,
+		 * that seems like a small risk, and the algorithm is considerably less
+		 * expensive. O(n).
+		 * 
+		 * 3. Try each statement on the assumption that all of the statements
+		 * (including itself) have already been added. If all statements pass
+		 * this test, declare success. This has the additional minor flaw of
+		 * allowing a statement to authorize its own addition, but this seems
+		 * very unlikely. This is about as expensive as choice 2., but much
+		 * simpler to code.
+		 * 
+		 * For now, I am going with choice 3.
+		 */
+
+		boolean result = true;
+
+		OntModel modelToTestAgainst = ModelFactory
+				.createOntologyModel(OntModelSpec.OWL_MEM);
+		modelToTestAgainst.addSubModel(additions);
+		modelToTestAgainst.addSubModel(modelBeingModified);
+
+		StmtIterator addStmts = additions.listStatements();
+		try {
+			while (addStmts.hasNext()) {
+				Statement stmt = addStmts.next();
+				if (isAuthorizedToAdd(req, stmt, modelToTestAgainst)) {
+					if (log.isDebugEnabled()) {
+						log.debug("Last-chance authorization check: "
+								+ "authorized to add statement: "
+								+ formatStatement(stmt));
+					}
+				} else {
+					log.warn("Last-chance authorization check reveals not "
+							+ "authorized to add statement: "
+							+ formatStatement(stmt));
+					result = false;
+				}
+			}
+		} finally {
+			addStmts.close();
+		}
+
+		/*
+		 * For retractions, there is no such conundrum. Assume that all of the
+		 * additions have been added, and check the authorization of each
+		 * retraction.
+		 */
+
+		StmtIterator dropStmts = retractions.listStatements();
+		try {
+			while (dropStmts.hasNext()) {
+				Statement stmt = dropStmts.next();
+				if (isAuthorizedToDrop(req, stmt, modelToTestAgainst)) {
+					if (log.isDebugEnabled()) {
+						log.debug("Last-chance authorization check: "
+								+ "authorized to drop statement: "
+								+ formatStatement(stmt));
+					}
+				} else {
+					log.warn("Last-chance authorization check reveals not "
+							+ "authorized to drop statement: "
+							+ formatStatement(stmt));
+					result = false;
+				}
+			}
+		} finally {
+			dropStmts.close();
+		}
+
+		return result;
+	}
+
+	private static String formatStatement(Statement stmt) {
+		if (stmt == null) {
+			return "null statement";
+		}
+		return "<" + stmt.getSubject() + "> <" + stmt.getPredicate() + "> <"
+				+ stmt.getObject() + ">";
 	}
 
 	/**

webapp/test/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper_ModelsTest.java

@@ -0,0 +1,370 @@
+/* $This file is distributed under the terms of the license in /doc/license.txt$ */
+
+package edu.cornell.mannlib.vitro.webapp.auth.policy;
+
+import static org.junit.Assert.assertEquals;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+
+import org.apache.log4j.Level;
+import org.junit.Before;
+import org.junit.Test;
+
+import stubs.javax.servlet.ServletContextStub;
+import stubs.javax.servlet.http.HttpServletRequestStub;
+import stubs.javax.servlet.http.HttpSessionStub;
+
+import com.hp.hpl.jena.ontology.OntModel;
+import com.hp.hpl.jena.ontology.OntModelSpec;
+import com.hp.hpl.jena.rdf.model.Model;
+import com.hp.hpl.jena.rdf.model.ModelFactory;
+import com.hp.hpl.jena.rdf.model.Property;
+import com.hp.hpl.jena.rdf.model.Resource;
+import com.hp.hpl.jena.rdf.model.Statement;
+import com.hp.hpl.jena.rdf.model.StmtIterator;
+import com.hp.hpl.jena.util.iterator.NiceIterator;
+
+import edu.cornell.mannlib.vitro.testing.AbstractTestClass;
+import edu.cornell.mannlib.vitro.webapp.auth.identifier.IdentifierBundle;
+import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.Authorization;
+import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyDecision;
+import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyIface;
+import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.ifaces.RequestedAction;
+import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.propstmt.AbstractPropertyStatementAction;
+
+/**
+ * Test the function of PolicyHelper in authorizing models of additions and
+ * retractions.
+ * 
+ * It is vital that these methods work even if the statements are presented in
+ * the "wrong" order: one statement being authorized by a statement that appears
+ * later in the list.
+ * 
+ * In order to test that, we need to create a Model that will list the
+ * statements in an order that we can predict, vis. the order in which they were
+ * added.
+ * 
+ * To avoid creating a SortedModel that implements dozens of methods, we instead
+ * create a Proxy class that keeps the statements in order and lists them on
+ * demand.
+ */
+public class PolicyHelper_ModelsTest extends AbstractTestClass {
+	private static final String PRIMARY_RESOURCE_URI = "http://primaryResource";
+	private static final String OTHER_RESOURCE_URI = "http://otherResource";
+	private static final String FRIEND_PREDICATE_URI = "http://friend";
+	private static final String SOME_PREDICATE_URI = "http://something";
+
+	private ServletContextStub ctx;
+	private HttpSessionStub session;
+	private HttpServletRequestStub req;
+	private OntModel ontModel = ModelFactory
+			.createOntologyModel(OntModelSpec.OWL_MEM);
+
+	private Model additions;
+	private Model retractions;
+
+	@Before
+	public void setup() {
+		ctx = new ServletContextStub();
+
+		session = new HttpSessionStub();
+		session.setServletContext(ctx);
+
+		req = new HttpServletRequestStub();
+		req.setSession(session);
+
+		setLoggerLevel(ServletPolicyList.class, Level.WARN);
+		ServletPolicyList.addPolicy(ctx, new MySimplePolicy());
+
+		setLoggerLevel(PolicyHelper.class, Level.DEBUG);
+	}
+
+	// ----------------------------------------------------------------------
+	// The tests.
+	// ----------------------------------------------------------------------
+
+	@Test
+	public void rejectNullRequest() {
+		setLoggerLevel(PolicyHelper.class, Level.ERROR); // suppress warning
+		req = null;
+		additions = model();
+		retractions = model();
+		assertAuthorized("reject null request", false);
+	}
+
+	@Test
+	public void rejectNullAdditions() {
+		setLoggerLevel(PolicyHelper.class, Level.ERROR); // suppress warning
+		additions = null;
+		retractions = model();
+		assertAuthorized("reject null additions", false);
+	}
+
+	@Test
+	public void rejectNullRetractions() {
+		setLoggerLevel(PolicyHelper.class, Level.ERROR); // suppress warning
+		additions = model();
+		retractions = null;
+		assertAuthorized("reject null retractions", false);
+	}
+
+	@Test
+	public void rejectNullOntModel() {
+		setLoggerLevel(PolicyHelper.class, Level.ERROR); // suppress warning
+		additions = model();
+		retractions = model();
+		ontModel = null;
+		assertAuthorized("reject null OntModel", false);
+	}
+
+	@Test
+	public void acceptEmptyChanges() {
+		additions = model();
+		retractions = model();
+		assertAuthorized("accept empty changes add", true);
+	}
+
+	@Test
+	public void acceptSimpleAdd() {
+		additions = model(dataStatement(PRIMARY_RESOURCE_URI,
+				SOME_PREDICATE_URI));
+		retractions = model();
+		assertAuthorized("accept simple add", true);
+	}
+
+	@Test
+	public void acceptSimpleDrop() {
+		additions = model();
+		retractions = model(dataStatement(PRIMARY_RESOURCE_URI,
+				SOME_PREDICATE_URI));
+		assertAuthorized("accept simple add", true);
+	}
+
+	@Test
+	public void rejectSimpleAdd() {
+		setLoggerLevel(PolicyHelper.class, Level.ERROR); // suppress warning
+		additions = model(dataStatement(OTHER_RESOURCE_URI, SOME_PREDICATE_URI));
+		retractions = model();
+		assertAuthorized("reject simple add", false);
+	}
+
+	@Test
+	public void rejectSimpleDrop() {
+		setLoggerLevel(PolicyHelper.class, Level.ERROR); // suppress warning
+		additions = model();
+		retractions = model(dataStatement(OTHER_RESOURCE_URI,
+				SOME_PREDICATE_URI));
+		assertAuthorized("reject simple drop", false);
+	}
+
+	@Test
+	public void acceptAddBecauseOfExistingStatement() {
+		ontModel.add(objectStatement(PRIMARY_RESOURCE_URI,
+				FRIEND_PREDICATE_URI, OTHER_RESOURCE_URI));
+		additions = model(dataStatement(OTHER_RESOURCE_URI, SOME_PREDICATE_URI));
+		retractions = model();
+		assertAuthorized("accept add because of existing statement", true);
+	}
+
+	@Test
+	public void acceptDropBecauseOfExistingStatement() {
+		ontModel.add(objectStatement(PRIMARY_RESOURCE_URI,
+				FRIEND_PREDICATE_URI, OTHER_RESOURCE_URI));
+		additions = model();
+		retractions = model(dataStatement(OTHER_RESOURCE_URI,
+				SOME_PREDICATE_URI));
+		assertAuthorized("accept drop because of existing statement", true);
+	}
+
+	/**
+	 * This test is the whole reason for the funky model that lists statements
+	 * in a known order. We need to know that the DataStatement is authorized
+	 * even though it relies on an ObjectStatement that appears later in the
+	 * list.
+	 */
+	@Test
+	public void acceptAddBecauseOfOtherAdd() {
+		additions = model(
+				dataStatement(OTHER_RESOURCE_URI, SOME_PREDICATE_URI),
+				objectStatement(PRIMARY_RESOURCE_URI, FRIEND_PREDICATE_URI,
+						OTHER_RESOURCE_URI));
+		retractions = model();
+		assertAuthorized("accept add because of other add", true);
+	}
+
+	@Test
+	public void acceptDropBecauseOfAdd() {
+		additions = model(objectStatement(PRIMARY_RESOURCE_URI,
+				FRIEND_PREDICATE_URI, OTHER_RESOURCE_URI));
+		retractions = model(dataStatement(OTHER_RESOURCE_URI,
+				SOME_PREDICATE_URI));
+		assertAuthorized("accept drop because of add", true);
+	}
+
+	// ----------------------------------------------------------------------
+	// Helper methods
+	// ----------------------------------------------------------------------
+
+	/** Build a data statement. */
+	private Statement dataStatement(String subjectUri, String predicateUri) {
+		Model model = ModelFactory.createDefaultModel();
+		Resource subject = model.createResource(subjectUri);
+		Property predicate = model.createProperty(predicateUri);
+		return model.createStatement(subject, predicate, "whoCares?");
+	}
+
+	/** Build an object statement. */
+	private Statement objectStatement(String subjectUri, String predicateUri,
+			String objectUri) {
+		Model model = ModelFactory.createDefaultModel();
+		Resource subject = model.createResource(subjectUri);
+		Resource object = model.createResource(objectUri);
+		Property predicate = model.createProperty(predicateUri);
+		return model.createStatement(subject, predicate, object);
+	}
+
+	/** Build a model. */
+	private Model model(Statement... stmts) {
+		Model innerModel = ModelFactory.createDefaultModel();
+		Model proxy = (Model) Proxy.newProxyInstance(
+				OrderedModelInvocationHandler.class.getClassLoader(),
+				new Class[] { Model.class }, new OrderedModelInvocationHandler(
+						innerModel));
+		proxy.add(stmts);
+		return proxy;
+	}
+
+	/**
+	 * Check whether we are authorized to make the additions and retractions to
+	 * the model.
+	 */
+	private void assertAuthorized(String message, boolean expected) {
+		boolean actual = PolicyHelper.isAuthorizedAsExpected(req, additions,
+				retractions, ontModel);
+		assertEquals(message, expected, actual);
+	}
+
+	// ----------------------------------------------------------------------
+	// Helper classes
+	// ----------------------------------------------------------------------
+
+	/**
+	 * A model Proxy object built around this will list statements in the order
+	 * they were added.
+	 * 
+	 * This only works if the statements were added by a call to
+	 * add(Statement[]), and if they are listed by a call to listStatements().
+	 * If the test used other methods to add statements, or if the PolicyHelper
+	 * used a different method to query the model, we would not be assured of
+	 * the order of the statements from the iterator.
+	 */
+	public static class OrderedModelInvocationHandler implements
+			InvocationHandler {
+		private final Model proxied;
+		private final List<Statement> stmts = new ArrayList<Statement>();
+
+		public OrderedModelInvocationHandler(Model proxied) {
+			this.proxied = proxied;
+		}
+
+		@Override
+		public Object invoke(Object proxy, Method method, Object[] args)
+				throws Throwable {
+			if (method.getName().equals("add") && (args.length == 1)
+					&& (args[0] instanceof Statement[])) {
+				stmts.addAll(Arrays.asList((Statement[]) args[0]));
+				return method.invoke(proxied, args);
+			}
+			if (method.getName().equals("listStatements")
+					&& ((args == null) || (args.length == 0))) {
+				return new StatementListIterator(stmts);
+			}
+
+			return method.invoke(proxied, args);
+		}
+	}
+
+	/**
+	 * A StmtIterator that iterates over a list of statements.
+	 */
+	public static class StatementListIterator extends NiceIterator<Statement>
+			implements StmtIterator {
+		private final Iterator<Statement> innerIterator;
+
+		public StatementListIterator(List<Statement> stmts) {
+			this.innerIterator = new ArrayList<Statement>(stmts).iterator();
+		}
+
+		@Override
+		public Statement nextStatement() throws NoSuchElementException {
+			return next();
+		}
+
+		@Override
+		public boolean hasNext() {
+			return innerIterator.hasNext();
+		}
+
+		@Override
+		public Statement next() {
+			return innerIterator.next();
+		}
+
+	}
+
+	/**
+	 * A Policy that authorizes a statement iff (1) The subject is the primary
+	 * resource, or (2) The subject is related to the primary resource by a
+	 * "friend" property statement.
+	 */
+	private class MySimplePolicy implements PolicyIface {
+		@Override
+		public PolicyDecision isAuthorized(IdentifierBundle whoToAuth,
+				RequestedAction whatToAuth) {
+			if (!(whatToAuth instanceof AbstractPropertyStatementAction)) {
+				return inconclusive();
+			}
+
+			AbstractPropertyStatementAction action = (AbstractPropertyStatementAction) whatToAuth;
+
+			String subjectUri = action.getResourceUris()[0];
+			if (PRIMARY_RESOURCE_URI.equals(subjectUri)) {
+				return authorized();
+			}
+
+			Statement friendStmt = objectStatement(PRIMARY_RESOURCE_URI,
+					FRIEND_PREDICATE_URI, subjectUri);
+			if (statementExists(action.getOntModel(), friendStmt)) {
+				return authorized();
+			}
+
+			return inconclusive();
+		}
+
+		private boolean statementExists(OntModel oModel, Statement stmt) {
+			StmtIterator stmts = oModel.listStatements(stmt.getSubject(),
+					stmt.getPredicate(), stmt.getObject());
+			try {
+				return stmts.hasNext();
+			} finally {
+				stmts.close();
+			}
+		}
+
+		private PolicyDecision authorized() {
+			return new BasicPolicyDecision(Authorization.AUTHORIZED, "");
+		}
+
+		private PolicyDecision inconclusive() {
+			return new BasicPolicyDecision(Authorization.INCONCLUSIVE, "");
+		}
+	}
+
+}

webapp/test/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper_StatementsTest.java

@@ -12,8 +12,8 @@ import stubs.javax.servlet.ServletContextStub;
 import stubs.javax.servlet.http.HttpServletRequestStub;
 import stubs.javax.servlet.http.HttpSessionStub;
 
+import com.hp.hpl.jena.ontology.OntModel;
 import com.hp.hpl.jena.ontology.OntModelSpec;
-import com.hp.hpl.jena.rdf.model.Model;
 import com.hp.hpl.jena.rdf.model.ModelFactory;
 import com.hp.hpl.jena.rdf.model.Property;
 import com.hp.hpl.jena.rdf.model.Resource;
@@ -27,7 +27,6 @@ import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyIface;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.ifaces.RequestedAction;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.propstmt.AbstractDataPropertyStatementAction;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.propstmt.AbstractObjectPropertyStatementAction;
-import edu.cornell.mannlib.vitro.webapp.controller.VitroRequest;
 
 /**
  * Test the function of PolicyHelper in authorizing statements and models.
@@ -40,7 +39,8 @@ public class PolicyHelper_StatementsTest extends AbstractTestClass {
 
 	private ServletContextStub ctx;
 	private HttpSessionStub session;
-	private VitroRequest vreq;
+	private HttpServletRequestStub req;
+	private OntModel ontModel;
 
 	@Before
 	public void setup() {
@@ -49,23 +49,23 @@ public class PolicyHelper_StatementsTest extends AbstractTestClass {
 		session = new HttpSessionStub();
 		session.setServletContext(ctx);
 
-		HttpServletRequestStub req = new HttpServletRequestStub();
+		req = new HttpServletRequestStub();
 		req.setSession(session);
-		vreq = new VitroRequest(req);
+
-		vreq.setJenaOntModel(ModelFactory.createOntologyModel(OntModelSpec.OWL_MEM));
+		ontModel = ModelFactory.createOntologyModel(OntModelSpec.OWL_MEM);
 
 		setLoggerLevel(ServletPolicyList.class, Level.WARN);
 		ServletPolicyList.addPolicy(ctx, new MySimplePolicy());
 	}
 
 	// ----------------------------------------------------------------------
-	// Statement-level tests.
+	// The tests.
 	// ----------------------------------------------------------------------
 
 	@Test
 	public void addNullStatement() {
 		assertEquals("null statement", false,
-				PolicyHelper.isAuthorizedToAdd(vreq, (Statement) null));
+				PolicyHelper.isAuthorizedToAdd(req, null, ontModel));
 	}
 
 	@Test
@@ -73,29 +73,53 @@ public class PolicyHelper_StatementsTest extends AbstractTestClass {
 		Statement stmt = dataStatement(APPROVED_SUBJECT_URI,
 				APPROVED_PREDICATE_URI);
 		assertEquals("null request", false,
-				PolicyHelper.isAuthorizedToAdd(null, stmt));
+				PolicyHelper.isAuthorizedToAdd(null, stmt, ontModel));
 	}
 
 	@Test
-	public void addAuthorizedStatement() {
+	public void addStatementToNullModel() {
+		Statement stmt = dataStatement(APPROVED_SUBJECT_URI,
+				APPROVED_PREDICATE_URI);
+		assertEquals("authorized", false,
+				PolicyHelper.isAuthorizedToAdd(req, stmt, null));
+	}
+
+	@Test
+	public void addAuthorizedDataStatement() {
 		Statement stmt = dataStatement(APPROVED_SUBJECT_URI,
 				APPROVED_PREDICATE_URI);
 		assertEquals("authorized", true,
-				PolicyHelper.isAuthorizedToAdd(vreq, stmt));
+				PolicyHelper.isAuthorizedToAdd(req, stmt, ontModel));
 	}
 
 	@Test
-	public void addUnauthorizedStatement() {
+	public void addAuthorizedObjectStatement() {
+		Statement stmt = objectStatement(APPROVED_SUBJECT_URI,
+				APPROVED_PREDICATE_URI, APPROVED_OBJECT_URI);
+		assertEquals("authorized", true,
+				PolicyHelper.isAuthorizedToAdd(req, stmt, ontModel));
+	}
+
+	@Test
+	public void addUnauthorizedDataStatement() {
 		Statement stmt = dataStatement(APPROVED_SUBJECT_URI,
 				UNAPPROVED_PREDICATE_URI);
 		assertEquals("not authorized", false,
-				PolicyHelper.isAuthorizedToAdd(vreq, stmt));
+				PolicyHelper.isAuthorizedToAdd(req, stmt, ontModel));
+	}
+
+	@Test
+	public void addUnauthorizedObjectStatement() {
+		Statement stmt = objectStatement(APPROVED_SUBJECT_URI,
+				UNAPPROVED_PREDICATE_URI, APPROVED_OBJECT_URI);
+		assertEquals("not authorized", false,
+				PolicyHelper.isAuthorizedToAdd(req, stmt, ontModel));
 	}
 
 	@Test
 	public void dropNullStatement() {
-		assertEquals("null statement", false,
+		assertEquals("null statement", false, PolicyHelper.isAuthorizedToDrop(
-				PolicyHelper.isAuthorizedToDrop(vreq, (Statement) null));
+				req, (Statement) null, ontModel));
 	}
 
 	@Test
@@ -103,103 +127,47 @@ public class PolicyHelper_StatementsTest extends AbstractTestClass {
 		Statement stmt = dataStatement(APPROVED_SUBJECT_URI,
 				APPROVED_PREDICATE_URI);
 		assertEquals("null request", false,
-				PolicyHelper.isAuthorizedToDrop(null, stmt));
+				PolicyHelper.isAuthorizedToDrop(null, stmt, ontModel));
 	}
 
 	@Test
-	public void dropAuthorizedStatement() {
+	public void dropStatementFromNullModel() {
+		Statement stmt = dataStatement(APPROVED_SUBJECT_URI,
+				APPROVED_PREDICATE_URI);
+		assertEquals("authorized", false,
+				PolicyHelper.isAuthorizedToDrop(req, stmt, null));
+	}
+
+	@Test
+	public void dropAuthorizedDataStatement() {
 		Statement stmt = dataStatement(APPROVED_SUBJECT_URI,
 				APPROVED_PREDICATE_URI);
 		assertEquals("authorized", true,
-				PolicyHelper.isAuthorizedToDrop(vreq, stmt));
+				PolicyHelper.isAuthorizedToDrop(req, stmt, ontModel));
 	}
 
 	@Test
-	public void dropUnauthorizedStatement() {
+	public void dropAuthorizedObjectStatement() {
+		Statement stmt = objectStatement(APPROVED_SUBJECT_URI,
+				APPROVED_PREDICATE_URI, APPROVED_OBJECT_URI);
+		assertEquals("authorized", true,
+				PolicyHelper.isAuthorizedToDrop(req, stmt, ontModel));
+	}
+
+	@Test
+	public void dropUnauthorizedDataStatement() {
 		Statement stmt = dataStatement(APPROVED_SUBJECT_URI,
 				UNAPPROVED_PREDICATE_URI);
 		assertEquals("not authorized", false,
-				PolicyHelper.isAuthorizedToDrop(vreq, stmt));
+				PolicyHelper.isAuthorizedToDrop(req, stmt, ontModel));
-	}
-
-	// ----------------------------------------------------------------------
-	// Model-level tests
-	// ----------------------------------------------------------------------
-
-	@Test
-	public void addNullModel() {
-		assertEquals("null statement", false,
-				PolicyHelper.isAuthorizedToAdd(vreq, (Model) null));
 	}
 
 	@Test
-	public void addModelWithNullRequest() {
+	public void dropUnauthorizedObjectStatement() {
-		assertEquals("empty model", false,
+		Statement stmt = objectStatement(APPROVED_SUBJECT_URI,
-				PolicyHelper.isAuthorizedToAdd(null, model()));
+				UNAPPROVED_PREDICATE_URI, APPROVED_OBJECT_URI);
-	}
+		assertEquals("not authorized", false,
-
+				PolicyHelper.isAuthorizedToDrop(req, stmt, ontModel));
-	@Test
-	public void addEmptyModel() {
-		assertEquals("empty model", true,
-				PolicyHelper.isAuthorizedToAdd(vreq, model()));
-	}
-
-	@Test
-	public void addAuthorizedModel() {
-		Model model = model(
-				dataStatement(APPROVED_SUBJECT_URI, APPROVED_PREDICATE_URI),
-				objectStatement(APPROVED_SUBJECT_URI, APPROVED_PREDICATE_URI,
-						APPROVED_OBJECT_URI));
-		assertEquals("authorized model", true,
-				PolicyHelper.isAuthorizedToAdd(vreq, model));
-	}
-
-	@Test
-	public void addUnauthorizedModel() {
-		Model model = model(
-				dataStatement(APPROVED_SUBJECT_URI, APPROVED_PREDICATE_URI),
-				objectStatement(APPROVED_SUBJECT_URI, UNAPPROVED_PREDICATE_URI,
-						APPROVED_OBJECT_URI));
-		assertEquals("unauthorized model", false,
-				PolicyHelper.isAuthorizedToAdd(vreq, model));
-	}
-
-	@Test
-	public void dropNullModel() {
-		assertEquals("null statement", false,
-				PolicyHelper.isAuthorizedToDrop(vreq, (Model) null));
-	}
-
-	@Test
-	public void dropModelWithNullRequest() {
-		assertEquals("empty model", false,
-				PolicyHelper.isAuthorizedToDrop(null, model()));
-	}
-
-	@Test
-	public void dropEmptyModel() {
-		assertEquals("empty model", true,
-				PolicyHelper.isAuthorizedToDrop(vreq, model()));
-	}
-
-	@Test
-	public void dropAuthorizedModel() {
-		Model model = model(
-				dataStatement(APPROVED_SUBJECT_URI, APPROVED_PREDICATE_URI),
-				objectStatement(APPROVED_SUBJECT_URI, APPROVED_PREDICATE_URI,
-						APPROVED_OBJECT_URI));
-		assertEquals("authorized model", true,
-				PolicyHelper.isAuthorizedToDrop(vreq, model));
-	}
-
-	@Test
-	public void dropUnauthorizedModel() {
-		Model model = model(
-				dataStatement(APPROVED_SUBJECT_URI, UNAPPROVED_PREDICATE_URI),
-				objectStatement(APPROVED_SUBJECT_URI, APPROVED_PREDICATE_URI,
-						APPROVED_OBJECT_URI));
-		assertEquals("unauthorized model", false,
-				PolicyHelper.isAuthorizedToDrop(vreq, model));
 	}
 
 	// ----------------------------------------------------------------------
@@ -208,27 +176,18 @@ public class PolicyHelper_StatementsTest extends AbstractTestClass {
 
 	/** Build a data statement. */
 	private Statement dataStatement(String subjectUri, String predicateUri) {
-		Model model = ModelFactory.createDefaultModel();
+		Resource subject = ontModel.createResource(subjectUri);
-		Resource subject = model.createResource(subjectUri);
+		Property predicate = ontModel.createProperty(predicateUri);
-		Property predicate = model.createProperty(predicateUri);
+		return ontModel.createStatement(subject, predicate, "whoCares?");
-		return model.createStatement(subject, predicate, "whoCares?");
 	}
 
 	/** Build a object statement. */
 	private Statement objectStatement(String subjectUri, String predicateUri,
 			String objectUri) {
-		Model model = ModelFactory.createDefaultModel();
+		Resource subject = ontModel.createResource(subjectUri);
-		Resource subject = model.createResource(subjectUri);
+		Resource object = ontModel.createResource(objectUri);
-		Resource object = model.createResource(objectUri);
+		Property predicate = ontModel.createProperty(predicateUri);
-		Property predicate = model.createProperty(predicateUri);
+		return ontModel.createStatement(subject, predicate, object);
-		return model.createStatement(subject, predicate, object);
-	}
-
-	/** Build a model. */
-	private Model model(Statement... stmts) {
-		Model model = ModelFactory.createDefaultModel();
-		model.add(stmts);
-		return model;
 	}
 
 	// ----------------------------------------------------------------------
@@ -262,8 +221,8 @@ public class PolicyHelper_StatementsTest extends AbstractTestClass {
 		private PolicyDecision isAuthorized(
 				AbstractObjectPropertyStatementAction whatToAuth) {
 			if ((APPROVED_SUBJECT_URI.equals(whatToAuth.getSubjectUri()))
-					&& (APPROVED_PREDICATE_URI
+					&& (APPROVED_PREDICATE_URI.equals(whatToAuth
-							.equals(whatToAuth.getPredicateUri()))
+							.getPredicateUri()))
 					&& (APPROVED_OBJECT_URI.equals(whatToAuth.getObjectUri()))) {
 				return authorized();
 			} else {