From 355e805124e49aeaa0fd9420c63e32189d0cf117 Mon Sep 17 00:00:00 2001 From: strantalis Date: Thu, 30 Jul 2026 14:53:07 -0400 Subject: [PATCH 1/4] fix(sdk): use bearer auth for non-dpop connect tokens Signed-off-by: strantalis --- sdk/auth/access_token_source.go | 9 +++++ sdk/auth/token_adding_interceptor.go | 5 +++ sdk/auth/token_adding_interceptor_test.go | 44 ++++++++++++++++++++--- sdk/idp_access_token_source.go | 9 +++++ sdk/idp_cert_exchange.go | 9 +++++ sdk/idp_oauth_access_token_source.go | 18 +++++++++- sdk/idp_oauth_access_token_source_test.go | 16 ++++++++- 7 files changed, 103 insertions(+), 7 deletions(-) diff --git a/sdk/auth/access_token_source.go b/sdk/auth/access_token_source.go index 516cc10b5b..5602aff794 100644 --- a/sdk/auth/access_token_source.go +++ b/sdk/auth/access_token_source.go @@ -15,3 +15,12 @@ type AccessTokenSource interface { // more closely linked to what happens in KAS in terms of crypto params MakeToken(func(jwk.Key) ([]byte, error)) ([]byte, error) } + +// DPoPSupportingAccessTokenSource is implemented by token sources that know +// whether the access token they most recently returned is DPoP-bound. +// +// Token sources without this optional interface retain the SDK's existing DPoP +// behavior for backwards compatibility. +type DPoPSupportingAccessTokenSource interface { + DPoPSupported() bool +} diff --git a/sdk/auth/token_adding_interceptor.go b/sdk/auth/token_adding_interceptor.go index 34d9166a03..265d9b3399 100644 --- a/sdk/auth/token_adding_interceptor.go +++ b/sdk/auth/token_adding_interceptor.go @@ -100,6 +100,11 @@ func (i TokenAddingInterceptor) AddCredentialsConnect() connect.UnaryInterceptor return nil, connect.NewError(connect.CodeUnauthenticated, err) } + if dpopTokenSource, ok := i.tokenSource.(DPoPSupportingAccessTokenSource); ok && !dpopTokenSource.DPoPSupported() { + req.Header().Set("Authorization", fmt.Sprintf("Bearer %s", accessToken)) + return next(ctx, req) + } + // Add Authorization header req.Header().Set("Authorization", fmt.Sprintf("DPoP %s", accessToken)) diff --git a/sdk/auth/token_adding_interceptor_test.go b/sdk/auth/token_adding_interceptor_test.go index 9ce5a150f0..80f9acd747 100644 --- a/sdk/auth/token_adding_interceptor_test.go +++ b/sdk/auth/token_adding_interceptor_test.go @@ -41,9 +41,12 @@ func setupTokenAddingInterceptor(t *testing.T) (TokenAddingInterceptor, jwk.Key) err = key.Set(jwk.AlgorithmKey, jwa.RS256) require.NoError(t, err, "error setting the algorithm on the JWK") - ts := FakeTokenSource{ - key: key, - accessToken: "thisisafakeaccesstoken", + ts := DPoPFakeTokenSource{ + FakeTokenSource: FakeTokenSource{ + key: key, + accessToken: "thisisafakeaccesstoken", + }, + dpopSupported: true, } oo := NewTokenAddingInterceptorWithClient(&ts, httputil.SafeHTTPClientWithTLSConfig(&tls.Config{ @@ -119,6 +122,26 @@ func TestAddingTokensToOutgoingRequest_Connect(t *testing.T) { checkAccessAndDpopTokens(t, serverConnect.accessToken, serverConnect.dpopToken, key) } +func TestAddingTokensToOutgoingRequest_Connect_BearerToken(t *testing.T) { + ts := DPoPFakeTokenSource{ + FakeTokenSource: FakeTokenSource{ + accessToken: "thisisafakeaccesstoken", + }, + dpopSupported: false, + } + interceptor := NewTokenAddingInterceptorWithClient(&ts, httputil.SafeHTTPClient()) + serverConnect := FakeAccessServiceServerConnect{} + clientConnect, stopC := runConnectServer(&serverConnect, interceptor) + defer stopC() + + _, err := clientConnect.PublicKey(t.Context(), connect.NewRequest(&kas.PublicKeyRequest{})) + require.NoError(t, err, "error making call") + + assert.Equal(t, []string{"Bearer thisisafakeaccesstoken"}, serverConnect.accessToken) + assert.Equal(t, []string{""}, serverConnect.dpopToken) + assert.False(t, ts.makeTokenCalled) +} + func Test_InvalidCredentials_DoesNotSendMessage(t *testing.T) { ts := FakeTokenSource{key: nil, accessToken: ""} serverGrpc := FakeAccessServiceServer{} @@ -194,8 +217,18 @@ func (f *FakeAccessServiceServer) Rewrap(context.Context, *kas.RewrapRequest) (* } type FakeTokenSource struct { - key jwk.Key - accessToken string + key jwk.Key + accessToken string + makeTokenCalled bool +} + +type DPoPFakeTokenSource struct { + FakeTokenSource + dpopSupported bool +} + +func (fts *DPoPFakeTokenSource) DPoPSupported() bool { + return fts.dpopSupported } func (fts *FakeTokenSource) AccessToken(context.Context, *http.Client) (AccessToken, error) { @@ -206,6 +239,7 @@ func (fts *FakeTokenSource) AccessToken(context.Context, *http.Client) (AccessTo } func (fts *FakeTokenSource) MakeToken(f func(jwk.Key) ([]byte, error)) ([]byte, error) { + fts.makeTokenCalled = true if fts.key == nil { return nil, errors.New("no such key") } diff --git a/sdk/idp_access_token_source.go b/sdk/idp_access_token_source.go index 7290be61dc..7a0b5ca8bb 100644 --- a/sdk/idp_access_token_source.go +++ b/sdk/idp_access_token_source.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/url" + "strings" "sync" "github.com/lestrrat-go/jwx/v2/jwa" @@ -112,3 +113,11 @@ func (t *IDPAccessTokenSource) AccessToken(_ context.Context, client *http.Clien func (t *IDPAccessTokenSource) MakeToken(tokenMaker func(jwk.Key) ([]byte, error)) ([]byte, error) { return tokenMaker(t.dpopKey) } + +// DPoPSupported reports whether the most recently retrieved access token is DPoP-bound. +func (t *IDPAccessTokenSource) DPoPSupported() bool { + t.tokenMutex.Lock() + defer t.tokenMutex.Unlock() + + return t.token != nil && strings.EqualFold(t.token.TokenType, "DPoP") +} diff --git a/sdk/idp_cert_exchange.go b/sdk/idp_cert_exchange.go index 5e0d2f4ee9..b3e1ca4888 100644 --- a/sdk/idp_cert_exchange.go +++ b/sdk/idp_cert_exchange.go @@ -4,6 +4,7 @@ import ( "context" "log/slog" "net/http" + "strings" "sync" "github.com/lestrrat-go/jwx/v2/jwk" @@ -60,3 +61,11 @@ func (c *CertExchangeTokenSource) AccessToken(ctx context.Context, _ *http.Clien func (c *CertExchangeTokenSource) MakeToken(tokenMaker func(jwk.Key) ([]byte, error)) ([]byte, error) { return tokenMaker(c.key) } + +// DPoPSupported reports whether the most recently retrieved access token is DPoP-bound. +func (c *CertExchangeTokenSource) DPoPSupported() bool { + c.tokenMutex.Lock() + defer c.tokenMutex.Unlock() + + return c.token != nil && strings.EqualFold(c.token.TokenType, "DPoP") +} diff --git a/sdk/idp_oauth_access_token_source.go b/sdk/idp_oauth_access_token_source.go index c9e39859d6..67fe7d9c05 100644 --- a/sdk/idp_oauth_access_token_source.go +++ b/sdk/idp_oauth_access_token_source.go @@ -4,6 +4,8 @@ import ( "context" "fmt" "net/http" + "strings" + "sync" "github.com/lestrrat-go/jwx/v2/jwk" "github.com/opentdf/platform/lib/ocrypto" @@ -11,13 +13,15 @@ import ( "golang.org/x/oauth2" ) -// OAuthAccessTokenSource allow connecting to an IDP and obtain a DPoP bound access token +// OAuthAccessTokenSource allows connecting to an IDP and obtaining an access token. type OAuthAccessTokenSource struct { source oauth2.TokenSource scopes []string dpopKey jwk.Key asymDecryption ocrypto.AsymDecryption dpopPEM string + dpopSupported bool + mutex sync.RWMutex } func NewOAuthAccessTokenSource( @@ -52,9 +56,21 @@ func (t *OAuthAccessTokenSource) AccessToken(_ context.Context, _ *http.Client) // TODO: refresh tokens if expired? } + t.mutex.Lock() + t.dpopSupported = strings.EqualFold(tok.Type(), "DPoP") + t.mutex.Unlock() + return auth.AccessToken(tok.AccessToken), nil } func (t *OAuthAccessTokenSource) MakeToken(tokenMaker func(jwk.Key) ([]byte, error)) ([]byte, error) { return tokenMaker(t.dpopKey) } + +// DPoPSupported reports whether the most recently retrieved access token is DPoP-bound. +func (t *OAuthAccessTokenSource) DPoPSupported() bool { + t.mutex.RLock() + defer t.mutex.RUnlock() + + return t.dpopSupported +} diff --git a/sdk/idp_oauth_access_token_source_test.go b/sdk/idp_oauth_access_token_source_test.go index 4936bf1124..8e611aa415 100644 --- a/sdk/idp_oauth_access_token_source_test.go +++ b/sdk/idp_oauth_access_token_source_test.go @@ -15,7 +15,7 @@ import ( func TestNewOAuthAccessTokenSource_Success(t *testing.T) { mockToken := "mockToken" // Expected - mockSource := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: mockToken}) + mockSource := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: mockToken, TokenType: "DPoP"}) mockScopes := []string{"scope1", "scope2"} mockKey, _ := ocrypto.NewRSAKeyPair(dpopKeySize) dpopPublicKeyPEM, dpopKey, asymDecryption, _ := getNewDPoPKey(&mockKey) @@ -36,11 +36,25 @@ func TestNewOAuthAccessTokenSource_Success(t *testing.T) { tok, err := tokenSource.AccessToken(t.Context(), nil) require.NoError(t, err) assert.Equal(t, tok, auth.AccessToken(mockToken)) + assert.True(t, tokenSource.DPoPSupported()) made, err := tokenSource.MakeToken(func(jwk.Key) ([]byte, error) { return []byte(mockToken), nil }) require.NoError(t, err) assert.Equal(t, made, []byte(mockToken)) } +func TestOAuthAccessTokenSource_DPoPSupported_BearerToken(t *testing.T) { + mockSource := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "mockToken", TokenType: "Bearer"}) + mockKey, err := ocrypto.NewRSAKeyPair(dpopKeySize) + require.NoError(t, err) + + tokenSource, err := NewOAuthAccessTokenSource(mockSource, nil, &mockKey) + require.NoError(t, err) + + _, err = tokenSource.AccessToken(t.Context(), nil) + require.NoError(t, err) + assert.False(t, tokenSource.DPoPSupported()) +} + func TestNewOAuthAccessTokenSource_ExpiredToken(t *testing.T) { // Expected pastTime := time.Now().Add(-time.Hour) From cfa8f0dcd9d5be8115b2d64627a9094831f415df Mon Sep 17 00:00:00 2001 From: strantalis Date: Thu, 30 Jul 2026 15:45:17 -0400 Subject: [PATCH 2/4] fix(sdk): bind connect auth scheme to token Address CodeRabbit's concurrent token-refresh finding by returning each access token with its DPoP status atomically. Signed-off-by: strantalis --- sdk/auth/access_token_source.go | 14 +++- sdk/auth/token_adding_interceptor.go | 16 ++-- sdk/auth/token_adding_interceptor_test.go | 95 ++++++++++++++++++++++- sdk/idp_access_token_source.go | 23 +++--- sdk/idp_cert_exchange.go | 21 ++--- sdk/idp_oauth_access_token_source.go | 32 ++++---- sdk/idp_oauth_access_token_source_test.go | 10 +-- sdk/idp_token_exchange_token_source.go | 14 +++- 8 files changed, 167 insertions(+), 58 deletions(-) diff --git a/sdk/auth/access_token_source.go b/sdk/auth/access_token_source.go index 5602aff794..c7be44fa8a 100644 --- a/sdk/auth/access_token_source.go +++ b/sdk/auth/access_token_source.go @@ -16,11 +16,17 @@ type AccessTokenSource interface { MakeToken(func(jwk.Key) ([]byte, error)) ([]byte, error) } -// DPoPSupportingAccessTokenSource is implemented by token sources that know -// whether the access token they most recently returned is DPoP-bound. +// AccessTokenCredential binds an access token to its DPoP status. +type AccessTokenCredential struct { + Token AccessToken + DPoPSupported bool +} + +// AccessTokenCredentialSource is implemented by token sources that can return +// an access token and its DPoP status atomically. // // Token sources without this optional interface retain the SDK's existing DPoP // behavior for backwards compatibility. -type DPoPSupportingAccessTokenSource interface { - DPoPSupported() bool +type AccessTokenCredentialSource interface { + AccessTokenCredential(ctx context.Context, client *http.Client) (AccessTokenCredential, error) } diff --git a/sdk/auth/token_adding_interceptor.go b/sdk/auth/token_adding_interceptor.go index 265d9b3399..0cec0cd0a5 100644 --- a/sdk/auth/token_adding_interceptor.go +++ b/sdk/auth/token_adding_interceptor.go @@ -94,19 +94,25 @@ func (i TokenAddingInterceptor) AddCredentialsConnect() connect.UnaryInterceptor ctx context.Context, req connect.AnyRequest, ) (connect.AnyResponse, error) { - accessToken, err := i.tokenSource.AccessToken(ctx, i.httpClient) + credential := AccessTokenCredential{DPoPSupported: true} + var err error + if credentialSource, ok := i.tokenSource.(AccessTokenCredentialSource); ok { + credential, err = credentialSource.AccessTokenCredential(ctx, i.httpClient) + } else { + credential.Token, err = i.tokenSource.AccessToken(ctx, i.httpClient) + } if err != nil { slog.ErrorContext(ctx, "error getting access token", slog.Any("error", err)) return nil, connect.NewError(connect.CodeUnauthenticated, err) } - if dpopTokenSource, ok := i.tokenSource.(DPoPSupportingAccessTokenSource); ok && !dpopTokenSource.DPoPSupported() { - req.Header().Set("Authorization", fmt.Sprintf("Bearer %s", accessToken)) + if !credential.DPoPSupported { + req.Header().Set("Authorization", fmt.Sprintf("Bearer %s", credential.Token)) return next(ctx, req) } // Add Authorization header - req.Header().Set("Authorization", fmt.Sprintf("DPoP %s", accessToken)) + req.Header().Set("Authorization", fmt.Sprintf("DPoP %s", credential.Token)) // Add DPoP header if possible slog.DebugContext( @@ -115,7 +121,7 @@ func (i TokenAddingInterceptor) AddCredentialsConnect() connect.UnaryInterceptor slog.String("dpop_htm", http.MethodPost), slog.Any("stream_type", req.Spec().StreamType), ) - dpopTok, err := i.GetDPoPToken(req.Spec().Procedure, http.MethodPost, string(accessToken)) + dpopTok, err := i.GetDPoPToken(req.Spec().Procedure, http.MethodPost, string(credential.Token)) if err == nil { req.Header().Set("DPoP", dpopTok) } else { diff --git a/sdk/auth/token_adding_interceptor_test.go b/sdk/auth/token_adding_interceptor_test.go index 80f9acd747..05da3d1c1b 100644 --- a/sdk/auth/token_adding_interceptor_test.go +++ b/sdk/auth/token_adding_interceptor_test.go @@ -142,6 +142,72 @@ func TestAddingTokensToOutgoingRequest_Connect_BearerToken(t *testing.T) { assert.False(t, ts.makeTokenCalled) } +func TestAddingTokensToOutgoingRequest_Connect_BindsSchemeToAtomicCredential(t *testing.T) { + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + key, err := jwk.FromRaw(privateKey) + require.NoError(t, err) + require.NoError(t, key.Set(jwk.AlgorithmKey, jwa.RS256)) + + for _, tc := range []struct { + name string + credential AccessTokenCredential + refreshed AccessTokenCredential + wantAuthorization string + wantDPoP bool + }{ + { + name: "dpop token is not downgraded when refresh returns bearer", + credential: AccessTokenCredential{ + Token: "dpop-access-token", + DPoPSupported: true, + }, + refreshed: AccessTokenCredential{ + Token: "bearer-access-token", + DPoPSupported: false, + }, + wantAuthorization: "DPoP dpop-access-token", + wantDPoP: true, + }, + { + name: "bearer token is not upgraded when refresh returns dpop", + credential: AccessTokenCredential{ + Token: "bearer-access-token", + DPoPSupported: false, + }, + refreshed: AccessTokenCredential{ + Token: "dpop-access-token", + DPoPSupported: true, + }, + wantAuthorization: "Bearer bearer-access-token", + wantDPoP: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ts := ConcurrentRefreshTokenSource{ + FakeTokenSource: FakeTokenSource{key: key}, + credential: tc.credential, + refreshed: tc.refreshed, + } + interceptor := NewTokenAddingInterceptorWithClient(&ts, httputil.SafeHTTPClient()) + serverConnect := FakeAccessServiceServerConnect{} + clientConnect, stopC := runConnectServer(&serverConnect, interceptor) + defer stopC() + + _, err := clientConnect.PublicKey(t.Context(), connect.NewRequest(&kas.PublicKeyRequest{})) + require.NoError(t, err) + + assert.Equal(t, []string{tc.wantAuthorization}, serverConnect.accessToken) + if tc.wantDPoP { + assert.NotEmpty(t, serverConnect.dpopToken[0]) + } else { + assert.Equal(t, []string{""}, serverConnect.dpopToken) + assert.False(t, ts.makeTokenCalled) + } + }) + } +} + func Test_InvalidCredentials_DoesNotSendMessage(t *testing.T) { ts := FakeTokenSource{key: nil, accessToken: ""} serverGrpc := FakeAccessServiceServer{} @@ -227,8 +293,33 @@ type DPoPFakeTokenSource struct { dpopSupported bool } -func (fts *DPoPFakeTokenSource) DPoPSupported() bool { - return fts.dpopSupported +func (fts *DPoPFakeTokenSource) AccessTokenCredential(ctx context.Context, client *http.Client) (AccessTokenCredential, error) { + token, err := fts.AccessToken(ctx, client) + if err != nil { + return AccessTokenCredential{}, err + } + + return AccessTokenCredential{Token: token, DPoPSupported: fts.dpopSupported}, nil +} + +// ConcurrentRefreshTokenSource models a refresh that completes between the +// legacy separate token and DPoP-status calls. +type ConcurrentRefreshTokenSource struct { + FakeTokenSource + credential AccessTokenCredential + refreshed AccessTokenCredential +} + +func (fts *ConcurrentRefreshTokenSource) AccessToken(context.Context, *http.Client) (AccessToken, error) { + return fts.credential.Token, nil +} + +func (fts *ConcurrentRefreshTokenSource) DPoPSupported() bool { + return fts.refreshed.DPoPSupported +} + +func (fts *ConcurrentRefreshTokenSource) AccessTokenCredential(context.Context, *http.Client) (AccessTokenCredential, error) { + return fts.credential, nil } func (fts *FakeTokenSource) AccessToken(context.Context, *http.Client) (AccessToken, error) { diff --git a/sdk/idp_access_token_source.go b/sdk/idp_access_token_source.go index 7a0b5ca8bb..9966ee4bf6 100644 --- a/sdk/idp_access_token_source.go +++ b/sdk/idp_access_token_source.go @@ -95,29 +95,30 @@ func NewIDPAccessTokenSource( } // AccessToken use a pointer receiver so that the token state is shared -func (t *IDPAccessTokenSource) AccessToken(_ context.Context, client *http.Client) (auth.AccessToken, error) { +func (t *IDPAccessTokenSource) AccessToken(ctx context.Context, client *http.Client) (auth.AccessToken, error) { + credential, err := t.AccessTokenCredential(ctx, client) + return credential.Token, err +} + +// AccessTokenCredential returns the cached access token and its DPoP status atomically. +func (t *IDPAccessTokenSource) AccessTokenCredential(_ context.Context, client *http.Client) (auth.AccessTokenCredential, error) { t.tokenMutex.Lock() defer t.tokenMutex.Unlock() if t.token == nil || t.token.Expired() { tok, err := oauth.GetAccessToken(client, t.idpTokenEndpoint.String(), t.scopes, t.credentials, t.dpopKey) if err != nil { - return "", fmt.Errorf("error getting access token: %w", err) + return auth.AccessTokenCredential{}, fmt.Errorf("error getting access token: %w", err) } t.token = tok } - return auth.AccessToken(t.token.AccessToken), nil + return auth.AccessTokenCredential{ + Token: auth.AccessToken(t.token.AccessToken), + DPoPSupported: strings.EqualFold(t.token.TokenType, "DPoP"), + }, nil } func (t *IDPAccessTokenSource) MakeToken(tokenMaker func(jwk.Key) ([]byte, error)) ([]byte, error) { return tokenMaker(t.dpopKey) } - -// DPoPSupported reports whether the most recently retrieved access token is DPoP-bound. -func (t *IDPAccessTokenSource) DPoPSupported() bool { - t.tokenMutex.Lock() - defer t.tokenMutex.Unlock() - - return t.token != nil && strings.EqualFold(t.token.TokenType, "DPoP") -} diff --git a/sdk/idp_cert_exchange.go b/sdk/idp_cert_exchange.go index b3e1ca4888..a19ad8c3d9 100644 --- a/sdk/idp_cert_exchange.go +++ b/sdk/idp_cert_exchange.go @@ -43,29 +43,30 @@ func NewCertExchangeTokenSource(logger *slog.Logger, info oauth.CertExchangeInfo } func (c *CertExchangeTokenSource) AccessToken(ctx context.Context, _ *http.Client) (auth.AccessToken, error) { + credential, err := c.AccessTokenCredential(ctx, nil) + return credential.Token, err +} + +// AccessTokenCredential returns the cached certificate-exchange token and its DPoP status atomically. +func (c *CertExchangeTokenSource) AccessTokenCredential(ctx context.Context, _ *http.Client) (auth.AccessTokenCredential, error) { c.tokenMutex.Lock() defer c.tokenMutex.Unlock() if c.token == nil || c.token.Expired() { tok, err := oauth.DoCertExchange(ctx, c.IdpEndpoint, c.info, c.credentials, c.key) if err != nil { - return "", err + return auth.AccessTokenCredential{}, err } c.token = tok } - return auth.AccessToken(c.token.AccessToken), nil + return auth.AccessTokenCredential{ + Token: auth.AccessToken(c.token.AccessToken), + DPoPSupported: strings.EqualFold(c.token.TokenType, "DPoP"), + }, nil } func (c *CertExchangeTokenSource) MakeToken(tokenMaker func(jwk.Key) ([]byte, error)) ([]byte, error) { return tokenMaker(c.key) } - -// DPoPSupported reports whether the most recently retrieved access token is DPoP-bound. -func (c *CertExchangeTokenSource) DPoPSupported() bool { - c.tokenMutex.Lock() - defer c.tokenMutex.Unlock() - - return c.token != nil && strings.EqualFold(c.token.TokenType, "DPoP") -} diff --git a/sdk/idp_oauth_access_token_source.go b/sdk/idp_oauth_access_token_source.go index 67fe7d9c05..29fa1422bc 100644 --- a/sdk/idp_oauth_access_token_source.go +++ b/sdk/idp_oauth_access_token_source.go @@ -5,7 +5,6 @@ import ( "fmt" "net/http" "strings" - "sync" "github.com/lestrrat-go/jwx/v2/jwk" "github.com/opentdf/platform/lib/ocrypto" @@ -20,8 +19,6 @@ type OAuthAccessTokenSource struct { dpopKey jwk.Key asymDecryption ocrypto.AsymDecryption dpopPEM string - dpopSupported bool - mutex sync.RWMutex } func NewOAuthAccessTokenSource( @@ -44,33 +41,30 @@ func NewOAuthAccessTokenSource( } // AccessToken use a pointer receiver so that the token state is shared -func (t *OAuthAccessTokenSource) AccessToken(_ context.Context, _ *http.Client) (auth.AccessToken, error) { // must satisfy auth.AccessTokenSource interface +func (t *OAuthAccessTokenSource) AccessToken(ctx context.Context, client *http.Client) (auth.AccessToken, error) { + credential, err := t.AccessTokenCredential(ctx, client) + return credential.Token, err +} + +// AccessTokenCredential returns an access token and its DPoP status from the same token response. +func (t *OAuthAccessTokenSource) AccessTokenCredential(_ context.Context, _ *http.Client) (auth.AccessTokenCredential, error) { tok, err := t.source.Token() if err != nil { - return "", fmt.Errorf("error getting access token: %w", err) + return auth.AccessTokenCredential{}, fmt.Errorf("error getting access token: %w", err) } // Non-nil with AccessToken and not Expired if !tok.Valid() { - return "", ErrAccessTokenInvalid + return auth.AccessTokenCredential{}, ErrAccessTokenInvalid // TODO: refresh tokens if expired? } - t.mutex.Lock() - t.dpopSupported = strings.EqualFold(tok.Type(), "DPoP") - t.mutex.Unlock() - - return auth.AccessToken(tok.AccessToken), nil + return auth.AccessTokenCredential{ + Token: auth.AccessToken(tok.AccessToken), + DPoPSupported: strings.EqualFold(tok.Type(), "DPoP"), + }, nil } func (t *OAuthAccessTokenSource) MakeToken(tokenMaker func(jwk.Key) ([]byte, error)) ([]byte, error) { return tokenMaker(t.dpopKey) } - -// DPoPSupported reports whether the most recently retrieved access token is DPoP-bound. -func (t *OAuthAccessTokenSource) DPoPSupported() bool { - t.mutex.RLock() - defer t.mutex.RUnlock() - - return t.dpopSupported -} diff --git a/sdk/idp_oauth_access_token_source_test.go b/sdk/idp_oauth_access_token_source_test.go index 8e611aa415..a391363517 100644 --- a/sdk/idp_oauth_access_token_source_test.go +++ b/sdk/idp_oauth_access_token_source_test.go @@ -33,10 +33,10 @@ func TestNewOAuthAccessTokenSource_Success(t *testing.T) { assert.Equal(t, dpopPublicKeyPEM, tokenSource.dpopPEM) assert.Equal(t, dpopKey, tokenSource.dpopKey) // Interface checks - tok, err := tokenSource.AccessToken(t.Context(), nil) + credential, err := tokenSource.AccessTokenCredential(t.Context(), nil) require.NoError(t, err) - assert.Equal(t, tok, auth.AccessToken(mockToken)) - assert.True(t, tokenSource.DPoPSupported()) + assert.Equal(t, credential.Token, auth.AccessToken(mockToken)) + assert.True(t, credential.DPoPSupported) made, err := tokenSource.MakeToken(func(jwk.Key) ([]byte, error) { return []byte(mockToken), nil }) require.NoError(t, err) assert.Equal(t, made, []byte(mockToken)) @@ -50,9 +50,9 @@ func TestOAuthAccessTokenSource_DPoPSupported_BearerToken(t *testing.T) { tokenSource, err := NewOAuthAccessTokenSource(mockSource, nil, &mockKey) require.NoError(t, err) - _, err = tokenSource.AccessToken(t.Context(), nil) + credential, err := tokenSource.AccessTokenCredential(t.Context(), nil) require.NoError(t, err) - assert.False(t, tokenSource.DPoPSupported()) + assert.False(t, credential.DPoPSupported) } func TestNewOAuthAccessTokenSource_ExpiredToken(t *testing.T) { diff --git a/sdk/idp_token_exchange_token_source.go b/sdk/idp_token_exchange_token_source.go index e3295650c8..576f33caf5 100644 --- a/sdk/idp_token_exchange_token_source.go +++ b/sdk/idp_token_exchange_token_source.go @@ -4,6 +4,7 @@ import ( "context" "log/slog" "net/http" + "strings" "github.com/lestrrat-go/jwx/v2/jwk" "github.com/opentdf/platform/lib/ocrypto" @@ -33,19 +34,28 @@ func NewIDPTokenExchangeTokenSource(logger *slog.Logger, exchangeInfo oauth.Toke } func (i *IDPTokenExchangeTokenSource) AccessToken(ctx context.Context, client *http.Client) (auth.AccessToken, error) { + credential, err := i.AccessTokenCredential(ctx, client) + return credential.Token, err +} + +// AccessTokenCredential returns the cached exchanged token and its DPoP status atomically. +func (i *IDPTokenExchangeTokenSource) AccessTokenCredential(ctx context.Context, client *http.Client) (auth.AccessTokenCredential, error) { i.tokenMutex.Lock() defer i.tokenMutex.Unlock() if i.token == nil || i.token.Expired() { tok, err := oauth.DoTokenExchange(ctx, client, i.idpTokenEndpoint.String(), i.scopes, i.credentials, i.TokenExchangeInfo, i.dpopKey) if err != nil { - return "", err + return auth.AccessTokenCredential{}, err } i.token = tok } - return auth.AccessToken(i.token.AccessToken), nil + return auth.AccessTokenCredential{ + Token: auth.AccessToken(i.token.AccessToken), + DPoPSupported: strings.EqualFold(i.token.TokenType, "DPoP"), + }, nil } func (i *IDPTokenExchangeTokenSource) MakeToken(keyMaker func(jwk.Key) ([]byte, error)) ([]byte, error) { From d5641b146b6881f8b0ebb578b690bb81fc279021 Mon Sep 17 00:00:00 2001 From: strantalis Date: Fri, 31 Jul 2026 10:14:37 -0400 Subject: [PATCH 3/4] refactor(sdk): model access token type Signed-off-by: strantalis --- sdk/auth/access_token_source.go | 26 +++++++++++++++++---- sdk/auth/access_token_source_test.go | 22 ++++++++++++++++++ sdk/auth/token_adding_interceptor.go | 4 ++-- sdk/auth/token_adding_interceptor_test.go | 28 ++++++++++------------- sdk/idp_access_token_source.go | 5 ++-- sdk/idp_cert_exchange.go | 5 ++-- sdk/idp_oauth_access_token_source.go | 5 ++-- sdk/idp_oauth_access_token_source_test.go | 6 ++--- sdk/idp_token_exchange_token_source.go | 5 ++-- 9 files changed, 69 insertions(+), 37 deletions(-) create mode 100644 sdk/auth/access_token_source_test.go diff --git a/sdk/auth/access_token_source.go b/sdk/auth/access_token_source.go index c7be44fa8a..791830957f 100644 --- a/sdk/auth/access_token_source.go +++ b/sdk/auth/access_token_source.go @@ -9,6 +9,24 @@ import ( type AccessToken string +type TokenType string + +const ( + TokenTypeBearer TokenType = "Bearer" + TokenTypeDPoP TokenType = "DPoP" +) + +// TokenTypeFromOAuthTokenType returns the authentication scheme required for +// an OAuth access token. Only the canonical DPoP token type requires a DPoP +// proof; all other values use the Bearer scheme. +func TokenTypeFromOAuthTokenType(tokenType string) TokenType { + if tokenType == string(TokenTypeDPoP) { + return TokenTypeDPoP + } + + return TokenTypeBearer +} + type AccessTokenSource interface { AccessToken(ctx context.Context, client *http.Client) (AccessToken, error) // MakeToken probably better to use `crypto.AsymDecryption` here than roll our own since this should be @@ -16,14 +34,14 @@ type AccessTokenSource interface { MakeToken(func(jwk.Key) ([]byte, error)) ([]byte, error) } -// AccessTokenCredential binds an access token to its DPoP status. +// AccessTokenCredential binds an access token to its authentication scheme. type AccessTokenCredential struct { - Token AccessToken - DPoPSupported bool + Token AccessToken + Type TokenType } // AccessTokenCredentialSource is implemented by token sources that can return -// an access token and its DPoP status atomically. +// an access token and its authentication scheme atomically. // // Token sources without this optional interface retain the SDK's existing DPoP // behavior for backwards compatibility. diff --git a/sdk/auth/access_token_source_test.go b/sdk/auth/access_token_source_test.go new file mode 100644 index 0000000000..28adfb521b --- /dev/null +++ b/sdk/auth/access_token_source_test.go @@ -0,0 +1,22 @@ +package auth + +import "testing" + +func TestTokenTypeFromOAuthTokenType(t *testing.T) { + for _, tc := range []struct { + name string + tokenType string + want TokenType + }{ + {name: "dpop", tokenType: "DPoP", want: TokenTypeDPoP}, + {name: "bearer", tokenType: "Bearer", want: TokenTypeBearer}, + {name: "lowercase dpop is not a token type", tokenType: "dpop", want: TokenTypeBearer}, + {name: "missing token type", want: TokenTypeBearer}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := TokenTypeFromOAuthTokenType(tc.tokenType); got != tc.want { + t.Errorf("TokenTypeFromOAuthTokenType(%q) = %q, want %q", tc.tokenType, got, tc.want) + } + }) + } +} diff --git a/sdk/auth/token_adding_interceptor.go b/sdk/auth/token_adding_interceptor.go index 0cec0cd0a5..0b18e08694 100644 --- a/sdk/auth/token_adding_interceptor.go +++ b/sdk/auth/token_adding_interceptor.go @@ -94,7 +94,7 @@ func (i TokenAddingInterceptor) AddCredentialsConnect() connect.UnaryInterceptor ctx context.Context, req connect.AnyRequest, ) (connect.AnyResponse, error) { - credential := AccessTokenCredential{DPoPSupported: true} + credential := AccessTokenCredential{Type: TokenTypeDPoP} var err error if credentialSource, ok := i.tokenSource.(AccessTokenCredentialSource); ok { credential, err = credentialSource.AccessTokenCredential(ctx, i.httpClient) @@ -106,7 +106,7 @@ func (i TokenAddingInterceptor) AddCredentialsConnect() connect.UnaryInterceptor return nil, connect.NewError(connect.CodeUnauthenticated, err) } - if !credential.DPoPSupported { + if credential.Type != TokenTypeDPoP { req.Header().Set("Authorization", fmt.Sprintf("Bearer %s", credential.Token)) return next(ctx, req) } diff --git a/sdk/auth/token_adding_interceptor_test.go b/sdk/auth/token_adding_interceptor_test.go index 05da3d1c1b..5c1203679b 100644 --- a/sdk/auth/token_adding_interceptor_test.go +++ b/sdk/auth/token_adding_interceptor_test.go @@ -46,7 +46,7 @@ func setupTokenAddingInterceptor(t *testing.T) (TokenAddingInterceptor, jwk.Key) key: key, accessToken: "thisisafakeaccesstoken", }, - dpopSupported: true, + tokenType: TokenTypeDPoP, } oo := NewTokenAddingInterceptorWithClient(&ts, httputil.SafeHTTPClientWithTLSConfig(&tls.Config{ @@ -127,7 +127,7 @@ func TestAddingTokensToOutgoingRequest_Connect_BearerToken(t *testing.T) { FakeTokenSource: FakeTokenSource{ accessToken: "thisisafakeaccesstoken", }, - dpopSupported: false, + tokenType: TokenTypeBearer, } interceptor := NewTokenAddingInterceptorWithClient(&ts, httputil.SafeHTTPClient()) serverConnect := FakeAccessServiceServerConnect{} @@ -159,12 +159,12 @@ func TestAddingTokensToOutgoingRequest_Connect_BindsSchemeToAtomicCredential(t * { name: "dpop token is not downgraded when refresh returns bearer", credential: AccessTokenCredential{ - Token: "dpop-access-token", - DPoPSupported: true, + Token: "dpop-access-token", + Type: TokenTypeDPoP, }, refreshed: AccessTokenCredential{ - Token: "bearer-access-token", - DPoPSupported: false, + Token: "bearer-access-token", + Type: TokenTypeBearer, }, wantAuthorization: "DPoP dpop-access-token", wantDPoP: true, @@ -172,12 +172,12 @@ func TestAddingTokensToOutgoingRequest_Connect_BindsSchemeToAtomicCredential(t * { name: "bearer token is not upgraded when refresh returns dpop", credential: AccessTokenCredential{ - Token: "bearer-access-token", - DPoPSupported: false, + Token: "bearer-access-token", + Type: TokenTypeBearer, }, refreshed: AccessTokenCredential{ - Token: "dpop-access-token", - DPoPSupported: true, + Token: "dpop-access-token", + Type: TokenTypeDPoP, }, wantAuthorization: "Bearer bearer-access-token", wantDPoP: false, @@ -290,7 +290,7 @@ type FakeTokenSource struct { type DPoPFakeTokenSource struct { FakeTokenSource - dpopSupported bool + tokenType TokenType } func (fts *DPoPFakeTokenSource) AccessTokenCredential(ctx context.Context, client *http.Client) (AccessTokenCredential, error) { @@ -299,7 +299,7 @@ func (fts *DPoPFakeTokenSource) AccessTokenCredential(ctx context.Context, clien return AccessTokenCredential{}, err } - return AccessTokenCredential{Token: token, DPoPSupported: fts.dpopSupported}, nil + return AccessTokenCredential{Token: token, Type: fts.tokenType}, nil } // ConcurrentRefreshTokenSource models a refresh that completes between the @@ -314,10 +314,6 @@ func (fts *ConcurrentRefreshTokenSource) AccessToken(context.Context, *http.Clie return fts.credential.Token, nil } -func (fts *ConcurrentRefreshTokenSource) DPoPSupported() bool { - return fts.refreshed.DPoPSupported -} - func (fts *ConcurrentRefreshTokenSource) AccessTokenCredential(context.Context, *http.Client) (AccessTokenCredential, error) { return fts.credential, nil } diff --git a/sdk/idp_access_token_source.go b/sdk/idp_access_token_source.go index 9966ee4bf6..ef45a29760 100644 --- a/sdk/idp_access_token_source.go +++ b/sdk/idp_access_token_source.go @@ -5,7 +5,6 @@ import ( "fmt" "net/http" "net/url" - "strings" "sync" "github.com/lestrrat-go/jwx/v2/jwa" @@ -114,8 +113,8 @@ func (t *IDPAccessTokenSource) AccessTokenCredential(_ context.Context, client * } return auth.AccessTokenCredential{ - Token: auth.AccessToken(t.token.AccessToken), - DPoPSupported: strings.EqualFold(t.token.TokenType, "DPoP"), + Token: auth.AccessToken(t.token.AccessToken), + Type: auth.TokenTypeFromOAuthTokenType(t.token.TokenType), }, nil } diff --git a/sdk/idp_cert_exchange.go b/sdk/idp_cert_exchange.go index a19ad8c3d9..5fec5182b6 100644 --- a/sdk/idp_cert_exchange.go +++ b/sdk/idp_cert_exchange.go @@ -4,7 +4,6 @@ import ( "context" "log/slog" "net/http" - "strings" "sync" "github.com/lestrrat-go/jwx/v2/jwk" @@ -62,8 +61,8 @@ func (c *CertExchangeTokenSource) AccessTokenCredential(ctx context.Context, _ * } return auth.AccessTokenCredential{ - Token: auth.AccessToken(c.token.AccessToken), - DPoPSupported: strings.EqualFold(c.token.TokenType, "DPoP"), + Token: auth.AccessToken(c.token.AccessToken), + Type: auth.TokenTypeFromOAuthTokenType(c.token.TokenType), }, nil } diff --git a/sdk/idp_oauth_access_token_source.go b/sdk/idp_oauth_access_token_source.go index 29fa1422bc..98011ef02f 100644 --- a/sdk/idp_oauth_access_token_source.go +++ b/sdk/idp_oauth_access_token_source.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/http" - "strings" "github.com/lestrrat-go/jwx/v2/jwk" "github.com/opentdf/platform/lib/ocrypto" @@ -60,8 +59,8 @@ func (t *OAuthAccessTokenSource) AccessTokenCredential(_ context.Context, _ *htt } return auth.AccessTokenCredential{ - Token: auth.AccessToken(tok.AccessToken), - DPoPSupported: strings.EqualFold(tok.Type(), "DPoP"), + Token: auth.AccessToken(tok.AccessToken), + Type: auth.TokenTypeFromOAuthTokenType(tok.Type()), }, nil } diff --git a/sdk/idp_oauth_access_token_source_test.go b/sdk/idp_oauth_access_token_source_test.go index a391363517..72233760cf 100644 --- a/sdk/idp_oauth_access_token_source_test.go +++ b/sdk/idp_oauth_access_token_source_test.go @@ -36,13 +36,13 @@ func TestNewOAuthAccessTokenSource_Success(t *testing.T) { credential, err := tokenSource.AccessTokenCredential(t.Context(), nil) require.NoError(t, err) assert.Equal(t, credential.Token, auth.AccessToken(mockToken)) - assert.True(t, credential.DPoPSupported) + assert.Equal(t, auth.TokenTypeDPoP, credential.Type) made, err := tokenSource.MakeToken(func(jwk.Key) ([]byte, error) { return []byte(mockToken), nil }) require.NoError(t, err) assert.Equal(t, made, []byte(mockToken)) } -func TestOAuthAccessTokenSource_DPoPSupported_BearerToken(t *testing.T) { +func TestOAuthAccessTokenSource_TokenTypeBearer(t *testing.T) { mockSource := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "mockToken", TokenType: "Bearer"}) mockKey, err := ocrypto.NewRSAKeyPair(dpopKeySize) require.NoError(t, err) @@ -52,7 +52,7 @@ func TestOAuthAccessTokenSource_DPoPSupported_BearerToken(t *testing.T) { credential, err := tokenSource.AccessTokenCredential(t.Context(), nil) require.NoError(t, err) - assert.False(t, credential.DPoPSupported) + assert.Equal(t, auth.TokenTypeBearer, credential.Type) } func TestNewOAuthAccessTokenSource_ExpiredToken(t *testing.T) { diff --git a/sdk/idp_token_exchange_token_source.go b/sdk/idp_token_exchange_token_source.go index 576f33caf5..0968a1fe8b 100644 --- a/sdk/idp_token_exchange_token_source.go +++ b/sdk/idp_token_exchange_token_source.go @@ -4,7 +4,6 @@ import ( "context" "log/slog" "net/http" - "strings" "github.com/lestrrat-go/jwx/v2/jwk" "github.com/opentdf/platform/lib/ocrypto" @@ -53,8 +52,8 @@ func (i *IDPTokenExchangeTokenSource) AccessTokenCredential(ctx context.Context, } return auth.AccessTokenCredential{ - Token: auth.AccessToken(i.token.AccessToken), - DPoPSupported: strings.EqualFold(i.token.TokenType, "DPoP"), + Token: auth.AccessToken(i.token.AccessToken), + Type: auth.TokenTypeFromOAuthTokenType(i.token.TokenType), }, nil } From 66a1010192448a20439d77ba001a2ed3b4e78819 Mon Sep 17 00:00:00 2001 From: strantalis Date: Fri, 31 Jul 2026 10:22:47 -0400 Subject: [PATCH 4/4] fix(sdk): honor OAuth token type casing Signed-off-by: strantalis --- sdk/auth/access_token_source.go | 7 +++--- sdk/auth/access_token_source_test.go | 2 +- sdk/auth/token_adding_interceptor_test.go | 29 +++++++---------------- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/sdk/auth/access_token_source.go b/sdk/auth/access_token_source.go index 791830957f..21dd817e9a 100644 --- a/sdk/auth/access_token_source.go +++ b/sdk/auth/access_token_source.go @@ -3,6 +3,7 @@ package auth import ( "context" "net/http" + "strings" "github.com/lestrrat-go/jwx/v2/jwk" ) @@ -17,10 +18,10 @@ const ( ) // TokenTypeFromOAuthTokenType returns the authentication scheme required for -// an OAuth access token. Only the canonical DPoP token type requires a DPoP -// proof; all other values use the Bearer scheme. +// an OAuth access token. A DPoP token type requires a DPoP proof; all other +// values use the Bearer scheme. func TokenTypeFromOAuthTokenType(tokenType string) TokenType { - if tokenType == string(TokenTypeDPoP) { + if strings.EqualFold(tokenType, string(TokenTypeDPoP)) { return TokenTypeDPoP } diff --git a/sdk/auth/access_token_source_test.go b/sdk/auth/access_token_source_test.go index 28adfb521b..6730875278 100644 --- a/sdk/auth/access_token_source_test.go +++ b/sdk/auth/access_token_source_test.go @@ -10,7 +10,7 @@ func TestTokenTypeFromOAuthTokenType(t *testing.T) { }{ {name: "dpop", tokenType: "DPoP", want: TokenTypeDPoP}, {name: "bearer", tokenType: "Bearer", want: TokenTypeBearer}, - {name: "lowercase dpop is not a token type", tokenType: "dpop", want: TokenTypeBearer}, + {name: "lowercase dpop", tokenType: "dpop", want: TokenTypeDPoP}, {name: "missing token type", want: TokenTypeBearer}, } { t.Run(tc.name, func(t *testing.T) { diff --git a/sdk/auth/token_adding_interceptor_test.go b/sdk/auth/token_adding_interceptor_test.go index 5c1203679b..eb9e6f4743 100644 --- a/sdk/auth/token_adding_interceptor_test.go +++ b/sdk/auth/token_adding_interceptor_test.go @@ -142,7 +142,7 @@ func TestAddingTokensToOutgoingRequest_Connect_BearerToken(t *testing.T) { assert.False(t, ts.makeTokenCalled) } -func TestAddingTokensToOutgoingRequest_Connect_BindsSchemeToAtomicCredential(t *testing.T) { +func TestAddingTokensToOutgoingRequest_Connect_UsesAtomicCredential(t *testing.T) { privateKey, err := rsa.GenerateKey(rand.Reader, 2048) require.NoError(t, err) key, err := jwk.FromRaw(privateKey) @@ -152,42 +152,32 @@ func TestAddingTokensToOutgoingRequest_Connect_BindsSchemeToAtomicCredential(t * for _, tc := range []struct { name string credential AccessTokenCredential - refreshed AccessTokenCredential wantAuthorization string wantDPoP bool }{ { - name: "dpop token is not downgraded when refresh returns bearer", + name: "dpop", credential: AccessTokenCredential{ Token: "dpop-access-token", Type: TokenTypeDPoP, }, - refreshed: AccessTokenCredential{ - Token: "bearer-access-token", - Type: TokenTypeBearer, - }, wantAuthorization: "DPoP dpop-access-token", wantDPoP: true, }, { - name: "bearer token is not upgraded when refresh returns dpop", + name: "bearer", credential: AccessTokenCredential{ Token: "bearer-access-token", Type: TokenTypeBearer, }, - refreshed: AccessTokenCredential{ - Token: "dpop-access-token", - Type: TokenTypeDPoP, - }, wantAuthorization: "Bearer bearer-access-token", wantDPoP: false, }, } { t.Run(tc.name, func(t *testing.T) { - ts := ConcurrentRefreshTokenSource{ + ts := AtomicCredentialTokenSource{ FakeTokenSource: FakeTokenSource{key: key}, credential: tc.credential, - refreshed: tc.refreshed, } interceptor := NewTokenAddingInterceptorWithClient(&ts, httputil.SafeHTTPClient()) serverConnect := FakeAccessServiceServerConnect{} @@ -302,19 +292,16 @@ func (fts *DPoPFakeTokenSource) AccessTokenCredential(ctx context.Context, clien return AccessTokenCredential{Token: token, Type: fts.tokenType}, nil } -// ConcurrentRefreshTokenSource models a refresh that completes between the -// legacy separate token and DPoP-status calls. -type ConcurrentRefreshTokenSource struct { +type AtomicCredentialTokenSource struct { FakeTokenSource credential AccessTokenCredential - refreshed AccessTokenCredential } -func (fts *ConcurrentRefreshTokenSource) AccessToken(context.Context, *http.Client) (AccessToken, error) { - return fts.credential.Token, nil +func (fts *AtomicCredentialTokenSource) AccessToken(context.Context, *http.Client) (AccessToken, error) { + return "", errors.New("AccessToken must not be called when AccessTokenCredential is available") } -func (fts *ConcurrentRefreshTokenSource) AccessTokenCredential(context.Context, *http.Client) (AccessTokenCredential, error) { +func (fts *AtomicCredentialTokenSource) AccessTokenCredential(context.Context, *http.Client) (AccessTokenCredential, error) { return fts.credential, nil }