Skip to content

Commit a6601d6

Browse files
committed
fix(stats): honor custom domains and stop misleading fallbacks
Review follow-ups: stats and export gain --domain and thread it through ResolveAlias; a resolve 404 with --domain errors instead of silently rendering a stranger's default-domain stats, and the convenience fallback without --domain is announced on stderr. Password-protected 401s no longer trigger a pointless token refresh or a fake session expiry, and the public-view notices now speak to ownership.
1 parent 793a4b3 commit a6601d6

11 files changed

Lines changed: 234 additions & 32 deletions

File tree

internal/api/client.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,24 @@ func (c *Client) request(ctx context.Context, method, path string, query url.Val
9999
if err != nil {
100100
return nil, err
101101
}
102-
if resp.StatusCode == http.StatusUnauthorized && creds != nil &&
103-
creds.Mode == auth.ModeDevice && creds.RefreshToken != "" {
104-
resp.Body.Close()
105-
if creds, err = c.refreshTokens(ctx, creds); err != nil {
106-
return nil, err
102+
if resp.StatusCode == http.StatusUnauthorized {
103+
// The public stats endpoint answers 401 for password-protected
104+
// links. That is a property of the link, not of the session, so
105+
// refreshing tokens can't help — and the CLI doesn't supply link
106+
// passwords, so say so instead of blaming the login.
107+
switch resp.Header.Get("X-Error-Code") {
108+
case "password_required", "invalid_password":
109+
resp.Body.Close()
110+
return nil, errors.New("this link's stats are password protected")
107111
}
108-
if resp, err = c.send(ctx, method, path, query, body, creds); err != nil {
109-
return nil, err
112+
if creds != nil && creds.Mode == auth.ModeDevice && creds.RefreshToken != "" {
113+
resp.Body.Close()
114+
if creds, err = c.refreshTokens(ctx, creds); err != nil {
115+
return nil, err
116+
}
117+
if resp, err = c.send(ctx, method, path, query, body, creds); err != nil {
118+
return nil, err
119+
}
110120
}
111121
}
112122
return resp, nil

internal/api/client_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"net/http"
77
"net/http/httptest"
8+
"strings"
89
"sync/atomic"
910
"testing"
1011

@@ -173,3 +174,39 @@ func TestClientHeaderKeptOnSameHostRedirect(t *testing.T) {
173174
t.Fatalf("X-Spoo-Client after same-host redirect = %q, want cli/dev", gotClient)
174175
}
175176
}
177+
178+
// a password-protected link answers 401 too, but that's about the link,
179+
// not the session — no token refresh, and an honest error instead of
180+
// "session expired".
181+
func TestDo401PasswordRequiredSkipsRefresh(t *testing.T) {
182+
var refreshCalls atomic.Int32
183+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
184+
if r.URL.Path == "/auth/device/refresh" {
185+
refreshCalls.Add(1)
186+
w.Write([]byte(`{"access_token":"newAT","refresh_token":"newRT"}`))
187+
return
188+
}
189+
w.Header().Set("X-Error-Code", "password_required")
190+
w.WriteHeader(http.StatusUnauthorized)
191+
w.Write([]byte(`{"error":"Password required","code":"password_required"}`))
192+
}))
193+
defer srv.Close()
194+
195+
store := newTestStore(t, &auth.Credentials{Mode: auth.ModeDevice, AccessToken: "goodAT", RefreshToken: "goodRT"})
196+
c := New(srv.URL, store)
197+
err := c.do(context.Background(), http.MethodGet, "/api/v1/public/stats/secret", nil, nil, nil)
198+
if err == nil || !strings.Contains(err.Error(), "password protected") {
199+
t.Fatalf("err = %v, want a password-protected explanation", err)
200+
}
201+
if refreshCalls.Load() != 0 {
202+
t.Fatalf("refresh called %d times, want 0", refreshCalls.Load())
203+
}
204+
// the healthy session must survive untouched
205+
got, loadErr := store.Load()
206+
if loadErr != nil {
207+
t.Fatal(loadErr)
208+
}
209+
if got.AccessToken != "goodAT" || got.RefreshToken != "goodRT" {
210+
t.Fatalf("tokens rotated pointlessly: %+v", got)
211+
}
212+
}

internal/api/urls.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,19 @@ func (c *Client) ListURLs(ctx context.Context, opts ListURLsOptions) (*URLPage,
8585

8686
// ResolveAlias looks up an owned link by alias via GET
8787
// /api/v1/urls/{domain}/{alias}, mainly to obtain its url id for the
88-
// per-link stats and export endpoints. The domain is the API base
89-
// URL's hostname, which covers spoo.me links. Unknown and foreign
90-
// aliases both answer 404 (no ownership oracle).
91-
func (c *Client) ResolveAlias(ctx context.Context, alias string) (*URLItem, error) {
92-
base, err := url.Parse(c.base)
93-
if err != nil {
94-
return nil, err
88+
// per-link stats and export endpoints. An empty domain defaults to the
89+
// API base URL's hostname, which covers spoo.me links; pass one of the
90+
// user's custom domains to resolve links living there. Unknown and
91+
// foreign aliases both answer 404 (no ownership oracle).
92+
func (c *Client) ResolveAlias(ctx context.Context, alias, domain string) (*URLItem, error) {
93+
if domain == "" {
94+
base, err := url.Parse(c.base)
95+
if err != nil {
96+
return nil, err
97+
}
98+
domain = base.Hostname()
9599
}
96-
path := "/api/v1/urls/" + url.PathEscape(base.Hostname()) + "/" + url.PathEscape(alias)
100+
path := "/api/v1/urls/" + url.PathEscape(domain) + "/" + url.PathEscape(alias)
97101
var out URLItem
98102
if err := c.do(ctx, http.MethodGet, path, nil, nil, &out); err != nil {
99103
return nil, err

internal/api/urls_test.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func TestResolveAliasUsesAPIHostAndEscapes(t *testing.T) {
4646
defer srv.Close()
4747

4848
c := New(srv.URL, newTestStore(t, nil))
49-
u, err := c.ResolveAlias(context.Background(), "🚀")
49+
u, err := c.ResolveAlias(context.Background(), "🚀", "")
5050
if err != nil {
5151
t.Fatal(err)
5252
}
@@ -68,12 +68,31 @@ func TestResolveAliasNotFound(t *testing.T) {
6868
defer srv.Close()
6969

7070
c := New(srv.URL, newTestStore(t, nil))
71-
_, err := c.ResolveAlias(context.Background(), "nope")
71+
_, err := c.ResolveAlias(context.Background(), "nope", "")
7272
if !IsNotFound(err) {
7373
t.Fatalf("err = %v, want IsNotFound", err)
7474
}
7575
}
7676

77+
// a custom domain replaces the API host in the resolve path, so links
78+
// on the user's own domains resolve to their real url ids.
79+
func TestResolveAliasCustomDomain(t *testing.T) {
80+
var gotPath string
81+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
82+
gotPath = r.URL.EscapedPath()
83+
w.Write([]byte(`{"id":"65f0abc123","alias":"promo","long_url":"https://x.com","status":"ACTIVE"}`))
84+
}))
85+
defer srv.Close()
86+
87+
c := New(srv.URL, newTestStore(t, nil))
88+
if _, err := c.ResolveAlias(context.Background(), "promo", "links.example.com"); err != nil {
89+
t.Fatal(err)
90+
}
91+
if gotPath != "/api/v1/urls/links.example.com/promo" {
92+
t.Fatalf("path = %q, want the custom domain in the path", gotPath)
93+
}
94+
}
95+
7796
func TestUpdateURLSendsPatch(t *testing.T) {
7897
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
7998
if r.Method != http.MethodPatch || r.URL.Path != "/api/v1/urls/abc123" {

internal/cmd/export.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
)
1414

1515
func newExportCmd() *cobra.Command {
16-
var format, output, from, to string
16+
var format, output, from, to, domain string
1717
cmd := &cobra.Command{
1818
Use: "export [short-code]",
1919
Short: "Export click analytics to a file",
@@ -46,9 +46,13 @@ workbook with one sheet per dimension.`,
4646
var name string
4747
var data []byte
4848
if len(args) == 1 {
49-
u, err := d.client.ResolveAlias(cmd.Context(), args[0])
49+
u, err := d.client.ResolveAlias(cmd.Context(), args[0], domain)
5050
if api.IsNotFound(err) {
51-
return fmt.Errorf("%s is not one of your links — export covers only links you own", args[0])
51+
where := args[0]
52+
if domain != "" {
53+
where += " on " + domain
54+
}
55+
return fmt.Errorf("%s is not one of your links — export covers only links you own", where)
5256
}
5357
if err != nil {
5458
return err
@@ -79,6 +83,8 @@ workbook with one sheet per dimension.`,
7983
cmd.Flags().StringVarP(&output, "output", "o", "", "output file (default: server-suggested name; - for stdout)")
8084
cmd.Flags().StringVar(&from, "from", "", "start date (ISO 8601)")
8185
cmd.Flags().StringVar(&to, "to", "", "end date (ISO 8601)")
86+
cmd.Flags().StringVar(&domain, "domain", "", "the link is on one of your custom domains")
8287
fixed(cmd, "format", "json", "csv", "xlsx", "xml")
88+
flagComp(cmd, "domain", completeDomain)
8389
return cmd
8490
}

internal/cmd/export_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,31 @@ func TestExportForeignCodeErrors(t *testing.T) {
9494
t.Fatalf("err = %v, want an ownership explanation", err)
9595
}
9696
}
97+
98+
// --domain must reach the resolve path, replacing the API host.
99+
func TestExportDomainFlagResolvesOnThatDomain(t *testing.T) {
100+
var paths []string
101+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
102+
paths = append(paths, r.URL.Path)
103+
if strings.HasPrefix(r.URL.Path, "/api/v1/urls/") {
104+
w.Write([]byte(`{"id":"65f0abc123","alias":"promo","long_url":"https://x.com","status":"ACTIVE"}`))
105+
return
106+
}
107+
w.Write([]byte(`{"export":"ok"}`))
108+
}))
109+
defer srv.Close()
110+
pointDepsAtLoggedIn(t, srv.URL)
111+
112+
root := NewRootCmd()
113+
var out bytes.Buffer
114+
root.SetOut(&out)
115+
root.SetErr(&out)
116+
root.SetArgs([]string{"export", "promo", "--domain", "links.example.com", "-o", "-"})
117+
if err := root.Execute(); err != nil {
118+
t.Fatal(err)
119+
}
120+
want := []string{"/api/v1/urls/links.example.com/promo", "/api/v1/export/links/65f0abc123"}
121+
if len(paths) != 2 || paths[0] != want[0] || paths[1] != want[1] {
122+
t.Fatalf("paths = %v, want %v", paths, want)
123+
}
124+
}

internal/cmd/helpers.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,23 @@ package cmd
22

33
import (
44
"io"
5+
"net/url"
56
"strings"
67
"time"
78

89
"github.com/charmbracelet/colorprofile"
910
"github.com/spf13/cobra"
1011
)
1112

13+
// apiHost is the hostname of the configured API base — the system
14+
// default domain that short links live on unless --domain says otherwise.
15+
func apiHost(base string) string {
16+
if u, err := url.Parse(base); err == nil && u.Hostname() != "" {
17+
return u.Hostname()
18+
}
19+
return base
20+
}
21+
1222
// timeNow is a seam for tests that need deterministic expiry math.
1323
var timeNow = time.Now
1424

internal/cmd/stats.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"errors"
77
"fmt"
8+
"io"
89
"time"
910

1011
tea "charm.land/bubbletea/v2"
@@ -16,16 +17,24 @@ import (
1617
)
1718

1819
// resolveTarget maps a short code and login state onto a stats surface.
19-
// Logged in with a code, the alias resolves to an owned link's url id;
20-
// a 404 means the link isn't yours (or doesn't exist), so it falls back
21-
// to the public endpoint, which answers for anyone's public link.
22-
func resolveTarget(ctx context.Context, client *api.Client, code string, loggedIn bool) (stats.Target, error) {
20+
// Logged in with a code, the alias resolves to an owned link's url id on
21+
// the given domain (empty means the system default). On a 404 with
22+
// --domain set there is nowhere to fall back to — the public endpoint
23+
// serves only default-domain links — so it errors instead of silently
24+
// showing a different link's stats. Without --domain the code may still
25+
// be someone else's public default-domain link, so it falls back to the
26+
// public endpoint, announcing the switch on errOut.
27+
func resolveTarget(ctx context.Context, client *api.Client, code, domain, defaultDomain string, loggedIn bool, errOut io.Writer) (stats.Target, error) {
2328
switch {
2429
case code == "":
2530
return stats.Target{Kind: stats.KindAccount}, nil
2631
case loggedIn:
27-
u, err := client.ResolveAlias(ctx, code)
32+
u, err := client.ResolveAlias(ctx, code, domain)
2833
if api.IsNotFound(err) {
34+
if domain != "" {
35+
return stats.Target{}, fmt.Errorf("%s on %s is not one of your links — public stats cover only %s links", code, domain, defaultDomain)
36+
}
37+
fmt.Fprintf(errOut, "note: %s isn't one of your links — showing public stats for %s/%s\n", code, defaultDomain, code)
2938
return stats.Target{Kind: stats.KindPublicLink, Alias: code}, nil
3039
}
3140
if err != nil {
@@ -38,7 +47,7 @@ func resolveTarget(ctx context.Context, client *api.Client, code string, loggedI
3847
}
3948

4049
func newStatsCmd() *cobra.Command {
41-
var from, to, tz string
50+
var from, to, tz, domain string
4251
var plain bool
4352
cmd := &cobra.Command{
4453
Use: "stats [short-code]",
@@ -75,7 +84,11 @@ With a short code, shows that link — public stats work without login.`,
7584
if !loggedIn && code == "" {
7685
return fmt.Errorf("not logged in — pass a short code for public stats, or run `spoo auth login`")
7786
}
78-
target, err := resolveTarget(cmd.Context(), d.client, code, loggedIn)
87+
defaultDomain := apiHost(d.cfg.APIBase)
88+
if domain != "" && !loggedIn {
89+
return fmt.Errorf("--domain requires login — public stats cover only %s links", defaultDomain)
90+
}
91+
target, err := resolveTarget(cmd.Context(), d.client, code, domain, defaultDomain, loggedIn, cmd.ErrOrStderr())
7992
if err != nil {
8093
return err
8194
}
@@ -132,5 +145,7 @@ With a short code, shows that link — public stats work without login.`,
132145
cmd.Flags().StringVar(&to, "to", "", "end date, ISO 8601 (static report; default: now)")
133146
cmd.Flags().StringVar(&tz, "tz", "", "IANA timezone for time buckets (default UTC)")
134147
cmd.Flags().BoolVar(&plain, "plain", false, "print the static report instead of the dashboard")
148+
cmd.Flags().StringVar(&domain, "domain", "", "the link is on one of your custom domains")
149+
flagComp(cmd, "domain", completeDomain)
135150
return cmd
136151
}

0 commit comments

Comments
 (0)