You cannot select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
tailscale/cmd
Alex Chan b7fe1cea9f cmd/tailscale/cli: only print authURLs and device approval URLs once
This patch fixes several issues related to printing login and device
approval URLs, especially when `tailscale up` is interrupted:

1.  Only print a login URL that will cause `tailscale up` to complete.
    Don't print expired URLs or URLs from previous login attempts.

2.  Print the device approval URL if you run `tailscale up` after
    previously completing a login, but before approving the device.

3.  Use the correct control URL for device approval if you run a bare
    `tailscale up` after previously completing a login, but before
    approving the device.

4.  Don't print the device approval URL more than once (or at least,
    not consecutively).

Updates tailscale/corp#31476
Updates #17361

## How these fixes work

This patch went through a lot of trial and error, and there may still
be bugs! These notes capture the different scenarios and considerations
as we wrote it, which are also captured by integration tests.

1.  We were getting stale login URLs from the initial IPN state
    notification.

    When the IPN watcher was moved to before Start() in c011369, we
    mistakenly continued to request the initial state. This is only
    necessary if you start watching after you call Start(), because
    you may have missed some notifications.

    By getting the initial state before calling Start(), we'd get
    a stale login URL. If you clicked that URL, you could complete
    the login in the control server (if it wasn't expired), but your
    instance of `tailscale up` would hang, because it's listening for
    login updates from a different login URL.

    In this patch, we no longer request the initial state, and so we
    don't print a stale URL.

2.  Once you skip the initial state from IPN, the following sequence:

    *   Run `tailscale up`
    *   Log into a tailnet with device approval
    *   ^C after the device approval URL is printed, but without approving
    *   Run `tailscale up` again

    means that nothing would ever be printed.

    `tailscale up` would send tailscaled the pref `WantRunning: true`,
    but that was already the case so nothing changes. You never get any
    IPN notifications, and in particular you never get a state change to
    `NeedsMachineAuth`. This means we'd never print the device approval URL.

    In this patch, we add a hard-coded rule that if you're doing a simple up
    (which won't trigger any other IPN notifications) and you start in the
    `NeedsMachineAuth` state, we print the device approval message without
    waiting for an IPN notification.

3.  Consider the following sequence:

    *   Run `tailscale up --login-server=<custom server>`
    *   Log into a tailnet with device approval
    *   ^C after the device approval URL is printed, but without approving
    *   Run `tailscale up` again

    We'd print the device approval URL for the default control server,
    rather than the real control server, because we were using the `prefs`
    from the CLI arguments (which are all the defaults) rather than the
    `curPrefs` (which contain the custom login server).

    In this patch, we use the `prefs` if the user has specified any settings
    (and other code will ensure this is a complete set of settings) or
    `curPrefs` if it's a simple `tailscale up`.

4.  Consider the following sequence: you've logged in, but not completed
    device approval, and you run `down` and `up` in quick succession.

    *   `up`: sees state=NeedsMachineAuth
    *   `up`: sends `{wantRunning: true}`, prints out the device approval URL
    *   `down`: changes state to Stopped
    *   `up`: changes state to Starting
    *   tailscaled: changes state to NeedsMachineAuth
    *   `up`: gets an IPN notification with the state change, and prints
        a second device approval URL

    Either URL works, but this is annoying for the user.

    In this patch, we track whether the last printed URL was the device
    approval URL, and if so, we skip printing it a second time.

Signed-off-by: Alex Chan <alexc@tailscale.com>
2 months ago
..
addlicense all: fix golangci-lint errors 11 months ago
build-webclient client/web: precompress assets 2 years ago
checkmetrics cmd/checkmetrics: add command for checking metrics against kb 1 year ago
cloner cmd/viewer, types/views: implement support for json/v2 (#16852) 4 months ago
connector-gen cmd/connector-gen: add helper tool for wide app connector configurations 2 years ago
containerboot util/backoff: rename logtail/backoff package to util/backoff 2 months ago
derper go.toolchain.rev: bump Go to 1.25.2 2 months ago
derpprobe cmd/derpprobe,prober: add run all probes handler (#16875) 4 months ago
dist {cmd/dist,release/dist}: add support for intermediary QNAP signing certificates 4 months ago
featuretags feature/featuretags,cmd/omitsize: support feature dependencies 3 months ago
get-authkey cmd,tsnet,internal/client: create internal shim to deprecated control plane API 10 months ago
gitops-pusher scripts/check_license_headers.sh: delete, rewrite as a Go test 6 months ago
hello cmd/hello: display native ipv4 (#15191) 9 months ago
k8s-nameserver k8s-operator: add IPv6 support for DNS records (#16691) 2 months ago
k8s-operator go.toolchain.rev: bump Go to 1.25.2 2 months ago
k8s-proxy cmd/k8s-proxy: switching to using ipn/store/kubestore (#17402) 2 months ago
mkmanifest cmd/mkmanifest, cmd/tailscale, cmd/tailscaled: remove Windows arm32 resources from OSS 3 years ago
mkpkg go.mod: upgrade nfpm to v2 (#8786) 2 years ago
mkversion version/mkversion: open-source version generation logic 3 years ago
nardump cmd/nardump: support symlinks, add basic test 8 months ago
natc cmd/natc,tsconsensus: add cluster config admin 3 months ago
netlogfmt all: use Go 1.22 range-over-int 2 years ago
nginx-auth tailcfg,all: add and use Node.IsTagged() 3 years ago
omitsize feature/featuretags,cmd/omitsize: support feature dependencies 3 months ago
pgproxy all: use new LocalAPI client package location 10 months ago
printdep cmd/printdep: print correct toolchain URL 3 years ago
proxy-test-server net/tlsdial: fix TLS cert validation of HTTPS proxies 6 months ago
proxy-to-grafana scripts/check_license_headers.sh: delete, rewrite as a Go test 6 months ago
sniproxy net/{netx,memnet},all: add netx.DialFunc, move memnet Network impl 8 months ago
speedtest all: update copyright and license headers 3 years ago
ssh-auth-none-demo ssh,tempfork/gliderlabs/ssh: replace github.com/tailscale/golang-x-crypto/ssh with golang.org/x/crypto/ssh 10 months ago
stunc cmd/stunc: enforce read timeout deadline (#14309) 1 year ago
stund go.toolchain.rev: bump Go to 1.25.2 2 months ago
stunstamp util/backoff: rename logtail/backoff package to util/backoff 2 months ago
sync-containers all: adjust some build tags for plan9 2 years ago
systray client/systray: allow specifying tailscaled socket 4 months ago
tailscale cmd/tailscale/cli: only print authURLs and device approval URLs once 2 months ago
tailscaled go.toolchain.rev: bump Go to 1.25.2 2 months ago
testcontrol all: use Go 1.22 range-over-int 2 years ago
testwrapper cmd/testwrapper/flakytest: use t.Attr annotation on flaky tests 2 months ago
tl-longchain all: use new LocalAPI client package location 10 months ago
tsconnect net/netmon: remove usage of direct callbacks from netmon (#17292) 2 months ago
tsidp go.toolchain.rev: bump Go to 1.25.2 2 months ago
tsshd all: update copyright and license headers 3 years ago
tta all: use new LocalAPI client package location 10 months ago
viewer cmd/viewer: add field comments to generated view methods 3 months ago
vnet cmd/vnet: add wsproxy mode 8 months ago
xdpderper tsweb: split promvarz into an optional dependency 9 months ago