Skip to content

Commit 36bac3e

Browse files
committed
fix(bigtable): fall back to classic path when per-RPC CallCredentials are set on session path
The session path (DivertingUnaryCallable) was silently dropping per-RPC CallCredentials set via GrpcCallContext.withCredentials(), because sessions are established with channel-level credentials and cannot forward per-RPC overrides. Any per-RPC credential supplied by the caller was ignored, causing those calls to use the channel-level credentials instead — a security bypass. Fix: detect per-RPC credentials via `context instanceof GrpcCallContext && context.getCallOptions().getCredentials() != null` and fall back to the classic path, which correctly applies the caller-supplied credentials. The `instanceof` guard is required because calls made without an explicit ApiCallContext pass a null context (no override → no bypass). Observability: wire DebugTagTracer through ShimImpl so DivertingUnaryCallable can record a `per_rpc_credentials_session_fallback` tag into the internal Cloud Monitoring metric `bigtable.googleapis.com/internal/client/debug_tags` whenever the fallback fires, giving the server side visibility.
1 parent 026e6c0 commit 36bac3e

2 files changed

Lines changed: 30 additions & 6 deletions

File tree

‎java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/compat/ShimImpl.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import com.google.cloud.bigtable.data.v2.internal.compat.ops.ReadRowShimInner;
4141
import com.google.cloud.bigtable.data.v2.internal.csm.Metrics;
4242
import com.google.cloud.bigtable.data.v2.internal.csm.attributes.ClientInfo;
43+
import com.google.cloud.bigtable.data.v2.internal.csm.tracers.DebugTagTracer;
4344
import com.google.cloud.bigtable.data.v2.internal.csm.tracers.DirectPathCompatibleTracer;
4445
import com.google.cloud.bigtable.data.v2.internal.dp.DirectAccessInvestigator;
4546
import com.google.cloud.bigtable.data.v2.internal.util.ClientConfigurationManager;
@@ -81,6 +82,7 @@ public class ShimImpl implements Shim {
8182
private final ClientConfigurationManager configManager;
8283
private final Resource<ClientConfigurationManager> configManagerResource;
8384
private final Client client;
85+
private final DebugTagTracer debugTagTracer;
8486

8587
private final ReadRowShimInner readRowShimInner;
8688
private final MutateRowShim mutateRowShim;
@@ -182,13 +184,20 @@ public static Shim create(
182184
userCallbackExecutor,
183185
clientChannelProvider);
184186

185-
return new ShimImpl(Resource.createOwned(configManager, configManager::close), client);
187+
return new ShimImpl(
188+
Resource.createOwned(configManager, configManager::close),
189+
client,
190+
metrics.getDebugTagTracer());
186191
}
187192

188-
public ShimImpl(Resource<ClientConfigurationManager> configManagerResource, Client client) {
193+
public ShimImpl(
194+
Resource<ClientConfigurationManager> configManagerResource,
195+
Client client,
196+
DebugTagTracer debugTagTracer) {
189197
this.configManagerResource = configManagerResource;
190198
this.configManager = configManagerResource.get();
191199
this.client = client;
200+
this.debugTagTracer = debugTagTracer;
192201

193202
this.readRowShimInner = new ReadRowShimInner(client);
194203
this.mutateRowShim = new MutateRowShim(client);
@@ -221,7 +230,8 @@ public static Shim createForFactoryChild(
221230
Resource.createShared(userCallbackExecutor),
222231
Resource.createShared(sharedChannelPool));
223232

224-
return new ShimImpl(Resource.createShared(sharedConfigManager), client);
233+
return new ShimImpl(
234+
Resource.createShared(sharedConfigManager), client, metrics.getDebugTagTracer());
225235
}
226236

227237
/** Returns the raw config manager (e.g. for sharing with factory children). */
@@ -355,13 +365,14 @@ public <RowT> UnaryCallable<Query, RowT> decorateReadRow(
355365
configManager,
356366
classic,
357367
new ReadRowShim<>(readRowShimInner, rowAdapter),
358-
Util.extractTimeout(settings));
368+
Util.extractTimeout(settings),
369+
debugTagTracer);
359370
}
360371

361372
@Override
362373
public UnaryCallable<RowMutation, Void> decorateMutateRow(
363374
UnaryCallable<RowMutation, Void> classic, UnaryCallSettings<?, ?> settings) {
364375
return new DivertingUnaryCallable<>(
365-
configManager, classic, mutateRowShim, Util.extractTimeout(settings));
376+
configManager, classic, mutateRowShim, Util.extractTimeout(settings), debugTagTracer);
366377
}
367378
}

‎java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/compat/ops/DivertingUnaryCallable.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
import com.google.api.gax.rpc.ApiException;
2323
import com.google.api.gax.rpc.ApiExceptionFactory;
2424
import com.google.api.gax.rpc.UnaryCallable;
25+
import com.google.bigtable.v2.TelemetryConfiguration;
2526
import com.google.cloud.bigtable.data.v2.internal.compat.FutureAdapter;
2627
import com.google.cloud.bigtable.data.v2.internal.compat.Util;
28+
import com.google.cloud.bigtable.data.v2.internal.csm.tracers.DebugTagTracer;
2729
import com.google.cloud.bigtable.data.v2.internal.util.ClientConfigurationManager;
2830
import com.google.common.base.Throwables;
2931
import io.grpc.Context;
@@ -46,23 +48,34 @@ public class DivertingUnaryCallable<ReqT, RespT> extends UnaryCallable<ReqT, Res
4648
private final UnaryShim<ReqT, RespT> experimental;
4749

4850
private final Duration defaultTimeout;
51+
private final DebugTagTracer debugTagTracer;
4952

5053
public DivertingUnaryCallable(
5154
ClientConfigurationManager configurationManager,
5255
UnaryCallable<ReqT, RespT> classic,
5356
UnaryShim<ReqT, RespT> experimental,
54-
Duration defaultTimeout) {
57+
Duration defaultTimeout,
58+
DebugTagTracer debugTagTracer) {
5559
this.configurationManager = configurationManager;
5660
this.classic = classic;
5761
this.experimental = experimental;
5862
this.defaultTimeout = defaultTimeout;
63+
this.debugTagTracer = debugTagTracer;
5964
}
6065

6166
@Override
6267
public ApiFuture<RespT> futureCall(ReqT request, ApiCallContext context) {
6368
if (!useExperimental(request)) {
6469
return classic.futureCall(request, context);
6570
}
71+
// Per-RPC credential overrides cannot be forwarded to the session path (sessions are
72+
// established with channel-level credentials). Fall back to classic to honor the override.
73+
if (context instanceof GrpcCallContext
74+
&& ((GrpcCallContext) context).getCallOptions().getCredentials() != null) {
75+
debugTagTracer.record(
76+
TelemetryConfiguration.Level.WARN, "per_rpc_credentials_session_fallback");
77+
return classic.futureCall(request, context);
78+
}
6679

6780
Deadline deadline = Util.extractDeadline((GrpcCallContext) context, defaultTimeout);
6881

0 commit comments

Comments
 (0)