Skip to content

Commit

Permalink
OA-42: Encode request parameter values in the logout url (#30)
Browse files Browse the repository at this point in the history
  • Loading branch information
@wluyima
wluyima authored Jan 29, 2025
1 parent 704d799 commit a83a630
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 4 deletions.
25 changes: 25 additions & 0 deletions omod/src/main/java/org/openmrs/module/oauth2login/web/Utils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import java.io.IOException;
import java.net.URLEncoder;
import java.nio.file.Paths;
import java.util.List;
import java.util.Map;
import java.util.Properties;

import javax.servlet.http.HttpServletRequest;
Expand All @@ -14,6 +16,7 @@
import org.openmrs.module.oauth2login.PropertyUtils;
import org.openmrs.util.OpenmrsUtil;
import org.springframework.util.MultiValueMap;
import org.springframework.web.util.UriComponents;
import org.springframework.web.util.UriComponentsBuilder;

/**
Expand All @@ -35,6 +38,7 @@ public static File getFileInAppDataDirectory(String filename) {
public static String getPostLogoutRedirectUrl(HttpServletRequest request) throws IOException {
Properties properties = PropertyUtils.getProperties(PropertyUtils.getOAuth2PropertiesPath());
String redirectPath = properties.getProperty("logoutUri");
boolean encodeDisabled = Boolean.valueOf(properties.getProperty("logoutUri.encode.disabled"));
//the redirect path can contain a [token] that should be replaced by the auth token
if (StringUtils.isNoneBlank(redirectPath) && redirectPath.contains("[token]")) {
String token = null;
Expand All @@ -56,7 +60,28 @@ public static String getPostLogoutRedirectUrl(HttpServletRequest request) throws
}
}

if (!encodeDisabled) {
redirectPath = encodeUrl(redirectPath);
}
return StringUtils.defaultIfBlank(redirectPath, request.getContextPath() + "/oauth2login");
}

protected static String encodeUrl(String logoutUrl) {
UriComponents urlComponents = UriComponentsBuilder.fromHttpUrl(logoutUrl).build();
MultiValueMap<String, String> params = urlComponents.getQueryParams();
UriComponentsBuilder urlBuilderEncoded = UriComponentsBuilder.newInstance();
urlBuilderEncoded.scheme(urlComponents.getScheme());
urlBuilderEncoded.host(urlComponents.getHost());
urlBuilderEncoded.port(urlComponents.getPort());
urlBuilderEncoded.path(urlComponents.getPath());
urlBuilderEncoded.fragment(urlComponents.getFragment());
if (params != null) {
for (Map.Entry<String, List<String>> entry : params.entrySet()) {
final String encodedKey = URLEncoder.encode(entry.getKey());
entry.getValue().forEach(v -> urlBuilderEncoded.queryParam(encodedKey, URLEncoder.encode(v)));
}
}

return urlBuilderEncoded.build().toString();
}
}
144 changes: 140 additions & 4 deletions omod/src/test/java/org/openmrs/module/oauth2login/web/UtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@
*/
package org.openmrs.module.oauth2login.web;

import static java.net.URLDecoder.decode;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.springframework.web.util.UriComponentsBuilder.fromUriString;

import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Properties;

import org.junit.BeforeClass;
import org.junit.Test;
Expand All @@ -19,25 +26,31 @@
import org.openmrs.User;
import org.openmrs.api.context.Context;
import org.openmrs.module.oauth2login.OAuth2LoginConstants;
import org.openmrs.module.oauth2login.PropertyUtils;
import org.openmrs.module.oauth2login.web.controller.OAuth2IntegrationTest;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.web.util.UriComponents;

@RunWith(PowerMockRunner.class)
@PrepareForTest(Context.class)
@PrepareForTest({ Context.class, PropertyUtils.class })
public class UtilsTest {

private static final String BASE_URL = "http://localhost:8081/auth/realms/demo/protocol/openid-connect/logout?";

private static final String PARAM_TOKEN = "id_token_hint";

protected static final String PARAM_POST_LOGOUT_URL = "post_logout_redirect_uri";

@BeforeClass
public static void setUp() {
OAuth2IntegrationTest.initPathInSystemProperties("Keycloak");
}

@Test
public void onLogoutSuccess_redirectToLogoutURL() throws Exception {
public void getPostLogoutRedirectUrl_redirectToLogoutURL() throws Exception {
final String idToken = "myToken";
PowerMockito.mockStatic(Context.class);
User user = new User();
Expand All @@ -51,7 +64,7 @@ public void onLogoutSuccess_redirectToLogoutURL() throws Exception {
}

@Test
public void onLogoutSuccess_shouldNotSetIdTokenIfUserIsNotAuthenticated() throws Exception {
public void getPostLogoutRedirectUrl_shouldNotSetIdTokenIfUserIsNotAuthenticated() throws Exception {
PowerMockito.mockStatic(Context.class);
MockHttpServletRequest request = new MockHttpServletRequest();

Expand All @@ -61,7 +74,7 @@ public void onLogoutSuccess_shouldNotSetIdTokenIfUserIsNotAuthenticated() throws
}

@Test
public void onLogoutSuccess_shouldNotSetIdTokenIfNoneIsSetForTheUser() throws Exception {
public void getPostLogoutRedirectUrl_shouldNotSetIdTokenIfNoneIsSetForTheUser() throws Exception {
PowerMockito.mockStatic(Context.class);
Mockito.when(Context.getAuthenticatedUser()).thenReturn(new User());
MockHttpServletRequest request = new MockHttpServletRequest();
Expand All @@ -71,4 +84,127 @@ public void onLogoutSuccess_shouldNotSetIdTokenIfNoneIsSetForTheUser() throws Ex
assertEquals(redirect, BASE_URL + "client_id=openmrs");
}

@Test
public void encodeUrl_shouldEncodeThePostLogoutUrl() {
final String baseLogoutUrl = "https://idp.com";
final String token = "myToken";
final String postLogoutUrl = "http://openmrs.org";
String logoutUrl = baseLogoutUrl + "?" + PARAM_TOKEN + "=" + token + "&" + PARAM_POST_LOGOUT_URL + "="
+ postLogoutUrl;

logoutUrl = Utils.encodeUrl(logoutUrl);

UriComponents logoutUrlComponents = fromUriString(logoutUrl).build();
assertEquals("https", logoutUrlComponents.getScheme());
assertEquals("idp.com", logoutUrlComponents.getHost());
assertEquals(-1, logoutUrlComponents.getPort());
assertNull(logoutUrlComponents.getPath());
assertEquals(2, logoutUrlComponents.getQueryParams().size());
assertEquals(token, logoutUrlComponents.getQueryParams().getFirst(PARAM_TOKEN));
assertEquals("http%3A%2F%2Fopenmrs.org", logoutUrlComponents.getQueryParams().getFirst(PARAM_POST_LOGOUT_URL));
}

@Test
public void encodeUrl_shouldEncodeAPostLogoutUrlWithPortAndPathAndOneParameter() throws Exception {
final String logoutPath = "/realm";
final String baseLogoutUrl = "https://idp.com:8080" + logoutPath;
final String token = "myToken";
final String postLogoutPath = "/spa";
final String postLogoutUrl = "http://openmrs.org:8081" + postLogoutPath + "?p=my+v";
String logoutUrl = baseLogoutUrl + "?" + PARAM_TOKEN + "=" + token + "&" + PARAM_POST_LOGOUT_URL + "="
+ postLogoutUrl;

logoutUrl = Utils.encodeUrl(logoutUrl);

UriComponents logoutUrlComponents = fromUriString(logoutUrl).build();
assertEquals("https", logoutUrlComponents.getScheme());
assertEquals("idp.com", logoutUrlComponents.getHost());
assertEquals(8080, logoutUrlComponents.getPort());
assertEquals(logoutPath, logoutUrlComponents.getPath());
assertEquals(2, logoutUrlComponents.getQueryParams().size());
assertEquals(token, logoutUrlComponents.getQueryParams().getFirst(PARAM_TOKEN));
final String encodedPostLogoutUrl = logoutUrlComponents.getQueryParams().getFirst(PARAM_POST_LOGOUT_URL);
UriComponents postLogoutUrlComponents = fromUriString(decode(encodedPostLogoutUrl, "UTF-8")).build();
assertEquals("http", postLogoutUrlComponents.getScheme());
assertEquals("openmrs.org", postLogoutUrlComponents.getHost());
assertEquals(8081, postLogoutUrlComponents.getPort());
assertEquals(postLogoutPath, postLogoutUrlComponents.getPath());
assertEquals("p=my+v", postLogoutUrlComponents.getQuery());
}

@Test
public void encodeUrl_shouldEncodeAPostLogoutUrlThatIsARelativePath() {
final String baseLogoutUrl = "https://idp.com";
final String token = "myToken";
final String postLogoutUrl = "/spa";
String logoutUrl = baseLogoutUrl + "?" + PARAM_TOKEN + "=" + token + "&" + PARAM_POST_LOGOUT_URL + "="
+ postLogoutUrl;

logoutUrl = Utils.encodeUrl(logoutUrl);

UriComponents logoutUrlComponents = fromUriString(logoutUrl).build();
assertEquals("https", logoutUrlComponents.getScheme());
assertEquals("idp.com", logoutUrlComponents.getHost());
assertEquals(2, logoutUrlComponents.getQueryParams().size());
assertEquals(token, logoutUrlComponents.getQueryParams().getFirst(PARAM_TOKEN));
assertEquals("%2Fspa", logoutUrlComponents.getQueryParams().getFirst(PARAM_POST_LOGOUT_URL));
}

@Test
public void encodeUrl_shouldEncodeAPostLogoutUrlThatIsARelativePathWithOneParameter() {
final String baseLogoutUrl = "https://idp.com";
final String token = "myToken";
final String postLogoutUrl = "/spa?p1=my+v";
String logoutUrl = baseLogoutUrl + "?" + PARAM_TOKEN + "=" + token + "&" + PARAM_POST_LOGOUT_URL + "="
+ postLogoutUrl;

logoutUrl = Utils.encodeUrl(logoutUrl);

UriComponents logoutUrlComponents = fromUriString(logoutUrl).build();
assertEquals("https", logoutUrlComponents.getScheme());
assertEquals("idp.com", logoutUrlComponents.getHost());
assertEquals(2, logoutUrlComponents.getQueryParams().size());
assertEquals(token, logoutUrlComponents.getQueryParams().getFirst(PARAM_TOKEN));
assertEquals("%2Fspa%3Fp1%3Dmy%2Bv", logoutUrlComponents.getQueryParams().getFirst(PARAM_POST_LOGOUT_URL));
}

@Test
public void getPostLogoutRedirectUrl_shouldEncodeTheUrl() throws Exception {
final String postLogoutUrl = "http://openmrs.org";
final String logoutUrl = "https://idp.com?" + PARAM_POST_LOGOUT_URL + "=" + postLogoutUrl;
Properties props = new Properties();
props.setProperty("logoutUri", logoutUrl);
PowerMockito.mockStatic(Context.class);
PowerMockito.mockStatic(PropertyUtils.class);
Path path = Paths.get("/test");
PowerMockito.when(PropertyUtils.getOAuth2PropertiesPath()).thenReturn(path);
PowerMockito.when(PropertyUtils.getProperties(path)).thenReturn(props);

MockHttpServletRequest request = new MockHttpServletRequest();

String redirect = Utils.getPostLogoutRedirectUrl(request);

assertEquals("https://idp.com?" + PARAM_POST_LOGOUT_URL + "=http%3A%2F%2Fopenmrs.org", redirect);
}

@Test
public void getPostLogoutRedirectUrl_shouldNotEncodeTheUrlIfDisabled() throws Exception {
final String postLogoutUrl = "http://openmrs.org";
final String logoutUrl = "https://idp.com?" + PARAM_POST_LOGOUT_URL + "=" + postLogoutUrl;
Properties props = new Properties();
props.setProperty("logoutUri", logoutUrl);
props.setProperty("logoutUri.encode.disabled", "true");
PowerMockito.mockStatic(Context.class);
PowerMockito.mockStatic(PropertyUtils.class);
Path path = Paths.get("/test");
PowerMockito.when(PropertyUtils.getOAuth2PropertiesPath()).thenReturn(path);
PowerMockito.when(PropertyUtils.getProperties(path)).thenReturn(props);

MockHttpServletRequest request = new MockHttpServletRequest();

String redirect = Utils.getPostLogoutRedirectUrl(request);

assertEquals(logoutUrl, redirect);
}

}

0 comments on commit a83a630

Please sign in to comment.