Validate session routes consistently

src/main/java/org/commcare/formplayer/annotations/ValidateSessionOwner.java

@@ -0,0 +1,20 @@
+package org.commcare.formplayer.annotations;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Marks a controller method that takes a
+ * {@link org.commcare.formplayer.beans.SessionRequestBean} argument (i.e. a request that addresses a
+ * form session by id). {@link org.commcare.formplayer.aspects.SessionOwnershipAspect} verifies,
+ * before the method runs, that the authenticated caller owns the referenced session.
+ *
+ * Every session-id-keyed route must carry this annotation; SessionOwnershipWiringTest enforces that
+ * structurally so a new route cannot silently omit the check.
+ */
+@Target(value = ElementType.METHOD)
+@Retention(value = RetentionPolicy.RUNTIME)
+public @interface ValidateSessionOwner {
+}

src/main/java/org/commcare/formplayer/application/DebuggerController.java

@@ -1,6 +1,7 @@
 package org.commcare.formplayer.application;
 
 import org.commcare.formplayer.annotations.AppInstall;
+import org.commcare.formplayer.annotations.ValidateSessionOwner;
 import org.commcare.formplayer.annotations.ConfigureStorageFromSession;
 import org.commcare.formplayer.annotations.UserLock;
 import org.commcare.formplayer.annotations.UserRestore;
@@ -65,6 +66,7 @@ public class DebuggerController extends AbstractBaseController {
     @RequestMapping(value = Constants.URL_DEBUGGER_FORMATTED_QUESTIONS, method = RequestMethod.POST)
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public DebuggerFormattedQuestionsResponseBean getFormattedQuesitons(
             @RequestBody SessionRequestBean debuggerRequest,
             @CookieValue(Constants.POSTGRES_DJANGO_SESSION_ID) String authToken) throws Exception {
@@ -150,6 +152,7 @@ public EvaluateXPathResponseBean menuEvaluateXpath(@RequestBody EvaluateXPathMen
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public EvaluateXPathResponseBean evaluateXpath(@RequestBody EvaluateXPathRequestBean evaluateXPathRequestBean,
                                                    @CookieValue(Constants.POSTGRES_DJANGO_SESSION_ID) String authToken) throws Exception {
         requireEditDataPermission();

src/main/java/org/commcare/formplayer/application/FormController.java

@@ -8,6 +8,7 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.commcare.formplayer.annotations.ConfigureStorageFromSession;
+import org.commcare.formplayer.annotations.ValidateSessionOwner;
 import org.commcare.formplayer.annotations.UserLock;
 import org.commcare.formplayer.annotations.UserRestore;
 import org.commcare.formplayer.api.json.JsonActionUtils;
@@ -134,6 +135,7 @@ public NewFormResponse newFormResponse(@RequestBody NewSessionRequestBean newSes
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public FormEntryResponseBean changeLocale(@RequestBody ChangeLocaleRequestBean changeLocaleBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken)
             throws Exception {
@@ -151,6 +153,7 @@ public FormEntryResponseBean changeLocale(@RequestBody ChangeLocaleRequestBean c
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public FormEntryResponseBean answerQuestion(@RequestBody AnswerQuestionRequestBean answerQuestionBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken)
             throws Exception {
@@ -165,6 +168,7 @@ public FormEntryResponseBean answerQuestion(@RequestBody AnswerQuestionRequestBe
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public FormEntryResponseBean answerMediaQuestion(
             @RequestPart(PART_ANSWER) AnswerQuestionRequestBean answerQuestionBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken,
@@ -177,6 +181,7 @@ public FormEntryResponseBean answerMediaQuestion(
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public FormEntryResponseBean clearAnswer(
             @RequestBody AnswerQuestionRequestBean answerQuestionBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken)
@@ -256,6 +261,7 @@ private FormEntryResponseBean saveAnswer(AnswerQuestionRequestBean answerQuestio
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public FormEntryResponseBean newRepeat(@RequestBody RepeatRequestBean newRepeatRequestBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken)
             throws Exception {
@@ -277,6 +283,7 @@ public FormEntryResponseBean newRepeat(@RequestBody RepeatRequestBean newRepeatR
     @ResponseBody
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public FormEntryResponseBean deleteRepeat(@RequestBody RepeatRequestBean deleteRepeatRequestBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken)
             throws Exception {
@@ -298,6 +305,7 @@ public FormEntryResponseBean deleteRepeat(@RequestBody RepeatRequestBean deleteR
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public FormEntryNavigationResponseBean getNext(@RequestBody SessionRequestBean requestBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken)
             throws Exception {
@@ -315,6 +323,7 @@ public FormEntryNavigationResponseBean getNext(@RequestBody SessionRequestBean r
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public FormEntryNavigationResponseBean getNextSms(@RequestBody SessionRequestBean requestBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken)
             throws Exception {
@@ -331,6 +340,7 @@ public FormEntryNavigationResponseBean getNextSms(@RequestBody SessionRequestBea
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public FormEntryNavigationResponseBean getPrevious(@RequestBody SessionRequestBean requestBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken)
             throws Exception {
@@ -352,6 +362,7 @@ public FormEntryNavigationResponseBean getPrevious(@RequestBody SessionRequestBe
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public GetInstanceResponseBean getRawInstance(@RequestBody SessionRequestBean requestBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken)
             throws Exception {
@@ -367,6 +378,7 @@ public GetInstanceResponseBean getRawInstance(@RequestBody SessionRequestBean re
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public FormEntryNavigationResponseBean getCurrent(@RequestBody SessionRequestBean requestBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken)
             throws Exception {

src/main/java/org/commcare/formplayer/application/FormSubmissionController.java

@@ -3,6 +3,7 @@
 import io.sentry.SentryLevel;
 
 import org.commcare.formplayer.annotations.ConfigureStorageFromSession;
+import org.commcare.formplayer.annotations.ValidateSessionOwner;
 import org.commcare.formplayer.annotations.UserLock;
 import org.commcare.formplayer.annotations.UserRestore;
 import org.commcare.formplayer.beans.SubmitRequestBean;
@@ -40,6 +41,7 @@ public class FormSubmissionController extends AbstractBaseController {
     @UserLock
     @UserRestore
     @ConfigureStorageFromSession
+    @ValidateSessionOwner
     public SubmitResponseBean submitForm(@RequestBody SubmitRequestBean submitRequestBean,
             @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken,
             HttpServletRequest request) throws Exception {

src/main/java/org/commcare/formplayer/application/IncompleteSessionController.java

@@ -1,6 +1,7 @@
 package org.commcare.formplayer.application;
 
 import org.commcare.formplayer.annotations.UserLock;
+import org.commcare.formplayer.annotations.ValidateSessionOwner;
 import org.commcare.formplayer.annotations.UserRestore;
 import org.commcare.formplayer.beans.FormsSessionsRequestBean;
 import org.commcare.formplayer.beans.GetSessionsResponse;
@@ -37,6 +38,7 @@ public class IncompleteSessionController extends AbstractBaseController {
     @RequestMapping(value = Constants.URL_INCOMPLETE_SESSION, method = RequestMethod.POST)
     @UserLock
     @UserRestore
+    @ValidateSessionOwner
     public NewFormResponse openIncompleteForm(@RequestBody SessionRequestBean incompleteSessionRequestBean,
                                               @CookieValue(Constants.POSTGRES_DJANGO_SESSION_ID) String authToken) throws Exception {
         SerializableFormSession session = formSessionService.getSessionById(incompleteSessionRequestBean.getSessionId());
@@ -66,6 +68,7 @@ public GetSessionsResponse getSessions(@RequestBody FormsSessionsRequestBean get
     }
 
     @RequestMapping(value = Constants.URL_DELETE_INCOMPLETE_SESSION, method = RequestMethod.POST)
+    @ValidateSessionOwner
     public NotificationMessage deleteIncompleteForm(
             @RequestBody SessionRequestBean incompleteSessionRequestBean,
             @CookieValue(Constants.POSTGRES_DJANGO_SESSION_ID) String authToken) throws Exception {

src/main/java/org/commcare/formplayer/application/WebAppContext.java

@@ -9,6 +9,7 @@
 
 import org.commcare.formplayer.aspects.AppInstallAspect;
 import org.commcare.formplayer.aspects.ConfigureStorageFromSessionAspect;
+import org.commcare.formplayer.aspects.SessionOwnershipAspect;
 import org.commcare.formplayer.aspects.LockAspect;
 import org.commcare.formplayer.aspects.LoggingAspect;
 import org.commcare.formplayer.aspects.MetricsAspect;
@@ -175,6 +176,11 @@ BrowserValuesProvider browserValuesProvider() {
         return new BrowserValuesProvider();
     }
 
+    @Bean
+    public SessionOwnershipAspect sessionOwnershipAspect() {
+        return new SessionOwnershipAspect();
+    }
+
     @Bean
     public LockAspect lockAspect() {
         return new LockAspect();

src/main/java/org/commcare/formplayer/aspects/SessionOwnershipAspect.java

@@ -0,0 +1,88 @@
+package org.commcare.formplayer.aspects;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.aspectj.lang.JoinPoint;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+import org.commcare.formplayer.beans.SessionRequestBean;
+import org.commcare.formplayer.exceptions.FormNotFoundException;
+import org.commcare.formplayer.objects.SerializableFormSession;
+import org.commcare.formplayer.services.FormSessionService;
+import org.commcare.formplayer.util.RequestUtils;
+import org.commcare.modern.database.TableBuilder;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.annotation.Order;
+
+import java.util.Arrays;
+
+/**
+ * Enforces that the authenticated caller owns the form session they address by id.
+ *
+ * <p>Formplayer authenticates the caller's username and domain (CommCareSessionAuthFilter ->
+ * HqUserDetailsService), but the by-id session lookups it relies on
+ * ({@link FormSessionService#getSessionById} / {@link FormSessionService#deleteSessionById}) resolve
+ * purely by primary key. Without this check any authenticated user can read and attempt to mutate
+ * another user's form session, including from a different domain, given only its id.
+ *
+ * <p>The caller's identity is taken from the request bean's username/domain, which the
+ * authentication filter has already bound to the authenticated session. This mirrors
+ * how {@link UserRestoreAspect} and {@link LockAspect} already derive identity from the bean.
+ *
+ * <p>Ordered after {@link PublicSessionLockAspect} (0), which pins a public session's bean identity
+ * to the authenticated principal, so this check compares against the pinned identity rather than the
+ * raw client value. Still ahead of {@link LockAspect} (2) and {@link UserRestoreAspect} (5) so a
+ * foreign session is refused before either derives a lock key or restore context from it.
+ */
+@Aspect
+@Order(1)
+public class SessionOwnershipAspect {
+
+    private static final Log log = LogFactory.getLog(SessionOwnershipAspect.class);
+
+    @Autowired
+    private FormSessionService formSessionService;
+
+    @Before(value = "@annotation(org.commcare.formplayer.annotations.ValidateSessionOwner)")
+    public void validateSessionOwner(JoinPoint joinPoint) {
+        // A handler binds at most one request body, so there is only ever one SessionRequestBean
+        // argument; find it wherever it sits rather than assuming a fixed position.
+        SessionRequestBean bean = Arrays.stream(joinPoint.getArgs())
+                .filter(arg -> arg instanceof SessionRequestBean)
+                .map(arg -> (SessionRequestBean)arg)
+                .findFirst()
+                .orElseThrow(() -> new RuntimeException(
+                        "Cannot validate session ownership: no SessionRequestBean argument on "
+                                + joinPoint.getSignature()));
+        String sessionId = bean.getSessionId();
+
+        // HMAC-signed requests (SMS and HQ-proxied Edit Forms) derive their identity from
+        // the session itself and are trusted via the shared HMAC key rather than by user
+        // credentials, so ownership is not meaningful for them.
+        if (RequestUtils.requestAuthedWithHmac()) {
+            return;
+        }
+
+        // For cookie-authenticated request, the HqUserDetailsService validates that the
+        // bean's username+domain exist and rejects the request before this point if
+        // either is missing. Treat their absence as unauthorized.
+        String callerUsername = bean.getUsername();
+        String callerDomain = bean.getDomain();
+        if (callerUsername == null || callerDomain == null) {
+            throw new FormNotFoundException(sessionId);
+        }
+
+        SerializableFormSession session = formSessionService.getSessionById(sessionId);
+        boolean owned = callerDomain.equals(session.getDomain())
+                && TableBuilder.scrubName(callerUsername).equals(session.getUsername());
+        if (!owned) {
+            log.warn(String.format(
+                    "Blocked user '%s' in domain '%s' from accessing form session %s owned by '%s' "
+                            + "in domain '%s'",
+                    TableBuilder.scrubName(callerUsername), callerDomain, sessionId,
+                    session.getUsername(), session.getDomain()));
+            // behaves the same as if the session does not exist
+            throw new FormNotFoundException(sessionId);
+        }
+    }
+}