various: change OAuth and WIF auth key resolvers to take struct args

Change signature of OAuth and identityfederation auth key resolution
hooks to take in structs instead of lists of args as they were getting
unwieldily.

Updates https://github.com/tailscale/tailscale/issues/20339

Signed-off-by: Mario Minardi <mario@tailscale.com>

cmd/gitops-pusher/gitops-pusher.go

@@ -255,7 +255,11 @@ func getCredentials() (*http.Client, string) {
 		} else if idok && idToken != "" && oiok && oauthId != "" {
 			if exchangeJWTForToken, ok := tailscale.HookExchangeJWTForTokenViaWIF.GetOk(); ok {
 				var err error
-				apiKeyEnv, err = exchangeJWTForToken(context.Background(), fmt.Sprintf("https://%s", *apiServer), oauthId, idToken)
+				apiKeyEnv, err = exchangeJWTForToken(context.Background(), tailscale.ExchangeJWTForTokenWIFArgs{
+					BaseURL:  fmt.Sprintf("https://%s", *apiServer),
+					ClientID: oauthId,
+					IDToken:  idToken,
+				})
 				if err != nil {
 					log.Fatal(err)
 				}

cmd/tailscale/cli/up.go

@@ -653,7 +653,10 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE
 				}
 			}
 
-			authKey, err = f(ctx, clientSecret, prefs.AdvertiseTags)
+			authKey, err = f(ctx, tailscale.ResolveAuthKeyArgs{
+				AuthKey: clientSecret,
+				Tags:    prefs.AdvertiseTags,
+			})
 			if err != nil {
 				return err
 			}
@@ -666,7 +669,13 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE
 				return err
 			}
 
-			authKey, err = f(ctx, prefs.ControlURL, upArgs.clientID, idToken, upArgs.audience, prefs.AdvertiseTags)
+			authKey, err = f(ctx, tailscale.ResolveAuthKeyWIFArgs{
+				BaseURL:  prefs.ControlURL,
+				ClientID: upArgs.clientID,
+				IDToken:  idToken,
+				Audience: upArgs.audience,
+				Tags:     prefs.AdvertiseTags,
+			})
 			if err != nil {
 				return err
 			}

feature/identityfederation/identityfederation.go

@@ -29,34 +29,38 @@ func init() {
 }
 
 // resolveAuthKey uses OIDC identity federation to exchange the provided ID token and client ID for an authkey.
-func resolveAuthKey(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error) {
+func resolveAuthKey(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error) {
-	if clientID == "" {
+	if args.ClientID == "" {
 		return "", nil // Short-circuit, no client ID means not using identity federation
 	}
 
-	if idToken == "" {
+	if args.IDToken == "" {
-		if audience == "" {
+		if args.Audience == "" {
 			return "", errors.New("federated identity requires either an ID token or an audience")
 		}
-		providerIdToken, err := wif.ObtainProviderToken(ctx, audience)
+		providerIdToken, err := wif.ObtainProviderToken(ctx, args.Audience)
 		if err != nil {
 			return "", errors.New("federated identity authkeys require --id-token")
 		}
-		idToken = providerIdToken
+		args.IDToken = providerIdToken
 	}
-	if len(tags) == 0 {
+	if len(args.Tags) == 0 {
 		return "", errors.New("federated identity authkeys require --advertise-tags")
 	}
-	if baseURL == "" {
+	if args.BaseURL == "" {
-		baseURL = ipn.DefaultControlURL
+		args.BaseURL = ipn.DefaultControlURL
 	}
 
-	strippedID, ephemeral, preauth, err := parseOptionalAttributes(clientID)
+	strippedID, ephemeral, preauth, err := parseOptionalAttributes(args.ClientID)
 	if err != nil {
 		return "", fmt.Errorf("failed to parse optional config attributes: %w", err)
 	}
 
-	accessToken, err := exchangeJWTForToken(ctx, baseURL, strippedID, idToken)
+	accessToken, err := exchangeJWTForToken(ctx, tailscale.ExchangeJWTForTokenWIFArgs{
+		BaseURL:  args.BaseURL,
+		ClientID: strippedID,
+		IDToken:  args.IDToken,
+	})
 	if err != nil {
 		return "", fmt.Errorf("failed to exchange JWT for access token: %w", err)
 	}
@@ -66,7 +70,7 @@ func resolveAuthKey(ctx context.Context, baseURL, clientID, idToken, audience st
 
 	tsClient := tailscale.NewClient("-", tailscale.APIKey(accessToken))
 	tsClient.UserAgent = "tailscale-cli-identity-federation"
-	tsClient.BaseURL = baseURL
+	tsClient.BaseURL = args.BaseURL
 
 	authkey, _, err := tsClient.CreateKey(ctx, tailscale.KeyCapabilities{
 		Devices: tailscale.KeyDeviceCapabilities{
@@ -74,7 +78,7 @@ func resolveAuthKey(ctx context.Context, baseURL, clientID, idToken, audience st
 				Reusable:      false,
 				Ephemeral:     ephemeral,
 				Preauthorized: preauth,
-				Tags:          tags,
+				Tags:          args.Tags,
 			},
 		},
 	})
@@ -117,15 +121,15 @@ func parseOptionalAttributes(clientID string) (strippedID string, ephemeral bool
 }
 
 // exchangeJWTForToken exchanges a JWT for a Tailscale access token.
-func exchangeJWTForToken(ctx context.Context, baseURL, clientID, idToken string) (string, error) {
+func exchangeJWTForToken(ctx context.Context, args tailscale.ExchangeJWTForTokenWIFArgs) (string, error) {
 	httpClient := &http.Client{Timeout: 10 * time.Second}
 	ctx = context.WithValue(ctx, oauth2.HTTPClient, httpClient)
 
 	token, err := (&oauth2.Config{
 		Endpoint: oauth2.Endpoint{
-			TokenURL: fmt.Sprintf("%s/api/v2/oauth/token-exchange", baseURL),
+			TokenURL: fmt.Sprintf("%s/api/v2/oauth/token-exchange", args.BaseURL),
 		},
-	}).Exchange(ctx, "", oauth2.SetAuthURLParam("client_id", clientID), oauth2.SetAuthURLParam("jwt", idToken))
+	}).Exchange(ctx, "", oauth2.SetAuthURLParam("client_id", args.ClientID), oauth2.SetAuthURLParam("jwt", args.IDToken))
 	if err != nil {
 		// Try to extract more detailed error message
 		if retrieveErr, ok := errors.AsType[*oauth2.RetrieveError](err); ok {

feature/identityfederation/identityfederation_test.go

@@ -9,6 +9,8 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+
+	"tailscale.com/internal/client/tailscale"
 )
 
 func TestResolveAuthKey(t *testing.T) {
@@ -70,7 +72,13 @@ func TestResolveAuthKey(t *testing.T) {
 			srv := mockedControlServer(t)
 			defer srv.Close()
 
-			authKey, err := resolveAuthKey(context.Background(), srv.URL, tt.clientID, tt.idToken, tt.audience, tt.tags)
+			authKey, err := resolveAuthKey(context.Background(), tailscale.ResolveAuthKeyWIFArgs{
+				BaseURL:  srv.URL,
+				ClientID: tt.clientID,
+				IDToken:  tt.idToken,
+				Audience: tt.audience,
+				Tags:     tt.tags,
+			})
 			if tt.wantErr != "" {
 				if err == nil {
 					t.Errorf("resolveAuthKey() error = nil, want %q", tt.wantErr)

feature/oauthkey/oauthkey.go

@@ -33,15 +33,15 @@ func init() {
 // false. The "baseURL" defaults to https://api.tailscale.com.
 // The passed in tags are required, and must be non-empty. These will be
 // set on the authkey generated by the OAuth2 dance.
-func resolveAuthKey(ctx context.Context, clientSecret string, tags []string) (string, error) {
+func resolveAuthKey(ctx context.Context, args tailscale.ResolveAuthKeyArgs) (string, error) {
-	if !strings.HasPrefix(clientSecret, "tskey-client-") {
+	if !strings.HasPrefix(args.AuthKey, "tskey-client-") {
-		return clientSecret, nil
+		return args.AuthKey, nil
 	}
-	if len(tags) == 0 {
+	if len(args.Tags) == 0 {
 		return "", errors.New("oauth authkeys require --advertise-tags")
 	}
 
-	strippedSecret, ephemeral, preauth, baseURL, err := parseOptionalAttributes(clientSecret)
+	strippedSecret, ephemeral, preauth, baseURL, err := parseOptionalAttributes(args.AuthKey)
 	if err != nil {
 		return "", err
 	}
@@ -63,7 +63,7 @@ func resolveAuthKey(ctx context.Context, clientSecret string, tags []string) (st
 				Reusable:      false,
 				Ephemeral:     ephemeral,
 				Preauthorized: preauth,
-				Tags:          tags,
+				Tags:          args.Tags,
 			},
 		},
 	}

feature/oauthkey/oauthkey_test.go

@@ -9,6 +9,8 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+
+	"tailscale.com/internal/client/tailscale"
 )
 
 func TestResolveAuthKey(t *testing.T) {
@@ -80,7 +82,10 @@ func TestResolveAuthKey(t *testing.T) {
 				}
 			}
 
-			got, err := resolveAuthKey(context.Background(), tt.clientID, tt.tags)
+			got, err := resolveAuthKey(context.Background(), tailscale.ResolveAuthKeyArgs{
+				AuthKey: tt.clientID,
+				Tags:    tt.tags,
+			})
 
 			if tt.wantErr {
 				if err == nil {

internal/client/tailscale/identityfederation.go

@@ -9,21 +9,32 @@ import (
 	"tailscale.com/feature"
 )
 
+type ResolveAuthKeyWIFArgs struct {
+	// BaseURL is the URL of the control server used for token exchange and authkey generation.
+	BaseURL string
+	// ClientID is the federated client ID used for token exchange.
+	ClientID string
+	// IDToken is the Identity token from the identity provider.
+	IDToken string
+	// Audience is the federated audience acquired by configuring the trust credential in the admin UI.
+	Audience string
+	// Tags is the list of tags to be associated with the auth key.
+	Tags []string
+}
+
+type ExchangeJWTForTokenWIFArgs struct {
+	// BaseURL is the URL of the control server used for token exchange.
+	BaseURL string
+	// ClientID is the federated client ID used for token exchange.
+	ClientID string
+	// IDToken is a JWT identity token to use in the token exchange operation.
+	IDToken string
+}
+
 // HookResolveAuthKeyViaWIF resolves to [identityfederation.resolveAuthKey] when the
 // corresponding feature tag is enabled in the build process.
-//
+var HookResolveAuthKeyViaWIF feature.Hook[func(ctx context.Context, args ResolveAuthKeyWIFArgs) (string, error)]
-// baseURL is the URL of the control server used for token exchange and authkey generation.
-// clientID is the federated client ID used for token exchange
-// idToken is the Identity token from the identity provider
-// tags is the list of tags to be associated with the auth key
-// audience is the federated audience acquired by configuring
-// the trusted credential in the admin UI
-var HookResolveAuthKeyViaWIF feature.Hook[func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error)]
 
 // HookExchangeJWTForTokenViaWIF resolves to [identityfederation.exchangeJWTForToken] when the
 // corresponding feature tag is enabled in the build process.
-//
+var HookExchangeJWTForTokenViaWIF feature.Hook[func(ctx context.Context, arg ExchangeJWTForTokenWIFArgs) (string, error)]
-// baseURL is the URL of the control server used for token exchange
-// clientID is the federated client ID used for token exchange
-// idToken is the Identity token from the identity provider
-var HookExchangeJWTForTokenViaWIF feature.Hook[func(ctx context.Context, baseURL, clientID, idToken string) (string, error)]

internal/client/tailscale/oauthkeys.go

@@ -9,12 +9,15 @@ import (
 	"tailscale.com/feature"
 )
 
+type ResolveAuthKeyArgs struct {
+	// Authkey is a standard device auth key or an OAuth client secret to resolve into an auth key.
+	AuthKey string
+	// Tags is the list of tags being advertised by the client (required to be provided for the
+	// OAuth secret case, and required to be the same as the list of tags for which the OAuth
+	// secret is allowed to issue auth keys).
+	Tags []string
+}
+
 // HookResolveAuthKey resolves to [oauthkey.ResolveAuthKey] when the
 // corresponding feature tag is enabled in the build process.
-//
+var HookResolveAuthKey feature.Hook[func(ctx context.Context, args ResolveAuthKeyArgs) (string, error)]
-// authKey is a standard device auth key or an OAuth client secret to
-// resolve into an auth key.
-// tags is the list of tags being advertised by the client (required to be
-// provided for the OAuth secret case, and required to be the same as the
-// list of tags for which the OAuth secret is allowed to issue auth keys).
-var HookResolveAuthKey feature.Hook[func(ctx context.Context, authKey string, tags []string) (string, error)]

tsnet/tsnet.go

@@ -1007,7 +1007,10 @@ func (s *Server) resolveAuthKey() (string, error) {
 		if authKey == "" {
 			clientSecret = s.getClientSecret()
 		}
-		authKey, err = resolveViaOAuth(s.shutdownCtx, clientSecret, s.AdvertiseTags)
+		authKey, err = resolveViaOAuth(s.shutdownCtx, tailscale.ResolveAuthKeyArgs{
+			AuthKey: clientSecret,
+			Tags:    s.AdvertiseTags,
+		})
 		if err != nil {
 			return "", err
 		}
@@ -1033,7 +1036,13 @@ func (s *Server) resolveAuthKey() (string, error) {
 				return "", fmt.Errorf("audience for workload identity federation found, but client ID is empty")
 			}
 		}
-		authKey, err = resolveViaWIF(s.shutdownCtx, s.getControlURL(), clientID, idToken, audience, s.AdvertiseTags)
+		authKey, err = resolveViaWIF(s.shutdownCtx, tailscale.ResolveAuthKeyWIFArgs{
+			BaseURL:  s.getControlURL(),
+			ClientID: clientID,
+			IDToken:  idToken,
+			Audience: audience,
+			Tags:     s.AdvertiseTags,
+		})
 		if err != nil {
 			return "", err
 		}

tsnet/tsnet_test.go

@@ -3450,8 +3450,8 @@ func TestResolveAuthKey(t *testing.T) {
 		audience        string
 		oauthAvailable  bool
 		wifAvailable    bool
-		resolveViaOAuth func(ctx context.Context, clientSecret string, tags []string) (string, error)
+		resolveViaOAuth func(ctx context.Context, args tailscale.ResolveAuthKeyArgs) (string, error)
-		resolveViaWIF   func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error)
+		resolveViaWIF   func(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error)
 		wantAuthKey     string
 		wantErr         bool
 		wantErrContains string
@@ -3460,9 +3460,9 @@ func TestResolveAuthKey(t *testing.T) {
 			name:           "success-oauth-client-secret",
 			clientSecret:   "tskey-client-secret-123",
 			oauthAvailable: true,
-			resolveViaOAuth: func(ctx context.Context, clientSecret string, tags []string) (string, error) {
+			resolveViaOAuth: func(ctx context.Context, args tailscale.ResolveAuthKeyArgs) (string, error) {
-				if clientSecret != "tskey-client-secret-123" {
+				if args.AuthKey != "tskey-client-secret-123" {
-					return "", fmt.Errorf("unexpected client secret: %s", clientSecret)
+					return "", fmt.Errorf("unexpected client secret: %s", args.AuthKey)
 				}
 				return "tskey-auth-via-oauth", nil
 			},
@@ -3473,7 +3473,7 @@ func TestResolveAuthKey(t *testing.T) {
 			name:           "fail-oauth-client-secret",
 			clientSecret:   "tskey-client-secret-123",
 			oauthAvailable: true,
-			resolveViaOAuth: func(ctx context.Context, clientSecret string, tags []string) (string, error) {
+			resolveViaOAuth: func(ctx context.Context, args tailscale.ResolveAuthKeyArgs) (string, error) {
 				return "", fmt.Errorf("resolution failed")
 			},
 			wantErrContains: "resolution failed",
@@ -3483,12 +3483,12 @@ func TestResolveAuthKey(t *testing.T) {
 			clientID:     "client-id-123",
 			idToken:      "id-token-456",
 			wifAvailable: true,
-			resolveViaWIF: func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error) {
+			resolveViaWIF: func(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error) {
-				if clientID != "client-id-123" {
+				if args.ClientID != "client-id-123" {
-					return "", fmt.Errorf("unexpected client ID: %s", clientID)
+					return "", fmt.Errorf("unexpected client ID: %s", args.ClientID)
 				}
-				if idToken != "id-token-456" {
+				if args.IDToken != "id-token-456" {
-					return "", fmt.Errorf("unexpected ID token: %s", idToken)
+					return "", fmt.Errorf("unexpected ID token: %s", args.IDToken)
 				}
 				return "tskey-auth-via-wif", nil
 			},
@@ -3500,12 +3500,12 @@ func TestResolveAuthKey(t *testing.T) {
 			clientID:     "client-id-123",
 			audience:     "api.tailscale.com",
 			wifAvailable: true,
-			resolveViaWIF: func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error) {
+			resolveViaWIF: func(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error) {
-				if clientID != "client-id-123" {
+				if args.ClientID != "client-id-123" {
-					return "", fmt.Errorf("unexpected client ID: %s", clientID)
+					return "", fmt.Errorf("unexpected client ID: %s", args.ClientID)
 				}
-				if audience != "api.tailscale.com" {
+				if args.Audience != "api.tailscale.com" {
-					return "", fmt.Errorf("unexpected ID token: %s", idToken)
+					return "", fmt.Errorf("unexpected audience: %s", args.Audience)
 				}
 				return "tskey-auth-via-wif", nil
 			},
@@ -3517,7 +3517,7 @@ func TestResolveAuthKey(t *testing.T) {
 			clientID:     "client-id-123",
 			idToken:      "id-token-456",
 			wifAvailable: true,
-			resolveViaWIF: func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error) {
+			resolveViaWIF: func(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error) {
 				return "", fmt.Errorf("resolution failed")
 			},
 			wantErrContains: "resolution failed",
@@ -3527,7 +3527,7 @@ func TestResolveAuthKey(t *testing.T) {
 			clientID:     "",
 			idToken:      "id-token-456",
 			wifAvailable: true,
-			resolveViaWIF: func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error) {
+			resolveViaWIF: func(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error) {
 				return "", fmt.Errorf("should not be called")
 			},
 			wantErrContains: "empty",
@@ -3537,7 +3537,7 @@ func TestResolveAuthKey(t *testing.T) {
 			clientID:     "",
 			audience:     "api.tailscale.com",
 			wifAvailable: true,
-			resolveViaWIF: func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error) {
+			resolveViaWIF: func(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error) {
 				return "", fmt.Errorf("should not be called")
 			},
 			wantErrContains: "empty",
@@ -3547,7 +3547,7 @@ func TestResolveAuthKey(t *testing.T) {
 			clientID:     "client-id-123",
 			idToken:      "",
 			wifAvailable: true,
-			resolveViaWIF: func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error) {
+			resolveViaWIF: func(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error) {
 				return "", fmt.Errorf("should not be called")
 			},
 			wantErrContains: "empty",
@@ -3558,7 +3558,7 @@ func TestResolveAuthKey(t *testing.T) {
 			idToken:      "id-token-456",
 			audience:     "api.tailscale.com",
 			wifAvailable: true,
-			resolveViaWIF: func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error) {
+			resolveViaWIF: func(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error) {
 				return "", fmt.Errorf("should not be called")
 			},
 			wantErrContains: "only one of ID token and audience",
@@ -3567,14 +3567,14 @@ func TestResolveAuthKey(t *testing.T) {
 			name:           "wif-skipped-oauth-succeeds",
 			clientSecret:   "tskey-client-secret-123",
 			oauthAvailable: true,
-			resolveViaOAuth: func(ctx context.Context, clientSecret string, tags []string) (string, error) {
+			resolveViaOAuth: func(ctx context.Context, args tailscale.ResolveAuthKeyArgs) (string, error) {
-				if clientSecret != "tskey-client-secret-123" {
+				if args.AuthKey != "tskey-client-secret-123" {
-					return "", fmt.Errorf("unexpected client secret: %s", clientSecret)
+					return "", fmt.Errorf("unexpected client secret: %s", args.AuthKey)
 				}
 				return "tskey-auth-via-oauth", nil
 			},
 			wifAvailable: true,
-			resolveViaWIF: func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error) {
+			resolveViaWIF: func(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error) {
 				return "", fmt.Errorf("should not be called")
 			},
 			wantAuthKey:     "tskey-auth-via-oauth",
@@ -3585,11 +3585,11 @@ func TestResolveAuthKey(t *testing.T) {
 			clientID:       "tskey-client-id-123",
 			idToken:        "",
 			oauthAvailable: true,
-			resolveViaOAuth: func(ctx context.Context, clientSecret string, tags []string) (string, error) {
+			resolveViaOAuth: func(ctx context.Context, args tailscale.ResolveAuthKeyArgs) (string, error) {
 				return "", fmt.Errorf("resolution failed")
 			},
 			wifAvailable: true,
-			resolveViaWIF: func(ctx context.Context, baseURL, clientID, idToken, audience string, tags []string) (string, error) {
+			resolveViaWIF: func(ctx context.Context, args tailscale.ResolveAuthKeyWIFArgs) (string, error) {
 				return "", fmt.Errorf("should not be called")
 			},
 			wantErrContains: "failed",
@@ -3613,9 +3613,9 @@ func TestResolveAuthKey(t *testing.T) {
 			name:           "authkey-client-secret-oauth-succeeds",
 			authKey:        "tskey-client-secret-123",
 			oauthAvailable: true,
-			resolveViaOAuth: func(ctx context.Context, clientSecret string, tags []string) (string, error) {
+			resolveViaOAuth: func(ctx context.Context, args tailscale.ResolveAuthKeyArgs) (string, error) {
-				if clientSecret != "tskey-client-secret-123" {
+				if args.AuthKey != "tskey-client-secret-123" {
-					return "", fmt.Errorf("unexpected client secret: %s", clientSecret)
+					return "", fmt.Errorf("unexpected client secret: %s", args.AuthKey)
 				}
 				return "tskey-auth-via-oauth", nil
 			},
@@ -3626,7 +3626,7 @@ func TestResolveAuthKey(t *testing.T) {
 			name:           "authkey-client-secret-oauth-fails",
 			authKey:        "tskey-client-secret-123",
 			oauthAvailable: true,
-			resolveViaOAuth: func(ctx context.Context, clientSecret string, tags []string) (string, error) {
+			resolveViaOAuth: func(ctx context.Context, args tailscale.ResolveAuthKeyArgs) (string, error) {
 				return "", fmt.Errorf("resolution failed")
 			},
 			wantErrContains: "resolution failed",