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.

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
..
archtest	all: don't rebind variables in for loops	2 months ago
deptest	ipn/ipnlocal: use policyclient.Client always, stop using global syspolicy funcs	3 months ago
integration	cmd/tailscale/cli: only print authURLs and device approval URLs once	2 months ago
iosdeps	feature/capture: move packet capture to feature/*, out of iOS + CLI	10 months ago
jsdeps	all: use math/rand/v2 more	2 years ago
mts	tstest/mts: add multiple-tailscaled development tool	8 months ago
natlab	derp/derpserver: clean up extraction of derp.Server (#17264)	2 months ago
nettest	net/{netx,memnet},all: add netx.DialFunc, move memnet Network impl	8 months ago
tailmac	tstest/tailmac: add support for mounting host directories in the guest (#13957)	1 year ago
tlstest	tstest/tlstest: simplify, don't even bake in any keys	6 months ago
tools	go.{mod,sum},tstest/tools,k8s-operator,cmd/k8s-operator: autogenerate CRD API docs (#12884)	1 year ago
allocs.go	all: update copyright and license headers	3 years ago
clock.go	tstime: add Since method (#8622)	2 years ago
clock_test.go	all: don't rebind variables in for loops	2 months ago
log.go	net/portmapper: fix test flakes from logging after test done	7 months ago
log_test.go	all: update copyright and license headers	3 years ago
reflect.go	net/dns: fix IsZero and Equal methods on OSConfig	1 year ago
resource.go	tstest: parse goroutines for diff in ResourceCheck (#15619)	8 months ago
resource_test.go	tstest: parse goroutines for diff in ResourceCheck (#15619)	8 months ago
test-wishlist.md	wf/firewall: allow link-local multicast for permitted local routes when the killswitch is on on Windows	1 year ago
tstest.go	util/backoff: rename logtail/backoff package to util/backoff	2 months ago
tstest_test.go	tstest: add method to Replace values for tests	3 years ago