Security fixes for 1.8.15: reset/invite tokens and attachment authorization

src/com/fuse/actions/Login.java

@@ -22,6 +22,7 @@
 import org.pac4j.core.profile.UserProfile;
 
 import com.fuse.dao.AssessmentType;
+import com.fuse.utils.PasswordResets;
 import com.fuse.dao.AuditLog;
 import com.fuse.dao.Campaign;
 import com.fuse.dao.Category;
@@ -379,22 +380,13 @@ public String sendReset() {
 				}
 			}*/
 
-			String key = UUID.randomUUID().toString();
-			PasswordReset reset = new PasswordReset();
-			reset.setKey(key);
-			reset.setUser(u);
-			reset.setCreated(new Date());
-			// You must register first and create a password to login.
+			// Short-lived token bound to this user; any earlier token stops working.
+			String key = PasswordResets.issue(em, u, PasswordResets.RESET_TTL_MILLIS);
 			String message = "Hello " + u.getFname() + " " + u.getLname() + "<br><br>";
-			message += "Click the link below to reset your password:<br><br>";
-			String url = request.getRequestURL().toString();
-			url = url.replace(request.getRequestURI(), "");
-			url = url + request.getContextPath() + "/portal/Register?uid=" + key;
+			message += "Click the link below to reset your password. The link expires in one hour.<br><br>";
+			// Built from the configured public address, never from the request's Host header.
+			String url = FSUtils.publicBaseUrl(request) + "/Register?uid=" + key;
 			message += "<a href='" + url + "'>Click here to Reset</a><br>";
-			HibHelper.getInstance().preJoin();
-			em.joinTransaction();
-			em.persist(reset);
-			HibHelper.getInstance().commit();
 			EmailThread emailThread = new EmailThread(u.getEmail(), "Password Reset", message);
 			TaskQueueExecutor.getInstance().execute(emailThread);
 

src/com/fuse/actions/Register.java

@@ -8,12 +8,15 @@
 import org.apache.struts2.convention.annotation.Result;
 
 import com.fuse.dao.AuditLog;
+import com.fuse.utils.PasswordResets;
 import com.fuse.dao.HibHelper;
 import com.fuse.dao.PasswordReset;
 import com.fuse.dao.User;
 import com.fuse.utils.AccessControl;
 
-@Namespace("/portal")
+// Root namespace: reset and invitation links are opened without a session, and the
+// AccessControlInterceptor only lets "/" and "/sso" through unauthenticated.
+@Namespace("/")
 @Result(name = "success", location = "/WEB-INF/jsp/register/newuser.jsp")
 public class Register extends FSActionSupport {
 
@@ -24,13 +27,10 @@ public class Register extends FSActionSupport {
 	private String message = "";
 
 	@Action(value = "Register", results = {
-			@Result(name = "gotologin", type = "redirectAction", location = "../login"), })
+			@Result(name = "gotologin", type = "redirectAction", location = "login"), })
 	public String execute() {
 
-		PasswordReset reset = (PasswordReset) em.createQuery("from PasswordReset where key = :key")
-				.setParameter("key", uid)
-				.getResultList().stream()
-				.findFirst().orElse(null);
+		PasswordReset reset = PasswordResets.redeem(em, uid);
 		if(reset == null) {
 			AuditLog.audit(this, "Registration Link was not valid", AuditLog.Login, true);
 			message = "Link is no longer valid.";

src/com/fuse/actions/admin/Users.java

@@ -10,6 +10,7 @@
 import org.apache.struts2.convention.annotation.Result;
 
 import com.fuse.actions.FSActionSupport;
+import com.fuse.utils.PasswordResets;
 import com.fuse.authentication.LDAPValidator;
 import com.fuse.authentication.oauth.SecurityConfigFactory;
 import com.fuse.dao.APIKeys;
@@ -160,7 +161,7 @@ public String addUser() {
 			return this.ERRORJSON;
 		}
 
-		PasswordReset reset = null;
+		boolean invite = false;
 
 		this.username = this.username.toLowerCase();
 		User userExists = (User) em.createQuery("from User where username = :username")
@@ -202,9 +203,7 @@ public String addUser() {
 					String key = UUID.randomUUID().toString();
 					u.setPasshash(key);
 					message += "Your account has been created. Please access the link below and click to access with your SSO Credentials<br><br>";
-					String url = request.getRequestURL().toString();
-					url = url.replace(request.getRequestURI(), "");
-					url = url + request.getContextPath();
+					String url = FSUtils.publicBaseUrl(request);
 					message += "<a href='" + url + "'>Click here to Login</a><br>";
 
 
@@ -214,27 +213,19 @@ public String addUser() {
 				if(passErrorMessage.equals("")) {
 					u.setPasshash(AccessControl.HashPass(this.credential));
 					message += "Your account has been created. Please access the link below and click to access the portal. You will need to ask your Administrator for the credentials.<br><br>";
-					String url = request.getRequestURL().toString();
-					url = url.replace(request.getRequestURI(), "");
-					url = url + request.getContextPath();
+					String url = FSUtils.publicBaseUrl(request);
 					message += "<a href='" + url + "'>Click here to Login</a><br>";
 				}else {
 					this._message = passErrorMessage;
 					return this.ERRORJSON;
 				}
 
 			}else {
-				String key = UUID.randomUUID().toString();
 				u.setPasshash(AccessControl.HashPass(UUID.randomUUID().toString()));
-				reset = new PasswordReset();
-				reset.setKey(key);
-				reset.setUser(u);
-				reset.setCreated(new Date());
-				message += "Click the link below to update your password:<br><br>";
-				String url = request.getRequestURL().toString();
-				url = url.replace(request.getRequestURI(), "");
-				url = url + request.getContextPath() + "/portal/Register?uid=" + key;
-				message += "<a href='" + url + "'>Click here to Register</a><br>";
+				// The invitation token is issued after the user is persisted so it is bound
+				// to the new account; the link is appended to the message then.
+				invite = true;
+				message += "Click the link below to set your password. The link expires in 72 hours:<br><br>";
 			}
 
 			Permissions p = new Permissions();
@@ -257,13 +248,14 @@ public String addUser() {
 			HibHelper.getInstance().preJoin();
 			em.joinTransaction();
 			em.persist(u);
-			if(reset != null) {
-				em.persist(reset);
-			}
 			AuditLog.audit(this, "User " + u.getUsername() + " added", AuditLog.UserAction, false);
 
 			HibHelper.getInstance().commit();
 
+			if (invite) {
+				String key = PasswordResets.issue(em, u, PasswordResets.INVITE_TTL_MILLIS);
+				message += "<a href='" + FSUtils.publicBaseUrl(request) + "/Register?uid=" + key + "'>Click here to Register</a><br>";
+			}
 			EmailThread emailThread = new EmailThread(this.email, "New Account Created", message);
 			TaskQueueExecutor.getInstance().execute(emailThread);
 			if (api) {

src/com/fuse/api/users.java

@@ -16,6 +16,8 @@
 import javax.ws.rs.core.Response;
 
 import com.fuse.api.util.Support;
+import com.fuse.utils.FSUtils;
+import com.fuse.utils.PasswordResets;
 import com.fuse.dao.HibHelper;
 import com.fuse.dao.PasswordReset;
 import com.fuse.dao.Permissions;
@@ -88,28 +90,10 @@ public Response createUser(
 							.build();
 				}
 				newUser.setTeam(t);
-				if (verify != null && verify == true) {
-					String key = UUID.randomUUID().toString();
+				boolean invite = verify != null && verify == true;
+				if (invite) {
+					// A random, unknown password until the invitee registers through the emailed link.
 					newUser.setPasshash(AccessControl.HashPass(UUID.randomUUID().toString()));
-					// Setting username to an empty string prevents logins with GUID.
-					// You must register first and create a password to login.
-					PasswordReset reset = new PasswordReset();
-					reset.setKey(key);
-					reset.setUser(u);
-					reset.setCreated(new Date());
-					HibHelper.getInstance().preJoin();
-					em.joinTransaction();
-					em.persist(reset);
-					HibHelper.getInstance().commit();
-					String message = "Hello " + fname + " " + lname + "<br><br>";
-					message += "Click the link below to update your password:<br><br>";
-					String url = req.getRequestURL().toString();
-					url = url.replace(req.getRequestURI(), "");
-					url = url + req.getContextPath() + "/portal/Register?uid=" + key;
-					message += "<a href='" + url + "'>Click here to Register</a><br>";
-					EmailThread emailThread = new EmailThread(email, "New Account Created", message);
-					TaskQueueExecutor.getInstance().execute(emailThread);
-
 				}
 				Permissions p = new Permissions();
 				if (isAdmin != null && isAdmin == true)
@@ -128,6 +112,15 @@ public Response createUser(
 				em.joinTransaction();
 				em.persist(newUser);
 				HibHelper.getInstance().commit();
+				if (invite) {
+					// Issued after persisting so the token is bound to the NEW user, not the calling admin.
+					String key = PasswordResets.issue(em, newUser, PasswordResets.INVITE_TTL_MILLIS);
+					String message = "Hello " + fname + " " + lname + "<br><br>";
+					message += "Click the link below to set your password. The link expires in 72 hours:<br><br>";
+					String url = FSUtils.publicBaseUrl(req) + "/Register?uid=" + key;
+					message += "<a href='" + url + "'>Click here to Register</a><br>";
+					TaskQueueExecutor.getInstance().execute(new EmailThread(email, "New Account Created", message));
+				}
 				return Response.status(200).entity(Support.SUCCESS).build();
 
 			} else {

src/com/fuse/dao/PasswordReset.java

@@ -29,6 +29,8 @@ public class PasswordReset {
 	private Long id;
 	private String key;
 	private Date created;
+	/** Absolute expiry; null on rows written before 1.8.15, which are treated as created + 1h. */
+	private Date expires;
 	@ManyToOne(fetch = FetchType.EAGER)
 	private User user;
 }

src/com/fuse/servlets/FileAccess.java

@@ -0,0 +1,60 @@
+package com.fuse.servlets;
+
+import javax.persistence.EntityManager;
+
+import com.fuse.dao.Assessment;
+import com.fuse.dao.Files;
+import com.fuse.dao.User;
+import com.fuse.dao.Verification;
+import com.fuse.dao.query.AssessmentQueries;
+
+/**
+ * Object-level authorization for stored attachments. The /service/* servlets
+ * sit outside the Struts access-control interceptor, so each one has to decide
+ * for itself; this applies the same rules used to open an assessment.
+ */
+public final class FileAccess {
+
+	private FileAccess() {
+	}
+
+	/** May {@code user} read or delete this stored file? */
+	public static boolean canAccess(EntityManager em, User user, Files file) {
+		if (user == null || file == null || file.getEntityId() == null) {
+			return false;
+		}
+		if (Files.VERIFICATION.equals(file.getType())) {
+			return canAccessVerification(em, user, file.getEntityId());
+		}
+		// Assessment and engagement attachments both hang off an assessment id.
+		return canAttachToAssessment(em, user, file.getEntityId());
+	}
+
+	/** May {@code user} add or remove attachments on this assessment? */
+	public static boolean canAttachToAssessment(EntityManager em, User user, Long assessmentId) {
+		if (user == null || assessmentId == null) {
+			return false;
+		}
+		Assessment asmt = em.find(Assessment.class, assessmentId);
+		return asmt != null && AssessmentQueries.canAccessAssessment(user, asmt);
+	}
+
+	/** May {@code user} work with attachments on this verification? */
+	public static boolean canAccessVerification(EntityManager em, User user, Long verificationId) {
+		if (user == null || verificationId == null) {
+			return false;
+		}
+		Verification v = em.find(Verification.class, verificationId);
+		if (v == null) {
+			return false;
+		}
+		if (isUser(v.getAssessor(), user) || isUser(v.getAssignedRemediation(), user)) {
+			return true;
+		}
+		return v.getAssessment() != null && AssessmentQueries.canAccessAssessment(user, v.getAssessment());
+	}
+
+	private static boolean isUser(User candidate, User user) {
+		return candidate != null && candidate.getId() == user.getId();
+	}
+}

src/com/fuse/servlets/fileUpload.java

@@ -59,6 +59,16 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
 			EntityManager em = HibHelper.getInstance().getEMF().createEntityManager();
 			Files f = (Files) em.createQuery("from Files where uuid = :id").setParameter("id", id).getResultList()
 					.stream().findFirst().orElse(null);
+			if (f == null) {
+				em.close();
+				response.sendError(HttpServletResponse.SC_NOT_FOUND);
+				return;
+			}
+			if (!FileAccess.canAccess(em, u, f)) {
+				em.close();
+				response.sendError(HttpServletResponse.SC_FORBIDDEN);
+				return;
+			}
 			response.setContentType(f.getContentType());
 			// response.setContentType("application/octet-stream");
 			// response.setHeader("Content-Disposition", "attachment; filename=" +
@@ -107,6 +117,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
 			Part filePart = request.getPart("file_data");
 			Files f = new Files();
 			f.setUuid(uuid);
+			f.setCreatorId(u.getId());
 			f.setContentType(filePart.getContentType());
 			String fileName = getFileName(filePart);
 			if (!isSafeFileName(fileName)) {
@@ -122,6 +133,11 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
 				f.setType(Files.ASSESSMENT);
 				f.setEntityId(Long.parseLong(apid));
 				EntityManager em = HibHelper.getInstance().getEMF().createEntityManager();
+				if (!FileAccess.canAttachToAssessment(em, u, f.getEntityId())) {
+					em.close();
+					response.sendError(HttpServletResponse.SC_FORBIDDEN);
+					return;
+				}
 				HibHelper.getInstance().preJoin();
 				em.joinTransaction();
 				em.persist(f);
@@ -133,6 +149,11 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
 				f.setType(Files.VERIFICATION);
 				f.setEntityId(Long.parseLong(verificationId));
 				EntityManager em = HibHelper.getInstance().getEMF().createEntityManager();
+				if (!FileAccess.canAccessVerification(em, u, f.getEntityId())) {
+					em.close();
+					response.sendError(HttpServletResponse.SC_FORBIDDEN);
+					return;
+				}
 				HibHelper.getInstance().preJoin();
 				em.joinTransaction();
 				em.persist(f);
@@ -205,6 +226,11 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
 			// Session session = HibHelper.getSessionFactory().openSession();
 			Files f = (Files) em.createQuery("from Files where uuid = :id").setParameter("id", delid).getResultList()
 					.stream().findFirst().orElse(null);
+			if (f != null && !FileAccess.canAccess(em, u, f)) {
+				em.close();
+				response.sendError(HttpServletResponse.SC_FORBIDDEN);
+				return;
+			}
 			if (f != null) {
 				HibHelper.getInstance().preJoin();
 				em.joinTransaction();
@@ -222,6 +248,11 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
 			// Session session = HibHelper.getSessionFactory().openSession();
 			// HibHelper hh = new HibHelper();
 			EntityManager em = HibHelper.getInstance().getEMF().createEntityManager();
+			if (!FileAccess.canAttachToAssessment(em, u, Long.parseLong(apid))) {
+				em.close();
+				response.sendError(HttpServletResponse.SC_FORBIDDEN);
+				return;
+			}
 			Files f = (Files) em.createQuery("from Files where name = :name and entityId = :eid")
 					.setParameter("name", name).setParameter("eid", Long.parseLong(apid)).getResultList().stream()
 					.findFirst().orElse(null);

src/com/fuse/utils/FSUtils.java

@@ -53,6 +53,7 @@
 import org.apache.struts2.ServletActionContext;
 import javax.persistence.EntityManager;
 import javax.servlet.ServletContext;
+import javax.servlet.http.HttpServletRequest;
 
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.codec.binary.Hex;
@@ -236,6 +237,35 @@ private static List<Path> uploadDirectories() {
 		return dirs;
 	}
 
+	/**
+	 * Base URL for links we email out (password resets, invitations). Taken from
+	 * FACTION_BASE_URL, then FACTION_OAUTH_CALLBACK, which SSO deployments already
+	 * set to the public address. Only when neither is configured does it fall back
+	 * to the request, whose Host header a sender controls, so set one of them in
+	 * any deployment that sends mail.
+	 */
+	public static String publicBaseUrl(HttpServletRequest request) {
+		String configured = getEnv("FACTION_BASE_URL");
+		if (configured.isEmpty()) {
+			configured = getEnv("FACTION_OAUTH_CALLBACK");
+		}
+		String base;
+		if (!configured.isEmpty()) {
+			base = configured.trim();
+		} else {
+			String url = request.getRequestURL().toString();
+			String uri = request.getRequestURI();
+			if (uri != null && url.endsWith(uri)) {
+				url = url.substring(0, url.length() - uri.length());
+			}
+			base = url + (request.getContextPath() == null ? "" : request.getContextPath());
+		}
+		while (base.endsWith("/")) {
+			base = base.substring(0, base.length() - 1);
+		}
+		return base;
+	}
+
 	public static boolean containsHTML(String value) {
 		return value != null && (value.indexOf('<') >= 0 || value.indexOf('>') >= 0);
 	}