From b5ac5c4cb893374cfcff89e309e4a43cf0d04e25 Mon Sep 17 00:00:00 2001 From: Anup Deka Date: Tue, 22 Sep 2026 23:18:59 +0530 Subject: [PATCH 01/10] Adding OAuth for doctl --- args.go | 11 + commands/auth.go | 29 +- commands/auth_client.go | 61 ++++ commands/auth_client_test.go | 59 ++++ commands/auth_login.go | 487 ++++++++++++++++++++++++++++ commands/auth_login_test.go | 420 ++++++++++++++++++++++++ commands/auth_test.go | 8 +- commands/command_config.go | 4 + commands/displayers/oauth_client.go | 90 +++++ internal/oauth/login.go | 261 +++++++++++++++ internal/oauth/login_test.go | 242 ++++++++++++++ internal/oauth/oauth.go | 402 +++++++++++++++++++++++ internal/oauth/oauth_test.go | 272 ++++++++++++++++ internal/oauth/pages.go | 82 +++++ 14 files changed, 2426 insertions(+), 2 deletions(-) create mode 100644 commands/auth_client.go create mode 100644 commands/auth_client_test.go create mode 100644 commands/auth_login.go create mode 100644 commands/auth_login_test.go create mode 100644 commands/displayers/oauth_client.go create mode 100644 internal/oauth/login.go create mode 100644 internal/oauth/login_test.go create mode 100644 internal/oauth/oauth.go create mode 100644 internal/oauth/oauth_test.go create mode 100644 internal/oauth/pages.go diff --git a/args.go b/args.go index 2430d04c1..8f244812a 100644 --- a/args.go +++ b/args.go @@ -745,6 +745,17 @@ const ( // ArgTokenValidationServer is the server used to validate an OAuth token ArgTokenValidationServer = "token-validation-server" + // ArgOAuthServer is the OAuth authorization server used to sign in. + ArgOAuthServer = "oauth-server" + // ArgOAuthScopes is the space-separated list of scopes requested when signing in. + ArgOAuthScopes = "scope" + // ArgOAuthCallbackPort is the local port that receives the OAuth redirect. + ArgOAuthCallbackPort = "callback-port" + // ArgOAuthNoBrowser prints the authorization URL instead of opening a browser. + ArgOAuthNoBrowser = "no-browser" + // ArgOAuthTimeout bounds how long to wait for browser authorization, in seconds. + ArgOAuthTimeout = "timeout" + // ArgGPUs specifies to list GPU Droplets ArgGPUs = "gpus" diff --git a/commands/auth.go b/commands/auth.go index 9cdc807c2..a9d2a88c9 100644 --- a/commands/auth.go +++ b/commands/auth.go @@ -26,6 +26,7 @@ import ( "github.com/digitalocean/doctl" "github.com/digitalocean/doctl/commands/charm/input" "github.com/digitalocean/doctl/commands/charm/template" + "github.com/digitalocean/doctl/internal/oauth" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -94,7 +95,9 @@ func Auth() *Command { Command: &cobra.Command{ Use: "auth", Short: "Display commands for authenticating doctl with an account", - Long: `The ` + "`" + `doctl auth` + "`" + ` commands allow you to authenticate doctl for use with your DigitalOcean account using tokens that you generate in the control panel at https://cloud.digitalocean.com/account/api/tokens. + Long: `The ` + "`" + `doctl auth` + "`" + ` commands allow you to authenticate doctl for use with your DigitalOcean account, either by signing in through your browser or with tokens that you generate in the control panel at https://cloud.digitalocean.com/account/api/tokens. + +The quickest way to get started is ` + "`" + `doctl auth login` + "`" + `, which authorizes doctl in your browser and keeps the resulting token up to date for you, so you don't have to generate a new one when it expires. If you work with a just one account, call ` + "`" + `doctl auth init` + "`" + ` and supply the token when prompted. This creates an authentication context named ` + "`" + `default` + "`" + `. @@ -117,6 +120,22 @@ You can use doctl without initializing it by adding the `+"`"+`--access-token`+" AddStringFlag(cmdAuthInit, doctl.ArgTokenValidationServer, "", TokenValidationServer, "The server used to validate a token") cmdAuthInit.Example = `The following example initializes doctl with a token for a single account with the context ` + "`" + `your-team` + "`" + `: doctl auth init --context your-team` + cmdAuthLogin := cmdBuilderWithInit(cmd, RunAuthLogin, "login", "Sign in to DigitalOcean in your browser", `This command signs doctl in to your DigitalOcean account using your browser, so you do not have to create and paste an API token. + +doctl uses the OAuth 2.1 authorization code flow with PKCE. The first time you run it, doctl registers itself with DigitalOcean as an application and saves the resulting client ID in its configuration file. Signing in then opens your browser, and once you approve the request, doctl saves the access token to your authentication context. + +Tokens issued this way are short-lived. doctl stores the accompanying refresh token and renews the access token automatically, so you can keep using doctl without returning to the control panel. + +By default doctl does not request any particular scopes, so you choose the permissions to grant on the authorization screen. Use `+"`"+`--scope`+"`"+` to request them up front instead: `+"`"+`--scope "read write"`+"`"+` asks for the same access a full-permission API token has, and you can also name individual permissions, such as `+"`"+`--scope "droplet:read account:read"`+"`"+`. + +The `+"`"+`--context`+"`"+` flag signs in to a named authentication context instead of the default one, which lets you keep several accounts or teams side by side. To use an API token instead of your browser, see the help for `+"`"+`doctl auth init`+"`"+`.`, Writer, false) + AddStringFlag(cmdAuthLogin, doctl.ArgOAuthServer, "", oauth.DefaultIssuer, "The OAuth authorization server to sign in to") + AddStringFlag(cmdAuthLogin, doctl.ArgOAuthScopes, "", defaultOAuthScopes, "A space-separated list of scopes to request, such as \"read write\" or \"droplet:read account:read\". When omitted, you choose the permissions to grant in your browser") + AddIntFlag(cmdAuthLogin, doctl.ArgOAuthCallbackPort, "", 0, "The local port to listen on for the authorization redirect. Defaults to an unused port") + AddBoolFlag(cmdAuthLogin, doctl.ArgOAuthNoBrowser, "", false, "Print the authorization URL instead of opening a browser") + AddDurationFlag(cmdAuthLogin, doctl.ArgOAuthTimeout, "", oauth.DefaultLoginTimeout, "How long to wait for you to authorize doctl in your browser") + cmdAuthLogin.Example = `The following example signs in to the context ` + "`" + `your-team` + "`" + ` and requests full read and write access: doctl auth login --context your-team --scope "read write"` + cmdAuthSwitch := cmdBuilderWithInit(cmd, RunAuthSwitch, "switch", "Switch between authentication contexts", `This command allows you to switch between authentication contexts you've already created. To see a list of available authentication contexts, call `+"`"+`doctl auth list`+"`"+`. @@ -151,6 +170,8 @@ To switch between the contexts use `+"`"+`doctl auth switch --context `+"` To create new contexts, see the help for `+"`"+`doctl auth init`+"`"+`.`, Writer, false, aliasOpt("t")) cmdAuthToken.Example = `The following example displays the token of the current context: doctl auth token` + cmd.AddCommand(authClient()) + return cmd } @@ -175,6 +196,10 @@ func RunAuthInit(retrieveUserTokenFunc func() (string, error)) func(c *CmdConfig } c.setContextAccessToken(token) + // An explicitly supplied token replaces whatever credential the + // context held, including a browser sign-in that would otherwise be + // refreshed over the top of it. + removeOAuthTokenState(context) template.Render(c.Out, `{{nl}}Validating token... `, nil) @@ -213,6 +238,8 @@ func RunAuthRemove(c *CmdConfig) error { return fmt.Errorf("Context not found") } + removeOAuthTokenState(context) + fmt.Println("Context deleted successfully") return writeConfig() diff --git a/commands/auth_client.go b/commands/auth_client.go new file mode 100644 index 000000000..22222d943 --- /dev/null +++ b/commands/auth_client.go @@ -0,0 +1,61 @@ +/* +Copyright 2018 The Doctl Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "errors" + + "github.com/spf13/cobra" + + "github.com/digitalocean/doctl/commands/displayers" +) + +// authClient creates the commands for inspecting the OAuth application doctl +// registered for itself. +func authClient() *Command { + cmd := &Command{ + Command: &cobra.Command{ + Use: "client", + Short: "Display the OAuth application doctl registered for this machine", + Long: `The subcommands of ` + "`" + `doctl auth client` + "`" + ` describe the OAuth application doctl registered with DigitalOcean the first time you ran ` + "`" + `doctl auth login` + "`" + `. + +doctl registers one application per authorization server and reuses it for every authentication context, so this is machine-wide rather than per account.`, + }, + } + + cmdClientShow := cmdBuilderWithInit(cmd, RunAuthClientShow, "show", "Display the registered OAuth application", `Display the OAuth application doctl registered for itself, including its client ID and the redirect URIs it accepts. + +The credential that manages the registration is never printed. + +If no application has been registered yet, run `+"`"+`doctl auth login`+"`"+` to create one.`, Writer, false, aliasOpt("get"), displayerType(&displayers.OAuthClient{})) + cmdClientShow.Example = `The following example displays the registered application as JSON: doctl auth client show --output json` + + return cmd +} + +// RunAuthClientShow displays the stored dynamic client registration. +func RunAuthClientShow(c *CmdConfig) error { + client := loadOAuthClientState() + if client == nil { + return errors.New("doctl has not registered an OAuth application yet. Run `doctl auth login` to register one") + } + + return c.Display(&displayers.OAuthClient{ + ClientID: client.ClientID, + Issuer: client.Issuer, + RedirectURIs: client.RedirectURIs, + RegisteredAt: client.RegisteredAt, + ManagementURL: client.RegistrationClientURI, + }) +} diff --git a/commands/auth_client_test.go b/commands/auth_client_test.go new file mode 100644 index 000000000..beaa1b682 --- /dev/null +++ b/commands/auth_client_test.go @@ -0,0 +1,59 @@ +/* +Copyright 2018 The Doctl Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "bytes" + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/digitalocean/doctl/internal/oauth" +) + +func TestRunAuthClientShow(t *testing.T) { + server := newTestAuthorizationServer(t, func() int { return 1 }) + defer server.Close() + + config, _ := newOAuthTestCmdConfig(t, server.URL) + out := &bytes.Buffer{} + config.Out = out + + stubOAuthLogin(t, func(context.Context, oauth.LoginOptions) (*oauth.Token, error) { + return &oauth.Token{AccessToken: "doo_v1_access"}, nil + }) + require.NoError(t, RunAuthLogin(config)) + + out.Reset() + require.NoError(t, RunAuthClientShow(config)) + + rendered := out.String() + assert.Contains(t, rendered, "client-1") + assert.Contains(t, rendered, server.URL) + assert.Contains(t, rendered, "http://127.0.0.1/callback") + assert.Contains(t, rendered, testClientIssuedAt.Format(time.RFC3339)) + assert.NotContains(t, rendered, "registration-token-1", "the credential that manages the registration is not display output") +} + +func TestRunAuthClientShowWithoutARegistration(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.digitalocean.com") + + err := RunAuthClientShow(config) + + require.Error(t, err) + assert.Contains(t, err.Error(), "doctl auth login") +} diff --git a/commands/auth_login.go b/commands/auth_login.go new file mode 100644 index 000000000..b0a8cb3d6 --- /dev/null +++ b/commands/auth_login.go @@ -0,0 +1,487 @@ +/* +Copyright 2018 The Doctl Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "context" + "errors" + "fmt" + "net/http" + "os" + "strings" + "time" + + "github.com/spf13/viper" + + "github.com/digitalocean/doctl" + "github.com/digitalocean/doctl/commands/charm/template" + "github.com/digitalocean/doctl/internal/oauth" +) + +const ( + // oauthClientConfigKey holds the dynamic client registration doctl shares + // across every authentication context on this machine. + oauthClientConfigKey = "oauth-client" + // oauthTokensConfigKey holds the per-context OAuth session state used to + // refresh access tokens. + oauthTokensConfigKey = "oauth-tokens" + + // oauthClientName and oauthClientURI identify doctl on the authorization + // screen the user is shown. + oauthClientName = "doctl" + oauthClientURI = "https://github.com/digitalocean/doctl" + + // defaultOAuthScopes is empty on purpose: when doctl sends no scope, the + // authorization screen decides what the token is granted, which lets the + // user pick the permissions they want rather than the CLI assuming them. + defaultOAuthScopes = "" + + // oauthRefreshSkew refreshes an access token slightly before it expires so + // a command that takes a moment to reach the API is not rejected. + oauthRefreshSkew = 2 * time.Minute + + // oauthHTTPTimeout bounds each individual call to the authorization + // server. It does not bound the wait for the user's browser. + oauthHTTPTimeout = 30 * time.Second +) + +// oauthLogin runs the browser-based authorization code flow. It is a variable +// so tests can exercise the command without a browser. +var oauthLogin = oauth.Login + +// oauthClientState is the dynamic client registration persisted in the config +// file. doctl registers itself once per authorization server and reuses the +// resulting public client for every login. +type oauthClientState struct { + Issuer string + ClientID string + RedirectURIs []string + RegisteredAt time.Time + RegistrationAccessToken string + RegistrationClientURI string +} + +// oauthTokenState is the per-context OAuth session persisted in the config +// file. The access token itself lives with the rest of the context's +// credentials; only what is needed to renew it is kept here. +type oauthTokenState struct { + Issuer string + ClientID string + TokenEndpoint string + RefreshToken string + Scope string + ExpiresAt time.Time +} + +func (s *oauthTokenState) needsRefresh(now time.Time) bool { + if s == nil || s.RefreshToken == "" { + return false + } + // An unknown expiry means the token was issued by a server that does not + // report one; leave it alone and let the API reject it if it is stale. + if s.ExpiresAt.IsZero() { + return false + } + + return !now.Add(oauthRefreshSkew).Before(s.ExpiresAt) +} + +// RunAuthLogin authenticates doctl using the OAuth 2.1 authorization code flow +// with PKCE, storing the resulting access token in the current context. +func RunAuthLogin(c *CmdConfig) error { + authContext := currentAuthContext() + + issuer, err := c.Doit.GetString(c.NS, doctl.ArgOAuthServer) + if err != nil { + return err + } + scopes, err := c.Doit.GetString(c.NS, doctl.ArgOAuthScopes) + if err != nil { + return err + } + port, err := c.Doit.GetInt(c.NS, doctl.ArgOAuthCallbackPort) + if err != nil { + return err + } + noBrowser, err := c.Doit.GetBool(c.NS, doctl.ArgOAuthNoBrowser) + if err != nil { + return err + } + timeout, err := c.Doit.GetDuration(c.NS, doctl.ArgOAuthTimeout) + if err != nil { + return err + } + + ctx := context.Background() + httpClient := &http.Client{Timeout: oauthHTTPTimeout} + metadata := oauth.ResolveServerMetadata(ctx, httpClient, issuer) + + client, registered, err := ensureOAuthClient(ctx, c, httpClient, metadata) + if err != nil { + return err + } + + opts := oauth.LoginOptions{ + Metadata: metadata, + ClientID: client.ClientID, + Scopes: strings.Fields(scopes), + Port: port, + Timeout: timeout, + HTTPClient: httpClient, + NoBrowser: noBrowser, + OnAuthorizationURL: authorizationURLPrinter(c, noBrowser), + } + + token, err := oauthLogin(ctx, opts) + if errors.Is(err, oauth.ErrInvalidClient) && !registered { + // The stored registration is no longer valid, which happens when it + // was deleted server side. Register again and make one more attempt. + template.Render(c.Out, `{{nl}}{{warning "The saved OAuth application is no longer valid."}}{{nl}}`, nil) + + client, err = registerOAuthClient(ctx, c, httpClient, metadata) + if err != nil { + return err + } + + opts.ClientID = client.ClientID + token, err = oauthLogin(ctx, opts) + } + if err != nil { + template.Render(c.Out, `{{error crossmark}}{{nl}}{{nl}}`, nil) + return fmt.Errorf("Unable to authenticate with DigitalOcean: %s", err) + } + + template.Render(c.Out, `{{success checkmark}}{{nl}}{{nl}}`, nil) + + c.setContextAccessToken(token.AccessToken) + if token.RefreshToken == "" { + // Without a refresh token there is nothing to renew later, so make + // sure an earlier session is not left behind to be refreshed over the + // top of this one. + removeOAuthTokenState(authContext) + } else { + storeOAuthTokenState(authContext, &oauthTokenState{ + Issuer: metadata.Issuer, + ClientID: client.ClientID, + TokenEndpoint: metadata.TokenEndpoint, + RefreshToken: token.RefreshToken, + Scope: token.Scope, + ExpiresAt: token.Expiry, + }) + } + + if err := writeConfig(); err != nil { + return err + } + + displayOAuthLoginSummary(c, authContext, token) + + return nil +} + +// ensureOAuthClient returns the dynamic client registration for the +// authorization server, registering doctl if this machine does not have one +// yet. The second return value reports whether a new client was registered. +func ensureOAuthClient(ctx context.Context, c *CmdConfig, httpClient *http.Client, metadata *oauth.ServerMetadata) (*oauthClientState, bool, error) { + if existing := loadOAuthClientState(); existing != nil && existing.Issuer == metadata.Issuer { + return existing, false, nil + } + + client, err := registerOAuthClient(ctx, c, httpClient, metadata) + if err != nil { + return nil, false, err + } + + return client, true, nil +} + +func registerOAuthClient(ctx context.Context, c *CmdConfig, httpClient *http.Client, metadata *oauth.ServerMetadata) (*oauthClientState, error) { + template.Render(c.Out, `Registering doctl with {{highlight .}}... `, metadata.Issuer) + + registration, err := oauth.RegisterClient( + ctx, httpClient, metadata.RegistrationEndpoint, + oauth.RegistrationRedirectURIs(), oauthClientName, oauthClientURI, + ) + if err != nil { + template.Render(c.Out, `{{error crossmark}}{{nl}}{{nl}}`, nil) + return nil, err + } + + template.Render(c.Out, `{{success checkmark}}{{nl}}`, nil) + + client := &oauthClientState{ + Issuer: metadata.Issuer, + ClientID: registration.ClientID, + RedirectURIs: registration.RedirectURIs, + RegistrationAccessToken: registration.RegistrationAccessToken, + RegistrationClientURI: registration.RegistrationClientURI, + } + if registration.ClientIDIssuedAt > 0 { + client.RegisteredAt = time.Unix(registration.ClientIDIssuedAt, 0).UTC() + } + storeOAuthClientState(client) + + // Persist the registration immediately so an interrupted login does not + // leave an orphaned application behind on the next attempt. + if err := writeConfig(); err != nil { + return nil, err + } + + return client, nil +} + +func authorizationURLPrinter(c *CmdConfig, noBrowser bool) func(string) { + return func(authURL string) { + if noBrowser { + template.Render(c.Out, `{{nl}}Visit the following URL to authorize doctl:{{nl}}{{nl}} {{underline .}}{{nl}}{{nl}}Waiting for authorization... `, authURL) + return + } + + template.Render(c.Out, + `{{nl}}Opening your browser to authorize doctl. If it does not open, visit:{{nl}}{{nl}} {{underline .}}{{nl}}{{nl}}Waiting for authorization... `, + authURL, + ) + } +} + +func displayOAuthLoginSummary(c *CmdConfig, authContext string, token *oauth.Token) { + who := token.Info.Email + if who == "" { + who = token.Info.Name + } + if who != "" { + template.Render(c.Out, `Signed in as {{highlight .}}{{nl}}`, who) + } + if token.Info.TeamName != "" { + template.Render(c.Out, `Team: {{highlight .}}{{nl}}`, token.Info.TeamName) + } + + template.Render(c.Out, `Saved to the {{highlight .}} authentication context.{{nl}}`, authContext) + + if token.RefreshToken != "" { + template.Render(c.Out, `{{nl}}{{muted "doctl renews this token automatically; run doctl auth login again if the session is revoked."}}{{nl}}`, nil) + } +} + +// refreshExpiredOAuthToken renews the current context's access token when it +// was obtained with doctl auth login and has expired. It is a no-op for +// contexts authenticated with a personal access token. +func refreshExpiredOAuthToken(c *CmdConfig) error { + if accessTokenOverridden() { + return nil + } + + authContext := currentAuthContext() + state := loadOAuthTokenState(authContext) + if !state.needsRefresh(time.Now()) { + return nil + } + + endpoint := state.TokenEndpoint + if endpoint == "" { + endpoint = oauth.ResolveServerMetadata(context.Background(), nil, state.Issuer).TokenEndpoint + } + + httpClient := &http.Client{Timeout: oauthHTTPTimeout} + token, err := oauth.RefreshToken(context.Background(), httpClient, endpoint, state.ClientID, state.RefreshToken) + if err != nil { + return fmt.Errorf("Your DigitalOcean session has expired and could not be renewed: %s\n\nRun `doctl auth login` to sign in again.", err) + } + + c.setContextAccessToken(token.AccessToken) + state.RefreshToken = token.RefreshToken + state.ExpiresAt = token.Expiry + if token.Scope != "" { + state.Scope = token.Scope + } + storeOAuthTokenState(authContext, state) + + return writeConfig() +} + +// accessTokenOverridden reports whether the user supplied a token for this +// invocation, in which case it takes precedence over any stored OAuth session. +func accessTokenOverridden() bool { + return Token != "" || os.Getenv("DIGITALOCEAN_ACCESS_TOKEN") != "" +} + +// currentAuthContext returns the name of the authentication context the +// command is running against. +func currentAuthContext() string { + authContext := strings.ToLower(Context) + if authContext == "" { + authContext = strings.ToLower(viper.GetString(doctl.ArgContext)) + } + if authContext == "" { + authContext = doctl.ArgDefaultContext + } + + return authContext +} + +func loadOAuthClientState() *oauthClientState { + values := viper.GetStringMap(oauthClientConfigKey) + if len(values) == 0 { + return nil + } + + client := &oauthClientState{ + Issuer: configMapString(values, "issuer"), + ClientID: configMapString(values, "client-id"), + RedirectURIs: configMapStringSlice(values, "redirect-uris"), + RegistrationAccessToken: configMapString(values, "registration-access-token"), + RegistrationClientURI: configMapString(values, "registration-client-uri"), + } + if registeredAt := configMapString(values, "registered-at"); registeredAt != "" { + if parsed, err := time.Parse(time.RFC3339, registeredAt); err == nil { + client.RegisteredAt = parsed + } + } + if client.ClientID == "" || client.Issuer == "" { + return nil + } + + return client +} + +func storeOAuthClientState(client *oauthClientState) { + var registeredAt string + if !client.RegisteredAt.IsZero() { + registeredAt = client.RegisteredAt.UTC().Format(time.RFC3339) + } + + viper.Set(oauthClientConfigKey, map[string]any{ + "issuer": client.Issuer, + "client-id": client.ClientID, + "redirect-uris": client.RedirectURIs, + "registered-at": registeredAt, + "registration-access-token": client.RegistrationAccessToken, + "registration-client-uri": client.RegistrationClientURI, + }) +} + +func loadOAuthTokenState(authContext string) *oauthTokenState { + values, ok := oauthTokenStates()[strings.ToLower(authContext)].(map[string]any) + if !ok || len(values) == 0 { + return nil + } + + state := &oauthTokenState{ + Issuer: configMapString(values, "issuer"), + ClientID: configMapString(values, "client-id"), + TokenEndpoint: configMapString(values, "token-endpoint"), + RefreshToken: configMapString(values, "refresh-token"), + Scope: configMapString(values, "scope"), + } + if expiresAt := configMapString(values, "expires-at"); expiresAt != "" { + if parsed, err := time.Parse(time.RFC3339, expiresAt); err == nil { + state.ExpiresAt = parsed + } + } + if state.RefreshToken == "" { + return nil + } + + return state +} + +func storeOAuthTokenState(authContext string, state *oauthTokenState) { + var expiresAt string + if !state.ExpiresAt.IsZero() { + expiresAt = state.ExpiresAt.UTC().Format(time.RFC3339) + } + + states := oauthTokenStates() + states[strings.ToLower(authContext)] = map[string]any{ + "issuer": state.Issuer, + "client-id": state.ClientID, + "token-endpoint": state.TokenEndpoint, + "refresh-token": state.RefreshToken, + "scope": state.Scope, + "expires-at": expiresAt, + } + + viper.Set(oauthTokensConfigKey, states) +} + +// removeOAuthTokenState drops the stored OAuth session for a context. It is +// called whenever the context's credentials are replaced or removed so a stale +// refresh token cannot overwrite them later. +func removeOAuthTokenState(authContext string) { + states := oauthTokenStates() + if len(states) == 0 { + return + } + + delete(states, strings.ToLower(authContext)) + viper.Set(oauthTokensConfigKey, states) +} + +// oauthTokenStates returns a mutable copy of the stored per-context sessions, +// normalizing the values viper hands back from YAML. +func oauthTokenStates() map[string]any { + states := map[string]any{} + for name, value := range viper.GetStringMap(oauthTokensConfigKey) { + switch typed := value.(type) { + case map[string]any: + states[name] = typed + case map[string]string: + converted := make(map[string]any, len(typed)) + for k, v := range typed { + converted[k] = v + } + states[name] = converted + case map[any]any: + converted := make(map[string]any, len(typed)) + for k, v := range typed { + converted[fmt.Sprintf("%v", k)] = v + } + states[name] = converted + } + } + + return states +} + +// configMapStringSlice reads a list of strings, which viper hands back as +// []any when it comes from the YAML file and as []string when it was set in +// this process. +func configMapStringSlice(values map[string]any, key string) []string { + switch typed := values[key].(type) { + case []string: + return typed + case []any: + items := make([]string, 0, len(typed)) + for _, item := range typed { + if s, ok := item.(string); ok { + items = append(items, s) + } + } + return items + default: + return nil + } +} + +func configMapString(values map[string]any, key string) string { + value, ok := values[key] + if !ok { + return "" + } + if s, ok := value.(string); ok { + return s + } + + return "" +} diff --git a/commands/auth_login_test.go b/commands/auth_login_test.go new file mode 100644 index 000000000..764ba913f --- /dev/null +++ b/commands/auth_login_test.go @@ -0,0 +1,420 @@ +/* +Copyright 2018 The Doctl Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package commands + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strconv" + "testing" + "time" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/digitalocean/doctl" + "github.com/digitalocean/doctl/do" + "github.com/digitalocean/doctl/internal/oauth" +) + +func TestRunAuthLogin(t *testing.T) { + var registrations int + server := newTestAuthorizationServer(t, func() int { registrations++; return registrations }) + defer server.Close() + + config, token := newOAuthTestCmdConfig(t, server.URL) + + var capturedOpts oauth.LoginOptions + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + capturedOpts = opts + return &oauth.Token{ + AccessToken: "doo_v1_access", + RefreshToken: "dor_v1_refresh", + Scope: "api:read api:write", + Expiry: time.Now().Add(time.Hour), + Info: oauth.TokenInfo{Email: "sammy@example.com", TeamName: "My Team"}, + }, nil + }) + + require.NoError(t, RunAuthLogin(config)) + + assert.Equal(t, 1, registrations) + assert.Equal(t, "doo_v1_access", *token) + + assert.Equal(t, "client-1", capturedOpts.ClientID) + assert.Empty(t, capturedOpts.Scopes, "without --scope the authorization screen decides the permissions") + assert.Equal(t, server.URL+"/v1/oauth/token", capturedOpts.Metadata.TokenEndpoint) + + client := loadOAuthClientState() + require.NotNil(t, client) + assert.Equal(t, "client-1", client.ClientID) + assert.Equal(t, server.URL, client.Issuer) + assert.Equal(t, "registration-token-1", client.RegistrationAccessToken) + assert.Equal(t, oauth.RegistrationRedirectURIs(), client.RedirectURIs) + assert.True(t, testClientIssuedAt.Equal(client.RegisteredAt)) + + state := loadOAuthTokenState(doctl.ArgDefaultContext) + require.NotNil(t, state) + assert.Equal(t, "dor_v1_refresh", state.RefreshToken) + assert.Equal(t, "client-1", state.ClientID) + assert.Equal(t, server.URL+"/v1/oauth/token", state.TokenEndpoint) + assert.False(t, state.ExpiresAt.IsZero()) +} + +func TestRunAuthLoginRequestsTheGivenScopes(t *testing.T) { + server := newTestAuthorizationServer(t, func() int { return 1 }) + defer server.Close() + + config, _ := newOAuthTestCmdConfig(t, server.URL) + config.Doit.Set(config.NS, doctl.ArgOAuthScopes, "read write") + + var capturedOpts oauth.LoginOptions + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + capturedOpts = opts + return &oauth.Token{AccessToken: "doo_v1_access"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + + assert.Equal(t, []string{"read", "write"}, capturedOpts.Scopes) +} + +func TestRunAuthLoginReusesTheRegisteredClient(t *testing.T) { + var registrations int + server := newTestAuthorizationServer(t, func() int { registrations++; return registrations }) + defer server.Close() + + config, _ := newOAuthTestCmdConfig(t, server.URL) + storeOAuthClientState(&oauthClientState{Issuer: server.URL, ClientID: "already-registered"}) + + var clientID string + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + clientID = opts.ClientID + return &oauth.Token{AccessToken: "doo_v1_access"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + + assert.Zero(t, registrations, "an existing registration should be reused") + assert.Equal(t, "already-registered", clientID) +} + +func TestRunAuthLoginRegistersAgainWhenTheClientIsRejected(t *testing.T) { + var registrations int + server := newTestAuthorizationServer(t, func() int { registrations++; return registrations }) + defer server.Close() + + config, token := newOAuthTestCmdConfig(t, server.URL) + storeOAuthClientState(&oauthClientState{Issuer: server.URL, ClientID: "deleted-client"}) + + var attempts []string + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + attempts = append(attempts, opts.ClientID) + if opts.ClientID == "deleted-client" { + return nil, &oauth.AuthorizationError{Code: "invalid_client"} + } + return &oauth.Token{AccessToken: "doo_v1_access"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + + assert.Equal(t, []string{"deleted-client", "client-1"}, attempts) + assert.Equal(t, 1, registrations) + assert.Equal(t, "doo_v1_access", *token) +} + +func TestRunAuthLoginFailure(t *testing.T) { + server := newTestAuthorizationServer(t, func() int { return 1 }) + defer server.Close() + + config, token := newOAuthTestCmdConfig(t, server.URL) + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + return nil, &oauth.AuthorizationError{Code: "access_denied"} + }) + + err := RunAuthLogin(config) + + require.Error(t, err) + assert.Contains(t, err.Error(), "access_denied") + assert.Empty(t, *token, "a failed login must not store a token") + assert.Nil(t, loadOAuthTokenState(doctl.ArgDefaultContext)) +} + +func TestRefreshExpiredOAuthToken(t *testing.T) { + tokenEndpoint := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + assert.Equal(t, "refresh_token", r.FormValue("grant_type")) + assert.Equal(t, "dor_v1_old", r.FormValue("refresh_token")) + + w.Header().Set("Content-Type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{ + "access_token": "doo_v1_new", + "refresh_token": "dor_v1_new", + "expires_in": 3600, + })) + })) + defer tokenEndpoint.Close() + + t.Run("renews an expired token", func(t *testing.T) { + config, token := newOAuthTestCmdConfig(t, tokenEndpoint.URL) + storeOAuthTokenState(doctl.ArgDefaultContext, &oauthTokenState{ + Issuer: tokenEndpoint.URL, + ClientID: "client-1", + TokenEndpoint: tokenEndpoint.URL, + RefreshToken: "dor_v1_old", + ExpiresAt: time.Now().Add(-time.Minute), + }) + + require.NoError(t, refreshExpiredOAuthToken(config)) + + assert.Equal(t, "doo_v1_new", *token) + + state := loadOAuthTokenState(doctl.ArgDefaultContext) + require.NotNil(t, state) + assert.Equal(t, "dor_v1_new", state.RefreshToken) + assert.True(t, state.ExpiresAt.After(time.Now())) + }) + + t.Run("leaves a valid token alone", func(t *testing.T) { + config, token := newOAuthTestCmdConfig(t, tokenEndpoint.URL) + storeOAuthTokenState(doctl.ArgDefaultContext, &oauthTokenState{ + TokenEndpoint: tokenEndpoint.URL, + RefreshToken: "dor_v1_old", + ExpiresAt: time.Now().Add(time.Hour), + }) + + require.NoError(t, refreshExpiredOAuthToken(config)) + + assert.Empty(t, *token) + }) + + t.Run("ignores contexts without an OAuth session", func(t *testing.T) { + config, token := newOAuthTestCmdConfig(t, tokenEndpoint.URL) + + require.NoError(t, refreshExpiredOAuthToken(config)) + + assert.Empty(t, *token) + }) + + t.Run("ignores a token supplied for this command", func(t *testing.T) { + config, token := newOAuthTestCmdConfig(t, tokenEndpoint.URL) + storeOAuthTokenState(doctl.ArgDefaultContext, &oauthTokenState{ + TokenEndpoint: tokenEndpoint.URL, + RefreshToken: "dor_v1_old", + ExpiresAt: time.Now().Add(-time.Minute), + }) + + Token = "dop_v1_explicit" + t.Cleanup(func() { Token = "" }) + + require.NoError(t, refreshExpiredOAuthToken(config)) + + assert.Empty(t, *token) + }) + + t.Run("explains how to recover when the refresh token is rejected", func(t *testing.T) { + rejecting := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{"error": "invalid_grant"})) + })) + defer rejecting.Close() + + config, _ := newOAuthTestCmdConfig(t, rejecting.URL) + storeOAuthTokenState(doctl.ArgDefaultContext, &oauthTokenState{ + TokenEndpoint: rejecting.URL, + RefreshToken: "dor_v1_old", + ExpiresAt: time.Now().Add(-time.Minute), + }) + + err := refreshExpiredOAuthToken(config) + + require.Error(t, err) + assert.Contains(t, err.Error(), "doctl auth login") + }) +} + +func TestAuthInitDiscardsTheOAuthSession(t *testing.T) { + resetOAuthConfig(t) + storeOAuthTokenState(doctl.ArgDefaultContext, &oauthTokenState{ + RefreshToken: "dor_v1_refresh", + ExpiresAt: time.Now().Add(-time.Minute), + }) + + previousToken := viper.Get(doctl.ArgAccessToken) + viper.Set(doctl.ArgAccessToken, "dop_v1_pat") + t.Cleanup(func() { viper.Set(doctl.ArgAccessToken, previousToken) }) + + withTestClient(t, func(config *CmdConfig, tm *tcMocks) { + tm.oauth.EXPECT().TokenInfo(gomock.Any()).Return(&do.OAuthTokenInfo{}, nil) + + err := RunAuthInit(func() (string, error) { return "", nil })(config) + require.NoError(t, err) + }) + + assert.Nil(t, loadOAuthTokenState(doctl.ArgDefaultContext)) +} + +func TestOAuthTokenStateIsScopedToAContext(t *testing.T) { + resetOAuthConfig(t) + + storeOAuthTokenState("default", &oauthTokenState{RefreshToken: "default-refresh", Scope: "read"}) + storeOAuthTokenState("Your-Team", &oauthTokenState{RefreshToken: "team-refresh"}) + + assert.Equal(t, "default-refresh", loadOAuthTokenState("default").RefreshToken) + assert.Equal(t, "team-refresh", loadOAuthTokenState("your-team").RefreshToken, "context names are case insensitive") + assert.Nil(t, loadOAuthTokenState("unknown")) + + removeOAuthTokenState("default") + + assert.Nil(t, loadOAuthTokenState("default")) + assert.NotNil(t, loadOAuthTokenState("your-team")) +} + +func TestOAuthTokenStateNeedsRefresh(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + state *oauthTokenState + expected bool + }{ + {name: "no session"}, + {name: "no refresh token", state: &oauthTokenState{ExpiresAt: now.Add(-time.Hour)}}, + {name: "unknown expiry", state: &oauthTokenState{RefreshToken: "refresh"}}, + {name: "valid", state: &oauthTokenState{RefreshToken: "refresh", ExpiresAt: now.Add(time.Hour)}, expected: false}, + {name: "expired", state: &oauthTokenState{RefreshToken: "refresh", ExpiresAt: now.Add(-time.Second)}, expected: true}, + {name: "expiring within the skew", state: &oauthTokenState{RefreshToken: "refresh", ExpiresAt: now.Add(time.Minute)}, expected: true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.expected, test.state.needsRefresh(now)) + }) + } +} + +// testClientIssuedAt is the registration timestamp the test authorization +// server reports. +var testClientIssuedAt = time.Date(2026, time.September, 22, 10, 30, 0, 0, time.UTC) + +// newTestAuthorizationServer serves the metadata and dynamic client +// registration endpoints doctl calls before starting the browser flow. +func newTestAuthorizationServer(t *testing.T, nextClient func() int) *httptest.Server { + t.Helper() + + server := httptest.NewUnstartedServer(nil) + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, r *http.Request) { + issuer := "http://" + r.Host + writeTestJSON(t, w, http.StatusOK, map[string]any{ + "issuer": issuer, + "authorization_endpoint": issuer + "/v1/oauth/authorize", + "token_endpoint": issuer + "/v1/oauth/token", + "registration_endpoint": issuer + "/v1/oauth/register", + "revocation_endpoint": issuer + "/v1/oauth/revoke", + }) + }) + mux.HandleFunc("/v1/oauth/register", func(w http.ResponseWriter, r *http.Request) { + var request struct { + RedirectURIs []string `json:"redirect_uris"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + + id := strconv.Itoa(nextClient()) + writeTestJSON(t, w, http.StatusCreated, map[string]any{ + "client_id": "client-" + id, + "client_id_issued_at": testClientIssuedAt.Unix(), + "redirect_uris": request.RedirectURIs, + "registration_access_token": "registration-token-" + id, + }) + }) + server.Config.Handler = mux + server.Start() + + return server +} + +func writeTestJSON(t *testing.T, w http.ResponseWriter, status int, body any) { + t.Helper() + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + require.NoError(t, json.NewEncoder(w).Encode(body)) +} + +// newOAuthTestCmdConfig returns a command config wired to an isolated view of +// the doctl configuration, along with a pointer to the access token the +// command stores for the current context. +func newOAuthTestCmdConfig(t *testing.T, issuer string) (*CmdConfig, *string) { + t.Helper() + + resetOAuthConfig(t) + + doitConfig := doctl.NewTestConfig() + var storedToken string + + config := &CmdConfig{ + NS: "test", + Doit: doitConfig, + Out: io.Discard, + setContextAccessToken: func(token string) { storedToken = token }, + getContextAccessToken: func() string { return storedToken }, + initServices: func(*CmdConfig) error { return nil }, + } + + doitConfig.Set(config.NS, doctl.ArgOAuthServer, issuer) + doitConfig.Set(config.NS, doctl.ArgOAuthTimeout, time.Minute) + + return config, &storedToken +} + +// resetOAuthConfig isolates a test from the global viper configuration the +// commands package writes to. +func resetOAuthConfig(t *testing.T) { + t.Helper() + + previousWriter := cfgFileWriter + previousContext := Context + previousClient := viper.Get(oauthClientConfigKey) + previousTokens := viper.Get(oauthTokensConfigKey) + previousViperContext := viper.Get(doctl.ArgContext) + + cfgFileWriter = func() (io.WriteCloser, error) { return &nopWriteCloser{Writer: io.Discard}, nil } + Context = "" + viper.Set(doctl.ArgContext, doctl.ArgDefaultContext) + viper.Set(oauthClientConfigKey, nil) + viper.Set(oauthTokensConfigKey, nil) + + t.Cleanup(func() { + cfgFileWriter = previousWriter + Context = previousContext + viper.Set(oauthClientConfigKey, previousClient) + viper.Set(oauthTokensConfigKey, previousTokens) + viper.Set(doctl.ArgContext, previousViperContext) + }) +} + +func stubOAuthLogin(t *testing.T, login func(context.Context, oauth.LoginOptions) (*oauth.Token, error)) { + t.Helper() + + previous := oauthLogin + oauthLogin = login + t.Cleanup(func() { oauthLogin = previous }) +} diff --git a/commands/auth_test.go b/commands/auth_test.go index 4a30f014a..69ebf86f2 100644 --- a/commands/auth_test.go +++ b/commands/auth_test.go @@ -32,7 +32,13 @@ import ( func TestAuthCommand(t *testing.T) { cmd := Auth() assert.NotNil(t, cmd) - assertCommandNames(t, cmd, "init", "list", "remove", "switch", "token") + assertCommandNames(t, cmd, "client", "init", "list", "login", "remove", "switch", "token") +} + +func TestAuthClientCommand(t *testing.T) { + cmd := authClient() + assert.NotNil(t, cmd) + assertCommandNames(t, cmd, "show") } func TestAuthInit(t *testing.T) { diff --git a/commands/command_config.go b/commands/command_config.go index c0cb076b3..5cd371a3e 100644 --- a/commands/command_config.go +++ b/commands/command_config.go @@ -106,6 +106,10 @@ func NewCmdConfig(ns string, dc doctl.Config, out io.Writer, args []string, init Args: args, initServices: func(c *CmdConfig) error { + if err := refreshExpiredOAuthToken(c); err != nil { + return err + } + accessToken := c.getContextAccessToken() godoClient, err := c.Doit.GetGodoClient(Trace, true, accessToken) if err != nil { diff --git a/commands/displayers/oauth_client.go b/commands/displayers/oauth_client.go new file mode 100644 index 000000000..bcfb48918 --- /dev/null +++ b/commands/displayers/oauth_client.go @@ -0,0 +1,90 @@ +/* +Copyright 2018 The Doctl Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package displayers + +import ( + "io" + "strings" + "time" +) + +// OAuthClient renders the OAuth application doctl registered for itself. It +// never carries the registration access token: that credential manages the +// registration and is kept out of command output. +type OAuthClient struct { + ClientID string + Issuer string + RedirectURIs []string + RegisteredAt time.Time + ManagementURL string +} + +var _ Displayable = &OAuthClient{} + +func (c *OAuthClient) JSON(out io.Writer) error { + return writeJSON(struct { + ClientID string `json:"client_id"` + Issuer string `json:"issuer"` + RedirectURIs []string `json:"redirect_uris,omitempty"` + RegisteredAt string `json:"registered_at,omitempty"` + ManagementURL string `json:"registration_client_uri,omitempty"` + }{ + ClientID: c.ClientID, + Issuer: c.Issuer, + RedirectURIs: c.RedirectURIs, + RegisteredAt: c.registeredAt(), + ManagementURL: c.ManagementURL, + }, out) +} + +func (c *OAuthClient) Cols() []string { + return []string{"ClientID", "Issuer", "RedirectURIs", "RegisteredAt", "ManagementURL"} +} + +func (c *OAuthClient) ColMap() map[string]string { + return map[string]string{ + "ClientID": "Client ID", + "Issuer": "Issuer", + "RedirectURIs": "Redirect URIs", + "RegisteredAt": "Registered At", + "ManagementURL": "Management URL", + } +} + +func (c *OAuthClient) KV() []map[string]any { + return []map[string]any{{ + "ClientID": c.ClientID, + "Issuer": c.Issuer, + "RedirectURIs": c.redirectURIs(), + "RegisteredAt": c.registeredAt(), + "ManagementURL": c.ManagementURL, + }} +} + +// redirectURIs reports unknown rather than blank for a registration created +// before doctl recorded them, so an empty column is not mistaken for a client +// that accepts no redirects. +func (c *OAuthClient) redirectURIs() string { + if len(c.RedirectURIs) == 0 { + return "unknown" + } + return strings.Join(c.RedirectURIs, ", ") +} + +func (c *OAuthClient) registeredAt() string { + if c.RegisteredAt.IsZero() { + return "" + } + return c.RegisteredAt.UTC().Format(time.RFC3339) +} diff --git a/internal/oauth/login.go b/internal/oauth/login.go new file mode 100644 index 000000000..b50b52104 --- /dev/null +++ b/internal/oauth/login.go @@ -0,0 +1,261 @@ +/* +Copyright 2018 The Doctl Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package oauth + +import ( + "context" + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "encoding/base64" + "errors" + "fmt" + "net" + "net/http" + "net/url" + "strings" + "time" + + "github.com/pkg/browser" +) + +// callbackPath is the path the local redirect server listens on. It is part of +// the registered redirect URIs, so it must stay stable across releases. +const callbackPath = "/callback" + +// DefaultLoginTimeout bounds how long the login flow waits for the user to +// finish authorizing in their browser. +const DefaultLoginTimeout = 5 * time.Minute + +// RegistrationRedirectURIs are the redirect URIs doctl registers for its +// dynamic client. Neither carries a port: the authorization server matches +// loopback redirects on any port (RFC 8252 section 7.3), which lets doctl bind +// an ephemeral port at login time. +func RegistrationRedirectURIs() []string { + return []string{ + "http://127.0.0.1" + callbackPath, + "http://localhost" + callbackPath, + } +} + +// AuthorizationError is an error the authorization server reported by +// redirecting back to doctl with an error parameter. +type AuthorizationError struct { + Code string + Description string +} + +func (e *AuthorizationError) Error() string { + if e.Description != "" { + return fmt.Sprintf("%s: %s", e.Code, e.Description) + } + return e.Code +} + +// Unwrap reports registration problems as ErrInvalidClient so callers can +// discard a stale dynamic client registration and try again. +func (e *AuthorizationError) Unwrap() error { + if isInvalidClientCode(e.Code) { + return ErrInvalidClient + } + return nil +} + +// LoginOptions configures a single authorization code login. +type LoginOptions struct { + // Metadata describes the authorization server endpoints to use. + Metadata *ServerMetadata + // ClientID identifies the dynamically registered public client. + ClientID string + // Scopes are the OAuth scopes to request. An empty list lets the + // authorization server apply its default scope. + Scopes []string + // Port is the local port to listen on for the redirect. Zero picks an + // unused port, which is the recommended behavior for native apps. + Port int + // Timeout bounds how long to wait for the browser redirect. Defaults to + // DefaultLoginTimeout. + Timeout time.Duration + // HTTPClient is used for the token exchange. + HTTPClient *http.Client + // NoBrowser prints the authorization URL instead of opening a browser. + NoBrowser bool + // OnAuthorizationURL, when set, is called with the authorization URL once + // the local redirect server is listening. + OnAuthorizationURL func(authURL string) + + // openURL is a test hook for opening the browser. + openURL func(authURL string) error +} + +// Login runs the OAuth 2.1 authorization code flow with PKCE against the +// configured authorization server and returns the resulting token grant. +func Login(ctx context.Context, opts LoginOptions) (*Token, error) { + if opts.Metadata == nil { + return nil, errors.New("authorization server metadata is required") + } + if opts.ClientID == "" { + return nil, errors.New("a client ID is required") + } + if opts.Timeout <= 0 { + opts.Timeout = DefaultLoginTimeout + } + if opts.openURL == nil { + opts.openURL = browser.OpenURL + } + + verifier, err := generateCodeVerifier() + if err != nil { + return nil, err + } + state, err := randomURLSafeString(32) + if err != nil { + return nil, err + } + + listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", opts.Port)) + if err != nil { + return nil, fmt.Errorf("listening for the authorization redirect: %w", err) + } + defer listener.Close() + + redirectURI := fmt.Sprintf("http://127.0.0.1:%d%s", listener.Addr().(*net.TCPAddr).Port, callbackPath) + authURL := authorizationURL(opts, redirectURI, state, verifier) + + ctx, cancel := context.WithTimeout(ctx, opts.Timeout) + defer cancel() + + results := make(chan callbackResult, 1) + server := &http.Server{Handler: &callbackHandler{state: state, results: results}} + defer server.Close() + + go func() { _ = server.Serve(listener) }() + + if opts.OnAuthorizationURL != nil { + opts.OnAuthorizationURL(authURL) + } + if !opts.NoBrowser { + if err := opts.openURL(authURL); err != nil { + return nil, fmt.Errorf("opening the authorization URL in a browser: %w", err) + } + } + + var result callbackResult + select { + case result = <-results: + case <-ctx.Done(): + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + return nil, fmt.Errorf("timed out after %s waiting for browser authorization", opts.Timeout) + } + return nil, ctx.Err() + } + + if result.err != nil { + return nil, result.err + } + + return ExchangeCode(ctx, opts.HTTPClient, opts.Metadata.TokenEndpoint, opts.ClientID, result.code, verifier, redirectURI) +} + +func authorizationURL(opts LoginOptions, redirectURI, state, verifier string) string { + query := url.Values{ + "client_id": {opts.ClientID}, + "redirect_uri": {redirectURI}, + "response_type": {responseTypeCode}, + "state": {state}, + "code_challenge": {codeChallengeS256(verifier)}, + "code_challenge_method": {CodeChallengeMethodS256}, + } + if scope := strings.TrimSpace(strings.Join(opts.Scopes, " ")); scope != "" { + query.Set("scope", scope) + } + + separator := "?" + if strings.Contains(opts.Metadata.AuthorizationEndpoint, "?") { + separator = "&" + } + + return opts.Metadata.AuthorizationEndpoint + separator + query.Encode() +} + +type callbackResult struct { + code string + err error +} + +type callbackHandler struct { + state string + results chan<- callbackResult +} + +func (h *callbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != callbackPath { + http.NotFound(w, r) + return + } + + query := r.URL.Query() + result, page, status := h.resultFor(query) + + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(status) + _, _ = w.Write([]byte(page)) + + select { + case h.results <- result: + default: + } +} + +func (h *callbackHandler) resultFor(query url.Values) (callbackResult, string, int) { + // The state check comes first: a mismatched state means the redirect did + // not originate from the authorization request we started. + if subtle.ConstantTimeCompare([]byte(query.Get("state")), []byte(h.state)) != 1 { + err := errors.New("the authorization response state did not match the request; the login attempt may have been forged") + return callbackResult{err: err}, errorPage(err.Error()), http.StatusBadRequest + } + + if code := query.Get("error"); code != "" { + err := &AuthorizationError{Code: code, Description: query.Get("error_description")} + return callbackResult{err: err}, errorPage(err.Error()), http.StatusBadRequest + } + + code := query.Get("code") + if code == "" { + err := errors.New("the authorization response did not include an authorization code") + return callbackResult{err: err}, errorPage(err.Error()), http.StatusBadRequest + } + + return callbackResult{code: code}, successPage, http.StatusOK +} + +// generateCodeVerifier returns a PKCE code verifier: 32 random bytes encoded +// as 43 unreserved characters, within the 43-128 character range RFC 7636 +// section 4.1 allows. +func generateCodeVerifier() (string, error) { + return randomURLSafeString(32) +} + +func codeChallengeS256(verifier string) string { + sum := sha256.Sum256([]byte(verifier)) + return base64.RawURLEncoding.EncodeToString(sum[:]) +} + +func randomURLSafeString(byteLen int) (string, error) { + buf := make([]byte, byteLen) + if _, err := rand.Read(buf); err != nil { + return "", fmt.Errorf("generating random data: %w", err) + } + return base64.RawURLEncoding.EncodeToString(buf), nil +} diff --git a/internal/oauth/login_test.go b/internal/oauth/login_test.go new file mode 100644 index 000000000..ce4b12dfe --- /dev/null +++ b/internal/oauth/login_test.go @@ -0,0 +1,242 @@ +/* +Copyright 2018 The Doctl Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package oauth + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLogin(t *testing.T) { + var challenge, verifier string + + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + verifier = r.FormValue("code_verifier") + assert.Equal(t, "auth-code", r.FormValue("code")) + + writeJSON(t, w, http.StatusOK, map[string]any{ + "access_token": "doo_v1_access", + "refresh_token": "dor_v1_refresh", + "expires_in": 3600, + }) + })) + defer tokenServer.Close() + + var authorizedRedirectURI string + // Stand in for the browser: read the authorization request, then call the + // redirect URI the way the authorization server would. + openURL := func(authURL string) error { + parsed, err := url.Parse(authURL) + require.NoError(t, err) + + query := parsed.Query() + assert.Equal(t, "client-id", query.Get("client_id")) + assert.Equal(t, responseTypeCode, query.Get("response_type")) + assert.Equal(t, CodeChallengeMethodS256, query.Get("code_challenge_method")) + assert.Equal(t, "read write", query.Get("scope")) + assert.NotEmpty(t, query.Get("state")) + assert.Empty(t, query.Get("resource"), "resource indicators are not requested") + + challenge = query.Get("code_challenge") + authorizedRedirectURI = query.Get("redirect_uri") + + return followRedirect(t, query.Get("redirect_uri"), url.Values{ + "code": {"auth-code"}, + "state": {query.Get("state")}, + }) + } + + token, err := Login(context.Background(), LoginOptions{ + Metadata: &ServerMetadata{AuthorizationEndpoint: "https://cloud.example.com/v1/oauth/authorize", TokenEndpoint: tokenServer.URL}, + ClientID: "client-id", + Scopes: []string{"read", "write"}, + HTTPClient: tokenServer.Client(), + openURL: openURL, + }) + require.NoError(t, err) + + assert.Equal(t, "doo_v1_access", token.AccessToken) + assert.Equal(t, "dor_v1_refresh", token.RefreshToken) + assert.Equal(t, challenge, codeChallengeS256(verifier), "the verifier must hash to the challenge sent to the authorization server") + assert.Regexp(t, `^http://127\.0\.0\.1:\d+/callback$`, authorizedRedirectURI) +} + +func TestLoginReportsAuthorizationErrors(t *testing.T) { + tests := []struct { + name string + params func(state string) url.Values + expectedError string + invalidClient bool + }{ + { + name: "access denied", + params: func(state string) url.Values { + return url.Values{ + "error": {"access_denied"}, + "error_description": {"the user declined the request"}, + "state": {state}, + } + }, + expectedError: "the user declined the request", + }, + { + name: "unknown client", + params: func(state string) url.Values { + return url.Values{"error": {"invalid_client"}, "state": {state}} + }, + expectedError: "invalid_client", + invalidClient: true, + }, + { + name: "mismatched state", + params: func(string) url.Values { + return url.Values{"code": {"auth-code"}, "state": {"not-the-state"}} + }, + expectedError: "state did not match", + }, + { + name: "missing code", + params: func(state string) url.Values { + return url.Values{"state": {state}} + }, + expectedError: "did not include an authorization code", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + openURL := func(authURL string) error { + parsed, err := url.Parse(authURL) + require.NoError(t, err) + + query := parsed.Query() + return followRedirect(t, query.Get("redirect_uri"), test.params(query.Get("state"))) + } + + _, err := Login(context.Background(), LoginOptions{ + Metadata: &ServerMetadata{AuthorizationEndpoint: "https://cloud.example.com/v1/oauth/authorize", TokenEndpoint: "https://cloud.example.com/v1/oauth/token"}, + ClientID: "client-id", + HTTPClient: failingClient(t), + openURL: openURL, + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), test.expectedError) + if test.invalidClient { + assert.ErrorIs(t, err, ErrInvalidClient) + } else { + assert.NotErrorIs(t, err, ErrInvalidClient) + } + }) + } +} + +func TestLoginTimesOut(t *testing.T) { + _, err := Login(context.Background(), LoginOptions{ + Metadata: &ServerMetadata{AuthorizationEndpoint: "https://cloud.example.com/v1/oauth/authorize"}, + ClientID: "client-id", + Timeout: 50 * time.Millisecond, + openURL: func(string) error { return nil }, + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), "timed out") +} + +func TestLoginRequiresClientID(t *testing.T) { + _, err := Login(context.Background(), LoginOptions{Metadata: &ServerMetadata{}}) + + assert.Error(t, err) +} + +func TestLoginPrintsAuthorizationURL(t *testing.T) { + var printed string + + _, err := Login(context.Background(), LoginOptions{ + Metadata: &ServerMetadata{AuthorizationEndpoint: "https://cloud.example.com/v1/oauth/authorize?foo=bar"}, + ClientID: "client-id", + Timeout: 50 * time.Millisecond, + NoBrowser: true, + OnAuthorizationURL: func(authURL string) { printed = authURL }, + openURL: func(string) error { + return errors.New("the browser must not be opened with --no-browser") + }, + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), "timed out") + assert.Contains(t, printed, "https://cloud.example.com/v1/oauth/authorize?foo=bar&client_id=client-id") +} + +func TestLoginOmitsAnEmptyScope(t *testing.T) { + var printed string + + _, err := Login(context.Background(), LoginOptions{ + Metadata: &ServerMetadata{AuthorizationEndpoint: "https://cloud.example.com/v1/oauth/authorize"}, + ClientID: "client-id", + Timeout: 50 * time.Millisecond, + OnAuthorizationURL: func(authURL string) { printed = authURL }, + openURL: func(string) error { return nil }, + }) + require.Error(t, err) + + parsed, err := url.Parse(printed) + require.NoError(t, err) + + // Sending no scope leaves the granted permissions up to the authorization + // screen rather than having doctl choose them. + assert.False(t, parsed.Query().Has("scope")) +} + +func TestCodeVerifier(t *testing.T) { + verifier, err := generateCodeVerifier() + require.NoError(t, err) + + // RFC 7636 section 4.1 allows 43 to 128 unreserved characters. + assert.Len(t, verifier, 43) + assert.Regexp(t, `^[A-Za-z0-9\-._~]+$`, verifier) + + other, err := generateCodeVerifier() + require.NoError(t, err) + assert.NotEqual(t, verifier, other) + + // The example verifier and challenge from RFC 7636 appendix B. + assert.Equal(t, + "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + codeChallengeS256("dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"), + ) +} + +// followRedirect simulates the browser being redirected back to doctl's local +// callback server. +func followRedirect(t *testing.T, redirectURI string, params url.Values) error { + t.Helper() + + resp, err := http.Get(redirectURI + "?" + params.Encode()) + if err != nil { + return err + } + defer resp.Body.Close() + + return nil +} diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go new file mode 100644 index 000000000..f8affc0b3 --- /dev/null +++ b/internal/oauth/oauth.go @@ -0,0 +1,402 @@ +/* +Copyright 2018 The Doctl Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package oauth implements the client side of DigitalOcean's OAuth 2.1 +// authorization code flow with PKCE (RFC 7636) for native applications +// (RFC 8252), including dynamic client registration (RFC 7591). +package oauth + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" +) + +const ( + // DefaultIssuer is the DigitalOcean authorization server. + DefaultIssuer = "https://cloud.digitalocean.com" + + // CodeChallengeMethodS256 is the only PKCE code challenge method the + // authorization server supports. + CodeChallengeMethodS256 = "S256" + + // TokenEndpointAuthMethodNone designates a public client that + // authenticates at the token endpoint with PKCE instead of a secret. + TokenEndpointAuthMethodNone = "none" + + grantTypeAuthorizationCode = "authorization_code" + grantTypeRefreshToken = "refresh_token" + responseTypeCode = "code" + + metadataPath = "/.well-known/oauth-authorization-server" + authorizePath = "/v1/oauth/authorize" + tokenPath = "/v1/oauth/token" + registrationPath = "/v1/oauth/register" + revocationPath = "/v1/oauth/revoke" + + formContentType = "application/x-www-form-urlencoded" +) + +// ErrInvalidClient is returned when the authorization server rejects the +// client we are registered as. Callers can use it to discard a stale dynamic +// registration and register again. +var ErrInvalidClient = errors.New("the authorization server rejected this client registration") + +// ServerMetadata is the subset of the RFC 8414 authorization server metadata +// document that doctl uses. +type ServerMetadata struct { + Issuer string `json:"issuer"` + AuthorizationEndpoint string `json:"authorization_endpoint"` + TokenEndpoint string `json:"token_endpoint"` + RegistrationEndpoint string `json:"registration_endpoint"` + RevocationEndpoint string `json:"revocation_endpoint"` + GrantTypesSupported []string `json:"grant_types_supported"` + CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported"` +} + +// ResolveServerMetadata fetches the authorization server metadata document for +// issuer. The document is advisory: any endpoint the server does not advertise +// falls back to its well-known DigitalOcean path, so a server that does not +// serve the metadata document is still usable. +func ResolveServerMetadata(ctx context.Context, client *http.Client, issuer string) *ServerMetadata { + issuer = strings.TrimSuffix(strings.TrimSpace(issuer), "/") + if issuer == "" { + issuer = DefaultIssuer + } + + md := &ServerMetadata{Issuer: issuer} + if discovered, err := discoverServerMetadata(ctx, client, issuer); err == nil { + md = discovered + } + + if md.Issuer == "" { + md.Issuer = issuer + } + if md.AuthorizationEndpoint == "" { + md.AuthorizationEndpoint = issuer + authorizePath + } + if md.TokenEndpoint == "" { + md.TokenEndpoint = issuer + tokenPath + } + if md.RegistrationEndpoint == "" { + md.RegistrationEndpoint = issuer + registrationPath + } + if md.RevocationEndpoint == "" { + md.RevocationEndpoint = issuer + revocationPath + } + + return md +} + +func discoverServerMetadata(ctx context.Context, client *http.Client, issuer string) (*ServerMetadata, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, issuer+metadataPath, nil) + if err != nil { + return nil, err + } + req.Header.Set("Accept", "application/json") + + resp, err := doRequest(client, req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("authorization server metadata request returned %s", resp.Status) + } + + md := &ServerMetadata{} + if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(md); err != nil { + return nil, err + } + + return md, nil +} + +// ClientRegistration is an RFC 7591 client information response for a +// dynamically registered public client. +type ClientRegistration struct { + ClientID string `json:"client_id"` + ClientIDIssuedAt int64 `json:"client_id_issued_at,omitempty"` + RedirectURIs []string `json:"redirect_uris,omitempty"` + TokenEndpointAuthMethod string `json:"token_endpoint_auth_method,omitempty"` + GrantTypes []string `json:"grant_types,omitempty"` + ResponseTypes []string `json:"response_types,omitempty"` + ClientName string `json:"client_name,omitempty"` + ClientURI string `json:"client_uri,omitempty"` + RegistrationAccessToken string `json:"registration_access_token,omitempty"` + RegistrationClientURI string `json:"registration_client_uri,omitempty"` +} + +type registerClientRequest struct { + RedirectURIs []string `json:"redirect_uris"` + TokenEndpointAuthMethod string `json:"token_endpoint_auth_method"` + GrantTypes []string `json:"grant_types"` + ResponseTypes []string `json:"response_types"` + ClientName string `json:"client_name,omitempty"` + ClientURI string `json:"client_uri,omitempty"` +} + +// RegisterClient performs RFC 7591 dynamic client registration against +// endpoint, registering a public client that authenticates with PKCE. The +// endpoint is unauthenticated, so no token is required. +func RegisterClient(ctx context.Context, client *http.Client, endpoint string, redirectURIs []string, name, uri string) (*ClientRegistration, error) { + if len(redirectURIs) == 0 { + return nil, errors.New("at least one redirect URI is required to register a client") + } + + body, err := json.Marshal(®isterClientRequest{ + RedirectURIs: redirectURIs, + TokenEndpointAuthMethod: TokenEndpointAuthMethodNone, + GrantTypes: []string{grantTypeAuthorizationCode, grantTypeRefreshToken}, + ResponseTypes: []string{responseTypeCode}, + ClientName: name, + ClientURI: uri, + }) + if err != nil { + return nil, err + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(string(body))) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") + + resp, err := doRequest(client, req) + if err != nil { + return nil, fmt.Errorf("registering doctl as an OAuth application: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { + return nil, responseError(resp, "registering doctl as an OAuth application") + } + + registration := &ClientRegistration{} + if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(registration); err != nil { + return nil, fmt.Errorf("decoding client registration response: %w", err) + } + if registration.ClientID == "" { + return nil, errors.New("the authorization server did not return a client ID") + } + + return registration, nil +} + +// Token is an access token grant issued by the authorization server. +type Token struct { + AccessToken string + RefreshToken string + TokenType string + Scope string + // Expiry is the absolute time the access token expires. It is the zero + // value when the server does not report an expiration. + Expiry time.Time + Info TokenInfo +} + +// TokenInfo identifies the user and team a token was issued for. +type TokenInfo struct { + Name string `json:"name"` + Email string `json:"email"` + UUID string `json:"uuid"` + TeamUUID string `json:"team_uuid"` + TeamName string `json:"team_name"` +} + +// tokenGrantResponse mirrors the authorization server's token response. Public +// clients receive the RFC 6749 token_type field; confidential clients receive +// the legacy bearer field instead. +type tokenGrantResponse struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + TokenType string `json:"token_type"` + LegacyBearer string `json:"bearer"` + ExpiresIn *int64 `json:"expires_in"` + Scope string `json:"scope"` + Info *TokenInfo `json:"info"` +} + +func (r *tokenGrantResponse) token(now time.Time) *Token { + t := &Token{ + AccessToken: r.AccessToken, + RefreshToken: r.RefreshToken, + TokenType: r.TokenType, + Scope: r.Scope, + } + if t.TokenType == "" { + t.TokenType = r.LegacyBearer + } + if r.ExpiresIn != nil && *r.ExpiresIn > 0 { + t.Expiry = now.Add(time.Duration(*r.ExpiresIn) * time.Second) + } + if r.Info != nil { + t.Info = *r.Info + } + + return t +} + +// ExchangeCode exchanges an authorization code for a token grant, proving +// possession of the PKCE code verifier in place of a client secret. +func ExchangeCode(ctx context.Context, client *http.Client, tokenEndpoint, clientID, code, codeVerifier, redirectURI string) (*Token, error) { + return postTokenRequest(ctx, client, tokenEndpoint, url.Values{ + "grant_type": {grantTypeAuthorizationCode}, + "code": {code}, + "client_id": {clientID}, + "code_verifier": {codeVerifier}, + "redirect_uri": {redirectURI}, + }, "exchanging the authorization code") +} + +// RefreshToken trades a refresh token for a new token grant. Refresh tokens +// are single use: the response carries the replacement refresh token. +func RefreshToken(ctx context.Context, client *http.Client, tokenEndpoint, clientID, refreshToken string) (*Token, error) { + values := url.Values{ + "grant_type": {grantTypeRefreshToken}, + "refresh_token": {refreshToken}, + } + if clientID != "" { + values.Set("client_id", clientID) + } + + token, err := postTokenRequest(ctx, client, tokenEndpoint, values, "refreshing the access token") + if err != nil { + return nil, err + } + + // The server rotates refresh tokens, but tolerate one that reuses the + // presented token rather than losing the ability to refresh again. + if token.RefreshToken == "" { + token.RefreshToken = refreshToken + } + + return token, nil +} + +func postTokenRequest(ctx context.Context, client *http.Client, endpoint string, values url.Values, action string) (*Token, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(values.Encode())) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", formContentType) + req.Header.Set("Accept", "application/json") + + resp, err := doRequest(client, req) + if err != nil { + return nil, fmt.Errorf("%s: %w", action, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, responseError(resp, action) + } + + grant := &tokenGrantResponse{} + if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(grant); err != nil { + return nil, fmt.Errorf("decoding the token response: %w", err) + } + if grant.AccessToken == "" { + return nil, errors.New("the authorization server did not return an access token") + } + + return grant.token(time.Now()), nil +} + +// RevokeToken invalidates an access token. The authorization server requires +// the token both as a bearer credential and as a form value. +func RevokeToken(ctx context.Context, client *http.Client, revocationEndpoint, accessToken string) error { + values := url.Values{"token": {accessToken}} + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, revocationEndpoint, strings.NewReader(values.Encode())) + if err != nil { + return err + } + req.Header.Set("Content-Type", formContentType) + req.Header.Set("Authorization", "Bearer "+accessToken) + + resp, err := doRequest(client, req) + if err != nil { + return fmt.Errorf("revoking the access token: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNoContent { + return responseError(resp, "revoking the access token") + } + + return nil +} + +func doRequest(client *http.Client, req *http.Request) (*http.Response, error) { + if client == nil { + client = http.DefaultClient + } + return client.Do(req) +} + +// responseError converts an OAuth error response body (RFC 6749 section 5.2, +// RFC 7591 section 3.2.2) into an error. +func responseError(resp *http.Response, action string) error { + var body struct { + Error string `json:"error"` + Description string `json:"error_description"` + Message string `json:"message"` + ID string `json:"id"` + } + _ = json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&body) + + code := body.Error + if code == "" { + code = body.ID + } + detail := body.Description + if detail == "" { + detail = body.Message + } + + var err error + switch { + case code != "" && detail != "": + err = fmt.Errorf("%s: %s: %s", action, code, detail) + case code != "": + err = fmt.Errorf("%s: %s", action, code) + case detail != "": + err = fmt.Errorf("%s: %s", action, detail) + default: + err = fmt.Errorf("%s: the authorization server returned %s", action, resp.Status) + } + + if isInvalidClientCode(code) { + return fmt.Errorf("%w: %s", ErrInvalidClient, err) + } + + return err +} + +func isInvalidClientCode(code string) bool { + switch code { + case "invalid_client", "unauthorized_client", "invalid_redirect_uri": + return true + default: + return false + } +} diff --git a/internal/oauth/oauth_test.go b/internal/oauth/oauth_test.go new file mode 100644 index 000000000..181432711 --- /dev/null +++ b/internal/oauth/oauth_test.go @@ -0,0 +1,272 @@ +/* +Copyright 2018 The Doctl Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package oauth + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResolveServerMetadata(t *testing.T) { + t.Run("uses the advertised endpoints", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, metadataPath, r.URL.Path) + writeJSON(t, w, http.StatusOK, map[string]any{ + "issuer": "https://cloud.digitalocean.com", + "authorization_endpoint": "https://cloud.digitalocean.com/v1/oauth/authorize", + "token_endpoint": "https://cloud.digitalocean.com/v1/oauth/token", + "registration_endpoint": "https://cloud.digitalocean.com/v1/oauth/register", + "revocation_endpoint": "https://cloud.digitalocean.com/v1/oauth/revoke", + "code_challenge_methods_supported": []string{"S256"}, + }) + })) + defer server.Close() + + md := ResolveServerMetadata(context.Background(), server.Client(), server.URL) + + assert.Equal(t, "https://cloud.digitalocean.com", md.Issuer) + assert.Equal(t, "https://cloud.digitalocean.com/v1/oauth/token", md.TokenEndpoint) + assert.Equal(t, []string{"S256"}, md.CodeChallengeMethodsSupported) + }) + + t.Run("falls back to the well-known paths", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + + md := ResolveServerMetadata(context.Background(), server.Client(), server.URL+"/") + + assert.Equal(t, server.URL, md.Issuer) + assert.Equal(t, server.URL+authorizePath, md.AuthorizationEndpoint) + assert.Equal(t, server.URL+tokenPath, md.TokenEndpoint) + assert.Equal(t, server.URL+registrationPath, md.RegistrationEndpoint) + assert.Equal(t, server.URL+revocationPath, md.RevocationEndpoint) + }) + + t.Run("defaults to the DigitalOcean issuer", func(t *testing.T) { + md := ResolveServerMetadata(context.Background(), failingClient(t), "") + + assert.Equal(t, DefaultIssuer, md.Issuer) + assert.Equal(t, DefaultIssuer+tokenPath, md.TokenEndpoint) + }) +} + +func TestRegisterClient(t *testing.T) { + t.Run("registers a public client", func(t *testing.T) { + var body registerClientRequest + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + writeJSON(t, w, http.StatusCreated, map[string]any{ + "client_id": "client-id", + "redirect_uris": body.RedirectURIs, + "registration_access_token": "registration-token", + "registration_client_uri": "https://cloud.digitalocean.com/v1/oauth/register/client-id", + "token_endpoint_auth_method": TokenEndpointAuthMethodNone, + }) + })) + defer server.Close() + + registration, err := RegisterClient( + context.Background(), server.Client(), server.URL, + RegistrationRedirectURIs(), "doctl", "https://example.com", + ) + require.NoError(t, err) + + assert.Equal(t, "client-id", registration.ClientID) + assert.Equal(t, "registration-token", registration.RegistrationAccessToken) + + assert.Equal(t, TokenEndpointAuthMethodNone, body.TokenEndpointAuthMethod) + assert.Equal(t, []string{grantTypeAuthorizationCode, grantTypeRefreshToken}, body.GrantTypes) + assert.Equal(t, []string{responseTypeCode}, body.ResponseTypes) + assert.Equal(t, "doctl", body.ClientName) + assert.Equal(t, []string{"http://127.0.0.1/callback", "http://localhost/callback"}, body.RedirectURIs) + }) + + t.Run("surfaces registration errors", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(t, w, http.StatusBadRequest, map[string]any{ + "error": "invalid_client_metadata", + "error_description": "grant_types must include \"authorization_code\"", + }) + })) + defer server.Close() + + _, err := RegisterClient(context.Background(), server.Client(), server.URL, RegistrationRedirectURIs(), "doctl", "") + + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid_client_metadata") + assert.Contains(t, err.Error(), "grant_types must include") + }) + + t.Run("requires a redirect URI", func(t *testing.T) { + _, err := RegisterClient(context.Background(), failingClient(t), "https://example.com", nil, "doctl", "") + assert.Error(t, err) + }) +} + +func TestExchangeCode(t *testing.T) { + expiresIn := int64(3600) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + assert.Equal(t, grantTypeAuthorizationCode, r.FormValue("grant_type")) + assert.Equal(t, "auth-code", r.FormValue("code")) + assert.Equal(t, "client-id", r.FormValue("client_id")) + assert.Equal(t, "verifier", r.FormValue("code_verifier")) + assert.Equal(t, "http://127.0.0.1:1234/callback", r.FormValue("redirect_uri")) + assert.Empty(t, r.FormValue("client_secret"), "public clients must not send a secret") + assert.Empty(t, r.FormValue("resource")) + + writeJSON(t, w, http.StatusOK, map[string]any{ + "access_token": "doo_v1_access", + "refresh_token": "dor_v1_refresh", + "token_type": "Bearer", + "expires_in": expiresIn, + "scope": "api:read api:write", + "info": map[string]any{ + "name": "Sammy", + "email": "sammy@example.com", + "team_name": "My Team", + }, + }) + })) + defer server.Close() + + before := time.Now() + token, err := ExchangeCode( + context.Background(), server.Client(), server.URL, + "client-id", "auth-code", "verifier", "http://127.0.0.1:1234/callback", + ) + require.NoError(t, err) + + assert.Equal(t, "doo_v1_access", token.AccessToken) + assert.Equal(t, "dor_v1_refresh", token.RefreshToken) + assert.Equal(t, "Bearer", token.TokenType) + assert.Equal(t, "api:read api:write", token.Scope) + assert.Equal(t, "sammy@example.com", token.Info.Email) + assert.Equal(t, "My Team", token.Info.TeamName) + assert.WithinDuration(t, before.Add(time.Duration(expiresIn)*time.Second), token.Expiry, time.Minute) +} + +func TestExchangeCodeReportsInvalidClient(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(t, w, http.StatusUnauthorized, map[string]any{"error": "invalid_client"}) + })) + defer server.Close() + + _, err := ExchangeCode(context.Background(), server.Client(), server.URL, "client-id", "code", "verifier", "http://127.0.0.1/callback") + + assert.ErrorIs(t, err, ErrInvalidClient) +} + +func TestRefreshToken(t *testing.T) { + t.Run("returns the rotated refresh token", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + assert.Equal(t, grantTypeRefreshToken, r.FormValue("grant_type")) + assert.Equal(t, "old-refresh", r.FormValue("refresh_token")) + + writeJSON(t, w, http.StatusOK, map[string]any{ + "access_token": "new-access", + "refresh_token": "new-refresh", + "expires_in": 3600, + }) + })) + defer server.Close() + + token, err := RefreshToken(context.Background(), server.Client(), server.URL, "client-id", "old-refresh") + require.NoError(t, err) + + assert.Equal(t, "new-access", token.AccessToken) + assert.Equal(t, "new-refresh", token.RefreshToken) + }) + + t.Run("keeps the presented refresh token when none is returned", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(t, w, http.StatusOK, map[string]any{"access_token": "new-access"}) + })) + defer server.Close() + + token, err := RefreshToken(context.Background(), server.Client(), server.URL, "", "old-refresh") + require.NoError(t, err) + + assert.Equal(t, "old-refresh", token.RefreshToken) + assert.True(t, token.Expiry.IsZero()) + }) + + t.Run("errors when the grant is rejected", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(t, w, http.StatusUnauthorized, map[string]any{ + "error": "invalid_grant", + "error_description": "refresh token is expired", + }) + })) + defer server.Close() + + _, err := RefreshToken(context.Background(), server.Client(), server.URL, "client-id", "old-refresh") + + require.Error(t, err) + assert.Contains(t, err.Error(), "refresh token is expired") + assert.NotErrorIs(t, err, ErrInvalidClient) + }) +} + +func TestRevokeToken(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + assert.Equal(t, "Bearer doo_v1_access", r.Header.Get("Authorization")) + assert.Equal(t, "doo_v1_access", r.FormValue("token")) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + assert.NoError(t, RevokeToken(context.Background(), server.Client(), server.URL, "doo_v1_access")) +} + +func TestTokenGrantResponseUsesLegacyBearerField(t *testing.T) { + grant := &tokenGrantResponse{AccessToken: "token", LegacyBearer: "bearer"} + + assert.Equal(t, "bearer", grant.token(time.Now()).TokenType) +} + +func writeJSON(t *testing.T, w http.ResponseWriter, status int, body any) { + t.Helper() + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + require.NoError(t, json.NewEncoder(w).Encode(body)) +} + +// failingClient fails any request, asserting that a code path does not reach +// the network. +func failingClient(t *testing.T) *http.Client { + t.Helper() + + return &http.Client{ + Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) { + return nil, assert.AnError + }), + } +} + +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } diff --git a/internal/oauth/pages.go b/internal/oauth/pages.go new file mode 100644 index 000000000..f776585c8 --- /dev/null +++ b/internal/oauth/pages.go @@ -0,0 +1,82 @@ +/* +Copyright 2018 The Doctl Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package oauth + +import ( + "fmt" + "html/template" +) + +const pageTemplate = ` + + + +doctl + + + +
+
%s
+

%s

+

%s

+
+ + +` + +var successPage = fmt.Sprintf( + pageTemplate, + "ok", "✓", + "You're signed in", + "doctl is now authenticated with your DigitalOcean account. You can close this tab and return to your terminal.", +) + +func errorPage(reason string) string { + return fmt.Sprintf( + pageTemplate, + "fail", "✕", + "Authorization failed", + template.HTMLEscapeString(reason)+" You can close this tab and try again in your terminal.", + ) +} From 2e038e0678305e6e64cba45dc47a82d9ca8410e6 Mon Sep 17 00:00:00 2001 From: Anup Deka Date: Mon, 28 Sep 2026 21:58:29 +0530 Subject: [PATCH 02/10] Updating doctl oauth flow --- commands/auth_login.go | 14 ++++++++++++- commands/auth_login_test.go | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/commands/auth_login.go b/commands/auth_login.go index b0a8cb3d6..b8feb961e 100644 --- a/commands/auth_login.go +++ b/commands/auth_login.go @@ -114,10 +114,13 @@ func RunAuthLogin(c *CmdConfig) error { if err != nil { return err } - noBrowser, err := c.Doit.GetBool(c.NS, doctl.ArgOAuthNoBrowser) + noBrowser, err := oauthNoBrowser(c) if err != nil { return err } + // writeConfig persists every viper setting, so a single --no-browser would + // otherwise stay true and keep the browser closed on later logins. + viper.Set(c.NS+"."+doctl.ArgOAuthNoBrowser, false) timeout, err := c.Doit.GetDuration(c.NS, doctl.ArgOAuthTimeout) if err != nil { return err @@ -241,6 +244,15 @@ func registerOAuthClient(ctx context.Context, c *CmdConfig, httpClient *http.Cli return client, nil } +// oauthNoBrowser reports whether this invocation passed --no-browser. +// A value left in the config file from an earlier login is ignored. +func oauthNoBrowser(c *CmdConfig) (bool, error) { + if !c.Doit.IsSet(doctl.ArgOAuthNoBrowser) { + return false, nil + } + return c.Doit.GetBool(c.NS, doctl.ArgOAuthNoBrowser) +} + func authorizationURLPrinter(c *CmdConfig, noBrowser bool) func(string) { return func(authURL string) { if noBrowser { diff --git a/commands/auth_login_test.go b/commands/auth_login_test.go index 764ba913f..11a4631e2 100644 --- a/commands/auth_login_test.go +++ b/commands/auth_login_test.go @@ -60,6 +60,7 @@ func TestRunAuthLogin(t *testing.T) { assert.Equal(t, "client-1", capturedOpts.ClientID) assert.Empty(t, capturedOpts.Scopes, "without --scope the authorization screen decides the permissions") assert.Equal(t, server.URL+"/v1/oauth/token", capturedOpts.Metadata.TokenEndpoint) + assert.False(t, capturedOpts.NoBrowser) client := loadOAuthClientState() require.NotNil(t, client) @@ -77,6 +78,44 @@ func TestRunAuthLogin(t *testing.T) { assert.False(t, state.ExpiresAt.IsZero()) } +func TestRunAuthLoginNoBrowserAppliesOnlyToThisInvocation(t *testing.T) { + server := newTestAuthorizationServer(t, func() int { return 1 }) + defer server.Close() + + config, _ := newOAuthTestCmdConfig(t, server.URL) + // Left behind by an earlier `doctl auth login --no-browser`, which + // writeConfig saves with the rest of the settings. + config.Doit.Set(config.NS, doctl.ArgOAuthNoBrowser, true) + config.Doit.(*doctl.TestConfig).IsSetMap[doctl.ArgOAuthNoBrowser] = false + + var capturedOpts oauth.LoginOptions + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + capturedOpts = opts + return &oauth.Token{AccessToken: "doo_v1_access", RefreshToken: "dor_v1_refresh"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + assert.False(t, capturedOpts.NoBrowser, "a saved --no-browser must not keep the browser closed") + assert.Equal(t, false, viper.Get(config.NS+"."+doctl.ArgOAuthNoBrowser)) +} + +func TestRunAuthLoginHonorsNoBrowserFlag(t *testing.T) { + server := newTestAuthorizationServer(t, func() int { return 1 }) + defer server.Close() + + config, _ := newOAuthTestCmdConfig(t, server.URL) + config.Doit.Set(config.NS, doctl.ArgOAuthNoBrowser, true) + + var capturedOpts oauth.LoginOptions + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + capturedOpts = opts + return &oauth.Token{AccessToken: "doo_v1_access", RefreshToken: "dor_v1_refresh"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + assert.True(t, capturedOpts.NoBrowser) +} + func TestRunAuthLoginRequestsTheGivenScopes(t *testing.T) { server := newTestAuthorizationServer(t, func() int { return 1 }) defer server.Close() @@ -395,6 +434,8 @@ func resetOAuthConfig(t *testing.T) { previousClient := viper.Get(oauthClientConfigKey) previousTokens := viper.Get(oauthTokensConfigKey) previousViperContext := viper.Get(doctl.ArgContext) + noBrowserKey := "test." + doctl.ArgOAuthNoBrowser + previousNoBrowser := viper.Get(noBrowserKey) cfgFileWriter = func() (io.WriteCloser, error) { return &nopWriteCloser{Writer: io.Discard}, nil } Context = "" @@ -408,6 +449,7 @@ func resetOAuthConfig(t *testing.T) { viper.Set(oauthClientConfigKey, previousClient) viper.Set(oauthTokensConfigKey, previousTokens) viper.Set(doctl.ArgContext, previousViperContext) + viper.Set(noBrowserKey, previousNoBrowser) }) } From ed3c12b35343a8e04d234d285fcf311c46cc2cec Mon Sep 17 00:00:00 2001 From: Anup Deka Date: Wed, 30 Sep 2026 00:47:18 +0530 Subject: [PATCH 03/10] Updating doctl oauth --- args.go | 4 + commands/auth.go | 22 +-- commands/auth_client.go | 61 ------ commands/auth_client_test.go | 59 ------ commands/auth_login.go | 273 ++++++++++++-------------- commands/auth_login_test.go | 285 +++++++++++++++++----------- commands/auth_test.go | 8 +- commands/displayers/oauth_client.go | 90 --------- internal/oauth/login.go | 10 +- internal/oauth/oauth.go | 175 +++-------------- internal/oauth/oauth_test.go | 98 +--------- 11 files changed, 355 insertions(+), 730 deletions(-) delete mode 100644 commands/auth_client.go delete mode 100644 commands/auth_client_test.go delete mode 100644 commands/displayers/oauth_client.go diff --git a/args.go b/args.go index 8f244812a..a4adc2f22 100644 --- a/args.go +++ b/args.go @@ -747,8 +747,12 @@ const ( // ArgOAuthServer is the OAuth authorization server used to sign in. ArgOAuthServer = "oauth-server" + // ArgOAuthClientID overrides the OAuth application doctl signs in as. + ArgOAuthClientID = "client-id" // ArgOAuthScopes is the space-separated list of scopes requested when signing in. ArgOAuthScopes = "scope" + // ArgOAuthSaveScope persists --scope as the default for later logins. + ArgOAuthSaveScope = "save-scope" // ArgOAuthCallbackPort is the local port that receives the OAuth redirect. ArgOAuthCallbackPort = "callback-port" // ArgOAuthNoBrowser prints the authorization URL instead of opening a browser. diff --git a/commands/auth.go b/commands/auth.go index a9d2a88c9..6e91e843e 100644 --- a/commands/auth.go +++ b/commands/auth.go @@ -95,13 +95,13 @@ func Auth() *Command { Command: &cobra.Command{ Use: "auth", Short: "Display commands for authenticating doctl with an account", - Long: `The ` + "`" + `doctl auth` + "`" + ` commands allow you to authenticate doctl for use with your DigitalOcean account, either by signing in through your browser or with tokens that you generate in the control panel at https://cloud.digitalocean.com/account/api/tokens. + Long: `The ` + "`" + `doctl auth` + "`" + ` commands allow you to authenticate doctl for use with your DigitalOcean account, either by signing in through your browser with OAuth or with tokens that you generate in the control panel at https://cloud.digitalocean.com/account/api/tokens. -The quickest way to get started is ` + "`" + `doctl auth login` + "`" + `, which authorizes doctl in your browser and keeps the resulting token up to date for you, so you don't have to generate a new one when it expires. +The quickest way to get started is ` + "`" + `doctl auth login` + "`" + `, which uses the OAuth 2.1 authorization code flow with PKCE. It opens your browser so you can approve access, then saves the resulting access token to your authentication context. Tokens issued this way are short-lived; doctl stores a refresh token and renews the access token automatically, so you do not have to return to the control panel when it expires. Use ` + "`" + `--scope` + "`" + ` to request permissions up front, such as ` + "`" + `--scope "read write"` + "`" + `, or omit it to choose them on the authorization screen. -If you work with a just one account, call ` + "`" + `doctl auth init` + "`" + ` and supply the token when prompted. This creates an authentication context named ` + "`" + `default` + "`" + `. +If you prefer a personal access token instead, call ` + "`" + `doctl auth init` + "`" + ` and supply the token when prompted. This creates an authentication context named ` + "`" + `default` + "`" + `. -To switch between multiple DigitalOcean accounts, including team accounts, create named contexts using ` + "`" + `doctl auth init --context ` + "`" + `, then providing the applicable token when prompted. This saves the token under the name you provide. To switch between contexts, use ` + "`" + `doctl auth switch --context ` + "`" + `. +To switch between multiple DigitalOcean accounts, including team accounts, create named contexts using ` + "`" + `doctl auth login --context ` + "`" + ` or ` + "`" + `doctl auth init --context ` + "`" + `, then providing the applicable credentials when prompted. This saves the credentials under the name you provide. To switch between contexts, use ` + "`" + `doctl auth switch --context ` + "`" + `. To remove accounts from the configuration file, run ` + "`" + `doctl auth remove --context ` + "`" + `. This removes the token under the name you provide.`, GroupID: configureDoctlGroup, @@ -120,21 +120,23 @@ You can use doctl without initializing it by adding the `+"`"+`--access-token`+" AddStringFlag(cmdAuthInit, doctl.ArgTokenValidationServer, "", TokenValidationServer, "The server used to validate a token") cmdAuthInit.Example = `The following example initializes doctl with a token for a single account with the context ` + "`" + `your-team` + "`" + `: doctl auth init --context your-team` - cmdAuthLogin := cmdBuilderWithInit(cmd, RunAuthLogin, "login", "Sign in to DigitalOcean in your browser", `This command signs doctl in to your DigitalOcean account using your browser, so you do not have to create and paste an API token. + cmdAuthLogin := cmdBuilderWithInit(cmd, RunAuthLogin, "login", "Sign in to DigitalOcean via OAuth 2.1 in your browser", `This command signs doctl in to your DigitalOcean account using your browser, so you do not have to create and paste an API token. -doctl uses the OAuth 2.1 authorization code flow with PKCE. The first time you run it, doctl registers itself with DigitalOcean as an application and saves the resulting client ID in its configuration file. Signing in then opens your browser, and once you approve the request, doctl saves the access token to your authentication context. +doctl uses the OAuth 2.1 authorization code flow with PKCE. It signs in as the doctl application DigitalOcean publishes, so there is nothing to register first. Signing in opens your browser, and once you approve the request, doctl saves the access token to your authentication context. Tokens issued this way are short-lived. doctl stores the accompanying refresh token and renews the access token automatically, so you can keep using doctl without returning to the control panel. -By default doctl does not request any particular scopes, so you choose the permissions to grant on the authorization screen. Use `+"`"+`--scope`+"`"+` to request them up front instead: `+"`"+`--scope "read write"`+"`"+` asks for the same access a full-permission API token has, and you can also name individual permissions, such as `+"`"+`--scope "droplet:read account:read"`+"`"+`. +By default doctl does not request any particular scopes, so you choose the permissions to grant on the authorization screen. Use `+"`"+`--scope`+"`"+` to request them up front instead: `+"`"+`--scope "read write"`+"`"+` asks for the same access a full-permission API token has, and you can also name individual permissions, such as `+"`"+`--scope "droplet:read account:read"`+"`"+`. Pass `+"`"+`--save-scope`+"`"+` with `+"`"+`--scope`+"`"+` to reuse those scopes on later logins; pass `+"`"+`--scope "" --save-scope`+"`"+` to clear the saved default. The `+"`"+`--context`+"`"+` flag signs in to a named authentication context instead of the default one, which lets you keep several accounts or teams side by side. To use an API token instead of your browser, see the help for `+"`"+`doctl auth init`+"`"+`.`, Writer, false) AddStringFlag(cmdAuthLogin, doctl.ArgOAuthServer, "", oauth.DefaultIssuer, "The OAuth authorization server to sign in to") - AddStringFlag(cmdAuthLogin, doctl.ArgOAuthScopes, "", defaultOAuthScopes, "A space-separated list of scopes to request, such as \"read write\" or \"droplet:read account:read\". When omitted, you choose the permissions to grant in your browser") + AddStringFlag(cmdAuthLogin, doctl.ArgOAuthClientID, "", oauth.DefaultClientID, "The OAuth application to sign in as. Defaults to the doctl application and only needs changing for a non-production authorization server") + AddStringFlag(cmdAuthLogin, doctl.ArgOAuthScopes, "", defaultOAuthScopes, "A space-separated list of scopes to request, such as \"read write\" or \"droplet:read account:read\". When omitted, uses any default saved with --save-scope; otherwise you choose the permissions to grant in your browser") + AddBoolFlag(cmdAuthLogin, doctl.ArgOAuthSaveScope, "", false, "Save --scope as the default for later logins. Use with --scope \"\" to clear the saved default") AddIntFlag(cmdAuthLogin, doctl.ArgOAuthCallbackPort, "", 0, "The local port to listen on for the authorization redirect. Defaults to an unused port") AddBoolFlag(cmdAuthLogin, doctl.ArgOAuthNoBrowser, "", false, "Print the authorization URL instead of opening a browser") AddDurationFlag(cmdAuthLogin, doctl.ArgOAuthTimeout, "", oauth.DefaultLoginTimeout, "How long to wait for you to authorize doctl in your browser") - cmdAuthLogin.Example = `The following example signs in to the context ` + "`" + `your-team` + "`" + ` and requests full read and write access: doctl auth login --context your-team --scope "read write"` + cmdAuthLogin.Example = `The following example signs in to the context ` + "`" + `your-team` + "`" + `, requests full read and write access, and saves those scopes for later logins: doctl auth login --context your-team --scope "read write" --save-scope` cmdAuthSwitch := cmdBuilderWithInit(cmd, RunAuthSwitch, "switch", "Switch between authentication contexts", `This command allows you to switch between authentication contexts you've already created. @@ -170,8 +172,6 @@ To switch between the contexts use `+"`"+`doctl auth switch --context `+"` To create new contexts, see the help for `+"`"+`doctl auth init`+"`"+`.`, Writer, false, aliasOpt("t")) cmdAuthToken.Example = `The following example displays the token of the current context: doctl auth token` - cmd.AddCommand(authClient()) - return cmd } diff --git a/commands/auth_client.go b/commands/auth_client.go deleted file mode 100644 index 22222d943..000000000 --- a/commands/auth_client.go +++ /dev/null @@ -1,61 +0,0 @@ -/* -Copyright 2018 The Doctl Authors All rights reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package commands - -import ( - "errors" - - "github.com/spf13/cobra" - - "github.com/digitalocean/doctl/commands/displayers" -) - -// authClient creates the commands for inspecting the OAuth application doctl -// registered for itself. -func authClient() *Command { - cmd := &Command{ - Command: &cobra.Command{ - Use: "client", - Short: "Display the OAuth application doctl registered for this machine", - Long: `The subcommands of ` + "`" + `doctl auth client` + "`" + ` describe the OAuth application doctl registered with DigitalOcean the first time you ran ` + "`" + `doctl auth login` + "`" + `. - -doctl registers one application per authorization server and reuses it for every authentication context, so this is machine-wide rather than per account.`, - }, - } - - cmdClientShow := cmdBuilderWithInit(cmd, RunAuthClientShow, "show", "Display the registered OAuth application", `Display the OAuth application doctl registered for itself, including its client ID and the redirect URIs it accepts. - -The credential that manages the registration is never printed. - -If no application has been registered yet, run `+"`"+`doctl auth login`+"`"+` to create one.`, Writer, false, aliasOpt("get"), displayerType(&displayers.OAuthClient{})) - cmdClientShow.Example = `The following example displays the registered application as JSON: doctl auth client show --output json` - - return cmd -} - -// RunAuthClientShow displays the stored dynamic client registration. -func RunAuthClientShow(c *CmdConfig) error { - client := loadOAuthClientState() - if client == nil { - return errors.New("doctl has not registered an OAuth application yet. Run `doctl auth login` to register one") - } - - return c.Display(&displayers.OAuthClient{ - ClientID: client.ClientID, - Issuer: client.Issuer, - RedirectURIs: client.RedirectURIs, - RegisteredAt: client.RegisteredAt, - ManagementURL: client.RegistrationClientURI, - }) -} diff --git a/commands/auth_client_test.go b/commands/auth_client_test.go deleted file mode 100644 index beaa1b682..000000000 --- a/commands/auth_client_test.go +++ /dev/null @@ -1,59 +0,0 @@ -/* -Copyright 2018 The Doctl Authors All rights reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package commands - -import ( - "bytes" - "context" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/digitalocean/doctl/internal/oauth" -) - -func TestRunAuthClientShow(t *testing.T) { - server := newTestAuthorizationServer(t, func() int { return 1 }) - defer server.Close() - - config, _ := newOAuthTestCmdConfig(t, server.URL) - out := &bytes.Buffer{} - config.Out = out - - stubOAuthLogin(t, func(context.Context, oauth.LoginOptions) (*oauth.Token, error) { - return &oauth.Token{AccessToken: "doo_v1_access"}, nil - }) - require.NoError(t, RunAuthLogin(config)) - - out.Reset() - require.NoError(t, RunAuthClientShow(config)) - - rendered := out.String() - assert.Contains(t, rendered, "client-1") - assert.Contains(t, rendered, server.URL) - assert.Contains(t, rendered, "http://127.0.0.1/callback") - assert.Contains(t, rendered, testClientIssuedAt.Format(time.RFC3339)) - assert.NotContains(t, rendered, "registration-token-1", "the credential that manages the registration is not display output") -} - -func TestRunAuthClientShowWithoutARegistration(t *testing.T) { - config, _ := newOAuthTestCmdConfig(t, "https://cloud.digitalocean.com") - - err := RunAuthClientShow(config) - - require.Error(t, err) - assert.Contains(t, err.Error(), "doctl auth login") -} diff --git a/commands/auth_login.go b/commands/auth_login.go index b8feb961e..9170a93ab 100644 --- a/commands/auth_login.go +++ b/commands/auth_login.go @@ -30,17 +30,12 @@ import ( ) const ( - // oauthClientConfigKey holds the dynamic client registration doctl shares - // across every authentication context on this machine. - oauthClientConfigKey = "oauth-client" // oauthTokensConfigKey holds the per-context OAuth session state used to // refresh access tokens. oauthTokensConfigKey = "oauth-tokens" - - // oauthClientName and oauthClientURI identify doctl on the authorization - // screen the user is shown. - oauthClientName = "doctl" - oauthClientURI = "https://github.com/digitalocean/doctl" + // oauthDefaultScopesConfigKey holds the optional default scopes reused on + // later logins when the user passed --save-scope. + oauthDefaultScopesConfigKey = "oauth-default-scopes" // defaultOAuthScopes is empty on purpose: when doctl sends no scope, the // authorization screen decides what the token is granted, which lets the @@ -60,18 +55,6 @@ const ( // so tests can exercise the command without a browser. var oauthLogin = oauth.Login -// oauthClientState is the dynamic client registration persisted in the config -// file. doctl registers itself once per authorization server and reuses the -// resulting public client for every login. -type oauthClientState struct { - Issuer string - ClientID string - RedirectURIs []string - RegisteredAt time.Time - RegistrationAccessToken string - RegistrationClientURI string -} - // oauthTokenState is the per-context OAuth session persisted in the config // file. The access token itself lives with the rest of the context's // credentials; only what is needed to renew it is kept here. @@ -102,15 +85,26 @@ func (s *oauthTokenState) needsRefresh(now time.Time) bool { func RunAuthLogin(c *CmdConfig) error { authContext := currentAuthContext() - issuer, err := c.Doit.GetString(c.NS, doctl.ArgOAuthServer) + issuer, err := oauthServer(c) + if err != nil { + return err + } + clientID, err := oauthClientID(c) if err != nil { return err } - scopes, err := c.Doit.GetString(c.NS, doctl.ArgOAuthScopes) + scopes, err := oauthScopes(c) if err != nil { return err } - port, err := c.Doit.GetInt(c.NS, doctl.ArgOAuthCallbackPort) + saveScope, err := oauthSaveScope(c) + if err != nil { + return err + } + if saveScope && !c.Doit.IsSet(doctl.ArgOAuthScopes) { + return errors.New("--save-scope requires --scope") + } + port, err := oauthCallbackPort(c) if err != nil { return err } @@ -118,47 +112,31 @@ func RunAuthLogin(c *CmdConfig) error { if err != nil { return err } - // writeConfig persists every viper setting, so a single --no-browser would - // otherwise stay true and keep the browser closed on later logins. - viper.Set(c.NS+"."+doctl.ArgOAuthNoBrowser, false) - timeout, err := c.Doit.GetDuration(c.NS, doctl.ArgOAuthTimeout) + timeout, err := oauthTimeout(c) if err != nil { return err } + // writeConfig persists every viper setting, so login-only flags from this + // invocation must not stick around for later runs. + clearEphemeralOAuthLoginFlags(c) ctx := context.Background() httpClient := &http.Client{Timeout: oauthHTTPTimeout} - metadata := oauth.ResolveServerMetadata(ctx, httpClient, issuer) - - client, registered, err := ensureOAuthClient(ctx, c, httpClient, metadata) - if err != nil { - return err - } + metadata := oauth.ServerMetadataFor(issuer) - opts := oauth.LoginOptions{ + token, err := oauthLogin(ctx, oauth.LoginOptions{ Metadata: metadata, - ClientID: client.ClientID, + ClientID: clientID, Scopes: strings.Fields(scopes), Port: port, Timeout: timeout, HTTPClient: httpClient, NoBrowser: noBrowser, OnAuthorizationURL: authorizationURLPrinter(c, noBrowser), - } - - token, err := oauthLogin(ctx, opts) - if errors.Is(err, oauth.ErrInvalidClient) && !registered { - // The stored registration is no longer valid, which happens when it - // was deleted server side. Register again and make one more attempt. - template.Render(c.Out, `{{nl}}{{warning "The saved OAuth application is no longer valid."}}{{nl}}`, nil) - - client, err = registerOAuthClient(ctx, c, httpClient, metadata) - if err != nil { - return err - } - - opts.ClientID = client.ClientID - token, err = oauthLogin(ctx, opts) + }) + if errors.Is(err, oauth.ErrInvalidClient) { + template.Render(c.Out, `{{error crossmark}}{{nl}}{{nl}}`, nil) + return fmt.Errorf("%s rejected the doctl application (client ID %s): %s", metadata.Issuer, clientID, err) } if err != nil { template.Render(c.Out, `{{error crossmark}}{{nl}}{{nl}}`, nil) @@ -176,72 +154,86 @@ func RunAuthLogin(c *CmdConfig) error { } else { storeOAuthTokenState(authContext, &oauthTokenState{ Issuer: metadata.Issuer, - ClientID: client.ClientID, + ClientID: clientID, TokenEndpoint: metadata.TokenEndpoint, RefreshToken: token.RefreshToken, Scope: token.Scope, ExpiresAt: token.Expiry, }) } + if saveScope { + storeOAuthDefaultScopes(scopes) + } if err := writeConfig(); err != nil { return err } displayOAuthLoginSummary(c, authContext, token) + if saveScope { + displayOAuthDefaultScopesSaved(c, scopes) + } return nil } -// ensureOAuthClient returns the dynamic client registration for the -// authorization server, registering doctl if this machine does not have one -// yet. The second return value reports whether a new client was registered. -func ensureOAuthClient(ctx context.Context, c *CmdConfig, httpClient *http.Client, metadata *oauth.ServerMetadata) (*oauthClientState, bool, error) { - if existing := loadOAuthClientState(); existing != nil && existing.Issuer == metadata.Issuer { - return existing, false, nil - } - - client, err := registerOAuthClient(ctx, c, httpClient, metadata) - if err != nil { - return nil, false, err +// oauthServer returns the authorization server for this invocation. +// A value left in the config file from an earlier login is ignored. +func oauthServer(c *CmdConfig) (string, error) { + if !c.Doit.IsSet(doctl.ArgOAuthServer) { + return oauth.DefaultIssuer, nil } - - return client, true, nil + return c.Doit.GetString(c.NS, doctl.ArgOAuthServer) } -func registerOAuthClient(ctx context.Context, c *CmdConfig, httpClient *http.Client, metadata *oauth.ServerMetadata) (*oauthClientState, error) { - template.Render(c.Out, `Registering doctl with {{highlight .}}... `, metadata.Issuer) +// oauthClientID returns the OAuth application doctl signs in as. It is the +// application DigitalOcean registered for doctl unless --client-id names +// another one, which is what a non-production authorization server needs. +func oauthClientID(c *CmdConfig) (string, error) { + if !c.Doit.IsSet(doctl.ArgOAuthClientID) { + return oauth.DefaultClientID, nil + } - registration, err := oauth.RegisterClient( - ctx, httpClient, metadata.RegistrationEndpoint, - oauth.RegistrationRedirectURIs(), oauthClientName, oauthClientURI, - ) + clientID, err := c.Doit.GetString(c.NS, doctl.ArgOAuthClientID) if err != nil { - template.Render(c.Out, `{{error crossmark}}{{nl}}{{nl}}`, nil) - return nil, err + return "", err + } + if strings.TrimSpace(clientID) == "" { + return "", errors.New("--client-id cannot be empty") } - template.Render(c.Out, `{{success checkmark}}{{nl}}`, nil) + return clientID, nil +} - client := &oauthClientState{ - Issuer: metadata.Issuer, - ClientID: registration.ClientID, - RedirectURIs: registration.RedirectURIs, - RegistrationAccessToken: registration.RegistrationAccessToken, - RegistrationClientURI: registration.RegistrationClientURI, +// oauthScopes returns the scopes requested for this invocation. An explicit +// --scope wins; otherwise any default saved with --save-scope is used. A bare +// --scope value left in the config file from an earlier login is ignored. +func oauthScopes(c *CmdConfig) (string, error) { + if c.Doit.IsSet(doctl.ArgOAuthScopes) { + return c.Doit.GetString(c.NS, doctl.ArgOAuthScopes) } - if registration.ClientIDIssuedAt > 0 { - client.RegisteredAt = time.Unix(registration.ClientIDIssuedAt, 0).UTC() + if saved := loadOAuthDefaultScopes(); saved != "" { + return saved, nil } - storeOAuthClientState(client) + return defaultOAuthScopes, nil +} - // Persist the registration immediately so an interrupted login does not - // leave an orphaned application behind on the next attempt. - if err := writeConfig(); err != nil { - return nil, err +// oauthSaveScope reports whether this invocation passed --save-scope. +// A value left in the config file from an earlier login is ignored. +func oauthSaveScope(c *CmdConfig) (bool, error) { + if !c.Doit.IsSet(doctl.ArgOAuthSaveScope) { + return false, nil } + return c.Doit.GetBool(c.NS, doctl.ArgOAuthSaveScope) +} - return client, nil +// oauthCallbackPort returns the local callback port for this invocation. +// A value left in the config file from an earlier login is ignored. +func oauthCallbackPort(c *CmdConfig) (int, error) { + if !c.Doit.IsSet(doctl.ArgOAuthCallbackPort) { + return 0, nil + } + return c.Doit.GetInt(c.NS, doctl.ArgOAuthCallbackPort) } // oauthNoBrowser reports whether this invocation passed --no-browser. @@ -253,6 +245,42 @@ func oauthNoBrowser(c *CmdConfig) (bool, error) { return c.Doit.GetBool(c.NS, doctl.ArgOAuthNoBrowser) } +// oauthTimeout returns how long to wait for browser authorization. +// A value left in the config file from an earlier login is ignored. +func oauthTimeout(c *CmdConfig) (time.Duration, error) { + if !c.Doit.IsSet(doctl.ArgOAuthTimeout) { + return oauth.DefaultLoginTimeout, nil + } + return c.Doit.GetDuration(c.NS, doctl.ArgOAuthTimeout) +} + +// clearEphemeralOAuthLoginFlags resets login-only flags so writeConfig does +// not persist them into the doctl configuration file. +func clearEphemeralOAuthLoginFlags(c *CmdConfig) { + viper.Set(c.NS+"."+doctl.ArgOAuthServer, oauth.DefaultIssuer) + viper.Set(c.NS+"."+doctl.ArgOAuthClientID, oauth.DefaultClientID) + viper.Set(c.NS+"."+doctl.ArgOAuthScopes, defaultOAuthScopes) + viper.Set(c.NS+"."+doctl.ArgOAuthSaveScope, false) + viper.Set(c.NS+"."+doctl.ArgOAuthCallbackPort, 0) + viper.Set(c.NS+"."+doctl.ArgOAuthNoBrowser, false) + viper.Set(c.NS+"."+doctl.ArgOAuthTimeout, oauth.DefaultLoginTimeout) +} + +// loadOAuthDefaultScopes returns the scopes saved with --save-scope, if any. +func loadOAuthDefaultScopes() string { + return viper.GetString(oauthDefaultScopesConfigKey) +} + +// storeOAuthDefaultScopes persists the default scopes for later logins. An +// empty value clears any previously saved default. +func storeOAuthDefaultScopes(scopes string) { + if scopes == "" { + viper.Set(oauthDefaultScopesConfigKey, nil) + return + } + viper.Set(oauthDefaultScopesConfigKey, scopes) +} + func authorizationURLPrinter(c *CmdConfig, noBrowser bool) func(string) { return func(authURL string) { if noBrowser { @@ -286,6 +314,14 @@ func displayOAuthLoginSummary(c *CmdConfig, authContext string, token *oauth.Tok } } +func displayOAuthDefaultScopesSaved(c *CmdConfig, scopes string) { + if scopes == "" { + template.Render(c.Out, `{{muted "Cleared the saved default scopes."}}{{nl}}`, nil) + return + } + template.Render(c.Out, `{{muted "Saved default scopes for later logins:"}} {{highlight .}}{{nl}}`, scopes) +} + // refreshExpiredOAuthToken renews the current context's access token when it // was obtained with doctl auth login and has expired. It is a no-op for // contexts authenticated with a personal access token. @@ -302,7 +338,7 @@ func refreshExpiredOAuthToken(c *CmdConfig) error { endpoint := state.TokenEndpoint if endpoint == "" { - endpoint = oauth.ResolveServerMetadata(context.Background(), nil, state.Issuer).TokenEndpoint + endpoint = oauth.ServerMetadataFor(state.Issuer).TokenEndpoint } httpClient := &http.Client{Timeout: oauthHTTPTimeout} @@ -342,47 +378,6 @@ func currentAuthContext() string { return authContext } -func loadOAuthClientState() *oauthClientState { - values := viper.GetStringMap(oauthClientConfigKey) - if len(values) == 0 { - return nil - } - - client := &oauthClientState{ - Issuer: configMapString(values, "issuer"), - ClientID: configMapString(values, "client-id"), - RedirectURIs: configMapStringSlice(values, "redirect-uris"), - RegistrationAccessToken: configMapString(values, "registration-access-token"), - RegistrationClientURI: configMapString(values, "registration-client-uri"), - } - if registeredAt := configMapString(values, "registered-at"); registeredAt != "" { - if parsed, err := time.Parse(time.RFC3339, registeredAt); err == nil { - client.RegisteredAt = parsed - } - } - if client.ClientID == "" || client.Issuer == "" { - return nil - } - - return client -} - -func storeOAuthClientState(client *oauthClientState) { - var registeredAt string - if !client.RegisteredAt.IsZero() { - registeredAt = client.RegisteredAt.UTC().Format(time.RFC3339) - } - - viper.Set(oauthClientConfigKey, map[string]any{ - "issuer": client.Issuer, - "client-id": client.ClientID, - "redirect-uris": client.RedirectURIs, - "registered-at": registeredAt, - "registration-access-token": client.RegistrationAccessToken, - "registration-client-uri": client.RegistrationClientURI, - }) -} - func loadOAuthTokenState(authContext string) *oauthTokenState { values, ok := oauthTokenStates()[strings.ToLower(authContext)].(map[string]any) if !ok || len(values) == 0 { @@ -466,26 +461,6 @@ func oauthTokenStates() map[string]any { return states } -// configMapStringSlice reads a list of strings, which viper hands back as -// []any when it comes from the YAML file and as []string when it was set in -// this process. -func configMapStringSlice(values map[string]any, key string) []string { - switch typed := values[key].(type) { - case []string: - return typed - case []any: - items := make([]string, 0, len(typed)) - for _, item := range typed { - if s, ok := item.(string); ok { - items = append(items, s) - } - } - return items - default: - return nil - } -} - func configMapString(values map[string]any, key string) string { value, ok := values[key] if !ok { diff --git a/commands/auth_login_test.go b/commands/auth_login_test.go index 11a4631e2..24783f304 100644 --- a/commands/auth_login_test.go +++ b/commands/auth_login_test.go @@ -19,7 +19,6 @@ import ( "io" "net/http" "net/http/httptest" - "strconv" "testing" "time" @@ -34,11 +33,7 @@ import ( ) func TestRunAuthLogin(t *testing.T) { - var registrations int - server := newTestAuthorizationServer(t, func() int { registrations++; return registrations }) - defer server.Close() - - config, token := newOAuthTestCmdConfig(t, server.URL) + config, token := newOAuthTestCmdConfig(t, "https://cloud.example.com") var capturedOpts oauth.LoginOptions stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { @@ -54,35 +49,99 @@ func TestRunAuthLogin(t *testing.T) { require.NoError(t, RunAuthLogin(config)) - assert.Equal(t, 1, registrations) assert.Equal(t, "doo_v1_access", *token) - assert.Equal(t, "client-1", capturedOpts.ClientID) + assert.Equal(t, oauth.DefaultClientID, capturedOpts.ClientID, "doctl signs in as its own published application") assert.Empty(t, capturedOpts.Scopes, "without --scope the authorization screen decides the permissions") - assert.Equal(t, server.URL+"/v1/oauth/token", capturedOpts.Metadata.TokenEndpoint) + assert.Equal(t, "https://cloud.example.com/v1/oauth/token", capturedOpts.Metadata.TokenEndpoint) assert.False(t, capturedOpts.NoBrowser) - client := loadOAuthClientState() - require.NotNil(t, client) - assert.Equal(t, "client-1", client.ClientID) - assert.Equal(t, server.URL, client.Issuer) - assert.Equal(t, "registration-token-1", client.RegistrationAccessToken) - assert.Equal(t, oauth.RegistrationRedirectURIs(), client.RedirectURIs) - assert.True(t, testClientIssuedAt.Equal(client.RegisteredAt)) - state := loadOAuthTokenState(doctl.ArgDefaultContext) require.NotNil(t, state) assert.Equal(t, "dor_v1_refresh", state.RefreshToken) - assert.Equal(t, "client-1", state.ClientID) - assert.Equal(t, server.URL+"/v1/oauth/token", state.TokenEndpoint) + assert.Equal(t, oauth.DefaultClientID, state.ClientID) + assert.Equal(t, "https://cloud.example.com/v1/oauth/token", state.TokenEndpoint) assert.False(t, state.ExpiresAt.IsZero()) } -func TestRunAuthLoginNoBrowserAppliesOnlyToThisInvocation(t *testing.T) { - server := newTestAuthorizationServer(t, func() int { return 1 }) +func TestRunAuthLoginDoesNotCallTheAuthorizationServerBeforeTheBrowser(t *testing.T) { + var requests []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests = append(requests, r.URL.Path) + http.NotFound(w, r) + })) defer server.Close() config, _ := newOAuthTestCmdConfig(t, server.URL) + stubOAuthLogin(t, func(_ context.Context, _ oauth.LoginOptions) (*oauth.Token, error) { + return &oauth.Token{AccessToken: "doo_v1_access"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + + assert.Empty(t, requests, "endpoints are derived locally, so no metadata or registration request is made") +} + +func TestRunAuthLoginHonorsClientIDFlag(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") + config.Doit.Set(config.NS, doctl.ArgOAuthClientID, "staging-client") + + var capturedOpts oauth.LoginOptions + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + capturedOpts = opts + return &oauth.Token{AccessToken: "doo_v1_access", RefreshToken: "dor_v1_refresh"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + + assert.Equal(t, "staging-client", capturedOpts.ClientID) + assert.Equal(t, "staging-client", loadOAuthTokenState(doctl.ArgDefaultContext).ClientID) + assert.Equal(t, oauth.DefaultClientID, viper.Get(config.NS+"."+doctl.ArgOAuthClientID)) +} + +func TestRunAuthLoginClientIDAppliesOnlyToThisInvocation(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") + // Left behind by an earlier `doctl auth login --client-id staging-client`, + // which writeConfig saves with the rest of the settings. + config.Doit.Set(config.NS, doctl.ArgOAuthClientID, "staging-client") + config.Doit.(*doctl.TestConfig).IsSetMap[doctl.ArgOAuthClientID] = false + + var capturedOpts oauth.LoginOptions + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + capturedOpts = opts + return &oauth.Token{AccessToken: "doo_v1_access"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + + assert.Equal(t, oauth.DefaultClientID, capturedOpts.ClientID) +} + +func TestRunAuthLoginRejectsAnEmptyClientID(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") + config.Doit.Set(config.NS, doctl.ArgOAuthClientID, "") + + err := RunAuthLogin(config) + + require.Error(t, err) + assert.Contains(t, err.Error(), "--client-id cannot be empty") +} + +func TestRunAuthLoginExplainsARejectedClient(t *testing.T) { + config, token := newOAuthTestCmdConfig(t, "https://cloud.example.com") + stubOAuthLogin(t, func(_ context.Context, _ oauth.LoginOptions) (*oauth.Token, error) { + return nil, &oauth.AuthorizationError{Code: "invalid_client"} + }) + + err := RunAuthLogin(config) + + require.Error(t, err) + assert.Contains(t, err.Error(), oauth.DefaultClientID) + assert.Empty(t, *token) +} + +func TestRunAuthLoginNoBrowserAppliesOnlyToThisInvocation(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") // Left behind by an earlier `doctl auth login --no-browser`, which // writeConfig saves with the rest of the settings. config.Doit.Set(config.NS, doctl.ArgOAuthNoBrowser, true) @@ -100,10 +159,7 @@ func TestRunAuthLoginNoBrowserAppliesOnlyToThisInvocation(t *testing.T) { } func TestRunAuthLoginHonorsNoBrowserFlag(t *testing.T) { - server := newTestAuthorizationServer(t, func() int { return 1 }) - defer server.Close() - - config, _ := newOAuthTestCmdConfig(t, server.URL) + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") config.Doit.Set(config.NS, doctl.ArgOAuthNoBrowser, true) var capturedOpts oauth.LoginOptions @@ -117,10 +173,7 @@ func TestRunAuthLoginHonorsNoBrowserFlag(t *testing.T) { } func TestRunAuthLoginRequestsTheGivenScopes(t *testing.T) { - server := newTestAuthorizationServer(t, func() int { return 1 }) - defer server.Close() - - config, _ := newOAuthTestCmdConfig(t, server.URL) + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") config.Doit.Set(config.NS, doctl.ArgOAuthScopes, "read write") var capturedOpts oauth.LoginOptions @@ -132,57 +185,97 @@ func TestRunAuthLoginRequestsTheGivenScopes(t *testing.T) { require.NoError(t, RunAuthLogin(config)) assert.Equal(t, []string{"read", "write"}, capturedOpts.Scopes) + assert.Equal(t, defaultOAuthScopes, viper.Get(config.NS+"."+doctl.ArgOAuthScopes)) } -func TestRunAuthLoginReusesTheRegisteredClient(t *testing.T) { - var registrations int - server := newTestAuthorizationServer(t, func() int { registrations++; return registrations }) - defer server.Close() - - config, _ := newOAuthTestCmdConfig(t, server.URL) - storeOAuthClientState(&oauthClientState{Issuer: server.URL, ClientID: "already-registered"}) +func TestRunAuthLoginScopeAppliesOnlyToThisInvocation(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") + // Left behind by an earlier `doctl auth login --scope "read write"`, which + // writeConfig saves with the rest of the settings. + config.Doit.Set(config.NS, doctl.ArgOAuthScopes, "read write") + config.Doit.(*doctl.TestConfig).IsSetMap[doctl.ArgOAuthScopes] = false - var clientID string + var capturedOpts oauth.LoginOptions stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { - clientID = opts.ClientID - return &oauth.Token{AccessToken: "doo_v1_access"}, nil + capturedOpts = opts + return &oauth.Token{AccessToken: "doo_v1_access", RefreshToken: "dor_v1_refresh"}, nil }) require.NoError(t, RunAuthLogin(config)) + assert.Empty(t, capturedOpts.Scopes, "a saved --scope must not be reused on later logins") + assert.Equal(t, defaultOAuthScopes, viper.Get(config.NS+"."+doctl.ArgOAuthScopes)) +} + +func TestRunAuthLoginSaveScopePersistsDefault(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") + config.Doit.Set(config.NS, doctl.ArgOAuthScopes, "read write") + config.Doit.Set(config.NS, doctl.ArgOAuthSaveScope, true) - assert.Zero(t, registrations, "an existing registration should be reused") - assert.Equal(t, "already-registered", clientID) + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + assert.Equal(t, []string{"read", "write"}, opts.Scopes) + return &oauth.Token{AccessToken: "doo_v1_access", RefreshToken: "dor_v1_refresh"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + assert.Equal(t, "read write", loadOAuthDefaultScopes()) + assert.Equal(t, false, viper.Get(config.NS+"."+doctl.ArgOAuthSaveScope)) } -func TestRunAuthLoginRegistersAgainWhenTheClientIsRejected(t *testing.T) { - var registrations int - server := newTestAuthorizationServer(t, func() int { registrations++; return registrations }) - defer server.Close() +func TestRunAuthLoginUsesSavedDefaultScopes(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") + storeOAuthDefaultScopes("droplet:read account:read") - config, token := newOAuthTestCmdConfig(t, server.URL) - storeOAuthClientState(&oauthClientState{Issuer: server.URL, ClientID: "deleted-client"}) + var capturedOpts oauth.LoginOptions + stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { + capturedOpts = opts + return &oauth.Token{AccessToken: "doo_v1_access", RefreshToken: "dor_v1_refresh"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + assert.Equal(t, []string{"droplet:read", "account:read"}, capturedOpts.Scopes) +} - var attempts []string +func TestRunAuthLoginExplicitScopeOverridesSavedDefault(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") + storeOAuthDefaultScopes("read write") + config.Doit.Set(config.NS, doctl.ArgOAuthScopes, "droplet:read") + + var capturedOpts oauth.LoginOptions stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { - attempts = append(attempts, opts.ClientID) - if opts.ClientID == "deleted-client" { - return nil, &oauth.AuthorizationError{Code: "invalid_client"} - } + capturedOpts = opts return &oauth.Token{AccessToken: "doo_v1_access"}, nil }) require.NoError(t, RunAuthLogin(config)) + assert.Equal(t, []string{"droplet:read"}, capturedOpts.Scopes) + assert.Equal(t, "read write", loadOAuthDefaultScopes(), "override without --save-scope must leave the default alone") +} - assert.Equal(t, []string{"deleted-client", "client-1"}, attempts) - assert.Equal(t, 1, registrations) - assert.Equal(t, "doo_v1_access", *token) +func TestRunAuthLoginSaveScopeClearsDefault(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") + storeOAuthDefaultScopes("read write") + config.Doit.Set(config.NS, doctl.ArgOAuthScopes, "") + config.Doit.Set(config.NS, doctl.ArgOAuthSaveScope, true) + + stubOAuthLogin(t, func(_ context.Context, _ oauth.LoginOptions) (*oauth.Token, error) { + return &oauth.Token{AccessToken: "doo_v1_access"}, nil + }) + + require.NoError(t, RunAuthLogin(config)) + assert.Empty(t, loadOAuthDefaultScopes()) } -func TestRunAuthLoginFailure(t *testing.T) { - server := newTestAuthorizationServer(t, func() int { return 1 }) - defer server.Close() +func TestRunAuthLoginSaveScopeRequiresScopeFlag(t *testing.T) { + config, _ := newOAuthTestCmdConfig(t, "https://cloud.example.com") + config.Doit.Set(config.NS, doctl.ArgOAuthSaveScope, true) - config, token := newOAuthTestCmdConfig(t, server.URL) + err := RunAuthLogin(config) + require.Error(t, err) + assert.Contains(t, err.Error(), "--save-scope requires --scope") +} + +func TestRunAuthLoginFailure(t *testing.T) { + config, token := newOAuthTestCmdConfig(t, "https://cloud.example.com") stubOAuthLogin(t, func(_ context.Context, opts oauth.LoginOptions) (*oauth.Token, error) { return nil, &oauth.AuthorizationError{Code: "access_denied"} }) @@ -349,55 +442,6 @@ func TestOAuthTokenStateNeedsRefresh(t *testing.T) { } } -// testClientIssuedAt is the registration timestamp the test authorization -// server reports. -var testClientIssuedAt = time.Date(2026, time.September, 22, 10, 30, 0, 0, time.UTC) - -// newTestAuthorizationServer serves the metadata and dynamic client -// registration endpoints doctl calls before starting the browser flow. -func newTestAuthorizationServer(t *testing.T, nextClient func() int) *httptest.Server { - t.Helper() - - server := httptest.NewUnstartedServer(nil) - mux := http.NewServeMux() - mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, r *http.Request) { - issuer := "http://" + r.Host - writeTestJSON(t, w, http.StatusOK, map[string]any{ - "issuer": issuer, - "authorization_endpoint": issuer + "/v1/oauth/authorize", - "token_endpoint": issuer + "/v1/oauth/token", - "registration_endpoint": issuer + "/v1/oauth/register", - "revocation_endpoint": issuer + "/v1/oauth/revoke", - }) - }) - mux.HandleFunc("/v1/oauth/register", func(w http.ResponseWriter, r *http.Request) { - var request struct { - RedirectURIs []string `json:"redirect_uris"` - } - require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) - - id := strconv.Itoa(nextClient()) - writeTestJSON(t, w, http.StatusCreated, map[string]any{ - "client_id": "client-" + id, - "client_id_issued_at": testClientIssuedAt.Unix(), - "redirect_uris": request.RedirectURIs, - "registration_access_token": "registration-token-" + id, - }) - }) - server.Config.Handler = mux - server.Start() - - return server -} - -func writeTestJSON(t *testing.T, w http.ResponseWriter, status int, body any) { - t.Helper() - - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - require.NoError(t, json.NewEncoder(w).Encode(body)) -} - // newOAuthTestCmdConfig returns a command config wired to an isolated view of // the doctl configuration, along with a pointer to the access token the // command stores for the current context. @@ -431,25 +475,38 @@ func resetOAuthConfig(t *testing.T) { previousWriter := cfgFileWriter previousContext := Context - previousClient := viper.Get(oauthClientConfigKey) previousTokens := viper.Get(oauthTokensConfigKey) + previousDefaultScopes := viper.Get(oauthDefaultScopesConfigKey) previousViperContext := viper.Get(doctl.ArgContext) - noBrowserKey := "test." + doctl.ArgOAuthNoBrowser - previousNoBrowser := viper.Get(noBrowserKey) + ephemeralKeys := []string{ + "test." + doctl.ArgOAuthServer, + "test." + doctl.ArgOAuthClientID, + "test." + doctl.ArgOAuthScopes, + "test." + doctl.ArgOAuthSaveScope, + "test." + doctl.ArgOAuthCallbackPort, + "test." + doctl.ArgOAuthNoBrowser, + "test." + doctl.ArgOAuthTimeout, + } + previousEphemeral := make(map[string]any, len(ephemeralKeys)) + for _, key := range ephemeralKeys { + previousEphemeral[key] = viper.Get(key) + } cfgFileWriter = func() (io.WriteCloser, error) { return &nopWriteCloser{Writer: io.Discard}, nil } Context = "" viper.Set(doctl.ArgContext, doctl.ArgDefaultContext) - viper.Set(oauthClientConfigKey, nil) viper.Set(oauthTokensConfigKey, nil) + viper.Set(oauthDefaultScopesConfigKey, nil) t.Cleanup(func() { cfgFileWriter = previousWriter Context = previousContext - viper.Set(oauthClientConfigKey, previousClient) viper.Set(oauthTokensConfigKey, previousTokens) + viper.Set(oauthDefaultScopesConfigKey, previousDefaultScopes) viper.Set(doctl.ArgContext, previousViperContext) - viper.Set(noBrowserKey, previousNoBrowser) + for key, value := range previousEphemeral { + viper.Set(key, value) + } }) } diff --git a/commands/auth_test.go b/commands/auth_test.go index 69ebf86f2..1b3f63f05 100644 --- a/commands/auth_test.go +++ b/commands/auth_test.go @@ -32,13 +32,7 @@ import ( func TestAuthCommand(t *testing.T) { cmd := Auth() assert.NotNil(t, cmd) - assertCommandNames(t, cmd, "client", "init", "list", "login", "remove", "switch", "token") -} - -func TestAuthClientCommand(t *testing.T) { - cmd := authClient() - assert.NotNil(t, cmd) - assertCommandNames(t, cmd, "show") + assertCommandNames(t, cmd, "init", "list", "login", "remove", "switch", "token") } func TestAuthInit(t *testing.T) { diff --git a/commands/displayers/oauth_client.go b/commands/displayers/oauth_client.go deleted file mode 100644 index bcfb48918..000000000 --- a/commands/displayers/oauth_client.go +++ /dev/null @@ -1,90 +0,0 @@ -/* -Copyright 2018 The Doctl Authors All rights reserved. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package displayers - -import ( - "io" - "strings" - "time" -) - -// OAuthClient renders the OAuth application doctl registered for itself. It -// never carries the registration access token: that credential manages the -// registration and is kept out of command output. -type OAuthClient struct { - ClientID string - Issuer string - RedirectURIs []string - RegisteredAt time.Time - ManagementURL string -} - -var _ Displayable = &OAuthClient{} - -func (c *OAuthClient) JSON(out io.Writer) error { - return writeJSON(struct { - ClientID string `json:"client_id"` - Issuer string `json:"issuer"` - RedirectURIs []string `json:"redirect_uris,omitempty"` - RegisteredAt string `json:"registered_at,omitempty"` - ManagementURL string `json:"registration_client_uri,omitempty"` - }{ - ClientID: c.ClientID, - Issuer: c.Issuer, - RedirectURIs: c.RedirectURIs, - RegisteredAt: c.registeredAt(), - ManagementURL: c.ManagementURL, - }, out) -} - -func (c *OAuthClient) Cols() []string { - return []string{"ClientID", "Issuer", "RedirectURIs", "RegisteredAt", "ManagementURL"} -} - -func (c *OAuthClient) ColMap() map[string]string { - return map[string]string{ - "ClientID": "Client ID", - "Issuer": "Issuer", - "RedirectURIs": "Redirect URIs", - "RegisteredAt": "Registered At", - "ManagementURL": "Management URL", - } -} - -func (c *OAuthClient) KV() []map[string]any { - return []map[string]any{{ - "ClientID": c.ClientID, - "Issuer": c.Issuer, - "RedirectURIs": c.redirectURIs(), - "RegisteredAt": c.registeredAt(), - "ManagementURL": c.ManagementURL, - }} -} - -// redirectURIs reports unknown rather than blank for a registration created -// before doctl recorded them, so an empty column is not mistaken for a client -// that accepts no redirects. -func (c *OAuthClient) redirectURIs() string { - if len(c.RedirectURIs) == 0 { - return "unknown" - } - return strings.Join(c.RedirectURIs, ", ") -} - -func (c *OAuthClient) registeredAt() string { - if c.RegisteredAt.IsZero() { - return "" - } - return c.RegisteredAt.UTC().Format(time.RFC3339) -} diff --git a/internal/oauth/login.go b/internal/oauth/login.go index b50b52104..40c19fa41 100644 --- a/internal/oauth/login.go +++ b/internal/oauth/login.go @@ -38,11 +38,11 @@ const callbackPath = "/callback" // finish authorizing in their browser. const DefaultLoginTimeout = 5 * time.Minute -// RegistrationRedirectURIs are the redirect URIs doctl registers for its -// dynamic client. Neither carries a port: the authorization server matches +// RedirectURIs are the redirect URIs the doctl OAuth application registers +// with DigitalOcean. Neither carries a port: the authorization server matches // loopback redirects on any port (RFC 8252 section 7.3), which lets doctl bind // an ephemeral port at login time. -func RegistrationRedirectURIs() []string { +func RedirectURIs() []string { return []string{ "http://127.0.0.1" + callbackPath, "http://localhost" + callbackPath, @@ -63,8 +63,8 @@ func (e *AuthorizationError) Error() string { return e.Code } -// Unwrap reports registration problems as ErrInvalidClient so callers can -// discard a stale dynamic client registration and try again. +// Unwrap reports a rejected client as ErrInvalidClient so callers can explain +// that the doctl application, rather than the user, was turned away. func (e *AuthorizationError) Unwrap() error { if isInvalidClientCode(e.Code) { return ErrInvalidClient diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go index f8affc0b3..0d27c23b4 100644 --- a/internal/oauth/oauth.go +++ b/internal/oauth/oauth.go @@ -13,7 +13,7 @@ limitations under the License. // Package oauth implements the client side of DigitalOcean's OAuth 2.1 // authorization code flow with PKCE (RFC 7636) for native applications -// (RFC 8252), including dynamic client registration (RFC 7591). +// (RFC 8252). package oauth import ( @@ -32,173 +32,58 @@ const ( // DefaultIssuer is the DigitalOcean authorization server. DefaultIssuer = "https://cloud.digitalocean.com" + // DefaultClientID identifies the doctl OAuth application registered with + // DigitalOcean. doctl is a public client (RFC 8252): it ships to users' + // machines, so it holds no client secret and this identifier is not a + // secret either. PKCE, not client authentication, is what binds an + // authorization code to the doctl process that requested it. Embedding the + // client ID in the binary is the same approach the GitHub CLI, gcloud, and + // the Azure CLI take. + DefaultClientID = "2446a57f71830bf88511fa3e3920d471124084d06b8befc24a4e406eaaeac170" + // CodeChallengeMethodS256 is the only PKCE code challenge method the // authorization server supports. CodeChallengeMethodS256 = "S256" - // TokenEndpointAuthMethodNone designates a public client that - // authenticates at the token endpoint with PKCE instead of a secret. - TokenEndpointAuthMethodNone = "none" - grantTypeAuthorizationCode = "authorization_code" grantTypeRefreshToken = "refresh_token" responseTypeCode = "code" - metadataPath = "/.well-known/oauth-authorization-server" - authorizePath = "/v1/oauth/authorize" - tokenPath = "/v1/oauth/token" - registrationPath = "/v1/oauth/register" - revocationPath = "/v1/oauth/revoke" + authorizePath = "/v1/oauth/authorize" + tokenPath = "/v1/oauth/token" + revocationPath = "/v1/oauth/revoke" formContentType = "application/x-www-form-urlencoded" ) // ErrInvalidClient is returned when the authorization server rejects the -// client we are registered as. Callers can use it to discard a stale dynamic -// registration and register again. -var ErrInvalidClient = errors.New("the authorization server rejected this client registration") +// OAuth application doctl signs in as. +var ErrInvalidClient = errors.New("the authorization server rejected this client") -// ServerMetadata is the subset of the RFC 8414 authorization server metadata -// document that doctl uses. +// ServerMetadata describes the authorization server endpoints doctl uses. type ServerMetadata struct { - Issuer string `json:"issuer"` - AuthorizationEndpoint string `json:"authorization_endpoint"` - TokenEndpoint string `json:"token_endpoint"` - RegistrationEndpoint string `json:"registration_endpoint"` - RevocationEndpoint string `json:"revocation_endpoint"` - GrantTypesSupported []string `json:"grant_types_supported"` - CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported"` + Issuer string + AuthorizationEndpoint string + TokenEndpoint string + RevocationEndpoint string } -// ResolveServerMetadata fetches the authorization server metadata document for -// issuer. The document is advisory: any endpoint the server does not advertise -// falls back to its well-known DigitalOcean path, so a server that does not -// serve the metadata document is still usable. -func ResolveServerMetadata(ctx context.Context, client *http.Client, issuer string) *ServerMetadata { +// ServerMetadataFor returns the endpoints for issuer. DigitalOcean's paths are +// stable and doctl is only ever pointed at a DigitalOcean authorization +// server, so the endpoints are derived locally rather than discovered over the +// network: there is no metadata request to make, fail, or have redirected. +func ServerMetadataFor(issuer string) *ServerMetadata { issuer = strings.TrimSuffix(strings.TrimSpace(issuer), "/") if issuer == "" { issuer = DefaultIssuer } - md := &ServerMetadata{Issuer: issuer} - if discovered, err := discoverServerMetadata(ctx, client, issuer); err == nil { - md = discovered - } - - if md.Issuer == "" { - md.Issuer = issuer - } - if md.AuthorizationEndpoint == "" { - md.AuthorizationEndpoint = issuer + authorizePath - } - if md.TokenEndpoint == "" { - md.TokenEndpoint = issuer + tokenPath - } - if md.RegistrationEndpoint == "" { - md.RegistrationEndpoint = issuer + registrationPath - } - if md.RevocationEndpoint == "" { - md.RevocationEndpoint = issuer + revocationPath - } - - return md -} - -func discoverServerMetadata(ctx context.Context, client *http.Client, issuer string) (*ServerMetadata, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, issuer+metadataPath, nil) - if err != nil { - return nil, err - } - req.Header.Set("Accept", "application/json") - - resp, err := doRequest(client, req) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("authorization server metadata request returned %s", resp.Status) + return &ServerMetadata{ + Issuer: issuer, + AuthorizationEndpoint: issuer + authorizePath, + TokenEndpoint: issuer + tokenPath, + RevocationEndpoint: issuer + revocationPath, } - - md := &ServerMetadata{} - if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(md); err != nil { - return nil, err - } - - return md, nil -} - -// ClientRegistration is an RFC 7591 client information response for a -// dynamically registered public client. -type ClientRegistration struct { - ClientID string `json:"client_id"` - ClientIDIssuedAt int64 `json:"client_id_issued_at,omitempty"` - RedirectURIs []string `json:"redirect_uris,omitempty"` - TokenEndpointAuthMethod string `json:"token_endpoint_auth_method,omitempty"` - GrantTypes []string `json:"grant_types,omitempty"` - ResponseTypes []string `json:"response_types,omitempty"` - ClientName string `json:"client_name,omitempty"` - ClientURI string `json:"client_uri,omitempty"` - RegistrationAccessToken string `json:"registration_access_token,omitempty"` - RegistrationClientURI string `json:"registration_client_uri,omitempty"` -} - -type registerClientRequest struct { - RedirectURIs []string `json:"redirect_uris"` - TokenEndpointAuthMethod string `json:"token_endpoint_auth_method"` - GrantTypes []string `json:"grant_types"` - ResponseTypes []string `json:"response_types"` - ClientName string `json:"client_name,omitempty"` - ClientURI string `json:"client_uri,omitempty"` -} - -// RegisterClient performs RFC 7591 dynamic client registration against -// endpoint, registering a public client that authenticates with PKCE. The -// endpoint is unauthenticated, so no token is required. -func RegisterClient(ctx context.Context, client *http.Client, endpoint string, redirectURIs []string, name, uri string) (*ClientRegistration, error) { - if len(redirectURIs) == 0 { - return nil, errors.New("at least one redirect URI is required to register a client") - } - - body, err := json.Marshal(®isterClientRequest{ - RedirectURIs: redirectURIs, - TokenEndpointAuthMethod: TokenEndpointAuthMethodNone, - GrantTypes: []string{grantTypeAuthorizationCode, grantTypeRefreshToken}, - ResponseTypes: []string{responseTypeCode}, - ClientName: name, - ClientURI: uri, - }) - if err != nil { - return nil, err - } - - req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(string(body))) - if err != nil { - return nil, err - } - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Accept", "application/json") - - resp, err := doRequest(client, req) - if err != nil { - return nil, fmt.Errorf("registering doctl as an OAuth application: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { - return nil, responseError(resp, "registering doctl as an OAuth application") - } - - registration := &ClientRegistration{} - if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(registration); err != nil { - return nil, fmt.Errorf("decoding client registration response: %w", err) - } - if registration.ClientID == "" { - return nil, errors.New("the authorization server did not return a client ID") - } - - return registration, nil } // Token is an access token grant issued by the authorization server. diff --git a/internal/oauth/oauth_test.go b/internal/oauth/oauth_test.go index 181432711..8a9ee8b84 100644 --- a/internal/oauth/oauth_test.go +++ b/internal/oauth/oauth_test.go @@ -25,104 +25,24 @@ import ( "github.com/stretchr/testify/require" ) -func TestResolveServerMetadata(t *testing.T) { - t.Run("uses the advertised endpoints", func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, metadataPath, r.URL.Path) - writeJSON(t, w, http.StatusOK, map[string]any{ - "issuer": "https://cloud.digitalocean.com", - "authorization_endpoint": "https://cloud.digitalocean.com/v1/oauth/authorize", - "token_endpoint": "https://cloud.digitalocean.com/v1/oauth/token", - "registration_endpoint": "https://cloud.digitalocean.com/v1/oauth/register", - "revocation_endpoint": "https://cloud.digitalocean.com/v1/oauth/revoke", - "code_challenge_methods_supported": []string{"S256"}, - }) - })) - defer server.Close() - - md := ResolveServerMetadata(context.Background(), server.Client(), server.URL) - - assert.Equal(t, "https://cloud.digitalocean.com", md.Issuer) - assert.Equal(t, "https://cloud.digitalocean.com/v1/oauth/token", md.TokenEndpoint) - assert.Equal(t, []string{"S256"}, md.CodeChallengeMethodsSupported) - }) - - t.Run("falls back to the well-known paths", func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.NotFound(w, r) - })) - defer server.Close() - - md := ResolveServerMetadata(context.Background(), server.Client(), server.URL+"/") - - assert.Equal(t, server.URL, md.Issuer) - assert.Equal(t, server.URL+authorizePath, md.AuthorizationEndpoint) - assert.Equal(t, server.URL+tokenPath, md.TokenEndpoint) - assert.Equal(t, server.URL+registrationPath, md.RegistrationEndpoint) - assert.Equal(t, server.URL+revocationPath, md.RevocationEndpoint) +func TestServerMetadataFor(t *testing.T) { + t.Run("derives the endpoints from the issuer", func(t *testing.T) { + md := ServerMetadataFor("https://cloud.example.com/") + + assert.Equal(t, "https://cloud.example.com", md.Issuer) + assert.Equal(t, "https://cloud.example.com"+authorizePath, md.AuthorizationEndpoint) + assert.Equal(t, "https://cloud.example.com"+tokenPath, md.TokenEndpoint) + assert.Equal(t, "https://cloud.example.com"+revocationPath, md.RevocationEndpoint) }) t.Run("defaults to the DigitalOcean issuer", func(t *testing.T) { - md := ResolveServerMetadata(context.Background(), failingClient(t), "") + md := ServerMetadataFor(" ") assert.Equal(t, DefaultIssuer, md.Issuer) assert.Equal(t, DefaultIssuer+tokenPath, md.TokenEndpoint) }) } -func TestRegisterClient(t *testing.T) { - t.Run("registers a public client", func(t *testing.T) { - var body registerClientRequest - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) - writeJSON(t, w, http.StatusCreated, map[string]any{ - "client_id": "client-id", - "redirect_uris": body.RedirectURIs, - "registration_access_token": "registration-token", - "registration_client_uri": "https://cloud.digitalocean.com/v1/oauth/register/client-id", - "token_endpoint_auth_method": TokenEndpointAuthMethodNone, - }) - })) - defer server.Close() - - registration, err := RegisterClient( - context.Background(), server.Client(), server.URL, - RegistrationRedirectURIs(), "doctl", "https://example.com", - ) - require.NoError(t, err) - - assert.Equal(t, "client-id", registration.ClientID) - assert.Equal(t, "registration-token", registration.RegistrationAccessToken) - - assert.Equal(t, TokenEndpointAuthMethodNone, body.TokenEndpointAuthMethod) - assert.Equal(t, []string{grantTypeAuthorizationCode, grantTypeRefreshToken}, body.GrantTypes) - assert.Equal(t, []string{responseTypeCode}, body.ResponseTypes) - assert.Equal(t, "doctl", body.ClientName) - assert.Equal(t, []string{"http://127.0.0.1/callback", "http://localhost/callback"}, body.RedirectURIs) - }) - - t.Run("surfaces registration errors", func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - writeJSON(t, w, http.StatusBadRequest, map[string]any{ - "error": "invalid_client_metadata", - "error_description": "grant_types must include \"authorization_code\"", - }) - })) - defer server.Close() - - _, err := RegisterClient(context.Background(), server.Client(), server.URL, RegistrationRedirectURIs(), "doctl", "") - - require.Error(t, err) - assert.Contains(t, err.Error(), "invalid_client_metadata") - assert.Contains(t, err.Error(), "grant_types must include") - }) - - t.Run("requires a redirect URI", func(t *testing.T) { - _, err := RegisterClient(context.Background(), failingClient(t), "https://example.com", nil, "doctl", "") - assert.Error(t, err) - }) -} - func TestExchangeCode(t *testing.T) { expiresIn := int64(3600) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From fbc6c74fde8c4f0bb84b69e48c4db08eb04dd7ea Mon Sep 17 00:00:00 2001 From: Anup Deka Date: Thu, 1 Oct 2026 14:09:45 +0530 Subject: [PATCH 04/10] Adding test --- internal/oauth/login_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/oauth/login_test.go b/internal/oauth/login_test.go index ce4b12dfe..2f7861685 100644 --- a/internal/oauth/login_test.go +++ b/internal/oauth/login_test.go @@ -151,6 +151,25 @@ func TestLoginReportsAuthorizationErrors(t *testing.T) { } } +func TestCallbackPageEscapesServerSuppliedText(t *testing.T) { + const payload = `` + + handler := &callbackHandler{state: "the-state", results: make(chan callbackResult, 1)} + request := httptest.NewRequest(http.MethodGet, callbackPath+"?"+url.Values{ + "error": {payload}, + "error_description": {`">`}, + "state": {"the-state"}, + }.Encode(), nil) + + recorder := httptest.NewRecorder() + handler.ServeHTTP(recorder, request) + + page := recorder.Body.String() + assert.NotContains(t, page, "