From 6be4018e88dda4bfb4891ce043b50d7d2f59322c Mon Sep 17 00:00:00 2001 From: Iain McGinniss Date: Fri, 26 Feb 2016 18:41:56 -0800 Subject: [PATCH] Move request URI generation into AuthorizationRequest --- library/build.gradle | 1 + .../openid/appauth/AuthorizationRequest.java | 51 +++++++++++ .../openid/appauth/AuthorizationService.java | 32 +------ library/java/net/openid/appauth/UriUtil.java | 11 +++ .../appauth/AuthorizationRequestTest.java | 86 +++++++++++++++++++ 5 files changed, 150 insertions(+), 31 deletions(-) diff --git a/library/build.gradle b/library/build.gradle index 8af126d6..7d2ebc58 100644 --- a/library/build.gradle +++ b/library/build.gradle @@ -49,6 +49,7 @@ dependencies { testCompile 'junit:junit:4.12' testCompile 'org.mockito:mockito-core:1.10.19' testCompile 'org.robolectric:robolectric:2.4' + testCompile 'com.squareup.assertj:assertj-android:1.1.1' } checkstyle { diff --git a/library/java/net/openid/appauth/AuthorizationRequest.java b/library/java/net/openid/appauth/AuthorizationRequest.java index 5c8f8d03..8e8857f5 100644 --- a/library/java/net/openid/appauth/AuthorizationRequest.java +++ b/library/java/net/openid/appauth/AuthorizationRequest.java @@ -20,6 +20,7 @@ import android.net.Uri; import android.support.annotation.NonNull; import android.support.annotation.Nullable; +import android.support.annotation.VisibleForTesting; import android.text.TextUtils; import android.util.Base64; @@ -126,6 +127,30 @@ public class AuthorizationRequest { */ public static final String CODE_CHALLENGE_METHOD_PLAIN = "plain"; + @VisibleForTesting + static final String PARAM_CLIENT_ID = "client_id"; + + @VisibleForTesting + static final String PARAM_CODE_CHALLENGE = "code_challenge"; + + @VisibleForTesting + static final String PARAM_CODE_CHALLENGE_METHOD = "code_challenge_method"; + + @VisibleForTesting + static final String PARAM_REDIRECT_URI = "redirect_uri"; + + @VisibleForTesting + static final String PARAM_RESPONSE_MODE = "response_mode"; + + @VisibleForTesting + static final String PARAM_RESPONSE_TYPE = "response_type"; + + @VisibleForTesting + static final String PARAM_SCOPE = "scope"; + + @VisibleForTesting + static final String PARAM_STATE = "state"; + private static final String KEY_CONFIGURATION = "configuration"; private static final String KEY_CLIENT_ID = "clientId"; private static final String KEY_RESPONSE_TYPE = "responseType"; @@ -612,6 +637,32 @@ public Set getScopeSet() { return ScopeUtil.scopeStringToSet(scope); } + /** + * Produces a request URI, that can be used to dispath the authorization request. + */ + @NonNull + public Uri toUri() { + Uri.Builder uriBuilder = configuration.authorizationEndpoint.buildUpon() + .appendQueryParameter(PARAM_REDIRECT_URI, redirectUri.toString()) + .appendQueryParameter(PARAM_CLIENT_ID, clientId) + .appendQueryParameter(PARAM_RESPONSE_TYPE, responseType); + + UriUtil.appendQueryParameterIfNotNull(uriBuilder, PARAM_STATE, state); + UriUtil.appendQueryParameterIfNotNull(uriBuilder, PARAM_SCOPE, scope); + UriUtil.appendQueryParameterIfNotNull(uriBuilder, PARAM_RESPONSE_MODE, responseMode); + + if (codeVerifier != null) { + uriBuilder.appendQueryParameter(PARAM_CODE_CHALLENGE, codeVerifierChallenge) + .appendQueryParameter(PARAM_CODE_CHALLENGE_METHOD, codeVerifierChallengeMethod); + } + + for (Entry entry : additionalParameters.entrySet()) { + uriBuilder.appendQueryParameter(entry.getKey(), entry.getValue()); + } + + return uriBuilder.build(); + } + /** * Produces a JSON representation of the request for storage or transmission. */ diff --git a/library/java/net/openid/appauth/AuthorizationService.java b/library/java/net/openid/appauth/AuthorizationService.java index e1b7ccdc..911bdab9 100644 --- a/library/java/net/openid/appauth/AuthorizationService.java +++ b/library/java/net/openid/appauth/AuthorizationService.java @@ -224,38 +224,8 @@ public void performAuthorizationRequest( @NonNull PendingIntent resultHandlerIntent, @NonNull CustomTabsIntent customTabsIntent) { checkNotDisposed(); - Uri.Builder uriBuilder = request.configuration.authorizationEndpoint.buildUpon() - .appendQueryParameter(REDIRECT_URI, request.redirectUri.toString()) - .appendQueryParameter(CLIENT_ID, request.clientId) - .appendQueryParameter(RESPONSE_TYPE, request.responseType); - - if (request.state != null) { - uriBuilder.appendQueryParameter(STATE, request.state); - } - - if (request.codeVerifier != null) { - uriBuilder - .appendQueryParameter(CODE_CHALLENGE, - request.codeVerifierChallenge) - .appendQueryParameter(CODE_CHALLENGE_METHOD, - request.codeVerifierChallengeMethod); - } - - if (request.scope != null) { - uriBuilder.appendQueryParameter(SCOPE, request.scope); - } - - if (request.responseMode != null) { - uriBuilder.appendQueryParameter(RESPONSE_MODE, request.responseMode); - } - - for (String key : request.additionalParameters.keySet()) { - String value = request.additionalParameters.get(key); - uriBuilder.appendQueryParameter(key, value); - } - Uri requestUri = uriBuilder.build(); + Uri requestUri = request.toUri(); PendingIntentStore.getInstance().addPendingIntent(request, resultHandlerIntent); - Intent intent = customTabsIntent.intent; intent.setData(requestUri); if (TextUtils.isEmpty(intent.getPackage())) { diff --git a/library/java/net/openid/appauth/UriUtil.java b/library/java/net/openid/appauth/UriUtil.java index b4409634..ef32acc2 100644 --- a/library/java/net/openid/appauth/UriUtil.java +++ b/library/java/net/openid/appauth/UriUtil.java @@ -37,6 +37,17 @@ public static Uri parseUriIfAvailable(@Nullable String uri) { return Uri.parse(uri); } + public static void appendQueryParameterIfNotNull( + @NonNull Uri.Builder uriBuilder, + @NonNull String paramName, + @Nullable String value) { + if (value == null) { + return; + } + + uriBuilder.appendQueryParameter(paramName, value); + } + public static Map extractAdditionalParameters( @NonNull Uri uri, @NonNull Set ignore) { diff --git a/library/javatests/net/openid/appauth/AuthorizationRequestTest.java b/library/javatests/net/openid/appauth/AuthorizationRequestTest.java index 623ba881..36a2f989 100644 --- a/library/javatests/net/openid/appauth/AuthorizationRequestTest.java +++ b/library/javatests/net/openid/appauth/AuthorizationRequestTest.java @@ -18,17 +18,22 @@ import static net.openid.appauth.TestValues.TEST_CLIENT_ID; import static net.openid.appauth.TestValues.TEST_STATE; import static net.openid.appauth.TestValues.getTestServiceConfig; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import android.net.Uri; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; @RunWith(RobolectricTestRunner.class) @@ -56,8 +61,17 @@ public class AuthorizationRequestTest { private AuthorizationRequest.Builder mRequestBuilder; private AuthorizationRequest mRequest; + private AuthorizationRequest.Builder mMinimalRequestBuilder; + @Before public void setUp() { + + mMinimalRequestBuilder = new AuthorizationRequest.Builder( + getTestServiceConfig(), + TEST_CLIENT_ID, + AuthorizationRequest.RESPONSE_TYPE_CODE, + TEST_APP_REDIRECT_URI); + mRequestBuilder = new AuthorizationRequest.Builder( getTestServiceConfig(), TEST_CLIENT_ID, @@ -182,6 +196,78 @@ public void testScopes_emptyList() { assertNull(request.scope); } + @Test + public void testToUri() throws Exception { + Uri uri = mRequest.toUri(); + + Uri authEndpoint = mRequest.configuration.authorizationEndpoint; + assertThat(uri.getScheme()).isEqualTo(authEndpoint.getScheme()); + assertThat(uri.getAuthority()).isEqualTo(authEndpoint.getAuthority()); + assertThat(uri.getPath()).isEqualTo(authEndpoint.getPath()); + assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_REDIRECT_URI)) + .isEqualTo(mRequest.redirectUri.toString()); + assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_CLIENT_ID)) + .isEqualTo(mRequest.clientId); + assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_RESPONSE_TYPE)) + .isEqualTo(mRequest.responseType); + assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_STATE)) + .isEqualTo(mRequest.state); + assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_SCOPE)) + .isEqualTo(mRequest.scope); + assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_RESPONSE_MODE)) + .isEqualTo(mRequest.responseMode); + assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_CODE_CHALLENGE)) + .isEqualTo(mRequest.codeVerifierChallenge); + assertThat(uri.getQueryParameter(AuthorizationRequest.PARAM_CODE_CHALLENGE_METHOD)) + .isEqualTo(mRequest.codeVerifierChallengeMethod); + } + + @Test + public void testToUri_withMinimalConfiguration() throws Exception { + AuthorizationRequest req = mMinimalRequestBuilder.build(); + + Uri uri = req.toUri(); + assertThat(uri.getQueryParameterNames()) + .isEqualTo(new HashSet<>(Arrays.asList( + AuthorizationRequest.PARAM_CLIENT_ID, + AuthorizationRequest.PARAM_RESPONSE_TYPE, + AuthorizationRequest.PARAM_REDIRECT_URI, + AuthorizationRequest.PARAM_STATE, + AuthorizationRequest.PARAM_CODE_CHALLENGE, + AuthorizationRequest.PARAM_CODE_CHALLENGE_METHOD))); + } + + @Test + public void testToUri_withNoState() throws Exception { + AuthorizationRequest req = mMinimalRequestBuilder.setState(null).build(); + assertThat(req.toUri().getQueryParameterNames()) + .doesNotContain(AuthorizationRequest.PARAM_STATE); + } + + @Test + public void testToUri_withNoVerifier() throws Exception { + AuthorizationRequest req = mMinimalRequestBuilder.setCodeVerifier(null).build(); + assertThat(req.toUri().getQueryParameterNames()) + .doesNotContain(AuthorizationRequest.PARAM_CODE_CHALLENGE) + .doesNotContain(AuthorizationRequest.PARAM_CODE_CHALLENGE_METHOD); + } + + @Test + public void testToUri_withAdditionalParameters() throws Exception { + Map additionalParams = new HashMap<>(); + additionalParams.put("my_param", "1234"); + additionalParams.put("another_param", "5678"); + AuthorizationRequest req = mMinimalRequestBuilder + .setAdditionalParameters(additionalParams) + .build(); + + Uri uri = req.toUri(); + assertThat(uri.getQueryParameter("my_param")) + .isEqualTo("1234"); + assertThat(uri.getQueryParameter("another_param")) + .isEqualTo("5678"); + } + @Test public void testSerialization() throws Exception { AuthorizationRequest request = AuthorizationRequest.fromJson(mRequest.toJson());