From c37deca462d9007a51b3c40c4f6500f0692079ff Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 3 Oct 2026 01:01:27 -0500 Subject: [PATCH 1/2] Fix account takeover paths in reset/invite tokens and add file authorization Three reported issues (GHSA-3vm2-w3rc-mcgg, GHSA-c6h7-9644-p3pw, GHSA-gx88-52rh-qhcr): - The users API bound the invitation token to the calling admin instead of the new user, so an invitee could set the admin's password. Tokens are now issued by PasswordResets.issue() after the target user is persisted, in the API, the admin Users page and sendReset. - Reset and invitation links were built from the request's Host header and never expired. FSUtils.publicBaseUrl() builds them from FACTION_BASE_URL, then FACTION_OAUTH_CALLBACK, falling back to the request only when neither is configured. Tokens carry an expiry (1h reset, 72h invite), a new token supersedes the user's earlier ones, and Register redeems through PasswordResets.redeem(), which rejects and deletes expired tokens. Rows written before this release are treated as expiring an hour after creation. - /service/fileUpload checked only for a session. FileAccess now resolves a stored file to its assessment or verification and applies the same rules as opening the assessment (assessor, team, access level, or verification assessor/assigned remediation) to download, delete and attach, returning 403 otherwise and 404 for unknown ids. Uploads record the creator. Tests: PasswordResetsTest, PublicBaseUrlTest, FileAccessTest. --- src/com/fuse/actions/Login.java | 20 +- src/com/fuse/actions/Register.java | 6 +- src/com/fuse/actions/admin/Users.java | 32 ++-- src/com/fuse/api/users.java | 35 ++-- src/com/fuse/dao/PasswordReset.java | 2 + src/com/fuse/servlets/FileAccess.java | 60 ++++++ src/com/fuse/servlets/fileUpload.java | 31 ++++ src/com/fuse/utils/FSUtils.java | 30 +++ src/com/fuse/utils/PasswordResets.java | 96 ++++++++++ test/com/fuse/servlets/FileAccessTest.java | 192 ++++++++++++++++++++ test/com/fuse/utils/PasswordResetsTest.java | 160 ++++++++++++++++ test/com/fuse/utils/PublicBaseUrlTest.java | 49 +++++ 12 files changed, 654 insertions(+), 59 deletions(-) create mode 100644 src/com/fuse/servlets/FileAccess.java create mode 100644 src/com/fuse/utils/PasswordResets.java create mode 100644 test/com/fuse/servlets/FileAccessTest.java create mode 100644 test/com/fuse/utils/PasswordResetsTest.java create mode 100644 test/com/fuse/utils/PublicBaseUrlTest.java diff --git a/src/com/fuse/actions/Login.java b/src/com/fuse/actions/Login.java index 4c0b95a6..a3dd7bed 100644 --- a/src/com/fuse/actions/Login.java +++ b/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() + "

"; - message += "Click the link below to reset your password:

"; - 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.

"; + // Built from the configured public address, never from the request's Host header. + String url = FSUtils.publicBaseUrl(request) + "/portal/Register?uid=" + key; message += "Click here to Reset
"; - HibHelper.getInstance().preJoin(); - em.joinTransaction(); - em.persist(reset); - HibHelper.getInstance().commit(); EmailThread emailThread = new EmailThread(u.getEmail(), "Password Reset", message); TaskQueueExecutor.getInstance().execute(emailThread); diff --git a/src/com/fuse/actions/Register.java b/src/com/fuse/actions/Register.java index 2bd4140d..0835c96f 100644 --- a/src/com/fuse/actions/Register.java +++ b/src/com/fuse/actions/Register.java @@ -8,6 +8,7 @@ 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; @@ -27,10 +28,7 @@ public class Register extends FSActionSupport { @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."; diff --git a/src/com/fuse/actions/admin/Users.java b/src/com/fuse/actions/admin/Users.java index c9c516d5..788d83a3 100644 --- a/src/com/fuse/actions/admin/Users.java +++ b/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

"; - String url = request.getRequestURL().toString(); - url = url.replace(request.getRequestURI(), ""); - url = url + request.getContextPath(); + String url = FSUtils.publicBaseUrl(request); message += "Click here to Login
"; @@ -214,9 +213,7 @@ 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.

"; - String url = request.getRequestURL().toString(); - url = url.replace(request.getRequestURI(), ""); - url = url + request.getContextPath(); + String url = FSUtils.publicBaseUrl(request); message += "Click here to Login
"; }else { this._message = passErrorMessage; @@ -224,17 +221,11 @@ public String addUser() { } }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:

"; - String url = request.getRequestURL().toString(); - url = url.replace(request.getRequestURI(), ""); - url = url + request.getContextPath() + "/portal/Register?uid=" + key; - message += "Click here to Register
"; + // 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:

"; } 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 += "Click here to Register
"; + } EmailThread emailThread = new EmailThread(this.email, "New Account Created", message); TaskQueueExecutor.getInstance().execute(emailThread); if (api) { diff --git a/src/com/fuse/api/users.java b/src/com/fuse/api/users.java index 02d87878..fa2d2203 100644 --- a/src/com/fuse/api/users.java +++ b/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 + "

"; - message += "Click the link below to update your password:

"; - String url = req.getRequestURL().toString(); - url = url.replace(req.getRequestURI(), ""); - url = url + req.getContextPath() + "/portal/Register?uid=" + key; - message += "Click here to Register
"; - 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 + "

"; + message += "Click the link below to set your password. The link expires in 72 hours:

"; + String url = FSUtils.publicBaseUrl(req) + "/portal/Register?uid=" + key; + message += "Click here to Register
"; + TaskQueueExecutor.getInstance().execute(new EmailThread(email, "New Account Created", message)); + } return Response.status(200).entity(Support.SUCCESS).build(); } else { diff --git a/src/com/fuse/dao/PasswordReset.java b/src/com/fuse/dao/PasswordReset.java index 2e9199fd..18dbc64c 100644 --- a/src/com/fuse/dao/PasswordReset.java +++ b/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; } diff --git a/src/com/fuse/servlets/FileAccess.java b/src/com/fuse/servlets/FileAccess.java new file mode 100644 index 00000000..771290a1 --- /dev/null +++ b/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(); + } +} diff --git a/src/com/fuse/servlets/fileUpload.java b/src/com/fuse/servlets/fileUpload.java index c62e26fe..5833d9af 100644 --- a/src/com/fuse/servlets/fileUpload.java +++ b/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); diff --git a/src/com/fuse/utils/FSUtils.java b/src/com/fuse/utils/FSUtils.java index f35b2b75..3cee7246 100644 --- a/src/com/fuse/utils/FSUtils.java +++ b/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 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); } diff --git a/src/com/fuse/utils/PasswordResets.java b/src/com/fuse/utils/PasswordResets.java new file mode 100644 index 00000000..3a13feaf --- /dev/null +++ b/src/com/fuse/utils/PasswordResets.java @@ -0,0 +1,96 @@ +package com.fuse.utils; + +import java.util.Date; +import java.util.List; +import java.util.UUID; + +import javax.persistence.EntityManager; + +import com.fuse.dao.HibHelper; +import com.fuse.dao.PasswordReset; +import com.fuse.dao.User; + +/** + * Password reset and invitation tokens. + * + * A token is bound to the user whose password it will set, expires, and + * supersedes any earlier token for that user. Callers issue a token after the + * target user is persisted and put the returned key in the emailed link; + * {@link com.fuse.actions.Register} redeems it. + */ +public final class PasswordResets { + + /** Self-service password resets: one hour. */ + public static final long RESET_TTL_MILLIS = 60L * 60 * 1000; + /** Invitations for newly created accounts: three days. */ + public static final long INVITE_TTL_MILLIS = 72L * 60 * 60 * 1000; + + private PasswordResets() { + } + + /** + * Issues a fresh token for {@code target}, invalidating any outstanding ones. + * Manages its own transaction, so call it after the target user is committed. + */ + public static String issue(EntityManager em, User target, long ttlMillis) { + if (target == null || target.getId() == 0L) { + throw new IllegalArgumentException("reset target must be a persisted user"); + } + String key = UUID.randomUUID().toString(); + Date now = new Date(); + PasswordReset reset = new PasswordReset(); + reset.setKey(key); + reset.setUser(target); + reset.setCreated(now); + reset.setExpires(new Date(now.getTime() + ttlMillis)); + + HibHelper.getInstance().preJoin(); + em.joinTransaction(); + for (PasswordReset old : outstandingFor(em, target)) { + em.remove(old); + } + em.persist(reset); + HibHelper.getInstance().commit(); + return key; + } + + /** + * Returns the live token for {@code key}, or null when it is unknown or + * expired. Expired tokens are deleted on the way out. + */ + public static PasswordReset redeem(EntityManager em, String key) { + if (key == null || key.isEmpty()) { + return null; + } + PasswordReset reset = (PasswordReset) em.createQuery("from PasswordReset where key = :key") + .setParameter("key", key).getResultList().stream().findFirst().orElse(null); + if (reset == null) { + return null; + } + if (isExpired(reset)) { + HibHelper.getInstance().preJoin(); + em.joinTransaction(); + em.remove(reset); + HibHelper.getInstance().commit(); + return null; + } + return reset; + } + + static boolean isExpired(PasswordReset reset) { + Date expires = reset.getExpires(); + if (expires == null) { + // Pre-1.8.15 row: it never carried an expiry, so give it the reset window. + if (reset.getCreated() == null) { + return true; + } + expires = new Date(reset.getCreated().getTime() + RESET_TTL_MILLIS); + } + return !expires.after(new Date()); + } + + @SuppressWarnings("unchecked") + private static List outstandingFor(EntityManager em, User target) { + return em.createQuery("from PasswordReset where user = :user").setParameter("user", target).getResultList(); + } +} diff --git a/test/com/fuse/servlets/FileAccessTest.java b/test/com/fuse/servlets/FileAccessTest.java new file mode 100644 index 00000000..1b61a151 --- /dev/null +++ b/test/com/fuse/servlets/FileAccessTest.java @@ -0,0 +1,192 @@ +package com.fuse.servlets; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.UUID; + +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import com.fuse.api.MongoTestBase; +import com.fuse.dao.Assessment; +import com.fuse.dao.Files; +import com.fuse.dao.HibHelper; +import com.fuse.dao.Permissions; +import com.fuse.dao.Teams; +import com.fuse.dao.User; +import com.fuse.dao.Verification; + +/** + * /service/fileUpload must apply object-level authorization: a stored file is + * reachable only by users who may access the assessment (or verification) it + * belongs to, using the same rules as opening that assessment. + */ +public class FileAccessTest extends MongoTestBase { + + private static EntityManagerFactory emf; + private static String tag; + private static Teams teamA, teamB; + private static User ownerA, teammateA, strangerB, managerB, remediationB; + private static Assessment asmt; + private static Verification verification; + private static Files asmtFile, verFile; + + @BeforeClass + public static void seed() { + emf = HibHelper.getInstance().getEMF(); + org.junit.Assume.assumeNotNull("EntityManagerFactory unavailable - skipping integration test", emf); + tag = UUID.randomUUID().toString().substring(0, 8); + EntityManager em = emf.createEntityManager(); + try { + teamA = team(em, "A-" + tag); + teamB = team(em, "B-" + tag); + ownerA = user(em, "owner-" + tag, teamA, false, false, Permissions.AccessLevelUserOnly); + teammateA = user(em, "mate-" + tag, teamA, false, false, Permissions.AccessLevelTeamOnly); + strangerB = user(em, "stranger-" + tag, teamB, false, false, Permissions.AccessLevelUserOnly); + managerB = user(em, "manager-" + tag, teamB, true, false, Permissions.AccessLevelAllData); + remediationB = user(em, "rem-" + tag, teamB, false, true, Permissions.AccessLevelUserOnly); + asmt = new Assessment(); + asmt.setName("asmt-" + tag); + asmt.setAppId("APP-" + tag); + List assessors = new ArrayList<>(); + assessors.add(ownerA); + asmt.setAssessor(assessors); + asmt.setStart(new Date()); + asmt.setEnd(new Date()); + persist(em, asmt); + verification = new Verification(); + verification.setAssessment(asmt); + verification.setAssessor(ownerA); + verification.setAssignedRemediation(remediationB); + persist(em, verification); + asmtFile = file(em, Files.ASSESSMENT, asmt.getId()); + verFile = file(em, Files.VERIFICATION, verification.getId()); + } finally { + em.close(); + } + } + + @AfterClass + public static void cleanup() { + if (emf == null) + return; + EntityManager em = emf.createEntityManager(); + try { + for (Object o : new Object[] { asmtFile, verFile, verification, asmt, ownerA, teammateA, strangerB, managerB, + remediationB, teamA, teamB }) + if (o != null) + remove(em, o); + } finally { + em.close(); + } + } + + @Test + public void assessmentFileFollowsAssessmentAccess() { + EntityManager em = emf.createEntityManager(); + try { + assertTrue("assessor on the assessment", FileAccess.canAccess(em, ownerA, asmtFile)); + assertTrue("same team with team-level access", FileAccess.canAccess(em, teammateA, asmtFile)); + assertTrue("manager with all-data access", FileAccess.canAccess(em, managerB, asmtFile)); + assertFalse("user-only assessor from another team", FileAccess.canAccess(em, strangerB, asmtFile)); + } finally { + em.close(); + } + } + + @Test + public void verificationFileFollowsVerificationAccess() { + EntityManager em = emf.createEntityManager(); + try { + assertTrue("verification assessor", FileAccess.canAccess(em, ownerA, verFile)); + assertTrue("assigned remediation user", FileAccess.canAccess(em, remediationB, verFile)); + assertTrue("manager", FileAccess.canAccess(em, managerB, verFile)); + assertFalse("unrelated user-only user", FileAccess.canAccess(em, strangerB, verFile)); + } finally { + em.close(); + } + } + + @Test + public void attachingToAnAssessmentNeedsAccessToIt() { + EntityManager em = emf.createEntityManager(); + try { + assertTrue(FileAccess.canAttachToAssessment(em, ownerA, asmt.getId())); + assertFalse(FileAccess.canAttachToAssessment(em, strangerB, asmt.getId())); + assertFalse("unknown assessment", FileAccess.canAttachToAssessment(em, managerB, -1L)); + } finally { + em.close(); + } + } + + @Test + public void missingFileIsNeverAccessible() { + EntityManager em = emf.createEntityManager(); + try { + assertFalse(FileAccess.canAccess(em, managerB, null)); + } finally { + em.close(); + } + } + + private static Files file(EntityManager em, String type, Long entityId) { + Files f = new Files(); + f.setUuid(UUID.randomUUID().toString()); + f.setName("f-" + tag + ".txt"); + f.setContentType("text/plain"); + f.setType(type); + f.setEntityId(entityId); + f.setRealFile("hello".getBytes()); + persist(em, f); + return f; + } + + private static Teams team(EntityManager em, String name) { + Teams t = new Teams(); + t.setTeamName(name); + persist(em, t); + return t; + } + + private static User user(EntityManager em, String name, Teams team, boolean manager, boolean remediation, Integer level) { + Permissions p = new Permissions(); + p.setAssessor(!remediation); + p.setManager(manager); + p.setRemediation(remediation); + p.setEngagement(false); + p.setAdmin(false); + p.setExecutive(false); + p.setAccessLevel(level); + User u = new User(); + u.setUsername(name); + u.setFname(name); + u.setLname("t"); + u.setEmail(name + "@example.test"); + u.setTeam(team); + u.setPermissions(p); + persist(em, u); + return u; + } + + private static void persist(EntityManager em, Object entity) { + HibHelper.getInstance().preJoin(); + em.joinTransaction(); + em.persist(entity); + HibHelper.getInstance().commit(); + } + + private static void remove(EntityManager em, Object entity) { + HibHelper.getInstance().preJoin(); + em.joinTransaction(); + em.remove(em.contains(entity) ? entity : em.merge(entity)); + HibHelper.getInstance().commit(); + } +} diff --git a/test/com/fuse/utils/PasswordResetsTest.java b/test/com/fuse/utils/PasswordResetsTest.java new file mode 100644 index 00000000..0065acb0 --- /dev/null +++ b/test/com/fuse/utils/PasswordResetsTest.java @@ -0,0 +1,160 @@ +package com.fuse.utils; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.util.Date; +import java.util.List; +import java.util.UUID; + +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import com.fuse.api.MongoTestBase; +import com.fuse.dao.HibHelper; +import com.fuse.dao.PasswordReset; +import com.fuse.dao.User; + +/** + * Password reset / invitation tokens: bound to the target user, expiring, and + * one-at-a-time per user. Issued by sendReset, the admin "add user" page and the + * users API; redeemed by Register. + */ +public class PasswordResetsTest extends MongoTestBase { + + private static EntityManagerFactory emf; + private static String tag; + private static User target, admin; + + @BeforeClass + public static void seed() { + emf = HibHelper.getInstance().getEMF(); + org.junit.Assume.assumeNotNull("EntityManagerFactory unavailable - skipping integration test", emf); + tag = UUID.randomUUID().toString().substring(0, 8); + EntityManager em = emf.createEntityManager(); + try { + target = user(em, "target-" + tag); + admin = user(em, "admin-" + tag); + } finally { + em.close(); + } + } + + @AfterClass + public static void cleanup() { + if (emf == null) + return; + EntityManager em = emf.createEntityManager(); + try { + for (PasswordReset r : (List) em.createQuery("from PasswordReset").getResultList()) { + if (r.getUser() != null && (r.getUser().getId() == target.getId() || r.getUser().getId() == admin.getId())) + remove(em, r); + } + remove(em, em.find(User.class, target.getId())); + remove(em, em.find(User.class, admin.getId())); + } finally { + em.close(); + } + } + + @Test + public void issuedTokenIsBoundToTheTargetUserNotTheCaller() { + EntityManager em = emf.createEntityManager(); + try { + String key = PasswordResets.issue(em, target, PasswordResets.INVITE_TTL_MILLIS); + PasswordReset reset = PasswordResets.redeem(em, key); + assertNotNull(reset); + assertEquals(target.getId(), reset.getUser().getId()); + assertNotEquals(admin.getId(), reset.getUser().getId()); + assertNotNull("tokens carry an expiry", reset.getExpires()); + assertTrue(reset.getExpires().after(new Date())); + } finally { + em.close(); + } + } + + @Test + public void issuingANewTokenInvalidatesTheOlderOne() { + EntityManager em = emf.createEntityManager(); + try { + String first = PasswordResets.issue(em, target, PasswordResets.RESET_TTL_MILLIS); + String second = PasswordResets.issue(em, target, PasswordResets.RESET_TTL_MILLIS); + assertNull("superseded token must stop working", PasswordResets.redeem(em, first)); + assertNotNull(PasswordResets.redeem(em, second)); + } finally { + em.close(); + } + } + + @Test + public void expiredTokenIsRejectedAndRemoved() { + EntityManager em = emf.createEntityManager(); + try { + String key = PasswordResets.issue(em, target, -1000L); // already expired + assertNull(PasswordResets.redeem(em, key)); + assertTrue("expired token is cleaned up", + em.createQuery("from PasswordReset where key = :k").setParameter("k", key).getResultList().isEmpty()); + } finally { + em.close(); + } + } + + @Test + public void legacyTokenWithoutExpiryIsTreatedAsExpiringAnHourAfterCreation() { + EntityManager em = emf.createEntityManager(); + try { + PasswordReset legacy = new PasswordReset(); + legacy.setKey("legacy-" + tag); + legacy.setUser(target); + legacy.setCreated(new Date(System.currentTimeMillis() - 2 * 60 * 60 * 1000L)); // two hours old + persist(em, legacy); + assertNull(PasswordResets.redeem(em, "legacy-" + tag)); + } finally { + em.close(); + } + } + + @Test + public void unknownTokenIsRejected() { + EntityManager em = emf.createEntityManager(); + try { + assertNull(PasswordResets.redeem(em, "nope-" + tag)); + assertNull(PasswordResets.redeem(em, null)); + } finally { + em.close(); + } + } + + private static User user(EntityManager em, String name) { + User u = new User(); + u.setUsername(name); + u.setFname(name); + u.setLname("t"); + u.setEmail(name + "@example.test"); + persist(em, u); + return u; + } + + private static void persist(EntityManager em, Object entity) { + HibHelper.getInstance().preJoin(); + em.joinTransaction(); + em.persist(entity); + HibHelper.getInstance().commit(); + } + + private static void remove(EntityManager em, Object entity) { + if (entity == null) + return; + HibHelper.getInstance().preJoin(); + em.joinTransaction(); + em.remove(em.contains(entity) ? entity : em.merge(entity)); + HibHelper.getInstance().commit(); + } +} diff --git a/test/com/fuse/utils/PublicBaseUrlTest.java b/test/com/fuse/utils/PublicBaseUrlTest.java new file mode 100644 index 00000000..9cae66ff --- /dev/null +++ b/test/com/fuse/utils/PublicBaseUrlTest.java @@ -0,0 +1,49 @@ +package com.fuse.utils; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import javax.servlet.http.HttpServletRequest; + +import org.junit.After; +import org.junit.Test; + +/** + * Emailed links (password reset, invitations) must not be built from the + * request's Host header, which an attacker controls. They come from the + * configured public base URL, with the request as a last resort. + */ +public class PublicBaseUrlTest { + + @After + public void clearProps() { + System.clearProperty("FACTION_BASE_URL"); + System.clearProperty("FACTION_OAUTH_CALLBACK"); + } + + private static HttpServletRequest request() { + HttpServletRequest r = mock(HttpServletRequest.class); + when(r.getRequestURL()).thenReturn(new StringBuffer("http://attacker.example/portal/sendReset")); + when(r.getRequestURI()).thenReturn("/portal/sendReset"); + when(r.getContextPath()).thenReturn(""); + return r; + } + + @Test + public void configuredBaseUrlWinsOverTheHostHeader() { + System.setProperty("FACTION_BASE_URL", "https://faction.example.com/"); + assertEquals("https://faction.example.com", FSUtils.publicBaseUrl(request())); + } + + @Test + public void oauthCallbackIsUsedWhenNoBaseUrlIsConfigured() { + System.setProperty("FACTION_OAUTH_CALLBACK", "https://sso.example.com"); + assertEquals("https://sso.example.com", FSUtils.publicBaseUrl(request())); + } + + @Test + public void fallsBackToTheRequestOnlyWhenNothingIsConfigured() { + assertEquals("http://attacker.example", FSUtils.publicBaseUrl(request())); + } +} From 56ae1728d2846209677f20f203663ff6d0c6ea3d Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 3 Oct 2026 01:08:14 -0500 Subject: [PATCH 2/2] Serve Register from the root namespace so emailed links work when logged out Since 1.8.8 the AccessControlInterceptor only admits unauthenticated requests to the "/" and "/sso" namespaces, but Register stayed in /portal, so every password reset and invitation link redirected to the login page. Register now lives in "/" with login, reset and sendReset, the emailed links point at /Register, and RegisterReachabilityTest pins the mapping. --- src/com/fuse/actions/Login.java | 2 +- src/com/fuse/actions/Register.java | 6 +++-- src/com/fuse/actions/admin/Users.java | 2 +- src/com/fuse/api/users.java | 2 +- .../actions/RegisterReachabilityTest.java | 26 +++++++++++++++++++ 5 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 test/com/fuse/actions/RegisterReachabilityTest.java diff --git a/src/com/fuse/actions/Login.java b/src/com/fuse/actions/Login.java index a3dd7bed..1bc3bf94 100644 --- a/src/com/fuse/actions/Login.java +++ b/src/com/fuse/actions/Login.java @@ -385,7 +385,7 @@ public String sendReset() { String message = "Hello " + u.getFname() + " " + u.getLname() + "

"; message += "Click the link below to reset your password. The link expires in one hour.

"; // Built from the configured public address, never from the request's Host header. - String url = FSUtils.publicBaseUrl(request) + "/portal/Register?uid=" + key; + String url = FSUtils.publicBaseUrl(request) + "/Register?uid=" + key; message += "Click here to Reset
"; EmailThread emailThread = new EmailThread(u.getEmail(), "Password Reset", message); TaskQueueExecutor.getInstance().execute(emailThread); diff --git a/src/com/fuse/actions/Register.java b/src/com/fuse/actions/Register.java index 0835c96f..287cb824 100644 --- a/src/com/fuse/actions/Register.java +++ b/src/com/fuse/actions/Register.java @@ -14,7 +14,9 @@ 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 { @@ -25,7 +27,7 @@ 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 = PasswordResets.redeem(em, uid); diff --git a/src/com/fuse/actions/admin/Users.java b/src/com/fuse/actions/admin/Users.java index 788d83a3..1a6c5089 100644 --- a/src/com/fuse/actions/admin/Users.java +++ b/src/com/fuse/actions/admin/Users.java @@ -254,7 +254,7 @@ public String addUser() { if (invite) { String key = PasswordResets.issue(em, u, PasswordResets.INVITE_TTL_MILLIS); - message += "Click here to Register
"; + message += "Click here to Register
"; } EmailThread emailThread = new EmailThread(this.email, "New Account Created", message); TaskQueueExecutor.getInstance().execute(emailThread); diff --git a/src/com/fuse/api/users.java b/src/com/fuse/api/users.java index fa2d2203..b08ade15 100644 --- a/src/com/fuse/api/users.java +++ b/src/com/fuse/api/users.java @@ -117,7 +117,7 @@ public Response createUser( String key = PasswordResets.issue(em, newUser, PasswordResets.INVITE_TTL_MILLIS); String message = "Hello " + fname + " " + lname + "

"; message += "Click the link below to set your password. The link expires in 72 hours:

"; - String url = FSUtils.publicBaseUrl(req) + "/portal/Register?uid=" + key; + String url = FSUtils.publicBaseUrl(req) + "/Register?uid=" + key; message += "Click here to Register
"; TaskQueueExecutor.getInstance().execute(new EmailThread(email, "New Account Created", message)); } diff --git a/test/com/fuse/actions/RegisterReachabilityTest.java b/test/com/fuse/actions/RegisterReachabilityTest.java new file mode 100644 index 00000000..81501d61 --- /dev/null +++ b/test/com/fuse/actions/RegisterReachabilityTest.java @@ -0,0 +1,26 @@ +package com.fuse.actions; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import org.apache.struts2.junit.StrutsJUnit4TestCase; +import org.junit.Test; + +import com.opensymphony.xwork2.ActionProxy; + +/** + * Password reset and invitation links are opened by people without a session, + * so Register must live in the root namespace, the only one (besides /sso) the + * AccessControlInterceptor lets through unauthenticated. It sat in /portal from + * 1.8.8, which redirected every emailed link to the login page. + */ +public class RegisterReachabilityTest extends StrutsJUnit4TestCase { + + @Test + public void registerIsMappedInTheRootNamespace() throws Exception { + ActionProxy proxy = getActionProxy("/Register"); + assertNotNull("/Register must resolve", proxy); + assertEquals("/", proxy.getNamespace()); + assertEquals(Register.class, proxy.getAction().getClass()); + } +}