From 93b4433e6bc991c0cd362690c06da6eee35f4178 Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Tue, 1 Sep 2026 11:22:57 +0300 Subject: [PATCH 1/6] Validate prevUri redirect parameter --- .../server/controller/AdminController.java | 8 +- ...eOAuth2AuthorizationRequestRepository.java | 5 +- .../Oauth2AuthenticationSuccessHandler.java | 30 ++++-- .../auth/oauth2/PrevUriValidator.java | 47 ++++++++ .../controller/AdminControllerTest.java | 23 ++++ ...th2AuthorizationRequestRepositoryTest.java | 66 ++++++++++++ ...thenticationSuccessHandlerBaseUrlTest.java | 102 ++++++++++++++++++ ...auth2AuthenticationSuccessHandlerTest.java | 5 + .../auth/oauth2/PrevUriValidatorTest.java | 74 +++++++++++++ 9 files changed, 344 insertions(+), 16 deletions(-) create mode 100644 application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidator.java create mode 100644 application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/HttpCookieOAuth2AuthorizationRequestRepositoryTest.java create mode 100644 application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandlerBaseUrlTest.java create mode 100644 application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidatorTest.java diff --git a/application/src/main/java/org/thingsboard/server/controller/AdminController.java b/application/src/main/java/org/thingsboard/server/controller/AdminController.java index 0b04a6939f..cbe1114864 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AdminController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AdminController.java @@ -76,6 +76,7 @@ import org.thingsboard.server.dao.settings.SecuritySettingsService; import org.thingsboard.server.queue.util.TbCoreComponent; import org.thingsboard.server.service.security.auth.jwt.settings.JwtSettingsService; import org.thingsboard.server.service.security.auth.oauth2.CookieUtils; +import org.thingsboard.server.service.security.auth.oauth2.PrevUriValidator; import org.thingsboard.server.service.security.model.SecurityUser; import org.thingsboard.server.service.security.model.token.JwtTokenFactory; import org.thingsboard.server.service.security.permission.Operation; @@ -419,8 +420,9 @@ public class AdminController extends BaseController { @GetMapping(value = "/mail/oauth2/authorize", produces = "application/text") public String getAuthorizationUrl(HttpServletRequest request, HttpServletResponse response) throws ThingsboardException { String state = StringUtils.generateSafeToken(); - if (request.getParameter(PREV_URI_PATH_PARAMETER) != null) { - CookieUtils.addCookie(response, PREV_URI_COOKIE_NAME, request.getParameter(PREV_URI_PATH_PARAMETER), 180); + String prevUriParam = request.getParameter(PREV_URI_PATH_PARAMETER); + if (PrevUriValidator.isValid(prevUriParam)) { + CookieUtils.addCookie(response, PREV_URI_COOKIE_NAME, prevUriParam, 180); } CookieUtils.addCookie(response, STATE_COOKIE_NAME, state, 180); @@ -449,7 +451,7 @@ public class AdminController extends BaseController { Optional cookieState = CookieUtils.getCookie(request, STATE_COOKIE_NAME); String baseUrl = this.systemSecurityService.getBaseUrl(TenantId.SYS_TENANT_ID, new CustomerId(EntityId.NULL_UUID), request); - String prevUri = baseUrl + (prevUrlOpt.isPresent() ? prevUrlOpt.get().getValue() : "/settings/outgoing-mail"); + String prevUri = baseUrl + prevUrlOpt.map(Cookie::getValue).filter(PrevUriValidator::isValid).orElse("/settings/outgoing-mail"); if (cookieState.isEmpty() || !cookieState.get().getValue().equals(state)) { CookieUtils.deleteCookie(request, response, STATE_COOKIE_NAME); diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/HttpCookieOAuth2AuthorizationRequestRepository.java b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/HttpCookieOAuth2AuthorizationRequestRepository.java index b908a6c650..2b40e548e6 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/HttpCookieOAuth2AuthorizationRequestRepository.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/HttpCookieOAuth2AuthorizationRequestRepository.java @@ -43,8 +43,9 @@ public class HttpCookieOAuth2AuthorizationRequestRepository implements Authoriza CookieUtils.deleteCookie(request, response, OAUTH2_AUTHORIZATION_REQUEST_COOKIE_NAME); return; } - if (request.getParameter(PREV_URI_PARAMETER) != null) { - CookieUtils.addCookie(response, PREV_URI_COOKIE_NAME, request.getParameter(PREV_URI_PARAMETER), cookieExpireSeconds); + String prevUri = request.getParameter(PREV_URI_PARAMETER); + if (PrevUriValidator.isValid(prevUri)) { + CookieUtils.addCookie(response, PREV_URI_COOKIE_NAME, prevUri, cookieExpireSeconds); } CookieUtils.addCookie(response, OAUTH2_AUTHORIZATION_REQUEST_COOKIE_NAME, CookieUtils.serialize(authorizationRequest), cookieExpireSeconds); } 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 c22ceb944a..f3fb49f45b 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 @@ -83,17 +83,7 @@ public class Oauth2AuthenticationSuccessHandler extends SimpleUrlAuthenticationS Authentication authentication) throws IOException { OAuth2AuthorizationRequest authorizationRequest = httpCookieOAuth2AuthorizationRequestRepository.loadAuthorizationRequest(request); String callbackUrlScheme = authorizationRequest.getAttribute(TbOAuth2ParameterNames.CALLBACK_URL_SCHEME); - String baseUrl; - if (!StringUtils.isEmpty(callbackUrlScheme)) { - baseUrl = callbackUrlScheme + ":"; - } else { - 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()) { - baseUrl += prevUrlOpt.get().getValue(); - CookieUtils.deleteCookie(request, response, PREV_URI_COOKIE_NAME); - } - } + String baseUrl = getBaseUrl(request, response, callbackUrlScheme); try { OAuth2AuthenticationToken token = (OAuth2AuthenticationToken) authentication; @@ -125,6 +115,22 @@ public class Oauth2AuthenticationSuccessHandler extends SimpleUrlAuthenticationS } } + String getBaseUrl(HttpServletRequest request, HttpServletResponse response, 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 baseUrl; + } + protected void clearAuthenticationAttributes(HttpServletRequest request, HttpServletResponse response) { super.clearAuthenticationAttributes(request); httpCookieOAuth2AuthorizationRequestRepository.removeAuthorizationRequestCookies(request, response); @@ -133,6 +139,8 @@ public class Oauth2AuthenticationSuccessHandler extends SimpleUrlAuthenticationS String getRedirectUrl(String baseUrl, JwtPair tokenPair) { if (baseUrl.indexOf("?") > 0) { baseUrl += "&"; + } else if (baseUrl.endsWith("/")) { + baseUrl += "?"; } else { baseUrl += "/?"; } diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidator.java b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidator.java new file mode 100644 index 0000000000..005979408c --- /dev/null +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidator.java @@ -0,0 +1,47 @@ +/** + * 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.thingsboard.server.common.data.StringUtils; + +import java.util.Locale; + +public class PrevUriValidator { + + private static final int MAX_LENGTH = 2048; + + /** + * prevUri is appended to the platform base URL, which ends right after the authority, so the single leading '/' + * is what keeps the redirect on this host - it closes the authority before any of the value is read. The rest + * keeps an accepted value usable: it has to survive the cookie round trip (RFC 6265 allows neither control + * characters nor '"', ',', ';', '\' or non-ASCII) and to pass StrictHttpFirewall, which rejects '//', '%2f' + * and '%5c' in the path; a fragment would swallow the access token. + */ + public static boolean isValid(String prevUri) { + if (StringUtils.isEmpty(prevUri) || prevUri.length() > MAX_LENGTH || prevUri.charAt(0) != '/') { + return false; + } + for (int i = 0; i < prevUri.length(); i++) { + char c = prevUri.charAt(i); + if (c <= ' ' || c >= 127 || c == '"' || c == ',' || c == ';' || c == '\\' || c == '#') { + return false; + } + } + String path = StringUtils.substringBefore(prevUri, "?").toLowerCase(Locale.ROOT); + return !path.contains("//") && !path.contains("%2f") && !path.contains("%5c"); + } + +} diff --git a/application/src/test/java/org/thingsboard/server/controller/AdminControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/AdminControllerTest.java index 21a8b3b2e9..e0866a3d86 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AdminControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AdminControllerTest.java @@ -17,6 +17,7 @@ package org.thingsboard.server.controller; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import jakarta.servlet.http.Cookie; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.RandomStringUtils; import org.junit.Test; @@ -111,6 +112,28 @@ public class AdminControllerTest extends AbstractControllerTest { .andExpect(statusReason(containsString("is prohibited"))); } + @Test + public void testMailOAuth2AuthorizationStoresOnlyInAppPrevUri() throws Exception { + loginSysAdmin(); + AdminSettings mailSettings = doGet("/api/admin/settings/mail", AdminSettings.class); + ObjectNode jsonValue = JacksonUtil.fromString(mailSettings.getJsonValue().toString(), ObjectNode.class); + jsonValue.put("clientId", "clientId"); + jsonValue.put("authUri", "https://accounts.google.com/o/oauth2/v2/auth"); + jsonValue.put("redirectUri", "https://thingsboard.io/api/admin/mail/oauth2/code"); + jsonValue.set("scope", JacksonUtil.newArrayNode().add("https://mail.google.com/")); + mailSettings.setJsonValue(jsonValue); + doPost("/api/admin/settings", mailSettings, AdminSettings.class); + + Cookie prevUriCookie = doGet("/api/admin/mail/oauth2/authorize?prevUri=@evil.com/") + .andExpect(status().isOk()).andReturn().getResponse().getCookie("prev_uri"); + assertThat(prevUriCookie).isNull(); + + prevUriCookie = doGet("/api/admin/mail/oauth2/authorize?prevUri=/settings/outgoing-mail") + .andExpect(status().isOk()).andReturn().getResponse().getCookie("prev_uri"); + assertThat(prevUriCookie).isNotNull(); + assertThat(prevUriCookie.getValue()).isEqualTo("/settings/outgoing-mail"); + } + @Test public void testSendTestMail() throws Exception { Mockito.doNothing().when(mailService).sendTestMail(any(), anyString()); 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 new file mode 100644 index 0000000000..92aa8f18df --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/HttpCookieOAuth2AuthorizationRequestRepositoryTest.java @@ -0,0 +1,66 @@ +/** + * 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.Test; +import org.mockito.ArgumentCaptor; +import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.atLeastOnce; +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; +import static org.thingsboard.server.service.security.auth.oauth2.HttpCookieOAuth2AuthorizationRequestRepository.PREV_URI_PARAMETER; + +public class HttpCookieOAuth2AuthorizationRequestRepositoryTest { + + private final HttpCookieOAuth2AuthorizationRequestRepository repository = new HttpCookieOAuth2AuthorizationRequestRepository(); + + @Test + public void testPrevUriSavedForInAppPath() { + assertThat(savePrevUri("/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState")) + .isEqualTo("/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState"); + } + + @Test + public void testPrevUriNotSavedForExternalUri() { + assertThat(savePrevUri("@evil.com/")).isNull(); + assertThat(savePrevUri("https://evil.com")).isNull(); + assertThat(savePrevUri("//evil.com")).isNull(); + } + + private String savePrevUri(String prevUri) { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + when(request.getParameter(PREV_URI_PARAMETER)).thenReturn(prevUri); + + repository.saveAuthorizationRequest(OAuth2AuthorizationRequest.authorizationCode() + .authorizationUri("testUri").clientId("testId").build(), request, response); + + ArgumentCaptor cookieCaptor = ArgumentCaptor.forClass(Cookie.class); + verify(response, atLeastOnce()).addCookie(cookieCaptor.capture()); + return cookieCaptor.getAllValues().stream() + .filter(cookie -> PREV_URI_COOKIE_NAME.equals(cookie.getName())) + .map(Cookie::getValue) + .findAny().orElse(null); + } + +} 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 new file mode 100644 index 0000000000..5d85c742f1 --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/Oauth2AuthenticationSuccessHandlerBaseUrlTest.java @@ -0,0 +1,102 @@ +/** + * 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 7e4c2645c7..e3ec321e8a 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 @@ -64,5 +64,10 @@ public class Oauth2AuthenticationSuccessHandlerTest extends AbstractControllerTe redirectUrl = oauth2AuthenticationSuccessHandler.getRedirectUrl(urlWithParams, jwtPair); expectedUrl = urlWithParams + "&accessToken=" + jwtPair.getToken() + "&refreshToken=" + jwtPair.getRefreshToken(); assertEquals(expectedUrl, redirectUrl); + + String urlWithTrailingSlash = "http://localhost:8080/"; + redirectUrl = oauth2AuthenticationSuccessHandler.getRedirectUrl(urlWithTrailingSlash, jwtPair); + expectedUrl = urlWithTrailingSlash + "?accessToken=" + jwtPair.getToken() + "&refreshToken=" + jwtPair.getRefreshToken(); + assertEquals(expectedUrl, redirectUrl); } } \ No newline at end of file diff --git a/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidatorTest.java b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidatorTest.java new file mode 100644 index 0000000000..487abffc24 --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidatorTest.java @@ -0,0 +1,74 @@ +/** + * 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 static org.assertj.core.api.Assertions.assertThat; + +public class PrevUriValidatorTest { + + @ParameterizedTest + @ValueSource(strings = { + "/", + "/login", + "/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e", + "/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState&page=1", + "/settings/outgoing-mail", + "/some%20path?q=a+b", + "/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=W3siaWQiOiJhL2IifV0%3D" + }) + public void testValidPrevUri(String prevUri) { + assertThat(PrevUriValidator.isValid(prevUri)).isTrue(); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = { + "@evil.com/", + "evil.com", + "https://evil.com", + "//evil.com", + "/\\evil.com", + "/\tevil.com", + "/ evil.com", + "/dashboards\\..\\evil.com", + "/login\nLocation: https://evil.com", + "/login\r\nSet-Cookie: a=b", + "/dashboards//evil.com", + "/dashboards%2Fevil.com", + "/dashboards%5cevil.com", + "/dashboards;jsessionid=1", + "/dashboards?title=a,b", + "/dashboards,list", + "/dashboards\"list", + "/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e#fragment", + "/панель" + }) + public void testInvalidPrevUri(String prevUri) { + assertThat(PrevUriValidator.isValid(prevUri)).isFalse(); + } + + @Test + public void testPrevUriLengthLimit() { + assertThat(PrevUriValidator.isValid("/" + "a".repeat(2047))).isTrue(); + assertThat(PrevUriValidator.isValid("/" + "a".repeat(2048))).isFalse(); + } + +} From ee64fa1938ff8e9324c0f2ae96a13d0c9b0006f4 Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Tue, 1 Sep 2026 12:14:30 +0300 Subject: [PATCH 2/6] Validate mobile app callback url scheme --- .../model/token/OAuth2AppTokenFactory.java | 12 +++ .../token/OAuth2AppTokenFactoryTest.java | 76 +++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 application/src/test/java/org/thingsboard/server/service/security/model/token/OAuth2AppTokenFactoryTest.java 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 a353aac86b..4a9b18cafd 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 @@ -29,7 +29,10 @@ import org.thingsboard.server.common.data.StringUtils; 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 @@ -39,6 +42,9 @@ 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 { @@ -65,6 +71,12 @@ public class OAuth2AppTokenFactory { 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))) { + throw new IllegalArgumentException("Application token has invalid callbackUrlScheme"); + } return callbackUrlScheme; } 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 new file mode 100644 index 0000000000..ec4da607f8 --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/service/security/model/token/OAuth2AppTokenFactoryTest.java @@ -0,0 +1,76 @@ +/** + * 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.model.token; + +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; +import java.util.Date; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class OAuth2AppTokenFactoryTest { + + private static final String APP_PACKAGE = "org.thingsboard.demo.app"; + private static final byte[] KEY_BYTES = "yjNyylzT1TmiVE2jV3YTnUpZzwLLLdPDJKmhLNyXDPnLtVCLcJIjIGmDPKHNoDMK".getBytes(); + + private final OAuth2AppTokenFactory tokenFactory = new OAuth2AppTokenFactory(); + + @Test + public void testMobileAppSchemeIsAccepted() { + assertThat(validate("tb-mobile.app1")).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)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("callbackUrlScheme"); + } + + private String validate(String callbackUrlScheme) { + SecretKey key = Keys.hmacShaKeyFor(KEY_BYTES); + String appToken = Jwts.builder() + .issuer(APP_PACKAGE) + .expiration(new Date(System.currentTimeMillis() + TimeUnit.MINUTES.toMillis(1))) + .claim("callbackUrlScheme", callbackUrlScheme) + .signWith(key) + .compact(); + return tokenFactory.validateTokenAndGetCallbackUrlScheme(APP_PACKAGE, appToken, Base64.getEncoder().encodeToString(KEY_BYTES)); + } + +} From 1d60693835c0f7cc5ecb98f6eb7ca9cd58083e53 Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Tue, 1 Sep 2026 14:42:14 +0300 Subject: [PATCH 3/6] 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)); } From e6ab6871e98a9cb60a993e873d9c3b7c98a63af6 Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Tue, 1 Sep 2026 14:42:14 +0300 Subject: [PATCH 4/6] Log rejected prevUri values and share the mail flow constants A rejected deep link is otherwise invisible to support. AdminController reuses the prevUri parameter and cookie names instead of its own copies, and its redirect target moves into a helper so the read side is covered by a test. --- .../server/controller/AdminController.java | 23 ++++-- .../auth/oauth2/PrevUriValidator.java | 23 +++++- .../AdminControllerMailOAuth2Test.java | 81 +++++++++++++++++++ .../controller/AdminControllerTest.java | 42 ++++++---- ...th2AuthorizationRequestRepositoryTest.java | 13 +-- 5 files changed, 151 insertions(+), 31 deletions(-) create mode 100644 application/src/test/java/org/thingsboard/server/controller/AdminControllerMailOAuth2Test.java diff --git a/application/src/main/java/org/thingsboard/server/controller/AdminController.java b/application/src/main/java/org/thingsboard/server/controller/AdminController.java index cbe1114864..8e8f4c4de3 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AdminController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AdminController.java @@ -93,6 +93,8 @@ import java.util.Optional; import static org.thingsboard.server.controller.ControllerConstants.SYSTEM_AUTHORITY_PARAGRAPH; import static org.thingsboard.server.controller.ControllerConstants.TENANT_AUTHORITY_PARAGRAPH; +import static org.thingsboard.server.service.security.auth.oauth2.HttpCookieOAuth2AuthorizationRequestRepository.PREV_URI_COOKIE_NAME; +import static org.thingsboard.server.service.security.auth.oauth2.HttpCookieOAuth2AuthorizationRequestRepository.PREV_URI_PARAMETER; @RestController @TbCoreComponent @@ -101,8 +103,7 @@ import static org.thingsboard.server.controller.ControllerConstants.TENANT_AUTHO @RequiredArgsConstructor public class AdminController extends BaseController { - private static final String PREV_URI_PATH_PARAMETER = "prevUri"; - private static final String PREV_URI_COOKIE_NAME = "prev_uri"; + private static final String DEFAULT_PREV_URI = "/settings/outgoing-mail"; private static final String STATE_COOKIE_NAME = "state"; private static final String MAIL_SETTINGS_KEY = "mail"; @@ -420,7 +421,7 @@ public class AdminController extends BaseController { @GetMapping(value = "/mail/oauth2/authorize", produces = "application/text") public String getAuthorizationUrl(HttpServletRequest request, HttpServletResponse response) throws ThingsboardException { String state = StringUtils.generateSafeToken(); - String prevUriParam = request.getParameter(PREV_URI_PATH_PARAMETER); + String prevUriParam = request.getParameter(PREV_URI_PARAMETER); if (PrevUriValidator.isValid(prevUriParam)) { CookieUtils.addCookie(response, PREV_URI_COOKIE_NAME, prevUriParam, 180); } @@ -447,12 +448,9 @@ public class AdminController extends BaseController { public void codeProcessingUrl( @RequestParam(value = "code") String code, @RequestParam(value = "state") String state, HttpServletRequest request, HttpServletResponse response) throws ThingsboardException, IOException { - Optional prevUrlOpt = CookieUtils.getCookie(request, PREV_URI_COOKIE_NAME); + String redirectUrl = getMailOAuth2RedirectUrl(request); Optional cookieState = CookieUtils.getCookie(request, STATE_COOKIE_NAME); - String baseUrl = this.systemSecurityService.getBaseUrl(TenantId.SYS_TENANT_ID, new CustomerId(EntityId.NULL_UUID), request); - String prevUri = baseUrl + prevUrlOpt.map(Cookie::getValue).filter(PrevUriValidator::isValid).orElse("/settings/outgoing-mail"); - if (cookieState.isEmpty() || !cookieState.get().getValue().equals(state)) { CookieUtils.deleteCookie(request, response, STATE_COOKIE_NAME); throw new ThingsboardException("Refresh token was not generated, invalid state param", ThingsboardErrorCode.BAD_REQUEST_PARAMS); @@ -482,7 +480,16 @@ public class AdminController extends BaseController { ((ObjectNode) jsonValue).put("tokenGenerated", true); adminSettingsService.saveAdminSettings(TenantId.SYS_TENANT_ID, adminSettings); - response.sendRedirect(prevUri); + response.sendRedirect(redirectUrl); + } + + String getMailOAuth2RedirectUrl(HttpServletRequest request) { + String baseUrl = this.systemSecurityService.getBaseUrl(TenantId.SYS_TENANT_ID, new CustomerId(EntityId.NULL_UUID), request); + String prevUri = CookieUtils.getCookie(request, PREV_URI_COOKIE_NAME) + .map(Cookie::getValue) + .filter(PrevUriValidator::isValid) + .orElse(DEFAULT_PREV_URI); + return baseUrl + prevUri; } } diff --git a/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidator.java b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidator.java index 005979408c..14855ecda3 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidator.java +++ b/application/src/main/java/org/thingsboard/server/service/security/auth/oauth2/PrevUriValidator.java @@ -15,13 +15,27 @@ */ package org.thingsboard.server.service.security.auth.oauth2; +import lombok.extern.slf4j.Slf4j; import org.thingsboard.server.common.data.StringUtils; import java.util.Locale; +@Slf4j public class PrevUriValidator { private static final int MAX_LENGTH = 2048; + private static final int MAX_LOGGED_LENGTH = 128; + + public static boolean isValid(String prevUri) { + if (StringUtils.isEmpty(prevUri)) { + return false; + } + if (!isInAppPath(prevUri)) { + log.debug("Ignoring prevUri that is not an in-app path: [{}]", forLog(prevUri)); + return false; + } + return true; + } /** * prevUri is appended to the platform base URL, which ends right after the authority, so the single leading '/' @@ -30,8 +44,8 @@ public class PrevUriValidator { * characters nor '"', ',', ';', '\' or non-ASCII) and to pass StrictHttpFirewall, which rejects '//', '%2f' * and '%5c' in the path; a fragment would swallow the access token. */ - public static boolean isValid(String prevUri) { - if (StringUtils.isEmpty(prevUri) || prevUri.length() > MAX_LENGTH || prevUri.charAt(0) != '/') { + private static boolean isInAppPath(String prevUri) { + if (prevUri.length() > MAX_LENGTH || prevUri.charAt(0) != '/') { return false; } for (int i = 0; i < prevUri.length(); i++) { @@ -44,4 +58,9 @@ public class PrevUriValidator { return !path.contains("//") && !path.contains("%2f") && !path.contains("%5c"); } + // a rejected value is attacker-controlled: it must not be able to forge log lines + private static String forLog(String prevUri) { + return prevUri.substring(0, Math.min(prevUri.length(), MAX_LOGGED_LENGTH)).replaceAll("[^\\x20-\\x7E]", "?"); + } + } diff --git a/application/src/test/java/org/thingsboard/server/controller/AdminControllerMailOAuth2Test.java b/application/src/test/java/org/thingsboard/server/controller/AdminControllerMailOAuth2Test.java new file mode 100644 index 0000000000..b74acea73a --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/controller/AdminControllerMailOAuth2Test.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.controller; + +import jakarta.servlet.http.Cookie; +import jakarta.servlet.http.HttpServletRequest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +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.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.thingsboard.server.service.security.auth.oauth2.HttpCookieOAuth2AuthorizationRequestRepository.PREV_URI_COOKIE_NAME; + +@ExtendWith(MockitoExtension.class) +public class AdminControllerMailOAuth2Test { + + private static final String BASE_URL = "https://thingsboard.example.com"; + private static final String DEFAULT_PREV_URI = "/settings/outgoing-mail"; + + @Mock + private SystemSecurityService systemSecurityService; + + @InjectMocks + private AdminController adminController; + + private HttpServletRequest request; + + @BeforeEach + public void before() { + request = mock(HttpServletRequest.class); + when(systemSecurityService.getBaseUrl(any(TenantId.class), any(CustomerId.class), any(HttpServletRequest.class))).thenReturn(BASE_URL); + } + + @Test + public void testInAppPathIsTakenFromPrevUriCookie() { + givenPrevUriCookie("/settings/notifications?tab=1"); + assertThat(adminController.getMailOAuth2RedirectUrl(request)).isEqualTo(BASE_URL + "/settings/notifications?tab=1"); + } + + @ParameterizedTest + @ValueSource(strings = {"@evil.com/", "//evil.com", "https://evil.com", "/\\evil.com", "/settings#fragment"}) + public void testForgedPrevUriCookieIsIgnored(String prevUri) { + givenPrevUriCookie(prevUri); + assertThat(adminController.getMailOAuth2RedirectUrl(request)).isEqualTo(BASE_URL + DEFAULT_PREV_URI); + } + + @Test + public void testRedirectUrlWithoutPrevUriCookie() { + when(request.getCookies()).thenReturn(null); + assertThat(adminController.getMailOAuth2RedirectUrl(request)).isEqualTo(BASE_URL + DEFAULT_PREV_URI); + } + + 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/controller/AdminControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/AdminControllerTest.java index e0866a3d86..bbdd324d82 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AdminControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AdminControllerTest.java @@ -39,6 +39,8 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import static org.thingsboard.server.service.security.auth.oauth2.HttpCookieOAuth2AuthorizationRequestRepository.PREV_URI_COOKIE_NAME; +import static org.thingsboard.server.service.security.auth.oauth2.HttpCookieOAuth2AuthorizationRequestRepository.PREV_URI_PARAMETER; @Slf4j @DaoSqlTest @@ -116,22 +118,30 @@ public class AdminControllerTest extends AbstractControllerTest { public void testMailOAuth2AuthorizationStoresOnlyInAppPrevUri() throws Exception { loginSysAdmin(); AdminSettings mailSettings = doGet("/api/admin/settings/mail", AdminSettings.class); - ObjectNode jsonValue = JacksonUtil.fromString(mailSettings.getJsonValue().toString(), ObjectNode.class); - jsonValue.put("clientId", "clientId"); - jsonValue.put("authUri", "https://accounts.google.com/o/oauth2/v2/auth"); - jsonValue.put("redirectUri", "https://thingsboard.io/api/admin/mail/oauth2/code"); - jsonValue.set("scope", JacksonUtil.newArrayNode().add("https://mail.google.com/")); - mailSettings.setJsonValue(jsonValue); - doPost("/api/admin/settings", mailSettings, AdminSettings.class); - - Cookie prevUriCookie = doGet("/api/admin/mail/oauth2/authorize?prevUri=@evil.com/") - .andExpect(status().isOk()).andReturn().getResponse().getCookie("prev_uri"); - assertThat(prevUriCookie).isNull(); - - prevUriCookie = doGet("/api/admin/mail/oauth2/authorize?prevUri=/settings/outgoing-mail") - .andExpect(status().isOk()).andReturn().getResponse().getCookie("prev_uri"); - assertThat(prevUriCookie).isNotNull(); - assertThat(prevUriCookie.getValue()).isEqualTo("/settings/outgoing-mail"); + JsonNode originalJsonValue = mailSettings.getJsonValue(); + try { + ObjectNode jsonValue = JacksonUtil.fromString(originalJsonValue.toString(), ObjectNode.class); + jsonValue.put("clientId", "clientId"); + jsonValue.put("authUri", "https://accounts.google.com/o/oauth2/v2/auth"); + jsonValue.put("redirectUri", "https://thingsboard.io/api/admin/mail/oauth2/code"); + jsonValue.set("scope", JacksonUtil.newArrayNode().add("https://mail.google.com/")); + mailSettings.setJsonValue(jsonValue); + doPost("/api/admin/settings", mailSettings, AdminSettings.class); + + Cookie prevUriCookie = doGet("/api/admin/mail/oauth2/authorize?" + PREV_URI_PARAMETER + "=@evil.com/") + .andExpect(status().isOk()).andReturn().getResponse().getCookie(PREV_URI_COOKIE_NAME); + assertThat(prevUriCookie).isNull(); + + prevUriCookie = doGet("/api/admin/mail/oauth2/authorize?" + PREV_URI_PARAMETER + "=/settings/outgoing-mail") + .andExpect(status().isOk()).andReturn().getResponse().getCookie(PREV_URI_COOKIE_NAME); + assertThat(prevUriCookie).isNotNull(); + assertThat(prevUriCookie.getValue()).isEqualTo("/settings/outgoing-mail"); + } finally { + // the mail settings are shared by the whole test context + AdminSettings currentSettings = doGet("/api/admin/settings/mail", AdminSettings.class); + currentSettings.setJsonValue(originalJsonValue); + doPost("/api/admin/settings", currentSettings, AdminSettings.class); + } } @Test 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 92aa8f18df..194b5d7939 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 @@ -19,6 +19,9 @@ import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; 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.mockito.ArgumentCaptor; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; @@ -40,11 +43,11 @@ public class HttpCookieOAuth2AuthorizationRequestRepositoryTest { .isEqualTo("/dashboards/3fa13530-6597-11ed-bd76-8bd591f0ec3e?state=someState"); } - @Test - public void testPrevUriNotSavedForExternalUri() { - assertThat(savePrevUri("@evil.com/")).isNull(); - assertThat(savePrevUri("https://evil.com")).isNull(); - assertThat(savePrevUri("//evil.com")).isNull(); + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = {"@evil.com/"}) + public void testPrevUriNotSavedForExternalUri(String prevUri) { + assertThat(savePrevUri(prevUri)).isNull(); } private String savePrevUri(String prevUri) { From a6c28e79b90b647c606b78660e68f82fea6b6be9 Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Tue, 1 Sep 2026 15:37:07 +0300 Subject: [PATCH 5/6] Allow an underscore in the mobile app callback url scheme Mobile apps derive the scheme from their package name, which may contain an underscore. It cannot introduce an authority, so accepting it keeps the rule as strong as the RFC 3986 grammar. --- .../security/auth/oauth2/CallbackUrlSchemeValidator.java | 3 ++- .../security/auth/oauth2/CallbackUrlSchemeValidatorTest.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) 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 index a75af8dd5e..bdb4a6d4ab 100644 --- 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 @@ -26,7 +26,8 @@ import java.util.regex.Pattern; @Slf4j public class CallbackUrlSchemeValidator { - private static final Pattern SCHEME_PATTERN = Pattern.compile("[a-zA-Z][a-zA-Z0-9+.-]*"); + // RFC 3986 scheme grammar, plus '_': mobile apps derive the scheme from their package name, which may contain one + 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; 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 index 2ba11324ce..aa2eeb7982 100644 --- 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 @@ -26,7 +26,7 @@ import static org.assertj.core.api.Assertions.assertThat; public class CallbackUrlSchemeValidatorTest { @ParameterizedTest - @ValueSource(strings = {"tbmobile", "tb-mobile.app1", "TbMobile+1"}) + @ValueSource(strings = {"tbmobile", "tb-mobile.app1", "TbMobile+1", "org.mycompany.myapp.auth", "com.my_company.app.auth"}) public void testMobileAppSchemeIsValid(String callbackUrlScheme) { assertThat(CallbackUrlSchemeValidator.isValid(callbackUrlScheme)).isTrue(); } From 1655cf9296674e465be6af68450f8b35cdaf46ae Mon Sep 17 00:00:00 2001 From: Viacheslav Klimov Date: Tue, 1 Sep 2026 15:55:53 +0300 Subject: [PATCH 6/6] 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(); + } + }