From 1d60693835c0f7cc5ecb98f6eb7ca9cd58083e53 Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Tue, 1 Sep 2026 14:42:14 +0300 Subject: [PATCH] Validate the callback url scheme where the redirect is built The scheme is restored from the oauth2_auth_request cookie, which the client can replace, so checking it only while the authorization request is built still let a forged cookie point the token redirect at any host. Both handlers now re-check it on read, sharing the rule with OAuth2AppTokenFactory. The success handler keeps prevUri out of the base URL as well, so the error redirect no longer appends /login to an in-app path. --- .../oauth2/CallbackUrlSchemeValidator.java | 65 ++++++++++ .../Oauth2AuthenticationFailureHandler.java | 5 +- .../Oauth2AuthenticationSuccessHandler.java | 36 ++++-- .../model/token/OAuth2AppTokenFactory.java | 14 +- .../CallbackUrlSchemeValidatorTest.java | 81 ++++++++++++ ...thenticationSuccessHandlerBaseUrlTest.java | 102 --------------- ...auth2AuthenticationSuccessHandlerTest.java | 121 ++++++++++++------ .../token/OAuth2AppTokenFactoryTest.java | 50 ++++---- 8 files changed, 279 insertions(+), 195 deletions(-) create mode 100644 application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/CallbackUrlSchemeValidator.java create mode 100644 application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/CallbackUrlSchemeValidatorTest.java delete mode 100644 application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandlerBaseUrlTest.java diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/CallbackUrlSchemeValidator.java b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/CallbackUrlSchemeValidator.java new file mode 100644 index 0000000000..a75af8dd5e --- /dev/null +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/CallbackUrlSchemeValidator.java @@ -0,0 +1,65 @@ +/** + * 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 lombok.extern.slf4j.Slf4j; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; +import org.thingsboard.server.common.data.StringUtils; + +import java.util.Locale; +import java.util.Set; +import java.util.regex.Pattern; + +@Slf4j +public class CallbackUrlSchemeValidator { + + private static final Pattern SCHEME_PATTERN = Pattern.compile("[a-zA-Z][a-zA-Z0-9+.-]*"); + private static final Set FORBIDDEN_SCHEMES = Set.of("http", "https", "javascript", "data", "file", "vbscript"); + private static final int MAX_LOGGED_LENGTH = 128; + + /** + * The redirect carrying the access token is built as callbackUrlScheme + ':', so only a mobile app scheme may + * pass: a web scheme would send the token to whatever host follows it. + */ + public static boolean isValid(String callbackUrlScheme) { + return !StringUtils.isEmpty(callbackUrlScheme) + && SCHEME_PATTERN.matcher(callbackUrlScheme).matches() + && !FORBIDDEN_SCHEMES.contains(callbackUrlScheme.toLowerCase(Locale.ROOT)); + } + + /** + * The attribute is restored from the oauth2_auth_request cookie, which the client can replace, so the scheme is + * checked again on read and not only when the authorization request is built. + */ + public static String getCallbackUrlScheme(OAuth2AuthorizationRequest authorizationRequest) { + String callbackUrlScheme = authorizationRequest != null ? + authorizationRequest.getAttribute(TbOAuth2ParameterNames.CALLBACK_URL_SCHEME) : null; + if (StringUtils.isEmpty(callbackUrlScheme)) { + return null; + } + if (!isValid(callbackUrlScheme)) { + log.warn("Ignoring invalid callback url scheme: [{}]", forLog(callbackUrlScheme)); + return null; + } + return callbackUrlScheme; + } + + // a rejected value is attacker-controlled: it must not be able to forge log lines + private static String forLog(String value) { + return value.substring(0, Math.min(value.length(), MAX_LOGGED_LENGTH)).replaceAll("[^\\x20-\\x7E]", "?"); + } + +} diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationFailureHandler.java b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationFailureHandler.java index 3b9d325c38..421be12c00 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationFailureHandler.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationFailureHandler.java @@ -54,11 +54,8 @@ public class Oauth2AuthenticationFailureHandler extends SimpleUrlAuthenticationF throws IOException, ServletException { String baseUrl; String errorPrefix; - String callbackUrlScheme = null; OAuth2AuthorizationRequest authorizationRequest = httpCookieOAuth2AuthorizationRequestRepository.loadAuthorizationRequest(request); - if (authorizationRequest != null) { - callbackUrlScheme = authorizationRequest.getAttribute(TbOAuth2ParameterNames.CALLBACK_URL_SCHEME); - } + String callbackUrlScheme = CallbackUrlSchemeValidator.getCallbackUrlScheme(authorizationRequest); if (!StringUtils.isEmpty(callbackUrlScheme)) { baseUrl = callbackUrlScheme + ":"; errorPrefix = "/?error="; 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 f3fb49f45b..18717cc9ab 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 @@ -82,8 +82,9 @@ public class Oauth2AuthenticationSuccessHandler extends SimpleUrlAuthenticationS HttpServletResponse response, Authentication authentication) throws IOException { OAuth2AuthorizationRequest authorizationRequest = httpCookieOAuth2AuthorizationRequestRepository.loadAuthorizationRequest(request); - String callbackUrlScheme = authorizationRequest.getAttribute(TbOAuth2ParameterNames.CALLBACK_URL_SCHEME); - String baseUrl = getBaseUrl(request, response, callbackUrlScheme); + String callbackUrlScheme = CallbackUrlSchemeValidator.getCallbackUrlScheme(authorizationRequest); + String baseUrl = getBaseUrl(request, callbackUrlScheme); + String prevUri = getPrevUri(request, response, callbackUrlScheme); try { OAuth2AuthenticationToken token = (OAuth2AuthenticationToken) authentication; @@ -98,7 +99,7 @@ public class Oauth2AuthenticationSuccessHandler extends SimpleUrlAuthenticationS clearAuthenticationAttributes(request, response); JwtPair tokenPair = tokenFactory.createTokenPair(securityUser); - getRedirectStrategy().sendRedirect(request, response, getRedirectUrl(baseUrl, tokenPair)); + getRedirectStrategy().sendRedirect(request, response, getRedirectUrl(baseUrl + prevUri, tokenPair)); systemSecurityService.logLoginAction(securityUser, new RestAuthenticationDetails(request), ActionType.LOGIN, oauth2Client.getName(), null); } catch (Exception e) { log.debug("Error occurred during processing authentication success result. " + @@ -115,20 +116,29 @@ public class Oauth2AuthenticationSuccessHandler extends SimpleUrlAuthenticationS } } - String getBaseUrl(HttpServletRequest request, HttpServletResponse response, String callbackUrlScheme) { + String getBaseUrl(HttpServletRequest request, String callbackUrlScheme) { if (!StringUtils.isEmpty(callbackUrlScheme)) { return callbackUrlScheme + ":"; } - String baseUrl = this.systemSecurityService.getBaseUrl(TenantId.SYS_TENANT_ID, new CustomerId(EntityId.NULL_UUID), request); - Optional prevUrlOpt = CookieUtils.getCookie(request, PREV_URI_COOKIE_NAME); - if (prevUrlOpt.isPresent()) { - String prevUri = prevUrlOpt.get().getValue(); - if (PrevUriValidator.isValid(prevUri)) { - baseUrl += prevUri; - } - CookieUtils.deleteCookie(request, response, PREV_URI_COOKIE_NAME); + return this.systemSecurityService.getBaseUrl(TenantId.SYS_TENANT_ID, new CustomerId(EntityId.NULL_UUID), request); + } + + /** + * 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. + */ + String getPrevUri(HttpServletRequest request, HttpServletResponse response, String callbackUrlScheme) { + if (!StringUtils.isEmpty(callbackUrlScheme)) { + return ""; + } + Optional prevUriOpt = CookieUtils.getCookie(request, PREV_URI_COOKIE_NAME); + if (prevUriOpt.isEmpty()) { + return ""; } - return baseUrl; + String prevUri = prevUriOpt.get().getValue(); + CookieUtils.deleteCookie(request, response, PREV_URI_COOKIE_NAME); + return PrevUriValidator.isValid(prevUri) ? prevUri : ""; } protected void clearAuthenticationAttributes(HttpServletRequest request, HttpServletResponse response) { diff --git a/application/src/main/java/org/thingsboard/server/service/security/model/token/OAuth2AppTokenFactory.java b/application/src/main/java/org/thingsboard/server/service/security/model/token/OAuth2AppTokenFactory.java index 4a9b18cafd..9e6b525364 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/model/token/OAuth2AppTokenFactory.java +++ b/application/src/main/java/org/thingsboard/server/service/security/model/token/OAuth2AppTokenFactory.java @@ -26,13 +26,11 @@ import io.jsonwebtoken.security.SignatureException; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Component; import org.thingsboard.server.common.data.StringUtils; +import org.thingsboard.server.service.security.auth.oauth2.CallbackUrlSchemeValidator; import java.util.Base64; import java.util.Date; -import java.util.Locale; -import java.util.Set; import java.util.concurrent.TimeUnit; -import java.util.regex.Pattern; @Component @Slf4j @@ -42,9 +40,6 @@ public class OAuth2AppTokenFactory { private static final long MAX_EXPIRATION_TIME_DIFF_MS = TimeUnit.MINUTES.toMillis(5); - private static final Pattern CALLBACK_URL_SCHEME_PATTERN = Pattern.compile("[a-zA-Z][a-zA-Z0-9+.-]*"); - private static final Set FORBIDDEN_CALLBACK_URL_SCHEMES = Set.of("http", "https", "javascript", "data", "file", "vbscript"); - public String validateTokenAndGetCallbackUrlScheme(String appPackage, String appToken, String appSecret) { Jws jwsClaims; try { @@ -64,17 +59,14 @@ public class OAuth2AppTokenFactory { if (timeDiff > MAX_EXPIRATION_TIME_DIFF_MS) { throw new IllegalArgumentException("Application token expiration time can't be longer than 5 minutes"); } - if (!claims.getIssuer().equals(appPackage)) { + if (!appPackage.equals(claims.getIssuer())) { throw new IllegalArgumentException("Application token issuer doesn't match application package"); } String callbackUrlScheme = claims.get(CALLBACK_URL_SCHEME, String.class); if (StringUtils.isEmpty(callbackUrlScheme)) { throw new IllegalArgumentException("Application token doesn't have callbackUrlScheme"); } - // the redirect carrying the access token is built as callbackUrlScheme + ":", so only a mobile app scheme - // may pass: a web scheme would send the token to whatever host follows it - if (!CALLBACK_URL_SCHEME_PATTERN.matcher(callbackUrlScheme).matches() - || FORBIDDEN_CALLBACK_URL_SCHEMES.contains(callbackUrlScheme.toLowerCase(Locale.ROOT))) { + if (!CallbackUrlSchemeValidator.isValid(callbackUrlScheme)) { throw new IllegalArgumentException("Application token has invalid callbackUrlScheme"); } return callbackUrlScheme; diff --git a/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/CallbackUrlSchemeValidatorTest.java b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/CallbackUrlSchemeValidatorTest.java new file mode 100644 index 0000000000..2ba11324ce --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/CallbackUrlSchemeValidatorTest.java @@ -0,0 +1,81 @@ +/** + * 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 org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; + +import static org.assertj.core.api.Assertions.assertThat; + +public class CallbackUrlSchemeValidatorTest { + + @ParameterizedTest + @ValueSource(strings = {"tbmobile", "tb-mobile.app1", "TbMobile+1"}) + public void testMobileAppSchemeIsValid(String callbackUrlScheme) { + assertThat(CallbackUrlSchemeValidator.isValid(callbackUrlScheme)).isTrue(); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = { + "https://evil.com", + "http://evil.com", + "https", + "HTTPS", + "javascript", + "data", + "file", + "vbscript", + "//evil.com", + "tbmobile/evil.com", + "tbmobile:evil.com", + "tbmobile evil", + "1tbmobile", + "tbmobile@evil.com" + }) + public void testInvalidSchemeIsRejected(String callbackUrlScheme) { + assertThat(CallbackUrlSchemeValidator.isValid(callbackUrlScheme)).isFalse(); + } + + @Test + public void testValidSchemeIsTakenFromAuthorizationRequest() { + assertThat(CallbackUrlSchemeValidator.getCallbackUrlScheme(givenAuthorizationRequest("tbmobile"))).isEqualTo("tbmobile"); + } + + @Test + public void testForgedSchemeFromAuthorizationRequestIsIgnored() { + assertThat(CallbackUrlSchemeValidator.getCallbackUrlScheme(givenAuthorizationRequest("https://evil.com"))).isNull(); + } + + @Test + public void testAuthorizationRequestWithoutScheme() { + assertThat(CallbackUrlSchemeValidator.getCallbackUrlScheme(givenAuthorizationRequest(null))).isNull(); + assertThat(CallbackUrlSchemeValidator.getCallbackUrlScheme(null)).isNull(); + } + + private OAuth2AuthorizationRequest givenAuthorizationRequest(String callbackUrlScheme) { + OAuth2AuthorizationRequest.Builder builder = OAuth2AuthorizationRequest.authorizationCode() + .authorizationUri("testUri").clientId("testId"); + if (callbackUrlScheme != null) { + builder.attributes(attributes -> attributes.put(TbOAuth2ParameterNames.CALLBACK_URL_SCHEME, callbackUrlScheme)); + } + return builder.build(); + } + +} diff --git a/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandlerBaseUrlTest.java b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandlerBaseUrlTest.java deleted file mode 100644 index 5d85c742f1..0000000000 --- a/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandlerBaseUrlTest.java +++ /dev/null @@ -1,102 +0,0 @@ -/** - * 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.Cookie; -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.oauth2.client.OAuth2AuthorizedClientService; -import org.thingsboard.server.common.data.id.CustomerId; -import org.thingsboard.server.common.data.id.TenantId; -import org.thingsboard.server.dao.oauth2.OAuth2ClientService; -import org.thingsboard.server.service.security.model.token.JwtTokenFactory; -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.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; -import static org.thingsboard.server.service.security.auth.oauth2.HttpCookieOAuth2AuthorizationRequestRepository.PREV_URI_COOKIE_NAME; - -public class Oauth2AuthenticationSuccessHandlerBaseUrlTest { - - private static final String BASE_URL = "https://thingsboard.example.com"; - - 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); - - private HttpServletRequest request; - private HttpServletResponse response; - - @BeforeEach - public void before() { - request = mock(HttpServletRequest.class); - response = mock(HttpServletResponse.class); - when(systemSecurityService.getBaseUrl(any(TenantId.class), any(CustomerId.class), any(HttpServletRequest.class))).thenReturn(BASE_URL); - } - - @Test - public void testInAppPathIsAppendedToBaseUrl() { - givenPrevUriCookie("/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState"); - assertThat(successHandler.getBaseUrl(request, response, null)) - .isEqualTo(BASE_URL + "/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState"); - } - - @ParameterizedTest - @ValueSource(strings = {"@evil.com/", "//evil.com", "https://evil.com", "/\\evil.com", "/dashboards#fragment"}) - public void testForgedPrevUriCookieIsIgnored(String prevUri) { - givenPrevUriCookie(prevUri); - assertThat(successHandler.getBaseUrl(request, response, null)).isEqualTo(BASE_URL); - } - - @Test - public void testForgedPrevUriCookieIsDeleted() { - givenPrevUriCookie("@evil.com/"); - - successHandler.getBaseUrl(request, response, null); - - ArgumentCaptor cookieCaptor = ArgumentCaptor.forClass(Cookie.class); - verify(response).addCookie(cookieCaptor.capture()); - assertThat(cookieCaptor.getValue().getName()).isEqualTo(PREV_URI_COOKIE_NAME); - assertThat(cookieCaptor.getValue().getMaxAge()).isZero(); - } - - @Test - public void testBaseUrlWithoutPrevUriCookie() { - when(request.getCookies()).thenReturn(null); - assertThat(successHandler.getBaseUrl(request, response, null)).isEqualTo(BASE_URL); - } - - @Test - public void testCallbackUrlSchemeIgnoresPrevUri() { - givenPrevUriCookie("/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e"); - assertThat(successHandler.getBaseUrl(request, response, "tbmobile")).isEqualTo("tbmobile:"); - } - - private void givenPrevUriCookie(String prevUri) { - when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(PREV_URI_COOKIE_NAME, prevUri)}); - } - -} 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 e3ec321e8a..dc014a9091 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 @@ -15,59 +15,104 @@ */ package org.thingsboard.server.service.security.auth.oauth2; -import org.junit.Before; -import org.junit.Test; -import org.mockito.Mock; -import org.springframework.beans.factory.annotation.Autowired; -import org.thingsboard.server.common.data.id.UserId; +import jakarta.servlet.http.Cookie; +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.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; +import org.springframework.security.oauth2.client.OAuth2AuthorizedClientService; +import org.thingsboard.server.common.data.id.CustomerId; +import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.security.model.JwtPair; -import org.thingsboard.server.controller.AbstractControllerTest; -import org.thingsboard.server.dao.service.DaoSqlTest; -import org.thingsboard.server.service.security.model.SecurityUser; +import org.thingsboard.server.dao.oauth2.OAuth2ClientService; import org.thingsboard.server.service.security.model.token.JwtTokenFactory; +import org.thingsboard.server.service.security.system.SystemSecurityService; -import java.util.UUID; - -import static org.junit.Assert.assertEquals; -import static org.mockito.ArgumentMatchers.eq; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.thingsboard.server.service.security.auth.oauth2.HttpCookieOAuth2AuthorizationRequestRepository.PREV_URI_COOKIE_NAME; -@DaoSqlTest -public class Oauth2AuthenticationSuccessHandlerTest extends AbstractControllerTest { +public class Oauth2AuthenticationSuccessHandlerTest { - @Autowired - private Oauth2AuthenticationSuccessHandler oauth2AuthenticationSuccessHandler; + private static final String BASE_URL = "https://thingsboard.example.com"; - @Mock - private JwtTokenFactory jwtTokenFactory; + 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); - private SecurityUser securityUser; + private HttpServletRequest request; + private HttpServletResponse response; - @Before + @BeforeEach public void before() { - UserId userId = new UserId(UUID.randomUUID()); - securityUser = new SecurityUser(userId); - when(jwtTokenFactory.createTokenPair(eq(securityUser))).thenReturn(new JwtPair("testAccessToken", "testRefreshToken")); + request = mock(HttpServletRequest.class); + response = mock(HttpServletResponse.class); + when(systemSecurityService.getBaseUrl(any(TenantId.class), any(CustomerId.class), any(HttpServletRequest.class))).thenReturn(BASE_URL); } @Test - public void testGetRedirectUrl() { - JwtPair jwtPair = jwtTokenFactory.createTokenPair(securityUser); + public void testInAppPathIsTakenFromPrevUriCookie() { + givenPrevUriCookie("/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?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"); + } + + @ParameterizedTest + @ValueSource(strings = {"@evil.com/", "//evil.com", "https://evil.com", "/\\evil.com", "/dashboards#fragment"}) + public void testForgedPrevUriCookieIsIgnored(String prevUri) { + givenPrevUriCookie(prevUri); + assertThat(successHandler.getPrevUri(request, response, null)).isEmpty(); + } - String urlWithoutParams = "http://localhost:8080/dashboardGroups/3fa13530-6597-11ed-bd76-8bd591f0ec3e"; - String urlWithParams = "http://localhost:8080/dashboardGroups/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState&page=1"; + @Test + public void testForgedPrevUriCookieIsDeleted() { + givenPrevUriCookie("@evil.com/"); - String redirectUrl = oauth2AuthenticationSuccessHandler.getRedirectUrl(urlWithoutParams, jwtPair); - String expectedUrl = urlWithoutParams + "/?accessToken=" + jwtPair.getToken() + "&refreshToken=" + jwtPair.getRefreshToken(); - assertEquals(expectedUrl, redirectUrl); + successHandler.getPrevUri(request, response, null); - redirectUrl = oauth2AuthenticationSuccessHandler.getRedirectUrl(urlWithParams, jwtPair); - expectedUrl = urlWithParams + "&accessToken=" + jwtPair.getToken() + "&refreshToken=" + jwtPair.getRefreshToken(); - assertEquals(expectedUrl, redirectUrl); + ArgumentCaptor cookieCaptor = ArgumentCaptor.forClass(Cookie.class); + verify(response).addCookie(cookieCaptor.capture()); + assertThat(cookieCaptor.getValue().getName()).isEqualTo(PREV_URI_COOKIE_NAME); + assertThat(cookieCaptor.getValue().getMaxAge()).isZero(); + } - String urlWithTrailingSlash = "http://localhost:8080/"; - redirectUrl = oauth2AuthenticationSuccessHandler.getRedirectUrl(urlWithTrailingSlash, jwtPair); - expectedUrl = urlWithTrailingSlash + "?accessToken=" + jwtPair.getToken() + "&refreshToken=" + jwtPair.getRefreshToken(); - assertEquals(expectedUrl, redirectUrl); + @Test + public void testBaseUrlWithoutPrevUriCookie() { + when(request.getCookies()).thenReturn(null); + assertThat(successHandler.getBaseUrl(request, null)).isEqualTo(BASE_URL); + assertThat(successHandler.getPrevUri(request, response, null)).isEmpty(); } -} \ No newline at end of file + + @Test + public void testCallbackUrlSchemeIgnoresPrevUri() { + givenPrevUriCookie("/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e"); + assertThat(successHandler.getBaseUrl(request, "tbmobile")).isEqualTo("tbmobile:"); + assertThat(successHandler.getPrevUri(request, response, "tbmobile")).isEmpty(); + } + + @ParameterizedTest + @CsvSource({ + "https://thingsboard.example.com/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e, https://thingsboard.example.com/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e/?", + "https://thingsboard.example.com/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState&page=1, https://thingsboard.example.com/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState&page=1&", + "https://thingsboard.example.com/, https://thingsboard.example.com/?" + }) + public void testGetRedirectUrl(String baseUrl, String expectedPrefix) { + assertThat(successHandler.getRedirectUrl(baseUrl, new JwtPair("testAccessToken", "testRefreshToken"))) + .isEqualTo(expectedPrefix + "accessToken=testAccessToken&refreshToken=testRefreshToken"); + } + + private void givenPrevUriCookie(String prevUri) { + when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(PREV_URI_COOKIE_NAME, prevUri)}); + } + +} diff --git a/application/src/test/java/org/thingsboard/server/service/security/model/token/OAuth2AppTokenFactoryTest.java b/application/src/test/java/org/thingsboard/server/service/security/model/token/OAuth2AppTokenFactoryTest.java index ec4da607f8..0130ddfff7 100644 --- a/application/src/test/java/org/thingsboard/server/service/security/model/token/OAuth2AppTokenFactoryTest.java +++ b/application/src/test/java/org/thingsboard/server/service/security/model/token/OAuth2AppTokenFactoryTest.java @@ -15,11 +15,10 @@ */ package org.thingsboard.server.service.security.model.token; +import io.jsonwebtoken.JwtBuilder; import io.jsonwebtoken.Jwts; import io.jsonwebtoken.security.Keys; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; import javax.crypto.SecretKey; import java.util.Base64; @@ -38,38 +37,35 @@ public class OAuth2AppTokenFactoryTest { @Test public void testMobileAppSchemeIsAccepted() { - assertThat(validate("tb-mobile.app1")).isEqualTo("tb-mobile.app1"); + assertThat(validate(appToken("tb-mobile.app1", APP_PACKAGE))).isEqualTo("tb-mobile.app1"); } - @ParameterizedTest - @ValueSource(strings = { - "https://evil.com", - "http://evil.com", - "https", - "HTTPS", - "javascript", - "data", - "//evil.com", - "tbmobile/evil.com", - "tbmobile:evil.com", - "tbmobile evil", - "1tbmobile", - "tbmobile@evil.com" - }) - public void testInvalidCallbackUrlSchemeIsRejected(String callbackUrlScheme) { - assertThatThrownBy(() -> validate(callbackUrlScheme)) + @Test + public void testInvalidCallbackUrlSchemeIsRejected() { + assertThatThrownBy(() -> validate(appToken("https://evil.com", APP_PACKAGE))) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("callbackUrlScheme"); } - private String validate(String callbackUrlScheme) { - SecretKey key = Keys.hmacShaKeyFor(KEY_BYTES); - String appToken = Jwts.builder() - .issuer(APP_PACKAGE) + @Test + public void testTokenWithoutIssuerIsRejected() { + assertThatThrownBy(() -> validate(appToken("tbmobile", null))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("issuer"); + } + + private String appToken(String callbackUrlScheme, String issuer) { + JwtBuilder builder = Jwts.builder() .expiration(new Date(System.currentTimeMillis() + TimeUnit.MINUTES.toMillis(1))) - .claim("callbackUrlScheme", callbackUrlScheme) - .signWith(key) - .compact(); + .claim("callbackUrlScheme", callbackUrlScheme); + if (issuer != null) { + builder.issuer(issuer); + } + SecretKey key = Keys.hmacShaKeyFor(KEY_BYTES); + return builder.signWith(key).compact(); + } + + private String validate(String appToken) { return tokenFactory.validateTokenAndGetCallbackUrlScheme(APP_PACKAGE, appToken, Base64.getEncoder().encodeToString(KEY_BYTES)); }