Skip to content

Commit

Permalink
Adding nonce to Authentication Request spring-projects#4442
Browse files Browse the repository at this point in the history
  • Loading branch information
shazin committed Oct 13, 2017
1 parent ea64d10 commit 6fcc745
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
* once access is granted (or denied) by the end-user (resource owner).
*
* @author Joe Grandja
* @author Shazin Sadakath
* @since 5.0
* @see AuthorizationRequest
* @see AuthorizationRequestRepository
Expand Down Expand Up @@ -131,6 +132,8 @@ private void sendRedirectForAuthorization(HttpServletRequest request, HttpServle
Map<String,Object> additionalParameters = new HashMap<>();
additionalParameters.put(OAuth2Parameter.REGISTRATION_ID, clientRegistration.getRegistrationId());

String nonce = request.getParameter(OAuth2Parameter.NONCE);

AuthorizationRequest.Builder builder;
if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
builder = AuthorizationRequest.authorizationCode();
Expand All @@ -146,6 +149,7 @@ private void sendRedirectForAuthorization(HttpServletRequest request, HttpServle
.redirectUri(redirectUriStr)
.scope(clientRegistration.getScope())
.state(this.stateGenerator.generateKey())
.nonce(nonce)
.additionalParameters(additionalParameters)
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
* which internally uses a {@link UriComponentsBuilder} to construct the <i>OAuth 2.0 Authorization Request</i>.
*
* @author Joe Grandja
* @author Shazin Sadakath
* @since 5.0
* @see AuthorizationRequest
* @see <a target="_blank" href="https://tools.ietf.org/html/rfc6749#section-4.1.1">Section 4.1.1 Authorization Code Grant Request</a>
Expand All @@ -42,7 +43,8 @@ public URI build(AuthorizationRequest authorizationRequest) {
.queryParam(OAuth2Parameter.CLIENT_ID, authorizationRequest.getClientId())
.queryParam(OAuth2Parameter.SCOPE,
authorizationRequest.getScope().stream().collect(Collectors.joining(" ")))
.queryParam(OAuth2Parameter.STATE, authorizationRequest.getState());
.queryParam(OAuth2Parameter.STATE, authorizationRequest.getState())
.queryParam(OAuth2Parameter.NONCE, authorizationRequest.getNonce());
if (authorizationRequest.getRedirectUri() != null) {
uriBuilder.queryParam(OAuth2Parameter.REDIRECT_URI, authorizationRequest.getRedirectUri());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
* Tests {@link AuthorizationCodeAuthenticationFilter}.
*
* @author Joe Grandja
* @author Shazin Sadakath
*/
public class AuthorizationCodeAuthenticationFilterTests {

Expand Down Expand Up @@ -115,10 +116,11 @@ public void doFilterWhenAuthorizationCodeSuccessResponseThenAuthenticationSucces
MockHttpServletRequest request = this.setupRequest(clientRegistration);
String authCode = "some code";
String state = "some state";
String nonce = "some nonce";
request.addParameter(OAuth2Parameter.CODE, authCode);
request.addParameter(OAuth2Parameter.STATE, state);
MockHttpServletResponse response = new MockHttpServletResponse();
setupAuthorizationRequest(authorizationRequestRepository, request, response, clientRegistration, state);
setupAuthorizationRequest(authorizationRequestRepository, request, response, clientRegistration, state, nonce);
FilterChain filterChain = mock(FilterChain.class);

filter.doFilter(request, response, filterChain);
Expand Down Expand Up @@ -191,7 +193,8 @@ private void setupAuthorizationRequest(AuthorizationRequestRepository authorizat
HttpServletRequest request,
HttpServletResponse response,
ClientRegistration clientRegistration,
String state) {
String state,
String nonce) {

Map<String,Object> additionalParameters = new HashMap<>();
additionalParameters.put(OAuth2Parameter.REGISTRATION_ID, clientRegistration.getRegistrationId());
Expand All @@ -203,6 +206,7 @@ private void setupAuthorizationRequest(AuthorizationRequestRepository authorizat
.redirectUri(clientRegistration.getRedirectUri())
.scope(clientRegistration.getScope())
.state(state)
.nonce(nonce)
.additionalParameters(additionalParameters)
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.springframework.security.oauth2.client.registration.ClientRegistration;
import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
import org.springframework.security.oauth2.core.endpoint.AuthorizationRequest;
import org.springframework.security.oauth2.core.endpoint.OAuth2Parameter;

import javax.servlet.FilterChain;
import javax.servlet.http.HttpServletRequest;
Expand All @@ -34,6 +35,7 @@
* Tests {@link AuthorizationRequestRedirectFilter}.
*
* @author Joe Grandja
* @author Shazin Sadakath
*/
public class AuthorizationRequestRedirectFilterTests {

Expand Down Expand Up @@ -92,6 +94,7 @@ public void doFilterWhenRequestMatchesClientThenAuthorizationRequestSavedInSessi
String requestUri = TestUtil.AUTHORIZATION_BASE_URI + "/" + clientRegistration.getRegistrationId();
MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
request.setServletPath(requestUri);
request.addParameter(OAuth2Parameter.NONCE, "nonce");
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = Mockito.mock(FilterChain.class);

Expand All @@ -111,6 +114,7 @@ public void doFilterWhenRequestMatchesClientThenAuthorizationRequestSavedInSessi
Assertions.assertThat(authorizationRequest.getRedirectUri()).isNotNull();
Assertions.assertThat(authorizationRequest.getScope()).isNotNull();
Assertions.assertThat(authorizationRequest.getState()).isNotNull();
Assertions.assertThat(authorizationRequest.getNonce()).isNotNull();
}

private AuthorizationRequestRedirectFilter setupFilter(String authorizationUri,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
* for the authorization code grant type or implicit grant type.
*
* @author Joe Grandja
* @author Shazin Sadakath
* @since 5.0
* @see AuthorizationGrantType
* @see ResponseType
Expand All @@ -45,6 +46,7 @@ public final class AuthorizationRequest implements Serializable {
private String redirectUri;
private Set<String> scope;
private String state;
private String nonce;
private Map<String,Object> additionalParameters;

private AuthorizationRequest() {
Expand Down Expand Up @@ -78,6 +80,10 @@ public String getState() {
return this.state;
}

public String getNonce() {
return nonce;
}

public Map<String, Object> getAdditionalParameters() {
return this.additionalParameters;
}
Expand All @@ -98,6 +104,7 @@ public static class Builder {
private String redirectUri;
private Set<String> scope;
private String state;
private String nonce;
private Map<String,Object> additionalParameters;

private Builder(AuthorizationGrantType authorizationGrantType) {
Expand Down Expand Up @@ -135,6 +142,11 @@ public Builder state(String state) {
return this;
}

public Builder nonce(String nonce) {
this.nonce = nonce;
return this;
}

public Builder additionalParameters(Map<String,Object> additionalParameters) {
this.additionalParameters = additionalParameters;
return this;
Expand All @@ -154,6 +166,7 @@ public AuthorizationRequest build() {
authorizationRequest.clientId = this.clientId;
authorizationRequest.redirectUri = this.redirectUri;
authorizationRequest.state = this.state;
authorizationRequest.nonce = this.nonce;
authorizationRequest.scope = Collections.unmodifiableSet(
CollectionUtils.isEmpty(this.scope) ?
Collections.emptySet() : new LinkedHashSet<>(this.scope));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* and used by the authorization endpoint and token endpoint.
*
* @author Joe Grandja
* @author Shazin Sadakath
* @since 5.0
* @see <a target="_blank" href="https://tools.ietf.org/html/rfc6749#section-11.2">11.2 OAuth Parameters Registry</a>
*/
Expand All @@ -45,4 +46,6 @@ public interface OAuth2Parameter {

String REGISTRATION_ID = "registration_id"; // Non-standard additional parameter

String NONCE = "nonce";

}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class AuthorizationRequestTest {
private static final String REDIRECT_URI = "http://redirect.uri/";
private static final Set<String> SCOPE = Collections.singleton("scope");
private static final String STATE = "xyz";
private static final String NONCE = "1234-456-0393";

@Test(expected = IllegalArgumentException.class)
public void buildWhenAuthorizationUriIsNullThenThrowIllegalArgumentException() {
Expand All @@ -43,6 +44,7 @@ public void buildWhenAuthorizationUriIsNullThenThrowIllegalArgumentException() {
.redirectUri(REDIRECT_URI)
.scope(SCOPE)
.state(STATE)
.nonce(NONCE)
.build();
}

Expand All @@ -53,6 +55,7 @@ public void buildWhenAuthorizeUriNotSetThenThrowIllegalArgumentException() {
.redirectUri(REDIRECT_URI)
.scope(SCOPE)
.state(STATE)
.nonce(NONCE)
.build();
}

Expand All @@ -64,6 +67,7 @@ public void buildWhenClientIdIsNullThenThrowIllegalArgumentException() {
.redirectUri(REDIRECT_URI)
.scope(SCOPE)
.state(STATE)
.nonce(NONCE)
.build();
}

Expand All @@ -74,6 +78,7 @@ public void buildWhenClientIdNotSetThenThrowIllegalArgumentException() {
.redirectUri(REDIRECT_URI)
.scope(SCOPE)
.state(STATE)
.nonce(NONCE)
.build();
}

Expand All @@ -86,6 +91,7 @@ public void buildWhenGetResponseTypeIsCalledThenReturnCode() {
.redirectUri(REDIRECT_URI)
.scope(SCOPE)
.state(STATE)
.nonce(NONCE)
.build();

assertThat(authorizationRequest.getResponseType()).isEqualTo(ResponseType.CODE);
Expand All @@ -99,6 +105,7 @@ public void buildWhenRedirectUriIsNullThenDoesNotThrowAnyException() {
.redirectUri(null)
.scope(SCOPE)
.state(STATE)
.nonce(NONCE)
.build()).doesNotThrowAnyException();
}

Expand All @@ -109,6 +116,7 @@ public void buildWhenRedirectUriNotSetThenDoesNotThrowAnyException() {
.clientId(CLIENT_ID)
.scope(SCOPE)
.state(STATE)
.nonce(NONCE)
.build()).doesNotThrowAnyException();
}

Expand All @@ -120,6 +128,7 @@ public void buildWhenScopesIsNullThenDoesNotThrowAnyException() {
.redirectUri(REDIRECT_URI)
.scope(null)
.state(STATE)
.nonce(NONCE)
.build()).doesNotThrowAnyException();
}

Expand All @@ -130,6 +139,7 @@ public void buildWhenScopesNotSetThenDoesNotThrowAnyException() {
.clientId(CLIENT_ID)
.redirectUri(REDIRECT_URI)
.state(STATE)
.nonce(NONCE)
.build()).doesNotThrowAnyException();
}

Expand All @@ -141,6 +151,19 @@ public void buildWhenStateIsNullThenDoesNotThrowAnyException() {
.redirectUri(REDIRECT_URI)
.scope(SCOPE)
.state(null)
.nonce(NONCE)
.build()).doesNotThrowAnyException();
}

@Test
public void buildWhenNonceIsNullThenDoesNotThrowAnyException() {
assertThatCode(() -> AuthorizationRequest.authorizationCode()
.authorizationUri(AUTHORIZE_URI)
.clientId(CLIENT_ID)
.redirectUri(REDIRECT_URI)
.scope(SCOPE)
.state(STATE)
.nonce(null)
.build()).doesNotThrowAnyException();
}

Expand All @@ -151,6 +174,7 @@ public void buildWhenStateNotSetThenDoesNotThrowAnyException() {
.clientId(CLIENT_ID)
.redirectUri(REDIRECT_URI)
.scope(SCOPE)
.nonce(NONCE)
.build()).doesNotThrowAnyException();
}
}

0 comments on commit 6fcc745

Please sign in to comment.