From cd380793d2b375756d8d6b4681c2b91a727a6457 Mon Sep 17 00:00:00 2001 From: manumishra12 Date: Fri, 19 Jun 2026 14:30:01 -0700 Subject: [PATCH] fix(setup): derive signin URL from api_url for self-hosted instances The signin/re-authenticate flow always redirected to the cloud hexmos.com domain, even when git-lrc was configured against a self-hosted LiveReview instance. BuildSigninURL ignored the api_url and always parsed the hardcoded HexmosSigninBase constant, so self-hosted users saw 'not authenticated' and re-auth bounced them to hexmos.com instead of their own server. Thread api_url into BuildSigninURL and resolve the signin base via ResolveSigninBase: 1. LRC_SIGNIN_URL override, if set; 2. cloud default (hexmos.com/signin) when api_url is empty or the cloud URL -- cloud behavior is unchanged; 3. otherwise /signin, so a self-hosted instance signs in against itself. Also relax the signin base to allow http (local self-hosted instances), still requiring a valid scheme and host. Covers both initial setup and the UI re-authenticate button, which share this code path. Fixes #28 --- internal/appui/setup_flow.go | 2 +- setup/login_server.go | 53 +++++++++++++++++++--- setup/login_server_test.go | 86 +++++++++++++++++++++++++++++++++--- 3 files changed, 130 insertions(+), 11 deletions(-) diff --git a/internal/appui/setup_flow.go b/internal/appui/setup_flow.go index 7fca987..f458fde 100644 --- a/internal/appui/setup_flow.go +++ b/internal/appui/setup_flow.go @@ -295,7 +295,7 @@ func runHexmosLoginFlow(slog *setupLog, apiURL string) (*setupResult, error) { mux := http.NewServeMux() - signinURL, err := setuptpl.BuildSigninURL(callbackURL) + signinURL, err := setuptpl.BuildSigninURL(callbackURL, apiURL) if err != nil { return nil, fmt.Errorf("failed to build signin url: %w", err) } diff --git a/setup/login_server.go b/setup/login_server.go index 344bf14..c42c9c4 100644 --- a/setup/login_server.go +++ b/setup/login_server.go @@ -9,8 +9,44 @@ import ( "strings" ) -// BuildSigninURL builds the Hexmos signin URL for setup callback flow. -func BuildSigninURL(callbackURL string) (string, error) { +// SigninURLEnv allows self-hosted deployments to point the signin flow at an +// explicit URL, overriding both the cloud default and the api_url-derived value. +const SigninURLEnv = "LRC_SIGNIN_URL" + +// ResolveSigninBase returns the signin base URL to use for the given LiveReview +// api_url. Resolution order: +// 1. the LRC_SIGNIN_URL override, if set; +// 2. the cloud default (HexmosSigninBase) when api_url is empty or the cloud URL; +// 3. otherwise "/signin", so a self-hosted instance signs in against +// itself instead of hexmos.com. +func ResolveSigninBase(apiURL string) (string, error) { + if override := strings.TrimSpace(os.Getenv(SigninURLEnv)); override != "" { + return override, nil + } + + apiURL = strings.TrimSpace(apiURL) + if apiURL == "" || apiURL == CloudAPIURL { + return HexmosSigninBase, nil + } + + base, err := url.Parse(apiURL) + if err != nil { + return "", fmt.Errorf("invalid api url for signin derivation: %w", err) + } + if base.Scheme != "http" && base.Scheme != "https" { + return "", fmt.Errorf("api url scheme must be http or https for signin derivation, got %q", base.Scheme) + } + if base.Host == "" { + return "", fmt.Errorf("api url must include a host for signin derivation") + } + + return strings.TrimSuffix(apiURL, "/") + "/signin", nil +} + +// BuildSigninURL builds the signin URL for the setup callback flow. The signin +// base is derived from apiURL so self-hosted LiveReview instances sign in +// against their own server rather than the cloud hexmos.com domain. +func BuildSigninURL(callbackURL, apiURL string) (string, error) { cbURL, err := url.Parse(callbackURL) if err != nil { return "", fmt.Errorf("invalid callback url: %w", err) @@ -22,12 +58,19 @@ func BuildSigninURL(callbackURL string) (string, error) { return "", fmt.Errorf("callback url must use localhost/127.0.0.1 or the current GitHub Codespaces forwarded host") } - signinBase, err := url.Parse(HexmosSigninBase) + signinBaseStr, err := ResolveSigninBase(apiURL) + if err != nil { + return "", err + } + signinBase, err := url.Parse(signinBaseStr) if err != nil { return "", fmt.Errorf("invalid signin base url: %w", err) } - if signinBase.Scheme != "https" || signinBase.Host == "" { - return "", fmt.Errorf("signin base url must be a valid https url") + if signinBase.Scheme != "http" && signinBase.Scheme != "https" { + return "", fmt.Errorf("signin base url must be a valid http(s) url") + } + if signinBase.Host == "" { + return "", fmt.Errorf("signin base url must include a host") } q := signinBase.Query() diff --git a/setup/login_server_test.go b/setup/login_server_test.go index 61eff1b..b4ee89c 100644 --- a/setup/login_server_test.go +++ b/setup/login_server_test.go @@ -8,7 +8,7 @@ import ( ) func TestBuildSigninURL_ValidLocalhostCallback(t *testing.T) { - signinURL, err := BuildSigninURL("http://127.0.0.1:8080/callback") + signinURL, err := BuildSigninURL("http://127.0.0.1:8080/callback", CloudAPIURL) if err != nil { t.Fatalf("BuildSigninURL returned error: %v", err) } @@ -21,8 +21,8 @@ func TestBuildSigninURL_ValidLocalhostCallback(t *testing.T) { if parsed.Scheme != "https" { t.Fatalf("expected https scheme, got %q", parsed.Scheme) } - if parsed.Host == "" { - t.Fatalf("expected non-empty host") + if parsed.Host != "hexmos.com" { + t.Fatalf("expected cloud signin host hexmos.com, got %q", parsed.Host) } q := parsed.Query() @@ -35,7 +35,7 @@ func TestBuildSigninURL_ValidLocalhostCallback(t *testing.T) { } func TestBuildSigninURL_RejectsNonLocalCallback(t *testing.T) { - _, err := BuildSigninURL("https://evil.example.com/callback") + _, err := BuildSigninURL("https://evil.example.com/callback", CloudAPIURL) if err == nil { t.Fatal("expected error for non-local callback URL") } @@ -46,7 +46,7 @@ func TestBuildSigninURL_AllowsCodespacesForwardedCallback(t *testing.T) { t.Setenv("GITHUB_CODESPACES_PORT_FORWARDING_DOMAIN", "app.github.dev") callbackURL := "https://lively-space-train-4321.app.github.dev/callback" - signinURL, err := BuildSigninURL(callbackURL) + signinURL, err := BuildSigninURL(callbackURL, CloudAPIURL) if err != nil { t.Fatalf("BuildSigninURL returned error: %v", err) } @@ -60,6 +60,82 @@ func TestBuildSigninURL_AllowsCodespacesForwardedCallback(t *testing.T) { } } +func TestBuildSigninURL_EmptyAPIURLUsesCloud(t *testing.T) { + signinURL, err := BuildSigninURL("http://127.0.0.1:8080/callback", "") + if err != nil { + t.Fatalf("BuildSigninURL returned error: %v", err) + } + parsed, err := url.Parse(signinURL) + if err != nil { + t.Fatalf("failed to parse signin URL: %v", err) + } + if parsed.Host != "hexmos.com" { + t.Fatalf("expected cloud signin host hexmos.com for empty api url, got %q", parsed.Host) + } +} + +func TestBuildSigninURL_SelfHostedDerivesFromAPIURL(t *testing.T) { + callbackURL := "http://127.0.0.1:8080/callback" + signinURL, err := BuildSigninURL(callbackURL, "http://localhost:3000") + if err != nil { + t.Fatalf("BuildSigninURL returned error: %v", err) + } + + parsed, err := url.Parse(signinURL) + if err != nil { + t.Fatalf("failed to parse signin URL: %v", err) + } + if parsed.Scheme != "http" { + t.Fatalf("expected http scheme for self-hosted, got %q", parsed.Scheme) + } + if parsed.Host != "localhost:3000" { + t.Fatalf("expected self-hosted host localhost:3000, got %q", parsed.Host) + } + if parsed.Path != "/signin" { + t.Fatalf("expected /signin path, got %q", parsed.Path) + } + q := parsed.Query() + if got := q.Get("app"); got != "livereview" { + t.Fatalf("expected app=livereview, got %q", got) + } + if got := q.Get("appRedirectURI"); got != callbackURL { + t.Fatalf("unexpected appRedirectURI value %q", got) + } +} + +func TestBuildSigninURL_TrailingSlashAPIURL(t *testing.T) { + signinURL, err := BuildSigninURL("http://127.0.0.1:8080/callback", "https://lr.example.com/") + if err != nil { + t.Fatalf("BuildSigninURL returned error: %v", err) + } + parsed, err := url.Parse(signinURL) + if err != nil { + t.Fatalf("failed to parse signin URL: %v", err) + } + if parsed.Host != "lr.example.com" || parsed.Path != "/signin" { + t.Fatalf("expected lr.example.com/signin, got %q%q", parsed.Host, parsed.Path) + } +} + +func TestBuildSigninURL_EnvOverrideWins(t *testing.T) { + t.Setenv(SigninURLEnv, "https://auth.internal.example.com/login") + + signinURL, err := BuildSigninURL("http://127.0.0.1:8080/callback", "http://localhost:3000") + if err != nil { + t.Fatalf("BuildSigninURL returned error: %v", err) + } + parsed, err := url.Parse(signinURL) + if err != nil { + t.Fatalf("failed to parse signin URL: %v", err) + } + if parsed.Host != "auth.internal.example.com" || parsed.Path != "/login" { + t.Fatalf("expected override host/path, got %q%q", parsed.Host, parsed.Path) + } + if got := parsed.Query().Get("appRedirectURI"); got != "http://127.0.0.1:8080/callback" { + t.Fatalf("unexpected appRedirectURI value %q", got) + } +} + func TestNewSetupHTTPClient_BlocksCrossHostRedirect(t *testing.T) { client := newSetupHTTPClient(5 * time.Second) if client.CheckRedirect == nil {