Skip to content

Commit

Permalink
fix: Go and JS clients should not require v1+v2 auth (#5946)
Browse files Browse the repository at this point in the history
The Go client required sending v2 auth, but receiving v1 responses -
this change makes it consistently use v1 auth.

The JS client incorrectly closed the stream only after it had received
headers - now it can close right away, and just wait for headers to
arrive normally.

Prerequisite to #5922
  • Loading branch information
niloc132 authored Aug 16, 2024
1 parent 663fae6 commit 903abba
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 9 deletions.
27 changes: 19 additions & 8 deletions go/pkg/client/tokens.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import (
"fmt"
"github.com/apache/arrow/go/v8/arrow/flight"
configpb2 "github.com/deephaven/deephaven-core/go/internal/proto/config"
sessionpb2 "github.com/deephaven/deephaven-core/go/internal/proto/session"
"google.golang.org/grpc/metadata"
"google.golang.org/protobuf/proto"
"log"
"strconv"
"sync"
Expand Down Expand Up @@ -36,8 +38,19 @@ func withAuthToken(ctx context.Context, token []byte) context.Context {
}

// requestToken requests a new token from flight.
func requestToken(handshakeClient flight.FlightService_HandshakeClient, handshakeReq *flight.HandshakeRequest) ([]byte, error) {
err := handshakeClient.Send(handshakeReq)
func requestToken(handshakeClient flight.FlightService_HandshakeClient, authType string, authToken []byte) ([]byte, error) {

war := sessionpb2.WrappedAuthenticationRequest{
Type: authType,
Payload: authToken,
}
payload, err := proto.Marshal(&war)
if err != nil {
return nil, err
}
handshakeReq := flight.HandshakeRequest{Payload: []byte(payload)}

err = handshakeClient.Send(&handshakeReq)

if err != nil {
return nil, err
Expand Down Expand Up @@ -122,15 +135,13 @@ func (tr *tokenManager) Close() error {
// "user:password"; when auth_type is DefaultAuth, it will be ignored; when auth_type is a custom-built
// authenticator, it must conform to the specific requirement of the authenticator.
func newTokenManager(ctx context.Context, fs *flightStub, cfg configpb2.ConfigServiceClient, authType string, authToken string) (*tokenManager, error) {
authString := makeAuthString(authType, authToken)

handshakeClient, err := fs.handshake(withAuth(ctx, authString))
handshakeClient, err := fs.handshake(ctx)

if err != nil {
return nil, err
}

tkn, err := requestToken(handshakeClient, &flight.HandshakeRequest{Payload: []byte(authString)})
tkn, err := requestToken(handshakeClient, authType, []byte(authToken))

if err != nil {
return nil, err
Expand Down Expand Up @@ -174,10 +185,10 @@ func newTokenManager(ctx context.Context, fs *flightStub, cfg configpb2.ConfigSe
var tkn []byte

if err == nil {
tkn, err = requestToken(handshakeClient, &flight.HandshakeRequest{Payload: oldToken})
tkn, err = requestToken(handshakeClient, "Bearer", oldToken)
} else {
log.Println("Old token has an error during token update. Attempting to acquire a fresh token. err=", err)
tkn, err = requestToken(handshakeClient, &flight.HandshakeRequest{Payload: []byte(authString)})
tkn, err = requestToken(handshakeClient, authType, []byte(authToken))
}

if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,6 @@ private Promise<Void> authUpdate() {
info.fireEvent(EVENT_REFRESH_TOKEN_UPDATED, init);
}
}
handshake.end();
});
handshake.onStatus(status -> {
if (status.isOk()) {
Expand Down Expand Up @@ -526,6 +525,7 @@ private Promise<Void> authUpdate() {
});

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

Expand Down

0 comments on commit 903abba

Please sign in to comment.