Skip to content

Commit

Permalink
fix: JS API should use GetConfigurationConstants for auth calls (#5959)
Browse files Browse the repository at this point in the history
In addition to more closely following what other Deephaven clients do,
this prevents an issue where Open/Next pairs of gRPC calls (to
emulate a bidirectional call) can race each other to redeem a one-time
use auth token.

Fixes #5955
  • Loading branch information
niloc132 committed Aug 22, 2024
1 parent f61159a commit 7b13def
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 200 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,19 +131,6 @@ public Promise<Void> login(@TsTypeRef(LoginCredentials.class) JsPropertyMap<Obje
EventPair.of(CoreClient.EVENT_RECONNECT_AUTH_FAILED, loginPromise::fail));
Promise<Void> login = loginPromise.asPromise();

// fetch configs and check session timeout
login.then(ignore -> getServerConfigValues()).then(configs -> {
for (String[] config : configs) {
if (config[0].equals("http.session.durationMs")) {
workerConnection.setSessionTimeoutMs(Double.parseDouble(config[1]));
}
}
return null;
}).catch_(ignore -> {
// Ignore this failure and suppress browser logging, we have a safe fallback
return Promise.resolve((Object) null);
});

if (alreadyRunning) {
ideConnection.connection.get().forceReconnect();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,12 @@
import io.deephaven.javascript.proto.dhinternal.arrow.flight.flatbuf.schema_generated.org.apache.arrow.flatbuf.Schema;
import io.deephaven.javascript.proto.dhinternal.arrow.flight.protocol.browserflight_pb_service.BrowserFlightServiceClient;
import io.deephaven.javascript.proto.dhinternal.arrow.flight.protocol.flight_pb.FlightData;
import io.deephaven.javascript.proto.dhinternal.arrow.flight.protocol.flight_pb.HandshakeRequest;
import io.deephaven.javascript.proto.dhinternal.arrow.flight.protocol.flight_pb.HandshakeResponse;
import io.deephaven.javascript.proto.dhinternal.arrow.flight.protocol.flight_pb_service.FlightServiceClient;
import io.deephaven.javascript.proto.dhinternal.browserheaders.BrowserHeaders;
import io.deephaven.javascript.proto.dhinternal.flatbuffers.Builder;
import io.deephaven.javascript.proto.dhinternal.flatbuffers.Long;
import io.deephaven.javascript.proto.dhinternal.grpcweb.grpc.Code;
import io.deephaven.javascript.proto.dhinternal.grpcweb.grpc.UnaryOutput;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.barrage.flatbuf.barrage_generated.io.deephaven.barrage.flatbuf.BarrageMessageType;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.barrage.flatbuf.barrage_generated.io.deephaven.barrage.flatbuf.BarrageMessageWrapper;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.barrage.flatbuf.barrage_generated.io.deephaven.barrage.flatbuf.BarrageSubscriptionOptions;
Expand All @@ -40,6 +39,9 @@
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.application_pb.FieldsChangeUpdate;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.application_pb.ListFieldsRequest;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.application_pb_service.ApplicationServiceClient;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.config_pb.ConfigurationConstantsRequest;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.config_pb.ConfigurationConstantsResponse;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.config_pb_service.ConfigService;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.config_pb_service.ConfigServiceClient;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.console_pb.LogSubscriptionData;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.console_pb.LogSubscriptionRequest;
Expand Down Expand Up @@ -72,13 +74,13 @@
import io.deephaven.web.client.api.barrage.def.ColumnDefinition;
import io.deephaven.web.client.api.barrage.def.InitialTableDefinition;
import io.deephaven.web.client.api.barrage.stream.BiDiStream;
import io.deephaven.web.client.api.barrage.stream.HandshakeStreamFactory;
import io.deephaven.web.client.api.barrage.stream.ResponseStreamWrapper;
import io.deephaven.web.client.api.batch.RequestBatcher;
import io.deephaven.web.client.api.batch.TableConfig;
import io.deephaven.web.client.api.console.JsVariableChanges;
import io.deephaven.web.client.api.console.JsVariableDefinition;
import io.deephaven.web.client.api.console.JsVariableType;
import io.deephaven.web.client.api.grpc.UnaryWithHeaders;
import io.deephaven.web.client.api.i18n.JsTimeZone;
import io.deephaven.web.client.api.impl.TicketAndPromise;
import io.deephaven.web.client.api.lifecycle.HasLifecycle;
Expand Down Expand Up @@ -479,54 +481,58 @@ private Promise<Void> authUpdate() {
DomGlobal.clearTimeout(scheduledAuthUpdate);
scheduledAuthUpdate = null;
}
return new Promise<>((resolve, reject) -> {
// the streamfactory will automatically reference our existing metadata, but we can listen to update it
BiDiStream<HandshakeRequest, HandshakeResponse> handshake = HandshakeStreamFactory.create(this);
handshake.onHeaders(headers -> {
// unchecked cast is required here due to "aliasing" in ts/webpack resulting in BrowserHeaders !=
// Metadata
JsArray<String> authorization = Js.<BrowserHeaders>uncheckedCast(headers).get(FLIGHT_AUTH_HEADER_NAME);
if (authorization.length > 0) {
JsArray<String> existing = metadata().get(FLIGHT_AUTH_HEADER_NAME);
if (!existing.getAt(0).equals(authorization.getAt(0))) {
// use this new token
metadata().set(FLIGHT_AUTH_HEADER_NAME, authorization);
CustomEventInit<JsRefreshToken> init = CustomEventInit.create();
init.setDetail(new JsRefreshToken(authorization.getAt(0), sessionTimeoutMs));
info.fireEvent(EVENT_REFRESH_TOKEN_UPDATED, init);
return UnaryWithHeaders.<ConfigurationConstantsRequest, ConfigurationConstantsResponse>call(
this, ConfigService.GetConfigurationConstants, new ConfigurationConstantsRequest())
.then(result -> {
BrowserHeaders headers = result.getHeaders();
// unchecked cast is required here due to "aliasing" in ts/webpack resulting in BrowserHeaders !=
// Metadata
JsArray<String> authorization =
Js.<BrowserHeaders>uncheckedCast(headers).get(FLIGHT_AUTH_HEADER_NAME);
if (authorization.length > 0) {
JsArray<String> existing = metadata().get(FLIGHT_AUTH_HEADER_NAME);
if (!existing.getAt(0).equals(authorization.getAt(0))) {
// use this new token
metadata().set(FLIGHT_AUTH_HEADER_NAME, authorization);
CustomEventInit<JsRefreshToken> init = CustomEventInit.create();
init.setDetail(new JsRefreshToken(authorization.getAt(0), sessionTimeoutMs));
info.fireEvent(EVENT_REFRESH_TOKEN_UPDATED, init);
}
}
}
});
handshake.onStatus(status -> {
if (status.isOk()) {

// Read the timeout from the server, we'll refresh at less than that
result.getMessage().getConfigValuesMap().forEach((item, key) -> {
if (key.equals("http.session.durationMs")) {
sessionTimeoutMs = Double.parseDouble(item.getStringValue());
}
});

// schedule an update based on our currently configured delay
scheduledAuthUpdate = DomGlobal.setTimeout(ignore -> {
authUpdate();
}, sessionTimeoutMs / 2);

resolve.onInvoke((Void) null);
} else {
if (status.getCode() == Code.Unauthenticated) {
return Promise.resolve((Void) null);
}).catch_(err -> {
UnaryOutput<?> result = (UnaryOutput<?>) err;
if (result.getStatus() == Code.Unauthenticated) {
// explicitly clear out any metadata for authentication, and signal that auth failed
metadata.delete(FLIGHT_AUTH_HEADER_NAME);

// Fire an event for the UI to attempt to re-auth
info.fireEvent(CoreClient.EVENT_RECONNECT_AUTH_FAILED);
return;

// We return here rather than continue and call checkStatus()
return Promise.reject("Authentication failed, please reconnect");
}
// TODO deephaven-core#2564 fire an event for the UI to re-auth
checkStatus(status);
if (status.getDetails() == null || status.getDetails().isEmpty()) {
reject.onInvoke("Error occurred while authenticating, gRPC status " + status.getCode());
checkStatus(ResponseStreamWrapper.Status.of(result.getStatus(), result.getMessage().toString(),
result.getTrailers()));
if (result.getMessage() == null || result.getMessage().toString().isEmpty()) {
return Promise.reject(result.getMessage());
} else {
reject.onInvoke(status.getDetails());
return Promise.reject("Error occurred while authenticating, gRPC status " + result.getStatus());
}
}
});

handshake.send(new HandshakeRequest());
handshake.end();
});
});
}

private void subscribeToTerminationNotification() {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//
// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending
//
package io.deephaven.web.client.api.grpc;

import elemental2.promise.IThenable;
import elemental2.promise.Promise;
import io.deephaven.javascript.proto.dhinternal.grpcweb.Grpc;
import io.deephaven.javascript.proto.dhinternal.grpcweb.grpc.Code;
import io.deephaven.javascript.proto.dhinternal.grpcweb.unary.UnaryOutput;
import io.deephaven.javascript.proto.dhinternal.grpcweb.unary.UnaryRpcOptions;
import io.deephaven.web.client.api.WorkerConnection;

public class UnaryWithHeaders {

/**
* Improbable-eng's grpc-web implementation doesn't pass headers to api callers - this changes the contract a bit so
* that we can get a typed UnaryOutput with the headers/trailers intact.
*
* @param connection provides access to the metadata and the server url
* @param methodDescriptor the service method to invoke
* @return a promise that will resolve to the response plus headers/trailers, or reject with the headers/trailers
* @param <Res> type of the message object
*/
public static <Req, Res> Promise<UnaryOutput<Res>> call(WorkerConnection connection, Object methodDescriptor,
Req request) {
return new Promise<>((resolve, reject) -> {
UnaryRpcOptions<Object, Object> props = UnaryRpcOptions.create();
props.setHost(connection.configServiceClient().serviceHost);
props.setMetadata(connection.metadata());
props.setTransport(null);// ts doesn't expose these two, stick with defaults for now
props.setDebug(false);
props.setOnEnd(response -> {
if (response.getStatus() != Code.OK) {
reject.onInvoke(response);
} else {
resolve.onInvoke((IThenable<UnaryOutput<Res>>) response);
}
});
props.setRequest(request);
Grpc.unary.onInvoke(methodDescriptor, props);
});
}
}

0 comments on commit 7b13def

Please sign in to comment.