From d86fbc2093b2d47d4b963f9e04a08bbd8e692c28 Mon Sep 17 00:00:00 2001 From: wluyima Date: Fri, 25 Oct 2024 13:39:26 +0300 Subject: [PATCH] OZ-683 - Skip logout confirmation page OZ-683 - Skip logout confirmation if id_token exists in logout URL OZ-683 - Added and updated tests OZ-683 - Removed unused property --- .../oauth2login/OAuth2LoginConstants.java | 2 + .../CustomLogoutSuccessHandler.java | 40 +++------ .../web/controller/OAuth2LoginController.java | 18 ++-- .../CustomLogoutSuccessHandlerTest.java | 29 +++--- .../controller/OAuth2LoginControllerTest.java | 89 +++++++++++++++++++ 5 files changed, 131 insertions(+), 47 deletions(-) create mode 100644 omod/src/test/java/org/openmrs/module/oauth2login/web/controller/OAuth2LoginControllerTest.java diff --git a/api/src/main/java/org/openmrs/module/oauth2login/OAuth2LoginConstants.java b/api/src/main/java/org/openmrs/module/oauth2login/OAuth2LoginConstants.java index bc323e0..7db1db7 100644 --- a/api/src/main/java/org/openmrs/module/oauth2login/OAuth2LoginConstants.java +++ b/api/src/main/java/org/openmrs/module/oauth2login/OAuth2LoginConstants.java @@ -25,4 +25,6 @@ public class OAuth2LoginConstants { public static final String OAUTH_PROP_BEAN_NAME = "oauth2.properties"; + public static final String USER_PROP_ID_TOKEN = "oauth2IdToken"; + } diff --git a/omod/src/main/java/org/openmrs/module/oauth2login/web/controller/CustomLogoutSuccessHandler.java b/omod/src/main/java/org/openmrs/module/oauth2login/web/controller/CustomLogoutSuccessHandler.java index 30ed958..3ed3ac6 100644 --- a/omod/src/main/java/org/openmrs/module/oauth2login/web/controller/CustomLogoutSuccessHandler.java +++ b/omod/src/main/java/org/openmrs/module/oauth2login/web/controller/CustomLogoutSuccessHandler.java @@ -9,48 +9,32 @@ */ package org.openmrs.module.oauth2login.web.controller; +import static org.openmrs.module.oauth2login.OAuth2LoginConstants.USER_PROP_ID_TOKEN; + +import java.io.IOException; +import java.net.URLEncoder; +import java.util.Properties; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + import org.apache.commons.lang3.StringUtils; import org.openmrs.api.context.Context; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.security.core.Authentication; -import org.springframework.security.oauth2.client.OAuth2RestOperations; -import org.springframework.security.oauth2.common.OAuth2AccessToken; import org.springframework.security.web.authentication.logout.LogoutSuccessHandler; import org.springframework.security.web.authentication.logout.SimpleUrlLogoutSuccessHandler; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.net.URLEncoder; -import java.util.Properties; - public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler implements LogoutSuccessHandler { - private OAuth2RestOperations restTemplate; - - @Autowired - public void setRestTemplate(@Qualifier("oauth2.restTemplate") OAuth2RestOperations restTemplate) { - this.restTemplate = restTemplate; - } - - private String getToken() { - OAuth2AccessToken accessToken = restTemplate.getAccessToken(); - if (accessToken != null) { - return StringUtils.defaultString(accessToken.getValue()); - } - return StringUtils.EMPTY; - } - @Override public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) - throws IOException, ServletException { + throws IOException, ServletException { Properties properties = OAuth2BeanFactory.getProperties(OAuth2BeanFactory.getOAuth2PropertiesPath()); String redirectPath = properties.getProperty("logoutUri"); //the redirect path can contain a [token] that should be replaced by the aut token if (StringUtils.isNoneBlank(redirectPath) && redirectPath.contains("[token]")) { - String token = getToken(); + String token = Context.getAuthenticatedUser().getUserProperty(USER_PROP_ID_TOKEN); String encoded = URLEncoder.encode(token, "UTF-8"); redirectPath = StringUtils.replace(redirectPath, "[token]", encoded); } diff --git a/omod/src/main/java/org/openmrs/module/oauth2login/web/controller/OAuth2LoginController.java b/omod/src/main/java/org/openmrs/module/oauth2login/web/controller/OAuth2LoginController.java index 30c1b73..f0f340a 100644 --- a/omod/src/main/java/org/openmrs/module/oauth2login/web/controller/OAuth2LoginController.java +++ b/omod/src/main/java/org/openmrs/module/oauth2login/web/controller/OAuth2LoginController.java @@ -19,7 +19,6 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.openmrs.Person; import org.openmrs.Provider; import org.openmrs.User; import org.openmrs.api.PersonService; @@ -36,6 +35,7 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.oauth2.client.OAuth2RestOperations; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.client.RestOperations; @@ -94,6 +94,10 @@ public ModelAndView login() { try { Context.authenticate(new OAuth2TokenCredentials(userInfo)); if (Context.isAuthenticated()) { + User user = Context.getAuthenticatedUser(); + final String idToken = ((OAuth2RestOperations) restTemplate).getAccessToken().getAdditionalInformation() + .get("id_token").toString(); + user.setUserProperty(OAuth2LoginConstants.USER_PROP_ID_TOKEN, idToken); if ("true".equalsIgnoreCase(userInfo.getString(UserInfo.PROP_PROVIDER, "true"))) { activateProviderAccount(Context.getAuthenticatedUser()); } else { @@ -119,17 +123,17 @@ private void activateProviderAccount(User user) { Context.addProxyPrivilege(PrivilegeConstants.GET_USERS); Context.addProxyPrivilege(PrivilegeConstants.MANAGE_PROVIDERS); - Person person = personService.getPerson(user.getPerson().getId()); Collection possibleProvider = ps.getProvidersByPerson(user.getPerson()); if (possibleProvider.size() == 0) { Provider provider = new Provider(); provider.setIdentifier(user.getSystemId()); - provider.setPerson(person); + provider.setPerson(personService.getPerson(user.getPerson().getId())); provider.setCreator(userService.getUserByUsername("daemon")); ps.saveProvider(provider); } else { possibleProvider.stream().forEach(provider -> { - if (provider.getRetired()) ps.unretireProvider(provider); + if (provider.getRetired()) + ps.unretireProvider(provider); }); } } @@ -150,7 +154,8 @@ private void deactivateProviderAccount(User user) { Context.addProxyPrivilege(PrivilegeConstants.MANAGE_PROVIDERS); Collection possibleProvider = ps.getProvidersByPerson(user.getPerson()); - possibleProvider.stream().forEach(provider -> ps.retireProvider(provider, "Disabling provider account by " + OAuth2LoginConstants.MODULE_ARTIFACT_ID)); + possibleProvider.stream().forEach(provider -> ps.retireProvider(provider, + "Disabling provider account by " + OAuth2LoginConstants.MODULE_ARTIFACT_ID)); } catch (Exception e) { log.error("Could not retire provider account associated with user '" + user.getDisplayString(), e); @@ -181,8 +186,7 @@ private void authenticateWithSpringSecurity() { } /** - * Custom authentication used to have the token used by {@link CustomLogoutSuccessHandler} if - * needed + * Custom authentication used to have the token used by {@link CustomLogoutSuccessHandler} if needed */ public static class CustomAuthentication implements Authentication { diff --git a/omod/src/test/java/org/openmrs/module/oauth2login/web/controller/CustomLogoutSuccessHandlerTest.java b/omod/src/test/java/org/openmrs/module/oauth2login/web/controller/CustomLogoutSuccessHandlerTest.java index 758f9e3..a68cd5a 100644 --- a/omod/src/test/java/org/openmrs/module/oauth2login/web/controller/CustomLogoutSuccessHandlerTest.java +++ b/omod/src/test/java/org/openmrs/module/oauth2login/web/controller/CustomLogoutSuccessHandlerTest.java @@ -1,22 +1,26 @@ package org.openmrs.module.oauth2login.web.controller; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.io.IOException; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletResponse; + import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.openmrs.User; import org.openmrs.api.context.Context; +import org.openmrs.module.oauth2login.OAuth2LoginConstants; 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.security.oauth2.client.OAuth2RestOperations; -import org.springframework.security.oauth2.common.DefaultOAuth2AccessToken; import org.springframework.security.web.RedirectStrategy; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; - -import static org.mockito.Mockito.*; - @RunWith(PowerMockRunner.class) @PrepareForTest(Context.class) public class CustomLogoutSuccessHandlerTest { @@ -27,17 +31,18 @@ public CustomLogoutSuccessHandlerTest() { @Test public void onLogoutSuccess_redirectToLogoutURL() throws IOException, ServletException { + final String idToken = "myToken"; //setup PowerMockito.mockStatic(Context.class); + User user = new User(); + user.setUserProperty(OAuth2LoginConstants.USER_PROP_ID_TOKEN, idToken); + Mockito.when(Context.getAuthenticatedUser()).thenReturn(user); CustomLogoutSuccessHandler customLogoutSuccessHandler = new CustomLogoutSuccessHandler(); RedirectStrategy redirectStrategy = mock(RedirectStrategy.class); customLogoutSuccessHandler.setRedirectStrategy(redirectStrategy); MockHttpServletRequest request = new MockHttpServletRequest(); HttpServletResponse response = mock(HttpServletResponse.class); - OAuth2RestOperations restTemplate = mock(OAuth2RestOperations.class); - customLogoutSuccessHandler.setRestTemplate(restTemplate); - when(restTemplate.getAccessToken()).thenReturn(new DefaultOAuth2AccessToken("myToken")); //replay customLogoutSuccessHandler.onLogoutSuccess(request, response, null); @@ -46,7 +51,7 @@ public void onLogoutSuccess_redirectToLogoutURL() throws IOException, ServletExc Context.logout(); verify(redirectStrategy, times(1)).sendRedirect(request, response, - "http://localhost:8081/auth/realms/demo/protocol/openid-connect/logout?id_token_hint=myToken"); + "http://localhost:8081/auth/realms/demo/protocol/openid-connect/logout?id_token_hint=" + idToken); } } diff --git a/omod/src/test/java/org/openmrs/module/oauth2login/web/controller/OAuth2LoginControllerTest.java b/omod/src/test/java/org/openmrs/module/oauth2login/web/controller/OAuth2LoginControllerTest.java new file mode 100644 index 0000000..6374ed3 --- /dev/null +++ b/omod/src/test/java/org/openmrs/module/oauth2login/web/controller/OAuth2LoginControllerTest.java @@ -0,0 +1,89 @@ +/* + * Copyright (C) Amiyul LLC - All Rights Reserved + * + * This source code is protected under international copyright law. All rights + * reserved and protected by the copyright holder. + * + * This file is confidential and only available to authorized individuals with the + * permission of the copyright holder. If you encounter this file and do not have + * permission, please contact the copyright holder and delete this file. + */ +package org.openmrs.module.oauth2login.web.controller; + +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.openmrs.Person; +import org.openmrs.User; +import org.openmrs.api.AdministrationService; +import org.openmrs.api.PersonService; +import org.openmrs.api.ProviderService; +import org.openmrs.api.UserService; +import org.openmrs.api.context.Context; +import org.openmrs.module.oauth2login.OAuth2LoginConstants; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.powermock.reflect.Whitebox; +import org.springframework.security.oauth2.client.OAuth2RestOperations; +import org.springframework.security.oauth2.common.OAuth2AccessToken; + +@RunWith(PowerMockRunner.class) +@PrepareForTest({ Context.class }) +public class OAuth2LoginControllerTest { + + @Mock + private OAuth2RestOperations mockTemplate; + + @Mock + private OAuth2AccessToken mockAccessToken; + + @Mock + private ProviderService mockProviderService; + + @Mock + private PersonService mockPersonService; + + @Mock + private UserService mockUserService; + + @Mock + private AdministrationService mockAdminService; + + private OAuth2LoginController controller; + + @Test + public void login_shouldStoreTheIdTokenAsAUserProperty() { + final String idToken = "myToken"; + final String userInfUri = "http://test/userinfo"; + PowerMockito.mockStatic(Context.class); + Mockito.when(Context.isAuthenticated()).thenReturn(true); + Map additionalInfo = new HashMap(); + additionalInfo.put("id_token", idToken); + Mockito.when(mockAccessToken.getAdditionalInformation()).thenReturn(additionalInfo); + Mockito.when(mockTemplate.getAccessToken()).thenReturn(mockAccessToken); + User user = new User(); + user.setPerson(new Person()); + Mockito.when(Context.getAuthenticatedUser()).thenReturn(user); + controller = new OAuth2LoginController(); + Properties oauth2Props = new Properties(); + Whitebox.setInternalState(controller, "oauth2Props", oauth2Props); + Whitebox.setInternalState(controller, "userInfoUri", userInfUri); + Whitebox.setInternalState(controller, "restTemplate", mockTemplate); + Whitebox.setInternalState(controller, "ps", mockProviderService); + Whitebox.setInternalState(controller, "personService", mockPersonService); + Whitebox.setInternalState(controller, "userService", mockUserService); + Mockito.when(Context.getAdministrationService()).thenReturn(mockAdminService); + + controller.login(); + + Assert.assertEquals(idToken, user.getUserProperty(OAuth2LoginConstants.USER_PROP_ID_TOKEN)); + } + +}