diff --git a/src/main/java/org/commcare/formplayer/application/FormController.java b/src/main/java/org/commcare/formplayer/application/FormController.java index 9d6496cfe..bad7ba92b 100644 --- a/src/main/java/org/commcare/formplayer/application/FormController.java +++ b/src/main/java/org/commcare/formplayer/application/FormController.java @@ -257,7 +257,8 @@ private FormEntryResponseBean saveAnswer(AnswerQuestionRequestBean answerQuestio @UserRestore @ConfigureStorageFromSession public FormEntryResponseBean newRepeat(@RequestBody RepeatRequestBean newRepeatRequestBean, - @CookieValue(Constants.POSTGRES_DJANGO_SESSION_ID) String authToken) throws Exception { + @CookieValue(name = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken) + throws Exception { SerializableFormSession serializableFormSession = formSessionService.getSessionById( newRepeatRequestBean.getSessionId()); FormSession formEntrySession = formSessionFactory.getFormSession(serializableFormSession, newRepeatRequestBean.getWindowWidth()); diff --git a/src/main/java/org/commcare/formplayer/application/FormSessionFactory.java b/src/main/java/org/commcare/formplayer/application/FormSessionFactory.java index 1af7b43d1..e6d70a23b 100644 --- a/src/main/java/org/commcare/formplayer/application/FormSessionFactory.java +++ b/src/main/java/org/commcare/formplayer/application/FormSessionFactory.java @@ -1,5 +1,7 @@ package org.commcare.formplayer.application; +import org.commcare.formplayer.beans.auth.HqUserDetailsBean; +import org.commcare.formplayer.exceptions.FormNotFoundException; import org.commcare.formplayer.objects.SerializableFormSession; import org.commcare.formplayer.services.FormDefinitionService; import org.commcare.formplayer.services.FormplayerRemoteInstanceFetcher; @@ -8,6 +10,8 @@ import org.commcare.formplayer.services.RestoreFactory; import org.commcare.formplayer.services.VirtualDataInstanceService; import org.commcare.formplayer.session.FormSession; +import org.commcare.formplayer.util.RequestUtils; +import org.commcare.modern.database.TableBuilder; import org.commcare.session.CommCareSession; import org.javarosa.core.model.actions.FormSendCalloutHandler; import org.jetbrains.annotations.NotNull; @@ -15,6 +19,9 @@ import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; +import java.util.Objects; +import java.util.Optional; + @Component public class FormSessionFactory { @@ -40,6 +47,7 @@ public class FormSessionFactory { private CommCareSessionFactory commCareSessionFactory; public FormSession getFormSession(SerializableFormSession serializableFormSession, String windowWidth) throws Exception { + verifyPublicSessionOwnership(serializableFormSession); CommCareSession commCareSession = commCareSessionFactory.getCommCareSession(serializableFormSession.getMenuSessionId()); return getFormSession(serializableFormSession, commCareSession, windowWidth); } @@ -47,6 +55,7 @@ public FormSession getFormSession(SerializableFormSession serializableFormSessio @NotNull public FormSession getFormSession(SerializableFormSession serializableFormSession, @Nullable CommCareSession commCareSession, @Nullable String windowWidth) throws Exception { + verifyPublicSessionOwnership(serializableFormSession); FormplayerRemoteInstanceFetcher formplayerRemoteInstanceFetcher = new FormplayerRemoteInstanceFetcher( runnerService.getCaseSearchHelper(), virtualDataInstanceService); @@ -60,4 +69,24 @@ public FormSession getFormSession(SerializableFormSession serializableFormSessio windowWidth ); } + + /** + * A public web apps session may only operate on the form session that its own one-time link + * created. The session stores the scrubbed authoritative username, so any session bound to a + * different user/domain is rejected. No-op for non-public sessions and non-request contexts. + * + * Package-private for testing. + */ + void verifyPublicSessionOwnership(SerializableFormSession session) { + Optional userDetails = RequestUtils.getUserDetails(); + if (userDetails.isEmpty() || !userDetails.get().isPublicSession()) { + return; + } + HqUserDetailsBean details = userDetails.get(); + boolean owned = Objects.equals(session.getDomain(), details.getDomain()) + && Objects.equals(session.getUsername(), TableBuilder.scrubName(details.getUsername())); + if (!owned) { + throw new FormNotFoundException(session.getId()); + } + } } diff --git a/src/main/java/org/commcare/formplayer/application/MenuController.java b/src/main/java/org/commcare/formplayer/application/MenuController.java index 8898d7504..c2f228092 100644 --- a/src/main/java/org/commcare/formplayer/application/MenuController.java +++ b/src/main/java/org/commcare/formplayer/application/MenuController.java @@ -250,7 +250,7 @@ private static T setLocationNeeds(T res @UserRestore @AppInstall public BaseResponseBean navigateToEndpoint(@RequestBody SessionNavigationBean sessionNavigationBean, - @CookieValue(Constants.POSTGRES_DJANGO_SESSION_ID) String authToken, + @CookieValue(value = Constants.POSTGRES_DJANGO_SESSION_ID, required = false) String authToken, HttpServletRequest request) throws Exception { // Apps using aggressive syncs are likely to hit a sync whenever using endpoint-based navigation, // since they use it to jump between different sandboxes. Turn it off. diff --git a/src/main/java/org/commcare/formplayer/application/WebAppContext.java b/src/main/java/org/commcare/formplayer/application/WebAppContext.java index 3aebe2e06..a6360a372 100644 --- a/src/main/java/org/commcare/formplayer/application/WebAppContext.java +++ b/src/main/java/org/commcare/formplayer/application/WebAppContext.java @@ -12,6 +12,7 @@ import org.commcare.formplayer.aspects.LockAspect; import org.commcare.formplayer.aspects.LoggingAspect; import org.commcare.formplayer.aspects.MetricsAspect; +import org.commcare.formplayer.aspects.PublicSessionLockAspect; import org.commcare.formplayer.aspects.SetBrowserValuesAspect; import org.commcare.formplayer.aspects.TagTracingDisabledAspect; import org.commcare.formplayer.aspects.UserRestoreAspect; @@ -198,6 +199,11 @@ public AppInstallAspect appInstallAspect() { return new AppInstallAspect(); } + @Bean + public PublicSessionLockAspect publicSessionLockAspect() { + return new PublicSessionLockAspect(); + } + @Bean public ConfigureStorageFromSessionAspect configureStorageAspect() { return new ConfigureStorageFromSessionAspect(); diff --git a/src/main/java/org/commcare/formplayer/aspects/PublicSessionLockAspect.java b/src/main/java/org/commcare/formplayer/aspects/PublicSessionLockAspect.java new file mode 100644 index 000000000..ef3c5335d --- /dev/null +++ b/src/main/java/org/commcare/formplayer/aspects/PublicSessionLockAspect.java @@ -0,0 +1,104 @@ +package org.commcare.formplayer.aspects; + +import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.commcare.formplayer.beans.AuthenticatedRequestBean; +import org.commcare.formplayer.beans.InstallRequestBean; +import org.commcare.formplayer.beans.SessionNavigationBean; +import org.commcare.formplayer.beans.auth.HqUserDetailsBean; +import org.commcare.formplayer.util.RequestUtils; +import org.springframework.core.annotation.Order; +import org.springframework.util.StringUtils; + +import java.util.Objects; +import java.util.Optional; + +import lombok.extern.java.Log; + +/** + * Locks a public web apps session to the identity, app, and session endpoint that HQ bound its + * one-time link to, replacing the client-supplied values in the request with the + * HMAC-authenticated ones from session_details. Identity is pinned on every public request; the + * app and endpoint are pinned wherever an app is installed. + * + * Ordered ahead of every other formplayer aspect so that {@link LockAspect} derives its lock key, + * and {@link AppInstallAspect} keys the sandbox DB, from the authoritative values. + */ +@Aspect +@Order(0) +@Log +public class PublicSessionLockAspect { + + @Before(value = "@annotation(org.commcare.formplayer.annotations.UserRestore)") + public void pinPublicSessionIdentity(JoinPoint joinPoint) { + Optional userDetails = publicSessionDetails(); + if (userDetails.isEmpty()) { + return; + } + HqUserDetailsBean details = userDetails.get(); + Object[] args = joinPoint.getArgs(); + if (args.length == 0 || !(args[0] instanceof AuthenticatedRequestBean requestBean)) { + throw new IllegalStateException( + "Public web apps session reached a handler whose request identity cannot be " + + "pinned to the authenticated user"); + } + requestBean.setUsername(details.getUsername()); + requestBean.setDomain(details.getDomain()); + requestBean.setRestoreAs(null); + requestBean.setRestoreAsCaseId(null); + } + + @Before(value = "@annotation(org.commcare.formplayer.annotations.AppInstall)") + public void lockToPublicApp(JoinPoint joinPoint) { + Optional userDetails = publicSessionDetails(); + if (userDetails.isEmpty()) { + return; + } + HqUserDetailsBean details = userDetails.get(); + Object[] args = joinPoint.getArgs(); + if (args.length == 0 || !(args[0] instanceof InstallRequestBean requestBean)) { + // Fail closed: this only runs for a public session (non-public returned above), so an + // @AppInstall handler whose request we cannot lock must be rejected. + throw new IllegalStateException( + "Public web apps session reached an @AppInstall handler whose request cannot be " + + "locked to the authoritative app/endpoint"); + } + + // Fail closed: a public session must carry HQ's authoritative app id. A missing value means + // a misconfigured or out-of-date HQ; never fall back to the client-supplied app id. + if (!StringUtils.hasText(details.getPublicAppId())) { + throw new IllegalStateException( + "Public web apps session is missing an authoritative app id from HQ"); + } + if (!Objects.equals(requestBean.getAppId(), details.getPublicAppId())) { + log.warning("Public session request app id did not match the authoritative value; " + + "using the authoritative app id"); + } + requestBean.setAppId(details.getPublicAppId()); + requestBean.setPreview(false); + + if (requestBean instanceof SessionNavigationBean navigationBean) { + if (!StringUtils.hasText(details.getPublicEndpointId())) { + throw new IllegalStateException( + "Public web apps session is missing an authoritative endpoint id from HQ"); + } + boolean clientDiffers = !Objects.equals(navigationBean.getEndpointId(), + details.getPublicEndpointId()) + || (navigationBean.getEndpointArgs() != null + && !navigationBean.getEndpointArgs().isEmpty()); + if (clientDiffers) { + log.warning("Public session request endpoint/args did not match the authoritative " + + "endpoint; using the authoritative endpoint with no args"); + } + navigationBean.setEndpointId(details.getPublicEndpointId()); + // Public sessions have an empty restore, so endpoint args (e.g. case ids) cannot + // resolve; the designated public endpoint must take no required arguments. + navigationBean.setEndpointArgs(null); + } + } + + private Optional publicSessionDetails() { + return RequestUtils.getUserDetails().filter(HqUserDetailsBean::isPublicSession); + } +} diff --git a/src/main/java/org/commcare/formplayer/aspects/UserRestoreAspect.java b/src/main/java/org/commcare/formplayer/aspects/UserRestoreAspect.java index 5a9b46082..7fcd6779e 100644 --- a/src/main/java/org/commcare/formplayer/aspects/UserRestoreAspect.java +++ b/src/main/java/org/commcare/formplayer/aspects/UserRestoreAspect.java @@ -84,7 +84,15 @@ private void configureSentryScope(RestoreFactory restoreFactory) { }); } - private void configureRestoreFactory(AuthenticatedRequestBean requestBean, HqAuth auth) throws Exception { + // Package-private for testing. + void configureRestoreFactory(AuthenticatedRequestBean requestBean, HqAuth auth) throws Exception { + Optional userDetails = RequestUtils.getUserDetails(); + if (userDetails.isPresent() && userDetails.get().isPublicSession()) { + // A public web apps session restores only as its own user; never use client-supplied values + HqUserDetailsBean details = userDetails.get(); + restoreFactory.configure(details.getUsername(), details.getDomain(), null, auth); + return; + } if (requestBean.getRestoreAsCaseId() != null) { // SMS user filling out a form as a case restoreFactory.configure(requestBean.getDomain(), requestBean.getRestoreAsCaseId(), auth); diff --git a/src/main/java/org/commcare/formplayer/beans/auth/HqUserDetailsBean.java b/src/main/java/org/commcare/formplayer/beans/auth/HqUserDetailsBean.java index e6d2691c4..f6d9ec02d 100644 --- a/src/main/java/org/commcare/formplayer/beans/auth/HqUserDetailsBean.java +++ b/src/main/java/org/commcare/formplayer/beans/auth/HqUserDetailsBean.java @@ -35,6 +35,15 @@ public class HqUserDetailsBean implements UserDetails { @JsonProperty("public") private boolean publicSession; + // For a public web apps session, HQ returns the authoritative app and session endpoint the + // link is bound to. Formplayer uses these instead of the client-supplied values so a public + // session cannot navigate to any other app/form. Null for non-public sessions. + @JsonProperty("app_build_id") + private String publicAppId; + + @JsonProperty("endpoint_id") + private String publicEndpointId; + public HqUserDetailsBean() { } diff --git a/src/main/java/org/commcare/formplayer/configuration/WebSecurityConfig.java b/src/main/java/org/commcare/formplayer/configuration/WebSecurityConfig.java index 57df23abd..22286d6df 100644 --- a/src/main/java/org/commcare/formplayer/configuration/WebSecurityConfig.java +++ b/src/main/java/org/commcare/formplayer/configuration/WebSecurityConfig.java @@ -4,6 +4,7 @@ import org.commcare.formplayer.auth.CommCareSessionAuthFilter; import org.commcare.formplayer.auth.HmacAuthFilter; +import org.commcare.formplayer.beans.auth.HqUserDetailsBean; import org.commcare.formplayer.services.FormSessionService; import org.commcare.formplayer.services.HqUserDetailsService; import org.commcare.formplayer.util.Constants; @@ -18,6 +19,7 @@ import org.springframework.security.authorization.AuthenticatedAuthorizationManager; import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.core.Authentication; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer; @@ -30,7 +32,11 @@ import org.springframework.security.web.csrf.CookieCsrfTokenRepository; import org.springframework.security.web.csrf.CsrfTokenRequestAttributeHandler; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; + +import java.util.Arrays; @Configuration public class WebSecurityConfig { @@ -46,6 +52,30 @@ public class WebSecurityConfig { @Value("${formplayer.allowDoubleSlash:true}") private boolean allowDoubleSlash; + // The only routes a public web apps session may reach: get_endpoint to enter its locked form, + // plus every in-form action (endpoints that operate on an already-established form session). + // Every other route is denied (default-deny). + private static final String[] PUBLIC_SESSION_ALLOWED_URLS = { + Constants.URL_GET_ENDPOINT, + Constants.URL_ANSWER_QUESTION, + Constants.URL_ANSWER_MEDIA_QUESTION, + Constants.URL_CLEAR_ANSWER, + Constants.URL_NEW_REPEAT, + Constants.URL_DELETE_REPEAT, + Constants.URL_NEXT_INDEX, + Constants.URL_NEXT, + Constants.URL_PREV_INDEX, + Constants.URL_CURRENT, + Constants.URL_CHANGE_LANGUAGE, + Constants.URL_GET_INSTANCE, + Constants.URL_SUBMIT_FORM, + }; + + private final RequestMatcher allowedForPublicSession = new OrRequestMatcher( + Arrays.stream(PUBLIC_SESSION_ALLOWED_URLS) + .map(url -> new AntPathRequestMatcher("/" + url)) + .toArray(RequestMatcher[]::new)); + @Bean public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { disableDefaults(http); @@ -59,12 +89,15 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { .requestMatchers(new AntPathRequestMatcher("/serverup")).permitAll() .requestMatchers(new AntPathRequestMatcher("/favicon.ico")).permitAll() - // validate form auth + // validate form auth. Any route-specific rule added before anyRequest must be + // wrapped in restrictPublicSession, or a public session bypasses the allowlist here. .requestMatchers(new AntPathRequestMatcher("/validate_form")).access( - getFormValidationAuthManager()) + restrictPublicSession(getFormValidationAuthManager())) - // full auth required for all other requests - .anyRequest().authenticated() + // full auth required for all other requests; a public web apps session is + // further restricted to its allowlisted routes (default-deny) + .anyRequest().access( + restrictPublicSession(AuthenticatedAuthorizationManager.authenticated())) ); // Configure the authentication manager with a provider that will use the @@ -115,6 +148,31 @@ private AuthorizationManager getFormValidationAuthM }; } + /** + * Wraps an {@link AuthorizationManager} with the public web apps session route allowlist: a + * public session may reach only urls in {@code PUBLIC_SESSION_ALLOWED_URLS}; on every other + * route it is denied (default-deny), regardless of what {@code inner} decides. + * Non-public sessions get exactly {@code inner}'s decision. + */ + private AuthorizationManager restrictPublicSession( + AuthorizationManager inner) { + return (authentication, context) -> { + AuthorizationDecision authDecision = inner.check(authentication, context); + if (authDecision != null && authDecision.isGranted() + && isPublicSession(authentication.get()) + && !allowedForPublicSession.matches(context.getRequest())) { + return new AuthorizationDecision(false); + } + return authDecision; + }; + } + + private boolean isPublicSession(Authentication authentication) { + return authentication != null + && authentication.getPrincipal() instanceof HqUserDetailsBean bean + && bean.isPublicSession(); + } + private HmacAuthFilter getHmacAuthFilter() { return HmacAuthFilter.builder() .hmacKey(formplayerAuthKey) diff --git a/src/main/java/org/commcare/formplayer/services/MenuSessionRunnerService.java b/src/main/java/org/commcare/formplayer/services/MenuSessionRunnerService.java index 4dd8b6c94..1ec82380d 100644 --- a/src/main/java/org/commcare/formplayer/services/MenuSessionRunnerService.java +++ b/src/main/java/org/commcare/formplayer/services/MenuSessionRunnerService.java @@ -770,7 +770,10 @@ private NotificationMessage establishVolatility(FormSession session) { public BaseResponseBean advanceSessionWithEndpoint(MenuSession menuSession, String endpointId, @Nullable HashMap endpointArgs) throws Exception { - if (!FeatureFlagChecker.isToggleEnabled(TOGGLE_SESSION_ENDPOINTS)) { + // A public web apps session is a deep-link into a form itself; we use the underlying + // functionality of SESSION_ENDPOINTS without its toggle. + if (!FeatureFlagChecker.isToggleEnabled(TOGGLE_SESSION_ENDPOINTS) + && !RequestUtils.isPublicSession()) { throw new RuntimeException("Linking into applications has been disabled for this project."); } @@ -780,6 +783,11 @@ public BaseResponseBean advanceSessionWithEndpoint(MenuSession menuSession, Stri "This link does not exist. Your app may have changed so that the given link is no longer " + "valid"); } + // Endpoint args are stripped from a public session, so fail here rather than downstream + if (RequestUtils.isPublicSession() && !endpoint.getArguments().isEmpty()) { + throw new ApplicationConfigException( + "This link requires additional information and cannot be opened as a public link."); + } SessionWrapper sessionWrapper = menuSession.getSessionWrapper(); EvaluationContext evalContext = sessionWrapper.getEvaluationContext(); try { diff --git a/src/main/java/org/commcare/formplayer/util/RequestUtils.java b/src/main/java/org/commcare/formplayer/util/RequestUtils.java index 7f7f70fc4..dc58cb82e 100644 --- a/src/main/java/org/commcare/formplayer/util/RequestUtils.java +++ b/src/main/java/org/commcare/formplayer/util/RequestUtils.java @@ -101,13 +101,20 @@ public static HttpServletRequest getCurrentRequest() { public static Optional getUserDetails() { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication != null && !(authentication instanceof AnonymousAuthenticationToken)) { - HqUserDetailsBean userDetails = (HqUserDetailsBean)authentication.getPrincipal(); + if (authentication != null && !(authentication instanceof AnonymousAuthenticationToken) + && authentication.getPrincipal() instanceof HqUserDetailsBean userDetails) { return Optional.of(userDetails); } return Optional.empty(); } + /** + * @return True if the current request is authenticated as a public web apps session. + */ + public static boolean isPublicSession() { + return getUserDetails().map(HqUserDetailsBean::isPublicSession).orElse(false); + } + /** * @return True if there is request in the context AND the request was authenticated with HMAC * auth diff --git a/src/test/java/org/commcare/formplayer/application/FormSessionFactoryTest.java b/src/test/java/org/commcare/formplayer/application/FormSessionFactoryTest.java new file mode 100644 index 000000000..db3a6ee42 --- /dev/null +++ b/src/test/java/org/commcare/formplayer/application/FormSessionFactoryTest.java @@ -0,0 +1,116 @@ +package org.commcare.formplayer.application; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.commcare.formplayer.beans.auth.HqUserDetailsBean; +import org.commcare.formplayer.exceptions.FormNotFoundException; +import org.commcare.formplayer.objects.SerializableFormSession; +import org.commcare.formplayer.util.RequestUtils; +import org.commcare.modern.database.TableBuilder; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.springframework.test.util.ReflectionTestUtils; + +import java.util.Optional; + +/** + * Unit tests for {@link FormSessionFactory#verifyPublicSessionOwnership}: a public web apps session + * may only load the form session its own one-time link created, never another user's by id. + */ +public class FormSessionFactoryTest { + + private final FormSessionFactory factory = new FormSessionFactory(); + + private HqUserDetailsBean publicBean(String username, String domain) { + HqUserDetailsBean bean = new HqUserDetailsBean(domain, new String[]{domain}, username, + false, new String[]{}, new String[]{}); + bean.setPublicSession(true); + return bean; + } + + private SerializableFormSession session(String username, String domain) { + SerializableFormSession session = mock(SerializableFormSession.class); + // Sessions persist the scrubbed username (see FormSession's new-session constructor). + when(session.getUsername()).thenReturn(TableBuilder.scrubName(username)); + when(session.getDomain()).thenReturn(domain); + when(session.getId()).thenReturn("session-id"); + return session; + } + + @Test + public void publicSession_isRejectedBeforeTheForeignSessionIsLoaded() throws Exception { + CommCareSessionFactory commCareSessionFactory = mock(CommCareSessionFactory.class); + ReflectionTestUtils.setField(factory, "commCareSessionFactory", commCareSessionFactory); + SerializableFormSession victimSession = session("victim@domain", "domain"); + + try (MockedStatic mocked = mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("public_abc123@domain", "domain"))); + assertThrows(FormNotFoundException.class, + () -> factory.getFormSession(victimSession, "800")); + } + + // getCommCareSession runs configureApplication, which can delete that user's app DB. + verify(commCareSessionFactory, never()).getCommCareSession(any()); + } + + @Test + public void publicSession_cannotLoadAnotherUsersFormSession() { + SerializableFormSession victimSession = session("victim@domain", "domain"); + try (MockedStatic mocked = mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("public_abc@domain", "domain"))); + assertThrows(FormNotFoundException.class, + () -> factory.verifyPublicSessionOwnership(victimSession)); + } + } + + @Test + public void publicSession_cannotLoadFormSessionInAnotherDomain() { + SerializableFormSession otherDomainSession = session("public_abc@other", "other"); + try (MockedStatic mocked = mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("public_abc@domain", "domain"))); + assertThrows(FormNotFoundException.class, + () -> factory.verifyPublicSessionOwnership(otherDomainSession)); + } + } + + @Test + public void publicSession_canLoadItsOwnFormSession() { + SerializableFormSession ownSession = session("public_abc@domain", "domain"); + try (MockedStatic mocked = mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("public_abc@domain", "domain"))); + // Its own session passes the check (no exception). + factory.verifyPublicSessionOwnership(ownSession); + } + } + + @Test + public void nonPublicSession_ownershipCheckIsSkipped() { + // A regular session is unaffected: the check only constrains public sessions. + SerializableFormSession anySession = session("someone-else@domain", "domain"); + try (MockedStatic mocked = mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(new HqUserDetailsBean("domain", "user"))); + factory.verifyPublicSessionOwnership(anySession); + } + } + + @Test + public void noAuthenticatedUser_ownershipCheckIsSkipped() { + // Non-request / unauthenticated contexts (e.g. purge tasks) must not be blocked. + SerializableFormSession anySession = session("someone@domain", "domain"); + try (MockedStatic mocked = mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails).thenReturn(Optional.empty()); + factory.verifyPublicSessionOwnership(anySession); + } + } +} diff --git a/src/test/java/org/commcare/formplayer/aspects/PublicSessionLockAspectTest.java b/src/test/java/org/commcare/formplayer/aspects/PublicSessionLockAspectTest.java new file mode 100644 index 000000000..a5004dfed --- /dev/null +++ b/src/test/java/org/commcare/formplayer/aspects/PublicSessionLockAspectTest.java @@ -0,0 +1,272 @@ +package org.commcare.formplayer.aspects; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.aspectj.lang.JoinPoint; +import org.commcare.formplayer.beans.InstallRequestBean; +import org.commcare.formplayer.beans.SessionNavigationBean; +import org.commcare.formplayer.beans.SessionRequestBean; +import org.commcare.formplayer.beans.auth.HqUserDetailsBean; +import org.commcare.formplayer.util.RequestUtils; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.util.HashMap; +import java.util.Optional; + +/** + * Unit tests for {@link PublicSessionLockAspect}: a public web apps session must navigate the + * HQ-authoritative app/endpoint, never the client-supplied values. + */ +public class PublicSessionLockAspectTest { + + private final PublicSessionLockAspect aspect = new PublicSessionLockAspect(); + + private HqUserDetailsBean publicBean(String publicAppId, String publicEndpointId) { + HqUserDetailsBean bean = new HqUserDetailsBean("domain", new String[]{"domain"}, "user", + false, new String[]{}, new String[]{}); + bean.setPublicSession(true); + bean.setPublicAppId(publicAppId); + bean.setPublicEndpointId(publicEndpointId); + return bean; + } + + private SessionNavigationBean navBean(String appId, String endpointId, HashMap args) { + SessionNavigationBean bean = new SessionNavigationBean(); + bean.setAppId(appId); + bean.setEndpointId(endpointId); + bean.setEndpointArgs(args); + return bean; + } + + private JoinPoint joinPointForNoArgs() { + JoinPoint joinPoint = mock(JoinPoint.class); + when(joinPoint.getArgs()).thenReturn(new Object[]{}); + return joinPoint; + } + + private JoinPoint joinPointFor(Object bean) { + JoinPoint joinPoint = mock(JoinPoint.class); + when(joinPoint.getArgs()).thenReturn(new Object[]{bean, "token", null}); + return joinPoint; + } + + @Test + public void publicSession_overridesClientAppEndpointAndClearsArgs() { + HashMap args = new HashMap<>(); + args.put("case_id", "abc"); + SessionNavigationBean bean = navBean("attacker-app", "attacker-endpoint", args); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", "real-endpoint"))); + aspect.lockToPublicApp(joinPointFor(bean)); + } + + assertEquals("real-app", bean.getAppId()); + assertEquals("real-endpoint", bean.getEndpointId()); + assertNull(bean.getEndpointArgs()); + } + + @Test + public void publicSession_pinsIdentityOnRoutesThatInstallNoApp() { + // A form-entry route such as /answer is not @AppInstall, and its username feeds the + // @UserLock key, so identity has to be pinned there too. + SessionRequestBean bean = new SessionRequestBean(); + bean.setUsername("realuser@domain"); + bean.setDomain("attacker-domain"); + bean.setRestoreAs("victim"); + bean.setRestoreAsCaseId("victim-case-id"); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", "real-endpoint"))); + aspect.pinPublicSessionIdentity(joinPointFor(bean)); + } + + assertEquals("user", bean.getUsername()); + assertEquals("domain", bean.getDomain()); + assertNull(bean.getRestoreAs()); + assertNull(bean.getRestoreAsCaseId()); + } + + @Test + public void publicSession_forcesPreviewOff() { + SessionNavigationBean bean = navBean("real-app", "real-endpoint", null); + bean.setPreview(true); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", "real-endpoint"))); + aspect.lockToPublicApp(joinPointFor(bean)); + } + + assertFalse(bean.getPreview()); + } + + @Test + public void publicSession_unpinnableIdentityArg_failsClosed() { + JoinPoint joinPoint = joinPointFor("not-a-request-bean"); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", "real-endpoint"))); + assertThrows(IllegalStateException.class, + () -> aspect.pinPublicSessionIdentity(joinPoint)); + } + } + + @Test + public void nonPublicSession_identityPinningIsSkipped() { + SessionRequestBean bean = new SessionRequestBean(); + bean.setUsername("realuser@domain"); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(new HqUserDetailsBean("domain", "user"))); + aspect.pinPublicSessionIdentity(joinPointFor(bean)); + } + + assertEquals("realuser@domain", bean.getUsername()); + } + + @Test + public void publicSession_noRequestArgs_failsClosed() { + JoinPoint identityJoinPoint = joinPointForNoArgs(); + JoinPoint appJoinPoint = joinPointForNoArgs(); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", "real-endpoint"))); + assertThrows(IllegalStateException.class, + () -> aspect.pinPublicSessionIdentity(identityJoinPoint)); + assertThrows(IllegalStateException.class, () -> aspect.lockToPublicApp(appJoinPoint)); + } + } + + @Test + public void publicSession_installWithoutNavigation_pinsAppOnly() { + InstallRequestBean bean = new InstallRequestBean(); + bean.setAppId("attacker-app"); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", "real-endpoint"))); + aspect.lockToPublicApp(joinPointFor(bean)); + } + + assertEquals("real-app", bean.getAppId()); + } + + @Test + public void publicSession_endpointAlreadyAuthoritativeWithNoArgs_isUnchanged() { + SessionNavigationBean bean = navBean("real-app", "real-endpoint", null); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", "real-endpoint"))); + aspect.lockToPublicApp(joinPointFor(bean)); + } + + assertEquals("real-endpoint", bean.getEndpointId()); + assertNull(bean.getEndpointArgs()); + } + + @Test + public void publicSession_endpointAuthoritativeWithEmptyArgs_isUnchanged() { + SessionNavigationBean bean = navBean("real-app", "real-endpoint", new HashMap<>()); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", "real-endpoint"))); + aspect.lockToPublicApp(joinPointFor(bean)); + } + + assertNull(bean.getEndpointArgs()); + } + + @Test + public void publicSession_endpointAuthoritativeButArgsSupplied_clearsArgs() { + HashMap args = new HashMap<>(); + args.put("case_id", "abc"); + SessionNavigationBean bean = navBean("real-app", "real-endpoint", args); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", "real-endpoint"))); + aspect.lockToPublicApp(joinPointFor(bean)); + } + + assertNull(bean.getEndpointArgs()); + } + + @Test + public void nonPublicSession_leavesBeanUntouched() { + SessionNavigationBean bean = navBean("client-app", "client-endpoint", null); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + // publicSession defaults to false + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(new HqUserDetailsBean("domain", "user"))); + aspect.lockToPublicApp(joinPointFor(bean)); + } + + assertEquals("client-app", bean.getAppId()); + assertEquals("client-endpoint", bean.getEndpointId()); + } + + @Test + public void publicSession_missingAuthoritativeApp_failsClosed() { + SessionNavigationBean bean = navBean("client-app", "client-endpoint", null); + JoinPoint joinPoint = joinPointFor(bean); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean(null, "real-endpoint"))); + assertThrows(IllegalStateException.class, () -> aspect.lockToPublicApp(joinPoint)); + } + } + + @Test + public void publicSession_missingAuthoritativeEndpoint_failsClosed() { + SessionNavigationBean bean = navBean("client-app", "client-endpoint", null); + JoinPoint joinPoint = joinPointFor(bean); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", null))); + assertThrows(IllegalStateException.class, () -> aspect.lockToPublicApp(joinPoint)); + } + } + + @Test + public void publicSession_nonInstallRequestBeanArg_failsClosed() { + // A public session on an @AppInstall handler whose first arg cannot be locked must be + // rejected, not silently let through unlocked. + JoinPoint joinPoint = joinPointFor("not-an-install-request-bean"); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(publicBean("real-app", "real-endpoint"))); + assertThrows(IllegalStateException.class, () -> aspect.lockToPublicApp(joinPoint)); + } + } + + @Test + public void nonPublicSession_nonInstallRequestBeanArg_isIgnored() { + // The fail-closed guard is public-only: a non-public session on such a handler is untouched. + JoinPoint joinPoint = joinPointFor("not-an-install-request-bean"); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails) + .thenReturn(Optional.of(new HqUserDetailsBean("domain", "user"))); + aspect.lockToPublicApp(joinPoint); // no throw + } + } +} diff --git a/src/test/java/org/commcare/formplayer/aspects/PublicSessionLockAspectWeavingTest.java b/src/test/java/org/commcare/formplayer/aspects/PublicSessionLockAspectWeavingTest.java new file mode 100644 index 000000000..06a79f5b6 --- /dev/null +++ b/src/test/java/org/commcare/formplayer/aspects/PublicSessionLockAspectWeavingTest.java @@ -0,0 +1,121 @@ +package org.commcare.formplayer.aspects; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.commcare.formplayer.annotations.AppInstall; +import org.commcare.formplayer.application.MenuController; +import org.commcare.formplayer.beans.SessionNavigationBean; +import org.commcare.formplayer.beans.auth.HqUserDetailsBean; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.EnableAspectJAutoProxy; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; + +import java.util.Arrays; +import java.util.HashMap; + +/** + * Proves {@link PublicSessionLockAspect} actually intercepts {@code @AppInstall} handlers through + * real Spring AOP weaving. Catches a broken pointcut, missing weaving, or a lost bean. + * Complemented by two reflection guards for the wiring the aspect depends on. + */ +@SpringJUnitConfig(PublicSessionLockAspectWeavingTest.Config.class) +public class PublicSessionLockAspectWeavingTest { + + @Configuration + @EnableAspectJAutoProxy + static class Config { + @Bean + public PublicSessionLockAspect publicSessionLockAspect() { + return new PublicSessionLockAspect(); + } + + @Bean + public AppInstallHandler appInstallHandler() { + return new AppInstallHandler(); + } + } + + /** Stand-in for a controller: a Spring bean with an {@code @AppInstall} handler to weave into. */ + static class AppInstallHandler { + @AppInstall + public void install(SessionNavigationBean bean) { + // no-op; the aspect runs @Before this + } + } + + @Autowired + private AppInstallHandler handler; + + private void setPublicPrincipal(String appId, String endpointId) { + HqUserDetailsBean principal = new HqUserDetailsBean("domain", new String[]{"domain"}, "user", + false, new String[]{}, new String[]{}); + principal.setPublicSession(true); + principal.setPublicAppId(appId); + principal.setPublicEndpointId(endpointId); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(new PreAuthenticatedAuthenticationToken(principal, "creds", + principal.getAuthorities())); + SecurityContextHolder.setContext(context); + } + + @AfterEach + public void tearDown() { + SecurityContextHolder.clearContext(); + } + + @Test + public void aspectWeavesAndLocksPublicSessionToAuthoritativeAppEndpoint() { + setPublicPrincipal("real-app", "real-endpoint"); + HashMap args = new HashMap<>(); + args.put("case_id", "abc"); + SessionNavigationBean bean = new SessionNavigationBean(); + bean.setAppId("attacker-app"); + bean.setEndpointId("attacker-endpoint"); + bean.setEndpointArgs(args); + + // Call through the Spring proxy; if the aspect is woven it rewrites the bean @Before install. + handler.install(bean); + + assertEquals("real-app", bean.getAppId()); + assertEquals("real-endpoint", bean.getEndpointId()); + assertNull(bean.getEndpointArgs()); + } + + @Test + public void navigateToEndpointIsAnnotatedAppInstall() { + boolean annotated = Arrays.stream(MenuController.class.getDeclaredMethods()) + .filter(m -> m.getName().equals("navigateToEndpoint")) + .anyMatch(m -> m.isAnnotationPresent(AppInstall.class)); + assertTrue(annotated, + "get_endpoint (navigateToEndpoint) must stay @AppInstall so the lock aspect runs on it"); + } + + @Test + public void lockAspectOrderedAfterExposeInvocationButBeforeAppInstall() { + int lockOrder = orderOf(PublicSessionLockAspect.class); + int appInstallOrder = orderOf(AppInstallAspect.class); + // Must run after Spring's ExposeInvocationInterceptor (ordered HIGHEST_PRECEDENCE + 1) so the + // advice can read the JoinPoint, and before AppInstallAspect so the sandbox keys off the + // authoritative app id. + assertTrue(lockOrder > Ordered.HIGHEST_PRECEDENCE + 1, + "lock aspect must be ordered after ExposeInvocationInterceptor or it fails reading the JoinPoint"); + assertTrue(lockOrder < appInstallOrder, + "lock aspect must run before AppInstallAspect keys the sandbox off the app id"); + } + + private static int orderOf(Class aspect) { + Order order = aspect.getAnnotation(Order.class); + return order != null ? order.value() : Ordered.LOWEST_PRECEDENCE; + } +} diff --git a/src/test/java/org/commcare/formplayer/aspects/UserRestoreAspectTest.java b/src/test/java/org/commcare/formplayer/aspects/UserRestoreAspectTest.java index 25159311d..7ee354c1f 100644 --- a/src/test/java/org/commcare/formplayer/aspects/UserRestoreAspectTest.java +++ b/src/test/java/org/commcare/formplayer/aspects/UserRestoreAspectTest.java @@ -3,11 +3,19 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import org.commcare.formplayer.auth.DjangoAuth; import org.commcare.formplayer.auth.HqAuth; import org.commcare.formplayer.auth.PublicFormSessionAuth; +import org.commcare.formplayer.beans.AuthenticatedRequestBean; +import org.commcare.formplayer.beans.SessionNavigationBean; import org.commcare.formplayer.beans.auth.HqUserDetailsBean; +import org.commcare.formplayer.services.RestoreFactory; import org.commcare.formplayer.util.RequestUtils; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; @@ -86,4 +94,66 @@ public void noUserDetailsNoSessionToken_returnsNull() { assertNull(aspect.getHqAuth(null)); } } + + @Test + public void publicSession_restoresAsHqAuthenticatedUser_ignoringClientIdentity() throws Exception { + RestoreFactory restoreFactory = mock(RestoreFactory.class); + aspect.restoreFactory = restoreFactory; + HqAuth auth = new PublicFormSessionAuth("pkey"); + + // Client tries to restore as another user / case; all of it must be ignored. + SessionNavigationBean requestBean = new SessionNavigationBean(); + requestBean.setUsername("victim"); + requestBean.setDomain("other-domain"); + requestBean.setRestoreAs("victim"); + requestBean.setRestoreAsCaseId("case-123"); + + HqUserDetailsBean principal = bean(true, "pkey"); + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails).thenReturn(Optional.of(principal)); + aspect.configureRestoreFactory(requestBean, auth); + } + + // Restore is pinned to the principal's username+domain with no restore-as and no case id. + verify(restoreFactory).configure("user", "domain", null, auth); + verify(restoreFactory, never()).configure(any(AuthenticatedRequestBean.class), any()); + verify(restoreFactory, never()).configure(anyString(), anyString(), any()); + } + + @Test + public void noAuthenticatedUser_honorsRequestBeanIdentity() throws Exception { + RestoreFactory restoreFactory = mock(RestoreFactory.class); + aspect.restoreFactory = restoreFactory; + HqAuth auth = new DjangoAuth("sessionid-value"); + + SessionNavigationBean requestBean = new SessionNavigationBean(); + requestBean.setUsername("real-user"); + requestBean.setDomain("domain"); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails).thenReturn(Optional.empty()); + aspect.configureRestoreFactory(requestBean, auth); + } + + verify(restoreFactory).configure(requestBean, auth); + } + + @Test + public void regularSession_honorsRequestBeanIdentity() throws Exception { + RestoreFactory restoreFactory = mock(RestoreFactory.class); + aspect.restoreFactory = restoreFactory; + HqAuth auth = new DjangoAuth("sessionid-value"); + + SessionNavigationBean requestBean = new SessionNavigationBean(); + requestBean.setUsername("real-user"); + requestBean.setDomain("domain"); + + try (MockedStatic mocked = Mockito.mockStatic(RequestUtils.class)) { + mocked.when(RequestUtils::getUserDetails).thenReturn(Optional.of(bean(false, null))); + aspect.configureRestoreFactory(requestBean, auth); + } + + // Unchanged: a non-public session configures the restore from the request bean itself. + verify(restoreFactory).configure(requestBean, auth); + } } diff --git a/src/test/java/org/commcare/formplayer/auth/SessionAuthTests.java b/src/test/java/org/commcare/formplayer/auth/SessionAuthTests.java index b385eaf46..33996b1ab 100644 --- a/src/test/java/org/commcare/formplayer/auth/SessionAuthTests.java +++ b/src/test/java/org/commcare/formplayer/auth/SessionAuthTests.java @@ -1,6 +1,7 @@ package org.commcare.formplayer.auth; import static org.commcare.formplayer.auth.AuthTestUtils.getMultipartRequestBuilder; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -138,6 +139,96 @@ public void testEndpoint_PublicHeaderAndSessionCookie_PrefersPublicCredential() this.testEndpoint(builder, status().isOk()); } + /** + * A public web apps session is confined to its allowlisted routes: any other route is denied + * with 403 at the security layer, before the controller runs. (clear_user_data is not on the + * allowlist.) + */ + @Test + public void testPublicSession_DeniedOnNonAllowlistedRoute() throws Exception { + String sessionKey = "public-key-abc"; + mockValidPublicSessionAuth(sessionKey); + MockHttpServletRequestBuilder builder = getRequestBuilder(FULL_AUTH_BODY) + .header(Constants.PUBLIC_FORM_SESSION_HEADER, Constants.PUBLIC_FORM_SESSION_HEADER_VALUE) + .cookie(new Cookie(Constants.PUBLIC_FORM_SESSION_COOKIE_NAME, sessionKey)); + this.testEndpoint(builder, status().isForbidden()); + } + + /** + * A public web apps session is permitted on only specified routes. No controller for these is + * wired in this test, so an allowed request falls through (404) — the point is that the + * security layer does NOT deny it with 403. + */ + @Test + public void testPublicSession_AllowedOnAllowlistedRoutes() throws Exception { + String sessionKey = "public-key-abc"; + mockValidPublicSessionAuth(sessionKey); + // get_endpoint (enter the locked form) plus every in-form action a survey/registration form + // can invoke. None may be denied for a public session. + for (String path : new String[]{ + Constants.URL_GET_ENDPOINT, + Constants.URL_ANSWER_QUESTION, + Constants.URL_ANSWER_MEDIA_QUESTION, + Constants.URL_CLEAR_ANSWER, + Constants.URL_NEW_REPEAT, + Constants.URL_DELETE_REPEAT, + Constants.URL_NEXT_INDEX, + Constants.URL_NEXT, + Constants.URL_PREV_INDEX, + Constants.URL_CURRENT, + Constants.URL_CHANGE_LANGUAGE, + Constants.URL_GET_INSTANCE, + Constants.URL_SUBMIT_FORM}) { + MockHttpServletRequestBuilder builder = getRequestBuilder(path, FULL_AUTH_BODY) + .header(Constants.PUBLIC_FORM_SESSION_HEADER, Constants.PUBLIC_FORM_SESSION_HEADER_VALUE) + .cookie(new Cookie(Constants.PUBLIC_FORM_SESSION_COOKIE_NAME, sessionKey)); + this.testEndpoint(builder, result -> assertNotEquals(403, result.getResponse().getStatus(), + "Allowlisted route '" + path + "' must not be forbidden for a public session")); + } + } + + /** + * A public session may only enter its form at the HQ-assigned endpoint via get_endpoint; it must + * NOT be able to open an arbitrary form via new-form, which would bypass the endpoint lock. + */ + @Test + public void testPublicSession_DeniedOnNewForm() throws Exception { + String sessionKey = "public-key-abc"; + mockValidPublicSessionAuth(sessionKey); + MockHttpServletRequestBuilder builder = getRequestBuilder(Constants.URL_NEW_SESSION, FULL_AUTH_BODY) + .header(Constants.PUBLIC_FORM_SESSION_HEADER, Constants.PUBLIC_FORM_SESSION_HEADER_VALUE) + .cookie(new Cookie(Constants.PUBLIC_FORM_SESSION_COOKIE_NAME, sessionKey)); + this.testEndpoint(builder, status().isForbidden()); + } + + /** + * validate_form sits outside the public allowlist, so a public session is denied there too — + * even though validate_form's relaxed auth manager would otherwise grant any authenticated + * caller. Regression guard for the allowlist bypass. + */ + @Test + public void testPublicSession_DeniedOnValidateForm() throws Exception { + String sessionKey = "public-key-abc"; + mockValidPublicSessionAuth(sessionKey); + MockHttpServletRequestBuilder builder = getRequestBuilder(Constants.URL_VALIDATE_FORM, FULL_AUTH_BODY) + .header(Constants.PUBLIC_FORM_SESSION_HEADER, Constants.PUBLIC_FORM_SESSION_HEADER_VALUE) + .cookie(new Cookie(Constants.PUBLIC_FORM_SESSION_COOKIE_NAME, sessionKey)); + this.testEndpoint(builder, status().isForbidden()); + } + + /** + * A regular authenticated session is unaffected by the public allowlist on validate_form. + */ + @Test + public void testValidateForm_RegularSession_NotRestricted() throws Exception { + String sessionId = "123"; + mockValidAuth(sessionId); + MockHttpServletRequestBuilder builder = getRequestBuilder(Constants.URL_VALIDATE_FORM, FULL_AUTH_BODY) + .cookie(new Cookie(Constants.POSTGRES_DJANGO_SESSION_ID, sessionId)); + this.testEndpoint(builder, result -> assertNotEquals(403, result.getResponse().getStatus(), + "validate_form must not be forbidden for a regular authenticated session")); + } + @Test public void testMultipartEndpointWithFullAuth_WithAnyHmacAuth_Succeeds() throws Exception { String sessionId = "123"; @@ -156,6 +247,8 @@ private void mockValidAuth(String sessionId) { ); } + // Returns a non-public bean on purpose: these tests isolate the auth filter's credential + // routing from the route allowlist (which only restricts sessions whose bean is public). private void mockValidPublicAuth(String sessionKey) { PublicTokenMatcher matcher = new PublicTokenMatcher(DOMAIN, USERNAME, sessionKey); when(userDetailsService.loadUserDetails(argThat(matcher))).thenReturn( @@ -163,6 +256,13 @@ private void mockValidPublicAuth(String sessionKey) { ); } + private void mockValidPublicSessionAuth(String sessionKey) { + PublicTokenMatcher matcher = new PublicTokenMatcher(DOMAIN, USERNAME, sessionKey); + HqUserDetailsBean bean = new HqUserDetailsBean(DOMAIN, USERNAME); + bean.setPublicSession(true); + when(userDetailsService.loadUserDetails(argThat(matcher))).thenReturn(bean); + } + private void testEndpoint(MockHttpServletRequestBuilder requestBuilder, ResultMatcher... matchers) throws Exception { ResultActions actions = mvc.perform(requestBuilder) @@ -177,7 +277,11 @@ private void testEndpoint(MockHttpServletRequestBuilder requestBuilder, * Use the 'clear_user_data' endpoint for 'full auth' which required user details. */ private MockHttpServletRequestBuilder getRequestBuilder(String body) { - return post(String.format("/%s", Constants.URL_CLEAR_USER_DATA)) + return getRequestBuilder(Constants.URL_CLEAR_USER_DATA, body); + } + + private MockHttpServletRequestBuilder getRequestBuilder(String path, String body) { + return post(String.format("/%s", path)) .contentType(MediaType.APPLICATION_JSON) .content(body) .with(SecurityMockMvcRequestPostProcessors.csrf()); diff --git a/src/test/java/org/commcare/formplayer/tests/BaseTestClass.java b/src/test/java/org/commcare/formplayer/tests/BaseTestClass.java index 01e5517bb..98d9b953d 100644 --- a/src/test/java/org/commcare/formplayer/tests/BaseTestClass.java +++ b/src/test/java/org/commcare/formplayer/tests/BaseTestClass.java @@ -122,6 +122,7 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import org.springframework.test.web.servlet.ResultActions; import org.springframework.test.web.servlet.setup.MockMvcBuilders; @@ -841,6 +842,14 @@ T sessionNavigateWithEndpoint(String testName, String endpointId, HashMap endpointArgs, Class clazz) throws Exception { + return sessionNavigateWithEndpoint(testName, endpointId, endpointArgs, true, clazz); + } + + T sessionNavigateWithEndpoint(String testName, + String endpointId, + HashMap endpointArgs, + boolean withAuthCookie, + Class clazz) throws Exception { SessionNavigationBean sessionNavigationBean = new SessionNavigationBean(); sessionNavigationBean.setEndpointId(endpointId); if (endpointArgs != null) { @@ -854,6 +863,7 @@ T sessionNavigateWithEndpoint(String testName, RequestType.POST, Constants.URL_GET_ENDPOINT, sessionNavigationBean, + withAuthCookie, clazz); } @@ -947,6 +957,19 @@ private T generateMockQueryWithInstallReference(String installReference, ); } + private T generateMockQueryWithInstallReference(String installReference, + ControllerType controllerType, + RequestType requestType, + String urlPath, + Object bean, + boolean withAuthCookie, + Class clazz) throws Exception { + return Installer.mockInstallReference( + () -> generateMockQuery(controllerType, requestType, urlPath, bean, null, withAuthCookie, clazz), + installReference + ); + } + private T generateMockQuery(ControllerType controllerType, RequestType requestType, String urlPath, @@ -961,6 +984,16 @@ private T generateMockQuery(ControllerType controllerType, Object bean, MockMultipartFile file, Class clazz) throws Exception { + return generateMockQuery(controllerType, requestType, urlPath, bean, file, true, clazz); + } + + private T generateMockQuery(ControllerType controllerType, + RequestType requestType, + String urlPath, + Object bean, + MockMultipartFile file, + boolean withAuthCookie, + Class clazz) throws Exception { MockMvc controller = null; ResultActions result = null; @@ -1000,21 +1033,27 @@ private T generateMockQuery(ControllerType controllerType, break; } switch (requestType) { - case POST: - result = controller.perform( - post(urlPrepend(urlPath)) - .contentType(MediaType.APPLICATION_JSON) - .cookie(new Cookie(Constants.POSTGRES_DJANGO_SESSION_ID, "derp")) - .content((String)bean)); + case POST: { + MockHttpServletRequestBuilder builder = post(urlPrepend(urlPath)) + .contentType(MediaType.APPLICATION_JSON) + .content((String)bean); + if (withAuthCookie) { + builder.cookie(new Cookie(Constants.POSTGRES_DJANGO_SESSION_ID, "derp")); + } + result = controller.perform(builder); break; + } - case GET: - result = controller.perform( - get(urlPrepend(urlPath)) - .contentType(MediaType.APPLICATION_JSON) - .cookie(new Cookie(Constants.POSTGRES_DJANGO_SESSION_ID, "derp")) - .content((String)bean)); + case GET: { + MockHttpServletRequestBuilder builder = get(urlPrepend(urlPath)) + .contentType(MediaType.APPLICATION_JSON) + .content((String)bean); + if (withAuthCookie) { + builder.cookie(new Cookie(Constants.POSTGRES_DJANGO_SESSION_ID, "derp")); + } + result = controller.perform(builder); break; + } } restoreFactoryMock.getSQLiteDB().closeConnection(); storageFactoryMock.getSQLiteDB().closeConnection(); diff --git a/src/test/java/org/commcare/formplayer/tests/EndpointLaunchTest.java b/src/test/java/org/commcare/formplayer/tests/EndpointLaunchTest.java index 1b7ede6a9..dd4d7045b 100644 --- a/src/test/java/org/commcare/formplayer/tests/EndpointLaunchTest.java +++ b/src/test/java/org/commcare/formplayer/tests/EndpointLaunchTest.java @@ -3,6 +3,7 @@ import static org.commcare.formplayer.util.Constants.TOGGLE_SESSION_ENDPOINTS; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -14,6 +15,7 @@ import org.commcare.formplayer.beans.menus.CommandListResponseBean; import org.commcare.formplayer.beans.menus.PersistentCommand; import org.commcare.formplayer.beans.menus.CommandUtils.NavIconState; +import org.commcare.formplayer.exceptions.ApplicationConfigException; import org.commcare.formplayer.mocks.FormPlayerPropertyManagerMock; import org.commcare.formplayer.utils.FileUtils; import org.commcare.formplayer.utils.MockRequestUtils; @@ -140,6 +142,55 @@ public void testEndpoints() throws Exception { "f04bf0e8-2001-4885-a724-5497b34abe95"}); } + /** + * A public web apps session carries the public_form_session_key cookie, not the Django + * sessionid. Exercises the get_endpoint deep-link with NO sessionid cookie: it must reach the + * handler and navigate. + */ + @Test + @WithHqUser(enabledToggles = {TOGGLE_SESSION_ENDPOINTS}) + public void testEndpointLaunchWithoutSessionCookie() throws Exception { + NewFormResponse formResponse = sessionNavigateWithEndpoint(APP_NAME, + "add_parent", + null, + false, + NewFormResponse.class); + assert formResponse.getTitle().contentEquals("Add Parent"); + assertArrayEquals(formResponse.getSelections(), new String[]{"0", "0"}); + } + + /** + * A public web apps session must be able to launch its endpoint even when the domain does not + * have the SESSION_ENDPOINTS toggle enabled. No sessionid cookie, matching a real public + * session. The toggle-off + non-public case still throws, per testToggleOff. + */ + @Test + @WithHqUser(enabledToggles = {}, publicSession = true) + public void testEndpointLaunchForPublicSessionWithoutToggle() throws Exception { + NewFormResponse formResponse = sessionNavigateWithEndpoint(APP_NAME, + "add_parent", + null, + false, + NewFormResponse.class); + assert formResponse.getTitle().contentEquals("Add Parent"); + assertArrayEquals(formResponse.getSelections(), new String[]{"0", "0"}); + } + + /** + * A public link uses a single endpoint and its args are stripped, so reject early. + */ + @Test + @WithHqUser(enabledToggles = {}, publicSession = true) + public void testPublicSessionRejectsEndpointDeclaringArguments() { + ServletException exception = assertThrows(ServletException.class, + () -> sessionNavigateWithEndpoint(APP_NAME, + "followup", + null, + false, + NewFormResponse.class)); + assertInstanceOf(ApplicationConfigException.class, exception.getCause()); + } + @Test @WithHqUser(enabledToggles = {TOGGLE_SESSION_ENDPOINTS}) public void testEndpointsWithInlineCaseSearch() throws Exception { diff --git a/src/test/java/org/commcare/formplayer/tests/HqUserDetailsTests.java b/src/test/java/org/commcare/formplayer/tests/HqUserDetailsTests.java index 229549d47..bd5fca84d 100644 --- a/src/test/java/org/commcare/formplayer/tests/HqUserDetailsTests.java +++ b/src/test/java/org/commcare/formplayer/tests/HqUserDetailsTests.java @@ -70,19 +70,27 @@ public void testNonPublicSessionStillEnforcesUsername() { public void testPublicSessionDeserialization() throws Exception { ObjectMapper mapper = new ObjectMapper(); - // HQ sends the reserved word `public` for a public web apps session. + // HQ sends the reserved word `public` for a public web apps session, along with the + // authoritative app id and session endpoint the link is bound to. HqUserDetailsBean publicUser = mapper.readValue( - "{\"username\":\"pub\",\"public\":true}", HqUserDetailsBean.class); + "{\"username\":\"pub\",\"public\":true," + + "\"app_build_id\":\"app-1\",\"endpoint_id\":\"ep-1\"}", + HqUserDetailsBean.class); Assertions.assertTrue(publicUser.isPublicSession()); + Assertions.assertEquals("app-1", publicUser.getPublicAppId()); + Assertions.assertEquals("ep-1", publicUser.getPublicEndpointId()); HqUserDetailsBean regularUser = mapper.readValue( "{\"username\":\"reg\",\"public\":false}", HqUserDetailsBean.class); Assertions.assertFalse(regularUser.isPublicSession()); // Absent `public` defaults to false (primitive boolean; the bean also ignores unknowns). + // The app/endpoint fields are reference types, so they default to null. HqUserDetailsBean noField = mapper.readValue( "{\"username\":\"reg\"}", HqUserDetailsBean.class); Assertions.assertFalse(noField.isPublicSession()); + Assertions.assertNull(noField.getPublicAppId()); + Assertions.assertNull(noField.getPublicEndpointId()); } @Test diff --git a/src/test/java/org/commcare/formplayer/util/RequestUtilsTest.java b/src/test/java/org/commcare/formplayer/util/RequestUtilsTest.java new file mode 100644 index 000000000..21aa07ff7 --- /dev/null +++ b/src/test/java/org/commcare/formplayer/util/RequestUtilsTest.java @@ -0,0 +1,51 @@ +package org.commcare.formplayer.util; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.security.authentication.AnonymousAuthenticationToken; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.userdetails.User; + +public class RequestUtilsTest { + + @AfterEach + public void tearDown() { + SecurityContextHolder.clearContext(); + } + + @Test + public void getUserDetails_principalIsNotAnHqUser_isEmpty() { + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(new UsernamePasswordAuthenticationToken( + new User("someone", "pw", AuthorityUtils.NO_AUTHORITIES), "pw")); + SecurityContextHolder.setContext(context); + + assertTrue(RequestUtils.getUserDetails().isEmpty()); + assertFalse(RequestUtils.isPublicSession()); + } + + @Test + public void getUserDetails_anonymousAuthentication_isEmpty() { + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(new AnonymousAuthenticationToken("key", "anonymousUser", + AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS"))); + SecurityContextHolder.setContext(context); + + assertTrue(RequestUtils.getUserDetails().isEmpty()); + assertFalse(RequestUtils.isPublicSession()); + } + + @Test + public void getUserDetails_noAuthentication_isEmpty() { + SecurityContextHolder.clearContext(); + + assertTrue(RequestUtils.getUserDetails().isEmpty()); + assertFalse(RequestUtils.isPublicSession()); + } +} diff --git a/src/test/java/org/commcare/formplayer/utils/HqUserDetails.java b/src/test/java/org/commcare/formplayer/utils/HqUserDetails.java index 204abe815..873d4c70e 100644 --- a/src/test/java/org/commcare/formplayer/utils/HqUserDetails.java +++ b/src/test/java/org/commcare/formplayer/utils/HqUserDetails.java @@ -30,6 +30,7 @@ public class HqUserDetails { private boolean isSuperUser; private String[] enabledPreviews; private String[] enabledToggles; + private boolean publicSession; public HqUserDetails(WithHqUser withUser) { String username = StringUtils.hasLength(withUser.username()) ? withUser.username() @@ -42,9 +43,13 @@ public HqUserDetails(WithHqUser withUser) { this.isSuperUser = withUser.isSuperUser(); this.enabledPreviews = withUser.enabledPreviews(); this.enabledToggles = withUser.enabledToggles(); + this.publicSession = withUser.publicSession(); } public HqUserDetailsBean toBean() { - return new HqUserDetailsBean(domain, domains, username, isSuperUser, enabledToggles, enabledPreviews); + HqUserDetailsBean bean = new HqUserDetailsBean(domain, domains, username, isSuperUser, + enabledToggles, enabledPreviews); + bean.setPublicSession(publicSession); + return bean; } } diff --git a/src/test/java/org/commcare/formplayer/utils/WithHqUser.java b/src/test/java/org/commcare/formplayer/utils/WithHqUser.java index 1b3a4ff1b..f11b1b41c 100644 --- a/src/test/java/org/commcare/formplayer/utils/WithHqUser.java +++ b/src/test/java/org/commcare/formplayer/utils/WithHqUser.java @@ -92,4 +92,9 @@ * List of enabled toggles for the user. Defaults to a mock list of toggle_a and toggle_b */ String[] enabledToggles() default {"toggle_a", "toggle_b"}; + + /** + * Whether this is a public web apps session (one-time link). Defaults to false. + */ + boolean publicSession() default false; }