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.

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

feat(gax): add API key authentication to ClientSettings #3137

Merged
merged 42 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
558af8e
added setApiKey() method to client settings
ldetmer Aug 27, 2024
dad48d4
cleaned up and added logic for throwing error if both api key and cre…
ldetmer Aug 28, 2024
f6afbef
fixed formatting
ldetmer Aug 28, 2024
a516d95
fixed formatting
ldetmer Aug 28, 2024
f7ec0fa
wip
ldetmer Sep 12, 2024
db1674b
wip
ldetmer Sep 16, 2024
9a13951
wip
ldetmer Sep 16, 2024
62a3956
clean up
ldetmer Sep 17, 2024
f0a98e0
Merge branch 'main' into api-keys
ldetmer Sep 17, 2024
fa251cf
clean up
ldetmer Sep 17, 2024
334a4e8
cleaned up tests/logic
ldetmer Sep 19, 2024
edb658f
cleaned up formatting
ldetmer Sep 19, 2024
33f64a1
updated to use assertThrows
ldetmer Sep 19, 2024
c92322b
Merge branch 'main' into api-keys
ldetmer Sep 23, 2024
ae0f281
fixed imports
ldetmer Sep 23, 2024
5e346e6
fixed imports
ldetmer Sep 23, 2024
66cc0a7
updated logic to validate if multiple credentials are passed in via a…
ldetmer Sep 23, 2024
69c57e9
formatting
ldetmer Sep 23, 2024
023a4e4
cleanup
ldetmer Sep 23, 2024
d7c7a72
cleanup
ldetmer Sep 24, 2024
0d48f41
cleanup
ldetmer Sep 24, 2024
bce5abb
added handling of deduping credential headers for GRPC calls + additi…
ldetmer Sep 26, 2024
d4670c5
lint fixes
ldetmer Sep 26, 2024
1eda03f
lint fixes + additional showcase coverage
ldetmer Sep 26, 2024
364acae
Merge branch 'main' into api-keys
ldetmer Sep 30, 2024
50cbea1
cleaned up error checking in dedup + updated tests and java doc
ldetmer Sep 30, 2024
351389c
lint fix
ldetmer Sep 30, 2024
ca19304
lint fix
ldetmer Sep 30, 2024
aa6a006
cleaned up java docs so stub settings and client settings are matching
ldetmer Sep 30, 2024
0938405
lint fix
ldetmer Sep 30, 2024
a39bba0
fixed gdch IT tests
ldetmer Sep 30, 2024
f64279e
updated so credential deduping happens during the object build process
ldetmer Sep 30, 2024
703139b
additional cleanup
ldetmer Sep 30, 2024
245f8e2
lint
ldetmer Sep 30, 2024
0dc642e
lint
ldetmer Sep 30, 2024
60fbed4
updated to only dedup API key credential headers
ldetmer Oct 1, 2024
68f38ae
language fixes
ldetmer Oct 1, 2024
d3492b3
lint
ldetmer Oct 1, 2024
1330841
fixed changes to existing tests
ldetmer Oct 1, 2024
768140d
fixed test modifiers
ldetmer Oct 2, 2024
bc798db
Merge branch 'main' into api-keys
ldetmer Oct 2, 2024
e20771d
no longer need to pre-load gdch creds as we're not deduping headers f…
ldetmer Oct 2, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.google.api.gax.rpc.internal.QuotaProjectIdHidingCredentials;
import com.google.api.gax.tracing.ApiTracerFactory;
import com.google.api.gax.tracing.BaseApiTracerFactory;
import com.google.auth.ApiKeyCredentials;
import com.google.auth.Credentials;
import com.google.auth.oauth2.GdchCredentials;
import com.google.auto.value.AutoValue;
Expand Down Expand Up @@ -175,9 +176,19 @@ public static ClientContext create(StubSettings settings) throws IOException {
// A valid EndpointContext should have been created in the StubSettings
EndpointContext endpointContext = settings.getEndpointContext();
String endpoint = endpointContext.resolvedEndpoint();
String apiKey = settings.getApiKey();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe extract the whole section to a method like getCredentials?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately there is some logic that updates Endpoint metadata that is also used in transport provider. Without a more indepth refactor I tried to consolidate as much as possible

Credentials credentials = settings.getCredentialsProvider().getCredentials();
if (apiKey != null && credentials != null) {
ldetmer marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalArgumentException(
"You can not provide both ApiKey and Credentials for a client.");
}
if (apiKey != null) {
// if API key exists it becomes the default credential
credentials = ApiKeyCredentials.create(settings.getApiKey());
}

// check if need to adjust credentials/endpoint/endpointContext for GDC-H
String settingsGdchApiAudience = settings.getGdchApiAudience();
Credentials credentials = settings.getCredentialsProvider().getCredentials();
boolean usingGDCH = credentials instanceof GdchCredentials;
if (usingGDCH) {
// Can only determine if the GDC-H is being used via the Credentials. The Credentials object
Expand All @@ -187,22 +198,7 @@ public static ClientContext create(StubSettings settings) throws IOException {
// Resolve the new endpoint with the GDC-H flow
endpoint = endpointContext.resolvedEndpoint();
// We recompute the GdchCredentials with the audience
String audienceString;
if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) {
audienceString = settingsGdchApiAudience;
} else if (!Strings.isNullOrEmpty(endpoint)) {
audienceString = endpoint;
} else {
throw new IllegalArgumentException("Could not infer GDCH api audience from settings");
}

URI gdchAudienceUri;
try {
gdchAudienceUri = URI.create(audienceString);
} catch (IllegalArgumentException ex) { // thrown when passing a malformed uri string
throw new IllegalArgumentException("The GDC-H API audience string is not a valid URI", ex);
}
credentials = ((GdchCredentials) credentials).createWithGdchAudience(gdchAudienceUri);
credentials = getGdchCredentials(settingsGdchApiAudience, endpoint, credentials);
ldetmer marked this conversation as resolved.
Show resolved Hide resolved
} else if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) {
throw new IllegalArgumentException(
"GDC-H API audience can only be set when using GdchCredentials");
Expand Down Expand Up @@ -291,6 +287,30 @@ public static ClientContext create(StubSettings settings) throws IOException {
.build();
}

/**
* Constructs a new {@link Credentials} object based on credentials provided with a GDC-H audience
*/
private static Credentials getGdchCredentials(
ldetmer marked this conversation as resolved.
Show resolved Hide resolved
String settingsGdchApiAudience, String endpoint, Credentials credentials) throws IOException {
String audienceString;
if (!Strings.isNullOrEmpty(settingsGdchApiAudience)) {
audienceString = settingsGdchApiAudience;
} else if (!Strings.isNullOrEmpty(endpoint)) {
audienceString = endpoint;
} else {
throw new IllegalArgumentException("Could not infer GDCH api audience from settings");
}

URI gdchAudienceUri;
try {
gdchAudienceUri = URI.create(audienceString);
} catch (IllegalArgumentException ex) { // thrown when passing a malformed uri string
throw new IllegalArgumentException("The GDC-H API audience string is not a valid URI", ex);
}
credentials = ((GdchCredentials) credentials).createWithGdchAudience(gdchAudienceUri);
return credentials;
}

/**
* Getting a header map from HeaderProvider and InternalHeaderProvider from settings with Quota
* Project Id.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ public final WatchdogProvider getWatchdogProvider() {
return stubSettings.getStreamWatchdogProvider();
}

public final String getApiKey() {
return stubSettings.getApiKey();
}

/** This method is obsolete. Use {@link #getWatchdogCheckIntervalDuration()} instead. */
@Nonnull
@ObsoleteApi("Use getWatchdogCheckIntervalDuration() instead")
Expand Down Expand Up @@ -144,6 +148,7 @@ public String toString() {
.add("watchdogProvider", getWatchdogProvider())
.add("watchdogCheckInterval", getWatchdogCheckInterval())
.add("gdchApiAudience", getGdchApiAudience())
.add("apiKey", getApiKey())
.toString();
}

Expand Down Expand Up @@ -302,6 +307,18 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) {
return self();
}

/**
* Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in
* [CallContext].
*
* <p>Note: you can not set an API key and credentials object in the same Settings. It will fail
* when creating the client.
*/
public B setApiKey(String apiKey) {
stubSettings.setApiKey(apiKey);
return self();
}

/**
* Gets the ExecutorProvider that was previously set on this Builder. This ExecutorProvider is
* to use for running asynchronous API call logic (such as retries and long-running operations),
Expand Down Expand Up @@ -364,6 +381,11 @@ public WatchdogProvider getWatchdogProvider() {
return stubSettings.getStreamWatchdogProvider();
}

/** Gets the ApiKey that was previously set on this Builder. */
public String getApiKey() {
return stubSettings.getApiKey();
}

/** This method is obsolete. Use {@link #getWatchdogCheckIntervalDuration()} instead */
@Nullable
@ObsoleteApi("Use getWatchdogCheckIntervalDuration() instead")
Expand Down Expand Up @@ -405,6 +427,7 @@ public String toString() {
.add("watchdogProvider", getWatchdogProvider())
.add("watchdogCheckInterval", getWatchdogCheckIntervalDuration())
.add("gdchApiAudience", getGdchApiAudience())
.add("apiKey", getApiKey())
.toString();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public abstract class StubSettings<SettingsT extends StubSettings<SettingsT>> {
// Track if deprecated setExecutorProvider is called
private boolean deprecatedExecutorProviderSet;
@Nonnull private final EndpointContext endpointContext;
private final String apiKey;

/**
* Indicate when creating transport whether it is allowed to use mTLS endpoint instead of the
Expand All @@ -107,6 +108,7 @@ protected StubSettings(Builder builder) {
this.deprecatedExecutorProviderSet = builder.deprecatedExecutorProviderSet;
this.gdchApiAudience = builder.gdchApiAudience;
this.endpointContext = buildEndpointContext(builder);
this.apiKey = builder.apiKey;
}

/**
Expand Down Expand Up @@ -234,6 +236,14 @@ public final String getGdchApiAudience() {
return gdchApiAudience;
}

/**
* Gets the ApiKey that should be used for authentication. If an empty string was provided it will
* return null
*/
public final String getApiKey() {
return (apiKey == null || apiKey.isEmpty()) ? null : apiKey;
ldetmer marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
Expand All @@ -252,6 +262,7 @@ public String toString() {
.add("streamWatchdogCheckInterval", streamWatchdogCheckInterval)
.add("tracerFactory", tracerFactory)
.add("gdchApiAudience", gdchApiAudience)
.add("apiKey", apiKey)
.toString();
}

Expand All @@ -277,6 +288,7 @@ public abstract static class Builder<
private boolean deprecatedExecutorProviderSet;
private String universeDomain;
private final EndpointContext endpointContext;
private String apiKey;

/**
* Indicate when creating transport whether it is allowed to use mTLS endpoint instead of the
Expand All @@ -301,6 +313,7 @@ protected Builder(StubSettings settings) {
this.tracerFactory = settings.tracerFactory;
this.deprecatedExecutorProviderSet = settings.deprecatedExecutorProviderSet;
this.gdchApiAudience = settings.gdchApiAudience;
this.apiKey = settings.apiKey;

// The follow settings will be set to the original user configurations as the
// EndpointContext will be rebuilt in the constructor.
Expand Down Expand Up @@ -353,6 +366,7 @@ protected Builder(ClientContext clientContext) {
this.mtlsEndpoint = null;
this.switchToMtlsEndpointAllowed = false;
this.universeDomain = null;
this.apiKey = null;
// Attempt to create an empty, non-functioning EndpointContext by default. The client will
// have
// a valid EndpointContext with user configurations after the client has been initialized.
Expand Down Expand Up @@ -574,6 +588,15 @@ public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) {
return self();
}

/**
* Sets the API key. The API key will get translated to an [ApiKeyCredentials] and stored in
* [CallContext].
*/
public B setApiKey(String apiKey) {
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
this.apiKey = apiKey;
return self();
}

/** @deprecated Please use {@link #getBackgroundExecutorProvider()}. */
@Deprecated
public ExecutorProvider getExecutorProvider() {
Expand Down Expand Up @@ -616,6 +639,14 @@ public ApiClock getClock() {
return clock;
}

/**
* Gets the ApiKey that was previously set on this Builder. If an empty string was provided it
* will return null
*/
public final String getApiKey() {
return (apiKey == null || apiKey.isEmpty()) ? null : apiKey;
}

/**
* @return the resolved endpoint when the Builder was created. If invoked after
* `StubSettings.newBuilder()` is called, it will return the clientSettingsEndpoint value.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

Expand All @@ -52,6 +53,7 @@
import com.google.api.gax.rpc.testing.FakeClientSettings;
import com.google.api.gax.rpc.testing.FakeStubSettings;
import com.google.api.gax.rpc.testing.FakeTransportChannel;
import com.google.auth.ApiKeyCredentials;
import com.google.auth.Credentials;
import com.google.auth.oauth2.ComputeEngineCredentials;
import com.google.auth.oauth2.GdchCredentials;
Expand Down Expand Up @@ -205,9 +207,6 @@ public TransportChannelProvider withPoolSize(int size) {

@Override
public TransportChannel getTransportChannel() throws IOException {
if (needsCredentials()) {
throw new IllegalStateException("Needs Credentials");
}
transport.setExecutor(executor);
return transport;
}
Expand Down Expand Up @@ -1074,4 +1073,50 @@ public void testStreamWatchdogInterval_backportMethodsBehaveCorrectly() {
ct -> ct.getStreamWatchdogCheckIntervalDuration(),
ct -> ct.getStreamWatchdogCheckInterval());
}

@Test
public void testSetApiKey_createsApiCredentials() throws IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: These test methods don't have to be public anymore after we migrated to junit 5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah thanks! done

String apiKey = "key";
FakeStubSettings.Builder builder = new FakeStubSettings.Builder();
InterceptingExecutor executor = new InterceptingExecutor(1);
FakeTransportChannel transportChannel = FakeTransportChannel.create(new FakeChannel());
FakeTransportProvider transportProvider =
new FakeTransportProvider(
transportChannel, executor, true, ImmutableMap.of(), null, DEFAULT_ENDPOINT);
builder.setTransportChannelProvider(transportProvider);
HeaderProvider headerProvider = Mockito.mock(HeaderProvider.class);
Mockito.when(headerProvider.getHeaders()).thenReturn(ImmutableMap.of());
builder.setHeaderProvider(headerProvider);
builder.setApiKey(apiKey);

ClientContext context = ClientContext.create(builder.build());

FakeCallContext fakeCallContext = (FakeCallContext) context.getDefaultCallContext();
assertThat(fakeCallContext.getCredentials()).isInstanceOf(ApiKeyCredentials.class);
}

@Test
void testCreateClient_throwsErrorIfApiKeyAndCredentialsAreProvided() throws Exception {
String apiKey = "key";
FakeStubSettings.Builder builder = new FakeStubSettings.Builder();
InterceptingExecutor executor = new InterceptingExecutor(1);
FakeTransportChannel transportChannel = FakeTransportChannel.create(new FakeChannel());
FakeTransportProvider transportProvider =
new FakeTransportProvider(
transportChannel, executor, true, ImmutableMap.of(), null, DEFAULT_ENDPOINT);
builder.setTransportChannelProvider(transportProvider);
HeaderProvider headerProvider = Mockito.mock(HeaderProvider.class);
Mockito.when(headerProvider.getHeaders()).thenReturn(ImmutableMap.of());
builder.setHeaderProvider(headerProvider);
builder.setApiKey(apiKey);
builder.setCredentialsProvider(Mockito.mock(CredentialsProvider.class));

try {
ClientContext.create(builder.build());
fail("No exception raised");
} catch (IllegalArgumentException e) {
assert (e.getMessage()
.contains("You can not provide both ApiKey and Credentials for a client."));
}
}
}
Loading
Loading