From 1655cf9296674e465be6af68450f8b35cdaf46ae Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Tue, 1 Sep 2026 15:55:53 +0300 Subject: [PATCH] Assert the redirect URLs sent by the OAuth2 handlers --- .../Oauth2AuthenticationSuccessHandler.java | 6 +- ...th2AuthorizationRequestRepositoryTest.java | 2 +- ...auth2AuthenticationFailureHandlerTest.java | 103 ++++++++++++++++++ ...auth2AuthenticationSuccessHandlerTest.java | 90 +++++++++++++-- 4 files changed, 189 insertions(+), 12 deletions(-) create mode 100644 application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationFailureHandlerTest.java diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandler.java b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandler.java index 18717cc9ab..c99fb9378e 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandler.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandler.java @@ -124,9 +124,9 @@ public class Oauth2AuthenticationSuccessHandler extends SimpleUrlAuthenticationS } /** - * The in-app path the user was on before the login, or an empty string. The cookie is dropped either way - it is - * only meant to survive a single login round trip. It is kept out of the base URL so that the error redirect, - * which appends its own path, stays routable. + * The in-app path the user was on before the login, or an empty string. A present cookie is dropped whether or + * not its value passes validation - it is only meant to survive a single login round trip. The path is kept out + * of the base URL so that the error redirect, which appends its own path, stays routable. */ String getPrevUri(HttpServletRequest request, HttpServletResponse response, String callbackUrlScheme) { if (!StringUtils.isEmpty(callbackUrlScheme)) { diff --git a/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/HttpCookieOAuth2AuthorizationRequestRepositoryTest.java b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/HttpCookieOAuth2AuthorizationRequestRepositoryTest.java index 194b5d7939..5cada8477f 100644 --- a/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/HttpCookieOAuth2AuthorizationRequestRepositoryTest.java +++ b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/HttpCookieOAuth2AuthorizationRequestRepositoryTest.java @@ -46,7 +46,7 @@ public class HttpCookieOAuth2AuthorizationRequestRepositoryTest { @ParameterizedTest @NullAndEmptySource @ValueSource(strings = {"@evil.com/"}) - public void testPrevUriNotSavedForExternalUri(String prevUri) { + public void testPrevUriNotSavedForInvalidValue(String prevUri) { assertThat(savePrevUri(prevUri)).isNull(); } diff --git a/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationFailureHandlerTest.java b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationFailureHandlerTest.java new file mode 100644 index 0000000000..15110aa46d --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationFailureHandlerTest.java @@ -0,0 +1,103 @@ +/** + * Copyright © 2016-2026 The Thingsboard Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.thingsboard.server.service.security.auth.oauth2; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; +import org.springframework.security.authentication.AuthenticationServiceException; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; +import org.thingsboard.server.common.data.id.CustomerId; +import org.thingsboard.server.common.data.id.TenantId; +import org.thingsboard.server.service.security.system.SystemSecurityService; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class Oauth2AuthenticationFailureHandlerTest { + + private static final String BASE_URL = "https://thingsboard.example.com"; + + private final HttpCookieOAuth2AuthorizationRequestRepository authorizationRequestRepository = + mock(HttpCookieOAuth2AuthorizationRequestRepository.class); + private final SystemSecurityService systemSecurityService = mock(SystemSecurityService.class); + private final Oauth2AuthenticationFailureHandler failureHandler = + new Oauth2AuthenticationFailureHandler(authorizationRequestRepository, systemSecurityService); + + private HttpServletRequest request; + private HttpServletResponse response; + + @BeforeEach + public void before() { + request = mock(HttpServletRequest.class); + response = mock(HttpServletResponse.class); + when(request.getContextPath()).thenReturn(""); + when(response.encodeRedirectURL(anyString())).thenAnswer(invocation -> invocation.getArgument(0)); + when(systemSecurityService.getBaseUrl(any(TenantId.class), any(CustomerId.class), any(HttpServletRequest.class))).thenReturn(BASE_URL); + } + + @Test + public void testErrorIsSentToMobileAppScheme() throws Exception { + givenCallbackUrlScheme("tbmobile"); + assertThat(sendFailure()).isEqualTo("tbmobile:/?error=someError"); + } + + /** + * The scheme is restored from the oauth2_auth_request cookie, so a forged one must not turn the error redirect + * into a link to another host. + */ + @ParameterizedTest + @ValueSource(strings = {"https://evil.com", "javascript"}) + public void testForgedCallbackUrlSchemeFallsBackToLoginPage(String callbackUrlScheme) throws Exception { + givenCallbackUrlScheme(callbackUrlScheme); + assertThat(sendFailure()).isEqualTo(BASE_URL + "/login?loginError=someError"); + } + + @Test + public void testErrorIsSentToLoginPageWithoutCallbackUrlScheme() throws Exception { + givenCallbackUrlScheme(null); + assertThat(sendFailure()).isEqualTo(BASE_URL + "/login?loginError=someError"); + } + + @Test + public void testErrorIsSentToLoginPageWithoutAuthorizationRequest() throws Exception { + assertThat(sendFailure()).isEqualTo(BASE_URL + "/login?loginError=someError"); + } + + private void givenCallbackUrlScheme(String callbackUrlScheme) { + when(authorizationRequestRepository.loadAuthorizationRequest(request)).thenReturn( + OAuth2AuthorizationRequest.authorizationCode().authorizationUri("testUri").clientId("testId") + .attributes(attributes -> attributes.put(TbOAuth2ParameterNames.CALLBACK_URL_SCHEME, callbackUrlScheme)) + .build()); + } + + private String sendFailure() throws Exception { + failureHandler.onAuthenticationFailure(request, response, new AuthenticationServiceException("someError")); + + ArgumentCaptor redirectCaptor = ArgumentCaptor.forClass(String.class); + verify(response).sendRedirect(redirectCaptor.capture()); + return redirectCaptor.getValue(); + } + +} diff --git a/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandlerTest.java b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandlerTest.java index dc014a9091..eae9441711 100644 --- a/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandlerTest.java +++ b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandlerTest.java @@ -24,16 +24,28 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; +import org.springframework.security.oauth2.client.OAuth2AuthorizedClient; import org.springframework.security.oauth2.client.OAuth2AuthorizedClientService; +import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken; +import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.user.OAuth2User; import org.thingsboard.server.common.data.id.CustomerId; import org.thingsboard.server.common.data.id.TenantId; +import org.thingsboard.server.common.data.oauth2.MapperType; +import org.thingsboard.server.common.data.oauth2.OAuth2Client; +import org.thingsboard.server.common.data.oauth2.OAuth2MapperConfig; import org.thingsboard.server.common.data.security.model.JwtPair; import org.thingsboard.server.dao.oauth2.OAuth2ClientService; +import org.thingsboard.server.service.security.model.SecurityUser; import org.thingsboard.server.service.security.model.token.JwtTokenFactory; import org.thingsboard.server.service.security.system.SystemSecurityService; +import java.time.Instant; +import java.util.UUID; + import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -42,12 +54,17 @@ import static org.thingsboard.server.service.security.auth.oauth2.HttpCookieOAut public class Oauth2AuthenticationSuccessHandlerTest { private static final String BASE_URL = "https://thingsboard.example.com"; + private static final String PREV_URI = "/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e"; + private static final JwtPair TOKEN_PAIR = new JwtPair("testAccessToken", "testRefreshToken"); + private final JwtTokenFactory tokenFactory = mock(JwtTokenFactory.class); + private final OAuth2ClientMapperProvider oauth2ClientMapperProvider = mock(OAuth2ClientMapperProvider.class); + private final OAuth2ClientService oAuth2ClientService = mock(OAuth2ClientService.class); + private final OAuth2AuthorizedClientService oAuth2AuthorizedClientService = mock(OAuth2AuthorizedClientService.class); private final SystemSecurityService systemSecurityService = mock(SystemSecurityService.class); private final Oauth2AuthenticationSuccessHandler successHandler = new Oauth2AuthenticationSuccessHandler( - mock(JwtTokenFactory.class), mock(OAuth2ClientMapperProvider.class), mock(OAuth2ClientService.class), - mock(OAuth2AuthorizedClientService.class), mock(HttpCookieOAuth2AuthorizationRequestRepository.class), - systemSecurityService); + tokenFactory, oauth2ClientMapperProvider, oAuth2ClientService, oAuth2AuthorizedClientService, + mock(HttpCookieOAuth2AuthorizationRequestRepository.class), systemSecurityService); private HttpServletRequest request; private HttpServletResponse response; @@ -57,14 +74,14 @@ public class Oauth2AuthenticationSuccessHandlerTest { request = mock(HttpServletRequest.class); response = mock(HttpServletResponse.class); when(systemSecurityService.getBaseUrl(any(TenantId.class), any(CustomerId.class), any(HttpServletRequest.class))).thenReturn(BASE_URL); + when(response.encodeRedirectURL(anyString())).thenAnswer(invocation -> invocation.getArgument(0)); } @Test public void testInAppPathIsTakenFromPrevUriCookie() { - givenPrevUriCookie("/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState"); + givenPrevUriCookie(PREV_URI + "?state=someState"); assertThat(successHandler.getBaseUrl(request, null)).isEqualTo(BASE_URL); - assertThat(successHandler.getPrevUri(request, response, null)) - .isEqualTo("/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState"); + assertThat(successHandler.getPrevUri(request, response, null)).isEqualTo(PREV_URI + "?state=someState"); } @ParameterizedTest @@ -95,11 +112,36 @@ public class Oauth2AuthenticationSuccessHandlerTest { @Test public void testCallbackUrlSchemeIgnoresPrevUri() { - givenPrevUriCookie("/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e"); + givenPrevUriCookie(PREV_URI); assertThat(successHandler.getBaseUrl(request, "tbmobile")).isEqualTo("tbmobile:"); assertThat(successHandler.getPrevUri(request, response, "tbmobile")).isEmpty(); } + @Test + public void testSuccessRedirectCarriesTokensToPrevUri() throws Exception { + givenPrevUriCookie(PREV_URI); + givenSuccessfulLogin(); + + successHandler.onAuthenticationSuccess(request, response, givenAuthentication()); + + assertThat(captureRedirect()).isEqualTo(BASE_URL + PREV_URI + + "/?accessToken=testAccessToken&refreshToken=testRefreshToken"); + } + + /** + * The error redirect appends its own path, so it must be built from the base URL alone - with prevUri in it the + * result would be an unroutable https://host/dashboards/x/login?loginError=... + */ + @Test + public void testErrorRedirectDropsPrevUri() throws Exception { + givenPrevUriCookie(PREV_URI); + when(oAuth2ClientService.findOAuth2ClientById(any(), any())).thenThrow(new RuntimeException("someError")); + + successHandler.onAuthenticationSuccess(request, response, givenAuthentication()); + + assertThat(captureRedirect()).isEqualTo(BASE_URL + "/login?loginError=someError"); + } + @ParameterizedTest @CsvSource({ "https://thingsboard.example.com/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e, https://thingsboard.example.com/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e/?", @@ -107,7 +149,7 @@ public class Oauth2AuthenticationSuccessHandlerTest { "https://thingsboard.example.com/, https://thingsboard.example.com/?" }) public void testGetRedirectUrl(String baseUrl, String expectedPrefix) { - assertThat(successHandler.getRedirectUrl(baseUrl, new JwtPair("testAccessToken", "testRefreshToken"))) + assertThat(successHandler.getRedirectUrl(baseUrl, TOKEN_PAIR)) .isEqualTo(expectedPrefix + "accessToken=testAccessToken&refreshToken=testRefreshToken"); } @@ -115,4 +157,36 @@ public class Oauth2AuthenticationSuccessHandlerTest { when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(PREV_URI_COOKIE_NAME, prevUri)}); } + private OAuth2AuthenticationToken givenAuthentication() { + OAuth2User principal = mock(OAuth2User.class); + when(principal.getName()).thenReturn("testUser"); + OAuth2AuthenticationToken token = mock(OAuth2AuthenticationToken.class); + when(token.getAuthorizedClientRegistrationId()).thenReturn(UUID.randomUUID().toString()); + when(token.getPrincipal()).thenReturn(principal); + return token; + } + + private void givenSuccessfulLogin() { + OAuth2Client oauth2Client = new OAuth2Client(); + oauth2Client.setMapperConfig(OAuth2MapperConfig.builder().type(MapperType.BASIC).build()); + when(oAuth2ClientService.findOAuth2ClientById(any(), any())).thenReturn(oauth2Client); + + OAuth2AuthorizedClient authorizedClient = mock(OAuth2AuthorizedClient.class); + when(authorizedClient.getAccessToken()).thenReturn(new OAuth2AccessToken(OAuth2AccessToken.TokenType.BEARER, + "testProviderAccessToken", Instant.now(), Instant.now().plusSeconds(60))); + when(oAuth2AuthorizedClientService.loadAuthorizedClient(anyString(), anyString())).thenReturn(authorizedClient); + + SecurityUser securityUser = mock(SecurityUser.class); + OAuth2ClientMapper mapper = mock(OAuth2ClientMapper.class); + when(mapper.getOrCreateUserByClientPrincipal(any(), any(), anyString(), any())).thenReturn(securityUser); + when(oauth2ClientMapperProvider.getOAuth2ClientMapperByType(any())).thenReturn(mapper); + when(tokenFactory.createTokenPair(securityUser)).thenReturn(TOKEN_PAIR); + } + + private String captureRedirect() throws Exception { + ArgumentCaptor redirectCaptor = ArgumentCaptor.forClass(String.class); + verify(response).sendRedirect(redirectCaptor.capture()); + return redirectCaptor.getValue(); + } + }