From cc12e8bb380de01ff14dc0038ef28cefae31ccf1 Mon Sep 17 00:00:00 2001 From: Lexi Mattick Date: Mon, 26 Aug 2024 11:00:23 -0700 Subject: [PATCH] Simplify login process --- browser/browser_darwin.go | 17 --------------- browser/browser_linux.go | 15 ------------- libts/client.go | 19 +++++++++++------ menus.go | 10 ++------- update.go | 44 ++++++++++++--------------------------- view.go | 11 +++++----- 6 files changed, 34 insertions(+), 82 deletions(-) delete mode 100644 browser/browser_darwin.go delete mode 100644 browser/browser_linux.go diff --git a/browser/browser_darwin.go b/browser/browser_darwin.go deleted file mode 100644 index e88e6a8..0000000 --- a/browser/browser_darwin.go +++ /dev/null @@ -1,17 +0,0 @@ -package browser - -import ( - "io" - "os/exec" -) - -func OpenURL(url string) error { - cmd := exec.Command("open", url) - cmd.Stdout = io.Discard - cmd.Stderr = io.Discard - return cmd.Run() -} - -func IsSupported() bool { - return true -} diff --git a/browser/browser_linux.go b/browser/browser_linux.go deleted file mode 100644 index b027689..0000000 --- a/browser/browser_linux.go +++ /dev/null @@ -1,15 +0,0 @@ -// We're not going to support opening the user's browser on Linux because xdg-open doesn't work -// as expected when running as root and there isn't a good way to drop privileges. We favor -// consistency across sudo/non-sudo over having an extra feature for non-sudo users. - -package browser - -import "fmt" - -func OpenURL(url string) error { - return fmt.Errorf("opening browser is unsupported on linux") -} - -func IsSupported() bool { - return false -} diff --git a/libts/client.go b/libts/client.go index a6a5f20..98e4d98 100644 --- a/libts/client.go +++ b/libts/client.go @@ -2,6 +2,7 @@ package libts import ( "context" + "runtime" "tailscale.com/client/tailscale" "tailscale.com/ipn" @@ -16,13 +17,19 @@ func Status(ctx context.Context) (*ipnstate.Status, error) { return ts.Status(ctx) } -// Start an interactive login flow. This will automatically open the user's web browser. -// Note that this will NOT DO ANYTHING if the session has already started; i.e. an -// AuthURL is already populated in the state. +// Returns true if StartLoginInteractive will (probably) open the user's web browser. +// Can be used to decide whether to display UI elements related to interactive login. +func StartLoginInteractiveWillOpenBrowser() bool { + return runtime.GOOS == "darwin" +} + +// Start an interactive login flow. On macOS, this will automatically open the user's web browser. func StartLoginInteractive(ctx context.Context) error { - // Workaround for a Tailscale bug (?) where the AuthURL isn't populated when calling - // StartLoginInteractive the first time if the user is already logged in. For some reason, - // calling Start first with no options makes the AuthURL populate. + // Workaround for a Tailscale bug where Tailscale will go into the Starting... state + // without populating the AuthURL when reauthenticating. For some reason, calling + // Start first with no options makes the AuthURL populate. + // + // We need AuthURL so we can display UI elements related to the login process. err := ts.Start(ctx, ipn.Options{}) if err != nil { return err diff --git a/menus.go b/menus.go index ec80e1a..65fb130 100644 --- a/menus.go +++ b/menus.go @@ -259,14 +259,8 @@ func (m *model) updateMenus() { &ui.LabeledSubmenuItem{ Label: reauthenticateButtonLabel, - OnActivate: func() tea.Msg { - // Reauthenticating is basically the same as the first-time login flow. - err := libts.StartLoginInteractive(ctx) - if err != nil { - return errorMsg(err) - } - return successMsg("Starting reauthentication. This may take a few seconds.") - }, + // Reauthenticating is basically the same as the first-time login flow. + OnActivate: startLoginInteractive, }, &ui.LabeledSubmenuItem{ diff --git a/update.go b/update.go index 2961aff..985a011 100644 --- a/update.go +++ b/update.go @@ -5,7 +5,6 @@ import ( "time" tea "github.com/charmbracelet/bubbletea" - "github.com/neuralinkcorp/tsui/browser" "github.com/neuralinkcorp/tsui/libts" "github.com/neuralinkcorp/tsui/ui" "github.com/neuralinkcorp/tsui/version" @@ -56,6 +55,15 @@ func updateState() tea.Msg { return stateMsg(state) } +// Command that starts the interactive login flow. +func startLoginInteractive() tea.Msg { + err := libts.StartLoginInteractive(ctx) + if err != nil { + return errorMsg(err) + } + return successMsg("Starting login flow. This may take a few seconds.") +} + // Creates a command to gets the current latency of the specified peers. Takes some time. func makeDoPings(peers []*ipnstate.PeerStatus) tea.Cmd { return func() tea.Msg { @@ -158,39 +166,13 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // If we need to login... case ipn.NeedsLogin: - if m.state.AuthURL == "" { - // If we haven't started the login flow yet, do so. - // Tailscale will open their browser for us. - return m, func() tea.Msg { - err := libts.StartLoginInteractive(ctx) - if err != nil { - return errorMsg(err) - } - return successMsg("Starting login flow. This may take a few seconds.") - } - } else if browser.IsSupported() { - // If the auth flow has already started, we need to open the browser ourselves. - return m, func() tea.Msg { - err := browser.OpenURL(m.state.AuthURL) - if err != nil { - return errorMsg(err) - } - return nil - } - } + return m, startLoginInteractive case ipn.Starting: // If we have an AuthURL in the Starting state, that means the user is reauthenticating - // and we also need to open the browser! - // (But not if we're root on Linux.) - if m.state.AuthURL != "" && browser.IsSupported() { - return m, func() tea.Msg { - err := browser.OpenURL(m.state.AuthURL) - if err != nil { - return errorMsg(err) - } - return nil - } + // and we want to open the browser for them (if supported). + if m.state.AuthURL != "" && libts.StartLoginInteractiveWillOpenBrowser() { + return m, startLoginInteractive } } } diff --git a/view.go b/view.go index ad05b55..f665393 100644 --- a/view.go +++ b/view.go @@ -5,7 +5,7 @@ import ( "strings" "github.com/charmbracelet/lipgloss" - "github.com/neuralinkcorp/tsui/browser" + "github.com/neuralinkcorp/tsui/libts" "github.com/neuralinkcorp/tsui/ui" "tailscale.com/ipn" ) @@ -269,8 +269,7 @@ func (m model) View() string { lines = append(lines, fmt.Sprintf(`Login URL: %s`, styledAuthUrl), ) - if browser.IsSupported() { - // We can't open the browser for them if running as the root user on Linux. + if libts.StartLoginInteractiveWillOpenBrowser() { lines = append(lines, ``, `Press . to open in browser.`, @@ -294,7 +293,9 @@ func (m model) View() string { if m.state.AuthURL == "" { middle = renderMiddleBanner(&m, middleHeight, ui.PoggersAnimationFrame(m.animationT)) } else { - // If we have an AuthURL in the Starting state, that means the user is reauthenticating! + // If we have an AuthURL in the Starting state, that means the user is reauthenticating. + // TODO: This case only seems to show up sometimes, and may not anymore (?) in the latest + // version of Tailscale. lines := []string{ lipgloss.NewStyle(). Bold(true). @@ -302,7 +303,7 @@ func (m model) View() string { ``, fmt.Sprintf(`Login URL: %s`, styledAuthUrl), } - if browser.IsSupported() { + if libts.StartLoginInteractiveWillOpenBrowser() { // We can't open the browser for them if running as the root user on Linux. lines = append(lines, ``,