zeroshade commented on code in PR #579:
URL: https://github.com/apache/iceberg-go/pull/579#discussion_r2641355973
##########
catalog/rest/rest.go:
##########
@@ -587,23 +503,37 @@ func (r *Catalog) createSession(ctx context.Context, opts
*options) (*http.Clien
}
cl := &http.Client{Transport: session}
- token := opts.oauthToken
- if token == "" && opts.credential != "" {
- var err error
- if token, err = r.fetchAccessToken(cl, opts.credential, opts);
err != nil {
- return nil, fmt.Errorf("auth error: %w", err)
- }
- }
+ // This creates an OAuth2Manager if no auth manager is provided.
+ // This goal is to replicate existing behavior.
+ if opts.credential != "" {
+ if _, ok := opts.authManager.(*Oauth2AuthManager); !ok {
+ authURI := opts.authUri
+ if authURI == nil {
+ authURI = r.baseURI.JoinPath("oauth/tokens")
+ }
- if token != "" {
- session.defaultHeaders.Set(authorizationHeader, bearerPrefix+"
"+token)
+ opts.authManager = &Oauth2AuthManager{
+ Credential: opts.credential,
+ AuthURI: authURI,
+ Scope: opts.scope,
+ Client: cl,
+ }
+ }
Review Comment:
So if the user provides a credential and an auth manager that isn't an
Oauth2AuthManager we're going to throw away what they gave us and replace it
with an Oauth Manager? That seems wrong...
If the user gives us an AuthManager we should be using it, period. Perhaps
the `AuthManager` interface needs more methods like `SetCredential` and/or
`SetAuthURI`?
Also, why are we removing the token argument? If the user already has a
bearer token, they should be able to use it rather than requiring them to go
through the oauth flow again and get a new token.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
