Add OAuth support for CLI extensions #886

stephanos · 2025-12-12T01:40:32Z

What was changed

Added OAuth support. This includes:

OAuth client in cliext for use by CLI extensions
Extending the environment config with OAuth properties for a profile
Integrating new OAuth properties into CLI get/set commands
Temporal Client configuration to use OAuth token

Why?

To make OAuth a first-class authentication mechanism in the Temporal CLI and CLI extensions.

Checklist

Closes
How was this tested:

Any docs updates needed?

stephanos · 2025-12-12T17:05:34Z

cliext/freeport.go

@@ -1,4 +1,4 @@
-package devserver
+package cliext


Moved here as it's helpful to open a random port for the OAuth Flow callback.

OAuth flow callback is a very specific use case for one very specific extension, I don't think we need to expose general purpose Go utilities here (this is a case where I think cloud should copy/inline this utility if this is how they want to do it)

Are you saying we shouldn't have any code sharing between cliext and Temporal CLI (ie Temporal CLI has no dep on cliext or any other shared package). Or is it only about helpers like this?

Sorry, I mean only helpers like this (free port). I recognize it's a gap in Go stdlib that extension authors have to write this, but IMO this extension library shouldn't expose general Go helpers this specific.

stephanos · 2025-12-12T17:06:40Z

internal/temporalcli/client.go

+	if clientProfile.OAuth != nil &&
+		clientProfile.APIKey == "" &&
+		clientProfile.TLS == nil && !clientProfile.TLS.Disabled {


This makes OAuth being tried last; which seems reasonable to me as it might open a browser if the access token is expired and we wouldn't want to do that unnecessarily if there's an API Key or TLS cert available already.

It should never open a browser IMO, that should only be part of cloud CLI when a specific command is invoked

Right; let me be more specific. The reasoning stated above isn't quite accurate: when obtaining a new access token we have some overhead in terms of a network. No browser should open in that scenario. But it's still a good idea to only do that when needed.

Then there is usability question around whether it should do a re-login when the refresh token has expired. It could automatically open the browser etc. as all the OAuth config details are available; but I can see how that might also be surprising behavior. The alternative is to show an error instead.

It should not do a re-login with login fails or is expired IMO. The only time a login should occur is on temporal cloud login and not in CLI. IMO CLI shouldn't even have the code to do the initial login.

stephanos · 2025-12-12T17:14:07Z

internal/temporalcli/commands_test.go

-	for k := range e {
-		ret = append(ret, k)
+	for k, v := range e {
+		ret = append(ret, k+"="+v)


Drive-by change; doesn't seem to affect any behavior either way.

stephanos · 2025-12-12T21:47:43Z

internal/temporalcli/commands.config.go

+	} else if strings.HasPrefix(c.Prop, "oauth.request_params.") {
+		key := strings.TrimPrefix(c.Prop, "oauth.request_params.")
+		if confProfile.OAuth == nil || confProfile.OAuth.RequestParams == nil {
+			return fmt.Errorf("property %q not found", c.Prop)
+		}
+		if _, ok := confProfile.OAuth.RequestParams[key]; !ok {
+			return fmt.Errorf("property %q not found", c.Prop)
+		}
+		delete(confProfile.OAuth.RequestParams, key)


I'm going with the assumption that any profile property should be treated equally; however, it seems unlikely that anyone would meddle with these OAuth params esp. the OAuth RequestParams.

stephanos · 2025-12-13T00:23:01Z

cliext/config.go

+	TokenType     string          `toml:"token_type,omitempty"`
+	ExpiresAt     string          `toml:"expires_at,omitempty"`
+	Scopes        []string        `toml:"scopes,omitempty"`
+	RequestParams inlineStringMap `toml:"request_params,omitempty"`


Following the lead from grpc_meta which is also inline.

cretz · 2025-12-15T13:35:03Z

cliext/config.go

+	Config ClientConfig
+	// ConfigFilePath is the resolved path to the configuration file that was loaded.
+	// This may differ from the input if TEMPORAL_CONFIG_FILE env var was used.
+	ConfigFilePath string


I think we can be a bit more specific in this extension library to start. Specifically, I think we could just have a LoadClientOAuth and StoreClientOAuth and leave the rest unexposed and let extensions use envconfig if they really need to for other things, though I can't understand why they would (it's not like this includes the CLI arguments a user can set on each call). We can add more generic load/store for "extra extension info" if we want later.

cretz · 2025-12-15T13:37:17Z

cliext/freeport.go

@@ -1,4 +1,4 @@
-package devserver
+package cliext


OAuth flow callback is a very specific use case for one very specific extension, I don't think we need to expose general purpose Go utilities here (this is a case where I think cloud should copy/inline this utility if this is how they want to do it)

cretz · 2025-12-15T13:39:06Z

cliext/go.mod

Note, there will probably need to be a documented release process for this (likely somewhere internal alongside CLI release process). Specifically it'll need to note that it must be tagged w/ a special string format (e.g. cliext/v1.0.0) and that we probably don't want to use traditional GH releases for it (because it clutters up the CLI release list) but may need somewhere to put release notes

cretz · 2025-12-15T13:40:59Z

cliext/oauth.go

+//
+// It starts a local HTTP server to receive the callback, generates an authorization URL,
+// and exchanges the authorization code for a token.
+func (c *OAuthClient) Login(ctx context.Context) (OAuthToken, error) {


I would not expect this to be part of cliext, it is only for cloud CLI right? I think the extension should be in charge of all of this code, it is not code that will ever be used by the CLI or any other extension I would assume nor should you want to maintain it over here IMO.

That's fair; if we say "no" to re-login (as mentioned in #886 (comment)) then this code can be moved to Cloud CLI.

👍 No re-login is my preference

cretz · 2025-12-15T13:42:26Z

internal/temporalcli/client.go

+	if clientProfile.OAuth != nil &&
+		clientProfile.APIKey == "" &&
+		clientProfile.TLS == nil && !clientProfile.TLS.Disabled {


It should never open a browser IMO, that should only be part of cloud CLI when a specific command is invoked

cretz · 2025-12-15T13:47:28Z

cliext/oauth.go

+}
+
+// Token returns a valid access token, refreshing if necessary.
+func (c *OAuthClient) Token(ctx context.Context, config *OAuthConfig) (OAuthToken, error) {


I don't think obtaining a token is part of cliext, that is only something needed by client option resolution. Rather the CLI extension library should provide "client connection information" which leverages everything from envconfig, args, etc and gives the CLI extension what it may need to connect (addr, namespace, API key (aka this token sometimes), TLS, etc). Maybe even a DialClient since many CLI extensions will likely do data plane connections.

To clarify, DialContext is not sth we already have right?

So the idea would be to provide a DialContext with all the info for a client to connect to data plane (or control plane in the Cloud CLI case), right?

We'll still need the behavior of Token, but you're saying rather than exposing it as-is, it should be an impl detail of of the DialContext?

And Temporal CLI could also leverage the same DialContext internally when it connects to Temporal (as it might also need to refresh the token).

Would you expect the DialContext to always include a usable/fresh access token; ie refresh eagerly?

I see you've mentioned ClientConfig in another comment. So I suppose that would be more generic than DialContext (which would be for Go SDK data plane client).

So the idea would be to provide a DialContext with all the info for a client to connect to data plane (or control plane in the Cloud CLI case), right?

It would actually only be data plane client for DialContext, but we need to provide all the information for dialing to the extension too so if they are dialing a different endpoint (e.g. cloud dialing cloud client), they have the args.

We'll still need the behavior of Token, but you're saying rather than exposing it as-is, it should be an impl detail of of the DialContext?

We can/should definitely provide a OAuth token source in the client config this cliext can provide (and use it ourselves in data plane DialContext), but we don't need a separate OAuthClient structure IMO.

Would you expect the DialContext to always include a usable/fresh access token; ie refresh eagerly?

Yes, I'd expect the traditional OAuth token source thing (gRPC has its credentials, but for data plane, should be able to leverage client.NewAPIKeyDynamicCredentials).

cretz · 2025-12-15T13:55:55Z

cliext/config.go

Overall, I'm thinking the cliext library would look like:

LoadClientOAuth and StoreClientOAuth (can accept profile name and config file path override if we want) - for very specific use case of adding or removing OAuth in config file

CommonConfig - struct with all common config fields

ApplyAsFlags method on this struct for CLI extensions to configure Cobra flags. Can be configurable which flags maybe

ClientConfig - struct with all client-based config fields

ApplyAsFlags method on it, though cloud CLI will likely customize the address flag default afterwards

Dial - can wait for future need, though I think we might as well do it now and we ourselves should leverage it

Could have helpers to load given a string array set of args, though we can just support Cobra flags for now I think

And then we should dogfood common config, and client config stuff in this CLI (unsure if it'll require adjusting code gen or what, but our custom envconfig wrapping/loading should remain private to the CLI at this time IMO)

stephanos · 2025-12-18T04:46:58Z

Closed in favor of #892 as there were too many changes.

stephanos force-pushed the oauth branch 3 times, most recently from f85673f to 5dc8817 Compare December 12, 2025 17:04

stephanos commented Dec 12, 2025

View reviewed changes

stephanos force-pushed the oauth branch 24 times, most recently from 1038fe8 to 55212d4 Compare December 12, 2025 19:55

stephanos force-pushed the oauth branch 7 times, most recently from b2e0b64 to e7b814c Compare December 12, 2025 21:45

stephanos commented Dec 12, 2025

View reviewed changes

stephanos force-pushed the oauth branch 9 times, most recently from 87d49c3 to 77f17ce Compare December 13, 2025 00:21

stephanos commented Dec 13, 2025

View reviewed changes

stephanos marked this pull request as ready for review December 13, 2025 00:23

stephanos requested review from a team as code owners December 13, 2025 00:23

stephanos force-pushed the oauth branch 3 times, most recently from d6a55b7 to f516179 Compare December 13, 2025 03:07

Add OAuth support for CLI extensions

42855c9

stephanos force-pushed the oauth branch from a5617ab to 42855c9 Compare December 13, 2025 03:18

cretz reviewed Dec 15, 2025

View reviewed changes

stephanos closed this Dec 18, 2025

stephanos mentioned this pull request Dec 18, 2025

Add OAuth support for CLI extensions #892

Open

Add OAuth support for CLI extensions #886

Add OAuth support for CLI extensions #886

Uh oh!

Conversation

stephanos commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was changed

Why?

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephanos Dec 15, 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!

cretz Dec 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!

cretz Dec 15, 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!

stephanos Dec 15, 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!

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!

Choose a reason for hiding this comment

Uh oh!

cretz Dec 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!

cretz Dec 15, 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!

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!

stephanos commented Dec 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

stephanos commented Dec 12, 2025 •

edited

Loading

stephanos Dec 15, 2025 •

edited

Loading

cretz Dec 16, 2025 •

edited

Loading

cretz Dec 15, 2025 •

edited

Loading

stephanos Dec 15, 2025 •

edited

Loading

cretz Dec 16, 2025 •

edited

Loading

cretz Dec 15, 2025 •

edited

Loading