fix(oauth2): ensure single-valued hd parameter for Spring Boot 3.3.13+ compatibility (#41271)

## Description
**Problem:**
Spring Boot 3.3.13 enforces single-valued OAuth2 parameters, causing
failures when multiple hd values are present in authorization requests.

**Solution:**
- Single-valued hd: Always 0 or 1 hd parameter
- Domain selection: Use request context to pick the domain
- Fallback: Use the first allowed domain when no match is found
- Multi-TLD support: Works with .com, .org, .io, etc.
- Proxy support: Handles X-Forwarded-Host headers
- Case-insensitive: Normalizes domain matching

EE Counterpart PR: https://github.com/appsmithorg/appsmith-ee/pull/8211

Fixes #`Issue Number`  
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.Authentication,@tag.Sanity"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/18095565045>
> Commit: e4e0e93ddb4a2f9a7c2babd9247dcadafa73dc90
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=18095565045&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Authentication,@tag.Sanity`
> Spec:
> <hr>Mon, 29 Sep 2025 12:34:36 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- New Features
- Improved OAuth login for setups with multiple allowed domains. The
system now auto-derives the most appropriate domain from incoming
requests, supports subdomain and multi-level matches, and gracefully
falls back when no match is found. Ensures OAuth parameters remain
single-valued for better compatibility and reliability.
- Tests
- Added comprehensive test coverage for multi-domain handling, subdomain
matching, fallback behavior, empty configurations, and parameter
single-value validation.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
subratadeypappu 2025-09-29 19:25:37 +06:00 committed by GitHub
parent 77005b5798
commit 4df6b9258f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 441 additions and 8 deletions

View File

@ -38,7 +38,11 @@ import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Base64;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
/**
* This class is a copy of {@link org.springframework.security.oauth2.client.web.server.DefaultServerOAuth2AuthorizationRequestResolver}
@ -177,7 +181,7 @@ public class CustomServerOAuth2AuthorizationRequestResolverCE implements ServerO
builder = OAuth2AuthorizationRequest.authorizationCode();
Map<String, Object> additionalParameters = new HashMap<>();
addAttributesAndAdditionalParameters(clientRegistration, attributes, additionalParameters);
addAttributesAndAdditionalParameters(exchange, clientRegistration, attributes, additionalParameters);
builder.additionalParameters(additionalParameters);
// } else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType()))
@ -199,6 +203,7 @@ public class CustomServerOAuth2AuthorizationRequestResolverCE implements ServerO
}
protected void addAttributesAndAdditionalParameters(
ServerWebExchange exchange,
ClientRegistration clientRegistration,
Map<String, Object> attributes,
Map<String, Object> additionalParameters) {
@ -215,18 +220,139 @@ public class CustomServerOAuth2AuthorizationRequestResolverCE implements ServerO
addPkceParameters(attributes, additionalParameters);
}
if (!commonConfig.getOauthAllowedDomains().isEmpty()) {
if (commonConfig.getOauthAllowedDomains().size() == 1) {
// Incase there's only 1 domain, we can do a further optimization to let the user select a specific one
// from the list
additionalParameters.put(
"hd", commonConfig.getOauthAllowedDomains().get(0));
List<String> allowedDomains = commonConfig.getOauthAllowedDomains();
if (allowedDomains.size() == 1) {
// Single domain case: use it directly
additionalParameters.put("hd", allowedDomains.get(0));
} else {
// Add multiple domains to the list of allowed domains
additionalParameters.put("hd", commonConfig.getOauthAllowedDomains());
// Multiple domains case: derive candidate domain from request context
String candidateDomain = deriveDomainFromRequest(exchange);
if (candidateDomain != null) {
// Domain was successfully derived and matched
additionalParameters.put("hd", candidateDomain);
log.debug("Using derived domain '{}' for hd parameter", candidateDomain);
} else {
// No domain could be derived or matched, fallback to first allowed domain
String fallbackDomain = allowedDomains.get(0);
additionalParameters.put("hd", fallbackDomain);
log.debug(
"No matching domain derived, using fallback domain '{}' for hd parameter", fallbackDomain);
}
}
}
}
/**
* Derives a candidate domain from the incoming request using existing tenant/domain logic.
* This method leverages the same mechanisms used elsewhere in the codebase for domain inference.
*
* @param exchange The ServerWebExchange containing the request
* @return The derived domain candidate, or null if no domain could be derived
*/
protected String deriveDomainFromRequest(ServerWebExchange exchange) {
try {
ServerHttpRequest request = exchange.getRequest();
// Extract host from request headers with fallback chain
String host = extractHostFromRequest(request);
if (host == null || host.isEmpty()) {
return null;
}
// Normalize host: strip port, lowercase, remove trailing dot
host = normalizeHost(host);
// Get and normalize allowed domains
List<String> allowedDomains = commonConfig.getOauthAllowedDomains();
if (allowedDomains == null || allowedDomains.isEmpty()) {
return null;
}
List<String> normalizedAllowed = allowedDomains.stream()
.filter(Objects::nonNull)
.map(d -> d.trim().toLowerCase(Locale.ROOT))
.filter(s -> !s.isEmpty())
.collect(Collectors.toList());
// Find the most specific domain match
return findBestDomainMatch(host, normalizedAllowed);
} catch (Exception e) {
log.debug("Error deriving domain from request", e);
return null;
}
}
/**
* Extracts host from request using fallback chain: X-Forwarded-Host -> URI host -> Host header
*/
private String extractHostFromRequest(ServerHttpRequest request) {
// Prefer X-Forwarded-Host header (for proxy environments)
String xfHost = request.getHeaders().getFirst("X-Forwarded-Host");
if (xfHost != null && !xfHost.isBlank()) {
// If comma-separated, take the first
int comma = xfHost.indexOf(',');
return (comma >= 0 ? xfHost.substring(0, comma) : xfHost).trim();
}
// Fallback to request URI host
if (request.getURI() != null && request.getURI().getHost() != null) {
return request.getURI().getHost();
}
// Final fallback to Host header
if (request.getHeaders().getHost() != null) {
return request.getHeaders().getHost().getHostString();
}
return null;
}
/**
* Normalizes host by removing port, converting to lowercase, and removing trailing dots
*/
private String normalizeHost(String host) {
if (host == null || host.isEmpty()) {
return host;
}
// Strip port
int colon = host.indexOf(':');
if (colon >= 0) {
host = host.substring(0, colon);
}
// Convert to lowercase
host = host.toLowerCase(Locale.ROOT);
// Remove trailing dot
if (host.endsWith(".")) {
host = host.substring(0, host.length() - 1);
}
return host;
}
/**
* Finds the most specific domain match using suffix matching
*/
private String findBestDomainMatch(String host, List<String> normalizedAllowed) {
String best = null;
for (String allowed : normalizedAllowed) {
if (host.equals(allowed) || host.endsWith("." + allowed)) {
// Prefer the most specific (longest) match
if (best == null || allowed.length() > best.length()) {
best = allowed;
}
}
}
return best;
}
/**
* This function sets the state query parameter sent to the OAuth2 resource server along with the parameter of the
* referer which initiated this OAuth2 login. On successful login, we will redirect back to the client's index page

View File

@ -0,0 +1,307 @@
package com.appsmith.server.authentication.handlers;
import com.appsmith.server.authentication.handlers.ce.CustomServerOAuth2AuthorizationRequestResolverCE;
import com.appsmith.server.authentication.oauth2clientrepositories.CustomOauth2ClientRepositoryManager;
import com.appsmith.server.configurations.CommonConfig;
import com.appsmith.server.helpers.RedirectHelper;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
import org.springframework.mock.web.server.MockServerWebExchange;
import org.springframework.security.oauth2.client.registration.ClientRegistration;
import org.springframework.security.oauth2.client.registration.ReactiveClientRegistrationRepository;
import org.springframework.security.oauth2.core.AuthorizationGrantType;
import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
import org.springframework.security.oauth2.core.oidc.OidcScopes;
import org.springframework.web.server.ServerWebExchange;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.when;
/**
* Test class to verify OAuth2 hd parameter handling for Spring Boot 3.3.13+ compatibility.
*
* This test ensures that:
* - OAuth2 authorization requests contain at most one hd parameter
* - Domain selection works correctly for single and multiple domain configurations
* - The implementation is robust across different TLDs and domain patterns
* - Fallback behavior works when no domain match is found
*/
@ExtendWith(MockitoExtension.class)
class OAuth2HdParameterTest {
@Mock
private ReactiveClientRegistrationRepository clientRegistrationRepository;
@Mock
private CommonConfig commonConfig;
@Mock
private RedirectHelper redirectHelper;
@Mock
private CustomOauth2ClientRepositoryManager oauth2ClientManager;
private CustomServerOAuth2AuthorizationRequestResolverCE resolver;
private ClientRegistration clientRegistration;
@BeforeEach
void setUp() {
resolver = new CustomServerOAuth2AuthorizationRequestResolverCE(
clientRegistrationRepository, commonConfig, redirectHelper, oauth2ClientManager);
// Create a mock Google OAuth client registration
clientRegistration = ClientRegistration.withRegistrationId("google")
.clientId("test-client-id")
.clientSecret("test-client-secret")
.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC)
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
.redirectUri("http://localhost:8080/login/oauth2/code/google")
.scope(OidcScopes.OPENID, OidcScopes.EMAIL, OidcScopes.PROFILE)
.authorizationUri("https://accounts.google.com/o/oauth2/v2/auth")
.tokenUri("https://www.googleapis.com/oauth2/v4/token")
.userInfoUri("https://www.googleapis.com/oauth2/v3/userinfo")
.userNameAttributeName("email")
.clientName("Google")
.build();
}
@Test
void testSingleDomainConfiguration() throws Exception {
// Arrange
List<String> singleDomain = Arrays.asList("company.com");
when(commonConfig.getOauthAllowedDomains()).thenReturn(singleDomain);
ServerWebExchange exchange = MockServerWebExchange.from(
MockServerHttpRequest.get("https://app.appsmith.com/oauth2/authorization/google"));
// Act
Map<String, Object> attributes = new HashMap<>();
Map<String, Object> additionalParameters = new HashMap<>();
invokeAddAttributesAndAdditionalParameters(exchange, attributes, additionalParameters);
// Assert
assertTrue(additionalParameters.containsKey("hd"), "hd parameter should be present for single domain");
assertEquals("company.com", additionalParameters.get("hd"), "hd should be the single configured domain");
assertFalse(additionalParameters.get("hd") instanceof List, "hd should not be a List");
}
@Test
void testMultipleDomainsWithMatchingSubdomain() throws Exception {
// Arrange
List<String> multipleDomains = Arrays.asList("company.com", "partner.org");
when(commonConfig.getOauthAllowedDomains()).thenReturn(multipleDomains);
// Request from company.appsmith.com should derive "company" subdomain
ServerWebExchange exchange = MockServerWebExchange.from(
MockServerHttpRequest.get("https://company.appsmith.com/oauth2/authorization/google"));
// Act
Map<String, Object> attributes = new HashMap<>();
Map<String, Object> additionalParameters = new HashMap<>();
invokeAddAttributesAndAdditionalParameters(exchange, attributes, additionalParameters);
// Assert
assertTrue(additionalParameters.containsKey("hd"), "hd parameter should be present when domain matches");
assertEquals("company.com", additionalParameters.get("hd"), "hd should be the matching derived domain");
assertFalse(additionalParameters.get("hd") instanceof List, "hd should not be a List");
}
@Test
void testMultipleDomainsWithNonMatchingSubdomain() throws Exception {
// Arrange
List<String> multipleDomains = Arrays.asList("company.com", "partner.org");
when(commonConfig.getOauthAllowedDomains()).thenReturn(multipleDomains);
// Request from demo.appsmith.com should derive "demo" subdomain which doesn't match
ServerWebExchange exchange = MockServerWebExchange.from(
MockServerHttpRequest.get("https://demo.appsmith.com/oauth2/authorization/google"));
// Act
Map<String, Object> attributes = new HashMap<>();
Map<String, Object> additionalParameters = new HashMap<>();
invokeAddAttributesAndAdditionalParameters(exchange, attributes, additionalParameters);
// Assert - should fallback to first domain when no match found
assertTrue(
additionalParameters.containsKey("hd"),
"hd parameter should be present, falling back to first domain when derived domain doesn't match");
assertEquals(
"company.com",
additionalParameters.get("hd"),
"hd should fallback to the first allowed domain when no match is found");
assertFalse(additionalParameters.get("hd") instanceof List, "hd should not be a List");
}
@Test
void testEmptyDomainConfiguration() throws Exception {
// Arrange
List<String> emptyDomains = Arrays.asList();
when(commonConfig.getOauthAllowedDomains()).thenReturn(emptyDomains);
ServerWebExchange exchange = MockServerWebExchange.from(
MockServerHttpRequest.get("https://app.appsmith.com/oauth2/authorization/google"));
// Act
Map<String, Object> attributes = new HashMap<>();
Map<String, Object> additionalParameters = new HashMap<>();
invokeAddAttributesAndAdditionalParameters(exchange, attributes, additionalParameters);
// Assert
assertFalse(
additionalParameters.containsKey("hd"),
"hd parameter should not be present when no domains configured");
}
@Test
void testSpringBoot3_3_13Compatibility() throws Exception {
// Arrange - setup the scenario that would cause the original error
List<String> multipleDomains = Arrays.asList("example.com", "test.com", "demo.org");
when(commonConfig.getOauthAllowedDomains()).thenReturn(multipleDomains);
ServerWebExchange exchange = MockServerWebExchange.from(
MockServerHttpRequest.get("https://demo.appsmith.com/oauth2/authorization/google"));
// Act
Map<String, Object> attributes = new HashMap<>();
Map<String, Object> additionalParameters = new HashMap<>();
invokeAddAttributesAndAdditionalParameters(exchange, attributes, additionalParameters);
// Assert - verify all OAuth2 parameters are single-valued
for (Map.Entry<String, Object> entry : additionalParameters.entrySet()) {
Object value = entry.getValue();
// The critical assertion: no parameter should be a List, Set, or array
assertFalse(
value instanceof List,
"OAuth2 parameter '" + entry.getKey() + "' cannot be a List - this would cause "
+ "'OAuth 2 parameters can only have a single value: " + entry.getKey()
+ "' error in Spring Boot 3.3.13+");
assertFalse(value instanceof java.util.Set, "OAuth2 parameter '" + entry.getKey() + "' cannot be a Set");
assertFalse(value instanceof Object[], "OAuth2 parameter '" + entry.getKey() + "' cannot be an array");
// If present, must be a single value (String, etc.)
if ("hd".equals(entry.getKey())) {
assertTrue(value instanceof String, "hd parameter must be a String when present");
}
}
}
@Test
void testRobustDomainMatching() throws Exception {
// Test domain matching with various TLDs and multi-level subdomains
List<String> multipleDomains = Arrays.asList("acme.com", "widgets.org", "example.net");
when(commonConfig.getOauthAllowedDomains()).thenReturn(multipleDomains);
// Test 1: Exact domain match
ServerWebExchange exchange1 =
MockServerWebExchange.from(MockServerHttpRequest.get("https://acme.com/oauth2/authorization/google"));
Map<String, Object> attributes1 = new HashMap<>();
Map<String, Object> additionalParameters1 = new HashMap<>();
invokeAddAttributesAndAdditionalParameters(exchange1, attributes1, additionalParameters1);
assertTrue(additionalParameters1.containsKey("hd"), "hd parameter should be present for exact domain match");
assertEquals("acme.com", additionalParameters1.get("hd"), "Should match acme.com exactly");
// Test 2: Subdomain match with .org TLD
ServerWebExchange exchange2 = MockServerWebExchange.from(
MockServerHttpRequest.get("https://tools.widgets.org/oauth2/authorization/google"));
Map<String, Object> attributes2 = new HashMap<>();
Map<String, Object> additionalParameters2 = new HashMap<>();
invokeAddAttributesAndAdditionalParameters(exchange2, attributes2, additionalParameters2);
assertTrue(additionalParameters2.containsKey("hd"), "hd parameter should be present for subdomain match");
assertEquals("widgets.org", additionalParameters2.get("hd"), "Should match widgets.org by suffix");
// Test 3: Most specific match (longest domain wins)
List<String> specificDomains = Arrays.asList("example.com", "app.example.com");
when(commonConfig.getOauthAllowedDomains()).thenReturn(specificDomains);
ServerWebExchange exchange3 = MockServerWebExchange.from(
MockServerHttpRequest.get("https://admin.app.example.com/oauth2/authorization/google"));
Map<String, Object> attributes3 = new HashMap<>();
Map<String, Object> additionalParameters3 = new HashMap<>();
invokeAddAttributesAndAdditionalParameters(exchange3, attributes3, additionalParameters3);
assertTrue(additionalParameters3.containsKey("hd"), "hd parameter should be present for most specific match");
assertEquals(
"app.example.com", additionalParameters3.get("hd"), "Should pick the most specific (longest) domain");
}
@Test
void testDomainDerivationLogic() throws Exception {
// Test the domain derivation method directly with realistic scenarios
List<String> allowedDomains = Arrays.asList("acme.com", "widgets.org", "example.net");
when(commonConfig.getOauthAllowedDomains()).thenReturn(allowedDomains);
// Test case 1: Exact domain match - acme.com should match acme.com
ServerWebExchange exchange1 =
MockServerWebExchange.from(MockServerHttpRequest.get("https://acme.com/oauth2/authorization/google"));
String derived1 = invokeDeriveFromRequest(exchange1);
assertEquals("acme.com", derived1, "Should match acme.com exactly");
// Test case 2: Subdomain match - api.widgets.org should match widgets.org
ServerWebExchange exchange2 = MockServerWebExchange.from(
MockServerHttpRequest.get("https://api.widgets.org/oauth2/authorization/google"));
String derived2 = invokeDeriveFromRequest(exchange2);
assertEquals("widgets.org", derived2, "Should match widgets.org by subdomain");
// Test case 3: Multi-level subdomain - admin.api.example.net should match example.net
ServerWebExchange exchange3 = MockServerWebExchange.from(
MockServerHttpRequest.get("https://admin.api.example.net/oauth2/authorization/google"));
String derived3 = invokeDeriveFromRequest(exchange3);
assertEquals("example.net", derived3, "Should match example.net by multi-level subdomain");
// Test case 4: No match - unknown.com should return null
ServerWebExchange exchange4 = MockServerWebExchange.from(
MockServerHttpRequest.get("https://unknown.com/oauth2/authorization/google"));
String derived4 = invokeDeriveFromRequest(exchange4);
assertNull(derived4, "Should return null for unknown domain");
// Test case 5: Case insensitive match - ACME.COM should match acme.com
ServerWebExchange exchange5 =
MockServerWebExchange.from(MockServerHttpRequest.get("https://ACME.COM/oauth2/authorization/google"));
String derived5 = invokeDeriveFromRequest(exchange5);
assertEquals("acme.com", derived5, "Should match acme.com case-insensitively");
}
/**
* Uses reflection to invoke the protected addAttributesAndAdditionalParameters method.
*/
private void invokeAddAttributesAndAdditionalParameters(
ServerWebExchange exchange, Map<String, Object> attributes, Map<String, Object> additionalParameters)
throws Exception {
Method method = CustomServerOAuth2AuthorizationRequestResolverCE.class.getDeclaredMethod(
"addAttributesAndAdditionalParameters",
ServerWebExchange.class,
ClientRegistration.class,
Map.class,
Map.class);
method.setAccessible(true);
method.invoke(resolver, exchange, clientRegistration, attributes, additionalParameters);
}
/**
* Uses reflection to invoke the protected deriveDomainFromRequest method.
*/
private String invokeDeriveFromRequest(ServerWebExchange exchange) throws Exception {
Method method = CustomServerOAuth2AuthorizationRequestResolverCE.class.getDeclaredMethod(
"deriveDomainFromRequest", ServerWebExchange.class);
method.setAccessible(true);
return (String) method.invoke(resolver, exchange);
}
}