-
Notifications
You must be signed in to change notification settings - Fork 10
Public Webforms: Authentication #1791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
787a76c
Route public web apps session credentials in formplayer auth
nospame ec9ec73
Accept HQ `public` field on HqUserDetailsBean
nospame 0e9f605
Skip username authorization for public web apps sessions
nospame 0388456
Attach public session credentials on outbound HQ calls
nospame File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
37 changes: 37 additions & 0 deletions
37
src/main/java/org/commcare/formplayer/auth/PublicFormSessionAuth.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| package org.commcare.formplayer.auth; | ||
|
|
||
| import org.commcare.formplayer.util.Constants; | ||
| import org.springframework.http.HttpHeaders; | ||
| import org.springframework.util.Assert; | ||
|
|
||
| /** | ||
| * {@link HqAuth} for a public web apps session. | ||
| * | ||
| * Emits the credential pair HQ requires to recognize a public session on its receiver/restore | ||
| * endpoints: the {@code public_form_session_key} cookie carrying the session key together with the | ||
| * {@code CommCare-Public-Session: true} header. | ||
| */ | ||
| public class PublicFormSessionAuth implements HqAuth { | ||
|
|
||
| private final String sessionKey; | ||
|
|
||
| public PublicFormSessionAuth(String sessionKey) { | ||
| Assert.hasText(sessionKey, "A public form session key is required"); | ||
| this.sessionKey = sessionKey; | ||
| } | ||
|
|
||
| @Override | ||
| public HttpHeaders getAuthHeaders() { | ||
| return new HttpHeaders() { | ||
| { | ||
| add("Cookie", Constants.PUBLIC_FORM_SESSION_COOKIE_NAME + "=" + sessionKey); | ||
| add(Constants.PUBLIC_FORM_SESSION_HEADER, Constants.PUBLIC_FORM_SESSION_HEADER_VALUE); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "PublicFormSessionAuth"; | ||
| } | ||
| } | ||
15 changes: 15 additions & 0 deletions
15
src/main/java/org/commcare/formplayer/auth/PublicSessionCredential.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package org.commcare.formplayer.auth; | ||
|
|
||
| import lombok.Value; | ||
|
|
||
| /** | ||
| * Typed credential for a public web apps session (one-time link). | ||
| * | ||
| * Wraps the value of the {@code public_form_session_key} cookie so that the | ||
| * {@link org.commcare.formplayer.services.HqUserDetailsService} can distinguish a public session | ||
| * from a regular Django session. | ||
| */ | ||
| @Value | ||
| public class PublicSessionCredential { | ||
| String sessionKey; | ||
| } |
31 changes: 31 additions & 0 deletions
31
src/main/java/org/commcare/formplayer/beans/auth/HqPublicSessionKeyBean.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package org.commcare.formplayer.beans.auth; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonIgnoreProperties; | ||
|
|
||
| import java.io.Serializable; | ||
|
|
||
| /** | ||
| * HMAC-signed body sent to HQ's session_details endpoint for a public web apps session. | ||
| * | ||
| * Serializes to {@code {"publicSessionKey": ..., "domain": ...}}. HQ treats a request with a | ||
| * truthy {@code publicSessionKey} as a public session lookup, in contrast to | ||
| * {@link HqSessionKeyBean} which sends {@code sessionId}. | ||
| */ | ||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| public class HqPublicSessionKeyBean implements Serializable { | ||
| private String publicSessionKey; | ||
| private String domain; | ||
|
|
||
| public HqPublicSessionKeyBean(String domain, String publicSessionKey) { | ||
| this.domain = domain; | ||
| this.publicSessionKey = publicSessionKey; | ||
| } | ||
|
|
||
| public String getPublicSessionKey() { | ||
| return publicSessionKey; | ||
| } | ||
|
|
||
| public String getDomain() { | ||
| return domain; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| package org.commcare.formplayer.services; | ||
|
|
||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import org.commcare.formplayer.auth.PublicSessionCredential; | ||
| import org.commcare.formplayer.auth.UserDomainPreAuthPrincipal; | ||
| import org.commcare.formplayer.beans.auth.HqPublicSessionKeyBean; | ||
| import org.commcare.formplayer.beans.auth.HqSessionKeyBean; | ||
| import org.commcare.formplayer.beans.auth.HqUserDetailsBean; | ||
| import org.commcare.formplayer.exceptions.SessionAuthUnavailableException; | ||
|
|
@@ -38,10 +40,23 @@ public class HqUserDetailsService implements AuthenticationUserDetailsService<Pr | |
| private WebClient webClient; | ||
|
|
||
| public HqUserDetailsBean getUserDetails(String domain, String sessionKey) { | ||
| return requestUserDetails(domain, new HqSessionKeyBean(domain, sessionKey)); | ||
| } | ||
|
|
||
| /** | ||
| * Look up user details for a public web apps session. Sends the session key as | ||
| * {@code publicSessionKey} rather than {@code sessionId} so HQ knows to resolves it against | ||
| * the public form session. | ||
| */ | ||
| public HqUserDetailsBean getPublicUserDetails(String domain, String publicSessionKey) { | ||
| return requestUserDetails(domain, new HqPublicSessionKeyBean(domain, publicSessionKey)); | ||
| } | ||
|
|
||
| private HqUserDetailsBean requestUserDetails(String domain, Object requestBody) { | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| String data = null; | ||
| String data; | ||
| try { | ||
| data = objectMapper.writeValueAsString(new HqSessionKeyBean(domain, sessionKey)); | ||
| data = objectMapper.writeValueAsString(requestBody); | ||
| headers.set("X-MAC-DIGEST", getHmac(data)); | ||
| } catch (Exception e) { | ||
| throw new UserDetailsException(e); | ||
|
|
@@ -73,9 +88,14 @@ private String getHmac(String data) throws Exception { | |
| @Override | ||
| public UserDetails loadUserDetails(PreAuthenticatedAuthenticationToken token) throws UsernameNotFoundException { | ||
| final UserDomainPreAuthPrincipal principal = (UserDomainPreAuthPrincipal) token.getPrincipal(); | ||
| final String sessionId = (String) token.getCredentials(); | ||
| final Object credentials = token.getCredentials(); | ||
| try { | ||
| HqUserDetailsBean userDetails = getUserDetails(principal.getDomain(), sessionId); | ||
| HqUserDetailsBean userDetails; | ||
| if (credentials instanceof PublicSessionCredential publicCredential) { | ||
| userDetails = getPublicUserDetails(principal.getDomain(), publicCredential.getSessionKey()); | ||
| } else { | ||
| userDetails = getUserDetails(principal.getDomain(), (String) credentials); | ||
| } | ||
| if (!userDetails.isAuthorized(principal.getDomain(), principal.getUsername())) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: I believe at this point |
||
| throw new UsernameNotFoundException("Unable to authenticate user in requested domain"); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
89 changes: 89 additions & 0 deletions
89
src/test/java/org/commcare/formplayer/aspects/UserRestoreAspectTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| 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.auth.DjangoAuth; | ||
| import org.commcare.formplayer.auth.HqAuth; | ||
| import org.commcare.formplayer.auth.PublicFormSessionAuth; | ||
| 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.Optional; | ||
|
|
||
| /** | ||
| * Unit tests for {@link UserRestoreAspect#getHqAuth} credential selection, in particular the | ||
| * choice between a Django session and a public web apps session. | ||
| */ | ||
| public class UserRestoreAspectTest { | ||
|
|
||
| private final UserRestoreAspect aspect = new UserRestoreAspect(); | ||
|
|
||
| private HqUserDetailsBean bean(boolean isPublicSession, String authToken) { | ||
| HqUserDetailsBean bean = new HqUserDetailsBean("domain", new String[]{"domain"}, "user", | ||
| false, new String[]{}, new String[]{}); | ||
| bean.setPublicSession(isPublicSession); | ||
| bean.setAuthToken(authToken); | ||
| return bean; | ||
| } | ||
|
|
||
| @Test | ||
| public void publicSession_usesPublicFormSessionAuthWithTheSessionKey() { | ||
| try (MockedStatic<RequestUtils> mocked = Mockito.mockStatic(RequestUtils.class)) { | ||
| mocked.when(RequestUtils::getUserDetails).thenReturn(Optional.of(bean(true, "pkey"))); | ||
|
|
||
| HqAuth auth = aspect.getHqAuth(null); | ||
|
|
||
| assertTrue(auth instanceof PublicFormSessionAuth); | ||
| assertEquals("public_form_session_key=pkey", auth.getAuthHeaders().getFirst("Cookie")); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void publicSession_winsEvenWhenASessionidCookieIsAlsoPresent() { | ||
| try (MockedStatic<RequestUtils> mocked = Mockito.mockStatic(RequestUtils.class)) { | ||
| mocked.when(RequestUtils::getUserDetails).thenReturn(Optional.of(bean(true, "pkey"))); | ||
|
|
||
| // Both signals present: the public credential must win, matching inbound selection. | ||
| HqAuth auth = aspect.getHqAuth("sessionid-value"); | ||
|
|
||
| assertTrue(auth instanceof PublicFormSessionAuth); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void regularSession_usesDjangoAuth() { | ||
| try (MockedStatic<RequestUtils> mocked = Mockito.mockStatic(RequestUtils.class)) { | ||
| mocked.when(RequestUtils::getUserDetails).thenReturn(Optional.of(bean(false, null))); | ||
|
|
||
| HqAuth auth = aspect.getHqAuth("sessionid-value"); | ||
|
|
||
| assertTrue(auth instanceof DjangoAuth); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void noUserDetailsWithSessionToken_usesDjangoAuth() { | ||
| try (MockedStatic<RequestUtils> mocked = Mockito.mockStatic(RequestUtils.class)) { | ||
| mocked.when(RequestUtils::getUserDetails).thenReturn(Optional.empty()); | ||
|
|
||
| HqAuth auth = aspect.getHqAuth("sessionid-value"); | ||
|
|
||
| assertTrue(auth instanceof DjangoAuth); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void noUserDetailsNoSessionToken_returnsNull() { | ||
| try (MockedStatic<RequestUtils> mocked = Mockito.mockStatic(RequestUtils.class)) { | ||
| mocked.when(RequestUtils::getUserDetails).thenReturn(Optional.empty()); | ||
|
|
||
| // SMS requests have neither a public session nor a sessionid cookie. | ||
| assertNull(aspect.getHqAuth(null)); | ||
| } | ||
| } | ||
| } |
35 changes: 35 additions & 0 deletions
35
src/test/java/org/commcare/formplayer/auth/PublicFormSessionAuthTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package org.commcare.formplayer.auth; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.http.HttpHeaders; | ||
|
|
||
| public class PublicFormSessionAuthTest { | ||
|
|
||
| @Test | ||
| public void getAuthHeaders_emitsPublicCookieAndHeaderOnly() { | ||
| HttpHeaders headers = new PublicFormSessionAuth("session-key-123").getAuthHeaders(); | ||
|
|
||
| assertEquals("public_form_session_key=session-key-123", headers.getFirst("Cookie")); | ||
| assertEquals("true", headers.getFirst("CommCare-Public-Session")); | ||
|
|
||
| // Exactly the two public headers — no Django sessionid/Authorization leaks out. | ||
| assertEquals(2, headers.size()); | ||
| assertFalse(headers.containsKey("sessionid")); | ||
| assertFalse(headers.containsKey("Authorization")); | ||
| } | ||
|
|
||
| @Test | ||
| public void toString_doesNotLeakTheKey() { | ||
| assertFalse(new PublicFormSessionAuth("super-secret-key").toString().contains("super-secret-key")); | ||
| } | ||
|
|
||
| @Test | ||
| public void constructor_rejectsMissingKey() { | ||
| assertThrows(IllegalArgumentException.class, () -> new PublicFormSessionAuth(null)); | ||
| assertThrows(IllegalArgumentException.class, () -> new PublicFormSessionAuth("")); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good check to have, but I'd move this before we instantiate HqAuth rather than after, unless there's a specific reason it needs to live here? Also, this throws an
IllegalArgumentException, which likely won't surface as an auth failure, did you confirm it's caught and mapped to the right error path so the user sees an appropriate error message rather than a generic exception?