From 14911219309a80bd1ceee8053f8ae32e1f27cad7 Mon Sep 17 00:00:00 2001 From: Jason Erickson Date: Tue, 17 Jan 2017 12:12:04 -0800 Subject: [PATCH 1/3] DefaultTokenResponse.toJson did not include id_token in the JSON #1239 --- extensions/oauth/pom.xml | 4 + .../oauth/authz/DefaultTokenResponse.java | 25 +++-- .../authz/DefaultTokenResponseTest.groovy | 102 ++++++++++++++++++ 3 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 extensions/oauth/src/test/groovy/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponseTest.groovy diff --git a/extensions/oauth/pom.xml b/extensions/oauth/pom.xml index b59ce25668..73e1a82093 100644 --- a/extensions/oauth/pom.xml +++ b/extensions/oauth/pom.xml @@ -58,6 +58,10 @@ org.slf4j jcl-over-slf4j + + org.hamcrest + hamcrest-library + diff --git a/extensions/oauth/src/main/java/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponse.java b/extensions/oauth/src/main/java/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponse.java index 6d485eae99..65a62ae3ae 100644 --- a/extensions/oauth/src/main/java/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponse.java +++ b/extensions/oauth/src/main/java/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponse.java @@ -1,11 +1,11 @@ package com.stormpath.sdk.impl.oauth.authz; import com.stormpath.sdk.lang.Assert; +import com.stormpath.sdk.lang.Strings; import com.stormpath.sdk.oauth.TokenResponse; import org.apache.oltu.oauth2.as.response.OAuthASResponse; -import org.apache.oltu.oauth2.common.exception.OAuthSystemException; -import org.apache.oltu.oauth2.common.message.OAuthResponse; import org.apache.oltu.oauth2.common.message.types.TokenType; +import org.json.JSONObject; import javax.servlet.http.HttpServletResponse; @@ -26,7 +26,7 @@ public class DefaultTokenResponse implements TokenResponse { private final String applicationHref; - private final OAuthResponse oAuthResponse; + private final JSONObject oAuthResponse; private final String idToken; private DefaultTokenResponse(Builder builder) { @@ -42,10 +42,19 @@ private DefaultTokenResponse(Builder builder) { Assert.hasText(expiresIn); Assert.hasText(applicationHref); - try { - oAuthResponse = builder.tokenResponseBuilder.buildJSONMessage(); - } catch (OAuthSystemException e) { - throw new IllegalStateException("Unexpected error when building Json OAuth response.", e); + oAuthResponse = new JSONObject(); + initOAuthResponse(); + } + + private void initOAuthResponse() { + oAuthResponse.put("token_type", tokenType); + oAuthResponse.put("access_token", accessToken); + oAuthResponse.put("expires_in", Long.parseLong(expiresIn)); + if (Strings.hasText(refreshToken)) { + oAuthResponse.put("refresh_token", refreshToken); + } + if (Strings.hasText(idToken)) { + oAuthResponse.put("id_token", idToken); } } @@ -81,7 +90,7 @@ public String getApplicationHref() { @Override public String toJson() { - return oAuthResponse.getBody(); + return oAuthResponse.toString(); } public static Builder tokenType(TokenType tokenType) { diff --git a/extensions/oauth/src/test/groovy/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponseTest.groovy b/extensions/oauth/src/test/groovy/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponseTest.groovy new file mode 100644 index 0000000000..a24660f306 --- /dev/null +++ b/extensions/oauth/src/test/groovy/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponseTest.groovy @@ -0,0 +1,102 @@ +package com.stormpath.sdk.impl.oauth.authz + +import com.stormpath.sdk.oauth.TokenResponse +import org.apache.oltu.oauth2.common.message.types.TokenType +import org.hamcrest.Matchers +import org.json.JSONObject +import org.testng.annotations.Test + +import static Matchers.is +import static org.hamcrest.MatcherAssert.assertThat + +class DefaultTokenResponseTest { + + public static final String APP_HREF = "http://test.app.href.com" + public static final String ACCESS_TOKEN = "testAccessToken" + public static final String REFRESH_TOKEN = "testRefreshToken" + public static final String ID_TOKEN = "testIdToken" + public static final String SCOPE = "test scope" + public static final String EXPIRES_IN = "3600" + public static final String TOKEN_TYPE = "Bearer" + + @Test + void testBuildCompleteResponse() { + TokenResponse tokenResponse = DefaultTokenResponse.tokenType(TokenType.BEARER) + .accessToken(ACCESS_TOKEN) + .refreshToken(REFRESH_TOKEN) + .idToken(ID_TOKEN) + .scope(SCOPE) + .expiresIn(EXPIRES_IN) + .applicationHref(APP_HREF) + .build() + assertThat(tokenResponse.tokenType, is(TOKEN_TYPE)) + assertThat(tokenResponse.accessToken, is(ACCESS_TOKEN)) + assertThat(tokenResponse.refreshToken, is(REFRESH_TOKEN)) + assertThat(tokenResponse.idToken, is(ID_TOKEN)) + assertThat(tokenResponse.scope, is(SCOPE)) + assertThat(tokenResponse.expiresIn, is(EXPIRES_IN)) + assertThat(tokenResponse.applicationHref, is(APP_HREF)) + } + + @Test + void testJsonWithOnlyAccessToken() { + TokenResponse tokenResponse = DefaultTokenResponse.tokenType(TokenType.BEARER) + .accessToken(ACCESS_TOKEN) + .expiresIn(EXPIRES_IN) + .applicationHref(APP_HREF) + .build() + + String json = tokenResponse.toJson() + JSONObject actual = new JSONObject(json) + assertField(actual, "token_type", TOKEN_TYPE) + assertField(actual, "access_token", ACCESS_TOKEN) + assertField(actual, "expires_in", EXPIRES_IN) + assertNoField(actual, "refresh_token") + assertNoField(actual, "id_token") + } + + @Test + void testJsonWithAccessAndRefreshTokens() { + TokenResponse tokenResponse = DefaultTokenResponse.tokenType(TokenType.BEARER) + .accessToken(ACCESS_TOKEN) + .refreshToken(REFRESH_TOKEN) + .expiresIn(EXPIRES_IN) + .applicationHref(APP_HREF) + .build() + + String json = tokenResponse.toJson() + JSONObject actual = new JSONObject(json) + assertField(actual, "token_type", TOKEN_TYPE) + assertField(actual, "access_token", ACCESS_TOKEN) + assertField(actual, "refresh_token", REFRESH_TOKEN) + assertField(actual, "expires_in", EXPIRES_IN) + assertNoField(actual, "id_token") + } + + @Test + void testJsonWithAccessAndRefreshAndIdTokens() { + TokenResponse tokenResponse = DefaultTokenResponse.tokenType(TokenType.BEARER) + .accessToken(ACCESS_TOKEN) + .refreshToken(REFRESH_TOKEN) + .idToken(ID_TOKEN) + .expiresIn(EXPIRES_IN) + .applicationHref(APP_HREF) + .build() + + String json = tokenResponse.toJson() + JSONObject actual = new JSONObject(json) + assertField(actual, "token_type", TOKEN_TYPE) + assertField(actual, "access_token", ACCESS_TOKEN) + assertField(actual, "refresh_token", REFRESH_TOKEN) + assertField(actual, "id_token", ID_TOKEN) + assertField(actual, "expires_in", EXPIRES_IN) + } + + private static void assertField(JSONObject actual, String field, String expected) { + assertThat("${field} in ${actual.toString(2)}", actual.optString(field), is(expected)) + } + + private static void assertNoField(JSONObject actual, String field) { + assertThat("${field} present in ${actual.toString(2)}", actual.has(field), is(false)) + } +} From a56c68eef222d3001ebb98dc325f36a72490686a Mon Sep 17 00:00:00 2001 From: Jason Erickson Date: Tue, 17 Jan 2017 14:04:08 -0800 Subject: [PATCH 2/3] DefaultTokenResponse.toJson did not include id_token in the JSON #1239 --- .../stormpath/sdk/impl/oauth/authz/DefaultTokenResponse.java | 3 +++ .../sdk/impl/oauth/authz/DefaultTokenResponseTest.groovy | 2 ++ 2 files changed, 5 insertions(+) diff --git a/extensions/oauth/src/main/java/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponse.java b/extensions/oauth/src/main/java/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponse.java index 65a62ae3ae..ce2ec93fc0 100644 --- a/extensions/oauth/src/main/java/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponse.java +++ b/extensions/oauth/src/main/java/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponse.java @@ -50,6 +50,9 @@ private void initOAuthResponse() { oAuthResponse.put("token_type", tokenType); oAuthResponse.put("access_token", accessToken); oAuthResponse.put("expires_in", Long.parseLong(expiresIn)); + if (Strings.hasText(scope)) { + oAuthResponse.put("scope", scope); + } if (Strings.hasText(refreshToken)) { oAuthResponse.put("refresh_token", refreshToken); } diff --git a/extensions/oauth/src/test/groovy/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponseTest.groovy b/extensions/oauth/src/test/groovy/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponseTest.groovy index a24660f306..be098e7c4b 100644 --- a/extensions/oauth/src/test/groovy/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponseTest.groovy +++ b/extensions/oauth/src/test/groovy/com/stormpath/sdk/impl/oauth/authz/DefaultTokenResponseTest.groovy @@ -43,6 +43,7 @@ class DefaultTokenResponseTest { TokenResponse tokenResponse = DefaultTokenResponse.tokenType(TokenType.BEARER) .accessToken(ACCESS_TOKEN) .expiresIn(EXPIRES_IN) + .scope(SCOPE) .applicationHref(APP_HREF) .build() @@ -51,6 +52,7 @@ class DefaultTokenResponseTest { assertField(actual, "token_type", TOKEN_TYPE) assertField(actual, "access_token", ACCESS_TOKEN) assertField(actual, "expires_in", EXPIRES_IN) + assertField(actual, "scope", SCOPE) assertNoField(actual, "refresh_token") assertNoField(actual, "id_token") } From 2121bf944a2e7caf10322b52862e8d1e8ccff989 Mon Sep 17 00:00:00 2001 From: Micah Silverman Date: Wed, 18 Jan 2017 01:23:50 -0500 Subject: [PATCH 3/3] added test scope for hamcrest in oauth --- extensions/oauth/pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/oauth/pom.xml b/extensions/oauth/pom.xml index 73e1a82093..df526f5b5d 100644 --- a/extensions/oauth/pom.xml +++ b/extensions/oauth/pom.xml @@ -61,6 +61,7 @@ org.hamcrest hamcrest-library + test