identityfederation: added a utility method to perform the token exchange #58

mcoulombe · 2025-10-15T19:25:50Z

Updates tailscale/terraform-provider-tailscale#485

See tailscale/terraform-provider-tailscale#567 for usage example. At first I implemented the token exchange in the provider directly but the small GenerateAccessToken is generally useful and aligns with the small OauthConfig utility to help setup different auth options when instantiating the Tailscale client.

Bumped go and some dependencies as a drive-by.

oxtoacart

Left a couple of thoughts.

oxtoacart · 2025-10-15T19:36:03Z

go.mod

 module tailscale.com/client/tailscale/v2

-go 1.24.0
+go 1.25.1


Do we require anything in go 1.25? If not, why force people to upgrade?

Nothing specifically requires go1.25, I just thought we generally try to keep the Tailscale projects as up-to-date as possible. What's our cadence or strategy to keep projects on the latest Go version?

I'll remove the bump from the PR.

oxtoacart · 2025-10-15T20:49:33Z

identityfederation.go

+// GenerateAccessToken exchanges a federated identity client ID and identity token for a Tailscale API access token.
+// Used to delegate access to Tailscale resources via workload identity federation.
+//
+// Usage example:


This would be helpful in the README as well.

Whoops I missed the readme as usage examples, moved there.

oxtoacart · 2025-10-15T20:51:35Z

identityfederation.go

+//
+// Usage example:
+//
+//	accessToken, err := IdentityFederationConfig{


Since we get the token only once and then pass it to the client, after the token expires (in 1 hour), the client will stop working.

It might be better to follow the pattern of the Go OAuth library, which provides a special transport that uses a special token source that refreshes tokens when necessary.

I initially did not go with that approach because unlike with OAuth creds the ID token cannot indefinitely generate new API access tokens as needed. I updated the PR with a custom roundtripper implementation to make it a bit easier for consumers to handle token expiration and surface more useful error messages if their workflow lasts longer than the token lifetimes. Lmk if how if that make sense.

identityfederation.go

oxtoacart · 2025-10-16T17:14:09Z

README.md

+}
+```
+
+### With a dynamic ID token generator 


Good call including this example!

oxtoacart

Left some more comments.

identityfederation.go

oxtoacart · 2025-10-16T17:20:22Z

identityfederation.go

+
+// refreshToken performs the token exchange and updates the cached token.
+func (t *tokenTransport) refreshToken(ctx context.Context) error {
+	if t.config.IDTokenGenerator != nil && validateIDToken(t.config.IDToken) != nil {


Your suggestion of just having one IDTokenGenerator parameter on the config sounds good to me, as it gets rid of a lot of these if/else branches.

At that point, you could also just call it IDToken again.

oxtoacart · 2025-10-16T17:21:06Z

identityfederation.go

+	if t.config.IDTokenGenerator != nil && validateIDToken(t.config.IDToken) != nil {
+		idToken, err := t.config.IDTokenGenerator()
+		if err != nil {
+			return fmt.Errorf("failed to generate ID token: %w", err)


Nit: rather than referring to getting an ID token as "generating", consider just calling it "get" or "fetch".

oxtoacart · 2025-10-16T17:22:46Z

identityfederation.go

+	}
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	client := &http.Client{


I'm not sure the best way to solve it, but one of the reasons that the top-level Client type accepts an http.Client is to allow configuring things like timeouts, proxy settings, etc. Using a separate client for fetching the token skips that and could cause confusion if it breaks because it's missing something like a proxy setting.

Hmm that's a tricky one. Not having strictly the same timeouts etc. for the toke exchange compared to calling the Tailscale API is mostly fine, but I see how ignoring proxy settings could cause problems.

It creates a chicken-and-egg problem because ideally we want to validate the WIF config as soon as possible. The only place to do so since the client itself does not have a constructor is IdentityFederationConfig.HTTPClient to refresh (or in that case initialize) the API access token before the consumer had a chance to configure the underlying http client.

I'll have to think about this one.

It occurs to me that we have a similar problem with OAuth already, so I don't think we need to solve this here.

identityfederation.go

Updates tailscale/terraform-provider-tailscale#485 Signed-off-by: mcoulombe <[email protected]>

oxtoacart

Left a few more comments, but no absolute blockers.

I do think it would be good to let IDTokenFunc accept a Context, especially since we call it while holding a mutex.

oxtoacart · 2025-10-16T18:55:32Z

identityfederation.go

+	token     string
+	expiresAt time.Time
+
+	tokenRefreshMu sync.Mutex


Nit: our convention is to place mutexes before the values they guard, with a comment like "guards the below"

oxtoacart · 2025-10-16T20:22:26Z

identityfederation.go

+	}
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	client := &http.Client{


It occurs to me that we have a similar problem with OAuth already, so I don't think we need to solve this here.

oxtoacart · 2025-10-16T20:25:31Z

identityfederation.go

+	defer t.tokenRefreshMu.Unlock()
+
+	if t.idToken == "" || validateIDToken(t.idToken) != nil {
+		idToken, err := t.config.IDTokenFunc()


I think it would be good to let IDTokenFunc accept a context and pass our context here. Especially since we're holding tokenRefreshMu while making this potentially remote call, it's important for that to respect some kind of timeout so that we don't lock the whole process.

oxtoacart · 2025-10-16T20:26:27Z

identityfederation.go

+	// with a fresh ID token.
+	IDTokenFunc func() (string, error)
+	// BaseURL is an optional base URL for the API server to which we'll connect. Defaults to https://api.tailscale.com.
+	BaseURL string


It's unfortunate that we need that both here and in the Client, but I don't see an obvious way around it.

oxtoacart · 2025-10-16T20:29:21Z

identityfederation.go

+	}
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	client := &http.Client{


Consider moving this http.Client to the tokenTransport type so that you can reuse it (and therefore potentially reuse underlying TCP connections).

oxtoacart · 2025-10-16T20:32:35Z

identityfederation.go

+
+// HTTPClient constructs an HTTP client that authenticates using identity federation.
+// The client automatically handles token exchange and caching.
+func (c IdentityFederationConfig) HTTPClient() (*http.Client, error) {


It might be surprising that this makes a network call and can hang for a while. Consider:

Documenting that behavior in the comment

Accepting and respecting a Context to further emphasize this

mcoulombe · 2025-10-17T15:30:58Z

Closing, we'll use a different approach and refactor how OAuth is managed.

mcoulombe force-pushed the max/token-exchange branch 2 times, most recently from 7a1f5ac to 33fdadb Compare October 15, 2025 19:31

mcoulombe requested review from mpminardi and oxtoacart October 15, 2025 19:32

oxtoacart reviewed Oct 15, 2025

View reviewed changes

mcoulombe force-pushed the max/token-exchange branch 3 times, most recently from bdcb257 to e8e882c Compare October 16, 2025 15:45

mcoulombe commented Oct 16, 2025

View reviewed changes

identityfederation.go Show resolved Hide resolved

oxtoacart reviewed Oct 16, 2025

View reviewed changes

README.md Outdated

}

```

### With a dynamic ID token generator

Copy link

Collaborator

oxtoacart Oct 16, 2025

Choose a reason for hiding this comment

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

Good call including this example!

oxtoacart reviewed Oct 16, 2025

View reviewed changes

mcoulombe force-pushed the max/token-exchange branch from e8e882c to 3c6f3a0 Compare October 16, 2025 18:33

identityfederation: added a utility method to perform the token exchange

2645aaa

Updates tailscale/terraform-provider-tailscale#485 Signed-off-by: mcoulombe <[email protected]>

mcoulombe force-pushed the max/token-exchange branch from 3c6f3a0 to 2645aaa Compare October 16, 2025 19:08

oxtoacart approved these changes Oct 16, 2025

View reviewed changes

mcoulombe closed this Oct 17, 2025

mcoulombe deleted the max/token-exchange branch October 17, 2025 17:55

identityfederation: added a utility method to perform the token exchange #58

identityfederation: added a utility method to perform the token exchange #58

Uh oh!

Conversation

mcoulombe commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oxtoacart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcoulombe Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oxtoacart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcoulombe Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

oxtoacart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcoulombe commented Oct 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mcoulombe commented Oct 15, 2025 •

edited

Loading

mcoulombe Oct 16, 2025 •

edited

Loading

mcoulombe Oct 16, 2025 •

edited

Loading