Skip to content

Commit

Permalink
fix(sdk): if we encounter an error getting an access token then don't…
Browse files Browse the repository at this point in the history
… make the request (#872)

This has been confusing for other teams that have experienced errors
with authentication
  • Loading branch information
mkleene authored May 24, 2024
1 parent 1f9ee74 commit 19188d5
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
11 changes: 8 additions & 3 deletions sdk/auth/token_adding_interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import (
"github.com/lestrrat-go/jwx/v2/jws"
"github.com/lestrrat-go/jwx/v2/jwt"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/metadata"
"google.golang.org/grpc/status"
)

const (
Expand Down Expand Up @@ -52,15 +54,18 @@ func (i TokenAddingInterceptor) AddCredentials(
if err == nil {
newMetadata = append(newMetadata, "Authorization", fmt.Sprintf("DPoP %s", accessToken))
} else {
slog.ErrorContext(ctx, "error getting access token. request will be unauthenticated", "error", err)
return invoker(ctx, method, req, reply, cc, opts...)
slog.ErrorContext(ctx, "error getting access token", "error", err)
return status.Error(codes.Unauthenticated, err.Error())
}

dpopTok, err := i.GetDPoPToken(method, http.MethodPost, string(accessToken))
if err == nil {
newMetadata = append(newMetadata, "DPoP", dpopTok)
} else {
slog.ErrorContext(ctx, "error adding dpop token to outgoing request. Request will not have DPoP token", "error", err)
// since we don't have a setting about whether DPoP is in use on the client and this request _could_ succeed if
// they are talking to a server where DPoP is not required we will just let this through. this method is extremely
// unlikely to fail so hopefully this isn't confusing
slog.ErrorContext(ctx, "error getting DPoP token for outgoing request. Request will not have DPoP token", "error", err)
}

newCtx := metadata.AppendToOutgoingContext(ctx, newMetadata...)
Expand Down
11 changes: 7 additions & 4 deletions sdk/auth/token_adding_interceptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ func TestAddingTokensToOutgoingRequest(t *testing.T) {
}
}

func Test_InvalidCredentials_StillSendMessage(t *testing.T) {
ts := FakeTokenSource{key: nil}
func Test_InvalidCredentials_DoesNotSendMessage(t *testing.T) {
ts := FakeTokenSource{key: nil, accessToken: ""}
server := FakeAccessServiceServer{}
oo := NewTokenAddingInterceptor(&ts, &tls.Config{
MinVersion: tls.VersionTLS12,
Expand All @@ -129,8 +129,8 @@ func Test_InvalidCredentials_StillSendMessage(t *testing.T) {

_, err := client.Info(context.Background(), &kas.InfoRequest{})

if err != nil {
t.Fatalf("got an error when sending the message")
if err == nil {
t.Fatalf("should not have sent message because the token source returned an error")
}
}

Expand Down Expand Up @@ -169,6 +169,9 @@ type FakeTokenSource struct {
}

func (fts *FakeTokenSource) AccessToken(context.Context, *http.Client) (AccessToken, error) {
if fts.accessToken == "" {
return "", errors.New("no token to provide")
}
return AccessToken(fts.accessToken), nil
}
func (fts *FakeTokenSource) MakeToken(f func(jwk.Key) ([]byte, error)) ([]byte, error) {
Expand Down

0 comments on commit 19188d5

Please sign in to comment.