Skip to content

Commit 14341ed

Browse files
zedkippzedkipp
authored
fix(cli): fix coder login token failing without --url flag (#22742)
Previously `coder login token` didn't load the server URL from config, so it always required --url or CODER_URL when using the keyring to store the session token. This command would only print out the token when already logged in to a deployment and file storage is used to store the session token (keyring is the default on Windows/macOS). It would also print out an incorrect token when --url was specified and the session token stored on disk was for a different deployment that the user logged into. This change fixes all of these issues, and also errors out when using session token file storage with a `--url` argument that doesn't match the stored config URL, since the file only stores one token and would silently return the wrong one. See #22733 for a table of the before/after behaviors.
1 parent e7ea649 commit 14341ed

3 files changed

Lines changed: 68 additions & 23 deletions

File tree

‎cli/login.go‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,26 @@ func (r *RootCmd) loginToken() *serpent.Command {
475475
Long: "Print the session token for use in scripts and automation.",
476476
Middleware: serpent.RequireNArgs(0),
477477
Handler: func(inv *serpent.Invocation) error {
478-
tok, err := r.ensureTokenBackend().Read(r.clientURL)
478+
if err := r.ensureClientURL(); err != nil {
479+
return err
480+
}
481+
// When using the file storage, a session token is stored for a single
482+
// deployment URL that the user is logged in to. They keyring can store
483+
// multiple deployment session tokens. Error if the requested URL doesn't
484+
// match the stored config URL when using file storage to avoid returning
485+
// a token for the wrong deployment.
486+
backend := r.ensureTokenBackend()
487+
if _, ok := backend.(*sessionstore.File); ok {
488+
conf := r.createConfig()
489+
storedURL, err := conf.URL().Read()
490+
if err == nil {
491+
storedURL = strings.TrimSpace(storedURL)
492+
if storedURL != r.clientURL.String() {
493+
return xerrors.Errorf("file session token storage only supports one server at a time: requested %s but logged into %s", r.clientURL.String(), storedURL)
494+
}
495+
}
496+
}
497+
tok, err := backend.Read(r.clientURL)
479498
if err != nil {
480499
if xerrors.Is(err, os.ErrNotExist) {
481500
return xerrors.New("no session token found - run 'coder login' first")

‎cli/login_test.go‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,10 +558,33 @@ func TestLoginToken(t *testing.T) {
558558

559559
t.Run("NoTokenStored", func(t *testing.T) {
560560
t.Parallel()
561-
inv, _ := clitest.New(t, "login", "token")
561+
client := coderdtest.New(t, nil)
562+
inv, _ := clitest.New(t, "login", "token", "--url", client.URL.String())
562563
ctx := testutil.Context(t, testutil.WaitShort)
563564
err := inv.WithContext(ctx).Run()
564565
require.Error(t, err)
565566
require.Contains(t, err.Error(), "no session token found")
566567
})
568+
569+
t.Run("NoURLProvided", func(t *testing.T) {
570+
t.Parallel()
571+
inv, _ := clitest.New(t, "login", "token")
572+
ctx := testutil.Context(t, testutil.WaitShort)
573+
err := inv.WithContext(ctx).Run()
574+
require.Error(t, err)
575+
require.Contains(t, err.Error(), "You are not logged in")
576+
})
577+
578+
t.Run("URLMismatchFileBackend", func(t *testing.T) {
579+
t.Parallel()
580+
client := coderdtest.New(t, nil)
581+
coderdtest.CreateFirstUser(t, client)
582+
583+
inv, root := clitest.New(t, "login", "token", "--url", "https://other.example.com")
584+
clitest.SetupConfig(t, client, root)
585+
ctx := testutil.Context(t, testutil.WaitShort)
586+
err := inv.WithContext(ctx).Run()
587+
require.Error(t, err)
588+
require.Contains(t, err.Error(), "file session token storage only supports one server")
589+
})
567590
}

‎cli/root.go‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -550,30 +550,33 @@ type RootCmd struct {
550550
useKeyringWithGlobalConfig bool
551551
}
552552

553-
// InitClient creates and configures a new client with authentication, telemetry,
554-
// and version checks.
555-
func (r *RootCmd) InitClient(inv *serpent.Invocation) (*codersdk.Client, error) {
556-
conf := r.createConfig()
557-
var err error
558-
// Read the client URL stored on disk.
559-
if r.clientURL == nil || r.clientURL.String() == "" {
560-
rawURL, err := conf.URL().Read()
561-
// If the configuration files are absent, the user is logged out
562-
if os.IsNotExist(err) {
563-
binPath, err := os.Executable()
564-
if err != nil {
565-
binPath = "coder"
566-
}
567-
return nil, xerrors.Errorf(notLoggedInMessage, binPath)
568-
}
553+
// ensureClientURL loads the client URL from the config file if it
554+
// wasn't provided via --url or CODER_URL.
555+
func (r *RootCmd) ensureClientURL() error {
556+
if r.clientURL != nil && r.clientURL.String() != "" {
557+
return nil
558+
}
559+
rawURL, err := r.createConfig().URL().Read()
560+
// If the configuration files are absent, the user is logged out.
561+
if os.IsNotExist(err) {
562+
binPath, err := os.Executable()
569563
if err != nil {
570-
return nil, err
564+
binPath = "coder"
571565
}
566+
return xerrors.Errorf(notLoggedInMessage, binPath)
567+
}
568+
if err != nil {
569+
return err
570+
}
571+
r.clientURL, err = url.Parse(strings.TrimSpace(rawURL))
572+
return err
573+
}
572574

573-
r.clientURL, err = url.Parse(strings.TrimSpace(rawURL))
574-
if err != nil {
575-
return nil, err
576-
}
575+
// InitClient creates and configures a new client with authentication, telemetry,
576+
// and version checks.
577+
func (r *RootCmd) InitClient(inv *serpent.Invocation) (*codersdk.Client, error) {
578+
if err := r.ensureClientURL(); err != nil {
579+
return nil, err
577580
}
578581
if r.token == "" {
579582
tok, err := r.ensureTokenBackend().Read(r.clientURL)

0 commit comments

Comments
 (0)