remove user oauth flow. hold now contains captain record indicating owner

2026-09-20 01:04:15 +00:00 · 2025-10-15 14:47:53 -05:00
parent a271d3d8e3
commit fade86abaa
12 changed files with 1087 additions and 562 deletions
@@ -146,7 +146,8 @@ Hold Service = Minimal PDS (did:web:hold1.example.com)
 │   ├── io.atcr.hold.exportImage (data portability)
 │   └── io.atcr.hold.getStats (metadata)
 └── Records (hold's own PDS):
-    ├── io.atcr.hold.crew (crew membership)
+    ├── io.atcr.hold.captain (single record: ownership & metadata)
+    ├── io.atcr.hold.crew/* (crew membership & permissions)
    └── io.atcr.hold.config (hold configuration)
 ```

@@ -270,8 +271,20 @@ Storage continues to use distribution's existing S3 layout. The PDS interface is

 **This is standard ATProto federation** - services pass OAuth tokens with DPoP proofs between each other. Hold independently validates tokens against the user's PDS, so there's no trust relationship required.

-**Crew records stored in hold's PDS:**
+**Records stored in hold's PDS:**
+
 ```json
+// io.atcr.hold.captain (single record - hold metadata)
+{
+  "$type": "io.atcr.hold.captain",
+  "owner": "did:plc:alice123",
+  "public": false,
+  "deployedAt": "2025-10-14T...",
+  "region": "iad",
+  "provider": "fly.io"
+}
+
+// io.atcr.hold.crew/* (access control records)
 {
  "$type": "io.atcr.hold.crew",
  "member": "did:plc:alice123",
@@ -281,6 +294,10 @@ Storage continues to use distribution's existing S3 layout. The PDS interface is
 }
 ```

+**Semantic separation:**
+- **Captain record** = Hold ownership and metadata (who owns it, where it's deployed)
+- **Crew records** = Access control (who can use it, what permissions they have)
+
 **Security considerations:**
 - User's OAuth token is exposed to hold during delegation
 - However, hold independently validates it (can't be forged)
@@ -467,14 +484,85 @@ Can we build a reusable "static PDS" library for apps like ATCR, tangled.org, st

 ### 6. Hold Discovery & Registration

-**Current:** Hold registers by creating records in owner's PDS
-**New:** Hold is its own identity - how does AppView discover available holds?
+**Decision: No registration records needed in owner's PDS.**

-Possibilities:
- Holds publish to feeds
- AppView maintains directory
- DIDs are manually configured
- ATProto directory service
+Since holds are ATProto actors with did:web identity, they are self-describing:
+
+**Hold's PDS contains everything:**
+```
+did:web:hold01.atcr.io
+├── io.atcr.hold.captain → { owner: "did:plc:alice123", ... }
+└── io.atcr.hold.crew/*  → Access control records
+```
+
+**DID Document with Multiple Services:**
+
+Holds expose multiple service endpoints to distinguish themselves from generic PDSs:
+
+```json
+{
+  "@context": ["https://www.w3.org/ns/did/v1", ...],
+  "id": "did:web:hold01.atcr.io",
+  "service": [
+    {
+      "id": "#atproto_pds",
+      "type": "AtprotoPersonalDataServer",
+      "serviceEndpoint": "https://hold01.atcr.io"
+    },
+    {
+      "id": "#atcr_hold",
+      "type": "AtcrHoldService",
+      "serviceEndpoint": "https://hold01.atcr.io"
+    }
+  ]
+}
+```
+
+**Service semantics:**
+- **`#atproto_pds`** - Standard ATProto PDS operations (crew queries, record sync)
+- **`#atcr_hold`** - ATCR-specific operations (blob storage, presigned URLs)
+
+**Discovery patterns:**
+
+1. **Direct deployment** - Owner deploys hold, knows the DID
+2. **Sailor profiles** - Users reference holds by DID in their profile
+3. **DID resolution** - `did:web:hold01.atcr.io` → `https://hold01.atcr.io/.well-known/did.json`
+4. **Service lookup** - Check for `#atcr_hold` service to identify ATCR holds
+5. **Crew queries** - AppView queries hold's PDS directly via `#atproto_pds` endpoint
+
+**AppView resolution flow:**
+```go
+// 1. Get hold DID from sailor profile
+holdDID := profile.DefaultHold  // "did:web:hold01.atcr.io"
+
+// 2. Resolve DID document
+didDoc := resolveDidWeb(holdDID)
+
+// 3. Extract service endpoints
+pdsEndpoint := didDoc.GetService("#atproto_pds")  // XRPC operations
+holdEndpoint := didDoc.GetService("#atcr_hold")   // Blob operations
+
+// 4. Query crew list via PDS endpoint
+crew := xrpcClient.ListRecords(pdsEndpoint, "io.atcr.hold.crew")
+
+// 5. Check if user has access
+hasAccess := crew.Contains(userDID)
+```
+
+**No need for reverse lookup** (owner → holds). Users know their holds because they deployed them.
+
+**Benefits:**
+- ✅ Single source of truth (hold's PDS)
+- ✅ No cross-PDS writes during registration
+- ✅ Self-describing ATProto actors
+- ✅ Standard DID resolution patterns
+- ✅ Clear service semantics (PDS vs ATCR-specific)
+- ✅ Discoverable via service type
+
+**OAuth implications:**
+- OAuth registration flow no longer needed (hold is self-describing)
+- OAuth code kept for backward compatibility with legacy registration records
+- Future: Remove OAuth after migration period

 ### 7. Multi-Tenancy

@@ -603,12 +691,22 @@ We're building the minimal PDS needed for discoverability, not a full social cli

 **Key insight:** Other ATProto services will "just work" as long as they can retrieve records from the hold's PDS. We don't need to implement full social features for the hold to participate in the ecosystem.

-### Crew Management: Individual Records
+### Crew Management: Captain + Individual Records

-**Decision: Individual crew record per user (remove wildcard logic)**
+**Decision: Captain record (ownership) + Individual crew records (access control)**

 ```json
-// io.atcr.hold.crew/{rkey}
+// io.atcr.hold.captain (single record - hold metadata)
+{
+  "$type": "io.atcr.hold.captain",
+  "owner": "did:plc:alice123",
+  "public": false,
+  "deployedAt": "2025-10-14T...",
+  "region": "iad",
+  "provider": "fly.io"
+}
+
+// io.atcr.hold.crew/{rkey} (access control)
 {
  "$type": "io.atcr.hold.crew",
  "member": "did:plc:alice123",
@@ -617,7 +715,7 @@ We're building the minimal PDS needed for discoverability, not a full social cli
  "addedAt": "2025-10-14T..."
 }

-// io.atcr.hold.config/policy
+// io.atcr.hold.config/policy (optional)
 {
  "$type": "io.atcr.hold.config",
  "access": "public",     // or "allowlist"
@@ -627,6 +725,11 @@ We're building the minimal PDS needed for discoverability, not a full social cli
 }
 ```

+**Semantic separation:**
+- **Captain record** = Who owns/deployed the hold (billing, deletion, migration rights)
+- **Crew records** = Who can use the hold (access control, permissions)
+- **Config record** = Hold-wide policies
+
 **Authorization logic:**
 ```go
 func (p *HoldPDS) CheckAccess(ctx context.Context, userDID string) (bool, error) {
@@ -661,13 +764,159 @@ func (p *HoldPDS) CheckAccess(ctx context.Context, userDID string) (bool, error)
 - **Private team hold:** `access: "allowlist"` - explicit crew membership
 - **Hybrid:** Public access + explicit admin crew records for elevated permissions

+### Phase 2: XRPC Endpoints Implementation ✅ COMPLETED
+
+**Critical Implementation Lessons Learned:**
+
+#### 1. Custom Record Types Require Manual CBOR Decoding
+
+Indigo's `repo.GetRecord()` uses its lexicon decoder which only knows about built-in ATProto types. For custom types, you must use `GetRecordBytes()` and decode manually:
+
+```go
+// ❌ WRONG - Fails with "unrecognized lexicon type"
+record, err := repo.GetRecord(ctx, path, &CrewRecord{})
+
+// ✅ CORRECT - Manual CBOR decoding
+recordCID, recBytes, err := repo.GetRecordBytes(ctx, path)
+var crewRecord CrewRecord
+err = crewRecord.UnmarshalCBOR(bytes.NewReader(*recBytes))
+```
+
+**Why:** Indigo's lexicon system doesn't know about `io.atcr.hold.crew` or other custom types.
+
+#### 2. JSON Struct Tags Must Match CBOR Tags Exactly
+
+For CID verification to work, JSON and CBOR encodings must produce identical bytes:
+
+```go
+// ❌ WRONG - JSON uses capital field names (Member, Role)
+type CrewRecord struct {
+    Type        string   `cborgen:"$type"`
+    Member      string   `cborgen:"member"`
+    Role        string   `cborgen:"role"`
+    Permissions []string `cborgen:"permissions"`
+    AddedAt     string   `cborgen:"addedAt"`
+}
+
+// ✅ CORRECT - JSON tags match CBOR tags
+type CrewRecord struct {
+    Type        string   `json:"$type" cborgen:"$type"`
+    Member      string   `json:"member" cborgen:"member"`
+    Role        string   `json:"role" cborgen:"role"`
+    Permissions []string `json:"permissions" cborgen:"permissions"`
+    AddedAt     string   `json:"addedAt" cborgen:"addedAt"`
+}
+```
+
+**Why:** Verification code CBOR-encodes the JSON record and compares the CID. Mismatched field names produce different bytes and thus different CIDs.
+
+#### 3. MST ForEach Returns Full Paths
+
+The `repo.ForEach()` callback receives full collection paths, not just record keys:
+
+```go
+// ❌ WRONG - Prepends collection prefix again
+err := repo.ForEach(ctx, "io.atcr.hold.crew", func(k string, v cid.Cid) error {
+    // k is already "io.atcr.hold.crew/3m37dr2ddit22"
+    path := fmt.Sprintf("%s/%s", collection, k) // Double path!
+    return nil
+})
+
+// ✅ CORRECT - Extract just the rkey
+err := repo.ForEach(ctx, "io.atcr.hold.crew", func(k string, v cid.Cid) error {
+    // k = "io.atcr.hold.crew/3m37dr2ddit22"
+    parts := strings.Split(k, "/")
+    rkey := parts[len(parts)-1] // "3m37dr2ddit22"
+    return nil
+})
+```
+
+#### 4. All Record Endpoints Must Return CIDs
+
+Per ATProto spec, `com.atproto.repo.getRecord` and `listRecords` must include the record's CID:
+
+```go
+// ✅ CORRECT - Include CID in response
+response := map[string]any{
+    "uri":   fmt.Sprintf("at://%s/%s/%s", did, collection, rkey),
+    "cid":   recordCID.String(),  // Required!
+    "value": record,
+}
+```
+
+**Why:** Clients need the CID to verify record integrity via `com.atproto.sync.getRecord`.
+
+#### 5. sync.getRecord CAR Files Must Include Full MST Path
+
+The `com.atproto.sync.getRecord` endpoint must return a CAR file with ALL blocks needed to verify the record:
+
+```go
+// ❌ WRONG - Only includes the record block
+blk, _ := repo.Blockstore().Get(ctx, recordCID)
+// Write single block to CAR
+
+// ✅ CORRECT - Capture all accessed blocks
+loggingBS := util.NewLoggingBstore(session)
+tempRepo, _ := repo.OpenRepo(ctx, loggingBS, repoHead)
+_, _, _ = tempRepo.GetRecordBytes(ctx, path)
+blocks := loggingBS.GetLoggedBlocks()  // Commit + MST nodes + record
+// Write all blocks to CAR
+```
+
+**Components included:**
+1. **Commit block** - Repo head with signature, data root, version
+2. **MST tree nodes** - Path from root to record (log N depth)
+3. **Record block** - The actual record data
+
+**Why:** Clients need the full Merkle path to cryptographically verify the record against the repo head.
+
+#### 6. CAR Root Must Be Repo Head, Not Record CID
+
+The CAR file's root CID must be the repo head (commit), not the record:
+
+```go
+// ❌ WRONG - Uses record CID as root
+header := &car.CarHeader{
+    Roots:   []cid.Cid{recordCID},
+    Version: 1,
+}
+
+// ✅ CORRECT - Uses repo head as root
+repoHead, _ := carstore.GetUserRepoHead(ctx, uid)
+header := &car.CarHeader{
+    Roots:   []cid.Cid{repoHead},  // Commit CID
+    Version: 1,
+}
+```
+
+**Why:** The CAR represents a slice of the repo from head to record, not just the record itself.
+
+#### 7. Empty Collections Should Return Empty Arrays
+
+Handle empty collections gracefully instead of returning errors:
+
+```go
+// ✅ CORRECT - Return empty array for missing collection
+err := repo.ForEach(ctx, collection, func(k string, v cid.Cid) error {
+    // ...
+})
+if err != nil {
+    if err.Error() == "mst: not found" {
+        return []*CrewMemberWithKey{}, nil  // Empty collection
+    }
+    return nil, err  // Real error
+}
+```
+
+**Why:** ATProto expects empty arrays for non-existent collections, not 404 errors.
+
 ### Next Steps

-1. **Add indigo dependencies** - carstore, repo, MST
-2. **Implement HoldPDS with carstore** - Create pkg/hold/pds
-3. **Add crew management** - CRUD operations for crew records
-4. **Implement standard PDS endpoints** - describeServer, describeRepo, getRecord, listRecords
-5. **Add DID document** - did:web identity generation
+1. ~~**Add indigo dependencies**~~ ✅
+2. ~~**Implement HoldPDS with carstore**~~ ✅
+3. ~~**Add crew management**~~ ✅
+4. ~~**Implement standard PDS endpoints**~~ ✅
+5. ~~**Add DID document**~~ ✅
 6. **Custom XRPC methods** - getUploadUrl, getDownloadUrl (presigned URLs)
 7. **Wire up in cmd/hold** - Serve XRPC alongside existing HTTP
 8. **Test basic operations** - Add/list crew, policy checks