From 2900c3eebeb79a3211e8c7231eda4a013556b054 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 17:39:22 -0700 Subject: [PATCH 01/16] =?UTF-8?q?=E2=9C=A8=20feat:=20add=20OAuth2=20login/?= =?UTF-8?q?logout=20with=20PKCE=20via=20golang.org/x/oauth2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add browser-based OAuth2 Authorization Code + PKCE flow for CLI authentication. Uses magic client_id="rootly-cli" so no configuration is needed — just `rootly login`. - `rootly login` opens browser, runs local callback server on :19797, exchanges code for tokens - `rootly logout` clears stored tokens - Tokens stored in ~/.rootly-cli/tokens.yaml (mode 0600) with auto-refresh via oauth2.Transport - API client falls back to OAuth tokens when no API key is set - Supports localhost dev servers (http) and production (https with api.->app. rewrite) - Uses golang.org/x/oauth2 built-in PKCE (GenerateVerifier, S256ChallengeOption, VerifierOption) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- go.mod | 3 +- go.sum | 2 + internal/api/client.go | 181 +++++++++++++++------------- internal/cmd/alerts/alerts.go | 5 +- internal/cmd/alerts/cmd_test.go | 15 ++- internal/cmd/auth/cmd_test.go | 77 ++++++++++++ internal/cmd/auth/login.go | 165 +++++++++++++++++++++++++ internal/cmd/auth/logout.go | 21 ++++ internal/cmd/auth_register.go | 8 ++ internal/cmd/incidents/cmd_test.go | 10 +- internal/cmd/incidents/incidents.go | 5 +- internal/cmd/oncall/cmd_test.go | 11 +- internal/cmd/oncall/oncall.go | 5 +- internal/cmd/pulse/cmd_test.go | 10 +- internal/cmd/pulse/pulse.go | 5 +- internal/cmd/root.go | 15 ++- internal/cmd/services/cmd_test.go | 10 +- internal/cmd/services/services.go | 5 +- internal/cmd/teams/cmd_test.go | 10 +- internal/cmd/teams/teams.go | 5 +- internal/config/config.go | 8 +- internal/oauth/oauth.go | 52 ++++++++ internal/oauth/oauth_test.go | 36 ++++++ internal/oauth/tokens.go | 91 ++++++++++++++ internal/oauth/tokens_test.go | 112 +++++++++++++++++ internal/oauth/transport.go | 63 ++++++++++ internal/oauth/transport_test.go | 137 +++++++++++++++++++++ 27 files changed, 941 insertions(+), 126 deletions(-) create mode 100644 internal/cmd/auth/cmd_test.go create mode 100644 internal/cmd/auth/login.go create mode 100644 internal/cmd/auth/logout.go create mode 100644 internal/cmd/auth_register.go create mode 100644 internal/oauth/oauth.go create mode 100644 internal/oauth/oauth_test.go create mode 100644 internal/oauth/tokens.go create mode 100644 internal/oauth/tokens_test.go create mode 100644 internal/oauth/transport.go create mode 100644 internal/oauth/transport_test.go diff --git a/go.mod b/go.mod index 4d2cc6c..02a0e2f 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/rootlyhq/rootly-cli -go 1.24.2 +go 1.25.0 require ( github.com/jedib0t/go-pretty/v6 v6.7.8 @@ -8,6 +8,7 @@ require ( github.com/rootlyhq/rootly-go v0.8.0 github.com/spf13/cobra v1.10.2 github.com/spf13/viper v1.21.0 + golang.org/x/oauth2 v0.36.0 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index fc77e22..2bfb605 100644 --- a/go.sum +++ b/go.sum @@ -92,6 +92,8 @@ github.com/woodsbury/decimal128 v1.4.0 h1:xJATj7lLu4f2oObouMt2tgGiElE5gO6mSWUjQs github.com/woodsbury/decimal128 v1.4.0/go.mod h1:BP46FUrVjVhdTbKT+XuQh2xfQaGki9LMIRJSFuh6THU= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= +golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= +golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k= golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= diff --git a/internal/api/client.go b/internal/api/client.go index 46be3f9..ce5313d 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -14,6 +14,7 @@ import ( rootly "github.com/rootlyhq/rootly-go" "github.com/rootlyhq/rootly-cli/internal/config" + "github.com/rootlyhq/rootly-cli/internal/oauth" ) // Version is set by the main package to include in User-Agent @@ -733,30 +734,59 @@ func parseIncidentDetailRelationships(incident *Incident, a *incidentDetailAttri // NewClient creates a stateless API client for CLI usage. func NewClient(cfg *config.Config) (*Client, error) { endpoint := cfg.Endpoint - if endpoint != "" && !strings.HasPrefix(endpoint, "http://") && !strings.HasPrefix(endpoint, "https://") { - endpoint = "https://" + endpoint + if endpoint != "" { + endpoint = ensureScheme(endpoint) } - // Build HTTP client with optional debug transport - httpClient := http.DefaultClient - if cfg.Debug { - transport := http.DefaultTransport - if httpClient.Transport != nil { - transport = httpClient.Transport + // Determine auth: use OAuth tokens only when no API key is set + useOAuth := false + if cfg.APIKey == "" { + if t, err := oauth.LoadTokens(); err == nil && t.AccessToken != "" { + _ = t + useOAuth = true } - httpClient = &http.Client{ - Transport: &debugTransport{transport: transport}, + } + + // Build base transport + var transport http.RoundTripper = http.DefaultTransport + + if cfg.Debug { + transport = &debugTransport{transport: transport} + } + + var httpClient *http.Client + if useOAuth { + authBaseURL := deriveAuthBaseURL(cfg.Endpoint) + oauthCfg := oauth.NewConfig(authBaseURL) + var err error + httpClient, err = oauth.NewHTTPClient(oauthCfg, transport, "rootly-cli/"+Version) + if err != nil { + return nil, fmt.Errorf("failed to create OAuth client: %w", err) } + } else { + httpClient = &http.Client{Transport: transport} } - client, err := rootly.NewClientWithResponses(endpoint, - rootly.WithHTTPClient(httpClient), - rootly.WithRequestEditorFn(func(ctx context.Context, req *http.Request) error { + var reqEditorFn rootly.RequestEditorFn + if useOAuth { + // OAuth transport handles Authorization header + reqEditorFn = func(ctx context.Context, req *http.Request) error { + req.Header.Set("Content-Type", "application/vnd.api+json") + req.Header.Set("User-Agent", "rootly-cli/"+Version) + return nil + } + } else { + reqEditorFn = func(ctx context.Context, req *http.Request) error { req.Header.Set("Authorization", "Bearer "+cfg.APIKey) req.Header.Set("Content-Type", "application/vnd.api+json") req.Header.Set("User-Agent", "rootly-cli/"+Version) return nil - }), + } + } + + client, err := rootly.NewClientWithResponses(endpoint, + rootly.WithHTTPClient(httpClient), + rootly.WithRequestEditorFn(reqEditorFn), ) if err != nil { return nil, fmt.Errorf("failed to create rootly client: %w", err) @@ -764,12 +794,37 @@ func NewClient(cfg *config.Config) (*Client, error) { return &Client{ client: client, - endpoint: cfg.Endpoint, + endpoint: endpoint, apiKey: cfg.APIKey, httpClient: httpClient, }, nil } +// ensureScheme adds a scheme if missing, using http for localhost/127.0.0.1. +func ensureScheme(url string) string { + if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") { + return url + } + if strings.HasPrefix(url, "localhost") || strings.HasPrefix(url, "127.0.0.1") { + return "http://" + url + } + return "https://" + url +} + +// deriveAuthBaseURL builds the OAuth base URL from the API host. +func deriveAuthBaseURL(apiHost string) string { + if strings.HasPrefix(apiHost, "http://") || strings.HasPrefix(apiHost, "https://") { + return apiHost + } + if strings.HasPrefix(apiHost, "localhost") || strings.HasPrefix(apiHost, "127.0.0.1") { + return "http://" + apiHost + } + if strings.HasPrefix(apiHost, "api.") { + return "https://app." + apiHost[4:] + } + return "https://" + apiHost +} + func (c *Client) ValidateAPIKey(ctx context.Context) error { // Use /v1/users/me endpoint to validate the API key resp, err := c.client.GetCurrentUserWithResponse(ctx) @@ -865,9 +920,7 @@ func (c *Client) ListIncidentsCLI(ctx context.Context, page, pageSize int, sort // Build URL with query parameters baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/incidents?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { @@ -967,9 +1020,7 @@ func (c *Client) ListIncidentsCLI(ctx context.Context, page, pageSize int, sort func (c *Client) GetIncidentByID(ctx context.Context, id string) (*Incident, error) { // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/incidents/%s?include=roles,causes,incident_types,functionalities,services,environments,groups,user", baseURL, id) req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) if err != nil { @@ -1042,9 +1093,7 @@ func (c *Client) CreateIncident(ctx context.Context, title string, opts map[stri // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/incidents", baseURL) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(bodyBytes))) @@ -1120,9 +1169,7 @@ func (c *Client) UpdateIncident(ctx context.Context, id string, opts map[string] // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/incidents/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "PUT", url, strings.NewReader(string(bodyBytes))) @@ -1173,9 +1220,7 @@ func (c *Client) UpdateIncident(ctx context.Context, id string, opts map[string] func (c *Client) DeleteIncident(ctx context.Context, id string) error { // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/incidents/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "DELETE", url, http.NoBody) @@ -1292,9 +1337,7 @@ func (c *Client) ListAlertsCLI(ctx context.Context, page, pageSize int, sort str // Build URL with query parameters baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/alerts?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { @@ -1394,9 +1437,7 @@ func (c *Client) ListAlertsCLI(ctx context.Context, page, pageSize int, sort str func (c *Client) GetAlertByID(ctx context.Context, id string) (*Alert, error) { // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/alerts/%s?include=services,environments,groups,responders,alert_urgency", baseURL, id) req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) if err != nil { @@ -1632,9 +1673,7 @@ func (c *Client) CreateAlertCLI(ctx context.Context, summary string, opts map[st // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/alerts", baseURL) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(bodyBytes))) @@ -1713,9 +1752,7 @@ func (c *Client) UpdateAlertCLI(ctx context.Context, id string, opts map[string] // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/alerts/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "PUT", url, strings.NewReader(string(bodyBytes))) @@ -1766,9 +1803,7 @@ func (c *Client) UpdateAlertCLI(ctx context.Context, id string, opts map[string] func (c *Client) AcknowledgeAlertCLI(ctx context.Context, id string) error { // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/alerts/%s/acknowledge", baseURL, id) req, err := http.NewRequestWithContext(ctx, "POST", url, http.NoBody) @@ -1804,9 +1839,7 @@ func (c *Client) AcknowledgeAlertCLI(ctx context.Context, id string) error { func (c *Client) ResolveAlertCLI(ctx context.Context, id, resolutionMessage string, resolveIncidents bool) error { // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/alerts/%s/resolve", baseURL, id) var reqBody io.Reader = http.NoBody @@ -1876,9 +1909,7 @@ func (c *Client) ListServicesCLI(ctx context.Context, page, pageSize int, sort s // Build URL with query parameters baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/services?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { @@ -1981,9 +2012,7 @@ func (c *Client) ListServicesCLI(ctx context.Context, page, pageSize int, sort s func (c *Client) GetServiceByID(ctx context.Context, id string) (*Service, error) { // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/services/%s?include=owner_group", baseURL, id) req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) if err != nil { @@ -2107,9 +2136,7 @@ func (c *Client) CreateService(ctx context.Context, name string, opts map[string // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/services", baseURL) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(bodyBytes))) @@ -2206,9 +2233,7 @@ func (c *Client) UpdateService(ctx context.Context, id string, opts map[string]s // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/services/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "PUT", url, strings.NewReader(string(bodyBytes))) @@ -2283,9 +2308,7 @@ func (c *Client) UpdateService(ctx context.Context, id string, opts map[string]s func (c *Client) DeleteService(ctx context.Context, id string) error { // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/services/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "DELETE", url, http.NoBody) @@ -2329,9 +2352,7 @@ func (c *Client) ListTeamsCLI(ctx context.Context, page, pageSize int, sort stri // Build URL with query parameters baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/teams?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { @@ -2436,9 +2457,7 @@ func (c *Client) ListTeamsCLI(ctx context.Context, page, pageSize int, sort stri func (c *Client) GetTeamByID(ctx context.Context, id string) (*Team, error) { // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/teams/%s?include=users", baseURL, id) req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) if err != nil { @@ -2563,9 +2582,7 @@ func (c *Client) CreateTeam(ctx context.Context, name string, opts map[string]st // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/teams", baseURL) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(bodyBytes))) @@ -2655,9 +2672,7 @@ func (c *Client) UpdateTeam(ctx context.Context, id string, opts map[string]stri // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/teams/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "PATCH", url, strings.NewReader(string(bodyBytes))) @@ -2736,9 +2751,7 @@ func (c *Client) UpdateTeam(ctx context.Context, id string, opts map[string]stri func (c *Client) DeleteTeam(ctx context.Context, id string) error { // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/teams/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "DELETE", url, http.NoBody) @@ -2838,9 +2851,7 @@ func (c *Client) ListSchedulesCLI(ctx context.Context, page, pageSize int, filte // Build URL with query parameters baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/schedules?page[number]=%d&page[size]=%d", baseURL, page, pageSize) @@ -2929,9 +2940,7 @@ func (c *Client) ListSchedulesCLI(ctx context.Context, page, pageSize int, filte // ListOnCallsCLI lists on-call entries using the unified /v1/oncalls endpoint. func (c *Client) ListOnCallsCLI(ctx context.Context, params OnCallsParams) (*OnCallsResult, error) { baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/oncalls?", baseURL) @@ -3126,9 +3135,7 @@ func (c *Client) CreatePulseCLI(ctx context.Context, summary string, opts PulseO // Build URL baseURL := c.endpoint - if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") { - baseURL = "https://" + baseURL - } + baseURL = ensureScheme(baseURL) url := fmt.Sprintf("%s/v1/pulses", baseURL) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(bodyBytes))) diff --git a/internal/cmd/alerts/alerts.go b/internal/cmd/alerts/alerts.go index 885703a..c44e44f 100644 --- a/internal/cmd/alerts/alerts.go +++ b/internal/cmd/alerts/alerts.go @@ -8,6 +8,7 @@ import ( "github.com/rootlyhq/rootly-cli/internal/api" "github.com/rootlyhq/rootly-cli/internal/config" + "github.com/rootlyhq/rootly-cli/internal/oauth" ) // AlertsCmd is the parent command for all alert operations @@ -40,7 +41,9 @@ var AlertsCmd = &cobra.Command{ func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - return nil, fmt.Errorf("API key required: set ROOTLY_API_KEY or add api_key to ~/.rootly-cli/config.yaml") + if _, err := oauth.LoadTokens(); err != nil { + return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") + } } endpoint := viper.GetString("api_host") if endpoint == "" { diff --git a/internal/cmd/alerts/cmd_test.go b/internal/cmd/alerts/cmd_test.go index 31e0ea5..fce7f81 100644 --- a/internal/cmd/alerts/cmd_test.go +++ b/internal/cmd/alerts/cmd_test.go @@ -155,6 +155,7 @@ func TestRunListJSON(t *testing.T) { func TestRunListNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().Int("page", 1, "") @@ -167,8 +168,8 @@ func TestRunListNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } @@ -360,6 +361,7 @@ func TestRunAckSuccess(t *testing.T) { func TestRunAckNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() @@ -367,8 +369,8 @@ func TestRunAckNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } @@ -464,6 +466,7 @@ func TestRunResolveWithIncidents(t *testing.T) { func TestRunResolveNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().String("message", "", "") @@ -473,8 +476,8 @@ func TestRunResolveNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } diff --git a/internal/cmd/auth/cmd_test.go b/internal/cmd/auth/cmd_test.go new file mode 100644 index 0000000..05a9db3 --- /dev/null +++ b/internal/cmd/auth/cmd_test.go @@ -0,0 +1,77 @@ +package auth + +import ( + "bytes" + "os" + "testing" + "time" + + "github.com/rootlyhq/rootly-cli/internal/oauth" +) + +func TestDeriveAuthBaseURL(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"api.rootly.com", "https://app.rootly.com"}, + {"api.staging.rootly.com", "https://app.staging.rootly.com"}, + {"localhost:22166", "http://localhost:22166"}, + {"localhost:22166/api", "http://localhost:22166/api"}, + {"127.0.0.1:3000", "http://127.0.0.1:3000"}, + {"http://localhost:22166", "http://localhost:22166"}, + {"https://custom.example.com", "https://custom.example.com"}, + {"custom.example.com", "https://custom.example.com"}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got := deriveAuthBaseURL(tt.input) + if got != tt.want { + t.Errorf("deriveAuthBaseURL(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +func TestLogoutCmd(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + // Save tokens first + tokens := &oauth.TokenData{ + AccessToken: "test", + RefreshToken: "test", + ExpiresAt: time.Now().Add(time.Hour), + } + oauth.SaveTokens(tokens) + + // Run logout + var buf bytes.Buffer + LogoutCmd.SetOut(&buf) + LogoutCmd.SetErr(&buf) + LogoutCmd.SetArgs([]string{}) + + if err := LogoutCmd.Execute(); err != nil { + t.Fatalf("logout failed: %v", err) + } + + // Verify tokens cleared + if _, err := oauth.LoadTokens(); !os.IsNotExist(err) { + t.Errorf("expected tokens to be cleared, got err: %v", err) + } +} + +func TestLogoutCmd_NoTokens(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + var buf bytes.Buffer + LogoutCmd.SetOut(&buf) + LogoutCmd.SetErr(&buf) + LogoutCmd.SetArgs([]string{}) + + // Should not error when no tokens exist + if err := LogoutCmd.Execute(); err != nil { + t.Fatalf("logout with no tokens should not fail: %v", err) + } +} diff --git a/internal/cmd/auth/login.go b/internal/cmd/auth/login.go new file mode 100644 index 0000000..f57ab5f --- /dev/null +++ b/internal/cmd/auth/login.go @@ -0,0 +1,165 @@ +package auth + +import ( + "context" + "fmt" + "html" + "net" + "net/http" + "os/exec" + "runtime" + "strings" + "time" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + "golang.org/x/oauth2" + + xoauth "github.com/rootlyhq/rootly-cli/internal/oauth" +) + +const callbackPort = "19797" + +var LoginCmd = &cobra.Command{ + Use: "login", + Short: "Authenticate with Rootly via browser-based OAuth2", + Long: `Opens your browser to authenticate with Rootly using OAuth2 Authorization Code + PKCE. + +No configuration is needed — just run "rootly login" and follow the browser prompts. +By default, connects to api.rootly.com. Use --api-host to target a different environment.`, + Example: ` # Login to Rootly (production) + rootly login + + # Login to a local dev server + rootly login --api-host=localhost:22166`, + RunE: runLogin, +} + +func init() { + LoginCmd.Flags().String("client-id", "", "Override OAuth2 client ID (for debugging)") + _ = LoginCmd.Flags().MarkHidden("client-id") +} + +func runLogin(cmd *cobra.Command, args []string) error { + apiHost := viper.GetString("api_host") + if apiHost == "" { + apiHost = "api.rootly.com" + } + + authBaseURL := deriveAuthBaseURL(apiHost) + cfg := xoauth.NewConfig(authBaseURL) + + // Allow client-id override for debugging + if clientID, _ := cmd.Flags().GetString("client-id"); clientID != "" { + cfg.ClientID = clientID + } + + verifier := oauth2.GenerateVerifier() + + state, err := xoauth.GenerateState() + if err != nil { + return fmt.Errorf("failed to generate state: %w", err) + } + + // Channel to receive the authorization code + codeCh := make(chan string, 1) + errCh := make(chan error, 1) + + // Start local callback server + mux := http.NewServeMux() + mux.HandleFunc("/callback", func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("state") != state { + errCh <- fmt.Errorf("state mismatch") + http.Error(w, "State mismatch", http.StatusBadRequest) + return + } + if errMsg := r.URL.Query().Get("error"); errMsg != "" { + desc := r.URL.Query().Get("error_description") + errCh <- fmt.Errorf("authorization error: %s — %s", errMsg, desc) + fmt.Fprintf(w, "

Authorization Failed

%s

You can close this window.

", html.EscapeString(desc)) + return + } + code := r.URL.Query().Get("code") + if code == "" { + errCh <- fmt.Errorf("no code in callback") + http.Error(w, "Missing code", http.StatusBadRequest) + return + } + codeCh <- code + fmt.Fprint(w, "

Login Successful!

You can close this window and return to the terminal.

") + }) + + listener, err := net.Listen("tcp", "localhost:"+callbackPort) + if err != nil { + return fmt.Errorf("failed to start callback server on port %s: %w", callbackPort, err) + } + + server := &http.Server{Handler: mux} + go func() { _ = server.Serve(listener) }() + defer func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _ = server.Shutdown(ctx) + }() + + // Build authorization URL with PKCE (S256 challenge derived from verifier) + authURL := cfg.AuthCodeURL(state, oauth2.S256ChallengeOption(verifier)) + + fmt.Fprintf(cmd.OutOrStderr(), "Opening browser for authentication...\n") + fmt.Fprintf(cmd.OutOrStderr(), "If the browser doesn't open, visit:\n%s\n\n", authURL) + + if err := openBrowser(authURL); err != nil { + fmt.Fprintf(cmd.OutOrStderr(), "Failed to open browser: %v\n", err) + } + + fmt.Fprintf(cmd.OutOrStderr(), "Waiting for authorization...\n") + + // Wait for callback + var code string + select { + case code = <-codeCh: + case err := <-errCh: + return err + case <-time.After(5 * time.Minute): + return fmt.Errorf("login timed out after 5 minutes") + } + + // Exchange code for tokens + tok, err := xoauth.ExchangeCode(context.Background(), cfg, code, verifier) + if err != nil { + return err + } + + if err := xoauth.SaveOAuth2Token(tok); err != nil { + return fmt.Errorf("failed to save tokens: %w", err) + } + + fmt.Fprintf(cmd.OutOrStderr(), "Login successful! Tokens saved.\n") + return nil +} + +func deriveAuthBaseURL(apiHost string) string { + if strings.HasPrefix(apiHost, "http://") || strings.HasPrefix(apiHost, "https://") { + return apiHost + } + if strings.HasPrefix(apiHost, "localhost") || strings.HasPrefix(apiHost, "127.0.0.1") { + return "http://" + apiHost + } + if strings.HasPrefix(apiHost, "api.") { + return "https://app." + apiHost[4:] + } + return "https://" + apiHost +} + +func openBrowser(url string) error { + switch runtime.GOOS { + case "darwin": + return exec.Command("open", url).Start() + case "linux": + return exec.Command("xdg-open", url).Start() + case "windows": + return exec.Command("rundll32", "url.dll,FileProtocolHandler", url).Start() + default: + return fmt.Errorf("unsupported platform") + } +} diff --git a/internal/cmd/auth/logout.go b/internal/cmd/auth/logout.go new file mode 100644 index 0000000..75a2f24 --- /dev/null +++ b/internal/cmd/auth/logout.go @@ -0,0 +1,21 @@ +package auth + +import ( + "fmt" + + "github.com/spf13/cobra" + + "github.com/rootlyhq/rootly-cli/internal/oauth" +) + +var LogoutCmd = &cobra.Command{ + Use: "logout", + Short: "Clear stored OAuth2 tokens", + RunE: func(cmd *cobra.Command, args []string) error { + if err := oauth.ClearTokens(); err != nil { + return fmt.Errorf("failed to clear tokens: %w", err) + } + fmt.Fprintf(cmd.OutOrStderr(), "Logged out. OAuth tokens cleared.\n") + return nil + }, +} diff --git a/internal/cmd/auth_register.go b/internal/cmd/auth_register.go new file mode 100644 index 0000000..74d0b5f --- /dev/null +++ b/internal/cmd/auth_register.go @@ -0,0 +1,8 @@ +package cmd + +import "github.com/rootlyhq/rootly-cli/internal/cmd/auth" + +func init() { + rootCmd.AddCommand(auth.LoginCmd) + rootCmd.AddCommand(auth.LogoutCmd) +} diff --git a/internal/cmd/incidents/cmd_test.go b/internal/cmd/incidents/cmd_test.go index 3f2e3a1..d463ceb 100644 --- a/internal/cmd/incidents/cmd_test.go +++ b/internal/cmd/incidents/cmd_test.go @@ -383,6 +383,7 @@ func TestRunUpdateNoFlags(t *testing.T) { func TestRunListNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().Int("page", 1, "") @@ -395,8 +396,8 @@ func TestRunListNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } @@ -484,6 +485,7 @@ func TestRunDeleteNoConfirmNonInteractive(t *testing.T) { func TestRunDeleteNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().BoolP("yes", "y", false, "") @@ -493,8 +495,8 @@ func TestRunDeleteNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } diff --git a/internal/cmd/incidents/incidents.go b/internal/cmd/incidents/incidents.go index 87bc892..7ad0e99 100644 --- a/internal/cmd/incidents/incidents.go +++ b/internal/cmd/incidents/incidents.go @@ -8,6 +8,7 @@ import ( "github.com/rootlyhq/rootly-cli/internal/api" "github.com/rootlyhq/rootly-cli/internal/config" + "github.com/rootlyhq/rootly-cli/internal/oauth" ) // IncidentsCmd is the parent command for all incident operations @@ -37,7 +38,9 @@ var IncidentsCmd = &cobra.Command{ func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - return nil, fmt.Errorf("API key required: set ROOTLY_API_KEY or add api_key to ~/.rootly-cli/config.yaml") + if _, err := oauth.LoadTokens(); err != nil { + return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") + } } endpoint := viper.GetString("api_host") if endpoint == "" { diff --git a/internal/cmd/oncall/cmd_test.go b/internal/cmd/oncall/cmd_test.go index 6167c8e..bfed413 100644 --- a/internal/cmd/oncall/cmd_test.go +++ b/internal/cmd/oncall/cmd_test.go @@ -202,6 +202,7 @@ func TestRunListPagination(t *testing.T) { func TestRunListNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().Int("page", 1, "") @@ -211,8 +212,8 @@ func TestRunListNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } @@ -564,6 +565,7 @@ func TestRunWhoWithScheduleFilter(t *testing.T) { func TestRunShiftsNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().Int("days", 7, "") @@ -578,14 +580,15 @@ func TestRunShiftsNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } func TestRunWhoNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().String("schedule-id", "", "") diff --git a/internal/cmd/oncall/oncall.go b/internal/cmd/oncall/oncall.go index 418d885..aa694fb 100644 --- a/internal/cmd/oncall/oncall.go +++ b/internal/cmd/oncall/oncall.go @@ -8,6 +8,7 @@ import ( "github.com/rootlyhq/rootly-cli/internal/api" "github.com/rootlyhq/rootly-cli/internal/config" + "github.com/rootlyhq/rootly-cli/internal/oauth" ) // OncallCmd is the parent command for all on-call operations @@ -40,7 +41,9 @@ Note: Schedules are managed in the Rootly UI. This command provides read-only ac func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - return nil, fmt.Errorf("API key required: set ROOTLY_API_KEY or add api_key to ~/.rootly-cli/config.yaml") + if _, err := oauth.LoadTokens(); err != nil { + return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") + } } endpoint := viper.GetString("api_host") if endpoint == "" { diff --git a/internal/cmd/pulse/cmd_test.go b/internal/cmd/pulse/cmd_test.go index 308d89d..5b71aa0 100644 --- a/internal/cmd/pulse/cmd_test.go +++ b/internal/cmd/pulse/cmd_test.go @@ -192,6 +192,7 @@ func TestRunCreateNoSummary(t *testing.T) { func TestRunCreateNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().StringP("labels", "l", "", "") @@ -205,8 +206,8 @@ func TestRunCreateNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API key") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } @@ -498,6 +499,7 @@ func TestRunRunNoArgs(t *testing.T) { func TestRunRunNoAPIKey(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().String("summary", "", "") @@ -511,8 +513,8 @@ func TestRunRunNoAPIKey(t *testing.T) { if err == nil { t.Fatal("expected error when no API key") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } diff --git a/internal/cmd/pulse/pulse.go b/internal/cmd/pulse/pulse.go index 6a1dfd9..85caab3 100644 --- a/internal/cmd/pulse/pulse.go +++ b/internal/cmd/pulse/pulse.go @@ -8,6 +8,7 @@ import ( "github.com/rootlyhq/rootly-cli/internal/api" "github.com/rootlyhq/rootly-cli/internal/config" + "github.com/rootlyhq/rootly-cli/internal/oauth" ) // PulseCmd is the parent command for pulse operations @@ -34,7 +35,9 @@ var PulseCmd = &cobra.Command{ func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - return nil, fmt.Errorf("API key required: set ROOTLY_API_KEY or add api_key to ~/.rootly-cli/config.yaml") + if _, err := oauth.LoadTokens(); err != nil { + return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") + } } endpoint := viper.GetString("api_host") if endpoint == "" { diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 14a91b2..231549d 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -43,10 +43,21 @@ Or use a config file at ~/.rootly-cli/config.yaml: SilenceUsage: true, SilenceErrors: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - // Bind flags to viper + // Bind flags to viper (both local and inherited persistent flags) if err := viper.BindPFlags(cmd.Flags()); err != nil { return err } + if err := viper.BindPFlags(cmd.InheritedFlags()); err != nil { + return err + } + + // Explicitly bind hyphenated flags to underscore viper keys + if f := cmd.Flag("api-host"); f != nil && f.Changed { + viper.Set("api_host", f.Value.String()) + } + if f := cmd.Flag("api-key"); f != nil && f.Changed { + viper.Set("api_key", f.Value.String()) + } // Configure config file viper.SetConfigName("config") @@ -74,7 +85,7 @@ Or use a config file at ~/.rootly-cli/config.yaml: // Skip auth validation for commands that don't need it cmdName := cmd.Name() - if cmdName == "version" || cmdName == "completion" || cmdName == "help" { + if cmdName == "version" || cmdName == "completion" || cmdName == "help" || cmdName == "login" || cmdName == "logout" { return nil } diff --git a/internal/cmd/services/cmd_test.go b/internal/cmd/services/cmd_test.go index d856f14..e38a3ae 100644 --- a/internal/cmd/services/cmd_test.go +++ b/internal/cmd/services/cmd_test.go @@ -152,6 +152,7 @@ func TestRunListJSON(t *testing.T) { func TestRunListNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().Int("page", 1, "") @@ -164,8 +165,8 @@ func TestRunListNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } @@ -377,6 +378,7 @@ func TestRunDeleteNoConfirmNonInteractive(t *testing.T) { func TestRunDeleteNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().BoolP("yes", "y", false, "") @@ -386,8 +388,8 @@ func TestRunDeleteNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } diff --git a/internal/cmd/services/services.go b/internal/cmd/services/services.go index 4558e91..2b64338 100644 --- a/internal/cmd/services/services.go +++ b/internal/cmd/services/services.go @@ -8,6 +8,7 @@ import ( "github.com/rootlyhq/rootly-cli/internal/api" "github.com/rootlyhq/rootly-cli/internal/config" + "github.com/rootlyhq/rootly-cli/internal/oauth" ) // ServicesCmd is the parent command for all service operations @@ -37,7 +38,9 @@ var ServicesCmd = &cobra.Command{ func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - return nil, fmt.Errorf("API key required: set ROOTLY_API_KEY or add api_key to ~/.rootly-cli/config.yaml") + if _, err := oauth.LoadTokens(); err != nil { + return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") + } } endpoint := viper.GetString("api_host") if endpoint == "" { diff --git a/internal/cmd/teams/cmd_test.go b/internal/cmd/teams/cmd_test.go index 56f5b0e..4b4fd88 100644 --- a/internal/cmd/teams/cmd_test.go +++ b/internal/cmd/teams/cmd_test.go @@ -150,6 +150,7 @@ func TestRunListJSON(t *testing.T) { func TestRunListNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().Int("page", 1, "") @@ -161,8 +162,8 @@ func TestRunListNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } @@ -374,6 +375,7 @@ func TestRunDeleteNoConfirmNonInteractive(t *testing.T) { func TestRunDeleteNoToken(t *testing.T) { viper.Reset() defer viper.Reset() + t.Setenv("HOME", t.TempDir()) cmd := newTestCmd() cmd.Flags().BoolP("yes", "y", false, "") @@ -383,8 +385,8 @@ func TestRunDeleteNoToken(t *testing.T) { if err == nil { t.Fatal("expected error when no API token") } - if !strings.Contains(err.Error(), "API key required") { - t.Errorf("expected 'API key required' error, got: %v", err) + if !strings.Contains(err.Error(), "authentication required") { + t.Errorf("expected 'authentication required' error, got: %v", err) } } diff --git a/internal/cmd/teams/teams.go b/internal/cmd/teams/teams.go index 5ea99fe..f767237 100644 --- a/internal/cmd/teams/teams.go +++ b/internal/cmd/teams/teams.go @@ -8,6 +8,7 @@ import ( "github.com/rootlyhq/rootly-cli/internal/api" "github.com/rootlyhq/rootly-cli/internal/config" + "github.com/rootlyhq/rootly-cli/internal/oauth" ) // TeamsCmd is the parent command for all team operations @@ -37,7 +38,9 @@ var TeamsCmd = &cobra.Command{ func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - return nil, fmt.Errorf("API key required: set ROOTLY_API_KEY or add api_key to ~/.rootly-cli/config.yaml") + if _, err := oauth.LoadTokens(); err != nil { + return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") + } } endpoint := viper.GetString("api_host") if endpoint == "" { diff --git a/internal/config/config.go b/internal/config/config.go index 303e9ab..3d37fe4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -20,10 +20,10 @@ const ( type Config struct { APIKey string `yaml:"api_key"` Endpoint string `yaml:"api_host"` - Timezone string `yaml:"timezone"` // TUI-specific - Language string `yaml:"language"` // TUI-specific - Layout string `yaml:"layout"` // TUI-specific - Debug bool `yaml:"-"` // Runtime only, not persisted + Timezone string `yaml:"timezone"` // TUI-specific + Language string `yaml:"language"` // TUI-specific + Layout string `yaml:"layout"` // TUI-specific + Debug bool `yaml:"-"` // Runtime only, not persisted } const DefaultTimezone = "UTC" diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go new file mode 100644 index 0000000..4ca4921 --- /dev/null +++ b/internal/oauth/oauth.go @@ -0,0 +1,52 @@ +package oauth + +import ( + "context" + "crypto/rand" + "encoding/base64" + + "golang.org/x/oauth2" +) + +const ( + ClientID = "rootly-cli" + RedirectURI = "http://localhost:19797/callback" +) + +// NewConfig creates an oauth2.Config for the given auth base URL. +func NewConfig(authBaseURL string) *oauth2.Config { + return &oauth2.Config{ + ClientID: ClientID, + RedirectURL: RedirectURI, + Scopes: []string{"openid", "profile", "email", "all"}, + Endpoint: oauth2.Endpoint{ + AuthURL: authBaseURL + "/oauth/authorize", + TokenURL: authBaseURL + "/oauth/token", + AuthStyle: oauth2.AuthStyleInParams, + }, + } +} + +// GenerateState creates a cryptographically random state parameter. +func GenerateState() (string, error) { + b := make([]byte, 32) + if _, err := rand.Read(b); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(b), nil +} + +// ExchangeCode exchanges an authorization code for tokens using PKCE. +func ExchangeCode(ctx context.Context, cfg *oauth2.Config, code, codeVerifier string) (*oauth2.Token, error) { + return cfg.Exchange(ctx, code, oauth2.VerifierOption(codeVerifier)) +} + +// TokenSourceFromStored creates a token source that auto-refreshes using stored tokens. +func TokenSourceFromStored(cfg *oauth2.Config) (oauth2.TokenSource, error) { + stored, err := LoadTokens() + if err != nil { + return nil, err + } + tok := stored.ToOAuth2Token() + return cfg.TokenSource(context.Background(), tok), nil +} diff --git a/internal/oauth/oauth_test.go b/internal/oauth/oauth_test.go new file mode 100644 index 0000000..9dd1e43 --- /dev/null +++ b/internal/oauth/oauth_test.go @@ -0,0 +1,36 @@ +package oauth + +import ( + "testing" +) + +func TestNewConfig(t *testing.T) { + cfg := NewConfig("https://app.rootly.com") + + if cfg.ClientID != "rootly-cli" { + t.Errorf("ClientID = %q", cfg.ClientID) + } + if cfg.RedirectURL != "http://localhost:19797/callback" { + t.Errorf("RedirectURL = %q", cfg.RedirectURL) + } + if cfg.Endpoint.AuthURL != "https://app.rootly.com/oauth/authorize" { + t.Errorf("AuthURL = %q", cfg.Endpoint.AuthURL) + } + if cfg.Endpoint.TokenURL != "https://app.rootly.com/oauth/token" { + t.Errorf("TokenURL = %q", cfg.Endpoint.TokenURL) + } + if len(cfg.Scopes) != 4 { + t.Errorf("Scopes = %v", cfg.Scopes) + } +} + +func TestNewConfig_Localhost(t *testing.T) { + cfg := NewConfig("http://localhost:22166") + + if cfg.Endpoint.AuthURL != "http://localhost:22166/oauth/authorize" { + t.Errorf("AuthURL = %q", cfg.Endpoint.AuthURL) + } + if cfg.Endpoint.TokenURL != "http://localhost:22166/oauth/token" { + t.Errorf("TokenURL = %q", cfg.Endpoint.TokenURL) + } +} diff --git a/internal/oauth/tokens.go b/internal/oauth/tokens.go new file mode 100644 index 0000000..a2206d3 --- /dev/null +++ b/internal/oauth/tokens.go @@ -0,0 +1,91 @@ +package oauth + +import ( + "os" + "path/filepath" + "time" + + "golang.org/x/oauth2" + "gopkg.in/yaml.v3" +) + +const ( + tokenDir = ".rootly-cli" + tokenFile = "tokens.yaml" +) + +type TokenData struct { + AccessToken string `yaml:"access_token"` + RefreshToken string `yaml:"refresh_token"` + ExpiresAt time.Time `yaml:"expires_at"` + TokenType string `yaml:"token_type"` +} + +func tokenPath() string { + home, err := os.UserHomeDir() + if err != nil { + return "" + } + return filepath.Join(home, tokenDir, tokenFile) +} + +func LoadTokens() (*TokenData, error) { + data, err := os.ReadFile(tokenPath()) + if err != nil { + return nil, err + } + var t TokenData + if err := yaml.Unmarshal(data, &t); err != nil { + return nil, err + } + return &t, nil +} + +func SaveTokens(t *TokenData) error { + dir := filepath.Dir(tokenPath()) + if err := os.MkdirAll(dir, 0700); err != nil { + return err + } + data, err := yaml.Marshal(t) + if err != nil { + return err + } + return os.WriteFile(tokenPath(), data, 0600) +} + +// SaveOAuth2Token converts and persists an oauth2.Token. +func SaveOAuth2Token(tok *oauth2.Token) error { + return SaveTokens(TokenDataFromOAuth2(tok)) +} + +func ClearTokens() error { + path := tokenPath() + if _, err := os.Stat(path); os.IsNotExist(err) { + return nil + } + return os.Remove(path) +} + +func (t *TokenData) IsExpired() bool { + return time.Now().After(t.ExpiresAt.Add(-30 * time.Second)) +} + +// ToOAuth2Token converts stored token data to an oauth2.Token. +func (t *TokenData) ToOAuth2Token() *oauth2.Token { + return &oauth2.Token{ + AccessToken: t.AccessToken, + RefreshToken: t.RefreshToken, + TokenType: t.TokenType, + Expiry: t.ExpiresAt, + } +} + +// TokenDataFromOAuth2 converts an oauth2.Token to TokenData for storage. +func TokenDataFromOAuth2(tok *oauth2.Token) *TokenData { + return &TokenData{ + AccessToken: tok.AccessToken, + RefreshToken: tok.RefreshToken, + ExpiresAt: tok.Expiry, + TokenType: tok.TokenType, + } +} diff --git a/internal/oauth/tokens_test.go b/internal/oauth/tokens_test.go new file mode 100644 index 0000000..a023696 --- /dev/null +++ b/internal/oauth/tokens_test.go @@ -0,0 +1,112 @@ +package oauth + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +func TestTokenData_IsExpired(t *testing.T) { + tests := []struct { + name string + expires time.Time + want bool + }{ + {"future", time.Now().Add(10 * time.Minute), false}, + {"past", time.Now().Add(-10 * time.Minute), true}, + {"within 30s buffer", time.Now().Add(20 * time.Second), true}, + {"just outside buffer", time.Now().Add(60 * time.Second), false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + td := &TokenData{ExpiresAt: tt.expires} + if got := td.IsExpired(); got != tt.want { + t.Errorf("IsExpired() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestSaveAndLoadTokens(t *testing.T) { + // Use temp dir to avoid touching real config + tmpDir := t.TempDir() + origHome := os.Getenv("HOME") + t.Setenv("HOME", tmpDir) + defer os.Setenv("HOME", origHome) + + tokens := &TokenData{ + AccessToken: "test-access", + RefreshToken: "test-refresh", + ExpiresAt: time.Now().Add(1 * time.Hour).Truncate(time.Second), + TokenType: "Bearer", + } + + if err := SaveTokens(tokens); err != nil { + t.Fatalf("SaveTokens: %v", err) + } + + // Verify file permissions + path := filepath.Join(tmpDir, tokenDir, tokenFile) + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat token file: %v", err) + } + if perm := info.Mode().Perm(); perm != 0600 { + t.Errorf("file permissions = %o, want 0600", perm) + } + + loaded, err := LoadTokens() + if err != nil { + t.Fatalf("LoadTokens: %v", err) + } + if loaded.AccessToken != tokens.AccessToken { + t.Errorf("AccessToken = %q, want %q", loaded.AccessToken, tokens.AccessToken) + } + if loaded.RefreshToken != tokens.RefreshToken { + t.Errorf("RefreshToken = %q, want %q", loaded.RefreshToken, tokens.RefreshToken) + } +} + +func TestClearTokens(t *testing.T) { + tmpDir := t.TempDir() + origHome := os.Getenv("HOME") + t.Setenv("HOME", tmpDir) + defer os.Setenv("HOME", origHome) + + tokens := &TokenData{AccessToken: "x", RefreshToken: "y", ExpiresAt: time.Now().Add(time.Hour)} + _ = SaveTokens(tokens) + + if err := ClearTokens(); err != nil { + t.Fatalf("ClearTokens: %v", err) + } + + _, err := LoadTokens() + if err == nil { + t.Error("expected error after clearing tokens") + } +} + +func TestClearTokens_NoFile(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + // Should not error when file doesn't exist + if err := ClearTokens(); err != nil { + t.Fatalf("ClearTokens on missing file: %v", err) + } +} + +func TestLoadTokens_InvalidYAML(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + dir := filepath.Join(tmpDir, tokenDir) + os.MkdirAll(dir, 0700) + os.WriteFile(filepath.Join(dir, tokenFile), []byte("not: [valid: yaml: {{"), 0600) + + _, err := LoadTokens() + if err == nil { + t.Error("expected error for invalid YAML") + } +} diff --git a/internal/oauth/transport.go b/internal/oauth/transport.go new file mode 100644 index 0000000..a97370e --- /dev/null +++ b/internal/oauth/transport.go @@ -0,0 +1,63 @@ +package oauth + +import ( + "net/http" + + "golang.org/x/oauth2" +) + +// NewHTTPClient creates an http.Client that uses stored OAuth tokens with auto-refresh. +// The userAgent is set on all requests. The base transport is used for underlying HTTP calls. +func NewHTTPClient(cfg *oauth2.Config, base http.RoundTripper, userAgent string) (*http.Client, error) { + ts, err := TokenSourceFromStored(cfg) + if err != nil { + return nil, err + } + + // Wrap the token source to save refreshed tokens + ts = &persistingTokenSource{ + base: ts, + } + + transport := &oauth2.Transport{ + Source: ts, + Base: base, + } + + // Wrap with user-agent transport + return &http.Client{ + Transport: &userAgentTransport{ + base: transport, + userAgent: userAgent, + }, + }, nil +} + +// persistingTokenSource wraps a TokenSource and saves new tokens to disk. +type persistingTokenSource struct { + base oauth2.TokenSource +} + +func (p *persistingTokenSource) Token() (*oauth2.Token, error) { + tok, err := p.base.Token() + if err != nil { + return nil, err + } + // Save on every call — oauth2.ReuseTokenSource ensures this only happens on refresh + _ = SaveOAuth2Token(tok) + return tok, nil +} + +// userAgentTransport sets User-Agent on all requests. +type userAgentTransport struct { + base http.RoundTripper + userAgent string +} + +func (t *userAgentTransport) RoundTrip(req *http.Request) (*http.Response, error) { + req2 := req.Clone(req.Context()) + if t.userAgent != "" { + req2.Header.Set("User-Agent", t.userAgent) + } + return t.base.RoundTrip(req2) +} diff --git a/internal/oauth/transport_test.go b/internal/oauth/transport_test.go new file mode 100644 index 0000000..f1c46b6 --- /dev/null +++ b/internal/oauth/transport_test.go @@ -0,0 +1,137 @@ +package oauth + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "golang.org/x/oauth2" +) + +func TestNewHTTPClient_SetsAuthAndUserAgent(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + tokens := &TokenData{ + AccessToken: "my-token", + RefreshToken: "my-refresh", + ExpiresAt: time.Now().Add(1 * time.Hour), + TokenType: "Bearer", + } + if err := SaveTokens(tokens); err != nil { + t.Fatal(err) + } + + var gotAuth, gotUA string + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + gotUA = r.Header.Get("User-Agent") + w.WriteHeader(200) + })) + defer backend.Close() + + cfg := &oauth2.Config{ + ClientID: "test", + Endpoint: oauth2.Endpoint{ + TokenURL: backend.URL + "/oauth/token", + AuthStyle: oauth2.AuthStyleInParams, + }, + } + + client, err := NewHTTPClient(cfg, http.DefaultTransport, "rootly-cli/test") + if err != nil { + t.Fatalf("NewHTTPClient: %v", err) + } + + resp, err := client.Get(backend.URL + "/test") + if err != nil { + t.Fatalf("request failed: %v", err) + } + resp.Body.Close() + + if gotAuth != "Bearer my-token" { + t.Errorf("Authorization = %q, want %q", gotAuth, "Bearer my-token") + } + if gotUA != "rootly-cli/test" { + t.Errorf("User-Agent = %q, want %q", gotUA, "rootly-cli/test") + } +} + +func TestNewHTTPClient_NoTokens(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + cfg := &oauth2.Config{ + ClientID: "test", + Endpoint: oauth2.Endpoint{TokenURL: "http://localhost/token"}, + } + + _, err := NewHTTPClient(cfg, http.DefaultTransport, "") + if err == nil { + t.Error("expected error when no tokens exist") + } +} + +func TestNewHTTPClient_RefreshesExpiredToken(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + // Token server for refresh + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]interface{}{ + "access_token": "fresh-token", + "refresh_token": "fresh-refresh", + "expires_in": 3600, + "token_type": "Bearer", + }) + })) + defer tokenServer.Close() + + // Save expired tokens + tokens := &TokenData{ + AccessToken: "expired-token", + RefreshToken: "valid-refresh", + ExpiresAt: time.Now().Add(-1 * time.Hour), + TokenType: "Bearer", + } + SaveTokens(tokens) + + cfg := &oauth2.Config{ + ClientID: "test", + Endpoint: oauth2.Endpoint{ + TokenURL: tokenServer.URL, + AuthStyle: oauth2.AuthStyleInParams, + }, + } + + client, err := NewHTTPClient(cfg, http.DefaultTransport, "") + if err != nil { + t.Fatalf("NewHTTPClient: %v", err) + } + + var gotAuth string + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + w.WriteHeader(200) + })) + defer backend.Close() + + resp, err := client.Get(backend.URL + "/test") + if err != nil { + t.Fatalf("request failed: %v", err) + } + resp.Body.Close() + + if gotAuth != "Bearer fresh-token" { + t.Errorf("Authorization = %q, want %q", gotAuth, "Bearer fresh-token") + } + + // Verify refreshed tokens were persisted + saved, _ := LoadTokens() + if saved.AccessToken != "fresh-token" { + t.Errorf("saved AccessToken = %q", saved.AccessToken) + } +} From f6a947ca43bd1045298c07f250a4eab23003d57a Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 17:43:52 -0700 Subject: [PATCH 02/16] =?UTF-8?q?=F0=9F=90=9B=20fix:=20resolve=20CI=20lint?= =?UTF-8?q?=20and=20Go=20version=20compatibility=20issues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Downgrade golang.org/x/oauth2 to v0.28.0 (Go 1.24 compatible) - Pin go.mod to go 1.24.2 (CI runs Go 1.24) - Fix errcheck: add _, _ to fmt.Fprint/Fprintf return values - Fix noctx: use net.ListenConfig.Listen and exec.CommandContext - Fix staticcheck ST1023: use type inference for transport - Fix goimports: align config.go struct fields - Fix noctx in tests: use http.NewRequestWithContext instead of client.Get 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- go.mod | 4 ++-- go.sum | 4 ++-- internal/api/client.go | 2 +- internal/cmd/auth/login.go | 35 +++++++++++++++++--------------- internal/config/config.go | 8 ++++---- internal/oauth/transport_test.go | 7 +++++-- 6 files changed, 33 insertions(+), 27 deletions(-) diff --git a/go.mod b/go.mod index 02a0e2f..6c95a06 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/rootlyhq/rootly-cli -go 1.25.0 +go 1.24.2 require ( github.com/jedib0t/go-pretty/v6 v6.7.8 @@ -8,7 +8,7 @@ require ( github.com/rootlyhq/rootly-go v0.8.0 github.com/spf13/cobra v1.10.2 github.com/spf13/viper v1.21.0 - golang.org/x/oauth2 v0.36.0 + golang.org/x/oauth2 v0.28.0 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index 2bfb605..2e99bbf 100644 --- a/go.sum +++ b/go.sum @@ -92,8 +92,8 @@ github.com/woodsbury/decimal128 v1.4.0 h1:xJATj7lLu4f2oObouMt2tgGiElE5gO6mSWUjQs github.com/woodsbury/decimal128 v1.4.0/go.mod h1:BP46FUrVjVhdTbKT+XuQh2xfQaGki9LMIRJSFuh6THU= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= -golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= +golang.org/x/oauth2 v0.28.0 h1:CrgCKl8PPAVtLnU3c+EDw6x11699EWlsDeWNWKdIOkc= +golang.org/x/oauth2 v0.28.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k= golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= diff --git a/internal/api/client.go b/internal/api/client.go index ce5313d..31e442a 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -748,7 +748,7 @@ func NewClient(cfg *config.Config) (*Client, error) { } // Build base transport - var transport http.RoundTripper = http.DefaultTransport + transport := http.RoundTripper(http.DefaultTransport) if cfg.Debug { transport = &debugTransport{transport: transport} diff --git a/internal/cmd/auth/login.go b/internal/cmd/auth/login.go index f57ab5f..4ba9461 100644 --- a/internal/cmd/auth/login.go +++ b/internal/cmd/auth/login.go @@ -41,6 +41,8 @@ func init() { } func runLogin(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + apiHost := viper.GetString("api_host") if apiHost == "" { apiHost = "api.rootly.com" @@ -76,7 +78,7 @@ func runLogin(cmd *cobra.Command, args []string) error { if errMsg := r.URL.Query().Get("error"); errMsg != "" { desc := r.URL.Query().Get("error_description") errCh <- fmt.Errorf("authorization error: %s — %s", errMsg, desc) - fmt.Fprintf(w, "

Authorization Failed

%s

You can close this window.

", html.EscapeString(desc)) + _, _ = fmt.Fprintf(w, "

Authorization Failed

%s

You can close this window.

", html.EscapeString(desc)) return } code := r.URL.Query().Get("code") @@ -86,10 +88,11 @@ func runLogin(cmd *cobra.Command, args []string) error { return } codeCh <- code - fmt.Fprint(w, "

Login Successful!

You can close this window and return to the terminal.

") + _, _ = fmt.Fprint(w, "

Login Successful!

You can close this window and return to the terminal.

") }) - listener, err := net.Listen("tcp", "localhost:"+callbackPort) + lc := net.ListenConfig{} + listener, err := lc.Listen(ctx, "tcp", "localhost:"+callbackPort) if err != nil { return fmt.Errorf("failed to start callback server on port %s: %w", callbackPort, err) } @@ -97,22 +100,22 @@ func runLogin(cmd *cobra.Command, args []string) error { server := &http.Server{Handler: mux} go func() { _ = server.Serve(listener) }() defer func() { - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + shutdownCtx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - _ = server.Shutdown(ctx) + _ = server.Shutdown(shutdownCtx) }() // Build authorization URL with PKCE (S256 challenge derived from verifier) authURL := cfg.AuthCodeURL(state, oauth2.S256ChallengeOption(verifier)) - fmt.Fprintf(cmd.OutOrStderr(), "Opening browser for authentication...\n") - fmt.Fprintf(cmd.OutOrStderr(), "If the browser doesn't open, visit:\n%s\n\n", authURL) + _, _ = fmt.Fprintf(cmd.OutOrStderr(), "Opening browser for authentication...\n") + _, _ = fmt.Fprintf(cmd.OutOrStderr(), "If the browser doesn't open, visit:\n%s\n\n", authURL) - if err := openBrowser(authURL); err != nil { - fmt.Fprintf(cmd.OutOrStderr(), "Failed to open browser: %v\n", err) + if err := openBrowser(ctx, authURL); err != nil { + _, _ = fmt.Fprintf(cmd.OutOrStderr(), "Failed to open browser: %v\n", err) } - fmt.Fprintf(cmd.OutOrStderr(), "Waiting for authorization...\n") + _, _ = fmt.Fprintf(cmd.OutOrStderr(), "Waiting for authorization...\n") // Wait for callback var code string @@ -125,7 +128,7 @@ func runLogin(cmd *cobra.Command, args []string) error { } // Exchange code for tokens - tok, err := xoauth.ExchangeCode(context.Background(), cfg, code, verifier) + tok, err := xoauth.ExchangeCode(ctx, cfg, code, verifier) if err != nil { return err } @@ -134,7 +137,7 @@ func runLogin(cmd *cobra.Command, args []string) error { return fmt.Errorf("failed to save tokens: %w", err) } - fmt.Fprintf(cmd.OutOrStderr(), "Login successful! Tokens saved.\n") + _, _ = fmt.Fprintf(cmd.OutOrStderr(), "Login successful! Tokens saved.\n") return nil } @@ -151,14 +154,14 @@ func deriveAuthBaseURL(apiHost string) string { return "https://" + apiHost } -func openBrowser(url string) error { +func openBrowser(ctx context.Context, url string) error { switch runtime.GOOS { case "darwin": - return exec.Command("open", url).Start() + return exec.CommandContext(ctx, "open", url).Start() case "linux": - return exec.Command("xdg-open", url).Start() + return exec.CommandContext(ctx, "xdg-open", url).Start() case "windows": - return exec.Command("rundll32", "url.dll,FileProtocolHandler", url).Start() + return exec.CommandContext(ctx, "rundll32", "url.dll,FileProtocolHandler", url).Start() default: return fmt.Errorf("unsupported platform") } diff --git a/internal/config/config.go b/internal/config/config.go index 3d37fe4..303e9ab 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -20,10 +20,10 @@ const ( type Config struct { APIKey string `yaml:"api_key"` Endpoint string `yaml:"api_host"` - Timezone string `yaml:"timezone"` // TUI-specific - Language string `yaml:"language"` // TUI-specific - Layout string `yaml:"layout"` // TUI-specific - Debug bool `yaml:"-"` // Runtime only, not persisted + Timezone string `yaml:"timezone"` // TUI-specific + Language string `yaml:"language"` // TUI-specific + Layout string `yaml:"layout"` // TUI-specific + Debug bool `yaml:"-"` // Runtime only, not persisted } const DefaultTimezone = "UTC" diff --git a/internal/oauth/transport_test.go b/internal/oauth/transport_test.go index f1c46b6..035f21e 100644 --- a/internal/oauth/transport_test.go +++ b/internal/oauth/transport_test.go @@ -1,6 +1,7 @@ package oauth import ( + "context" "encoding/json" "net/http" "net/http/httptest" @@ -45,7 +46,8 @@ func TestNewHTTPClient_SetsAuthAndUserAgent(t *testing.T) { t.Fatalf("NewHTTPClient: %v", err) } - resp, err := client.Get(backend.URL + "/test") + req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, backend.URL+"/test", nil) + resp, err := client.Do(req) if err != nil { t.Fatalf("request failed: %v", err) } @@ -119,7 +121,8 @@ func TestNewHTTPClient_RefreshesExpiredToken(t *testing.T) { })) defer backend.Close() - resp, err := client.Get(backend.URL + "/test") + req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, backend.URL+"/test", nil) + resp, err := client.Do(req) if err != nil { t.Fatalf("request failed: %v", err) } From 724eb4dcd2fc596c8e4f261fd3fba39545a4ce87 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 17:45:41 -0700 Subject: [PATCH 03/16] =?UTF-8?q?=F0=9F=90=9B=20fix:=20resolve=20remaining?= =?UTF-8?q?=20lint=20issues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - errcheck: check fmt.Fprintf return in logout.go - gocritic/httpNoBody: use http.NoBody instead of nil in test requests - unconvert: split transport declaration and assignment to satisfy both staticcheck and unconvert 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/api/client.go | 3 ++- internal/cmd/auth/logout.go | 2 +- internal/oauth/transport_test.go | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 31e442a..c2b1486 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -748,7 +748,8 @@ func NewClient(cfg *config.Config) (*Client, error) { } // Build base transport - transport := http.RoundTripper(http.DefaultTransport) + var transport http.RoundTripper //nolint:staticcheck // need explicit type for reassignment + transport = http.DefaultTransport if cfg.Debug { transport = &debugTransport{transport: transport} diff --git a/internal/cmd/auth/logout.go b/internal/cmd/auth/logout.go index 75a2f24..7150975 100644 --- a/internal/cmd/auth/logout.go +++ b/internal/cmd/auth/logout.go @@ -15,7 +15,7 @@ var LogoutCmd = &cobra.Command{ if err := oauth.ClearTokens(); err != nil { return fmt.Errorf("failed to clear tokens: %w", err) } - fmt.Fprintf(cmd.OutOrStderr(), "Logged out. OAuth tokens cleared.\n") + _, _ = fmt.Fprintf(cmd.OutOrStderr(), "Logged out. OAuth tokens cleared.\n") return nil }, } diff --git a/internal/oauth/transport_test.go b/internal/oauth/transport_test.go index 035f21e..71bd34b 100644 --- a/internal/oauth/transport_test.go +++ b/internal/oauth/transport_test.go @@ -46,7 +46,7 @@ func TestNewHTTPClient_SetsAuthAndUserAgent(t *testing.T) { t.Fatalf("NewHTTPClient: %v", err) } - req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, backend.URL+"/test", nil) + req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, backend.URL+"/test", http.NoBody) resp, err := client.Do(req) if err != nil { t.Fatalf("request failed: %v", err) @@ -121,7 +121,7 @@ func TestNewHTTPClient_RefreshesExpiredToken(t *testing.T) { })) defer backend.Close() - req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, backend.URL+"/test", nil) + req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, backend.URL+"/test", http.NoBody) resp, err := client.Do(req) if err != nil { t.Fatalf("request failed: %v", err) From e892f7e95104d5d18284ed4f420b25540f3eca92 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 17:47:35 -0700 Subject: [PATCH 04/16] =?UTF-8?q?=F0=9F=90=9B=20fix:=20remove=20unused=20n?= =?UTF-8?q?olint=20directive?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/api/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/client.go b/internal/api/client.go index c2b1486..d7245f7 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -748,7 +748,7 @@ func NewClient(cfg *config.Config) (*Client, error) { } // Build base transport - var transport http.RoundTripper //nolint:staticcheck // need explicit type for reassignment + var transport http.RoundTripper transport = http.DefaultTransport if cfg.Debug { From f3b1f49a4b8abfe2f46fb76c6dc36686fa839836 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 17:51:41 -0700 Subject: [PATCH 05/16] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor:=20consolid?= =?UTF-8?q?ate=20DeriveAuthBaseURL=20into=20oauth=20package?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove duplicate deriveAuthBaseURL from client.go and login.go - Export as oauth.DeriveAuthBaseURL for shared use - Remove dead `_ = t` assignment in client.go - Move DeriveAuthBaseURL tests to oauth package 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/api/client.go | 18 ++---------------- internal/cmd/auth/cmd_test.go | 24 ------------------------ internal/cmd/auth/login.go | 15 +-------------- internal/oauth/oauth.go | 17 +++++++++++++++++ internal/oauth/oauth_test.go | 24 ++++++++++++++++++++++++ 5 files changed, 44 insertions(+), 54 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index d7245f7..730dc83 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -741,8 +741,7 @@ func NewClient(cfg *config.Config) (*Client, error) { // Determine auth: use OAuth tokens only when no API key is set useOAuth := false if cfg.APIKey == "" { - if t, err := oauth.LoadTokens(); err == nil && t.AccessToken != "" { - _ = t + if tokens, err := oauth.LoadTokens(); err == nil && tokens.AccessToken != "" { useOAuth = true } } @@ -757,7 +756,7 @@ func NewClient(cfg *config.Config) (*Client, error) { var httpClient *http.Client if useOAuth { - authBaseURL := deriveAuthBaseURL(cfg.Endpoint) + authBaseURL := oauth.DeriveAuthBaseURL(cfg.Endpoint) oauthCfg := oauth.NewConfig(authBaseURL) var err error httpClient, err = oauth.NewHTTPClient(oauthCfg, transport, "rootly-cli/"+Version) @@ -812,19 +811,6 @@ func ensureScheme(url string) string { return "https://" + url } -// deriveAuthBaseURL builds the OAuth base URL from the API host. -func deriveAuthBaseURL(apiHost string) string { - if strings.HasPrefix(apiHost, "http://") || strings.HasPrefix(apiHost, "https://") { - return apiHost - } - if strings.HasPrefix(apiHost, "localhost") || strings.HasPrefix(apiHost, "127.0.0.1") { - return "http://" + apiHost - } - if strings.HasPrefix(apiHost, "api.") { - return "https://app." + apiHost[4:] - } - return "https://" + apiHost -} func (c *Client) ValidateAPIKey(ctx context.Context) error { // Use /v1/users/me endpoint to validate the API key diff --git a/internal/cmd/auth/cmd_test.go b/internal/cmd/auth/cmd_test.go index 05a9db3..d2ea34f 100644 --- a/internal/cmd/auth/cmd_test.go +++ b/internal/cmd/auth/cmd_test.go @@ -9,30 +9,6 @@ import ( "github.com/rootlyhq/rootly-cli/internal/oauth" ) -func TestDeriveAuthBaseURL(t *testing.T) { - tests := []struct { - input string - want string - }{ - {"api.rootly.com", "https://app.rootly.com"}, - {"api.staging.rootly.com", "https://app.staging.rootly.com"}, - {"localhost:22166", "http://localhost:22166"}, - {"localhost:22166/api", "http://localhost:22166/api"}, - {"127.0.0.1:3000", "http://127.0.0.1:3000"}, - {"http://localhost:22166", "http://localhost:22166"}, - {"https://custom.example.com", "https://custom.example.com"}, - {"custom.example.com", "https://custom.example.com"}, - } - for _, tt := range tests { - t.Run(tt.input, func(t *testing.T) { - got := deriveAuthBaseURL(tt.input) - if got != tt.want { - t.Errorf("deriveAuthBaseURL(%q) = %q, want %q", tt.input, got, tt.want) - } - }) - } -} - func TestLogoutCmd(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) diff --git a/internal/cmd/auth/login.go b/internal/cmd/auth/login.go index 4ba9461..1c6a0a9 100644 --- a/internal/cmd/auth/login.go +++ b/internal/cmd/auth/login.go @@ -8,7 +8,6 @@ import ( "net/http" "os/exec" "runtime" - "strings" "time" "github.com/spf13/cobra" @@ -48,7 +47,7 @@ func runLogin(cmd *cobra.Command, args []string) error { apiHost = "api.rootly.com" } - authBaseURL := deriveAuthBaseURL(apiHost) + authBaseURL := xoauth.DeriveAuthBaseURL(apiHost) cfg := xoauth.NewConfig(authBaseURL) // Allow client-id override for debugging @@ -141,18 +140,6 @@ func runLogin(cmd *cobra.Command, args []string) error { return nil } -func deriveAuthBaseURL(apiHost string) string { - if strings.HasPrefix(apiHost, "http://") || strings.HasPrefix(apiHost, "https://") { - return apiHost - } - if strings.HasPrefix(apiHost, "localhost") || strings.HasPrefix(apiHost, "127.0.0.1") { - return "http://" + apiHost - } - if strings.HasPrefix(apiHost, "api.") { - return "https://app." + apiHost[4:] - } - return "https://" + apiHost -} func openBrowser(ctx context.Context, url string) error { switch runtime.GOOS { diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go index 4ca4921..794069f 100644 --- a/internal/oauth/oauth.go +++ b/internal/oauth/oauth.go @@ -4,6 +4,7 @@ import ( "context" "crypto/rand" "encoding/base64" + "strings" "golang.org/x/oauth2" ) @@ -41,6 +42,22 @@ func ExchangeCode(ctx context.Context, cfg *oauth2.Config, code, codeVerifier st return cfg.Exchange(ctx, code, oauth2.VerifierOption(codeVerifier)) } +// DeriveAuthBaseURL builds the OAuth base URL from the API host. +// For api.rootly.com it returns https://app.rootly.com. +// For localhost it returns http://localhost:. +func DeriveAuthBaseURL(apiHost string) string { + if strings.HasPrefix(apiHost, "http://") || strings.HasPrefix(apiHost, "https://") { + return apiHost + } + if strings.HasPrefix(apiHost, "localhost") || strings.HasPrefix(apiHost, "127.0.0.1") { + return "http://" + apiHost + } + if strings.HasPrefix(apiHost, "api.") { + return "https://app." + apiHost[4:] + } + return "https://" + apiHost +} + // TokenSourceFromStored creates a token source that auto-refreshes using stored tokens. func TokenSourceFromStored(cfg *oauth2.Config) (oauth2.TokenSource, error) { stored, err := LoadTokens() diff --git a/internal/oauth/oauth_test.go b/internal/oauth/oauth_test.go index 9dd1e43..e5451fe 100644 --- a/internal/oauth/oauth_test.go +++ b/internal/oauth/oauth_test.go @@ -34,3 +34,27 @@ func TestNewConfig_Localhost(t *testing.T) { t.Errorf("TokenURL = %q", cfg.Endpoint.TokenURL) } } + +func TestDeriveAuthBaseURL(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"api.rootly.com", "https://app.rootly.com"}, + {"api.staging.rootly.com", "https://app.staging.rootly.com"}, + {"localhost:22166", "http://localhost:22166"}, + {"localhost:22166/api", "http://localhost:22166/api"}, + {"127.0.0.1:3000", "http://127.0.0.1:3000"}, + {"http://localhost:22166", "http://localhost:22166"}, + {"https://custom.example.com", "https://custom.example.com"}, + {"custom.example.com", "https://custom.example.com"}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got := DeriveAuthBaseURL(tt.input) + if got != tt.want { + t.Errorf("DeriveAuthBaseURL(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} From c786503ee88e7d4bd80159634d1f965d0552edda Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 17:54:18 -0700 Subject: [PATCH 06/16] =?UTF-8?q?=F0=9F=92=84=20style:=20remove=20extra=20?= =?UTF-8?q?blank=20lines=20flagged=20by=20goimports?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/api/client.go | 1 - internal/cmd/auth/login.go | 1 - 2 files changed, 2 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 730dc83..4d416a0 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -811,7 +811,6 @@ func ensureScheme(url string) string { return "https://" + url } - func (c *Client) ValidateAPIKey(ctx context.Context) error { // Use /v1/users/me endpoint to validate the API key resp, err := c.client.GetCurrentUserWithResponse(ctx) diff --git a/internal/cmd/auth/login.go b/internal/cmd/auth/login.go index 1c6a0a9..8cbb0fd 100644 --- a/internal/cmd/auth/login.go +++ b/internal/cmd/auth/login.go @@ -140,7 +140,6 @@ func runLogin(cmd *cobra.Command, args []string) error { return nil } - func openBrowser(ctx context.Context, url string) error { switch runtime.GOOS { case "darwin": From a0bca7930c208139ccaddd221f301dd8a4cb39ff Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 18:00:02 -0700 Subject: [PATCH 07/16] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor:=20simplify?= =?UTF-8?q?=20OAuth=20integration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove 24 redundant ensureScheme() calls — endpoint already normalized in NewClient() - Add oauth.HasTokens() for cheap file-exists check, avoid double LoadTokens() reads - Replace fragile command-name string matching with Annotations["skipAuth"] pattern - Update getAPIClient() in all 6 resource packages to use HasTokens() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/api/client.go | 48 ++++++++++++++--------------- internal/cmd/alerts/alerts.go | 2 +- internal/cmd/auth/login.go | 3 +- internal/cmd/auth/logout.go | 5 +-- internal/cmd/completion.go | 1 + internal/cmd/incidents/incidents.go | 2 +- internal/cmd/oncall/oncall.go | 2 +- internal/cmd/pulse/pulse.go | 2 +- internal/cmd/root.go | 5 ++- internal/cmd/services/services.go | 2 +- internal/cmd/teams/teams.go | 2 +- internal/cmd/version.go | 7 +++-- internal/oauth/tokens.go | 6 ++++ 13 files changed, 48 insertions(+), 39 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 4d416a0..e5c1729 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -906,7 +906,7 @@ func (c *Client) ListIncidentsCLI(ctx context.Context, page, pageSize int, sort // Build URL with query parameters baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/incidents?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { @@ -1006,7 +1006,7 @@ func (c *Client) ListIncidentsCLI(ctx context.Context, page, pageSize int, sort func (c *Client) GetIncidentByID(ctx context.Context, id string) (*Incident, error) { // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/incidents/%s?include=roles,causes,incident_types,functionalities,services,environments,groups,user", baseURL, id) req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) if err != nil { @@ -1079,7 +1079,7 @@ func (c *Client) CreateIncident(ctx context.Context, title string, opts map[stri // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/incidents", baseURL) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(bodyBytes))) @@ -1155,7 +1155,7 @@ func (c *Client) UpdateIncident(ctx context.Context, id string, opts map[string] // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/incidents/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "PUT", url, strings.NewReader(string(bodyBytes))) @@ -1206,7 +1206,7 @@ func (c *Client) UpdateIncident(ctx context.Context, id string, opts map[string] func (c *Client) DeleteIncident(ctx context.Context, id string) error { // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/incidents/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "DELETE", url, http.NoBody) @@ -1323,7 +1323,7 @@ func (c *Client) ListAlertsCLI(ctx context.Context, page, pageSize int, sort str // Build URL with query parameters baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/alerts?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { @@ -1423,7 +1423,7 @@ func (c *Client) ListAlertsCLI(ctx context.Context, page, pageSize int, sort str func (c *Client) GetAlertByID(ctx context.Context, id string) (*Alert, error) { // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/alerts/%s?include=services,environments,groups,responders,alert_urgency", baseURL, id) req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) if err != nil { @@ -1659,7 +1659,7 @@ func (c *Client) CreateAlertCLI(ctx context.Context, summary string, opts map[st // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/alerts", baseURL) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(bodyBytes))) @@ -1738,7 +1738,7 @@ func (c *Client) UpdateAlertCLI(ctx context.Context, id string, opts map[string] // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/alerts/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "PUT", url, strings.NewReader(string(bodyBytes))) @@ -1789,7 +1789,7 @@ func (c *Client) UpdateAlertCLI(ctx context.Context, id string, opts map[string] func (c *Client) AcknowledgeAlertCLI(ctx context.Context, id string) error { // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/alerts/%s/acknowledge", baseURL, id) req, err := http.NewRequestWithContext(ctx, "POST", url, http.NoBody) @@ -1825,7 +1825,7 @@ func (c *Client) AcknowledgeAlertCLI(ctx context.Context, id string) error { func (c *Client) ResolveAlertCLI(ctx context.Context, id, resolutionMessage string, resolveIncidents bool) error { // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/alerts/%s/resolve", baseURL, id) var reqBody io.Reader = http.NoBody @@ -1895,7 +1895,7 @@ func (c *Client) ListServicesCLI(ctx context.Context, page, pageSize int, sort s // Build URL with query parameters baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/services?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { @@ -1998,7 +1998,7 @@ func (c *Client) ListServicesCLI(ctx context.Context, page, pageSize int, sort s func (c *Client) GetServiceByID(ctx context.Context, id string) (*Service, error) { // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/services/%s?include=owner_group", baseURL, id) req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) if err != nil { @@ -2122,7 +2122,7 @@ func (c *Client) CreateService(ctx context.Context, name string, opts map[string // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/services", baseURL) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(bodyBytes))) @@ -2219,7 +2219,7 @@ func (c *Client) UpdateService(ctx context.Context, id string, opts map[string]s // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/services/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "PUT", url, strings.NewReader(string(bodyBytes))) @@ -2294,7 +2294,7 @@ func (c *Client) UpdateService(ctx context.Context, id string, opts map[string]s func (c *Client) DeleteService(ctx context.Context, id string) error { // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/services/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "DELETE", url, http.NoBody) @@ -2338,7 +2338,7 @@ func (c *Client) ListTeamsCLI(ctx context.Context, page, pageSize int, sort stri // Build URL with query parameters baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/teams?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { @@ -2443,7 +2443,7 @@ func (c *Client) ListTeamsCLI(ctx context.Context, page, pageSize int, sort stri func (c *Client) GetTeamByID(ctx context.Context, id string) (*Team, error) { // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/teams/%s?include=users", baseURL, id) req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) if err != nil { @@ -2568,7 +2568,7 @@ func (c *Client) CreateTeam(ctx context.Context, name string, opts map[string]st // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/teams", baseURL) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(bodyBytes))) @@ -2658,7 +2658,7 @@ func (c *Client) UpdateTeam(ctx context.Context, id string, opts map[string]stri // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/teams/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "PATCH", url, strings.NewReader(string(bodyBytes))) @@ -2737,7 +2737,7 @@ func (c *Client) UpdateTeam(ctx context.Context, id string, opts map[string]stri func (c *Client) DeleteTeam(ctx context.Context, id string) error { // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/teams/%s", baseURL, id) req, err := http.NewRequestWithContext(ctx, "DELETE", url, http.NoBody) @@ -2837,7 +2837,7 @@ func (c *Client) ListSchedulesCLI(ctx context.Context, page, pageSize int, filte // Build URL with query parameters baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/schedules?page[number]=%d&page[size]=%d", baseURL, page, pageSize) @@ -2926,7 +2926,7 @@ func (c *Client) ListSchedulesCLI(ctx context.Context, page, pageSize int, filte // ListOnCallsCLI lists on-call entries using the unified /v1/oncalls endpoint. func (c *Client) ListOnCallsCLI(ctx context.Context, params OnCallsParams) (*OnCallsResult, error) { baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/oncalls?", baseURL) @@ -3121,7 +3121,7 @@ func (c *Client) CreatePulseCLI(ctx context.Context, summary string, opts PulseO // Build URL baseURL := c.endpoint - baseURL = ensureScheme(baseURL) + url := fmt.Sprintf("%s/v1/pulses", baseURL) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(bodyBytes))) diff --git a/internal/cmd/alerts/alerts.go b/internal/cmd/alerts/alerts.go index c44e44f..faa1e3d 100644 --- a/internal/cmd/alerts/alerts.go +++ b/internal/cmd/alerts/alerts.go @@ -41,7 +41,7 @@ var AlertsCmd = &cobra.Command{ func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - if _, err := oauth.LoadTokens(); err != nil { + if !oauth.HasTokens() { return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") } } diff --git a/internal/cmd/auth/login.go b/internal/cmd/auth/login.go index 8cbb0fd..37c8c16 100644 --- a/internal/cmd/auth/login.go +++ b/internal/cmd/auth/login.go @@ -31,7 +31,8 @@ By default, connects to api.rootly.com. Use --api-host to target a different env # Login to a local dev server rootly login --api-host=localhost:22166`, - RunE: runLogin, + Annotations: map[string]string{"skipAuth": "true"}, + RunE: runLogin, } func init() { diff --git a/internal/cmd/auth/logout.go b/internal/cmd/auth/logout.go index 7150975..368d110 100644 --- a/internal/cmd/auth/logout.go +++ b/internal/cmd/auth/logout.go @@ -9,8 +9,9 @@ import ( ) var LogoutCmd = &cobra.Command{ - Use: "logout", - Short: "Clear stored OAuth2 tokens", + Use: "logout", + Short: "Clear stored OAuth2 tokens", + Annotations: map[string]string{"skipAuth": "true"}, RunE: func(cmd *cobra.Command, args []string) error { if err := oauth.ClearTokens(); err != nil { return fmt.Errorf("failed to clear tokens: %w", err) diff --git a/internal/cmd/completion.go b/internal/cmd/completion.go index 6b4949f..9b80b2c 100644 --- a/internal/cmd/completion.go +++ b/internal/cmd/completion.go @@ -28,6 +28,7 @@ PowerShell: DisableFlagsInUseLine: true, ValidArgs: []string{"bash", "zsh", "fish", "powershell"}, Args: cobra.MatchAll(cobra.ExactArgs(1), cobra.OnlyValidArgs), + Annotations: map[string]string{"skipAuth": "true"}, RunE: func(cmd *cobra.Command, args []string) error { switch args[0] { case "bash": diff --git a/internal/cmd/incidents/incidents.go b/internal/cmd/incidents/incidents.go index 7ad0e99..b6ecf23 100644 --- a/internal/cmd/incidents/incidents.go +++ b/internal/cmd/incidents/incidents.go @@ -38,7 +38,7 @@ var IncidentsCmd = &cobra.Command{ func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - if _, err := oauth.LoadTokens(); err != nil { + if !oauth.HasTokens() { return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") } } diff --git a/internal/cmd/oncall/oncall.go b/internal/cmd/oncall/oncall.go index aa694fb..3587346 100644 --- a/internal/cmd/oncall/oncall.go +++ b/internal/cmd/oncall/oncall.go @@ -41,7 +41,7 @@ Note: Schedules are managed in the Rootly UI. This command provides read-only ac func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - if _, err := oauth.LoadTokens(); err != nil { + if !oauth.HasTokens() { return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") } } diff --git a/internal/cmd/pulse/pulse.go b/internal/cmd/pulse/pulse.go index 85caab3..2ad104f 100644 --- a/internal/cmd/pulse/pulse.go +++ b/internal/cmd/pulse/pulse.go @@ -35,7 +35,7 @@ var PulseCmd = &cobra.Command{ func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - if _, err := oauth.LoadTokens(); err != nil { + if !oauth.HasTokens() { return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") } } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 231549d..41fba8b 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -83,9 +83,8 @@ Or use a config file at ~/.rootly-cli/config.yaml: // Config file not found is OK - we'll use flags/env vars } - // Skip auth validation for commands that don't need it - cmdName := cmd.Name() - if cmdName == "version" || cmdName == "completion" || cmdName == "help" || cmdName == "login" || cmdName == "logout" { + // Skip auth validation for commands that opt out + if cmd.Annotations["skipAuth"] == "true" { return nil } diff --git a/internal/cmd/services/services.go b/internal/cmd/services/services.go index 2b64338..fbc7b6e 100644 --- a/internal/cmd/services/services.go +++ b/internal/cmd/services/services.go @@ -38,7 +38,7 @@ var ServicesCmd = &cobra.Command{ func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - if _, err := oauth.LoadTokens(); err != nil { + if !oauth.HasTokens() { return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") } } diff --git a/internal/cmd/teams/teams.go b/internal/cmd/teams/teams.go index f767237..5956753 100644 --- a/internal/cmd/teams/teams.go +++ b/internal/cmd/teams/teams.go @@ -38,7 +38,7 @@ var TeamsCmd = &cobra.Command{ func getAPIClient() (*api.Client, error) { token := viper.GetString("api_key") if token == "" { - if _, err := oauth.LoadTokens(); err != nil { + if !oauth.HasTokens() { return nil, fmt.Errorf("authentication required: run 'rootly login' or set ROOTLY_API_KEY") } } diff --git a/internal/cmd/version.go b/internal/cmd/version.go index 8326279..ee5f56c 100644 --- a/internal/cmd/version.go +++ b/internal/cmd/version.go @@ -9,9 +9,10 @@ import ( ) var versionCmd = &cobra.Command{ - Use: "version", - Short: "Print version information", - Long: `Print the version, commit hash, and build date of the rootly CLI.`, + Use: "version", + Short: "Print version information", + Long: `Print the version, commit hash, and build date of the rootly CLI.`, + Annotations: map[string]string{"skipAuth": "true"}, RunE: func(cmd *cobra.Command, args []string) error { format := viper.GetString("format") diff --git a/internal/oauth/tokens.go b/internal/oauth/tokens.go index a2206d3..105638a 100644 --- a/internal/oauth/tokens.go +++ b/internal/oauth/tokens.go @@ -58,6 +58,12 @@ func SaveOAuth2Token(tok *oauth2.Token) error { return SaveTokens(TokenDataFromOAuth2(tok)) } +// HasTokens returns true if a token file exists (cheap stat, no parsing). +func HasTokens() bool { + _, err := os.Stat(tokenPath()) + return err == nil +} + func ClearTokens() error { path := tokenPath() if _, err := os.Stat(path); os.IsNotExist(err) { From 6fff5ada4597c4bba85ff11f3a5bdf94142b41b3 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 18:03:31 -0700 Subject: [PATCH 08/16] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor:=20merge=20?= =?UTF-8?q?OAuth=20tokens=20into=20config.yaml?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Store OAuth tokens under an `oauth` key in ~/.rootly-cli/config.yaml instead of a separate tokens.yaml file. One config file to rule them all. - Add OAuthData struct to config.Config with `yaml:"oauth,omitempty"` - Rewrite oauth/tokens.go to read/write from config.yaml, preserving existing fields - SaveTokens/ClearTokens now do read-modify-write to preserve api_key, api_host, etc. - Remove separate tokens.yaml file concept entirely - Update all tests for new storage format 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/config/config.go | 21 +++++-- internal/oauth/oauth.go | 2 +- internal/oauth/tokens.go | 104 ++++++++++++++++++++++------------ internal/oauth/tokens_test.go | 99 +++++++++++++++++++++++++++----- 4 files changed, 170 insertions(+), 56 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 303e9ab..3f82b6d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -18,12 +18,21 @@ const ( ) type Config struct { - APIKey string `yaml:"api_key"` - Endpoint string `yaml:"api_host"` - Timezone string `yaml:"timezone"` // TUI-specific - Language string `yaml:"language"` // TUI-specific - Layout string `yaml:"layout"` // TUI-specific - Debug bool `yaml:"-"` // Runtime only, not persisted + APIKey string `yaml:"api_key"` + Endpoint string `yaml:"api_host"` + Timezone string `yaml:"timezone"` // TUI-specific + Language string `yaml:"language"` // TUI-specific + Layout string `yaml:"layout"` // TUI-specific + OAuth *OAuthData `yaml:"oauth,omitempty"` + Debug bool `yaml:"-"` // Runtime only, not persisted +} + +// OAuthData holds OAuth2 token data within the config file. +type OAuthData struct { + AccessToken string `yaml:"access_token"` + RefreshToken string `yaml:"refresh_token"` + ExpiresAt time.Time `yaml:"expires_at"` + TokenType string `yaml:"token_type"` } const DefaultTimezone = "UTC" diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go index 794069f..9ea4447 100644 --- a/internal/oauth/oauth.go +++ b/internal/oauth/oauth.go @@ -64,6 +64,6 @@ func TokenSourceFromStored(cfg *oauth2.Config) (oauth2.TokenSource, error) { if err != nil { return nil, err } - tok := stored.ToOAuth2Token() + tok := ToOAuth2Token(stored) return cfg.TokenSource(context.Background(), tok), nil } diff --git a/internal/oauth/tokens.go b/internal/oauth/tokens.go index 105638a..483cc4c 100644 --- a/internal/oauth/tokens.go +++ b/internal/oauth/tokens.go @@ -2,55 +2,54 @@ package oauth import ( "os" - "path/filepath" "time" "golang.org/x/oauth2" "gopkg.in/yaml.v3" -) -const ( - tokenDir = ".rootly-cli" - tokenFile = "tokens.yaml" + "github.com/rootlyhq/rootly-cli/internal/config" ) -type TokenData struct { - AccessToken string `yaml:"access_token"` - RefreshToken string `yaml:"refresh_token"` - ExpiresAt time.Time `yaml:"expires_at"` - TokenType string `yaml:"token_type"` -} - -func tokenPath() string { - home, err := os.UserHomeDir() - if err != nil { - return "" - } - return filepath.Join(home, tokenDir, tokenFile) -} +// TokenData is an alias for config.OAuthData used within the oauth package. +type TokenData = config.OAuthData +// LoadTokens reads OAuth tokens from ~/.rootly-cli/config.yaml. func LoadTokens() (*TokenData, error) { - data, err := os.ReadFile(tokenPath()) + data, err := os.ReadFile(config.Path()) if err != nil { return nil, err } - var t TokenData - if err := yaml.Unmarshal(data, &t); err != nil { + var cfg config.Config + if err := yaml.Unmarshal(data, &cfg); err != nil { return nil, err } - return &t, nil + if cfg.OAuth == nil || cfg.OAuth.AccessToken == "" { + return nil, os.ErrNotExist + } + return cfg.OAuth, nil } +// SaveTokens writes OAuth tokens into ~/.rootly-cli/config.yaml, +// preserving all other config fields. func SaveTokens(t *TokenData) error { - dir := filepath.Dir(tokenPath()) - if err := os.MkdirAll(dir, 0700); err != nil { + path := config.Path() + + // Read existing config to preserve other fields + var cfg config.Config + if data, err := os.ReadFile(path); err == nil { + _ = yaml.Unmarshal(data, &cfg) + } + + cfg.OAuth = t + + if err := os.MkdirAll(config.Dir(), 0700); err != nil { return err } - data, err := yaml.Marshal(t) + data, err := yaml.Marshal(&cfg) if err != nil { return err } - return os.WriteFile(tokenPath(), data, 0600) + return os.WriteFile(path, data, 0600) } // SaveOAuth2Token converts and persists an oauth2.Token. @@ -58,26 +57,61 @@ func SaveOAuth2Token(tok *oauth2.Token) error { return SaveTokens(TokenDataFromOAuth2(tok)) } -// HasTokens returns true if a token file exists (cheap stat, no parsing). +// HasTokens returns true if OAuth tokens exist in the config file (cheap check). func HasTokens() bool { - _, err := os.Stat(tokenPath()) - return err == nil + data, err := os.ReadFile(config.Path()) + if err != nil { + return false + } + // Quick check without full unmarshal + var cfg struct { + OAuth *struct { + AccessToken string `yaml:"access_token"` + } `yaml:"oauth"` + } + if err := yaml.Unmarshal(data, &cfg); err != nil { + return false + } + return cfg.OAuth != nil && cfg.OAuth.AccessToken != "" } +// ClearTokens removes OAuth tokens from ~/.rootly-cli/config.yaml, +// preserving all other config fields. func ClearTokens() error { - path := tokenPath() - if _, err := os.Stat(path); os.IsNotExist(err) { + path := config.Path() + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + + var cfg config.Config + if err := yaml.Unmarshal(data, &cfg); err != nil { + return err + } + + if cfg.OAuth == nil { return nil } - return os.Remove(path) + + cfg.OAuth = nil + + out, err := yaml.Marshal(&cfg) + if err != nil { + return err + } + return os.WriteFile(path, out, 0600) } -func (t *TokenData) IsExpired() bool { +// IsExpired returns true if the token is expired or within 30s of expiring. +func IsExpired(t *TokenData) bool { return time.Now().After(t.ExpiresAt.Add(-30 * time.Second)) } // ToOAuth2Token converts stored token data to an oauth2.Token. -func (t *TokenData) ToOAuth2Token() *oauth2.Token { +func ToOAuth2Token(t *TokenData) *oauth2.Token { return &oauth2.Token{ AccessToken: t.AccessToken, RefreshToken: t.RefreshToken, diff --git a/internal/oauth/tokens_test.go b/internal/oauth/tokens_test.go index a023696..f089349 100644 --- a/internal/oauth/tokens_test.go +++ b/internal/oauth/tokens_test.go @@ -7,7 +7,7 @@ import ( "time" ) -func TestTokenData_IsExpired(t *testing.T) { +func TestIsExpired(t *testing.T) { tests := []struct { name string expires time.Time @@ -21,7 +21,7 @@ func TestTokenData_IsExpired(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { td := &TokenData{ExpiresAt: tt.expires} - if got := td.IsExpired(); got != tt.want { + if got := IsExpired(td); got != tt.want { t.Errorf("IsExpired() = %v, want %v", got, tt.want) } }) @@ -29,11 +29,8 @@ func TestTokenData_IsExpired(t *testing.T) { } func TestSaveAndLoadTokens(t *testing.T) { - // Use temp dir to avoid touching real config tmpDir := t.TempDir() - origHome := os.Getenv("HOME") t.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", origHome) tokens := &TokenData{ AccessToken: "test-access", @@ -47,10 +44,10 @@ func TestSaveAndLoadTokens(t *testing.T) { } // Verify file permissions - path := filepath.Join(tmpDir, tokenDir, tokenFile) + path := filepath.Join(tmpDir, ".rootly-cli", "config.yaml") info, err := os.Stat(path) if err != nil { - t.Fatalf("stat token file: %v", err) + t.Fatalf("stat config file: %v", err) } if perm := info.Mode().Perm(); perm != 0600 { t.Errorf("file permissions = %o, want 0600", perm) @@ -68,11 +65,51 @@ func TestSaveAndLoadTokens(t *testing.T) { } } +func TestSaveTokens_PreservesExistingConfig(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + // Write a config with an API key first + dir := filepath.Join(tmpDir, ".rootly-cli") + os.MkdirAll(dir, 0700) + os.WriteFile(filepath.Join(dir, "config.yaml"), []byte("api_key: my-key\napi_host: custom.rootly.com\n"), 0600) + + // Save tokens + tokens := &TokenData{AccessToken: "tok", RefreshToken: "ref", ExpiresAt: time.Now().Add(time.Hour)} + if err := SaveTokens(tokens); err != nil { + t.Fatalf("SaveTokens: %v", err) + } + + // Verify existing fields preserved + data, _ := os.ReadFile(filepath.Join(dir, "config.yaml")) + content := string(data) + if !contains(content, "api_key: my-key") { + t.Errorf("api_key not preserved in config:\n%s", content) + } + if !contains(content, "api_host: custom.rootly.com") { + t.Errorf("api_host not preserved in config:\n%s", content) + } + if !contains(content, "access_token: tok") { + t.Errorf("oauth tokens not written:\n%s", content) + } +} + +func contains(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsSubstring(s, substr)) +} + +func containsSubstring(s, sub string) bool { + for i := 0; i <= len(s)-len(sub); i++ { + if s[i:i+len(sub)] == sub { + return true + } + } + return false +} + func TestClearTokens(t *testing.T) { tmpDir := t.TempDir() - origHome := os.Getenv("HOME") t.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", origHome) tokens := &TokenData{AccessToken: "x", RefreshToken: "y", ExpiresAt: time.Now().Add(time.Hour)} _ = SaveTokens(tokens) @@ -87,26 +124,60 @@ func TestClearTokens(t *testing.T) { } } +func TestClearTokens_PreservesConfig(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + // Write config with API key + tokens + dir := filepath.Join(tmpDir, ".rootly-cli") + os.MkdirAll(dir, 0700) + os.WriteFile(filepath.Join(dir, "config.yaml"), []byte("api_key: my-key\noauth:\n access_token: tok\n refresh_token: ref\n"), 0600) + + if err := ClearTokens(); err != nil { + t.Fatalf("ClearTokens: %v", err) + } + + // API key should still be there + data, _ := os.ReadFile(filepath.Join(dir, "config.yaml")) + if !contains(string(data), "api_key: my-key") { + t.Errorf("api_key not preserved after clear:\n%s", string(data)) + } +} + func TestClearTokens_NoFile(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) - // Should not error when file doesn't exist if err := ClearTokens(); err != nil { t.Fatalf("ClearTokens on missing file: %v", err) } } -func TestLoadTokens_InvalidYAML(t *testing.T) { +func TestHasTokens(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + + if HasTokens() { + t.Error("HasTokens should be false with no config") + } + + SaveTokens(&TokenData{AccessToken: "x", RefreshToken: "y", ExpiresAt: time.Now().Add(time.Hour)}) + + if !HasTokens() { + t.Error("HasTokens should be true after saving tokens") + } +} + +func TestLoadTokens_NoOAuthSection(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) - dir := filepath.Join(tmpDir, tokenDir) + dir := filepath.Join(tmpDir, ".rootly-cli") os.MkdirAll(dir, 0700) - os.WriteFile(filepath.Join(dir, tokenFile), []byte("not: [valid: yaml: {{"), 0600) + os.WriteFile(filepath.Join(dir, "config.yaml"), []byte("api_key: my-key\n"), 0600) _, err := LoadTokens() if err == nil { - t.Error("expected error for invalid YAML") + t.Error("expected error when no oauth section exists") } } From e29c8b79855f1dfeb125ec5a75284ab9133c85b8 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 18:05:27 -0700 Subject: [PATCH 09/16] =?UTF-8?q?=F0=9F=90=9B=20fix:=20lint=20=E2=80=94=20?= =?UTF-8?q?use=20strings.Contains=20in=20test,=20remove=20double=20blank?= =?UTF-8?q?=20lines?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/api/client.go | 6 ------ internal/oauth/tokens_test.go | 21 +++++---------------- 2 files changed, 5 insertions(+), 22 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index e5c1729..477d978 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -907,7 +907,6 @@ func (c *Client) ListIncidentsCLI(ctx context.Context, page, pageSize int, sort // Build URL with query parameters baseURL := c.endpoint - url := fmt.Sprintf("%s/v1/incidents?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { url += fmt.Sprintf("&sort=%s", sort) @@ -1324,7 +1323,6 @@ func (c *Client) ListAlertsCLI(ctx context.Context, page, pageSize int, sort str // Build URL with query parameters baseURL := c.endpoint - url := fmt.Sprintf("%s/v1/alerts?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { url += fmt.Sprintf("&sort=%s", sort) @@ -1896,7 +1894,6 @@ func (c *Client) ListServicesCLI(ctx context.Context, page, pageSize int, sort s // Build URL with query parameters baseURL := c.endpoint - url := fmt.Sprintf("%s/v1/services?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { url += fmt.Sprintf("&sort=%s", sort) @@ -2339,7 +2336,6 @@ func (c *Client) ListTeamsCLI(ctx context.Context, page, pageSize int, sort stri // Build URL with query parameters baseURL := c.endpoint - url := fmt.Sprintf("%s/v1/teams?page[number]=%d&page[size]=%d", baseURL, page, pageSize) if sort != "" { url += fmt.Sprintf("&sort=%s", sort) @@ -2838,7 +2834,6 @@ func (c *Client) ListSchedulesCLI(ctx context.Context, page, pageSize int, filte // Build URL with query parameters baseURL := c.endpoint - url := fmt.Sprintf("%s/v1/schedules?page[number]=%d&page[size]=%d", baseURL, page, pageSize) // Add filters (e.g., filter[name]=foo) @@ -2927,7 +2922,6 @@ func (c *Client) ListSchedulesCLI(ctx context.Context, page, pageSize int, filte func (c *Client) ListOnCallsCLI(ctx context.Context, params OnCallsParams) (*OnCallsResult, error) { baseURL := c.endpoint - url := fmt.Sprintf("%s/v1/oncalls?", baseURL) qp := make([]string, 0) diff --git a/internal/oauth/tokens_test.go b/internal/oauth/tokens_test.go index f089349..288b168 100644 --- a/internal/oauth/tokens_test.go +++ b/internal/oauth/tokens_test.go @@ -3,6 +3,7 @@ package oauth import ( "os" "path/filepath" + "strings" "testing" "time" ) @@ -83,29 +84,17 @@ func TestSaveTokens_PreservesExistingConfig(t *testing.T) { // Verify existing fields preserved data, _ := os.ReadFile(filepath.Join(dir, "config.yaml")) content := string(data) - if !contains(content, "api_key: my-key") { + if !strings.Contains(content, "api_key: my-key") { t.Errorf("api_key not preserved in config:\n%s", content) } - if !contains(content, "api_host: custom.rootly.com") { + if !strings.Contains(content, "api_host: custom.rootly.com") { t.Errorf("api_host not preserved in config:\n%s", content) } - if !contains(content, "access_token: tok") { + if !strings.Contains(content, "access_token: tok") { t.Errorf("oauth tokens not written:\n%s", content) } } -func contains(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsSubstring(s, substr)) -} - -func containsSubstring(s, sub string) bool { - for i := 0; i <= len(s)-len(sub); i++ { - if s[i:i+len(sub)] == sub { - return true - } - } - return false -} func TestClearTokens(t *testing.T) { tmpDir := t.TempDir() @@ -139,7 +128,7 @@ func TestClearTokens_PreservesConfig(t *testing.T) { // API key should still be there data, _ := os.ReadFile(filepath.Join(dir, "config.yaml")) - if !contains(string(data), "api_key: my-key") { + if !strings.Contains(string(data), "api_key: my-key") { t.Errorf("api_key not preserved after clear:\n%s", string(data)) } } From 5046a2f8cf4c27d250cd12ec1156b25ea5b9ec57 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 18:11:49 -0700 Subject: [PATCH 10/16] =?UTF-8?q?=F0=9F=90=9B=20fix:=20Windows=20CI=20?= =?UTF-8?q?=E2=80=94=20set=20USERPROFILE=20alongside=20HOME=20in=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows uses USERPROFILE for os.UserHomeDir(), not HOME. Add USERPROFILE to all test setups that override HOME. Also skip file permission checks on Windows and fix double blank line. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/cmd/alerts/cmd_test.go | 12 ++++++++--- internal/cmd/auth/cmd_test.go | 2 ++ internal/cmd/incidents/cmd_test.go | 8 +++++-- internal/cmd/oncall/cmd_test.go | 12 ++++++++--- internal/cmd/pulse/cmd_test.go | 8 +++++-- internal/cmd/services/cmd_test.go | 8 +++++-- internal/cmd/teams/cmd_test.go | 8 +++++-- internal/oauth/tokens_test.go | 34 ++++++++++++++++++++---------- internal/oauth/transport_test.go | 3 +++ 9 files changed, 70 insertions(+), 25 deletions(-) diff --git a/internal/cmd/alerts/cmd_test.go b/internal/cmd/alerts/cmd_test.go index fce7f81..011f5aa 100644 --- a/internal/cmd/alerts/cmd_test.go +++ b/internal/cmd/alerts/cmd_test.go @@ -155,7 +155,9 @@ func TestRunListJSON(t *testing.T) { func TestRunListNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().Int("page", 1, "") @@ -361,7 +363,9 @@ func TestRunAckSuccess(t *testing.T) { func TestRunAckNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() @@ -466,7 +470,9 @@ func TestRunResolveWithIncidents(t *testing.T) { func TestRunResolveNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().String("message", "", "") diff --git a/internal/cmd/auth/cmd_test.go b/internal/cmd/auth/cmd_test.go index d2ea34f..3f748e3 100644 --- a/internal/cmd/auth/cmd_test.go +++ b/internal/cmd/auth/cmd_test.go @@ -12,6 +12,7 @@ import ( func TestLogoutCmd(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) // Save tokens first tokens := &oauth.TokenData{ @@ -40,6 +41,7 @@ func TestLogoutCmd(t *testing.T) { func TestLogoutCmd_NoTokens(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) var buf bytes.Buffer LogoutCmd.SetOut(&buf) diff --git a/internal/cmd/incidents/cmd_test.go b/internal/cmd/incidents/cmd_test.go index d463ceb..b1edbc7 100644 --- a/internal/cmd/incidents/cmd_test.go +++ b/internal/cmd/incidents/cmd_test.go @@ -383,7 +383,9 @@ func TestRunUpdateNoFlags(t *testing.T) { func TestRunListNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().Int("page", 1, "") @@ -485,7 +487,9 @@ func TestRunDeleteNoConfirmNonInteractive(t *testing.T) { func TestRunDeleteNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().BoolP("yes", "y", false, "") diff --git a/internal/cmd/oncall/cmd_test.go b/internal/cmd/oncall/cmd_test.go index bfed413..6a1a6fd 100644 --- a/internal/cmd/oncall/cmd_test.go +++ b/internal/cmd/oncall/cmd_test.go @@ -202,7 +202,9 @@ func TestRunListPagination(t *testing.T) { func TestRunListNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().Int("page", 1, "") @@ -565,7 +567,9 @@ func TestRunWhoWithScheduleFilter(t *testing.T) { func TestRunShiftsNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().Int("days", 7, "") @@ -588,7 +592,9 @@ func TestRunShiftsNoToken(t *testing.T) { func TestRunWhoNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().String("schedule-id", "", "") diff --git a/internal/cmd/pulse/cmd_test.go b/internal/cmd/pulse/cmd_test.go index 5b71aa0..f381b88 100644 --- a/internal/cmd/pulse/cmd_test.go +++ b/internal/cmd/pulse/cmd_test.go @@ -192,7 +192,9 @@ func TestRunCreateNoSummary(t *testing.T) { func TestRunCreateNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().StringP("labels", "l", "", "") @@ -499,7 +501,9 @@ func TestRunRunNoArgs(t *testing.T) { func TestRunRunNoAPIKey(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().String("summary", "", "") diff --git a/internal/cmd/services/cmd_test.go b/internal/cmd/services/cmd_test.go index e38a3ae..d2fe6ed 100644 --- a/internal/cmd/services/cmd_test.go +++ b/internal/cmd/services/cmd_test.go @@ -152,7 +152,9 @@ func TestRunListJSON(t *testing.T) { func TestRunListNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().Int("page", 1, "") @@ -378,7 +380,9 @@ func TestRunDeleteNoConfirmNonInteractive(t *testing.T) { func TestRunDeleteNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().BoolP("yes", "y", false, "") diff --git a/internal/cmd/teams/cmd_test.go b/internal/cmd/teams/cmd_test.go index 4b4fd88..0440a2b 100644 --- a/internal/cmd/teams/cmd_test.go +++ b/internal/cmd/teams/cmd_test.go @@ -150,7 +150,9 @@ func TestRunListJSON(t *testing.T) { func TestRunListNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().Int("page", 1, "") @@ -375,7 +377,9 @@ func TestRunDeleteNoConfirmNonInteractive(t *testing.T) { func TestRunDeleteNoToken(t *testing.T) { viper.Reset() defer viper.Reset() - t.Setenv("HOME", t.TempDir()) + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cmd := newTestCmd() cmd.Flags().BoolP("yes", "y", false, "") diff --git a/internal/oauth/tokens_test.go b/internal/oauth/tokens_test.go index 288b168..83a21fd 100644 --- a/internal/oauth/tokens_test.go +++ b/internal/oauth/tokens_test.go @@ -3,11 +3,21 @@ package oauth import ( "os" "path/filepath" + "runtime" "strings" "testing" "time" ) +// setTestHome sets HOME (and USERPROFILE on Windows) so os.UserHomeDir() returns tmpDir. +func setTestHome(t *testing.T, tmpDir string) { + t.Helper() + t.Setenv("HOME", tmpDir) + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", tmpDir) + } +} + func TestIsExpired(t *testing.T) { tests := []struct { name string @@ -31,7 +41,7 @@ func TestIsExpired(t *testing.T) { func TestSaveAndLoadTokens(t *testing.T) { tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) + setTestHome(t, tmpDir) tokens := &TokenData{ AccessToken: "test-access", @@ -44,14 +54,17 @@ func TestSaveAndLoadTokens(t *testing.T) { t.Fatalf("SaveTokens: %v", err) } - // Verify file permissions + // Verify file exists path := filepath.Join(tmpDir, ".rootly-cli", "config.yaml") info, err := os.Stat(path) if err != nil { t.Fatalf("stat config file: %v", err) } - if perm := info.Mode().Perm(); perm != 0600 { - t.Errorf("file permissions = %o, want 0600", perm) + // File permission check (skip on Windows where permissions work differently) + if runtime.GOOS != "windows" { + if perm := info.Mode().Perm(); perm != 0600 { + t.Errorf("file permissions = %o, want 0600", perm) + } } loaded, err := LoadTokens() @@ -68,7 +81,7 @@ func TestSaveAndLoadTokens(t *testing.T) { func TestSaveTokens_PreservesExistingConfig(t *testing.T) { tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) + setTestHome(t, tmpDir) // Write a config with an API key first dir := filepath.Join(tmpDir, ".rootly-cli") @@ -95,10 +108,9 @@ func TestSaveTokens_PreservesExistingConfig(t *testing.T) { } } - func TestClearTokens(t *testing.T) { tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) + setTestHome(t, tmpDir) tokens := &TokenData{AccessToken: "x", RefreshToken: "y", ExpiresAt: time.Now().Add(time.Hour)} _ = SaveTokens(tokens) @@ -115,7 +127,7 @@ func TestClearTokens(t *testing.T) { func TestClearTokens_PreservesConfig(t *testing.T) { tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) + setTestHome(t, tmpDir) // Write config with API key + tokens dir := filepath.Join(tmpDir, ".rootly-cli") @@ -135,7 +147,7 @@ func TestClearTokens_PreservesConfig(t *testing.T) { func TestClearTokens_NoFile(t *testing.T) { tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) + setTestHome(t, tmpDir) if err := ClearTokens(); err != nil { t.Fatalf("ClearTokens on missing file: %v", err) @@ -144,7 +156,7 @@ func TestClearTokens_NoFile(t *testing.T) { func TestHasTokens(t *testing.T) { tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) + setTestHome(t, tmpDir) if HasTokens() { t.Error("HasTokens should be false with no config") @@ -159,7 +171,7 @@ func TestHasTokens(t *testing.T) { func TestLoadTokens_NoOAuthSection(t *testing.T) { tmpDir := t.TempDir() - t.Setenv("HOME", tmpDir) + setTestHome(t, tmpDir) dir := filepath.Join(tmpDir, ".rootly-cli") os.MkdirAll(dir, 0700) diff --git a/internal/oauth/transport_test.go b/internal/oauth/transport_test.go index 71bd34b..022629b 100644 --- a/internal/oauth/transport_test.go +++ b/internal/oauth/transport_test.go @@ -14,6 +14,7 @@ import ( func TestNewHTTPClient_SetsAuthAndUserAgent(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) tokens := &TokenData{ AccessToken: "my-token", @@ -64,6 +65,7 @@ func TestNewHTTPClient_SetsAuthAndUserAgent(t *testing.T) { func TestNewHTTPClient_NoTokens(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) cfg := &oauth2.Config{ ClientID: "test", @@ -79,6 +81,7 @@ func TestNewHTTPClient_NoTokens(t *testing.T) { func TestNewHTTPClient_RefreshesExpiredToken(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) // Token server for refresh tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From 9bf21ce96337fff45d3293bf1f374547cabe9081 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 18:26:47 -0700 Subject: [PATCH 11/16] =?UTF-8?q?=E2=9C=A8=20feat:=20auto-append=20/api=20?= =?UTF-8?q?for=20localhost=20endpoints?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When --api-host is localhost or 127.0.0.1 without an explicit path, automatically append /api since the Rails monolith serves under /api/v1. Before: rootly incidents list --api-host=localhost:22166/api After: rootly incidents list --api-host=localhost:22166 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/api/client.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 477d978..f226221 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -801,14 +801,21 @@ func NewClient(cfg *config.Config) (*Client, error) { } // ensureScheme adds a scheme if missing, using http for localhost/127.0.0.1. -func ensureScheme(url string) string { - if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") { - return url - } - if strings.HasPrefix(url, "localhost") || strings.HasPrefix(url, "127.0.0.1") { - return "http://" + url +// For localhost without an explicit path, it also appends /api since the +// Rails monolith serves the API under /api/v1 rather than /v1. +func ensureScheme(endpoint string) string { + if strings.HasPrefix(endpoint, "http://") || strings.HasPrefix(endpoint, "https://") { + return endpoint + } + if strings.HasPrefix(endpoint, "localhost") || strings.HasPrefix(endpoint, "127.0.0.1") { + result := "http://" + endpoint + // Auto-append /api for localhost if no path is present + if !strings.Contains(endpoint, "/") { + result += "/api" + } + return result } - return "https://" + url + return "https://" + endpoint } func (c *Client) ValidateAPIKey(ctx context.Context) error { From 41380aa9ec9e140596f8eca6abcbf1f55e377bc7 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 18:55:02 -0700 Subject: [PATCH 12/16] =?UTF-8?q?=E2=9C=A8=20feat:=20suggest=20'rootly=20l?= =?UTF-8?q?ogin'=20when=20token=20refresh=20fails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the refresh token is expired or revoked, surface a user-friendly error message instead of a cryptic oauth2 error. Also update CHANGELOG with all unreleased changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- CHANGELOG.md | 14 +++++++++- internal/oauth/transport.go | 7 +++++ internal/oauth/transport_test.go | 46 ++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d2b246..f0eef71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- `rootly login` — browser-based OAuth2 authentication with PKCE (no API key needed) +- `rootly logout` — clear stored OAuth tokens +- OAuth2 auto-refresh transport using `golang.org/x/oauth2` +- Auto-append `/api` for localhost endpoints (no need to pass `--api-host=localhost:22166/api`) +- `http://` scheme auto-detection for localhost/127.0.0.1 endpoints + ### Changed +- OAuth tokens stored in `~/.rootly-cli/config.yaml` under `oauth` key (single config file) +- API client uses OAuth Bearer tokens when available, falls back to API key +- Auth-exempt commands use `Annotations["skipAuth"]` instead of hardcoded name list - Switch `oncall who` and `oncall shifts` to unified `/v1/oncalls` endpoint with richer data (escalation policy, level, user email) - Add new filter flags: `--schedule-id`, `--service-id`, `--escalation-policy-id`, `--user-id`, `--time-zone`, `--earliest` - Table output now includes Escalation Policy, Level, and Email columns @@ -15,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix `oncall schedules` 404 error (use correct `/v1/schedules` endpoint) +- Windows test compatibility (`USERPROFILE` alongside `HOME`) ### Removed - Removed legacy `/v1/shifts` endpoint usage and associated `Shift`/`ShiftsResult` types @@ -54,7 +65,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Homebrew tap distribution - GitHub Actions CI (lint, test, build) and release workflows -[Unreleased]: https://github.com/rootlyhq/rootly-cli/compare/v0.1.4...HEAD +[Unreleased]: https://github.com/rootlyhq/rootly-cli/compare/v0.1.5...HEAD +[0.1.5]: https://github.com/rootlyhq/rootly-cli/compare/v0.1.4...v0.1.5 [0.1.4]: https://github.com/rootlyhq/rootly-cli/compare/v0.1.3...v0.1.4 [0.1.3]: https://github.com/rootlyhq/rootly-cli/compare/v0.1.2...v0.1.3 [0.1.2]: https://github.com/rootlyhq/rootly-cli/releases/tag/v0.1.2 diff --git a/internal/oauth/transport.go b/internal/oauth/transport.go index a97370e..cd6af64 100644 --- a/internal/oauth/transport.go +++ b/internal/oauth/transport.go @@ -1,7 +1,9 @@ package oauth import ( + "fmt" "net/http" + "strings" "golang.org/x/oauth2" ) @@ -41,6 +43,11 @@ type persistingTokenSource struct { func (p *persistingTokenSource) Token() (*oauth2.Token, error) { tok, err := p.base.Token() if err != nil { + // Surface a user-friendly message when refresh fails + errMsg := err.Error() + if strings.Contains(errMsg, "token") || strings.Contains(errMsg, "401") || strings.Contains(errMsg, "invalid_grant") { + return nil, fmt.Errorf("session expired — run 'rootly login' to re-authenticate: %w", err) + } return nil, err } // Save on every call — oauth2.ReuseTokenSource ensures this only happens on refresh diff --git a/internal/oauth/transport_test.go b/internal/oauth/transport_test.go index 022629b..80dc063 100644 --- a/internal/oauth/transport_test.go +++ b/internal/oauth/transport_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -141,3 +142,48 @@ func TestNewHTTPClient_RefreshesExpiredToken(t *testing.T) { t.Errorf("saved AccessToken = %q", saved.AccessToken) } } + +func TestNewHTTPClient_RefreshFailsSuggestsLogin(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) + + // Token server rejects refresh with invalid_grant + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(`{"error":"invalid_grant","error_description":"refresh token revoked"}`)) + })) + defer tokenServer.Close() + + // Save expired tokens so refresh is triggered + tokens := &TokenData{ + AccessToken: "expired-token", + RefreshToken: "revoked-refresh", + ExpiresAt: time.Now().Add(-1 * time.Hour), + TokenType: "Bearer", + } + SaveTokens(tokens) + + cfg := &oauth2.Config{ + ClientID: "test", + Endpoint: oauth2.Endpoint{ + TokenURL: tokenServer.URL, + AuthStyle: oauth2.AuthStyleInParams, + }, + } + + client, err := NewHTTPClient(cfg, http.DefaultTransport, "") + if err != nil { + t.Fatalf("NewHTTPClient: %v", err) + } + + req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tokenServer.URL+"/test", http.NoBody) + _, err = client.Do(req) + if err == nil { + t.Fatal("expected error when refresh token is revoked") + } + if !strings.Contains(err.Error(), "rootly login") { + t.Errorf("error should suggest 'rootly login', got: %v", err) + } +} From 2b7a44f19650027825af580ffada199f6e387985 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Fri, 27 Mar 2026 19:02:39 -0700 Subject: [PATCH 13/16] =?UTF-8?q?=F0=9F=90=9B=20fix:=20close=20response=20?= =?UTF-8?q?body=20in=20refresh=20failure=20test=20(bodyclose=20lint)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/oauth/transport_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/oauth/transport_test.go b/internal/oauth/transport_test.go index 80dc063..b411a3c 100644 --- a/internal/oauth/transport_test.go +++ b/internal/oauth/transport_test.go @@ -179,7 +179,10 @@ func TestNewHTTPClient_RefreshFailsSuggestsLogin(t *testing.T) { } req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tokenServer.URL+"/test", http.NoBody) - _, err = client.Do(req) + resp, err := client.Do(req) + if resp != nil { + resp.Body.Close() + } if err == nil { t.Fatal("expected error when refresh token is revoked") } From 566a62cdb61e22b661b5fa0e865a8293dc687760 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Sat, 28 Mar 2026 09:45:43 -0700 Subject: [PATCH 14/16] =?UTF-8?q?=F0=9F=90=9B=20fix:=20address=20Greptile?= =?UTF-8?q?=20P1/P2=20review=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - P1: DeriveAuthBaseURL now strips scheme before applying api.→app. mapping, fixing token refresh endpoint mismatch in production - P2: Deduplicate callback port — single CallbackPort constant in oauth package, used by both oauth.go and login.go - P2: Only persist tokens on actual refresh (compare access_token), fix misleading comment about save frequency 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/cmd/auth/login.go | 3 ++- internal/oauth/oauth.go | 36 +++++++++++++++++++++++++++--------- internal/oauth/oauth_test.go | 5 ++++- internal/oauth/transport.go | 12 ++++++++---- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/internal/cmd/auth/login.go b/internal/cmd/auth/login.go index 37c8c16..70b7d79 100644 --- a/internal/cmd/auth/login.go +++ b/internal/cmd/auth/login.go @@ -17,7 +17,8 @@ import ( xoauth "github.com/rootlyhq/rootly-cli/internal/oauth" ) -const callbackPort = "19797" +// Use the canonical port from the oauth package +var callbackPort = xoauth.CallbackPort var LoginCmd = &cobra.Command{ Use: "login", diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go index 9ea4447..bee5123 100644 --- a/internal/oauth/oauth.go +++ b/internal/oauth/oauth.go @@ -10,8 +10,9 @@ import ( ) const ( - ClientID = "rootly-cli" - RedirectURI = "http://localhost:19797/callback" + ClientID = "rootly-cli" + CallbackPort = "19797" + RedirectURI = "http://localhost:" + CallbackPort + "/callback" ) // NewConfig creates an oauth2.Config for the given auth base URL. @@ -46,16 +47,33 @@ func ExchangeCode(ctx context.Context, cfg *oauth2.Config, code, codeVerifier st // For api.rootly.com it returns https://app.rootly.com. // For localhost it returns http://localhost:. func DeriveAuthBaseURL(apiHost string) string { - if strings.HasPrefix(apiHost, "http://") || strings.HasPrefix(apiHost, "https://") { - return apiHost + // Strip scheme to normalize, then re-apply appropriate scheme + scheme := "" + host := apiHost + if strings.HasPrefix(apiHost, "http://") { + scheme = "http://" + host = apiHost[7:] + } else if strings.HasPrefix(apiHost, "https://") { + scheme = "https://" + host = apiHost[8:] } - if strings.HasPrefix(apiHost, "localhost") || strings.HasPrefix(apiHost, "127.0.0.1") { - return "http://" + apiHost + + // Strip /api suffix (used for localhost API endpoints, not OAuth) + host = strings.TrimSuffix(host, "/api") + + if strings.HasPrefix(host, "localhost") || strings.HasPrefix(host, "127.0.0.1") { + if scheme == "" { + scheme = "http://" + } + return scheme + host + } + if strings.HasPrefix(host, "api.") { + return "https://app." + host[4:] } - if strings.HasPrefix(apiHost, "api.") { - return "https://app." + apiHost[4:] + if scheme == "" { + scheme = "https://" } - return "https://" + apiHost + return scheme + host } // TokenSourceFromStored creates a token source that auto-refreshes using stored tokens. diff --git a/internal/oauth/oauth_test.go b/internal/oauth/oauth_test.go index e5451fe..6b2570a 100644 --- a/internal/oauth/oauth_test.go +++ b/internal/oauth/oauth_test.go @@ -42,8 +42,11 @@ func TestDeriveAuthBaseURL(t *testing.T) { }{ {"api.rootly.com", "https://app.rootly.com"}, {"api.staging.rootly.com", "https://app.staging.rootly.com"}, + {"https://api.rootly.com", "https://app.rootly.com"}, + {"https://api.staging.rootly.com", "https://app.staging.rootly.com"}, {"localhost:22166", "http://localhost:22166"}, - {"localhost:22166/api", "http://localhost:22166/api"}, + {"localhost:22166/api", "http://localhost:22166"}, + {"http://localhost:22166/api", "http://localhost:22166"}, {"127.0.0.1:3000", "http://127.0.0.1:3000"}, {"http://localhost:22166", "http://localhost:22166"}, {"https://custom.example.com", "https://custom.example.com"}, diff --git a/internal/oauth/transport.go b/internal/oauth/transport.go index cd6af64..82a34d6 100644 --- a/internal/oauth/transport.go +++ b/internal/oauth/transport.go @@ -35,9 +35,10 @@ func NewHTTPClient(cfg *oauth2.Config, base http.RoundTripper, userAgent string) }, nil } -// persistingTokenSource wraps a TokenSource and saves new tokens to disk. +// persistingTokenSource wraps a TokenSource and saves refreshed tokens to disk. type persistingTokenSource struct { - base oauth2.TokenSource + base oauth2.TokenSource + lastAccessToken string } func (p *persistingTokenSource) Token() (*oauth2.Token, error) { @@ -50,8 +51,11 @@ func (p *persistingTokenSource) Token() (*oauth2.Token, error) { } return nil, err } - // Save on every call — oauth2.ReuseTokenSource ensures this only happens on refresh - _ = SaveOAuth2Token(tok) + // Only save when the token was actually refreshed (new access token) + if tok.AccessToken != p.lastAccessToken { + p.lastAccessToken = tok.AccessToken + _ = SaveOAuth2Token(tok) + } return tok, nil } From 62a84ee532037633c7416f1a0a3b665b3b1889a8 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Sat, 28 Mar 2026 10:41:49 -0700 Subject: [PATCH 15/16] =?UTF-8?q?=E2=9C=A8=20feat:=20replace=20hardcoded?= =?UTF-8?q?=20client=5Fid=20with=20dynamic=20OAuth=20registration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Rootly monolith no longer supports magic client IDs. On first login, POST /oauth/register to obtain a client_id dynamically, cache it in ~/.rootly-cli/config.yaml, and re-register automatically if the authorize endpoint returns 404 (deleted app). --- internal/api/client.go | 3 +- internal/cmd/auth/login.go | 120 ++++++++++++++++++++++++++++------- internal/config/config.go | 3 +- internal/oauth/oauth.go | 99 +++++++++++++++++++++++++++-- internal/oauth/oauth_test.go | 97 ++++++++++++++++++++++++++-- 5 files changed, 289 insertions(+), 33 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index f226221..1d6d838 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -757,7 +757,8 @@ func NewClient(cfg *config.Config) (*Client, error) { var httpClient *http.Client if useOAuth { authBaseURL := oauth.DeriveAuthBaseURL(cfg.Endpoint) - oauthCfg := oauth.NewConfig(authBaseURL) + clientID := oauth.LoadCachedClientID() + oauthCfg := oauth.NewConfig(authBaseURL, clientID) var err error httpClient, err = oauth.NewHTTPClient(oauthCfg, transport, "rootly-cli/"+Version) if err != nil { diff --git a/internal/cmd/auth/login.go b/internal/cmd/auth/login.go index 70b7d79..1e61000 100644 --- a/internal/cmd/auth/login.go +++ b/internal/cmd/auth/login.go @@ -2,6 +2,7 @@ package auth import ( "context" + "errors" "fmt" "html" "net" @@ -14,12 +15,10 @@ import ( "github.com/spf13/viper" "golang.org/x/oauth2" + "github.com/rootlyhq/rootly-cli/internal/config" xoauth "github.com/rootlyhq/rootly-cli/internal/oauth" ) -// Use the canonical port from the oauth package -var callbackPort = xoauth.CallbackPort - var LoginCmd = &cobra.Command{ Use: "login", Short: "Authenticate with Rootly via browser-based OAuth2", @@ -41,27 +40,110 @@ func init() { _ = LoginCmd.Flags().MarkHidden("client-id") } +// resolveClientID returns a client ID, registering dynamically if needed. +func resolveClientID(ctx context.Context, authBaseURL string, cmd *cobra.Command) (string, error) { + // Allow explicit override for debugging + if clientID, _ := cmd.Flags().GetString("client-id"); clientID != "" { + return clientID, nil + } + + // Check cached client_id + if clientID := xoauth.LoadCachedClientID(); clientID != "" { + return clientID, nil + } + + // Register dynamically + return registerAndCache(ctx, authBaseURL, cmd) +} + +// registerAndCache calls POST /oauth/register and saves the client_id. +func registerAndCache(ctx context.Context, authBaseURL string, cmd *cobra.Command) (string, error) { + _, _ = fmt.Fprintf(cmd.OutOrStderr(), "Registering OAuth client...\n") + clientID, err := xoauth.RegisterClient(ctx, authBaseURL) + if err != nil { + return "", err + } + if err := xoauth.SaveClientID(clientID); err != nil { + return "", fmt.Errorf("failed to cache client ID: %w", err) + } + return clientID, nil +} + +// httpClientWithTimeout is used for pre-flight checks (HEAD) and registration. +var httpClientWithTimeout = &http.Client{Timeout: 10 * time.Second} + func runLogin(cmd *cobra.Command, args []string) error { ctx := cmd.Context() apiHost := viper.GetString("api_host") if apiHost == "" { - apiHost = "api.rootly.com" + apiHost = config.DefaultEndpoint } authBaseURL := xoauth.DeriveAuthBaseURL(apiHost) - cfg := xoauth.NewConfig(authBaseURL) - // Allow client-id override for debugging - if clientID, _ := cmd.Flags().GetString("client-id"); clientID != "" { - cfg.ClientID = clientID + clientID, err := resolveClientID(ctx, authBaseURL, cmd) + if err != nil { + return err + } + + tok, err := doOAuthFlow(ctx, cmd, authBaseURL, clientID) + if err != nil { + // If authorize returned 404, the cached client_id may be stale — re-register once + if isAuthorize404(err) { + _, _ = fmt.Fprintf(cmd.OutOrStderr(), "OAuth client not found, re-registering...\n") + _ = xoauth.ClearClientID() + clientID, regErr := registerAndCache(ctx, authBaseURL, cmd) + if regErr != nil { + return regErr + } + tok, err = doOAuthFlow(ctx, cmd, authBaseURL, clientID) + if err != nil { + return err + } + } else { + return err + } + } + + if err := xoauth.SaveOAuth2Token(tok); err != nil { + return fmt.Errorf("failed to save tokens: %w", err) } + _, _ = fmt.Fprintf(cmd.OutOrStderr(), "Login successful! Tokens saved.\n") + return nil +} + +// errAuthorize404 is returned when the authorize endpoint returns 404. +var errAuthorize404 = errors.New("authorization endpoint returned 404") + +func isAuthorize404(err error) bool { + return errors.Is(err, errAuthorize404) +} + +func doOAuthFlow(ctx context.Context, cmd *cobra.Command, authBaseURL, clientID string) (*oauth2.Token, error) { + cfg := xoauth.NewConfig(authBaseURL, clientID) + verifier := oauth2.GenerateVerifier() state, err := xoauth.GenerateState() if err != nil { - return fmt.Errorf("failed to generate state: %w", err) + return nil, fmt.Errorf("failed to generate state: %w", err) + } + + // Build authorization URL with PKCE (S256 challenge derived from verifier) + authURL := cfg.AuthCodeURL(state, oauth2.S256ChallengeOption(verifier)) + + // Pre-check: HEAD the authorize URL to detect 404 (stale client_id) + // Done before starting the callback server to avoid binding a port unnecessarily. + headReq, headReqErr := http.NewRequestWithContext(ctx, http.MethodHead, authURL, http.NoBody) + if headReqErr == nil { + if headResp, headErr := httpClientWithTimeout.Do(headReq); headErr == nil { + _ = headResp.Body.Close() + if headResp.StatusCode == http.StatusNotFound { + return nil, errAuthorize404 + } + } } // Channel to receive the authorization code @@ -93,9 +175,9 @@ func runLogin(cmd *cobra.Command, args []string) error { }) lc := net.ListenConfig{} - listener, err := lc.Listen(ctx, "tcp", "localhost:"+callbackPort) + listener, err := lc.Listen(ctx, "tcp", "localhost:"+xoauth.CallbackPort) if err != nil { - return fmt.Errorf("failed to start callback server on port %s: %w", callbackPort, err) + return nil, fmt.Errorf("failed to start callback server on port %s: %w", xoauth.CallbackPort, err) } server := &http.Server{Handler: mux} @@ -106,9 +188,6 @@ func runLogin(cmd *cobra.Command, args []string) error { _ = server.Shutdown(shutdownCtx) }() - // Build authorization URL with PKCE (S256 challenge derived from verifier) - authURL := cfg.AuthCodeURL(state, oauth2.S256ChallengeOption(verifier)) - _, _ = fmt.Fprintf(cmd.OutOrStderr(), "Opening browser for authentication...\n") _, _ = fmt.Fprintf(cmd.OutOrStderr(), "If the browser doesn't open, visit:\n%s\n\n", authURL) @@ -123,23 +202,18 @@ func runLogin(cmd *cobra.Command, args []string) error { select { case code = <-codeCh: case err := <-errCh: - return err + return nil, err case <-time.After(5 * time.Minute): - return fmt.Errorf("login timed out after 5 minutes") + return nil, fmt.Errorf("login timed out after 5 minutes") } // Exchange code for tokens tok, err := xoauth.ExchangeCode(ctx, cfg, code, verifier) if err != nil { - return err + return nil, err } - if err := xoauth.SaveOAuth2Token(tok); err != nil { - return fmt.Errorf("failed to save tokens: %w", err) - } - - _, _ = fmt.Fprintf(cmd.OutOrStderr(), "Login successful! Tokens saved.\n") - return nil + return tok, nil } func openBrowser(ctx context.Context, url string) error { diff --git a/internal/config/config.go b/internal/config/config.go index 3f82b6d..a8bddac 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -24,7 +24,8 @@ type Config struct { Language string `yaml:"language"` // TUI-specific Layout string `yaml:"layout"` // TUI-specific OAuth *OAuthData `yaml:"oauth,omitempty"` - Debug bool `yaml:"-"` // Runtime only, not persisted + ClientID string `yaml:"client_id,omitempty"` // Dynamically registered OAuth client ID + Debug bool `yaml:"-"` // Runtime only, not persisted } // OAuthData holds OAuth2 token data within the config file. diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go index bee5123..ebebc8c 100644 --- a/internal/oauth/oauth.go +++ b/internal/oauth/oauth.go @@ -1,24 +1,29 @@ package oauth import ( + "bytes" "context" "crypto/rand" "encoding/base64" + "encoding/json" + "fmt" + "net/http" "strings" "golang.org/x/oauth2" + + "github.com/rootlyhq/rootly-cli/internal/config" ) const ( - ClientID = "rootly-cli" CallbackPort = "19797" RedirectURI = "http://localhost:" + CallbackPort + "/callback" ) -// NewConfig creates an oauth2.Config for the given auth base URL. -func NewConfig(authBaseURL string) *oauth2.Config { +// NewConfig creates an oauth2.Config for the given auth base URL and client ID. +func NewConfig(authBaseURL, clientID string) *oauth2.Config { return &oauth2.Config{ - ClientID: ClientID, + ClientID: clientID, RedirectURL: RedirectURI, Scopes: []string{"openid", "profile", "email", "all"}, Endpoint: oauth2.Endpoint{ @@ -29,6 +34,92 @@ func NewConfig(authBaseURL string) *oauth2.Config { } } +// registrationRequest is the payload for POST /oauth/register. +type registrationRequest struct { + ClientName string `json:"client_name"` + RedirectURIs []string `json:"redirect_uris"` + TokenEndpointAuthMethod string `json:"token_endpoint_auth_method"` + GrantTypes []string `json:"grant_types"` + ResponseTypes []string `json:"response_types"` +} + +// registrationResponse is the response from POST /oauth/register. +type registrationResponse struct { + ClientID string `json:"client_id"` +} + +// RegisterClient dynamically registers an OAuth client and returns the client_id. +func RegisterClient(ctx context.Context, authBaseURL string) (string, error) { + reqBody := registrationRequest{ + ClientName: "Rootly CLI", + RedirectURIs: []string{RedirectURI}, + TokenEndpointAuthMethod: "none", + GrantTypes: []string{"authorization_code"}, + ResponseTypes: []string{"code"}, + } + + body, err := json.Marshal(reqBody) + if err != nil { + return "", fmt.Errorf("failed to marshal registration request: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, authBaseURL+"/oauth/register", bytes.NewReader(body)) + if err != nil { + return "", fmt.Errorf("failed to create registration request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to register OAuth client: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + return "", fmt.Errorf("could not register OAuth client (status %d)", resp.StatusCode) + } + + var regResp registrationResponse + if err := json.NewDecoder(resp.Body).Decode(®Resp); err != nil { + return "", fmt.Errorf("failed to parse registration response: %w", err) + } + + if regResp.ClientID == "" { + return "", fmt.Errorf("registration response missing client_id") + } + + return regResp.ClientID, nil +} + +// LoadCachedClientID reads the cached OAuth client_id from config. +func LoadCachedClientID() string { + cfg, err := config.Load() + if err != nil { + return "" + } + return cfg.ClientID +} + +// SaveClientID persists the OAuth client_id to config, preserving other fields. +func SaveClientID(clientID string) error { + cfg, err := config.Load() + if err != nil { + cfg = &config.Config{} + } + cfg.ClientID = clientID + return config.Save(cfg) +} + +// ClearClientID removes the cached client_id from config. +func ClearClientID() error { + cfg, err := config.Load() + if err != nil { + return nil // No config file means nothing to clear + } + cfg.ClientID = "" + return config.Save(cfg) +} + // GenerateState creates a cryptographically random state parameter. func GenerateState() (string, error) { b := make([]byte, 32) diff --git a/internal/oauth/oauth_test.go b/internal/oauth/oauth_test.go index 6b2570a..6645295 100644 --- a/internal/oauth/oauth_test.go +++ b/internal/oauth/oauth_test.go @@ -1,14 +1,19 @@ package oauth import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" "testing" ) func TestNewConfig(t *testing.T) { - cfg := NewConfig("https://app.rootly.com") + cfg := NewConfig("https://app.rootly.com", "test-client-id") - if cfg.ClientID != "rootly-cli" { - t.Errorf("ClientID = %q", cfg.ClientID) + if cfg.ClientID != "test-client-id" { + t.Errorf("ClientID = %q, want %q", cfg.ClientID, "test-client-id") } if cfg.RedirectURL != "http://localhost:19797/callback" { t.Errorf("RedirectURL = %q", cfg.RedirectURL) @@ -25,7 +30,7 @@ func TestNewConfig(t *testing.T) { } func TestNewConfig_Localhost(t *testing.T) { - cfg := NewConfig("http://localhost:22166") + cfg := NewConfig("http://localhost:22166", "my-client") if cfg.Endpoint.AuthURL != "http://localhost:22166/oauth/authorize" { t.Errorf("AuthURL = %q", cfg.Endpoint.AuthURL) @@ -61,3 +66,87 @@ func TestDeriveAuthBaseURL(t *testing.T) { }) } } + +func TestRegisterClient(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/oauth/register" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Method != http.MethodPost { + t.Errorf("unexpected method: %s", r.Method) + } + if r.Header.Get("Content-Type") != "application/json" { + t.Errorf("unexpected content-type: %s", r.Header.Get("Content-Type")) + } + + var req registrationRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + t.Fatalf("failed to decode request: %v", err) + } + if req.ClientName != "Rootly CLI" { + t.Errorf("ClientName = %q", req.ClientName) + } + + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(registrationResponse{ClientID: "dynamic-id-123"}) + })) + defer srv.Close() + + clientID, err := RegisterClient(context.Background(), srv.URL) + if err != nil { + t.Fatalf("RegisterClient() error: %v", err) + } + if clientID != "dynamic-id-123" { + t.Errorf("clientID = %q, want %q", clientID, "dynamic-id-123") + } +} + +func TestRegisterClient_NonCreated(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + _, err := RegisterClient(context.Background(), srv.URL) + if err == nil { + t.Fatal("expected error for non-201 status") + } +} + +func TestLoadSaveClearClientID(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("USERPROFILE", tmpDir) + + // Initially empty + if id := LoadCachedClientID(); id != "" { + t.Errorf("expected empty, got %q", id) + } + + // Save + if err := SaveClientID("cached-id"); err != nil { + t.Fatalf("SaveClientID: %v", err) + } + if id := LoadCachedClientID(); id != "cached-id" { + t.Errorf("got %q, want %q", id, "cached-id") + } + + // Clear + if err := ClearClientID(); err != nil { + t.Fatalf("ClearClientID: %v", err) + } + + // Verify cleared — load raw config to check + data, err := os.ReadFile(configPathForTest(tmpDir)) + if err != nil { + // File may not exist after clear, that's ok + return + } + if id := LoadCachedClientID(); id != "" { + t.Errorf("expected empty after clear, got %q (raw: %s)", id, string(data)) + } +} + +func configPathForTest(home string) string { + return home + "/.rootly-cli/config.yaml" +} From 904156320aff5064d046ef641de98efdc0414d0e Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Sat, 28 Mar 2026 21:46:58 -0700 Subject: [PATCH 16/16] =?UTF-8?q?=F0=9F=94=A7=20fix:=20derive=20auth=20bas?= =?UTF-8?q?e=20URL=20as=20rootly.com=20instead=20of=20app.rootly.com?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The monolith no longer uses app.rootly.com — strip the "api." prefix without prepending "app." for production and staging hosts. --- internal/oauth/oauth.go | 4 ++-- internal/oauth/oauth_test.go | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go index ebebc8c..d4a89a3 100644 --- a/internal/oauth/oauth.go +++ b/internal/oauth/oauth.go @@ -135,7 +135,7 @@ func ExchangeCode(ctx context.Context, cfg *oauth2.Config, code, codeVerifier st } // DeriveAuthBaseURL builds the OAuth base URL from the API host. -// For api.rootly.com it returns https://app.rootly.com. +// For api.rootly.com it returns https://rootly.com. // For localhost it returns http://localhost:. func DeriveAuthBaseURL(apiHost string) string { // Strip scheme to normalize, then re-apply appropriate scheme @@ -159,7 +159,7 @@ func DeriveAuthBaseURL(apiHost string) string { return scheme + host } if strings.HasPrefix(host, "api.") { - return "https://app." + host[4:] + return "https://" + host[4:] } if scheme == "" { scheme = "https://" diff --git a/internal/oauth/oauth_test.go b/internal/oauth/oauth_test.go index 6645295..f52dd86 100644 --- a/internal/oauth/oauth_test.go +++ b/internal/oauth/oauth_test.go @@ -10,7 +10,7 @@ import ( ) func TestNewConfig(t *testing.T) { - cfg := NewConfig("https://app.rootly.com", "test-client-id") + cfg := NewConfig("https://rootly.com", "test-client-id") if cfg.ClientID != "test-client-id" { t.Errorf("ClientID = %q, want %q", cfg.ClientID, "test-client-id") @@ -18,10 +18,10 @@ func TestNewConfig(t *testing.T) { if cfg.RedirectURL != "http://localhost:19797/callback" { t.Errorf("RedirectURL = %q", cfg.RedirectURL) } - if cfg.Endpoint.AuthURL != "https://app.rootly.com/oauth/authorize" { + if cfg.Endpoint.AuthURL != "https://rootly.com/oauth/authorize" { t.Errorf("AuthURL = %q", cfg.Endpoint.AuthURL) } - if cfg.Endpoint.TokenURL != "https://app.rootly.com/oauth/token" { + if cfg.Endpoint.TokenURL != "https://rootly.com/oauth/token" { t.Errorf("TokenURL = %q", cfg.Endpoint.TokenURL) } if len(cfg.Scopes) != 4 { @@ -45,10 +45,10 @@ func TestDeriveAuthBaseURL(t *testing.T) { input string want string }{ - {"api.rootly.com", "https://app.rootly.com"}, - {"api.staging.rootly.com", "https://app.staging.rootly.com"}, - {"https://api.rootly.com", "https://app.rootly.com"}, - {"https://api.staging.rootly.com", "https://app.staging.rootly.com"}, + {"api.rootly.com", "https://rootly.com"}, + {"api.staging.rootly.com", "https://staging.rootly.com"}, + {"https://api.rootly.com", "https://rootly.com"}, + {"https://api.staging.rootly.com", "https://staging.rootly.com"}, {"localhost:22166", "http://localhost:22166"}, {"localhost:22166/api", "http://localhost:22166"}, {"http://localhost:22166/api", "http://localhost:22166"},