Skip to content

Commit 31c554d

Browse files
Return more detailed error messaged when OAuth endpoints cannot be resolved. (#1250)
## What changes are proposed in this pull request? This PR slightly updates the endpoint resolution logic to return more detailed error messages when OAuth endpoints cannot be resolved. It also updates the TokenCache not found error to be more aligned with its semantic in the context of the token cache. ## How is this tested? Updated relevant unit tests.
1 parent 667c53b commit 31c554d

8 files changed

Lines changed: 17 additions & 12 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### New Features and Improvements
66

7+
* Return more detailed error messages when OAuth endpoints cannot be resolved.
78
* Use a free port in `u2m` authentication flows rather than 8020.
89

910
### Bug Fixes

config/auth_u2m.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (u u2mCredentials) makeVisitor(auth oauth2.TokenSource) func(*http.Request)
8282
func (u u2mCredentials) errorHandler(ctx context.Context, cfg *Config, arg u2m.OAuthArgument, err error) error {
8383
// If the current OAuth argument doesn't have a corresponding session
8484
// token, fall back to the next credentials strategy.
85-
if errors.Is(err, cache.ErrNotConfigured) {
85+
if errors.Is(err, cache.ErrNotFound) {
8686
return nil
8787
}
8888
// If there is an existing token but the refresh token is invalid,

config/auth_u2m_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func TestDatabricksCli_ErrorHandler(t *testing.T) {
9999
{
100100
name: "not configured is ignored",
101101
arg: workspaceArg,
102-
err: cache.ErrNotConfigured,
102+
err: cache.ErrNotFound,
103103
want: nil,
104104
},
105105
{

config/in_memory_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ type InMemoryTokenCache struct {
1313
func (i *InMemoryTokenCache) Lookup(key string) (*oauth2.Token, error) {
1414
token, ok := i.Tokens[key]
1515
if !ok {
16-
return nil, cache.ErrNotConfigured
16+
return nil, cache.ErrNotFound
1717
}
1818
return token, nil
1919
}

credentials/u2m/cache/cache.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,15 @@ import (
1818
"golang.org/x/oauth2"
1919
)
2020

21+
var ErrNotFound = errors.New("token not found")
22+
2123
// TokenCache is an interface for storing and looking up OAuth tokens.
2224
type TokenCache interface {
2325
// Store stores the token with the given key, replacing any existing token.
2426
// If t is nil, it deletes the token.
2527
Store(key string, t *oauth2.Token) error
2628

2729
// Lookup looks up the token with the given key. If the token is not found, it
28-
// returns ErrNotConfigured.
30+
// returns ErrNotFound.
2931
Lookup(key string) (*oauth2.Token, error)
3032
}
31-
32-
var ErrNotConfigured = errors.New("databricks OAuth is not configured for this host")

credentials/u2m/cache/file.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func (c *fileTokenCache) Lookup(key string) (*oauth2.Token, error) {
116116
}
117117
t, ok := f.Tokens[key]
118118
if !ok {
119-
return nil, ErrNotConfigured
119+
return nil, ErrNotFound
120120
}
121121
return t, nil
122122
}

credentials/u2m/cache/file_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ func TestStoreAndLookup(t *testing.T) {
3333
assert.Equal(t, "abc", tok.AccessToken)
3434

3535
_, err = c.Lookup("z")
36-
assert.Equal(t, ErrNotConfigured, err)
36+
assert.Equal(t, ErrNotFound, err)
3737
}
3838

3939
func TestNoCacheFileReturnsErrNotConfigured(t *testing.T) {
4040
l, err := NewFileTokenCache(WithFileLocation(setup(t)))
4141
require.NoError(t, err)
4242
_, err = l.Lookup("x")
43-
assert.Equal(t, ErrNotConfigured, err)
43+
assert.Equal(t, ErrNotFound, err)
4444
}
4545

4646
func TestLoadCorruptFile(t *testing.T) {

credentials/u2m/endpoint_supplier.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@ import (
55
"errors"
66
"fmt"
77

8+
"github.com/databricks/databricks-sdk-go/apierr"
89
"github.com/databricks/databricks-sdk-go/httpclient"
910
)
1011

12+
var ErrOAuthNotSupported = errors.New("databricks OAuth is not supported for this host")
13+
1114
// OAuthEndpointSupplier provides the http functionality needed for interacting with the
1215
// Databricks OAuth APIs.
1316
type OAuthEndpointSupplier interface {
@@ -31,7 +34,10 @@ func (c *BasicOAuthEndpointSupplier) GetWorkspaceOAuthEndpoints(ctx context.Cont
3134
oidc := fmt.Sprintf("%s/oidc/.well-known/oauth-authorization-server", workspaceHost)
3235
var oauthEndpoints OAuthAuthorizationServer
3336
if err := c.Client.Do(ctx, "GET", oidc, httpclient.WithResponseUnmarshal(&oauthEndpoints)); err != nil {
34-
return nil, ErrOAuthNotSupported
37+
if errors.Is(err, apierr.ErrNotFound) {
38+
return nil, ErrOAuthNotSupported
39+
}
40+
return nil, fmt.Errorf("failed to get OAuth endpoints: %w", err)
3541
}
3642
return &oauthEndpoints, nil
3743
}
@@ -45,8 +51,6 @@ func (c *BasicOAuthEndpointSupplier) GetAccountOAuthEndpoints(ctx context.Contex
4551
}, nil
4652
}
4753

48-
var ErrOAuthNotSupported = errors.New("databricks OAuth is not supported for this host")
49-
5054
// OAuthAuthorizationServer contains the OAuth endpoints for a Databricks account
5155
// or workspace.
5256
type OAuthAuthorizationServer struct {

0 commit comments

Comments
 (0)