NIHVIVO-2492 restrict ImageUploadController only by requested actions.

webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthorizationHelper.java

@@ -1,49 +0,0 @@
-/* $This file is distributed under the terms of the license in /doc/license.txt$ */
-
-package edu.cornell.mannlib.vitro.webapp.auth;
-
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-
-import edu.cornell.mannlib.vitro.webapp.auth.identifier.IdentifierBundle;
-import edu.cornell.mannlib.vitro.webapp.auth.identifier.RequestIdentifiers;
-import edu.cornell.mannlib.vitro.webapp.auth.policy.RequestPolicyList;
-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.controller.VitroRequest;
-
-public class AuthorizationHelper {
-
-    private static final Log log = LogFactory.getLog(AuthorizationHelper.class);
-    
-    private VitroRequest vreq;
-    
-    public AuthorizationHelper(VitroRequest vreq) {
-        this.vreq = vreq;
-    }
-
-    public boolean isAuthorizedForRequestedAction(RequestedAction action) {
-        PolicyIface policy = getPolicies();
-        PolicyDecision dec = policy.isAuthorized(getIdentifiers(), action);
-        if (dec != null && dec.getAuthorized() == Authorization.AUTHORIZED) {
-            log.debug("Authorized because self-editing.");
-            return true;
-        } else {
-            log.debug("Not Authorized even though self-editing: "
-                    + ((dec == null) ? "null" : dec.getMessage() + ", "
-                            + dec.getDebuggingInfo()));
-            return false;
-        }
-    }
-
-    private PolicyIface getPolicies() {
-        return RequestPolicyList.getPolicies(vreq);
-    }
-
-    private IdentifierBundle getIdentifiers() {
-        return RequestIdentifiers.getIdBundleForRequest(vreq);
-    }
-
-}

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

@@ -136,6 +136,15 @@ public class PolicyHelper {
 		}
 	}
 
+	/**
+	 * Are these actions authorized for the current user by the current
+	 * policies?
+	 */
+	public static boolean isAuthorizedForAction(HttpServletRequest req,
+			RequestedAction... actions) {
+		return isAuthorizedForActionClauses(req, new ActionClauses(actions));
+	}
+
 	/**
 	 * Actions must be authorized for the current user by the current policies.
 	 * If no actions, no problem.
@@ -229,6 +238,13 @@ public class PolicyHelper {
 					.singleton(instantiateAction(actionClass)));
 		}
 
+		ActionClauses(RequestedAction[] actions) {
+			HashSet<RequestedAction> actionSet = new HashSet<RequestedAction>(
+					Arrays.asList(actions));
+			this.clauseList = Collections.singletonList(Collections
+					.unmodifiableSet(actionSet));
+		}
+
 		private void addClause(List<Set<RequestedAction>> list,
 				Class<? extends RequestedAction>[] actionClasses)
 				throws PolicyHelperException {

webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/ImageUploadController.java

@@ -16,8 +16,8 @@ import org.apache.commons.fileupload.FileItem;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import edu.cornell.mannlib.vedit.beans.LoginStatusBean;
+import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper;
-import edu.cornell.mannlib.vitro.webapp.auth.AuthorizationHelper;
+import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper.RequiresAuthorizationFor;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.ifaces.RequestActionConstants;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.ifaces.RequestedAction;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.propstmt.AddDataPropStmt;
@@ -38,11 +38,11 @@ import edu.cornell.mannlib.vitro.webapp.filestorage.backend.FileStorageSetup;
 import edu.cornell.mannlib.vitro.webapp.filestorage.model.FileInfo;
 import edu.cornell.mannlib.vitro.webapp.filestorage.model.ImageInfo;
 import edu.cornell.mannlib.vitro.webapp.filestorage.uploadrequest.FileUploadServletRequest;
-import edu.cornell.mannlib.vitro.webapp.filters.VitroRequestPrep;
 
 /**
  * Handle adding, replacing or deleting the main image on an Individual.
  */
+@RequiresAuthorizationFor(/* restricted page, but checking is done internally. */)
 public class ImageUploadController extends FreemarkerHttpServlet {
 	private static final long serialVersionUID = 1L;
 	private static final Log log = LogFactory
@@ -66,8 +66,9 @@ public class ImageUploadController extends FreemarkerHttpServlet {
 	/** The form field of the uploaded file; use as a key to the FileItem map. */
 	public static final String PARAMETER_UPLOADED_FILE = "datafile";
 
-	/** The image to use as a placeholder when the individual has no image. Determined
+	/**
-	 * by the template.
+	 * The image to use as a placeholder when the individual has no image.
+	 * Determined by the template.
 	 */
 	public static final String PARAMETER_PLACEHOLDER_URL = "placeholder";
 
@@ -155,8 +156,7 @@ public class ImageUploadController extends FreemarkerHttpServlet {
 	protected ResponseValues processRequest(VitroRequest vreq) {
 		try {
 			// Parse the multi-part request.
-			FileUploadServletRequest request = FileUploadServletRequest
+			FileUploadServletRequest.parseRequest(vreq, MAXIMUM_FILE_SIZE);
-					.parseRequest(vreq, MAXIMUM_FILE_SIZE);
 			if (log.isTraceEnabled()) {
 				dumpRequestDetails(vreq);
 			}
@@ -383,7 +383,7 @@ public class ImageUploadController extends FreemarkerHttpServlet {
 	private TemplateResponseValues showAddImagePage(VitroRequest vreq,
 			Individual entity) {
 
-	    String placeholderUrl = (String) vreq.getParameter(PARAMETER_PLACEHOLDER_URL);
+		String placeholderUrl = vreq.getParameter(PARAMETER_PLACEHOLDER_URL);
 
 		String formAction = (entity == null) ? "" : formAction(entity.getURI(),
 				ACTION_UPLOAD, placeholderUrl);
@@ -392,7 +392,6 @@ public class ImageUploadController extends FreemarkerHttpServlet {
 
 		TemplateResponseValues rv = new TemplateResponseValues(TEMPLATE_NEW);
 
-
 		rv.put(BODY_THUMBNAIL_URL, placeholderUrl);
 		rv.put(BODY_FORM_ACTION, formAction);
 		rv.put(BODY_CANCEL_URL, cancelUrl);
@@ -413,7 +412,7 @@ public class ImageUploadController extends FreemarkerHttpServlet {
 	 */
 	private TemplateResponseValues showReplaceImagePage(VitroRequest vreq,
 			Individual entity, ImageInfo imageInfo) {
-	    String placeholderUrl = (String) vreq.getParameter(PARAMETER_PLACEHOLDER_URL);
+		String placeholderUrl = vreq.getParameter(PARAMETER_PLACEHOLDER_URL);
 		TemplateResponseValues rv = new TemplateResponseValues(TEMPLATE_REPLACE);
 		rv.put(BODY_THUMBNAIL_URL, UrlBuilder.getUrl(imageInfo.getThumbnail()
 				.getBytestreamAliasUrl()));
@@ -441,7 +440,7 @@ public class ImageUploadController extends FreemarkerHttpServlet {
 	 */
 	private TemplateResponseValues showCropImagePage(VitroRequest vreq,
 			Individual entity, String imageUrl, Dimensions dimensions) {
-        String placeholderUrl = (String) vreq.getParameter(PARAMETER_PLACEHOLDER_URL);	    
+		String placeholderUrl = vreq.getParameter(PARAMETER_PLACEHOLDER_URL);
 		TemplateResponseValues rv = new TemplateResponseValues(TEMPLATE_CROP);
 		rv.put(BODY_MAIN_IMAGE_URL, UrlBuilder.getUrl(imageUrl));
 		rv.put(BODY_MAIN_IMAGE_HEIGHT, dimensions.height);
@@ -491,9 +490,11 @@ public class ImageUploadController extends FreemarkerHttpServlet {
 	 * back to this controller, along with the desired action and the Entity
 	 * URI.
 	 */
-	private String formAction(String entityUri, String action, String placeholderUrl) {
+	private String formAction(String entityUri, String action,
+			String placeholderUrl) {
 		UrlBuilder.ParamMap params = new UrlBuilder.ParamMap(
-				PARAMETER_ENTITY_URI, entityUri, PARAMETER_ACTION, action, PARAMETER_PLACEHOLDER_URL, placeholderUrl);
+				PARAMETER_ENTITY_URI, entityUri, PARAMETER_ACTION, action,
+				PARAMETER_PLACEHOLDER_URL, placeholderUrl);
 		return UrlBuilder.getPath(URL_HERE, params);
 	}
 
@@ -601,17 +602,6 @@ public class ImageUploadController extends FreemarkerHttpServlet {
 	 */
 	private boolean checkAuthorized(VitroRequest vreq)
 			throws UserMistakeException {
-		if (LoginStatusBean.getBean(vreq).isLoggedInAtLeast(
-				LoginStatusBean.EDITOR)) {
-			log.debug("Authorized because logged in as Editor");
-			return true;
-		}
-
-		if (!VitroRequestPrep.isSelfEditing(vreq)) {
-			log.debug("Not Authorized because not self-editing");
-			return false;
-		}
-
 		String action = vreq.getParameter(PARAMETER_ACTION);
 		Individual entity = validateEntityUri(vreq);
 		String imageUri = entity.getMainImageUri();
@@ -630,11 +620,7 @@ public class ImageUploadController extends FreemarkerHttpServlet {
 					RequestActionConstants.SOME_LITERAL, null, null);
 		}
 
-		AuthorizationHelper helper = new AuthorizationHelper(vreq);
+		return PolicyHelper.isAuthorizedForAction(vreq, ra);
-		boolean authorized = helper.isAuthorizedForRequestedAction(ra);
-		log.debug((authorized ? "" : "Not ") + "Authorized for '" + action
-				+ "' as self-editor;  requested action = " + ra);
-		return authorized;
 	}
 
 	private String getDefaultNamespace() {
@@ -642,5 +628,4 @@ public class ImageUploadController extends FreemarkerHttpServlet {
 				.getProperty("Vitro.defaultNamespace");
 	}
 
-
 }