From 68bac57da567b97aa10e3123db8c113770ef886c Mon Sep 17 00:00:00 2001 From: mmuller Date: Tue, 21 Feb 2017 12:17:51 -0800 Subject: [PATCH] Store credentials under scope-qualified name Store the auth credentials under a name qualified by the set of OAuth scopes as well as the client id. This is implemented as the base64 encoded SHA1 hash of the concatenation of client id and sorted auth scopes. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=148127911 --- .../tools/DefaultRequestFactoryModule.java | 38 +++++++++++------- .../DefaultRequestFactoryModuleTest.java | 39 +++++++++++++++++++ 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/java/google/registry/tools/DefaultRequestFactoryModule.java b/java/google/registry/tools/DefaultRequestFactoryModule.java index 9856202e0..dfad274d3 100644 --- a/java/google/registry/tools/DefaultRequestFactoryModule.java +++ b/java/google/registry/tools/DefaultRequestFactoryModule.java @@ -29,15 +29,20 @@ import com.google.api.client.json.JsonFactory; import com.google.api.client.json.jackson2.JacksonFactory; import com.google.api.client.util.store.AbstractDataStoreFactory; import com.google.api.client.util.store.FileDataStoreFactory; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Ordering; import dagger.Binds; import dagger.Module; import dagger.Provides; +import google.registry.config.RegistryConfig.Config; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.lang.annotation.Documented; -import java.util.Collections; +import java.util.Collection; import javax.inject.Named; import javax.inject.Provider; import javax.inject.Qualifier; @@ -60,10 +65,6 @@ import javax.inject.Singleton; @Module class DefaultRequestFactoryModule { - // TODO(mmuller): Use @Config("requiredOauthScopes") - private static final String DEFAULT_SCOPE = - "https://www.googleapis.com/auth/userinfo.email"; - private static final File DATA_STORE_DIR = new File(System.getProperty("user.home"), ".config/nomulus/credentials"); @@ -124,8 +125,8 @@ class DefaultRequestFactoryModule { * Module for providing HttpRequestFactory. * *

Localhost connections go to the App Engine dev server. The dev server differs from most HTTP - * connections in that the types whose annotations affect the use of annotaty don't require - * OAuth2 credentials, but instead require a special cookie. + * connections in that it doesn't require OAuth2 credentials, but instead requires a special + * cookie. */ @Module abstract static class RequestFactoryModule { @@ -155,25 +156,23 @@ class DefaultRequestFactoryModule { static class AuthorizerModule { @Provides public Authorizer provideAuthorizer( - final JsonFactory jsonFactory, final AbstractDataStoreFactory dataStoreFactory) { + final JsonFactory jsonFactory, + final AbstractDataStoreFactory dataStoreFactory, + @Config("requiredOauthScopes") final ImmutableSet requiredOauthScopes) { return new Authorizer() { @Override public Credential authorize(GoogleClientSecrets clientSecrets) { try { // Run a new auth flow. GoogleAuthorizationCodeFlow flow = new GoogleAuthorizationCodeFlow.Builder( - new NetHttpTransport(), jsonFactory, clientSecrets, - Collections.singleton(DEFAULT_SCOPE)) + new NetHttpTransport(), jsonFactory, clientSecrets, requiredOauthScopes) .setDataStoreFactory(dataStoreFactory) .build(); - // TODO(mmuller): "credentials" directory needs to be qualified with the scopes and - // client id. - // We pass client id to the authorize method so we can safely persist credentials for - // multiple client ids. return new AuthorizationCodeInstalledApp(flow, new LocalServerReceiver()) - .authorize(clientSecrets.getDetails().getClientId()); + .authorize(createClientScopeQualifier( + clientSecrets.getDetails().getClientId(), requiredOauthScopes)); } catch (Exception ex) { throw new RuntimeException(ex); } @@ -182,6 +181,15 @@ class DefaultRequestFactoryModule { } } + /** + * Create a unique identifier for a given client id and collection of scopes, to be used as an + * identifier for a credential. + */ + @VisibleForTesting + static String createClientScopeQualifier(String clientId, Collection scopes) { + return clientId + " " + Joiner.on(" ").join(Ordering.natural().sortedCopy(scopes)); + } + /** * Interface that encapsulates the authorization logic to produce a credential for the user, * allowing us to override the behavior for unit tests. diff --git a/javatests/google/registry/tools/DefaultRequestFactoryModuleTest.java b/javatests/google/registry/tools/DefaultRequestFactoryModuleTest.java index 8ab9c6f0e..7382b96e8 100644 --- a/javatests/google/registry/tools/DefaultRequestFactoryModuleTest.java +++ b/javatests/google/registry/tools/DefaultRequestFactoryModuleTest.java @@ -15,6 +15,7 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; +import static google.registry.tools.DefaultRequestFactoryModule.createClientScopeQualifier; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -27,6 +28,7 @@ import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpRequestFactory; import com.google.api.client.http.HttpRequestInitializer; import com.google.api.client.util.store.AbstractDataStoreFactory; +import com.google.common.collect.ImmutableList; import com.google.common.net.HostAndPort; import google.registry.testing.Providers; import java.io.IOException; @@ -106,4 +108,41 @@ public class DefaultRequestFactoryModuleTest { credentialProvider); assertThat(factory.getInitializer()).isSameAs(FAKE_CREDENTIAL); } + + @Test + public void test_createClientScopeQualifier() { + String simpleQualifier = + createClientScopeQualifier("client-id", ImmutableList.of("foo", "bar")); + + // If we change the way we encode client id and scopes, this assertion will break. That's + // probably ok and you can just change the text. The things you have to be aware of are: + // - Names in the new encoding should have a low risk of collision with the old encoding. + // - Changing the encoding will force all OAuth users of the nomulus tool to do a new login + // (existing credentials will not be used). + assertThat(simpleQualifier).isEqualTo("client-id bar foo"); + + // Verify order independence. + assertThat(simpleQualifier).isEqualTo( + createClientScopeQualifier("client-id", ImmutableList.of("bar", "foo"))); + + // Verify changing client id produces a different value. + assertThat(simpleQualifier).isNotEqualTo( + createClientScopeQualifier("new-client", ImmutableList.of("bar", "foo"))); + + // Verify that adding/deleting/modifying scopes produces a different value. + assertThat(simpleQualifier).isNotEqualTo( + createClientScopeQualifier("client id", ImmutableList.of("bar", "foo", "baz"))); + assertThat(simpleQualifier).isNotEqualTo( + createClientScopeQualifier("client id", ImmutableList.of("barx", "foo"))); + assertThat(simpleQualifier).isNotEqualTo( + createClientScopeQualifier("client id", ImmutableList.of("bar", "foox"))); + assertThat(simpleQualifier).isNotEqualTo( + createClientScopeQualifier("client id", ImmutableList.of("bar"))); + + // Verify that delimiting works. + assertThat(simpleQualifier).isNotEqualTo( + createClientScopeQualifier("client-id", ImmutableList.of("barf", "oo"))); + assertThat(simpleQualifier).isNotEqualTo( + createClientScopeQualifier("client-idb", ImmutableList.of("ar", "foo"))); + } }