Enforce TeamMode store scopes and transaction lease checks
7 files changed,
+304
−92
internal/docs/grants.go+25 −36internal/docs/links.go+2 −2internal/docs/snapshots.go+4 −1internal/docs/store.go+33 −49internal/docs/team_access.go+45 −1internal/docs/team_access_test.go+192 −2internal/docs/team_identity_test.go+3 −1
CI passed — run details
diff --git a/internal/docs/grants.go b/internal/docs/grants.go| index 286dae4..fca5936 100644 |
| --- a/internal/docs/grants.go |
| +++ b/internal/docs/grants.go |
| @@ -1,6 +1,7 @@ |
| package docs |
| import ( |
| + "amadan.net/oficina/home/roster" |
| "context" |
| "errors" |
| "fmt" |
| @@ -85,18 +86,7 @@ func localActor(id int64, address string) Actor { |
| // absent from this query on purpose, and that absence is the role's whole |
| // meaning. |
| func writable(g *gorm.DB, a Actor) *gorm.DB { |
| - if a.PersonID <= 0 { |
| - return g.Where("1 = 0") |
| - } |
| - if a.VerifiedAddress == nil { |
| - return g.Where("documents.owner_id > 0 AND documents.owner_id = ?", a.PersonID) |
| - } |
| - return g.Where( |
| - "documents.owner_id > 0 AND (documents.owner_id = ? OR EXISTS ("+ |
| - "SELECT 1 FROM grants WHERE grants.doc_id = documents.id "+ |
| - "AND grants.subject = ? AND grants.role = ?))", |
| - a.PersonID, subjectOf(*a.VerifiedAddress), RoleWriter, |
| - ) |
| + return (&Store{}).permissionScope(context.Background(), g, a, permissionWrite) |
| } |
| // subjectOf normalises an address the way EnsurePerson stores one, so a grant |
| @@ -160,18 +150,7 @@ func looksLikeAnAddress(subject string) bool { |
| // tests pin that 0 matches nothing; a grants clause must not become the way |
| // around them. |
| func readable(g *gorm.DB, a Actor) *gorm.DB { |
| - owner := a.PersonID |
| - if owner <= 0 { |
| - return g.Where("1 = 0") |
| - } |
| - if a.VerifiedAddress == nil { |
| - return g.Where("documents.owner_id > 0 AND documents.owner_id = ?", a.PersonID) |
| - } |
| - return g.Where( |
| - "documents.owner_id > 0 AND (documents.owner_id = ? OR EXISTS ("+ |
| - "SELECT 1 FROM grants WHERE grants.doc_id = documents.id AND grants.subject = ?))", |
| - owner, subjectOf(*a.VerifiedAddress), |
| - ) |
| + return (&Store{}).permissionScope(context.Background(), g, a, permissionRead) |
| } |
| // Grant gives an address access to a document. Only the owner may call it: |
| @@ -216,10 +195,12 @@ func (s *Store) Grant(ctx context.Context, owner int64, sqid, address string, ro |
| CreatedBy: owner, |
| CreatedAt: time.Now().UTC(), |
| } |
| - return s.db.WithContext(ctx).Clauses(clause.OnConflict{ |
| - Columns: []clause.Column{{Name: "doc_id"}, {Name: "subject"}}, |
| - DoUpdates: clause.AssignmentColumns([]string{"role", "created_by"}), |
| - }).Create(&grant).Error |
| + return s.ownerMutation(ctx, owner, doc.ID, func(tx *gorm.DB) error { |
| + return tx.Clauses(clause.OnConflict{ |
| + Columns: []clause.Column{{Name: "doc_id"}, {Name: "subject"}}, |
| + DoUpdates: clause.AssignmentColumns([]string{"role", "created_by"}), |
| + }).Create(&grant).Error |
| + }) |
| } |
| // HasGrant reports whether an address holds a grant on any document here. |
| @@ -289,9 +270,11 @@ func (s *Store) Revoke(ctx context.Context, owner int64, sqid, address string) e |
| if err != nil { |
| return err |
| } |
| - return s.db.WithContext(ctx). |
| - Where("doc_id = ? AND subject = ?", doc.ID, subjectOf(address)). |
| - Delete(&Grant{}).Error |
| + return s.ownerMutation(ctx, owner, doc.ID, func(tx *gorm.DB) error { |
| + return tx. |
| + Where("doc_id = ? AND subject = ?", doc.ID, subjectOf(address)). |
| + Delete(&Grant{}).Error |
| + }) |
| } |
| // GetFor fetches a document the acting person may read: theirs, or one they |
| @@ -307,11 +290,11 @@ func (s *Store) Revoke(ctx context.Context, owner int64, sqid, address string) e |
| // as one that does not exist, and the handler turns both into 404 (§4.2). |
| func (s *Store) GetFor(ctx context.Context, a Actor, sqid string) (*Document, error) { |
| var doc Document |
| - err := readable(s.db.WithContext(ctx), a). |
| + err := s.permissionScope(ctx, s.db.WithContext(ctx), a, permissionRead). |
| Where("documents.sqid = ? AND documents.deleted_at IS NULL", sqid). |
| First(&doc).Error |
| if err != nil { |
| - return nil, err |
| + return nil, s.permissionError(ctx, a, err) |
| } |
| return &doc, nil |
| } |
| @@ -329,7 +312,7 @@ func (s *Store) GetFor(ctx context.Context, a Actor, sqid string) (*Document, er |
| // may not costs them their work. |
| func (s *Store) CanWrite(ctx context.Context, a Actor, docID int64) bool { |
| var count int64 |
| - if err := writable(s.db.WithContext(ctx).Model(&Document{}), a). |
| + if err := s.permissionScope(ctx, s.db.WithContext(ctx).Model(&Document{}), a, permissionWrite). |
| Where("documents.id = ?", docID). |
| Count(&count).Error; err != nil { |
| s.logger.Error("docs: checking write access", "doc", docID, "err", err) |
| @@ -354,12 +337,18 @@ func (s *Store) CanWrite(ctx context.Context, a Actor, docID int64) bool { |
| // docRow.SharedNote, without reordering the list. |
| func (s *Store) ListFor(ctx context.Context, a Actor) ([]*Document, error) { |
| var docs []*Document |
| - err := readable(s.db.WithContext(ctx), a). |
| + err := s.permissionScope(ctx, s.db.WithContext(ctx), a, permissionRead). |
| Where("documents.deleted_at IS NULL"). |
| Order("documents.updated_at DESC, documents.id DESC"). |
| Find(&docs).Error |
| if err != nil { |
| - return nil, fmt.Errorf("docs: list readable documents: %w", err) |
| + return nil, s.permissionError(ctx, a, fmt.Errorf("docs: list readable documents: %w", err)) |
| + } |
| + if s.team != nil && len(docs) == 0 { |
| + a, ok := s.access(ctx, a) |
| + if !ok || a.state() == roster.Unknown { |
| + return nil, errTeamUnavailable |
| + } |
| } |
| return docs, nil |
| } |
diff --git a/internal/docs/links.go b/internal/docs/links.go| index d6ef71f..5fb03ce 100644 |
| --- a/internal/docs/links.go |
| +++ b/internal/docs/links.go |
| @@ -99,7 +99,7 @@ func (s *Store) Publish(ctx context.Context, owner int64, sqid string) (string, |
| // replaced are the same outcome only if nothing else ever holds a |
| // reference to the row. Nothing does, and this way the intent reads |
| // off the code. |
| - err = s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { |
| + err = s.ownerMutation(ctx, owner, doc.ID, func(tx *gorm.DB) error { |
| if err := tx.Where("doc_id = ?", doc.ID).Delete(&Link{}).Error; err != nil { |
| return err |
| } |
| @@ -121,7 +121,7 @@ func (s *Store) Unpublish(ctx context.Context, owner int64, sqid string) error { |
| if err != nil { |
| return err |
| } |
| - return s.db.WithContext(ctx).Where("doc_id = ?", doc.ID).Delete(&Link{}).Error |
| + return s.ownerMutation(ctx, owner, doc.ID, func(tx *gorm.DB) error { return tx.Where("doc_id = ?", doc.ID).Delete(&Link{}).Error }) |
| } |
| // IsPublished reports whether a document currently has a public link. |
diff --git a/internal/docs/snapshots.go b/internal/docs/snapshots.go| index 3ffe465..38d0bb5 100644 |
| --- a/internal/docs/snapshots.go |
| +++ b/internal/docs/snapshots.go |
| @@ -109,6 +109,9 @@ func (s *Store) writeSnapshot(ctx context.Context, doc *Document, h *core.Handle |
| CreatedAt: time.Now().UTC(), |
| } |
| return s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { |
| + if err := s.snapshotPermission(ctx, tx, doc.ID); err != nil { |
| + return err |
| + } |
| if err := s.prepareContentSnapshot(tx, doc.GUID, row); err != nil { |
| return err |
| } |
| @@ -148,6 +151,6 @@ func (s *Store) writeSnapshot(ctx context.Context, doc *Document, h *core.Handle |
| Delete(&DocSnapshot{}).Error; err != nil { |
| return fmt.Errorf("docs: delete superseded snapshots for document %d at seq %d: %w", doc.ID, seq, err) |
| } |
| - return nil |
| + return s.snapshotPermission(ctx, tx, doc.ID) |
| }) |
| } |
diff --git a/internal/docs/store.go b/internal/docs/store.go| index c283b89..47d4e8e 100644 |
| --- a/internal/docs/store.go |
| +++ b/internal/docs/store.go |
| @@ -255,7 +255,7 @@ func (s *Store) EnsurePerson(ctx context.Context, email string) (int64, error) { |
| // yet; the first is this feature having broken sign-in, which is the |
| // exact failure this whole plan exists to fix, not reintroduce. |
| func (s *Store) seedIfEnabled(ctx context.Context, newOwner int64, email string) { |
| - if !s.sampleDocuments { |
| + if s.team != nil || !s.sampleDocuments { |
| return |
| } |
| if s.seedOnlyMembers != nil && !s.seedOnlyMembers(email) { |
| @@ -279,6 +279,10 @@ func (s *Store) seedIfEnabled(ctx context.Context, newOwner int64, email string) |
| // SKILL.md §3: "Creating takes the owner from the session, never the |
| // form." |
| func (s *Store) Create(ctx context.Context, owner int64, title string) (*Document, error) { |
| + actor := s.ownerActor(ctx, owner) |
| + if err := s.requireMember(ctx, actor); err != nil { |
| + return nil, err |
| + } |
| guid := uuid.NewString() |
| // math/rand/v2's Int64 returns a non-negative value in [0, 1<<63), |
| // so converting to uint64 never wraps — the brief's "a counter or |
| @@ -301,7 +305,15 @@ func (s *Store) Create(ctx context.Context, owner int64, title string) (*Documen |
| CreatedAt: now, |
| UpdatedAt: now, |
| } |
| - err = s.db.WithContext(ctx).Create(doc).Error |
| + err = s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { |
| + if err := s.requireMember(ctx, actor); err != nil { |
| + return err |
| + } |
| + if err := tx.Create(doc).Error; err != nil { |
| + return err |
| + } |
| + return s.requireMember(ctx, actor) |
| + }) |
| if err == nil { |
| return doc, nil |
| } |
| @@ -328,7 +340,7 @@ func (s *Store) Create(ctx context.Context, owner int64, title string) (*Documen |
| // above. |
| func (s *Store) List(ctx context.Context, owner int64) ([]*Document, error) { |
| var docs []*Document |
| - err := owned(s.db.WithContext(ctx), owner). |
| + err := s.permissionScope(ctx, s.db.WithContext(ctx), s.ownerActor(ctx, owner), permissionOwner). |
| Where("deleted_at IS NULL"). |
| Order("updated_at DESC, id DESC"). |
| Find(&docs).Error |
| @@ -346,11 +358,11 @@ func (s *Store) List(ctx context.Context, owner int64) ([]*Document, error) { |
| // document (spec §4.2). |
| func (s *Store) Get(ctx context.Context, owner int64, sqid string) (*Document, error) { |
| var doc Document |
| - err := owned(s.db.WithContext(ctx), owner). |
| + err := s.permissionScope(ctx, s.db.WithContext(ctx), s.ownerActor(ctx, owner), permissionOwner). |
| Where("sqid = ? AND deleted_at IS NULL", sqid). |
| First(&doc).Error |
| if err != nil { |
| - return nil, fmt.Errorf("docs: get document %q: %w", sqid, err) |
| + return nil, s.permissionError(ctx, s.ownerActor(ctx, owner), fmt.Errorf("docs: get document %q: %w", sqid, err)) |
| } |
| return &doc, nil |
| } |
| @@ -753,14 +765,8 @@ func (s *Store) ApplyUpdate(ctx context.Context, doc *Document, actor Actor, upd |
| // Reader and stranger are refused identically. The write path does not |
| // tell somebody which of the two they are, for the same reason §4.2 gives |
| // about reads. |
| - var writeCheck Document |
| - if err := writable(s.db.WithContext(ctx), actor). |
| - Where("documents.id = ?", doc.ID). |
| - First(&writeCheck).Error; err != nil { |
| - if errors.Is(err, gorm.ErrRecordNotFound) { |
| - return 0, fmt.Errorf("docs: apply update to document %d: %w", doc.ID, ErrNotWritable) |
| - } |
| - return 0, fmt.Errorf("docs: apply update to document %d: verify writer %d: %w", doc.ID, actor.PersonID, err) |
| + if err := s.checkPermission(ctx, s.db.WithContext(ctx), actor, doc.ID, permissionWrite); err != nil { |
| + return 0, err |
| } |
| h, replayedSeq, lastSnapshotSeq, err := s.replay(ctx, doc) |
| @@ -792,6 +798,12 @@ func (s *Store) ApplyUpdate(ctx context.Context, doc *Document, actor Actor, upd |
| // unconditionally truthful about what the handle holds. |
| if replayedSeq-lastSnapshotSeq >= snapshotCadence { |
| if err := s.writeSnapshot(ctx, doc, h, replayedSeq); err != nil { |
| + if s.team != nil { |
| + if errors.Is(err, ErrNotWritable) { |
| + return 0, err |
| + } |
| + return 0, errTeamUnavailable |
| + } |
| // Never fatal: the update below still stands, the log is |
| // untouched, and the next POST tries again — and so does |
| // every one after it, since the distance only grows until |
| @@ -842,6 +854,9 @@ func (s *Store) ApplyUpdate(ctx context.Context, doc *Document, actor Actor, upd |
| // by observing the number the closure already computed. |
| var assignedSeq int64 |
| if err := s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { |
| + if err := s.checkPermission(ctx, tx, actor, doc.ID, permissionWrite); err != nil { |
| + return err |
| + } |
| var maxSeq int64 |
| if err := tx.Model(&DocUpdate{}). |
| Where("doc_id = ?", doc.ID). |
| @@ -862,46 +877,15 @@ func (s *Store) ApplyUpdate(ctx context.Context, doc *Document, actor Actor, upd |
| if err := tx.Create(row).Error; err != nil { |
| return fmt.Errorf("docs: persist update for document %d: %w", doc.ID, err) |
| } |
| + |
| + if err := s.permissionScope(ctx, tx, actor, permissionWrite).Model(&Document{}).Where("documents.id = ?", doc.ID).Update("updated_at", time.Now().UTC()).Error; err != nil { |
| + return s.permissionError(ctx, actor, err) |
| + } |
| assignedSeq = row.Seq |
| - return nil |
| + return s.checkPermission(ctx, tx, actor, doc.ID, permissionWrite) |
| }); err != nil { |
| return 0, err |
| } |
| - // Bumping documents.updated_at here — not just appending the |
| - // doc_updates row above — is what keeps Store.List's ordering and |
| - // handlers.go's groupByRecency/formatWhen honest about the same |
| - // clock. Skipping this was fix round 2's bug: with it skipped, |
| - // UpdatedAt only ever equalled CreatedAt (Store.Create sets both |
| - // to the same `now`), so List ordering by either column looked |
| - // identical and the divergence stayed invisible until something |
| - // — this method — actually moved UpdatedAt away from CreatedAt. |
| - // |
| - // Still routed through owned(doc.OwnerID), same as every other |
| - // write in this method now — ownership was already verified above |
| - // before replay ran, so by this point the row is guaranteed to |
| - // exist under doc.OwnerID and this can never match zero rows. It |
| - // stays scoped anyway rather than falling back to a bare |
| - // `Where("id = ?", doc.ID)`: SKILL.md §3, verbatim, "Scope the |
| - // write, not just the read that loaded the row," and a defense |
| - // that only holds because of a check earlier in the same function |
| - // is one accidental reordering away from not holding at all. |
| - if err := owned(s.db.WithContext(ctx), doc.OwnerID). |
| - Model(&Document{}). |
| - Where("id = ?", doc.ID). |
| - Update("updated_at", time.Now().UTC()).Error; err != nil { |
| - // The doc_updates row above is already committed at this |
| - // point, so this error is returned over durable content. That |
| - // is deliberate and its consequences are named in the design |
| - // (D4): the caller must not broadcast and must not report a |
| - // seq, the client's save queue treats the resulting 500 as |
| - // retryable, and the retry writes a duplicate row — which Yjs |
| - // integrates idempotently, costing one row against |
| - // maxRetainedUpdates rather than any content. Returning 0 here |
| - // rather than assignedSeq is what makes "seq is meaningful |
| - // only on a nil error" true by construction instead of by |
| - // convention. |
| - return 0, fmt.Errorf("docs: bump updated_at for document %d: %w", doc.ID, err) |
| - } |
| return assignedSeq, nil |
| } |
diff --git a/internal/docs/team_access.go b/internal/docs/team_access.go| index 80c5cff..3120183 100644 |
| --- a/internal/docs/team_access.go |
| +++ b/internal/docs/team_access.go |
| @@ -100,7 +100,7 @@ func (s *Store) permissionScope(ctx context.Context, g *gorm.DB, actor Actor, p |
| clause += " OR " |
| } |
| clause += "EXISTS (SELECT 1 FROM grants WHERE grants.doc_id = documents.id AND grants.subject = ?" |
| - args = append(args, *actor.VerifiedAddress) |
| + args = append(args, subjectOf(*actor.VerifiedAddress)) |
| if p == permissionWrite { |
| clause += " AND grants.role = ?" |
| args = append(args, RoleWriter) |
| @@ -139,3 +139,47 @@ func (s *Store) requireMember(ctx context.Context, actor Actor) error { |
| return errTeamUnavailable |
| } |
| } |
| + |
| +func (s *Store) ownerActor(ctx context.Context, owner int64) Actor { |
| + if s.team == nil { |
| + return Actor{PersonID: owner} |
| + } |
| + a, ok := ctx.Value(accessContextKey{}).(*requestAccess) |
| + if !ok || a == nil || a.actor.PersonID != owner { |
| + return Actor{PersonID: owner} |
| + } |
| + return a.actor |
| +} |
| + |
| +func (s *Store) checkPermission(ctx context.Context, db *gorm.DB, actor Actor, docID int64, p permission) error { |
| + var doc Document |
| + err := s.permissionScope(ctx, db, actor, p).Where("documents.id = ? AND documents.deleted_at IS NULL", docID).First(&doc).Error |
| + err = s.permissionError(ctx, actor, err) |
| + if p == permissionWrite && errors.Is(err, gorm.ErrRecordNotFound) { |
| + return ErrNotWritable |
| + } |
| + return err |
| +} |
| +func (s *Store) snapshotPermission(ctx context.Context, db *gorm.DB, docID int64) error { |
| + if s.team == nil { |
| + return nil |
| + } |
| + a, ok := ctx.Value(accessContextKey{}).(*requestAccess) |
| + if !ok || a == nil { |
| + return errTeamUnavailable |
| + } |
| + return s.checkPermission(ctx, db, a.actor, docID, permissionWrite) |
| +} |
| + |
| +func (s *Store) ownerMutation(ctx context.Context, owner, docID int64, mutate func(*gorm.DB) error) error { |
| + actor := s.ownerActor(ctx, owner) |
| + return s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { |
| + if err := s.checkPermission(ctx, tx, actor, docID, permissionOwner); err != nil { |
| + return err |
| + } |
| + if err := mutate(tx); err != nil { |
| + return s.permissionError(ctx, actor, err) |
| + } |
| + return s.checkPermission(ctx, tx, actor, docID, permissionOwner) |
| + }) |
| +} |
diff --git a/internal/docs/team_access_test.go b/internal/docs/team_access_test.go| index b3fccf6..b0618ea 100644 |
| --- a/internal/docs/team_access_test.go |
| +++ b/internal/docs/team_access_test.go |
| @@ -8,6 +8,7 @@ import ( |
| "context" |
| "encoding/base64" |
| "errors" |
| + "github.com/carlosframework/docs/internal/docs/principalstore" |
| "github.com/carlosframework/docs/internal/docs/teamstate" |
| "github.com/carlosframework/rastrillo/migrate" |
| "gorm.io/gorm" |
| @@ -67,7 +68,28 @@ func TestTeamPermissionFormerOwnerOnlyReachesExactGrant(t *testing.T) { |
| } |
| } |
| } |
| - e := s.permissionError(c, actor, gorm.ErrRecordNotFound) |
| + |
| + for _, sqid := range []string{private.Sqid, stranger.Sqid, "nonexistent"} { |
| + _, e := s.GetFor(c, actor, sqid) |
| + if state == roster.Unknown && !errors.Is(e, errTeamUnavailable) { |
| + t.Fatal("unknown item must be retryable", e) |
| + } |
| + if state == roster.NotMember && !errors.Is(e, gorm.ErrRecordNotFound) { |
| + t.Fatal("removed owner reached private item", e) |
| + } |
| + } |
| + got, e := s.ListFor(c, actor) |
| + if e != nil || len(got) != 1 || got[0].ID != shared.ID { |
| + t.Fatal("list escaped individual grant", e) |
| + } |
| + if _, e = s.GetFor(c, actor, shared.Sqid); e != nil { |
| + t.Fatal("independent grant refused", e) |
| + } |
| + if _, e = s.Get(c, owner, private.Sqid); e == nil { |
| + t.Fatal("owner administration escaped membership") |
| + } |
| + |
| + e = s.permissionError(c, actor, gorm.ErrRecordNotFound) |
| if state == roster.Unknown && !errors.Is(e, errTeamUnavailable) { |
| t.Fatal("uncertain absence was not retryable") |
| } |
| @@ -155,7 +177,11 @@ func testTeamRoster(t *testing.T, s *Store, app *App) (*teamRuntime, *teamTestCl |
| if e != nil { |
| t.Fatal(e) |
| } |
| - rt := &teamRuntime{cache: cache, bindings: b, db: app.db.Writer()} |
| + ps, e := principalstore.New(app.db.Writer(), principalstore.Config{Issuer: b.Issuer, Team: b.Team}) |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + rt := &teamRuntime{cache: cache, bindings: b, db: app.db.Writer(), principals: ps} |
| s.team = rt |
| return rt, clock, subject |
| } |
| @@ -199,3 +225,167 @@ func TestTeamPermissionMemberLeaseEndsAtSixtySeconds(t *testing.T) { |
| t.Fatal("expired member edit must retry") |
| } |
| } |
| + |
| +func TestTeamWriterGrantRevokedWhileWaitingForSQLiteWriter(t *testing.T) { |
| + app := newSampleDocumentsInternalTestApp(t, "0") |
| + s := app.Store() |
| + ctx := t.Context() |
| + owner, e := s.EnsurePerson(ctx, "owner@example.com") |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + guest, e := s.EnsurePerson(ctx, "guest@example.com") |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + doc, e := s.Create(ctx, owner, "shared") |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + if e = s.Grant(ctx, owner, doc.Sqid, "guest@example.com", RoleWriter); e != nil { |
| + t.Fatal(e) |
| + } |
| + s.team = &teamRuntime{} |
| + actor := localActor(guest, "guest@example.com") |
| + ctx = context.WithValue(ctx, accessContextKey{}, &requestAccess{runtime: s.team, actor: actor, decision: roster.Decision{State: roster.NotMember}}) |
| + writer := app.db.Writer() |
| + tx, e := writer.BeginTx(ctx, nil) |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + defer tx.Rollback() |
| + before := writer.Stats().WaitCount |
| + done := make(chan error, 1) |
| + update := validUpdate(t) |
| + go func() { _, e := s.ApplyUpdate(ctx, doc, actor, update); done <- e }() |
| + deadline := time.Now().Add(3 * time.Second) |
| + for writer.Stats().WaitCount == before && time.Now().Before(deadline) { |
| + select { |
| + case e := <-done: |
| + t.Fatalf("update did not reach writer queue: %v", e) |
| + default: |
| + } |
| + time.Sleep(time.Millisecond) |
| + } |
| + if writer.Stats().WaitCount == before { |
| + t.Fatal("update did not wait for actual SQLite writer") |
| + } |
| + if _, e = tx.ExecContext(ctx, "DELETE FROM grants WHERE doc_id=? AND subject=?", doc.ID, "guest@example.com"); e != nil { |
| + t.Fatal(e) |
| + } |
| + if e = tx.Commit(); e != nil { |
| + t.Fatal(e) |
| + } |
| + select { |
| + case e = <-done: |
| + if !errors.Is(e, ErrNotWritable) { |
| + t.Fatal("revoked writer did not receive definite refusal", e) |
| + } |
| + case <-time.After(3 * time.Second): |
| + t.Fatal("writer deadlock") |
| + } |
| + var n int64 |
| + if e = s.db.Model(&DocUpdate{}).Where("doc_id=?", doc.ID).Count(&n).Error; e != nil || n != 0 { |
| + t.Fatal("revoked write persisted", e) |
| + } |
| + var current Document |
| + if e = s.db.First(¤t, doc.ID).Error; e != nil || !current.UpdatedAt.Equal(doc.UpdatedAt) { |
| + t.Fatal("revoked write changed timestamp", e) |
| + } |
| +} |
| + |
| +func TestTeamLeaseExpiresAfterInsertBeforeCommit(t *testing.T) { |
| + app := newSampleDocumentsInternalTestApp(t, "0") |
| + s := app.Store() |
| + ctx := t.Context() |
| + owner, e := s.EnsurePerson(ctx, "owner@example.com") |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + doc, e := s.Create(ctx, owner, "owned") |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + rt, clock, subject := testTeamRoster(t, s, app) |
| + actor := localActor(owner, "owner@example.com") |
| + ctx = context.WithValue(ctx, accessContextKey{}, rt.resolveAccess(ctx, requestIdentity{Actor: actor, HomeSubject: subject})) |
| + fired := false |
| + if e = s.db.Callback().Create().After("gorm:create").Register("test:expire-team-lease", func(tx *gorm.DB) { |
| + if tx.Statement.Table == "doc_updates" { |
| + fired = true |
| + clock.step(60 * time.Second) |
| + } |
| + }); e != nil { |
| + t.Fatal(e) |
| + } |
| + defer s.db.Callback().Create().Remove("test:expire-team-lease") |
| + seq, e := s.ApplyUpdate(ctx, doc, actor, validUpdate(t)) |
| + if !fired || seq != 0 || !errors.Is(e, errTeamUnavailable) { |
| + t.Fatal("final expiry did not refuse commit", seq, e) |
| + } |
| + var n int64 |
| + if e = s.db.Model(&DocUpdate{}).Where("doc_id=?", doc.ID).Count(&n).Error; e != nil || n != 0 { |
| + t.Fatal("expired update committed", e) |
| + } |
| + var current Document |
| + if e = s.db.First(¤t, doc.ID).Error; e != nil || !current.UpdatedAt.Equal(doc.UpdatedAt) { |
| + t.Fatal("expired update changed timestamp", e) |
| + } |
| +} |
| + |
| +func TestTeamMutationLeaseExpiryRollsBackEveryTable(t *testing.T) { |
| + for _, table := range []string{"documents", "grants", "links", "doc_snapshots"} { |
| + t.Run(table, func(t *testing.T) { |
| + app := newSampleDocumentsInternalTestApp(t, "0") |
| + s := app.Store() |
| + ctx := t.Context() |
| + owner, e := s.EnsurePerson(ctx, "owner@example.com") |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + doc, e := s.Create(ctx, owner, "owned") |
| + if e != nil { |
| + t.Fatal(e) |
| + } |
| + rt, clock, subject := testTeamRoster(t, s, app) |
| + actor := localActor(owner, "owner@example.com") |
| + ctx = context.WithValue(ctx, accessContextKey{}, rt.resolveAccess(ctx, requestIdentity{Actor: actor, HomeSubject: subject})) |
| + var before int64 |
| + if e = s.db.Table(table).Count(&before).Error; e != nil { |
| + t.Fatal(e) |
| + } |
| + fired := false |
| + if e = s.db.Callback().Create().After("gorm:create").Register("test:expire-admin", func(tx *gorm.DB) { |
| + if tx.Statement.Table == table { |
| + fired = true |
| + clock.step(60 * time.Second) |
| + } |
| + }); e != nil { |
| + t.Fatal(e) |
| + } |
| + defer s.db.Callback().Create().Remove("test:expire-admin") |
| + switch table { |
| + case "documents": |
| + _, e = s.Create(ctx, owner, "refused") |
| + case "grants": |
| + e = s.Grant(ctx, owner, doc.Sqid, "guest@example.com", RoleReader) |
| + case "links": |
| + _, e = s.Publish(ctx, owner, doc.Sqid) |
| + case "doc_snapshots": |
| + h, seq, _, err := s.replay(ctx, doc) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Close(ctx) |
| + e = s.writeSnapshot(ctx, doc, h, seq) |
| + } |
| + if !fired || !errors.Is(e, errTeamUnavailable) { |
| + t.Fatal("mutation survived final lease expiry", e) |
| + } |
| + var after int64 |
| + if e = s.db.Table(table).Count(&after).Error; e != nil || after != before { |
| + t.Fatal("mutation committed after lease expiry", e) |
| + } |
| + }) |
| + } |
| +} |
diff --git a/internal/docs/team_identity_test.go b/internal/docs/team_identity_test.go| index 02800b5..1d37662 100644 |
| --- a/internal/docs/team_identity_test.go |
| +++ b/internal/docs/team_identity_test.go |
| @@ -96,13 +96,15 @@ func TestTeamIdentityAddressAbsenceNeverRedeemsEmptyGrant(t *testing.T) { |
| if e != nil { |
| t.Fatal(e) |
| } |
| - rt := &teamRuntime{principals: ps, bindings: roster.Bindings{Issuer: "https://id.example", Team: "TeamA"}, db: app.db.Writer()} |
| + rt, _, _ := testTeamRoster(t, s, app) |
| + rt.principals = ps |
| s.team = rt |
| resolver := identityResolver{store: s, team: rt} |
| id, e := resolver.resolve(identityRequest("GET", subject, homeAssertionMethod)) |
| if e != nil || id.Actor.PersonID != p.ID || id.Actor.VerifiedAddress != nil || id.HomeSubject != subject { |
| t.Fatal("wrong Home principal", e) |
| } |
| + ctx = context.WithValue(ctx, accessContextKey{}, rt.resolveAccess(ctx, id)) |
| if e = s.db.Create(&Grant{DocID: doc.ID, Subject: "", Role: RoleWriter, CreatedBy: local}).Error; e != nil { |
| t.Fatal(e) |
| } |