Skip to content

Conversation

@mcoulombe
Copy link
Collaborator

No description provided.

client.go Outdated
}

// Wrap the HTTP client's transport with the federated-idetity-aware transport
c.HTTP.Transport = transport
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutating the HTTP client that's passed in isn't great, as it may be used elsewhere.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out we can just make a new http.Client that takes the important bits from the original client, like here.

@mcoulombe mcoulombe force-pushed the max/token-exchange-take-2 branch from ef1a6ba to 9031f13 Compare October 17, 2025 01:53
@mcoulombe mcoulombe force-pushed the max/token-exchange-take-2 branch from 9031f13 to f965ee1 Compare October 17, 2025 02:54
// When set along with IDTokenFunc, the client will use identity federation for authentication.
// For OAuth, if empty and ClientSecret is provided, the ClientID will be derived from the ClientSecret.
// Overwrites APIKey if set.
ClientID string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding more and more parameters to the Client type, let's just make different pluggable authentication mechanisms and plug them into a single place like here.

return client
}

// oauthTokenSource implements oauth2.TokenSource using client credentials flow.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, I don't know how I missed that you did this already!

@mcoulombe
Copy link
Collaborator Author

Closing, we'll use a slightly different approach

@mcoulombe mcoulombe closed this Oct 17, 2025
@mcoulombe mcoulombe deleted the max/token-exchange-take-2 branch October 17, 2025 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants