Skip to content

Commit 485b7e9

Browse files
committed
Validate authorization request before authenticated check
Issue gh-66
1 parent cf70ddb commit 485b7e9

File tree

2 files changed

+35
-29
lines changed

2 files changed

+35
-29
lines changed

core/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java

+18-14
Original file line numberDiff line numberDiff line change
@@ -114,17 +114,14 @@ public OAuth2AuthorizationEndpointFilter(RegisteredClientRepository registeredCl
114114
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
115115
throws ServletException, IOException {
116116

117-
if (!this.authorizationEndpointMatcher.matches(request) || !isPrincipalAuthenticated()) {
117+
if (!this.authorizationEndpointMatcher.matches(request)) {
118118
filterChain.doFilter(request, response);
119119
return;
120120
}
121121

122-
// TODO
123-
// The authorization server validates the request to ensure that all
124-
// required parameters are present and valid. If the request is valid,
125-
// the authorization server authenticates the resource owner and obtains
126-
// an authorization decision (by asking the resource owner or by
127-
// establishing approval via other means).
122+
// ---------------
123+
// Validate the request to ensure that all required parameters are present and valid
124+
// ---------------
128125

129126
MultiValueMap<String, String> parameters = getParameters(request);
130127
String stateParameter = parameters.getFirst(OAuth2ParameterNames.STATE);
@@ -179,7 +176,18 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
179176
return;
180177
}
181178

179+
// ---------------
180+
// The request is valid - ensure the resource owner is authenticated
181+
// ---------------
182+
182183
Authentication principal = SecurityContextHolder.getContext().getAuthentication();
184+
if (!isPrincipalAuthenticated(principal)) {
185+
// Pass through the chain with the expectation that the authentication process
186+
// will commence via AuthenticationEntryPoint
187+
filterChain.doFilter(request, response);
188+
return;
189+
}
190+
183191
String code = this.codeGenerator.generateKey();
184192
OAuth2AuthorizationRequest authorizationRequest = convertAuthorizationRequest(request);
185193

@@ -238,8 +246,9 @@ private void sendErrorResponse(HttpServletRequest request, HttpServletResponse r
238246
this.redirectStrategy.sendRedirect(request, response, uriBuilder.toUriString());
239247
}
240248

241-
private static boolean isPrincipalAuthenticated() {
242-
return isPrincipalAuthenticated(SecurityContextHolder.getContext().getAuthentication());
249+
private static OAuth2Error createError(String errorCode, String parameterName) {
250+
return new OAuth2Error(errorCode, "OAuth 2.0 Parameter: " + parameterName,
251+
"https://tools.ietf.org/html/rfc6749#section-4.1.2.1");
243252
}
244253

245254
private static boolean isPrincipalAuthenticated(Authentication principal) {
@@ -248,11 +257,6 @@ private static boolean isPrincipalAuthenticated(Authentication principal) {
248257
principal.isAuthenticated();
249258
}
250259

251-
private static OAuth2Error createError(String errorCode, String parameterName) {
252-
return new OAuth2Error(errorCode, "OAuth 2.0 Parameter: " + parameterName,
253-
"https://tools.ietf.org/html/rfc6749#section-4.1.2.1");
254-
}
255-
256260
private static OAuth2AuthorizationRequest convertAuthorizationRequest(HttpServletRequest request) {
257261
MultiValueMap<String, String> parameters = getParameters(request);
258262

core/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java

+17-15
Original file line numberDiff line numberDiff line change
@@ -128,21 +128,6 @@ public void doFilterWhenAuthorizationRequestPostThenNotProcessed() throws Except
128128
verify(filterChain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
129129
}
130130

131-
@Test
132-
public void doFilterWhenAuthorizationRequestNotAuthenticatedThenNotProcessed() throws Exception {
133-
String requestUri = OAuth2AuthorizationEndpointFilter.DEFAULT_AUTHORIZATION_ENDPOINT_URI;
134-
MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
135-
request.setServletPath(requestUri);
136-
MockHttpServletResponse response = new MockHttpServletResponse();
137-
FilterChain filterChain = mock(FilterChain.class);
138-
139-
this.authentication.setAuthenticated(false);
140-
141-
this.filter.doFilter(request, response, filterChain);
142-
143-
verify(filterChain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
144-
}
145-
146131
@Test
147132
public void doFilterWhenAuthorizationRequestMissingClientIdThenInvalidRequestError() throws Exception {
148133
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
@@ -341,6 +326,23 @@ public void doFilterWhenAuthorizationRequestInvalidResponseTypeThenUnsupportedRe
341326
"state=state");
342327
}
343328

329+
@Test
330+
public void doFilterWhenAuthorizationRequestValidNotAuthenticatedThenContinueChainToCommenceAuthentication() throws Exception {
331+
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
332+
when(this.registeredClientRepository.findByClientId((eq(registeredClient.getClientId()))))
333+
.thenReturn(registeredClient);
334+
335+
MockHttpServletRequest request = createAuthorizationRequest(registeredClient);
336+
MockHttpServletResponse response = new MockHttpServletResponse();
337+
FilterChain filterChain = mock(FilterChain.class);
338+
339+
this.authentication.setAuthenticated(false);
340+
341+
this.filter.doFilter(request, response, filterChain);
342+
343+
verify(filterChain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
344+
}
345+
344346
@Test
345347
public void doFilterWhenAuthorizationRequestValidThenAuthorizationResponse() throws Exception {
346348
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();

0 commit comments

Comments
 (0)