From abefcfd1ed3eb4824ad202fea1370a8eac059388 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Sun, 15 Feb 2026 14:20:02 -0600 Subject: [PATCH] let appview work with did:plc based storage servers --- pkg/appview/handlers/attestation_details.go | 9 +- pkg/appview/handlers/delete.go | 11 +- pkg/appview/handlers/device.go | 2 +- pkg/appview/handlers/export.go | 13 +- pkg/appview/handlers/storage.go | 6 +- pkg/appview/handlers/subscription.go | 16 ++- pkg/appview/holdhealth/checker.go | 8 +- pkg/appview/holdhealth/checker_test.go | 14 +- pkg/appview/jetstream/backfill.go | 10 +- pkg/appview/middleware/registry.go | 28 ++-- pkg/appview/middleware/registry_test.go | 6 +- pkg/appview/storage/context.go | 3 +- pkg/appview/storage/crew.go | 6 +- pkg/appview/storage/manifest_store.go | 5 +- pkg/appview/storage/proxy_blob_store.go | 4 +- pkg/appview/storage/proxy_blob_store_test.go | 25 ++-- pkg/atproto/resolver.go | 64 ++++++--- pkg/atproto/resolver_test.go | 137 +++++++------------ pkg/auth/hold_remote.go | 10 +- 19 files changed, 199 insertions(+), 178 deletions(-) diff --git a/pkg/appview/handlers/attestation_details.go b/pkg/appview/handlers/attestation_details.go index a2f5d57..ed67760 100644 --- a/pkg/appview/handlers/attestation_details.go +++ b/pkg/appview/handlers/attestation_details.go @@ -263,10 +263,13 @@ func fetchLayersFromPDS(ctx context.Context, pdsEndpoint, did, attestationDigest // Two-hop flow: (1) get presigned URL from hold, (2) fetch blob from S3. // serviceToken is optional — pass "" for public holds. func fetchLayerBlob(ctx context.Context, holdEndpoint, layerDigest, serviceToken string) ([]byte, error) { - holdURL := atproto.ResolveHoldURL(holdEndpoint) + holdURL, err := atproto.ResolveHoldURL(ctx, holdEndpoint) + if err != nil { + return nil, fmt.Errorf("could not resolve hold endpoint %s: %w", holdEndpoint, err) + } holdDID := atproto.ResolveHoldDIDFromURL(holdEndpoint) - if holdURL == "" || holdDID == "" { - return nil, fmt.Errorf("could not resolve hold endpoint: %s", holdEndpoint) + if holdDID == "" { + return nil, fmt.Errorf("could not resolve hold DID from: %s", holdEndpoint) } // Step 1: Request presigned URL from hold diff --git a/pkg/appview/handlers/delete.go b/pkg/appview/handlers/delete.go index cbe8bc9..16b17fb 100644 --- a/pkg/appview/handlers/delete.go +++ b/pkg/appview/handlers/delete.go @@ -196,8 +196,7 @@ func (h *DeleteAccountHandler) deleteFromHolds(ctx context.Context, user *db.Use // deleteFromSingleHold deletes user data from a single hold func (h *DeleteAccountHandler) deleteFromSingleHold(ctx context.Context, user *db.User, holdDID, relationship string) HoldDeleteResult { // Resolve hold DID to URL - holdURL := atproto.ResolveHoldURL(holdDID) - endpoint := holdURL + "/xrpc/io.atcr.hold.deleteUserData" + holdURL, err := atproto.ResolveHoldURL(ctx, holdDID) result := HoldDeleteResult{ HoldDID: holdDID, @@ -205,6 +204,14 @@ func (h *DeleteAccountHandler) deleteFromSingleHold(ctx context.Context, user *d Status: "failed", } + if err != nil { + slog.Warn("Failed to resolve hold URL for deletion", "holdDid", holdDID, "error", err) + result.Error = fmt.Sprintf("Failed to resolve hold URL: %v", err) + return result + } + + endpoint := holdURL + "/xrpc/io.atcr.hold.deleteUserData" + // Check if we have OAuth refresher (needed for service tokens) if h.Refresher == nil { result.Error = "OAuth not configured - cannot authenticate to hold" diff --git a/pkg/appview/handlers/device.go b/pkg/appview/handlers/device.go index 7bed18f..e72e8b2 100644 --- a/pkg/appview/handlers/device.go +++ b/pkg/appview/handlers/device.go @@ -527,7 +527,7 @@ const deviceSuccessTemplate = `

✓ Device Authorized!

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

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

-

View your authorized devices

+

View your authorized devices

diff --git a/pkg/appview/handlers/export.go b/pkg/appview/handlers/export.go index 616ab0e..fbfed8c 100644 --- a/pkg/appview/handlers/export.go +++ b/pkg/appview/handlers/export.go @@ -167,17 +167,24 @@ func (h *ExportUserDataHandler) fetchHoldExports(ctx context.Context, user *db.U // fetchSingleHoldExport fetches export data from a single hold func (h *ExportUserDataHandler) fetchSingleHoldExport(ctx context.Context, user *db.User, holdDID string, meta holdMetadata) HoldExportResult { // Resolve hold DID to URL - holdURL := atproto.ResolveHoldURL(holdDID) - endpoint := holdURL + "/xrpc/io.atcr.hold.exportUserData" + holdURL, err := atproto.ResolveHoldURL(ctx, holdDID) result := HoldExportResult{ HoldDID: holdDID, - Endpoint: endpoint, Relationship: meta.relationship, FirstSeen: meta.firstSeen, Status: "failed", } + if err != nil { + slog.Warn("Failed to resolve hold URL for export", "holdDid", holdDID, "error", err) + result.Error = fmt.Sprintf("Failed to resolve hold URL: %v", err) + return result + } + + endpoint := holdURL + "/xrpc/io.atcr.hold.exportUserData" + result.Endpoint = endpoint + // Check if we have OAuth refresher (needed for service tokens) if h.Refresher == nil { result.Error = "OAuth not configured - cannot authenticate to hold" diff --git a/pkg/appview/handlers/storage.go b/pkg/appview/handlers/storage.go index 94d74bf..bd8f3b0 100644 --- a/pkg/appview/handlers/storage.go +++ b/pkg/appview/handlers/storage.go @@ -52,9 +52,9 @@ func (h *StorageHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Resolve hold URL from DID - holdURL := atproto.ResolveHoldURL(holdDID) - if holdURL == "" { - slog.Warn("Failed to resolve hold URL", "did", user.DID, "holdDid", holdDID) + holdURL, err := atproto.ResolveHoldURL(r.Context(), holdDID) + if err != nil { + slog.Warn("Failed to resolve hold URL", "did", user.DID, "holdDid", holdDID, "error", err) h.renderError(w, "Failed to resolve hold service") return } diff --git a/pkg/appview/handlers/subscription.go b/pkg/appview/handlers/subscription.go index 9315216..66f145c 100644 --- a/pkg/appview/handlers/subscription.go +++ b/pkg/appview/handlers/subscription.go @@ -77,9 +77,9 @@ func (h *SubscriptionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } // Resolve hold DID to endpoint - holdEndpoint := atproto.ResolveHoldURL(holdDID) - if holdEndpoint == "" { - slog.Warn("Failed to resolve hold endpoint", "holdDid", holdDID) + holdEndpoint, err := atproto.ResolveHoldURL(r.Context(), holdDID) + if err != nil { + slog.Warn("Failed to resolve hold endpoint", "holdDid", holdDID, "error", err) h.renderHidden(w) return } @@ -197,8 +197,9 @@ func (h *SubscriptionCheckoutHandler) ServeHTTP(w http.ResponseWriter, r *http.R } // Resolve hold endpoint - holdEndpoint := atproto.ResolveHoldURL(holdDID) - if holdEndpoint == "" { + holdEndpoint, err := atproto.ResolveHoldURL(r.Context(), holdDID) + if err != nil { + slog.Warn("Failed to resolve hold endpoint", "holdDid", holdDID, "error", err) http.Error(w, "Failed to resolve hold", http.StatusInternalServerError) return } @@ -285,8 +286,9 @@ func (h *SubscriptionPortalHandler) ServeHTTP(w http.ResponseWriter, r *http.Req } // Resolve hold endpoint - holdEndpoint := atproto.ResolveHoldURL(holdDID) - if holdEndpoint == "" { + holdEndpoint, err := atproto.ResolveHoldURL(r.Context(), holdDID) + if err != nil { + slog.Warn("Failed to resolve hold endpoint", "holdDid", holdDID, "error", err) http.Error(w, "Failed to resolve hold", http.StatusInternalServerError) return } diff --git a/pkg/appview/holdhealth/checker.go b/pkg/appview/holdhealth/checker.go index f1ac758..a8616fb 100644 --- a/pkg/appview/holdhealth/checker.go +++ b/pkg/appview/holdhealth/checker.go @@ -51,9 +51,11 @@ func NewCheckerWithTimeout(cacheTTL, httpTimeout time.Duration) *Checker { // Checks {endpoint}/xrpc/_health and returns true if reachable func (c *Checker) CheckHealth(ctx context.Context, endpoint string) (bool, error) { // Convert DID to HTTP URL if needed - // did:web:hold.example.com → https://hold.example.com - // https://hold.example.com → https://hold.example.com (passthrough) - httpURL := atproto.ResolveHoldURL(endpoint) + // Resolves any DID (did:web, did:plc) via identity directory + httpURL, err := atproto.ResolveHoldURL(ctx, endpoint) + if err != nil { + return false, fmt.Errorf("failed to resolve hold URL: %w", err) + } // Build health check URL healthURL := httpURL + "/xrpc/_health" diff --git a/pkg/appview/holdhealth/checker_test.go b/pkg/appview/holdhealth/checker_test.go index 2107a00..23e57c8 100644 --- a/pkg/appview/holdhealth/checker_test.go +++ b/pkg/appview/holdhealth/checker_test.go @@ -65,19 +65,15 @@ func TestCheckHealth_WithDID(t *testing.T) { checker := NewChecker(15 * time.Minute) ctx := context.Background() - // Test with DID format (did:web:host) - // Extract host:port from test server URL - // http://127.0.0.1:12345 → did:web:127.0.0.1:12345 - serverURL := server.URL - didFormat := "did:web:" + serverURL[7:] // Remove "http://" - - reachable, err := checker.CheckHealth(ctx, didFormat) + // Test with URL format (DID resolution requires real identity directory, + // so we test with the URL format which passes through directly) + reachable, err := checker.CheckHealth(ctx, server.URL) if err != nil { - t.Errorf("CheckHealth with DID returned error: %v", err) + t.Errorf("CheckHealth with URL returned error: %v", err) } if !reachable { - t.Error("Expected hold to be reachable with DID format") + t.Error("Expected hold to be reachable with URL format") } } diff --git a/pkg/appview/jetstream/backfill.go b/pkg/appview/jetstream/backfill.go index 60fe268..dfdaa9d 100644 --- a/pkg/appview/jetstream/backfill.go +++ b/pkg/appview/jetstream/backfill.go @@ -396,7 +396,10 @@ func (b *BackfillWorker) queryCaptainRecord(ctx context.Context, holdDID string) } // Resolve hold DID to URL - holdURL := atproto.ResolveHoldURL(holdDID) + holdURL, err := atproto.ResolveHoldURL(ctx, holdDID) + if err != nil { + return fmt.Errorf("failed to resolve hold URL for %s: %w", holdDID, err) + } // Create client for hold's PDS holdClient := atproto.NewClient(holdURL, holdDID, "") @@ -442,7 +445,10 @@ func (b *BackfillWorker) queryCaptainRecord(ctx context.Context, holdDID string) // This is necessary for localhost/private holds that aren't discoverable via the relay func (b *BackfillWorker) queryCrewRecords(ctx context.Context, holdDID string) error { // Resolve hold DID to URL - holdURL := atproto.ResolveHoldURL(holdDID) + holdURL, err := atproto.ResolveHoldURL(ctx, holdDID) + if err != nil { + return fmt.Errorf("failed to resolve hold URL for %s: %w", holdDID, err) + } // Create client for hold's PDS holdClient := atproto.NewClient(holdURL, holdDID, "") diff --git a/pkg/appview/middleware/registry.go b/pkg/appview/middleware/registry.go index 690eb31..115404f 100644 --- a/pkg/appview/middleware/registry.go +++ b/pkg/appview/middleware/registry.go @@ -296,6 +296,13 @@ func (nr *NamespaceResolver) Repository(ctx context.Context, name reference.Name // Single-hop hold migration: check if this hold has declared a successor holdDID = nr.resolveSuccessor(ctx, holdDID) + + // Resolve hold DID to HTTP URL via identity directory (cached 24h) + holdURL, err := atproto.ResolveHoldURL(ctx, holdDID) + if err != nil { + return nil, fmt.Errorf("failed to resolve hold URL for %s: %w", holdDID, err) + } + // Auto-reconcile crew membership on first push/pull // This ensures users can push immediately after docker login without web sign-in // EnsureCrewMembership is best-effort and logs errors without failing the request @@ -463,6 +470,7 @@ func (nr *NamespaceResolver) Repository(ctx context.Context, name reference.Name DID: did, Handle: handle, HoldDID: holdDID, + HoldURL: holdURL, PDSEndpoint: pdsEndpoint, Repository: repositoryName, ServiceToken: serviceToken, // Cached service token from puller's PDS @@ -551,20 +559,16 @@ func (nr *NamespaceResolver) resolveSuccessor(ctx context.Context, holdDID strin // isHoldReachable checks if a hold service is reachable // Used in test mode to fallback to default hold when user's hold is unavailable func (nr *NamespaceResolver) isHoldReachable(ctx context.Context, holdDID string) bool { - // Try to fetch the DID document - hostname := strings.TrimPrefix(holdDID, "did:web:") - - // Try HTTP first (local), then HTTPS - for _, scheme := range []string{"http", "https"} { - testURL := fmt.Sprintf("%s://%s/.well-known/did.json", scheme, hostname) - client := atproto.NewClient("", "", "") - _, err := client.FetchDIDDocument(ctx, testURL) - if err == nil { - return true - } + holdURL, err := atproto.ResolveHoldURL(ctx, holdDID) + if err != nil { + slog.Debug("Cannot resolve hold URL for reachability check", "component", "registry/middleware", "holdDID", holdDID, "error", err) + return false } - return false + testURL := holdURL + "/.well-known/did.json" + client := atproto.NewClient("", "", "") + _, err = client.FetchDIDDocument(ctx, testURL) + return err == nil } // ExtractAuthMethod is an HTTP middleware that extracts the auth method and puller DID from the JWT Authorization header diff --git a/pkg/appview/middleware/registry_test.go b/pkg/appview/middleware/registry_test.go index f9e13c3..f2e6235 100644 --- a/pkg/appview/middleware/registry_test.go +++ b/pkg/appview/middleware/registry_test.go @@ -270,10 +270,8 @@ func TestIsHoldReachable(t *testing.T) { ctx := context.Background() t.Run("reachable hold", func(t *testing.T) { - // Extract hostname from test server URL - // The mock server URL is like http://127.0.0.1:port, so we use the host part - holdDID := fmt.Sprintf("did:web:%s", mockHold.Listener.Addr().String()) - reachable := resolver.isHoldReachable(ctx, holdDID) + // Use URL format directly — DID resolution requires real identity directory + reachable := resolver.isHoldReachable(ctx, mockHold.URL) assert.True(t, reachable, "should detect reachable hold") }) diff --git a/pkg/appview/storage/context.go b/pkg/appview/storage/context.go index e094b48..7df74f8 100644 --- a/pkg/appview/storage/context.go +++ b/pkg/appview/storage/context.go @@ -21,7 +21,8 @@ type RegistryContext struct { // Puller = the authenticated user making the request (from JWT Subject) DID string // Owner's DID - whose repo is being accessed (e.g., "did:plc:abc123") Handle string // Owner's handle (e.g., "alice.bsky.social") - HoldDID string // Hold service DID (e.g., "did:web:hold01.atcr.io") + HoldDID string // Hold service DID (e.g., "did:web:hold01.atcr.io" or "did:plc:abc123") + HoldURL string // Resolved HTTP URL for the hold service PDSEndpoint string // Owner's PDS endpoint URL Repository string // Image repository name (e.g., "debian") ServiceToken string // Service token for hold authentication (from puller's PDS) diff --git a/pkg/appview/storage/crew.go b/pkg/appview/storage/crew.go index 47747f8..65ca7ff 100644 --- a/pkg/appview/storage/crew.go +++ b/pkg/appview/storage/crew.go @@ -30,7 +30,11 @@ func EnsureCrewMembership(ctx context.Context, client *atproto.Client, refresher } // Resolve hold DID to HTTP endpoint - holdEndpoint := atproto.ResolveHoldURL(holdDID) + 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 diff --git a/pkg/appview/storage/manifest_store.go b/pkg/appview/storage/manifest_store.go index 29f4a17..6400c4a 100644 --- a/pkg/appview/storage/manifest_store.go +++ b/pkg/appview/storage/manifest_store.go @@ -337,9 +337,8 @@ func (s *ManifestStore) notifyHoldAboutManifest(ctx context.Context, manifestRec return nil } - // Resolve hold DID to HTTP endpoint - // For did:web, this is straightforward (e.g., did:web:hold01.atcr.io → https://hold01.atcr.io) - holdEndpoint := atproto.ResolveHoldURL(s.ctx.HoldDID) + // Use pre-resolved hold URL from RegistryContext + holdEndpoint := s.ctx.HoldURL // Use service token from middleware (already cached and validated) serviceToken := s.ctx.ServiceToken diff --git a/pkg/appview/storage/proxy_blob_store.go b/pkg/appview/storage/proxy_blob_store.go index e001673..d237642 100644 --- a/pkg/appview/storage/proxy_blob_store.go +++ b/pkg/appview/storage/proxy_blob_store.go @@ -40,8 +40,8 @@ type ProxyBlobStore struct { // NewProxyBlobStore creates a new proxy blob store func NewProxyBlobStore(ctx *RegistryContext) *ProxyBlobStore { - // Resolve DID to URL once at construction time - holdURL := atproto.ResolveHoldURL(ctx.HoldDID) + // Use pre-resolved URL from RegistryContext (resolved in Registry.Repository()) + holdURL := ctx.HoldURL slog.Debug("NewProxyBlobStore created", "component", "proxy_blob_store", "hold_did", ctx.HoldDID, "hold_url", holdURL, "user_did", ctx.DID, "repo", ctx.Repository) diff --git a/pkg/appview/storage/proxy_blob_store_test.go b/pkg/appview/storage/proxy_blob_store_test.go index 2040160..701e312 100644 --- a/pkg/appview/storage/proxy_blob_store_test.go +++ b/pkg/appview/storage/proxy_blob_store_test.go @@ -197,33 +197,37 @@ func TestDoAuthenticatedRequest_ErrorWhenTokenUnavailable(t *testing.T) { } } -// TestResolveHoldURL tests DID to URL conversion +// TestResolveHoldURL tests URL passthrough (no network needed) func TestResolveHoldURL(t *testing.T) { + ctx := context.Background() tests := []struct { name string - holdDID string + holdURL string expected string }{ { - name: "did:web with http (TEST_MODE)", - holdDID: "did:web:localhost:8080", + name: "http URL passthrough", + holdURL: "http://localhost:8080", expected: "http://localhost:8080", }, { - name: "did:web with https (production)", - holdDID: "did:web:hold01.atcr.io", + name: "https URL passthrough", + holdURL: "https://hold01.atcr.io", expected: "https://hold01.atcr.io", }, { - name: "did:web with port", - holdDID: "did:web:hold.example.com:3000", + name: "http URL with port passthrough", + holdURL: "http://hold.example.com:3000", expected: "http://hold.example.com:3000", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := atproto.ResolveHoldURL(tt.holdDID) + result, err := atproto.ResolveHoldURL(ctx, tt.holdURL) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } if result != tt.expected { t.Errorf("Expected %s, got %s", tt.expected, result) } @@ -277,9 +281,11 @@ func TestServiceTokenCacheKeyFormat(t *testing.T) { // TestNewProxyBlobStore tests ProxyBlobStore creation func TestNewProxyBlobStore(t *testing.T) { + expectedURL := "https://hold.example.com" ctx := &RegistryContext{ DID: "did:plc:test", HoldDID: "did:web:hold.example.com", + HoldURL: expectedURL, PDSEndpoint: "https://pds.example.com", Repository: "test-repo", } @@ -298,7 +304,6 @@ func TestNewProxyBlobStore(t *testing.T) { t.Error("Expected holdURL to be set") } - expectedURL := "https://hold.example.com" if store.holdURL != expectedURL { t.Errorf("Expected holdURL %s, got %s", expectedURL, store.holdURL) } diff --git a/pkg/atproto/resolver.go b/pkg/atproto/resolver.go index 3967ba7..a8d4e77 100644 --- a/pkg/atproto/resolver.go +++ b/pkg/atproto/resolver.go @@ -8,34 +8,56 @@ import ( "github.com/bluesky-social/indigo/atproto/syntax" ) -// ResolveHoldURL converts a hold identifier (DID or URL) to an HTTP/HTTPS URL -// Handles both formats for backward compatibility: -// - DID format: did:web:hold01.atcr.io → https://hold01.atcr.io -// - DID with port: did:web:172.28.0.3:8080 → http://172.28.0.3:8080 -// - URL format: https://hold.example.com → https://hold.example.com (passthrough) -func ResolveHoldURL(holdIdentifier string) string { +// ResolveHoldURL converts a hold identifier (DID or URL) to an HTTP/HTTPS URL. +// For DIDs (both did:web and did:plc), resolves via the indigo identity directory +// which caches results (24h TTL). Prefers the #atcr_hold service endpoint, +// falls back to #atproto_pds. +// +// Supported formats: +// - URL: https://hold.example.com → passthrough +// - DID: did:web:hold01.atcr.io → resolved via /.well-known/did.json +// - DID: did:plc:abc123 → resolved via PLC directory +func ResolveHoldURL(ctx context.Context, holdIdentifier string) (string, error) { // If it's already a URL (has scheme), return as-is if strings.HasPrefix(holdIdentifier, "http://") || strings.HasPrefix(holdIdentifier, "https://") { - return holdIdentifier + return holdIdentifier, nil } - // If it's a DID, convert to URL - if after, ok := strings.CutPrefix(holdIdentifier, "did:web:"); ok { - hostname := after - - // Use HTTP for localhost/IP addresses with ports, HTTPS for domains - if strings.Contains(hostname, ":") || - strings.Contains(hostname, "127.0.0.1") || - strings.Contains(hostname, "localhost") || - // Check if it's an IP address (contains only digits and dots in first part) - (len(hostname) > 0 && hostname[0] >= '0' && hostname[0] <= '9') { - return "http://" + hostname - } - return "https://" + hostname + // If it's a DID, resolve via identity directory + if strings.HasPrefix(holdIdentifier, "did:") { + return ResolveHoldDIDToURL(ctx, holdIdentifier) } // Fallback: assume it's a hostname and use HTTPS - return "https://" + holdIdentifier + return "https://" + holdIdentifier, nil +} + +// ResolveHoldDIDToURL resolves a hold DID to its HTTP service endpoint. +// Prefers the #atcr_hold service endpoint, falls back to #atproto_pds. +// Uses the shared identity directory with cache TTL and event-driven invalidation. +func ResolveHoldDIDToURL(ctx context.Context, did string) (string, error) { + directory := GetDirectory() + didParsed, err := syntax.ParseDID(did) + if err != nil { + return "", fmt.Errorf("invalid hold DID %q: %w", did, err) + } + + ident, err := directory.LookupDID(ctx, didParsed) + if err != nil { + return "", fmt.Errorf("failed to resolve hold DID %s: %w", did, err) + } + + // Prefer #atcr_hold service (hold-specific endpoint) + if url := ident.GetServiceEndpoint("atcr_hold"); url != "" { + return url, nil + } + + // Fall back to #atproto_pds (hold publishes both with same URL) + if url := ident.PDSEndpoint(); url != "" { + return url, nil + } + + return "", fmt.Errorf("no hold or PDS service endpoint found for DID %s", did) } // ResolveDIDToPDS resolves a DID to its PDS endpoint. diff --git a/pkg/atproto/resolver_test.go b/pkg/atproto/resolver_test.go index fd74e0d..7bbef4e 100644 --- a/pkg/atproto/resolver_test.go +++ b/pkg/atproto/resolver_test.go @@ -7,6 +7,9 @@ import ( ) func TestResolveHoldURL(t *testing.T) { + ctx := context.Background() + + // URL passthrough and hostname fallback tests (no network needed) tests := []struct { name string holdIdentifier string @@ -39,77 +42,7 @@ func TestResolveHoldURL(t *testing.T) { want: "http://hold.example.com/some/path", }, - // did:web to HTTPS (domain names) - { - name: "did:web domain to https", - holdIdentifier: "did:web:hold01.atcr.io", - want: "https://hold01.atcr.io", - }, - { - name: "did:web subdomain to https", - holdIdentifier: "did:web:my-hold.example.com", - want: "https://my-hold.example.com", - }, - { - name: "did:web simple domain to https", - holdIdentifier: "did:web:example.com", - want: "https://example.com", - }, - - // did:web to HTTP (ports) - { - name: "did:web with port to http", - holdIdentifier: "did:web:172.28.0.3:8080", - want: "http://172.28.0.3:8080", - }, - { - name: "did:web domain with port to http", - holdIdentifier: "did:web:hold.example.com:8080", - want: "http://hold.example.com:8080", - }, - { - name: "did:web localhost with port to http", - holdIdentifier: "did:web:localhost:8080", - want: "http://localhost:8080", - }, - - // did:web to HTTP (localhost) - { - name: "did:web localhost to http", - holdIdentifier: "did:web:localhost", - want: "http://localhost", - }, - - // did:web to HTTP (127.0.0.1) - { - name: "did:web 127.0.0.1 to http", - holdIdentifier: "did:web:127.0.0.1", - want: "http://127.0.0.1", - }, - { - name: "did:web 127.0.0.1 with port to http", - holdIdentifier: "did:web:127.0.0.1:8080", - want: "http://127.0.0.1:8080", - }, - - // did:web to HTTP (IP addresses) - { - name: "did:web IPv4 address to http", - holdIdentifier: "did:web:192.168.1.1", - want: "http://192.168.1.1", - }, - { - name: "did:web IPv4 with port to http", - holdIdentifier: "did:web:10.0.0.5:3000", - want: "http://10.0.0.5:3000", - }, - { - name: "did:web private IP to http", - holdIdentifier: "did:web:172.16.0.1", - want: "http://172.16.0.1", - }, - - // Fallback behavior (plain hostname) + // Fallback behavior (plain hostname — not a DID, not a URL) { name: "plain hostname fallback to https", holdIdentifier: "hold.example.com", @@ -127,21 +60,14 @@ func TestResolveHoldURL(t *testing.T) { holdIdentifier: "", want: "https://", }, - { - name: "did:web empty hostname", - holdIdentifier: "did:web:", - want: "https://", - }, - { - name: "just did:web prefix", - holdIdentifier: "did:web", - want: "https://did:web", - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := ResolveHoldURL(tt.holdIdentifier) + got, err := ResolveHoldURL(ctx, tt.holdIdentifier) + if err != nil { + t.Errorf("ResolveHoldURL(%q) unexpected error: %v", tt.holdIdentifier, err) + } if got != tt.want { t.Errorf("ResolveHoldURL(%q) = %q, want %q", tt.holdIdentifier, got, tt.want) } @@ -149,25 +75,58 @@ func TestResolveHoldURL(t *testing.T) { } } -// TestResolveHoldURLRoundTrip tests that converting back and forth works +// TestResolveHoldURLDIDRequiresNetwork tests that DID resolution requires +// the identity directory (which needs network access) +func TestResolveHoldURLDIDRequiresNetwork(t *testing.T) { + ctx := context.Background() + + // did:web and did:plc both go through the identity directory now. + // Without a real server, these should return errors. + tests := []struct { + name string + holdIdentifier string + }{ + {"did:web nonexistent", "did:web:nonexistent.example.invalid"}, + {"did:plc nonexistent", "did:plc:nonexistent000000000000"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := ResolveHoldURL(ctx, tt.holdIdentifier) + if err == nil { + t.Errorf("ResolveHoldURL(%q) expected error for unresolvable DID, got nil", tt.holdIdentifier) + } + }) + } +} + +// TestResolveHoldURLRoundTrip tests that URL passthrough is idempotent func TestResolveHoldURLRoundTrip(t *testing.T) { + ctx := context.Background() + tests := []struct { name string input string wantHTTP bool // true if result should be http, false for https }{ - {"domain to https and idempotent", "did:web:hold.atcr.io", false}, - {"IP to http and idempotent", "did:web:192.168.1.1", true}, - {"port to http and idempotent", "did:web:example.com:8080", true}, + {"http URL idempotent", "http://192.168.1.1", true}, + {"https URL idempotent", "https://hold.atcr.io", false}, + {"http URL with port idempotent", "http://example.com:8080", true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // First conversion - first := ResolveHoldURL(tt.input) + // First conversion (URL passthrough) + first, err := ResolveHoldURL(ctx, tt.input) + if err != nil { + t.Fatalf("First ResolveHoldURL(%q) error: %v", tt.input, err) + } // Second conversion (should be idempotent since output is URL) - second := ResolveHoldURL(first) + second, err := ResolveHoldURL(ctx, first) + if err != nil { + t.Fatalf("Second ResolveHoldURL(%q) error: %v", first, err) + } if first != second { t.Errorf("ResolveHoldURL is not idempotent: first=%q, second=%q", first, second) diff --git a/pkg/auth/hold_remote.go b/pkg/auth/hold_remote.go index f7bed3d..b209315 100644 --- a/pkg/auth/hold_remote.go +++ b/pkg/auth/hold_remote.go @@ -222,7 +222,10 @@ func (a *RemoteHoldAuthorizer) setCachedCaptainRecord(holdDID string, record *at // fetchCaptainRecordFromXRPC queries the hold's XRPC endpoint for captain record func (a *RemoteHoldAuthorizer) fetchCaptainRecordFromXRPC(ctx context.Context, holdDID string) (*atproto.CaptainRecord, error) { // Resolve DID to URL - holdURL := atproto.ResolveHoldURL(holdDID) + holdURL, err := atproto.ResolveHoldURL(ctx, holdDID) + if err != nil { + return nil, fmt.Errorf("failed to resolve hold URL: %w", err) + } // Build XRPC request URL // GET /xrpc/com.atproto.repo.getRecord?repo={did}&collection=io.atcr.hold.captain&rkey=self @@ -327,7 +330,10 @@ func (a *RemoteHoldAuthorizer) IsCrewMember(ctx context.Context, holdDID, userDI // Uses O(1) lookup via getRecord with hash-based rkey instead of pagination func (a *RemoteHoldAuthorizer) isCrewMemberNoCache(ctx context.Context, holdDID, userDID string) (bool, error) { // Resolve DID to URL - holdURL := atproto.ResolveHoldURL(holdDID) + holdURL, err := atproto.ResolveHoldURL(ctx, holdDID) + if err != nil { + return false, fmt.Errorf("failed to resolve hold URL: %w", err) + } // Generate deterministic rkey from member DID (hash-based) rkey := atproto.CrewRecordKey(userDID) -- 2.51.2