Skip to content

Commit 3b86d2b

Browse files
committed
Merge branch 'copilot/check-parameter-size-issue'
2 parents ca52330 + d4a83fe commit 3b86d2b

File tree

7 files changed

+326
-137
lines changed

7 files changed

+326
-137
lines changed

oidc/src/main/java/oidc/endpoints/AuthorizationEndpoint.java

Lines changed: 115 additions & 54 deletions
Large diffs are not rendered by default.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package oidc.exceptions;
2+
3+
import org.springframework.http.HttpStatus;
4+
import org.springframework.web.bind.annotation.ResponseStatus;
5+
6+
@ResponseStatus(value = HttpStatus.URI_TOO_LONG)
7+
public class UriTooLongException extends BaseException {
8+
9+
public UriTooLongException(String message) {
10+
super(message);
11+
}
12+
13+
@Override
14+
public String getErrorCode() {
15+
return "uri_too_long";
16+
}
17+
18+
@Override
19+
protected boolean suppressStackTrace() {
20+
return true;
21+
}
22+
}

oidc/src/main/resources/application.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ server:
2828
max-swallow-size: 10000000
2929
max-http-request-header-size: 10000000
3030

31+
# Maximum length of query parameters in bytes for authorization endpoint
32+
max-query-param-size: 8184
33+
3134
mongodb_db: oidc_test
3235
oidc_saml_mapping_path: classpath:/oidc/saml_mapping.json
3336
openid_configuration_path: classpath:/openid-configuration.json

oidc/src/test/java/oidc/endpoints/AuthorizationEndpointTest.java

Lines changed: 113 additions & 67 deletions
Large diffs are not rendered by default.

oidc/src/test/java/oidc/endpoints/AuthorizationEndpointUnitTest.java

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import org.junit.Test;
1717
import org.springframework.http.HttpMethod;
1818
import org.springframework.mock.web.MockHttpServletRequest;
19+
import org.springframework.util.LinkedMultiValueMap;
20+
import org.springframework.util.MultiValueMap;
1921
import org.springframework.util.StringUtils;
2022
import org.springframework.web.util.UriComponentsBuilder;
2123

@@ -227,4 +229,62 @@ private AuthorizationRequest authorizationRequest(Map<String, String> parameters
227229
request.setQueryString(queryString);
228230
return AuthorizationRequest.parse(JakartaServletUtils.createHTTPRequest(request));
229231
}
232+
233+
@Test
234+
public void validateQueryParamSizeWithinLimit() {
235+
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
236+
parameters.add("scope", "openid");
237+
parameters.add("response_type", "code");
238+
parameters.add("client_id", "test");
239+
240+
// Should not throw exception
241+
AuthorizationEndpoint endpoint = createAuthorizationEndpoint(1024);
242+
endpoint.validateQueryParamSize(parameters);
243+
}
244+
245+
@Test(expected = oidc.exceptions.UriTooLongException.class)
246+
public void validateQueryParamSizeExceedsLimit() {
247+
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
248+
// Create a scope parameter that exceeds 100 bytes
249+
StringBuilder longScope = new StringBuilder("openid");
250+
for (int i = 0; i < 50; i++) {
251+
longScope.append(" openid");
252+
}
253+
parameters.add("scope", longScope.toString());
254+
parameters.add("response_type", "code");
255+
parameters.add("client_id", "test");
256+
257+
AuthorizationEndpoint endpoint = createAuthorizationEndpoint(100);
258+
endpoint.validateQueryParamSize(parameters);
259+
}
260+
261+
@Test
262+
public void validateQueryParamSizeNullParameters() {
263+
// Should not throw exception
264+
AuthorizationEndpoint endpoint = createAuthorizationEndpoint(1024);
265+
endpoint.validateQueryParamSize(null);
266+
}
267+
268+
@Test
269+
public void validateQueryParamSizeEmptyParameters() {
270+
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
271+
272+
// Should not throw exception
273+
AuthorizationEndpoint endpoint = createAuthorizationEndpoint(1024);
274+
endpoint.validateQueryParamSize(parameters);
275+
}
276+
277+
private AuthorizationEndpoint createAuthorizationEndpoint(int maxQueryParamSize) {
278+
return new AuthorizationEndpoint(
279+
null, // authorizationCodeRepository
280+
null, // accessTokenRepository
281+
null, // userRepository
282+
null, // openIDClientRepository
283+
null, // tokenGenerator
284+
"salt",
285+
"test",
286+
false,
287+
maxQueryParamSize
288+
);
289+
}
230290
}

oidc/src/test/java/oidc/saml/AbstractSamlUnitTest.java

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
import org.springframework.security.saml2.provider.service.authentication.OpenSaml5AuthenticationProvider;
1919
import org.springframework.security.saml2.provider.service.authentication.Saml2AuthenticationToken;
2020
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration;
21-
import org.springframework.security.web.PortResolver;
22-
import org.springframework.security.web.PortResolverImpl;
2321
import org.w3c.dom.Document;
2422
import org.w3c.dom.Element;
2523

@@ -35,7 +33,6 @@
3533
public abstract class AbstractSamlUnitTest {
3634

3735
protected static Saml2X509Credential saml2X509Credential;
38-
protected static PortResolver portResolver = new PortResolverImpl();
3936
protected static RelyingPartyRegistration relyingParty;
4037

4138
@BeforeAll
@@ -45,15 +42,15 @@ public static void beforeClass() {
4542
OpenSamlInitializationService.initialize();
4643

4744
relyingParty = RelyingPartyRegistration
48-
.withRegistrationId("oidcng")
45+
.withRegistrationId("oidcng")
46+
.entityId("entityID")
47+
.signingX509Credentials(c -> c.add(saml2X509Credential))
48+
.assertionConsumerServiceLocation("https://acs")
49+
.assertingPartyMetadata(builder -> builder
4950
.entityId("entityID")
50-
.signingX509Credentials(c -> c.add(saml2X509Credential))
51-
.assertionConsumerServiceLocation("https://acs")
52-
.assertingPartyMetadata(builder -> builder
53-
.entityId("entityID")
54-
.wantAuthnRequestsSigned(false)
55-
.singleSignOnServiceLocation("https://sso").build())
56-
.build();
51+
.wantAuthnRequestsSigned(false)
52+
.singleSignOnServiceLocation("https://sso").build())
53+
.build();
5754
}
5855

5956
@SneakyThrows
@@ -81,7 +78,7 @@ public OpenSaml5AuthenticationProvider.ResponseToken getResponseToken(Response r
8178
public Response unmarshall(String saml2Response) throws UnmarshallingException, XMLParserException {
8279
XMLObjectProviderRegistry registry = ConfigurationService.get(XMLObjectProviderRegistry.class);
8380
ResponseUnmarshaller responseUnmarshaller = (ResponseUnmarshaller) registry.getUnmarshallerFactory()
84-
.getUnmarshaller(Response.DEFAULT_ELEMENT_NAME);
81+
.getUnmarshaller(Response.DEFAULT_ELEMENT_NAME);
8582
ParserPool parserPool = registry.getParserPool();
8683
Document doc = parserPool.parse(new ByteArrayInputStream(saml2Response.getBytes()));
8784
Element samlElement = doc.getDocumentElement();

oidc/src/test/java/oidc/saml/AuthnRequestContextConsumerUnitTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public void testSaml() throws Exception {
6565
request.addParameter("request", signedJWT.serialize());
6666

6767
when(requestCache.getRequest(any(HttpServletRequest.class), any()))
68-
.thenReturn(new DefaultSavedRequest(request, portResolver));
68+
.thenReturn(new DefaultSavedRequest(request));
6969

7070
AuthnRequest authnRequest = getAuthnRequest();
7171

@@ -107,7 +107,7 @@ public void testJWTRequestURIMismatch() {
107107
HttpServletRequest servletRequest = new MockHttpServletRequest();
108108

109109
when(requestCache.getRequest(any(HttpServletRequest.class), any()))
110-
.thenReturn(new DefaultSavedRequest(request, portResolver));
110+
.thenReturn(new DefaultSavedRequest(request));
111111
AuthnRequest authnRequest = getAuthnRequest();
112112

113113
OpenSaml5AuthenticationRequestResolver.AuthnRequestContext ctx =
@@ -128,7 +128,7 @@ public void testSamlForceAuthn() throws Exception {
128128
request.addParameter("client_id", "mock_sp");
129129

130130
when(requestCache.getRequest(any(HttpServletRequest.class), any()))
131-
.thenReturn(new DefaultSavedRequest(request, portResolver));
131+
.thenReturn(new DefaultSavedRequest(request));
132132

133133
AuthnRequest authnRequest = getAuthnRequest();
134134

@@ -151,4 +151,4 @@ public void noCookies() {
151151
subject.accept(ctx);
152152
}
153153

154-
}
154+
}

0 commit comments

Comments
 (0)