Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Using google-auth-library-oauth2-http to get default credentials #151

Merged
merged 1 commit into from
Aug 5, 2019
Merged

Using google-auth-library-oauth2-http to get default credentials #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@
<version>v1beta4-rev20190510-1.28.0</version>
</dependency>

<dependency>
<groupId>com.google.auth</groupId>
<artifactId>google-auth-library-oauth2-http</artifactId>
<version>0.16.2</version>
</dependency>

<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/com/google/cloud/sql/CredentialFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
package com.google.cloud.sql;

import com.google.api.client.auth.oauth2.Credential;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import?

Copy link
Contributor Author

@yhrn yhrn Aug 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually used in the class comment but maybe the comment should be updated? I left it as is because I figured that in most cases it would be a Credential that is returned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah that's fine then.

import com.google.api.client.http.HttpRequestInitializer;

/** Factory for creating {@link Credential}s for interaction with Cloud SQL Admin API. */
public interface CredentialFactory {
/** Name of system property that can specify an alternative credential factory. */
String CREDENTIAL_FACTORY_PROPERTY = "cloudSql.socketFactory.credentialFactory";

Credential create();
HttpRequestInitializer create();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking changing to a public interface.
I would expect a major version bump for this to permissible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might break binary compatibility, but it's source compatible

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize Credential implements HttpRequestInitializer. This is actually OK then.

}
45 changes: 10 additions & 35 deletions core/src/main/java/com/google/cloud/sql/core/CoreSocketFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

package com.google.cloud.sql.core;

import com.google.api.client.auth.oauth2.Credential;
import com.google.api.client.googleapis.auth.oauth2.GoogleCredential;
import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport;
import com.google.api.client.http.HttpRequestInitializer;
import com.google.api.client.http.HttpTransport;
Expand All @@ -26,6 +24,8 @@
import com.google.api.services.sqladmin.SQLAdmin;
import com.google.api.services.sqladmin.SQLAdmin.Builder;
import com.google.api.services.sqladmin.SQLAdminScopes;
import com.google.auth.http.HttpCredentialsAdapter;
import com.google.auth.oauth2.GoogleCredentials;
import com.google.cloud.sql.CredentialFactory;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
Expand All @@ -40,8 +40,6 @@
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand All @@ -50,7 +48,6 @@
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.logging.Logger;
import javax.annotation.Nullable;
import javax.net.ssl.SSLSocket;
import jnr.unixsocket.UnixSocketAddress;
import jnr.unixsocket.UnixSocketChannel;
Expand Down Expand Up @@ -90,9 +87,7 @@ public final class CoreSocketFactory {

private static CoreSocketFactory coreSocketFactory;

private final CertificateFactory certificateFactory;
private final ListenableFuture<KeyPair> localKeyPair;
private final Credential credential;
private final ConcurrentHashMap<String, CloudSqlInstance> instances = new ConcurrentHashMap<>();
private final ListeningScheduledExecutorService executor;
private final SQLAdmin adminApi;
Expand All @@ -101,12 +96,9 @@ public final class CoreSocketFactory {
@VisibleForTesting
CoreSocketFactory(
ListenableFuture<KeyPair> localKeyPair,
Credential credential,
SQLAdmin adminApi,
int serverProxyPort,
ListeningScheduledExecutorService executor) {
this.certificateFactory = x509CertificateFactory();
this.credential = credential;
this.adminApi = adminApi;
this.serverProxyPort = serverProxyPort;
this.executor = executor;
Expand All @@ -131,30 +123,20 @@ public static synchronized CoreSocketFactory getInstance() {
credentialFactory = new ApplicationDefaultCredentialFactory();
}

Credential credential = credentialFactory.create();
HttpRequestInitializer credential = credentialFactory.create();
SQLAdmin adminApi = createAdminApiClient(credential);
ListeningScheduledExecutorService executor = getDefaultExecutor();

coreSocketFactory =
new CoreSocketFactory(
executor.submit(CoreSocketFactory::generateRsaKeyPair),
credential,
adminApi,
DEFAULT_SERVER_PROXY_PORT,
executor);
}
return coreSocketFactory;
}

// Returns a factory used to create X.509 public certificates
private static CertificateFactory x509CertificateFactory() {
try {
return CertificateFactory.getInstance("X.509");
} catch (CertificateException err) {
throw new RuntimeException("X509 implementation not available", err);
}
}

// TODO(kvg): Figure out better executor to use for testing
@VisibleForTesting
// Returns a listenable, scheduled executor that exits upon shutdown.
Expand Down Expand Up @@ -283,13 +265,6 @@ private static List<String> listIpTypes(String cloudSqlIpTypes) {
return result;
}

@Nullable
private String getCredentialServiceAccount(Credential credential) {
return credential instanceof GoogleCredential
? ((GoogleCredential) credential).getServiceAccountId()
: null;
}

private static SQLAdmin createAdminApiClient(HttpRequestInitializer requestInitializer) {
HttpTransport httpTransport;
try {
Expand Down Expand Up @@ -318,19 +293,19 @@ private static SQLAdmin createAdminApiClient(HttpRequestInitializer requestIniti

private static class ApplicationDefaultCredentialFactory implements CredentialFactory {
@Override
public Credential create() {
GoogleCredential credential;
public HttpRequestInitializer create() {
GoogleCredentials credentials;
try {
credential = GoogleCredential.getApplicationDefault();
credentials = GoogleCredentials.getApplicationDefault();
} catch (IOException err) {
throw new RuntimeException(
"Unable to obtain credentials to communicate with the Cloud SQL API", err);
}
if (credential.createScopedRequired()) {
credential =
credential.createScoped(Collections.singletonList(SQLAdminScopes.SQLSERVICE_ADMIN));
if (credentials.createScopedRequired()) {
credentials =
credentials.createScoped(Collections.singletonList(SQLAdminScopes.SQLSERVICE_ADMIN));
}
return credential;
return new HttpCredentialsAdapter(credentials);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why not make this method return com.google.auth.Credentials instead and do the wrapping in the caller?
You would change the method signature to Credentials create() as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be a source code breaking change, com.google.api.client.auth.oauth2.Credential implements com.google.api.client.http.HttpRequestInitializer so this way the PR is source code compatible - existing implementations of com.google.cloud.sql.CredentialFactory will not need any source code changes, only be recompiled against the new library version. com.google.auth.Credentials on the other hand is a completely separate API and would require all existing implementations to change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You right. Thanks for the clarification.

}
}

Expand Down
16 changes: 8 additions & 8 deletions core/src/test/java/com/google/cloud/sql/core/CoreSocketFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public void setup() throws IOException, GeneralSecurityException, ExecutionExcep
@Test
public void create_throwsErrorForInvalidInstanceName() throws IOException {
CoreSocketFactory coreSocketFactory =
new CoreSocketFactory(clientKeyPair, credential, adminApi, 3307, defaultExecutor);
new CoreSocketFactory(clientKeyPair, adminApi, 3307, defaultExecutor);
try {
coreSocketFactory.createSslSocket("myProject", Arrays.asList("PRIMARY"));
fail();
Expand All @@ -195,7 +195,7 @@ public void create_throwsErrorForInvalidInstanceName() throws IOException {
@Test
public void create_throwsErrorForInvalidInstanceRegion() throws IOException {
CoreSocketFactory coreSocketFactory =
new CoreSocketFactory(clientKeyPair, credential, adminApi, 3307, defaultExecutor);
new CoreSocketFactory(clientKeyPair, adminApi, 3307, defaultExecutor);
try {
coreSocketFactory.createSslSocket(
"myProject:notMyRegion:myInstance", Arrays.asList("PRIMARY"));
Expand All @@ -218,7 +218,7 @@ public void create_successfulPrivateConnection()
int port = sslServer.start(PRIVATE_IP);

CoreSocketFactory coreSocketFactory =
new CoreSocketFactory(clientKeyPair, credential, adminApi, port, defaultExecutor);
new CoreSocketFactory(clientKeyPair, adminApi, port, defaultExecutor);
Socket socket =
coreSocketFactory.createSslSocket(
"myProject:myRegion:myInstance", Arrays.asList("PRIVATE"));
Expand All @@ -240,7 +240,7 @@ public void create_successfulConnection() throws IOException, InterruptedExcepti
int port = sslServer.start();

CoreSocketFactory coreSocketFactory =
new CoreSocketFactory(clientKeyPair, credential, adminApi, port, defaultExecutor);
new CoreSocketFactory(clientKeyPair, adminApi, port, defaultExecutor);
Socket socket =
coreSocketFactory.createSslSocket(
"myProject:myRegion:myInstance", Arrays.asList("PRIMARY"));
Expand Down Expand Up @@ -272,7 +272,7 @@ public void create_expiredCertificateOnFirstConnection_certificateRenewed()
.thenReturn(new SslCert().setCert(createEphemeralCert(Duration.ofMinutes(65))));

CoreSocketFactory coreSocketFactory =
new CoreSocketFactory(clientKeyPair, credential, adminApi, port, defaultExecutor);
new CoreSocketFactory(clientKeyPair, adminApi, port, defaultExecutor);
Socket socket =
coreSocketFactory.createSslSocket(
"myProject:myRegion:myInstance", Arrays.asList("PRIMARY"));
Expand All @@ -294,7 +294,7 @@ public void create_certificateReusedIfNotExpired() throws IOException, Interrupt
int port = sslServer.start();

CoreSocketFactory coreSocketFactory =
new CoreSocketFactory(clientKeyPair, credential, adminApi, port, defaultExecutor);
new CoreSocketFactory(clientKeyPair, adminApi, port, defaultExecutor);
coreSocketFactory.createSslSocket("myProject:myRegion:myInstance", Arrays.asList("PRIMARY"));

verify(adminApiInstances).get("myProject", "myInstance");
Expand All @@ -311,7 +311,7 @@ public void create_certificateReusedIfNotExpired() throws IOException, Interrupt
@Test
public void create_adminApiNotEnabled() throws IOException {
CoreSocketFactory coreSocketFactory =
new CoreSocketFactory(clientKeyPair, credential, adminApi, 3307, defaultExecutor);
new CoreSocketFactory(clientKeyPair, adminApi, 3307, defaultExecutor);
try {
// Use a different project to get Api Not Enabled Error.
coreSocketFactory.createSslSocket(
Expand All @@ -331,7 +331,7 @@ public void create_adminApiNotEnabled() throws IOException {
@Test
public void create_notAuthorized() throws IOException {
CoreSocketFactory coreSocketFactory =
new CoreSocketFactory(clientKeyPair, credential, adminApi, 3307, defaultExecutor);
new CoreSocketFactory(clientKeyPair, adminApi, 3307, defaultExecutor);
try {
// Use a different instance to simulate incorrect permissions.
coreSocketFactory.createSslSocket(
Expand Down