Navigation Menu

Skip to content

Commit

Permalink
fix: jwt authentication on batch-bigtable.googleapis.com (#892)
Browse files Browse the repository at this point in the history
* fix: jwt authentication on batch-bigtable.googleapis.com

In general jwt audiences and service endpoints align. However in some cases like batch-bigtable.googleapis.com, they diverge. This PR workaround the issue by patching the JWT audience for batch-bigtable.googleapis.com

* remove abandoned tst strategy

* deps

* fix settings

* fix batch tests
  • Loading branch information
igorbernstein2 committed Jul 1, 2021
1 parent 9290cd0 commit d2ca9c6
Show file tree
Hide file tree
Showing 8 changed files with 321 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .kokoro/nightly/integration.cfg
Expand Up @@ -8,7 +8,7 @@ env_vars: {

env_vars: {
key: "INTEGRATION_TEST_ARGS"
value: "-P bigtable-emulator-it,bigtable-prod-it -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
value: "-P bigtable-emulator-it,bigtable-prod-it,bigtable-prod-batch-it -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
}

env_vars: {
Expand Down
2 changes: 1 addition & 1 deletion .kokoro/presubmit/integration.cfg
Expand Up @@ -8,7 +8,7 @@ env_vars: {

env_vars: {
key: "INTEGRATION_TEST_ARGS"
value: "-P bigtable-emulator-it,bigtable-prod-it -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
value: "-P bigtable-emulator-it,bigtable-prod-it,bigtable-prod-batch-it -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
}

env_vars: {
Expand Down
61 changes: 61 additions & 0 deletions google-cloud-bigtable/pom.xml
Expand Up @@ -32,6 +32,10 @@
<bigtable.cfe-admin-endpoint/>
<bigtable.directpath-data-endpoint/>
<bigtable.directpath-admin-endpoint/>

<!-- This is used by bigtable-prod-batch-it profile to ensure that tests work on the batch endpoint.
Also, this property will be augmented by `internal-bigtable-prod-batch-it-prop-helper` profile -->
<bigtable.cfe-data-batch-endpoint>batch-bigtable.googleapis.com:443</bigtable.cfe-data-batch-endpoint>
</properties>

<dependencyManagement>
Expand Down Expand Up @@ -114,6 +118,14 @@
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>
<dependency>
<groupId>com.google.http-client</groupId>
<artifactId>google-http-client</artifactId>
</dependency>
<dependency>
<groupId>com.google.http-client</groupId>
<artifactId>google-http-client-gson</artifactId>
</dependency>
<dependency>
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java</artifactId>
Expand Down Expand Up @@ -323,6 +335,55 @@
</build>
</profile>

<!-- internal profile to provide a sensible default for bigtable.cfe-data-batch-endpoint based on
bigtable.cfe-data-endpoint -->
<profile>
<id>internal-bigtable-prod-batch-it-prop-helper</id>
<activation>
<property>
<name>bigtable.cfe-data-endpoint</name>
</property>
</activation>
<properties>
<bigtable.cfe-data-batch-endpoint>batch-${bigtable.cfe-data-endpoint}</bigtable.cfe-data-batch-endpoint>
</properties>
</profile>

<profile>
<id>bigtable-prod-batch-it</id>
<build>
<plugins>
<plugin>
<artifactId>maven-failsafe-plugin</artifactId>
<executions>
<execution>
<id>prod-batch-it</id>
<goals>
<goal>integration-test</goal>
<goal>verify</goal>
</goals>
<configuration>
<skip>false</skip>

<systemPropertyVariables>
<bigtable.env>cloud</bigtable.env>
<bigtable.data-endpoint>${bigtable.cfe-data-batch-endpoint}</bigtable.data-endpoint>
<bigtable.admin-endpoint>${bigtable.cfe-admin-endpoint}</bigtable.admin-endpoint>
<bigtable.grpc-log-dir>${project.build.directory}/test-grpc-logs/prod-batch-it</bigtable.grpc-log-dir>
</systemPropertyVariables>
<includes>
<include>com.google.cloud.bigtable.data.v2.it.*IT</include>
</includes>
<summaryFile>${project.build.directory}/failsafe-reports/failsafe-summary-prod-batch-it.xml</summaryFile>
<reportsDirectory>${project.build.directory}/failsafe-reports/prod-batch-it</reportsDirectory>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>

<profile>
<id>bigtable-directpath-it</id>
<build>
Expand Down
@@ -0,0 +1,79 @@
/*
* Copyright 2021 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.cloud.bigtable.data.v2.internal;

import com.google.api.core.InternalApi;
import com.google.auth.Credentials;
import com.google.auth.RequestMetadataCallback;
import com.google.auth.oauth2.ServiceAccountJwtAccessCredentials;
import java.io.IOException;
import java.net.URI;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Executor;

/**
* Internal helper to fix the mapping between JWT audiences and service endpoints.
*
* <p>In most cases JWT audiences correspond to service endpoints. However, in some cases they
* diverge. To workaround this, this class hardcodes the audience and forces the underlying
* implementation to use it.
*
* <p>Internal Only - public for technical reasons
*/
@InternalApi
public class JwtCredentialsWithAudience extends Credentials {
private final ServiceAccountJwtAccessCredentials delegate;

public JwtCredentialsWithAudience(ServiceAccountJwtAccessCredentials delegate, URI audience) {
this.delegate = delegate.toBuilder().setDefaultAudience(audience).build();
}

@Override
public String getAuthenticationType() {
return delegate.getAuthenticationType();
}

@Override
public Map<String, List<String>> getRequestMetadata() throws IOException {
return delegate.getRequestMetadata();
}

@Override
public void getRequestMetadata(URI ignored, Executor executor, RequestMetadataCallback callback) {
delegate.getRequestMetadata(null, executor, callback);
}

@Override
public Map<String, List<String>> getRequestMetadata(URI ignored) throws IOException {
return delegate.getRequestMetadata(null);
}

@Override
public boolean hasRequestMetadata() {
return delegate.hasRequestMetadata();
}

@Override
public boolean hasRequestMetadataOnly() {
return delegate.hasRequestMetadataOnly();
}

@Override
public void refresh() throws IOException {
delegate.refresh();
}
}
Expand Up @@ -21,6 +21,7 @@
import com.google.api.gax.batching.BatcherImpl;
import com.google.api.gax.batching.FlowController;
import com.google.api.gax.core.BackgroundResource;
import com.google.api.gax.core.CredentialsProvider;
import com.google.api.gax.core.FixedCredentialsProvider;
import com.google.api.gax.grpc.GaxGrpcProperties;
import com.google.api.gax.grpc.GrpcCallContext;
Expand All @@ -42,6 +43,7 @@
import com.google.api.gax.tracing.TracedServerStreamingCallable;
import com.google.api.gax.tracing.TracedUnaryCallable;
import com.google.auth.Credentials;
import com.google.auth.oauth2.ServiceAccountJwtAccessCredentials;
import com.google.bigtable.v2.BigtableGrpc;
import com.google.bigtable.v2.CheckAndMutateRowRequest;
import com.google.bigtable.v2.CheckAndMutateRowResponse;
Expand All @@ -56,6 +58,7 @@
import com.google.bigtable.v2.SampleRowKeysRequest;
import com.google.bigtable.v2.SampleRowKeysResponse;
import com.google.cloud.bigtable.Version;
import com.google.cloud.bigtable.data.v2.internal.JwtCredentialsWithAudience;
import com.google.cloud.bigtable.data.v2.internal.RequestContext;
import com.google.cloud.bigtable.data.v2.models.BulkMutation;
import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation;
Expand Down Expand Up @@ -94,6 +97,8 @@
import io.opencensus.tags.Tagger;
import io.opencensus.tags.Tags;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -146,6 +151,9 @@ public static EnhancedBigtableStubSettings finalizeSettings(
// TODO: this implementation is on the cusp of unwieldy, if we end up adding more features
// consider splitting it up by feature.

// workaround JWT audience issues
patchCredentials(builder);

// Inject channel priming
if (settings.isRefreshingChannel()) {
// Fix the credentials so that they can be shared
Expand Down Expand Up @@ -218,6 +226,41 @@ public static EnhancedBigtableStubSettings finalizeSettings(
return builder.build();
}

private static void patchCredentials(EnhancedBigtableStubSettings.Builder settings)
throws IOException {
int i = settings.getEndpoint().lastIndexOf(":");
String host = settings.getEndpoint().substring(0, i);
String audience = settings.getJwtAudienceMapping().get(host);

if (audience == null) {
return;
}
URI audienceUri = null;
try {
audienceUri = new URI(audience);
} catch (URISyntaxException e) {
throw new IllegalStateException("invalid JWT audience override", e);
}

CredentialsProvider credentialsProvider = settings.getCredentialsProvider();
if (credentialsProvider == null) {
return;
}

Credentials credentials = credentialsProvider.getCredentials();
if (credentials == null) {
return;
}

if (!(credentials instanceof ServiceAccountJwtAccessCredentials)) {
return;
}

ServiceAccountJwtAccessCredentials jwtCreds = (ServiceAccountJwtAccessCredentials) credentials;
JwtCredentialsWithAudience patchedCreds = new JwtCredentialsWithAudience(jwtCreds, audienceUri);
settings.setCredentialsProvider(FixedCredentialsProvider.create(patchedCreds));
}

public EnhancedBigtableStub(EnhancedBigtableStubSettings settings, ClientContext clientContext) {
this.settings = settings;
this.clientContext = clientContext;
Expand Down
Expand Up @@ -16,6 +16,7 @@
package com.google.cloud.bigtable.data.v2.stub;

import com.google.api.core.BetaApi;
import com.google.api.core.InternalApi;
import com.google.api.gax.batching.BatchingCallSettings;
import com.google.api.gax.batching.BatchingSettings;
import com.google.api.gax.batching.FlowControlSettings;
Expand Down Expand Up @@ -151,12 +152,20 @@ public class EnhancedBigtableStubSettings extends StubSettings<EnhancedBigtableS
.add("https://www.googleapis.com/auth/cloud-platform")
.build();

/**
* In most cases, jwt audience == service name. However in some cases, this is not the case. The
* following mapping is used to patch the audience in a JWT token.
*/
private static final Map<String, String> DEFAULT_JWT_AUDIENCE_MAPPING =
ImmutableMap.of("batch-bigtable.googleapis.com", "https://bigtable.googleapis.com/");

private final String projectId;
private final String instanceId;
private final String appProfileId;
private final boolean isRefreshingChannel;
private ImmutableList<String> primedTableIds;
private HeaderTracer headerTracer;
private final Map<String, String> jwtAudienceMapping;

private final ServerStreamingCallSettings<Query, Row> readRowsSettings;
private final UnaryCallSettings<Query, Row> readRowSettings;
Expand Down Expand Up @@ -191,6 +200,7 @@ private EnhancedBigtableStubSettings(Builder builder) {
isRefreshingChannel = builder.isRefreshingChannel;
primedTableIds = builder.primedTableIds;
headerTracer = builder.headerTracer;
jwtAudienceMapping = builder.jwtAudienceMapping;

// Per method settings.
readRowsSettings = builder.readRowsSettings.build();
Expand Down Expand Up @@ -240,6 +250,11 @@ HeaderTracer getHeaderTracer() {
return headerTracer;
}

@InternalApi("Used for internal testing")
public Map<String, String> getJwtAudienceMapping() {
return jwtAudienceMapping;
}

/** Returns a builder for the default ChannelProvider for this service. */
public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProviderBuilder() {
return BigtableStubSettings.defaultGrpcTransportProviderBuilder()
Expand Down Expand Up @@ -498,6 +513,7 @@ public static class Builder extends StubSettings.Builder<EnhancedBigtableStubSet
private boolean isRefreshingChannel;
private ImmutableList<String> primedTableIds;
private HeaderTracer headerTracer;
private Map<String, String> jwtAudienceMapping;

private final ServerStreamingCallSettings.Builder<Query, Row> readRowsSettings;
private final UnaryCallSettings.Builder<Query, Row> readRowSettings;
Expand All @@ -522,6 +538,7 @@ private Builder() {
this.isRefreshingChannel = false;
primedTableIds = ImmutableList.of();
headerTracer = HeaderTracer.newBuilder().build();
jwtAudienceMapping = DEFAULT_JWT_AUDIENCE_MAPPING;
setCredentialsProvider(defaultCredentialsProviderBuilder().build());

// Defaults provider
Expand Down Expand Up @@ -629,6 +646,7 @@ private Builder(EnhancedBigtableStubSettings settings) {
isRefreshingChannel = settings.isRefreshingChannel;
primedTableIds = settings.primedTableIds;
headerTracer = settings.headerTracer;
jwtAudienceMapping = settings.jwtAudienceMapping;

// Per method settings.
readRowsSettings = settings.readRowsSettings.toBuilder();
Expand Down Expand Up @@ -762,6 +780,17 @@ HeaderTracer getHeaderTracer() {
return headerTracer;
}

@InternalApi("Used for internal testing")
public Builder setJwtAudienceMapping(Map<String, String> jwtAudienceMapping) {
this.jwtAudienceMapping = Preconditions.checkNotNull(jwtAudienceMapping);
return this;
}

@InternalApi("Used for internal testing")
public Map<String, String> getJwtAudienceMapping() {
return jwtAudienceMapping;
}

/** Returns the builder for the settings used for calls to readRows. */
public ServerStreamingCallSettings.Builder<Query, Row> readRowsSettings() {
return readRowsSettings;
Expand Down Expand Up @@ -842,6 +871,7 @@ public String toString() {
.add("isRefreshingChannel", isRefreshingChannel)
.add("primedTableIds", primedTableIds)
.add("headerTracer", headerTracer)
.add("jwtAudienceMapping", jwtAudienceMapping)
.add("readRowsSettings", readRowsSettings)
.add("readRowSettings", readRowSettings)
.add("sampleRowKeysSettings", sampleRowKeysSettings)
Expand Down
Expand Up @@ -716,6 +716,7 @@ public void verifyDefaultHeaderTracerNotNullTest() {
"isRefreshingChannel",
"primedTableIds",
"headerTracer",
"jwtAudienceMapping",
"readRowsSettings",
"readRowSettings",
"sampleRowKeysSettings",
Expand Down

0 comments on commit d2ca9c6

Please sign in to comment.