Fixing OidcHybridService to return id token (and c_hash claim) in all cases when it is needed
Project: http://git-wip-us.apache.org/repos/asf/cxf/repo Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/f9a42a52 Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/f9a42a52 Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/f9a42a52 Branch: refs/heads/master-jaxrs-2.1 Commit: f9a42a528f4edfa7bcc62d5885eebaeb25224cec Parents: e2f9b7d Author: Sergey Beryozkin <[email protected]> Authored: Mon May 23 16:55:47 2016 +0100 Committer: Sergey Beryozkin <[email protected]> Committed: Mon May 23 16:55:47 2016 +0100 ---------------------------------------------------------------------- .../grants/code/AbstractCodeDataProvider.java | 1 + .../code/AuthorizationCodeGrantHandler.java | 1 + .../code/AuthorizationCodeRegistration.java | 7 +++ .../code/ServerAuthorizationCodeGrant.java | 9 +++ .../services/AuthorizationCodeGrantService.java | 20 +------ .../oidc/idp/IdTokenResponseFilter.java | 8 ++- .../rs/security/oidc/idp/OidcHybridService.java | 16 +++--- .../cxf/rs/security/oidc/utils/OidcUtils.java | 1 + .../jaxrs/security/oidc/OIDCFlowTest.java | 59 ++++++++++++++++---- 9 files changed, 86 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf/blob/f9a42a52/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AbstractCodeDataProvider.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AbstractCodeDataProvider.java b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AbstractCodeDataProvider.java index 9b5c3df..c69b7bc 100644 --- a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AbstractCodeDataProvider.java +++ b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AbstractCodeDataProvider.java @@ -60,6 +60,7 @@ public abstract class AbstractCodeDataProvider extends AbstractOAuthDataProvider grant.setRequestedScopes(reg.getRequestedScope()); grant.setApprovedScopes(reg.getApprovedScope()); grant.setAudience(reg.getAudience()); + grant.setResponseType(reg.getResponseType()); grant.setClientCodeChallenge(reg.getClientCodeChallenge()); grant.setNonce(reg.getNonce()); grant.getExtraProperties().putAll(reg.getExtraProperties()); http://git-wip-us.apache.org/repos/asf/cxf/blob/f9a42a52/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AuthorizationCodeGrantHandler.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AuthorizationCodeGrantHandler.java b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AuthorizationCodeGrantHandler.java index 8427c7e..7da48ef 100644 --- a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AuthorizationCodeGrantHandler.java +++ b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AuthorizationCodeGrantHandler.java @@ -138,6 +138,7 @@ public class AuthorizationCodeGrantHandler extends AbstractGrantHandler { reg.setApprovedScope(Collections.emptyList()); } reg.setAudiences(audiences); + reg.setResponseType(grant.getResponseType()); reg.setClientCodeVerifier(codeVerifier); reg.setGrantType(OAuthConstants.CODE_RESPONSE_TYPE); return getDataProvider().createAccessToken(reg); http://git-wip-us.apache.org/repos/asf/cxf/blob/f9a42a52/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AuthorizationCodeRegistration.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AuthorizationCodeRegistration.java b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AuthorizationCodeRegistration.java index 269e24e..c65cbf7 100644 --- a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AuthorizationCodeRegistration.java +++ b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/AuthorizationCodeRegistration.java @@ -38,6 +38,7 @@ public class AuthorizationCodeRegistration { private UserSubject subject; private String audience; private String nonce; + private String responseType; private String clientCodeChallenge; private boolean preauthorizedTokenAvailable; private Map<String, String> extraProperties = new LinkedHashMap<String, String>(); @@ -148,4 +149,10 @@ public class AuthorizationCodeRegistration { public void setExtraProperties(Map<String, String> extraProperties) { this.extraProperties = extraProperties; } + public String getResponseType() { + return responseType; + } + public void setResponseType(String responseType) { + this.responseType = responseType; + } } http://git-wip-us.apache.org/repos/asf/cxf/blob/f9a42a52/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/ServerAuthorizationCodeGrant.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/ServerAuthorizationCodeGrant.java b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/ServerAuthorizationCodeGrant.java index eee307d..932d690 100644 --- a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/ServerAuthorizationCodeGrant.java +++ b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/ServerAuthorizationCodeGrant.java @@ -47,6 +47,7 @@ public class ServerAuthorizationCodeGrant extends AuthorizationCodeGrant { private List<String> requestedScopes = new LinkedList<String>(); private UserSubject subject; private String audience; + private String responseType; private String clientCodeChallenge; private String nonce; private boolean preauthorizedTokenAvailable; @@ -196,4 +197,12 @@ public class ServerAuthorizationCodeGrant extends AuthorizationCodeGrant { public void setExtraProperties(Map<String, String> extraProperties) { this.extraProperties = extraProperties; } + + public String getResponseType() { + return responseType; + } + + public void setResponseType(String responseType) { + this.responseType = responseType; + } } http://git-wip-us.apache.org/repos/asf/cxf/blob/f9a42a52/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AuthorizationCodeGrantService.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AuthorizationCodeGrantService.java b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AuthorizationCodeGrantService.java index 9efee12..5ec47d7 100644 --- a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AuthorizationCodeGrantService.java +++ b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AuthorizationCodeGrantService.java @@ -108,7 +108,7 @@ public class AuthorizationCodeGrantService extends RedirectionBasedGrantService OOBAuthorizationResponse oobResponse = new OOBAuthorizationResponse(); oobResponse.setClientId(client.getClientId()); oobResponse.setClientDescription(client.getApplicationDescription()); - oobResponse.setAuthorizationCode(grant.getCode()); + oobResponse.setAuthorizationCode(grantCode); oobResponse.setUserId(userSubject.getLogin()); oobResponse.setExpiresIn(grant.getExpiresIn()); return deliverOOBResponse(oobResponse); @@ -120,7 +120,7 @@ public class AuthorizationCodeGrantService extends RedirectionBasedGrantService } } - protected ServerAuthorizationCodeGrant getGrantRepresentation(OAuthRedirectionState state, + public ServerAuthorizationCodeGrant getGrantRepresentation(OAuthRedirectionState state, Client client, List<String> requestedScope, List<String> approvedScope, @@ -141,21 +141,6 @@ public class AuthorizationCodeGrantService extends RedirectionBasedGrantService return grant; } - public String getGrantCode(OAuthRedirectionState state, - Client client, - List<String> requestedScope, - List<String> approvedScope, - UserSubject userSubject, - ServerAccessToken preauthorizedToken) { - ServerAuthorizationCodeGrant grant = getGrantRepresentation(state, - client, - requestedScope, - approvedScope, - userSubject, - preauthorizedToken); - return processCodeGrant(client, grant.getCode(), grant.getSubject()); - } - protected AuthorizationCodeRegistration createCodeRegistration(OAuthRedirectionState state, Client client, List<String> requestedScope, @@ -167,6 +152,7 @@ public class AuthorizationCodeGrantService extends RedirectionBasedGrantService codeReg.setClient(client); codeReg.setRedirectUri(state.getRedirectUri()); codeReg.setRequestedScope(requestedScope); + codeReg.setResponseType(state.getResponseType()); codeReg.setApprovedScope(getApprovedScope(requestedScope, approvedScope)); codeReg.setSubject(userSubject); codeReg.setAudience(state.getAudience()); http://git-wip-us.apache.org/repos/asf/cxf/blob/f9a42a52/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/idp/IdTokenResponseFilter.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/idp/IdTokenResponseFilter.java b/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/idp/IdTokenResponseFilter.java index 74daf71..ecf019b 100644 --- a/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/idp/IdTokenResponseFilter.java +++ b/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/idp/IdTokenResponseFilter.java @@ -45,7 +45,11 @@ public class IdTokenResponseFilter extends OAuthServerJoseJwtProducer implements @Override public void process(ClientAccessToken ct, ServerAccessToken st) { if (st.getResponseType() != null - && OidcUtils.CODE_AT_RESPONSE_TYPE.equals(st.getResponseType())) { + && OidcUtils.CODE_AT_RESPONSE_TYPE.equals(st.getResponseType()) + && OidcUtils.HYBRID_FLOW.equals(st.getGrantType())) { + // token post-processing as part of the current hybrid (implicit) flow + // so no id_token is returned now - however when the code gets exchanged later on + // this filter will add id_token to the returned access token return; } // Only add an IdToken if the client has the "openid" scope @@ -84,7 +88,7 @@ public class IdTokenResponseFilter extends OAuthServerJoseJwtProducer implements String rType = st.getResponseType(); boolean atHashRequired = idToken.getAccessTokenHash() == null && (rType == null || !rType.equals(OidcUtils.ID_TOKEN_RESPONSE_TYPE)); - boolean cHashRequired = idToken.getAuthorizationCodeHash() == null && st.getGrantCode() != null + boolean cHashRequired = idToken.getAuthorizationCodeHash() == null && rType != null && (rType.equals(OidcUtils.CODE_ID_TOKEN_AT_RESPONSE_TYPE) || rType.equals(OidcUtils.CODE_ID_TOKEN_RESPONSE_TYPE)); http://git-wip-us.apache.org/repos/asf/cxf/blob/f9a42a52/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/idp/OidcHybridService.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/idp/OidcHybridService.java b/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/idp/OidcHybridService.java index a77a0e4..c7dca0f 100644 --- a/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/idp/OidcHybridService.java +++ b/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/idp/OidcHybridService.java @@ -31,6 +31,7 @@ import org.apache.cxf.rs.security.oauth2.common.Client; import org.apache.cxf.rs.security.oauth2.common.OAuthRedirectionState; import org.apache.cxf.rs.security.oauth2.common.ServerAccessToken; import org.apache.cxf.rs.security.oauth2.common.UserSubject; +import org.apache.cxf.rs.security.oauth2.grants.code.ServerAuthorizationCodeGrant; import org.apache.cxf.rs.security.oauth2.utils.OAuthConstants; import org.apache.cxf.rs.security.oidc.utils.OidcUtils; @@ -42,7 +43,7 @@ public class OidcHybridService extends OidcImplicitService { this(false); } public OidcHybridService(boolean hybridOnly) { - super(getResponseTypes(hybridOnly), "hybrid"); + super(getResponseTypes(hybridOnly), OidcUtils.HYBRID_FLOW); } private static Set<String> getResponseTypes(boolean hybridOnly) { @@ -72,19 +73,20 @@ public class OidcHybridService extends OidcImplicitService { List<String> approvedScope, UserSubject userSubject, ServerAccessToken preAuthorizedToken) { - String code = null; + ServerAuthorizationCodeGrant codeGrant = null; if (state.getResponseType() != null && state.getResponseType().startsWith(OAuthConstants.CODE_RESPONSE_TYPE)) { - code = codeService.getGrantCode(state, client, requestedScope, - approvedScope, userSubject, preAuthorizedToken); - JAXRSUtils.getCurrentMessage().getExchange().put(OAuthConstants.AUTHORIZATION_CODE_VALUE, code); + codeGrant = codeService.getGrantRepresentation( + state, client, requestedScope, approvedScope, userSubject, preAuthorizedToken); + JAXRSUtils.getCurrentMessage().getExchange().put(OAuthConstants.AUTHORIZATION_CODE_VALUE, + codeGrant.getCode()); } StringBuilder sb = super.prepareGrant(state, client, requestedScope, approvedScope, userSubject, preAuthorizedToken); - if (code != null) { + if (codeGrant != null) { sb.append("&"); - sb.append(OAuthConstants.AUTHORIZATION_CODE_VALUE).append("=").append(code); + sb.append(OAuthConstants.AUTHORIZATION_CODE_VALUE).append("=").append(codeGrant.getCode()); } return sb; } http://git-wip-us.apache.org/repos/asf/cxf/blob/f9a42a52/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/utils/OidcUtils.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/utils/OidcUtils.java b/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/utils/OidcUtils.java index b29e16a..1f717c1 100644 --- a/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/utils/OidcUtils.java +++ b/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/utils/OidcUtils.java @@ -46,6 +46,7 @@ public final class OidcUtils { public static final String CODE_ID_TOKEN_RESPONSE_TYPE = "code id_token"; public static final String CODE_ID_TOKEN_AT_RESPONSE_TYPE = "code id_token token"; + public static final String HYBRID_FLOW = "hybrid"; public static final String ID_TOKEN = "id_token"; public static final String OPENID_SCOPE = "openid"; http://git-wip-us.apache.org/repos/asf/cxf/blob/f9a42a52/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oidc/OIDCFlowTest.java ---------------------------------------------------------------------- diff --git a/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oidc/OIDCFlowTest.java b/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oidc/OIDCFlowTest.java index bcf0db6..f6f5a39 100644 --- a/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oidc/OIDCFlowTest.java +++ b/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oidc/OIDCFlowTest.java @@ -50,6 +50,7 @@ import org.apache.cxf.systest.jaxrs.security.oauth2.common.OAuth2TestUtils.Autho import org.apache.cxf.testutil.common.AbstractBusClientServerTestBase; import org.apache.cxf.testutil.common.TestUtil; import org.apache.wss4j.common.util.Loader; + import org.junit.Assert; import org.junit.BeforeClass; @@ -438,6 +439,7 @@ public class OIDCFlowTest extends AbstractBusClientServerTestBase { String address = "https://localhost:"; + PORT + "/services/"; WebClient client = WebClient.create(address, OAuth2TestUtils.setupProviders(), "alice", "security", busFile.toString()); + WebClient.getConfig(client).getHttpConduit().getClient().setReceiveTimeout(100000000); // Save the Cookie for the second request... WebClient.getConfig(client).getRequestContext().put( org.apache.cxf.message.Message.MAINTAIN_SESSION, Boolean.TRUE); @@ -461,6 +463,10 @@ public class OIDCFlowTest extends AbstractBusClientServerTestBase { String idToken = OAuth2TestUtils.getSubstring(location, "id_token"); assertNotNull(idToken); validateIdToken(idToken, "123456789"); + // check the code hash is returned from the implicit authorization endpoint + JwsJwtCompactConsumer jwtConsumer = new JwsJwtCompactConsumer(idToken); + JwtToken jwt = jwtConsumer.getJwtToken(); + Assert.assertNotNull(jwt.getClaims().getClaim(IdToken.AUTH_CODE_HASH_CLAIM)); // Now get the access token client = WebClient.create(address, OAuth2TestUtils.setupProviders(), @@ -478,10 +484,10 @@ public class OIDCFlowTest extends AbstractBusClientServerTestBase { idToken = accessToken.getParameters().get("id_token"); assertNotNull(idToken); validateIdToken(idToken, null); - - // JwsJwtCompactConsumer jwtConsumer = new JwsJwtCompactConsumer(idToken); - // JwtToken jwt = jwtConsumer.getJwtToken(); - // TODO Assert.assertNotNull(jwt.getClaims().getClaim(IdToken.AUTH_CODE_HASH_CLAIM)); + // check the code hash is returned from the token endpoint + jwtConsumer = new JwsJwtCompactConsumer(idToken); + jwt = jwtConsumer.getJwtToken(); + Assert.assertNotNull(jwt.getClaims().getClaim(IdToken.AUTH_CODE_HASH_CLAIM)); } @org.junit.Test @@ -505,14 +511,42 @@ public class OIDCFlowTest extends AbstractBusClientServerTestBase { String location = OAuth2TestUtils.getLocation(client, parameters); assertNotNull(location); - + // Check code String code = OAuth2TestUtils.getSubstring(location, "code"); assertNotNull(code); + // Check id_token + String idToken = OAuth2TestUtils.getSubstring(location, "id_token"); + assertNull(idToken); + // Check Access Token - String accessToken = OAuth2TestUtils.getSubstring(location, "access_token"); - assertNotNull(accessToken); + String implicitAccessToken = OAuth2TestUtils.getSubstring(location, "access_token"); + assertNotNull(implicitAccessToken); + + idToken = OAuth2TestUtils.getSubstring(location, "id_token"); + assertNull(idToken); + + // Now get the access token with the code + client = WebClient.create(address, OAuth2TestUtils.setupProviders(), + "consumer-id", "this-is-a-secret", busFile.toString()); + // Save the Cookie for the second request... + WebClient.getConfig(client).getRequestContext().put( + org.apache.cxf.message.Message.MAINTAIN_SESSION, Boolean.TRUE); + + ClientAccessToken accessToken = + OAuth2TestUtils.getAccessTokenWithAuthorizationCode(client, code); + assertNotNull(accessToken.getTokenKey()); + assertTrue(accessToken.getApprovedScope().contains("openid")); + + // Check id_token from the token endpoint + idToken = accessToken.getParameters().get("id_token"); + assertNotNull(idToken); + validateIdToken(idToken, null); + // check the code hash is returned from the token endpoint + JwsJwtCompactConsumer jwtConsumer = new JwsJwtCompactConsumer(idToken); + // returning c_hash in the id_token returned after exchanging the code is optional + Assert.assertNull(jwtConsumer.getJwtClaims().getClaim(IdToken.AUTH_CODE_HASH_CLAIM)); } @org.junit.Test @@ -546,15 +580,20 @@ public class OIDCFlowTest extends AbstractBusClientServerTestBase { assertNotNull(idToken); validateIdToken(idToken, "123456789"); + // check the code hash is returned from the implicit authorization endpoint + JwsJwtCompactConsumer jwtConsumer = new JwsJwtCompactConsumer(idToken); + JwtToken jwt = jwtConsumer.getJwtToken(); + Assert.assertNotNull(jwt.getClaims().getClaim(IdToken.AUTH_CODE_HASH_CLAIM)); + // Check Access Token String accessToken = OAuth2TestUtils.getSubstring(location, "access_token"); assertNotNull(accessToken); - JwsJwtCompactConsumer jwtConsumer = new JwsJwtCompactConsumer(idToken); - JwtToken jwt = jwtConsumer.getJwtToken(); + jwtConsumer = new JwsJwtCompactConsumer(idToken); + jwt = jwtConsumer.getJwtToken(); Assert.assertNotNull(jwt.getClaims().getClaim(IdToken.ACCESS_TOKEN_HASH_CLAIM)); OidcUtils.validateAccessTokenHash(accessToken, jwt, true); - // TODO Assert.assertNotNull(jwt.getClaims().getClaim(IdToken.AUTH_CODE_HASH_CLAIM)); + Assert.assertNotNull(jwt.getClaims().getClaim(IdToken.AUTH_CODE_HASH_CLAIM)); } @org.junit.Test
