From 98a2cfea590fee9b9a8f78fd9ce8a19aa769509c Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Fri, 8 May 2026 20:44:04 -0500 Subject: [PATCH] improve UI around credential helper authorization. have the hold requestCrawl on restart. Update comments that relay_endpoints must suport listreposbycollection --- cmd/credential-helper/cmd_update.go | 11 +- cmd/credential-helper/device_auth.go | 8 +- cmd/credential-helper/http.go | 75 +++++++ cmd/credential-helper/http_test.go | 70 ++++++ config-appview.example.yaml | 6 +- config-hold.example.yaml | 6 +- deploy/upcloud/configs/hold.yaml.tmpl | 4 +- pkg/appview/config.go | 4 +- pkg/appview/handlers/device.go | 272 +++++++++++------------ pkg/appview/handlers/settings.go | 7 +- pkg/appview/middleware/registry.go | 9 +- pkg/appview/server.go | 23 +- pkg/appview/src/css/main.css | 91 ++++++++ pkg/appview/storage/crew.go | 61 +++--- pkg/appview/storage/crew_test.go | 32 +-- pkg/appview/templates/pages/device.html | 273 ++++++++++++++++++++++++ pkg/hold/config.go | 13 +- pkg/hold/pds/scan_broadcaster.go | 87 ++++++-- pkg/hold/server.go | 62 +++++- 19 files changed, 865 insertions(+), 249 deletions(-) create mode 100644 cmd/credential-helper/http.go create mode 100644 cmd/credential-helper/http_test.go create mode 100644 pkg/appview/templates/pages/device.html diff --git a/cmd/credential-helper/cmd_update.go b/cmd/credential-helper/cmd_update.go index 34572bf..075625b 100644 --- a/cmd/credential-helper/cmd_update.go +++ b/cmd/credential-helper/cmd_update.go @@ -62,12 +62,9 @@ func runUpdate(cmd *cobra.Command, args []string) error { // fetchLatestVersion resolves the latest released version by reading the // redirect Location header of {tangledReleasesBase}/tags/latest. func fetchLatestVersion() (string, error) { - client := &http.Client{ - Timeout: 10 * time.Second, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - return http.ErrUseLastResponse - }, - } + client := httpClientWithTimeout(10*time.Second, func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }) resp, err := client.Get(tangledReleasesBase + "/tags/latest") if err != nil { @@ -219,7 +216,7 @@ func performUpdate(latest string) error { // downloadFile downloads a file from a URL to a local path func downloadFile(url, destPath string) error { - resp, err := http.Get(url) //nolint:gosec + resp, err := httpClient().Get(url) //nolint:gosec if err != nil { return err } diff --git a/cmd/credential-helper/device_auth.go b/cmd/credential-helper/device_auth.go index 079576d..1a2ceb7 100644 --- a/cmd/credential-helper/device_auth.go +++ b/cmd/credential-helper/device_auth.go @@ -57,7 +57,7 @@ func requestDeviceCode(serverURL string) (*DeviceCodeResponse, string, error) { deviceName := hostname() reqBody, _ := json.Marshal(DeviceCodeRequest{DeviceName: deviceName}) - resp, err := http.Post(appViewURL+"/auth/device/code", "application/json", bytes.NewReader(reqBody)) + resp, err := httpClient().Post(appViewURL+"/auth/device/code", "application/json", bytes.NewReader(reqBody)) if err != nil { return nil, appViewURL, fmt.Errorf("failed to request device code: %w", err) } @@ -88,7 +88,7 @@ func pollDeviceToken(appViewURL string, codeResp *DeviceCodeResponse) (*Account, time.Sleep(pollInterval) tokenReqBody, _ := json.Marshal(DeviceTokenRequest{DeviceCode: codeResp.DeviceCode}) - tokenResp, err := http.Post(appViewURL+"/auth/device/token", "application/json", bytes.NewReader(tokenReqBody)) + tokenResp, err := httpClient().Post(appViewURL+"/auth/device/token", "application/json", bytes.NewReader(tokenReqBody)) if err != nil { continue } @@ -120,9 +120,7 @@ func pollDeviceToken(appViewURL string, codeResp *DeviceCodeResponse) (*Account, // validateCredentials checks if the credentials are still valid by making a test request func validateCredentials(appViewURL, handle, deviceSecret string) ValidationResult { - client := &http.Client{ - Timeout: 5 * time.Second, - } + client := httpClientWithTimeout(5*time.Second, nil) tokenURL := appViewURL + "/auth/token?service=" + appViewURL diff --git a/cmd/credential-helper/http.go b/cmd/credential-helper/http.go new file mode 100644 index 0000000..78e3865 --- /dev/null +++ b/cmd/credential-helper/http.go @@ -0,0 +1,75 @@ +package main + +import ( + "fmt" + "net/http" + "runtime" + "sync" + "time" +) + +// userAgent returns the User-Agent string for outgoing HTTP requests. +// +// Format: docker-credential-atcr/ (/; commit ) +// +// Format follows the convention Docker's own clients use, so it parses +// cleanly with the same regexes server-side log analyzers already +// understand. The commit suffix lets users on the device-approval page +// distinguish two devices on the same version line if they ever need to. +func userAgent() string { + short := commit + if len(short) > 7 { + short = short[:7] + } + return fmt.Sprintf("docker-credential-atcr/%s (%s/%s; commit %s)", + version, runtime.GOOS, runtime.GOARCH, short) +} + +// uaTransport wraps another RoundTripper and sets the User-Agent header +// on every request that doesn't already carry one. Used as the default +// transport for the helper's shared http.Client so we can't forget to +// set the UA on a future call site. +type uaTransport struct { + base http.RoundTripper +} + +func (t *uaTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if req.Header.Get("User-Agent") == "" { + // Clone before mutating: net/http may retry a request and the + // caller could be using the same *Request elsewhere. + clone := req.Clone(req.Context()) + clone.Header.Set("User-Agent", userAgent()) + req = clone + } + base := t.base + if base == nil { + base = http.DefaultTransport + } + return base.RoundTrip(req) +} + +var sharedHTTPClient = sync.OnceValue(func() *http.Client { + return &http.Client{ + Transport: &uaTransport{base: http.DefaultTransport}, + } +}) + +// httpClient returns the shared UA-tagged http.Client used for all of +// the helper's outgoing HTTP requests. It carries no per-request +// timeout — call sites that want one should use httpClientWithTimeout. +func httpClient() *http.Client { + return sharedHTTPClient() +} + +// httpClientWithTimeout returns a fresh client that shares the shared +// transport (so connection pooling and the UA header are preserved) but +// scopes a per-client timeout. CheckRedirect can be supplied for cases +// like fetchLatestVersion that need to inspect a redirect rather than +// follow it. +func httpClientWithTimeout(timeout time.Duration, checkRedirect func(*http.Request, []*http.Request) error) *http.Client { + return &http.Client{ + Transport: sharedHTTPClient().Transport, + Timeout: timeout, + CheckRedirect: checkRedirect, + } +} diff --git a/cmd/credential-helper/http_test.go b/cmd/credential-helper/http_test.go new file mode 100644 index 0000000..9caa4f6 --- /dev/null +++ b/cmd/credential-helper/http_test.go @@ -0,0 +1,70 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestUserAgent_Format(t *testing.T) { + old := commit + commit = "abc1234deadbeef" + t.Cleanup(func() { commit = old }) + + ua := userAgent() + if !strings.HasPrefix(ua, "docker-credential-atcr/") { + t.Errorf("UA missing product prefix: %q", ua) + } + if !strings.Contains(ua, "commit abc1234)") { + t.Errorf("UA should truncate commit to 7 chars, got %q", ua) + } + if strings.Contains(ua, "Go-http-client") { + t.Errorf("UA leaked default Go client string: %q", ua) + } +} + +func TestHTTPClient_SetsUserAgent(t *testing.T) { + var got string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r.Header.Get("User-Agent") + w.WriteHeader(http.StatusNoContent) + })) + t.Cleanup(srv.Close) + + resp, err := httpClient().Get(srv.URL) + if err != nil { + t.Fatalf("get: %v", err) + } + resp.Body.Close() + + want := userAgent() + if got != want { + t.Errorf("server saw User-Agent %q, want %q", got, want) + } +} + +func TestHTTPClient_RespectsExplicitUserAgent(t *testing.T) { + var got string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r.Header.Get("User-Agent") + w.WriteHeader(http.StatusNoContent) + })) + t.Cleanup(srv.Close) + + req, err := http.NewRequest("GET", srv.URL, nil) + if err != nil { + t.Fatalf("new request: %v", err) + } + req.Header.Set("User-Agent", "explicit-test/1.0") + + resp, err := httpClient().Do(req) + if err != nil { + t.Fatalf("do: %v", err) + } + resp.Body.Close() + + if got != "explicit-test/1.0" { + t.Errorf("explicit UA was overwritten: got %q", got) + } +} diff --git a/config-appview.example.yaml b/config-appview.example.yaml index ca357b2..ab643ea 100644 --- a/config-appview.example.yaml +++ b/config-appview.example.yaml @@ -70,15 +70,13 @@ jetstream: backfill_enabled: true # How often to re-run backfill to catch missed events. Set to 0 to only backfill on startup. backfill_interval: 24h0m0s - # Relay endpoints for backfill, tried in order on failure. + # Endpoints used for backfill. MUST support com.atproto.sync.listReposByCollection. Tried in order on failure. relay_endpoints: - https://relay1.us-east.bsky.network - https://relay1.us-west.bsky.network # JWT authentication settings. auth: - # RSA private key for signing registry JWTs issued to Docker clients. - key_path: /var/lib/atcr/auth/private-key.pem - # X.509 certificate matching the JWT signing key. + # X.509 certificate matching the JWT signing key (auto-generated on each boot from the JWT key in the database). cert_path: /var/lib/atcr/auth/private-key.crt # Credential helper download settings. credential_helper: diff --git a/config-hold.example.yaml b/config-hold.example.yaml index 9de70ab..467bc5b 100644 --- a/config-hold.example.yaml +++ b/config-hold.example.yaml @@ -45,8 +45,10 @@ server: successor: "" # Use localhost for OAuth redirects during development. test_mode: false - # Request crawl from this relay on startup to make the embedded PDS discoverable. - relay_endpoint: "" + # Endpoints used for proactive scan discovery. MUST support com.atproto.sync.listReposByCollection. Also sent requestCrawl on startup (best-effort, in addition to built-in known relays). + relay_endpoints: + - https://relay1.us-east.bsky.network + - https://relay1.us-west.bsky.network # DID of the appview this hold is managed by (e.g. did:web:atcr.io). Resolved via did:web for URL and public key. appview_did: did:web:172.28.0.2%3A5000 # Read timeout for HTTP requests. diff --git a/deploy/upcloud/configs/hold.yaml.tmpl b/deploy/upcloud/configs/hold.yaml.tmpl index 89ac5ed..8584d9a 100644 --- a/deploy/upcloud/configs/hold.yaml.tmpl +++ b/deploy/upcloud/configs/hold.yaml.tmpl @@ -20,7 +20,9 @@ server: public: false successor: "" test_mode: false - relay_endpoint: "" + relay_endpoints: + - https://relay1.us-east.bsky.network + - https://relay1.us-west.bsky.network appview_did: did:web:seamark.dev read_timeout: 5m0s write_timeout: 5m0s diff --git a/pkg/appview/config.go b/pkg/appview/config.go index b8cf97f..c95dfb5 100644 --- a/pkg/appview/config.go +++ b/pkg/appview/config.go @@ -107,8 +107,8 @@ type JetstreamConfig struct { // How often to re-run backfill to catch missed events. Set to 0 to only backfill on startup. BackfillInterval time.Duration `yaml:"backfill_interval" comment:"How often to re-run backfill to catch missed events. Set to 0 to only backfill on startup."` - // Relay endpoints for backfill, tried in order on failure. - RelayEndpoints []string `yaml:"relay_endpoints" comment:"Relay endpoints for backfill, tried in order on failure."` + // Relay endpoints for backfill — MUST support com.atproto.sync.listReposByCollection. Tried in order on failure. + RelayEndpoints []string `yaml:"relay_endpoints" comment:"Endpoints used for backfill. MUST support com.atproto.sync.listReposByCollection. Tried in order on failure."` } // AuthConfig defines authentication settings diff --git a/pkg/appview/handlers/device.go b/pkg/appview/handlers/device.go index b4d372c..ee03531 100644 --- a/pkg/appview/handlers/device.go +++ b/pkg/appview/handlers/device.go @@ -1,14 +1,16 @@ package handlers import ( + "context" "fmt" - "html/template" "log/slog" "net/http" "net/url" "strings" + "time" "atcr.io/pkg/appview/db" + "atcr.io/pkg/atproto" "github.com/go-chi/chi/v5" "github.com/go-chi/render" ) @@ -205,18 +207,18 @@ func (h *DeviceApprovalPageHandler) ServeHTTP(w http.ResponseWriter, r *http.Req // Get pending authorization pending, ok := h.DeviceStore.GetPendingByUserCode(userCode) if !ok { - h.renderError(w, "Invalid or expired authorization code") + h.renderError(w, r, "That authorization code has expired or doesn't exist. Start a fresh `docker login` from your terminal to get a new one.") return } // Check if already approved if pending.ApprovedDID != nil && *pending.ApprovedDID != "" { - h.renderSuccess(w, pending.DeviceName) + h.renderSuccess(w, r, pending.DeviceName) return } // Render approval page - h.renderApprovalPage(w, sess.Handle, pending) + h.renderApprovalPage(w, r, sess, pending) } // DeviceApproveRequest is the request to approve a device @@ -359,60 +361,157 @@ func (h *RevokeDeviceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) w.WriteHeader(http.StatusOK) } -// Helper functions +// renderApprovalPage renders the device authorization confirmation page. +// The browser-side identity (avatar + handle + display name) and the +// terminal-side device facts are paired side-by-side so a wrong-account +// approval is visually obvious before the Approve button is clicked. +func (h *DeviceApprovalPageHandler) renderApprovalPage(w http.ResponseWriter, r *http.Request, sess *db.Session, pending *db.PendingAuthorization) { + // Hydrate the signed-in sailor: cached avatar/handle from our local + // users table; live displayName from their PDS (best-effort with a tight + // timeout — the page must still render quickly if Bluesky is slow). + user := &db.User{ + DID: sess.DID, + Handle: sess.Handle, + PDSEndpoint: sess.PDSEndpoint, + } + if h.ReadOnlyDB != nil { + if u, err := db.GetUserByDID(h.ReadOnlyDB, sess.DID); err == nil && u != nil { + user = u + } + } + + displayName := fetchDisplayName(r.Context(), sess) + + meta := NewPageMeta( + "Authorize device - "+h.ClientShortName, + "Confirm device authorization for "+h.ClientShortName, + ).WithRobots("noindex"). + WithSiteName(h.ClientShortName) + + pd := NewPageData(r, &h.BaseUIHandler) + pd.User = user -func (h *DeviceApprovalPageHandler) renderApprovalPage(w http.ResponseWriter, handle string, pending *db.PendingAuthorization) { - tmpl := template.Must(template.New("approval").Parse(deviceApprovalTemplate)) data := struct { - Handle string - DeviceName string - UserCode string - IPAddress string + PageData + Meta *PageMeta + Pending *db.PendingAuthorization + ProfileDisplayName string + UserDIDShort string + UserAgentShort string }{ - Handle: handle, - DeviceName: pending.DeviceName, - UserCode: pending.UserCode, - IPAddress: pending.IPAddress, + PageData: pd, + Meta: meta, + Pending: pending, + ProfileDisplayName: displayName, + UserDIDShort: shortenDID(sess.DID), + UserAgentShort: shortenUserAgent(pending.UserAgent), } w.Header().Set("Content-Type", "text/html; charset=utf-8") - if err := tmpl.Execute(w, data); err != nil { + if err := h.Templates.ExecuteTemplate(w, "device-approve", data); err != nil { + slog.Error("Failed to render device approval page", "component", "device/approve", "error", err) http.Error(w, "failed to render template", http.StatusInternalServerError) - return } } -func (h *DeviceApprovalPageHandler) renderSuccess(w http.ResponseWriter, deviceName string) { - tmpl := template.Must(template.New("success").Parse(deviceSuccessTemplate)) +func (h *DeviceApprovalPageHandler) renderSuccess(w http.ResponseWriter, r *http.Request, deviceName string) { + meta := NewPageMeta( + "Device authorized - "+h.ClientShortName, + "Device authorization complete", + ).WithRobots("noindex"). + WithSiteName(h.ClientShortName) + data := struct { + PageData + Meta *PageMeta DeviceName string }{ + PageData: NewPageData(r, &h.BaseUIHandler), + Meta: meta, DeviceName: deviceName, } w.Header().Set("Content-Type", "text/html; charset=utf-8") - if err := tmpl.Execute(w, data); err != nil { + if err := h.Templates.ExecuteTemplate(w, "device-approved", data); err != nil { + slog.Error("Failed to render device success page", "component", "device/approve", "error", err) http.Error(w, "failed to render template", http.StatusInternalServerError) - return } } -func (h *DeviceApprovalPageHandler) renderError(w http.ResponseWriter, message string) { - tmpl := template.Must(template.New("error").Parse(deviceErrorTemplate)) +func (h *DeviceApprovalPageHandler) renderError(w http.ResponseWriter, r *http.Request, message string) { + meta := NewPageMeta( + "Authorization error - "+h.ClientShortName, + "Device authorization could not be completed", + ).WithRobots("noindex"). + WithSiteName(h.ClientShortName) + data := struct { + PageData + Meta *PageMeta Message string }{ - Message: message, + PageData: NewPageData(r, &h.BaseUIHandler), + Meta: meta, + Message: message, } w.Header().Set("Content-Type", "text/html; charset=utf-8") w.WriteHeader(http.StatusBadRequest) - if err := tmpl.Execute(w, data); err != nil { - http.Error(w, "failed to render template", http.StatusInternalServerError) - return + if err := h.Templates.ExecuteTemplate(w, "device-error", data); err != nil { + slog.Error("Failed to render device error page", "component", "device/approve", "error", err) } } +// fetchDisplayName best-effort fetches the sailor's display name from +// their PDS. Returns "" on any failure — the template falls back to the +// handle so the page never blocks on a slow upstream. +func fetchDisplayName(ctx context.Context, sess *db.Session) string { + if sess == nil || sess.PDSEndpoint == "" { + return "" + } + timeoutCtx, cancel := context.WithTimeout(ctx, 1500*time.Millisecond) + defer cancel() + + client := atproto.NewClient(sess.PDSEndpoint, sess.DID, "") + profile, err := client.GetActorProfile(timeoutCtx, sess.DID) + if err != nil || profile == nil { + return "" + } + return strings.TrimSpace(profile.DisplayName) +} + +// shortenDID returns a compact DID for display (e.g. +// "did:plc:abc…xyz") without obscuring its kind. +func shortenDID(did string) string { + if len(did) <= 24 { + return did + } + // Keep the prefix (did:plc: / did:web:) and the last 6 chars. + prefixEnd := strings.Index(did[4:], ":") + if prefixEnd < 0 { + return did[:14] + "…" + did[len(did)-6:] + } + prefixEnd += 5 // include "did:" and the trailing ":" + if len(did)-prefixEnd <= 14 { + return did + } + return did[:prefixEnd+6] + "…" + did[len(did)-6:] +} + +// shortenUserAgent picks a readable summary of the device's UA string — +// almost always something like "docker-credential-atcr/0.x" — and caps +// the length so the device card doesn't blow up on long UA strings. +func shortenUserAgent(ua string) string { + ua = strings.TrimSpace(ua) + if ua == "" { + return "" + } + if len(ua) > 80 { + return ua[:80] + "…" + } + return ua +} + func getClientIP(r *http.Request) string { // Check X-Forwarded-For header xff := r.Header.Get("X-Forwarded-For") @@ -435,122 +534,3 @@ func getClientIP(r *http.Request) string { return r.RemoteAddr } - -// HTML templates - -const deviceApprovalTemplate = ` - - - - Authorize Device - ATCR - - - -
-

Authorize Device

-

User: {{.Handle}}

- -
{{.UserCode}}
- -
-
-
Device Name:
-
{{.DeviceName}}
-
IP Address:
-
{{.IPAddress}}
-
-
- -

Do you want to authorize this device?

-

This device will be able to push and pull container images to your registry.

- -
- - -
-
- - - - -` - -const deviceSuccessTemplate = ` - - - - Device Authorized - ATCR - - - -
-

✓ Device Authorized!

-

Device {{.DeviceName}} has been successfully authorized.

-

You can now close this window and return to your terminal.

-

View your authorized devices

-
- - -` - -const deviceErrorTemplate = ` - - - - Authorization Error - ATCR - - - -
-

✗ Authorization Error

-

{{.Message}}

-

Return to home

-
- - -` diff --git a/pkg/appview/handlers/settings.go b/pkg/appview/handlers/settings.go index 7813a10..7aaa970 100644 --- a/pkg/appview/handlers/settings.go +++ b/pkg/appview/handlers/settings.go @@ -16,6 +16,7 @@ import ( "atcr.io/pkg/appview/storage" "atcr.io/pkg/appview/webhooks" "atcr.io/pkg/atproto" + "atcr.io/pkg/auth" "github.com/bluesky-social/indigo/atproto/syntax" ) @@ -435,8 +436,10 @@ func (h *UpdateDefaultHoldHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ if holdDID != "" { go func() { storage.EnsureCrewMembership( - context.Background(), client, h.Refresher, - holdDID, middleware.GetGlobalAuthorizer(), + context.Background(), user.DID, holdDID, middleware.GetGlobalAuthorizer(), + func(ctx context.Context, holdDID string) (string, error) { + return auth.GetOrFetchServiceToken(ctx, h.Refresher, user.DID, holdDID, user.PDSEndpoint) + }, ) refreshCaptainRecord(holdDID, h.DB) refreshCrewMembership(holdDID, user.DID, h.DB) diff --git a/pkg/appview/middleware/registry.go b/pkg/appview/middleware/registry.go index f57c926..755a8a3 100644 --- a/pkg/appview/middleware/registry.go +++ b/pkg/appview/middleware/registry.go @@ -359,8 +359,13 @@ func (nr *NamespaceResolver) Repository(ctx context.Context, name reference.Name // (returns quickly if already a member - hold returns 200/201) if holdDID != "" && nr.refresher != nil { slog.Debug("Auto-reconciling crew membership", "component", "registry/middleware", "did", did, "hold_did", holdDID) - client := atproto.NewClient(pdsEndpoint, did, "") - storage.EnsureCrewMembership(ctx, client, nr.refresher, holdDID, nr.authorizer) + ownerDID := did + ownerPDS := pdsEndpoint + refresher := nr.refresher + storage.EnsureCrewMembership(ctx, ownerDID, holdDID, nr.authorizer, + func(ctx context.Context, holdDID string) (string, error) { + return auth.GetOrFetchServiceToken(ctx, refresher, ownerDID, holdDID, ownerPDS) + }) } // Get service token for hold authentication (only if authenticated) diff --git a/pkg/appview/server.go b/pkg/appview/server.go index 42de6a1..a600de0 100644 --- a/pkg/appview/server.go +++ b/pkg/appview/server.go @@ -448,10 +448,13 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer, } // Register crew in background slog.Debug("Attempting crew registration", "component", "appview/callback", "did", did, "hold_did", holdDID) - go func(client *atproto.Client, refresher *oauth.Refresher, holdDID string, authorizer auth.HoldAuthorizer) { + go func(userDID, pdsEndpoint, holdDID string, refresher *oauth.Refresher, authorizer auth.HoldAuthorizer) { ctx := context.Background() - storage.EnsureCrewMembership(ctx, client, refresher, holdDID, authorizer) - }(client, s.Refresher, holdDID, s.HoldAuthorizer) + storage.EnsureCrewMembership(ctx, userDID, holdDID, authorizer, + func(ctx context.Context, holdDID string) (string, error) { + return auth.GetOrFetchServiceToken(ctx, refresher, userDID, holdDID, pdsEndpoint) + }) + }(did, pdsEndpoint, holdDID, s.Refresher, s.HoldAuthorizer) } // Drain manifests from old hold to successor in background @@ -566,6 +569,20 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer, slog.Debug("Profile ensured with default hold", "component", "appview/callback", "did", did, "default_hold_did", defaultHoldDID) } + // Self-register as crew of the user's defaultHold so the first docker + // pull/push doesn't 404 because the hold has no crew record for them. + // The web OAuth callback already does this for OAuth flows; this is + // the parity path for app-password logins (only callers of this hook). + if profile, err := storage.GetProfile(ctx, atprotoClient); err == nil && profile != nil && profile.DefaultHold != "" { + go func(userDID, pdsEndpoint, holdDID string, authorizer auth.HoldAuthorizer) { + bgCtx := context.Background() + storage.EnsureCrewMembership(bgCtx, userDID, holdDID, authorizer, + func(ctx context.Context, holdDID string) (string, error) { + return auth.GetOrFetchServiceTokenWithAppPassword(ctx, userDID, holdDID, pdsEndpoint) + }) + }(did, pdsEndpoint, profile.DefaultHold, s.HoldAuthorizer) + } + // Run consumer hooks for _, hook := range s.tokenHooks { if err := hook(ctx, did, handle, pdsEndpoint, accessToken); err != nil { diff --git a/pkg/appview/src/css/main.css b/pkg/appview/src/css/main.css index f280886..6d60b11 100644 --- a/pkg/appview/src/css/main.css +++ b/pkg/appview/src/css/main.css @@ -967,6 +967,97 @@ .provider-cta { grid-area: cta; width: 100%; justify-content: center; } } + /* ---------------------------------------- + DEVICE AUTHORIZATION — pairing card + The approval page leans on the visual pairing of *account* and + *device*. `device-mark` is the round identity slot (avatar fits + inside); `device-mark-square` is its rounded-rect counterpart for + the device side, so the two halves read as related but distinct. + + `bearing-line` renders a dashed plotted-depth pattern as either a + horizontal or vertical separator — same texture used on the signup + provider rows, repurposed here as a maritime gesture on the + connector between the two cards and as a section rule. + ---------------------------------------- */ + .device-mark { + @apply rounded-full overflow-hidden bg-base-300; + width: 4.5rem; + height: 4.5rem; + box-shadow: + 0 0 0 1px var(--color-base-300), + 0 0 0 4px color-mix(in oklch, var(--color-primary) 14%, transparent); + } + .device-mark-square { + @apply rounded-lg; + box-shadow: + inset 0 0 0 1px oklch(100% 0 0 / 0.06), + 0 0 0 1px var(--color-base-300); + } + + /* Plotted-depth dashed line — same pattern as .provider-row dividers, + extracted into reusable utilities. The horizontal variant fills its + container; the vertical variant uses gradient direction swap. */ + .bearing-line { + display: block; + background-repeat: repeat; + } + .bearing-line-h { + flex: 1; + height: 1px; + background-image: linear-gradient( + to right, + var(--color-base-300) 0 4px, + transparent 4px 8px + ); + background-size: 8px 1px; + } + .bearing-line-v { + flex: 1; + width: 1px; + background-image: linear-gradient( + to bottom, + var(--color-base-300) 0 4px, + transparent 4px 8px + ); + background-size: 1px 8px; + } + /* Variant: dashed top border on a block (used as a section separator) */ + .bearing-line-h-full { + border-top: 0; + background-image: linear-gradient( + to right, + var(--color-base-300) 0 4px, + transparent 4px 8px + ); + background-size: 8px 1px; + background-repeat: repeat-x; + background-position: top left; + } + + /* Instrument readout — a boxed, mono, letter-spaced display for the + verification code. Tick marks at top and bottom evoke a depth + sounder or a chart's tick scale, signalling "instrument", not + "decorative pill". */ + .instrument-readout { + @apply relative bg-base-200 rounded-lg px-6 py-5; + @apply flex flex-col items-center gap-3; + box-shadow: inset 0 0 0 1px oklch(100% 0 0 / 0.06); + } + .instrument-tick { + display: block; + height: 6px; + width: 100%; + max-width: 22rem; + background-image: linear-gradient( + to right, + color-mix(in oklch, var(--color-primary) 70%, transparent) 0 1px, + transparent 1px 12px + ); + background-size: 12px 6px; + background-repeat: repeat-x; + opacity: 0.6; + } + /* ---------------------------------------- SIGNUP CONTINUE — handoff mark A slightly larger avatar ringed in primary, so the "you are leaving" diff --git a/pkg/appview/storage/crew.go b/pkg/appview/storage/crew.go index 7368fa2..659ad84 100644 --- a/pkg/appview/storage/crew.go +++ b/pkg/appview/storage/crew.go @@ -10,14 +10,27 @@ import ( "atcr.io/pkg/atproto" "atcr.io/pkg/auth" - "atcr.io/pkg/auth/oauth" ) +// ServiceTokenFetcher returns a hold service token for the resolved holdDID. +// Implementations wire to the appropriate auth path (OAuth refresher or +// app-password access token). +type ServiceTokenFetcher func(ctx context.Context, holdDID string) (string, error) + // EnsureCrewMembership attempts to register the user as a crew member on their default hold. -// The hold's requestCrew endpoint handles all authorization logic (checking allowAllCrew, existing membership, etc). -// On success, clears any cached denial to ensure immediate access. -// This is best-effort and does not fail on errors. -func EnsureCrewMembership(ctx context.Context, client *atproto.Client, refresher *oauth.Refresher, defaultHoldDID string, authorizer auth.HoldAuthorizer) { +// The hold's requestCrew endpoint handles all authorization logic (checking allowAllCrew, +// existing membership, etc). On success, warms the approval cache and clears any cached +// denial. Best-effort: logs and returns on any error. +// +// fetchServiceToken is invoked only on cache miss. Pass nil to skip when no auth path +// is available (callers that just want the cache short-circuit behavior). +func EnsureCrewMembership( + ctx context.Context, + userDID string, + defaultHoldDID string, + authorizer auth.HoldAuthorizer, + fetchServiceToken ServiceTokenFetcher, +) { if defaultHoldDID == "" { return } @@ -34,31 +47,27 @@ func EnsureCrewMembership(ctx context.Context, client *atproto.Client, refresher // is pure waste. Cache lookup uses the same key (holdDID, userDID) that // CheckWriteAccess will hit later in the request. if authorizer != nil { - if cached, err := authorizer.IsCachedCrewMember(ctx, holdDID, client.DID()); err == nil && cached { + if cached, err := authorizer.IsCachedCrewMember(ctx, holdDID, userDID); err == nil && cached { slog.Debug("crew membership cached, skipping requestCrew", - "holdDID", holdDID, "userDID", client.DID()) + "holdDID", holdDID, "userDID", userDID) return } } - // Resolve hold DID to HTTP endpoint + if fetchServiceToken == nil { + slog.Debug("skipping crew registration - no service token fetcher", "holdDID", holdDID, "userDID", userDID) + return + } + holdEndpoint, err := atproto.ResolveHoldURL(ctx, holdDID) if err != nil { slog.Warn("failed to resolve hold URL", "holdDID", holdDID, "error", err) return } - // Get service token for the hold - // Only works with OAuth (refresher required) - app passwords can't get service tokens - if refresher == nil { - slog.Debug("skipping crew registration - no OAuth refresher (app password flow)", "holdDID", holdDID) - return - } - - // Wrap the refresher to match OAuthSessionRefresher interface - serviceToken, err := auth.GetOrFetchServiceToken(ctx, refresher, client.DID(), holdDID, client.PDSEndpoint()) + serviceToken, err := fetchServiceToken(ctx, holdDID) if err != nil { - slog.Warn("failed to get service token", "holdDID", holdDID, "error", err) + slog.Warn("failed to get service token", "holdDID", holdDID, "userDID", userDID, "error", err) return } @@ -67,28 +76,24 @@ func EnsureCrewMembership(ctx context.Context, client *atproto.Client, refresher // - Checks if already a crew member (returns success if so) // - Creates crew record if authorized if err := requestCrewMembership(ctx, holdEndpoint, serviceToken); err != nil { - slog.Warn("failed to request crew membership", "holdDID", holdDID, "error", err) + slog.Warn("failed to request crew membership", "holdDID", holdDID, "userDID", userDID, "error", err) return } - slog.Info("successfully registered as crew member", "holdDID", holdDID, "userDID", client.DID()) + slog.Info("successfully registered as crew member", "holdDID", holdDID, "userDID", userDID) if authorizer != nil { // Warm the approval cache so subsequent CheckWriteAccess calls within this // request (e.g. each layer in a multi-layer push) skip the XRPC getRecord. - if err := authorizer.RecordCrewApproval(ctx, holdDID, client.DID()); err != nil { + if err := authorizer.RecordCrewApproval(ctx, holdDID, userDID); err != nil { slog.Warn("failed to record crew approval after crew registration", - "holdDID", holdDID, - "userDID", client.DID(), - "error", err) + "holdDID", holdDID, "userDID", userDID, "error", err) } // Clear any cached denial to ensure immediate access - if err := authorizer.ClearCrewDenial(ctx, holdDID, client.DID()); err != nil { + if err := authorizer.ClearCrewDenial(ctx, holdDID, userDID); err != nil { slog.Warn("failed to clear denial cache after crew registration", - "holdDID", holdDID, - "userDID", client.DID(), - "error", err) + "holdDID", holdDID, "userDID", userDID, "error", err) } } } diff --git a/pkg/appview/storage/crew_test.go b/pkg/appview/storage/crew_test.go index 86cd41c..ac563c7 100644 --- a/pkg/appview/storage/crew_test.go +++ b/pkg/appview/storage/crew_test.go @@ -11,7 +11,7 @@ import ( func TestEnsureCrewMembership_EmptyHoldDID(t *testing.T) { // Test that empty hold DID returns early without error (best-effort function) - EnsureCrewMembership(context.Background(), nil, nil, "", nil) + EnsureCrewMembership(context.Background(), "did:plc:user123", "", nil, nil) // If we get here without panic, test passes } @@ -51,22 +51,26 @@ func (f *fakeAuthorizer) RecordCrewApproval(ctx context.Context, holdDID, userDI var _ auth.HoldAuthorizer = (*fakeAuthorizer)(nil) // TestEnsureCrewMembership_SkipsRequestCrewWhenCached verifies the cache short-circuit: -// when IsCachedCrewMember returns true, the function returns before attempting the -// requestCrew POST (and before fetching a service token, which would otherwise fail -// with a nil refresher). +// when IsCachedCrewMember returns true, the function returns before invoking the +// service-token fetcher (and thus before any requestCrew POST). func TestEnsureCrewMembership_SkipsRequestCrewWhenCached(t *testing.T) { authz := &fakeAuthorizer{cachedReturn: true} - client := atproto.NewClient("https://pds.example", "did:plc:user123", "") holdDID := "did:web:hold01.atcr.io" - // Pass nil refresher: if the cache check did NOT short-circuit, the function - // would log "skipping crew registration" and still return without panic — but - // it also would NOT have called IsCachedCrewMember, which is what we assert. - EnsureCrewMembership(context.Background(), client, nil, holdDID, authz) + fetcherCalls := atomic.Int32{} + fetcher := func(ctx context.Context, _ string) (string, error) { + fetcherCalls.Add(1) + return "", nil + } + + EnsureCrewMembership(context.Background(), "did:plc:user123", holdDID, authz, fetcher) if authz.isCachedCalls.Load() != 1 { t.Errorf("Expected IsCachedCrewMember to be called once, got %d", authz.isCachedCalls.Load()) } + if fetcherCalls.Load() != 0 { + t.Errorf("Expected service token fetcher not to be called on cache hit, got %d", fetcherCalls.Load()) + } if authz.recordApprovalCalls.Load() != 0 { t.Errorf("Expected RecordCrewApproval not to be called on cache hit, got %d", authz.recordApprovalCalls.Load()) } @@ -75,15 +79,13 @@ func TestEnsureCrewMembership_SkipsRequestCrewWhenCached(t *testing.T) { } } -// TestEnsureCrewMembership_NoRefresherFallsThroughCacheCheck verifies that a cache -// miss with a nil refresher takes the existing app-password skip path, and does not -// call RecordCrewApproval (since requestCrew never ran). -func TestEnsureCrewMembership_NoRefresherFallsThroughCacheCheck(t *testing.T) { +// TestEnsureCrewMembership_NilFetcherSkipsAfterCacheCheck verifies that a cache miss +// with a nil fetcher returns silently and does not call RecordCrewApproval. +func TestEnsureCrewMembership_NilFetcherSkipsAfterCacheCheck(t *testing.T) { authz := &fakeAuthorizer{cachedReturn: false} - client := atproto.NewClient("https://pds.example", "did:plc:user123", "") holdDID := "did:web:hold01.atcr.io" - EnsureCrewMembership(context.Background(), client, nil, holdDID, authz) + EnsureCrewMembership(context.Background(), "did:plc:user123", holdDID, authz, nil) if authz.isCachedCalls.Load() != 1 { t.Errorf("Expected IsCachedCrewMember to be called once, got %d", authz.isCachedCalls.Load()) diff --git a/pkg/appview/templates/pages/device.html b/pkg/appview/templates/pages/device.html new file mode 100644 index 0000000..bb219bd --- /dev/null +++ b/pkg/appview/templates/pages/device.html @@ -0,0 +1,273 @@ +{{/* + Device authorization templates. + + Three states share a layout shell: + - device-approve : pending — show identity, device, code, actions + - device-approved : success + - device-error : invalid / expired + + The visual idea on the approval page is to make the *pairing* obvious: + the signed-in account on one side, the requesting device on the other. + If the user is staring at someone else's avatar, the page should make + that mismatch impossible to miss before the Approve button is clicked. +*/}} + +{{ define "device-approve" }} + + + + {{ template "head" . }} + {{ template "meta" .Meta }} + + + {{ template "nav-simple" . }} + +
+
+ + {{/* Eyebrow — small-caps bearing label */}} +
+ {{ icon "compass" "size-4 text-primary" }} + Device authorization +
+ +

Confirm the pairing

+

+ A device is requesting credentials for {{ .ClientShortName }}. + The pairing below will let it push and pull on your behalf, so make sure + both halves are correct before approving. +

+ + {{/* The pairing — identity on one side, device on the other. + On md+ they sit side-by-side with a connector between; on mobile + they stack with the connector rotated. */}} +
+ + {{/* Identity card */}} +
+ + Account + +
+ {{ if .User.Avatar }} + + {{ else }} +
+ {{ firstChar .User.Handle }} +
+ {{ end }} +
+
+ @{{ .User.Handle }} +
+ {{ if .ProfileDisplayName }} +
+ {{ .ProfileDisplayName }} +
+ {{ end }} +
+ {{ .UserDIDShort }} +
+
+ + {{/* Connector — horizontal divider through the verb */}} + + + {{/* Device card */}} +
+ + Device + +
+ {{ icon "terminal" "size-8" }} +
+
+ {{ .Pending.DeviceName }} +
+
+ {{ .Pending.IPAddress }} +
+ {{ if .UserAgentShort }} +
+ {{ .UserAgentShort }} +
+ {{ end }} +
+
+ + {{/* Verification code — instrument readout */}} +
+
+ + Verification code + + + Should match your terminal + +
+
+ + {{ .Pending.UserCode }} + +
+
+ + {{/* Actions */}} +
+ + +
+ + {{/* Switch-account affordance — bearing-line separator above it */}} +
+ Not @{{ .User.Handle }}? + + Switch accounts to authorize as someone else + +
+ + +
+
+ + {{ template "footer" . }} + + + + +{{ end }} + + +{{ define "device-approved" }} + + + + {{ template "head" . }} + {{ template "meta" .Meta }} + + + {{ template "nav-simple" . }} + +
+
+
+ {{ icon "check-circle" "size-8" }} +
+

Device authorized

+

+ {{ .DeviceName }} + is now authorized. You can return to your terminal and close this tab. +

+
+ Manage devices + Back to home +
+
+
+ + {{ template "footer" . }} + + +{{ end }} + + +{{ define "device-error" }} + + + + {{ template "head" . }} + {{ template "meta" .Meta }} + + + {{ template "nav-simple" . }} + +
+
+
+ {{ icon "alert-triangle" "size-8" }} +
+

Authorization couldn't be completed

+

{{ .Message }}

+
+ Back to home +
+
+
+ + {{ template "footer" . }} + + +{{ end }} diff --git a/pkg/hold/config.go b/pkg/hold/config.go index eb191ec..5f32cd2 100644 --- a/pkg/hold/config.go +++ b/pkg/hold/config.go @@ -163,8 +163,12 @@ type ServerConfig struct { // Use localhost for OAuth redirects during development. TestMode bool `yaml:"test_mode" comment:"Use localhost for OAuth redirects during development."` - // Request crawl from this relay on startup. - RelayEndpoint string `yaml:"relay_endpoint" comment:"Request crawl from this relay on startup to make the embedded PDS discoverable."` + // Relay endpoints used primarily for proactive scan discovery via + // com.atproto.sync.listReposByCollection. Endpoints listed here MUST + // support listReposByCollection. They are also sent requestCrawl on + // startup (in addition to the built-in known-relay list); endpoints that + // don't implement requestCrawl just 404 silently. + RelayEndpoints []string `yaml:"relay_endpoints" comment:"Endpoints used for proactive scan discovery. MUST support com.atproto.sync.listReposByCollection. Also sent requestCrawl on startup (best-effort, in addition to built-in known relays)."` // DID of the appview this hold is managed by. Resolved via did:web for URL and public key discovery. AppviewDID string `yaml:"appview_did" comment:"DID of the appview this hold is managed by (e.g. did:web:atcr.io). Resolved via did:web for URL and public key."` @@ -254,7 +258,10 @@ func setHoldDefaults(v *viper.Viper) { v.SetDefault("server.public", false) v.SetDefault("server.successor", "") v.SetDefault("server.test_mode", false) - v.SetDefault("server.relay_endpoint", "") + v.SetDefault("server.relay_endpoints", []string{ + "https://relay1.us-east.bsky.network", + "https://relay1.us-west.bsky.network", + }) v.SetDefault("server.appview_did", "did:web:atcr.io") v.SetDefault("server.read_timeout", "5m") v.SetDefault("server.write_timeout", "5m") diff --git a/pkg/hold/pds/scan_broadcaster.go b/pkg/hold/pds/scan_broadcaster.go index 380a08b..0cd1df1 100644 --- a/pkg/hold/pds/scan_broadcaster.go +++ b/pkg/hold/pds/scan_broadcaster.go @@ -42,7 +42,9 @@ type ScanBroadcaster struct { stopCh chan struct{} // Signal to stop background goroutines wg sync.WaitGroup // Wait for background goroutines to finish predecessorCache map[string]bool // holdDID → "has this hold been migrated (has successor)?" - relayEndpoint string // Relay URL for listReposByCollection + relayEndpoints []string // Relay URLs for listReposByCollection (failover order) + relayStartIdx int // Rotates per discovery pass so load is shared across endpoints + relayStartMu sync.Mutex // Work queues for proactive scanning (populated by discovery/stale goroutines) unscannedQueue chan *scanCandidate // Medium priority: manifests with no scan record @@ -99,7 +101,7 @@ type VulnerabilitySummary struct { // NewScanBroadcaster creates a new scan job broadcaster // dbPath should point to a SQLite database file (e.g., "/path/to/pds/db.sqlite3") -func NewScanBroadcaster(holdDID, holdEndpoint, secret, relayEndpoint, dbPath string, s3svc *s3.S3Service, holdPDS *HoldPDS, rescanInterval time.Duration) (*ScanBroadcaster, error) { +func NewScanBroadcaster(holdDID, holdEndpoint, secret string, relayEndpoints []string, dbPath string, s3svc *s3.S3Service, holdPDS *HoldPDS, rescanInterval time.Duration) (*ScanBroadcaster, error) { dsn := dbPath if dbPath != ":memory:" && !strings.HasPrefix(dbPath, "file:") { dsn = "file:" + dbPath @@ -125,9 +127,7 @@ func NewScanBroadcaster(holdDID, holdEndpoint, secret, relayEndpoint, dbPath str return nil, fmt.Errorf("failed to set busy_timeout: %w", err) } - if relayEndpoint == "" { - relayEndpoint = "https://relay1.us-east.bsky.network" - } + relayEndpoints = normalizeRelayEndpoints(relayEndpoints) sb := &ScanBroadcaster{ subscribers: make([]*ScanSubscriber, 0), @@ -142,7 +142,7 @@ func NewScanBroadcaster(holdDID, holdEndpoint, secret, relayEndpoint, dbPath str rescanInterval: rescanInterval, stopCh: make(chan struct{}), predecessorCache: make(map[string]bool), - relayEndpoint: relayEndpoint, + relayEndpoints: relayEndpoints, unscannedQueue: make(chan *scanCandidate, 500), staleQueue: make(chan *scanCandidate, 200), inflight: make(map[string]struct{}), @@ -164,7 +164,7 @@ func NewScanBroadcaster(holdDID, holdEndpoint, secret, relayEndpoint, dbPath str go sb.discoveryLoop() go sb.staleScanLoop() go sb.dispatchLoop() - slog.Info("Proactive scan scheduler started", "rescanInterval", rescanInterval, "relayEndpoint", relayEndpoint) + slog.Info("Proactive scan scheduler started", "rescanInterval", rescanInterval, "relayEndpoints", relayEndpoints) } return sb, nil @@ -172,10 +172,8 @@ func NewScanBroadcaster(holdDID, holdEndpoint, secret, relayEndpoint, dbPath str // NewScanBroadcasterWithDB creates a scan job broadcaster using an existing *sql.DB connection. // The caller is responsible for the DB lifecycle. -func NewScanBroadcasterWithDB(holdDID, holdEndpoint, secret, relayEndpoint string, db *sql.DB, s3svc *s3.S3Service, holdPDS *HoldPDS, rescanInterval time.Duration) (*ScanBroadcaster, error) { - if relayEndpoint == "" { - relayEndpoint = "https://relay1.us-east.bsky.network" - } +func NewScanBroadcasterWithDB(holdDID, holdEndpoint, secret string, relayEndpoints []string, db *sql.DB, s3svc *s3.S3Service, holdPDS *HoldPDS, rescanInterval time.Duration) (*ScanBroadcaster, error) { + relayEndpoints = normalizeRelayEndpoints(relayEndpoints) sb := &ScanBroadcaster{ subscribers: make([]*ScanSubscriber, 0), @@ -190,7 +188,7 @@ func NewScanBroadcasterWithDB(holdDID, holdEndpoint, secret, relayEndpoint strin rescanInterval: rescanInterval, stopCh: make(chan struct{}), predecessorCache: make(map[string]bool), - relayEndpoint: relayEndpoint, + relayEndpoints: relayEndpoints, unscannedQueue: make(chan *scanCandidate, 500), staleQueue: make(chan *scanCandidate, 200), inflight: make(map[string]struct{}), @@ -209,12 +207,30 @@ func NewScanBroadcasterWithDB(holdDID, holdEndpoint, secret, relayEndpoint strin go sb.discoveryLoop() go sb.staleScanLoop() go sb.dispatchLoop() - slog.Info("Proactive scan scheduler started", "rescanInterval", rescanInterval, "relayEndpoint", relayEndpoint) + slog.Info("Proactive scan scheduler started", "rescanInterval", rescanInterval, "relayEndpoints", relayEndpoints) } return sb, nil } +// normalizeRelayEndpoints drops empty entries and falls back to a sensible default +// if the resulting list is empty. The default mirrors the appview backfill default. +func normalizeRelayEndpoints(endpoints []string) []string { + out := make([]string, 0, len(endpoints)) + for _, e := range endpoints { + if e != "" { + out = append(out, e) + } + } + if len(out) == 0 { + return []string{ + "https://relay1.us-east.bsky.network", + "https://relay1.us-west.bsky.network", + } + } + return out +} + // initSchema creates the scan_jobs table if it doesn't exist func (sb *ScanBroadcaster) initSchema() error { // Execute statements individually for go-libsql compatibility @@ -913,18 +929,51 @@ func (sb *ScanBroadcaster) runDiscoveryPass() { slog.Info("Discovery: pass complete", "users", len(userDIDs), "unscannedFound", found) } -// fetchManifestDIDs queries the relay for all DIDs with io.atcr.manifest records. +// fetchManifestDIDs queries relays for all DIDs with io.atcr.manifest records. +// Uses failover: starts from a rotated index (so load is shared across passes) +// and falls through to the next relay if one fails. Returns DIDs from the first +// relay that produces a complete result; partial results from a failing relay +// are discarded so we don't mix incomplete responses. func (sb *ScanBroadcaster) fetchManifestDIDs(ctx context.Context) []string { - client := atproto.NewClient(sb.relayEndpoint, "", "") + endpoints := sb.relayEndpoints + if len(endpoints) == 0 { + return nil + } + + sb.relayStartMu.Lock() + start := sb.relayStartIdx % len(endpoints) + sb.relayStartIdx = (sb.relayStartIdx + 1) % len(endpoints) + sb.relayStartMu.Unlock() + + for i := 0; i < len(endpoints); i++ { + relay := endpoints[(start+i)%len(endpoints)] + dids, err := sb.fetchManifestDIDsFrom(ctx, relay) + if err != nil { + slog.Warn("Discovery: relay failed, trying next", + "relay", relay, "error", err) + continue + } + if i > 0 { + slog.Info("Discovery: succeeded after failover", "relay", relay, "attemptsTried", i+1) + } + return dids + } + + slog.Warn("Discovery: all relays failed", "relays", endpoints) + return nil +} + +// fetchManifestDIDsFrom paginates through io.atcr.manifest repos on a single relay. +// Any error during pagination invalidates the partial result. +func (sb *ScanBroadcaster) fetchManifestDIDsFrom(ctx context.Context, relay string) ([]string, error) { + client := atproto.NewClient(relay, "", "") var allDIDs []string var cursor string for { result, err := client.ListReposByCollection(ctx, atproto.ManifestCollection, 1000, cursor) if err != nil { - slog.Warn("Discovery: failed to list repos from relay", - "relay", sb.relayEndpoint, "error", err) - return allDIDs // Return what we have so far + return nil, err } for _, repo := range result.Repos { @@ -937,7 +986,7 @@ func (sb *ScanBroadcaster) fetchManifestDIDs(ctx context.Context) []string { cursor = result.Cursor } - return allDIDs + return allDIDs, nil } // discoverUnscannedForUser fetches manifests from a user's PDS and pushes any diff --git a/pkg/hold/server.go b/pkg/hold/server.go index ffc62f9..a7e7848 100644 --- a/pkg/hold/server.go +++ b/pkg/hold/server.go @@ -7,6 +7,7 @@ import ( "net/http" "os" "os/signal" + "sync" "syscall" "time" @@ -228,10 +229,10 @@ func NewHoldServer(cfg *Config) (*HoldServer, error) { rescanInterval := cfg.Scanner.RescanInterval var sb *pds.ScanBroadcaster if s.holdDB != nil { - sb, err = pds.NewScanBroadcasterWithDB(holdDID, cfg.Server.PublicURL, cfg.Scanner.Secret, cfg.Server.RelayEndpoint, s.holdDB.DB, s3Service, s.PDS, rescanInterval) + sb, err = pds.NewScanBroadcasterWithDB(holdDID, cfg.Server.PublicURL, cfg.Scanner.Secret, cfg.Server.RelayEndpoints, s.holdDB.DB, s3Service, s.PDS, rescanInterval) } else { scanDBPath := cfg.Database.Path + "/db.sqlite3" - sb, err = pds.NewScanBroadcaster(holdDID, cfg.Server.PublicURL, cfg.Scanner.Secret, cfg.Server.RelayEndpoint, scanDBPath, s3Service, s.PDS, rescanInterval) + sb, err = pds.NewScanBroadcaster(holdDID, cfg.Server.PublicURL, cfg.Scanner.Secret, cfg.Server.RelayEndpoints, scanDBPath, s3Service, s.PDS, rescanInterval) } if err != nil { return nil, fmt.Errorf("failed to initialize scan broadcaster: %w", err) @@ -374,14 +375,14 @@ func (s *HoldServer) Serve() error { } } - // Request crawl from relay to make PDS discoverable - if s.Config.Server.RelayEndpoint != "" { - slog.Info("Requesting crawl from relay", "relay", s.Config.Server.RelayEndpoint) - if err := atproto.RequestCrawl(s.Config.Server.RelayEndpoint, s.Config.Server.PublicURL); err != nil { - slog.Warn("Failed to request crawl from relay", "error", err) - } else { - slog.Info("Crawl requested successfully") - } + // Request crawl from every known relay (plus any custom endpoint) so the + // embedded PDS becomes discoverable. Without this, did:web holds are + // invisible to relays — and to any appview that backfills via them. + // Skipped in test_mode: local dev holds aren't reachable by public relays. + if !s.Config.Server.TestMode { + go s.requestCrawls() + } else { + slog.Info("Skipping relay crawl requests (test_mode enabled)") } // Start garbage collector (runs on startup + nightly) @@ -409,6 +410,47 @@ func (s *HoldServer) Serve() error { return nil } +// requestCrawls fans out com.atproto.sync.requestCrawl to every known relay so +// the embedded PDS becomes discoverable on the relay network. Configured +// RelayEndpoints (if any aren't already in KnownRelays) are included as well. +// Best-effort: per-relay failures are logged but never block startup. +func (s *HoldServer) requestCrawls() { + publicURL := s.Config.Server.PublicURL + if publicURL == "" { + return + } + + seen := make(map[string]bool) + targets := make([]string, 0, len(atproto.KnownRelays)+len(s.Config.Server.RelayEndpoints)) + for _, r := range atproto.KnownRelays { + if !seen[r.URL] { + seen[r.URL] = true + targets = append(targets, r.URL) + } + } + for _, custom := range s.Config.Server.RelayEndpoints { + if custom != "" && !seen[custom] { + seen[custom] = true + targets = append(targets, custom) + } + } + + slog.Info("Requesting crawl from relays", "count", len(targets)) + var wg sync.WaitGroup + for _, relay := range targets { + wg.Add(1) + go func(relay string) { + defer wg.Done() + if err := atproto.RequestCrawl(relay, publicURL); err != nil { + slog.Warn("Failed to request crawl from relay", "relay", relay, "error", err) + return + } + slog.Info("Crawl requested from relay", "relay", relay) + }(relay) + } + wg.Wait() +} + func (s *HoldServer) shutdown() { // Update status post to "offline" before shutdown if s.PDS != nil {