feat(api): record who created a document and who last wrote it

api/openapiv2/restcol.swagger.json

@@ -737,6 +737,13 @@
           "type": "string",
           "format": "date-time",
           "description": "ts when the record was last written.\n\nWriting a document that already exists is an UPSERT - CreateDocument with\nan existing documentId replaces the payload - and there is no separate\nUpdate rpc. Without this field an overwritten document is\nindistinguishable from one written once, in both content and metadata, so\na caller cannot tell that its data was replaced or when.\n\nNOT a new column: ModelDocument has carried `updated_at` all along and\ngorm has been maintaining it on every write. This only surfaces what the\ndatabase already records.\n\nNon-optional, matching _createdAt rather than _deletedAt: gorm sets\nupdated_at on create as well as on update, so every document has one. On\na document that has never been rewritten it equals _createdAt."
+        },
+        "CreatedBy": {
+          "type": "string",
+          "description": "Who created the document, and who last wrote it.\n\nTwo fields rather than one because writing an existing documentId is an\nUPSERT: with only a creator, a document overwritten by a DIFFERENT\nprincipal would still report the original author and the second write\nwould be invisible. \"A created this, B replaced its contents\" is the\nquestion these exist to answer.\n\n_createdBy is set once, on insert, and never moves - it pairs with\n_createdAt. _updatedBy is rewritten on every write, pairing with\n_updatedAt. The two move independently for the same reason the two\ntimestamps do.\n\nEMPTY MEANS UNATTRIBUTED, not anonymous-and-fine. A deployment whose\ncaller resolver is not wired records no writer, and an empty value here\nsays exactly that rather than inventing a principal."
+        },
+        "UpdatedBy": {
+          "type": "string"
         }
       }
     },

api/restcol.proto

@@ -299,6 +299,25 @@ message DataMetadata {
     // updated_at on create as well as on update, so every document has one. On
     // a document that has never been rewritten it equals _createdAt.
     google.protobuf.Timestamp _updatedAt = 13;
+
+    // Who created the document, and who last wrote it.
+    //
+    // Two fields rather than one because writing an existing documentId is an
+    // UPSERT: with only a creator, a document overwritten by a DIFFERENT
+    // principal would still report the original author and the second write
+    // would be invisible. "A created this, B replaced its contents" is the
+    // question these exist to answer.
+    //
+    // _createdBy is set once, on insert, and never moves - it pairs with
+    // _createdAt. _updatedBy is rewritten on every write, pairing with
+    // _updatedAt. The two move independently for the same reason the two
+    // timestamps do.
+    //
+    // EMPTY MEANS UNATTRIBUTED, not anonymous-and-fine. A deployment whose
+    // caller resolver is not wired records no writer, and an empty value here
+    // says exactly that rather than inventing a principal.
+    string _createdBy = 14;
+    string _updatedBy = 15;
 }
 
 

pkg/app/app.go

@@ -31,6 +31,10 @@ type RestColServiceServerService struct {
 	schemaBuilder *schemafinder.SchemaBuilder
 
 	defaultProjectResolver sdinsureruntime.ProjectResolver
+
+	// Optional. Nil records no writer rather than refusing writes - see
+	// callerFromCtx.
+	callerResolver CallerResolver
 }
 
 // NewRestColServiceServerService wires a new service handler. Call
@@ -49,6 +53,16 @@ func NewRestColServiceServerService(
 	}
 }
 
+// SetCallerResolver installs the resolver that names the principal behind a
+// request, so documents record who wrote them.
+//
+// OPTIONAL, unlike the project resolver. Without it restcol still serves every
+// request and records an empty writer: attribution is worth having, not worth
+// refusing data over. A deployment can therefore adopt this without a flag day.
+func (r *RestColServiceServerService) SetCallerResolver(cr CallerResolver) {
+	r.callerResolver = cr
+}
+
 // SetDefaultProjectResolver installs the resolver that maps incoming requests
 // to a project tenant. Without it, every handler returns an error.
 func (r *RestColServiceServerService) SetDefaultProjectResolver(projectResolver sdinsureruntime.ProjectResolver) {

pkg/app/caller.go

@@ -0,0 +1,38 @@
+package app
+
+import "context"
+
+// CallerResolver names the principal behind a request, so a document can record
+// who wrote it.
+//
+// An interface injected by the wrapping service, exactly like the project
+// resolver: restcol is a library and does not own an authentication scheme, so
+// it cannot know how a caller is represented. The service that authenticated
+// the request is the only thing that does.
+//
+// Returning "" means "no principal available", which is recorded as an empty
+// writer. That is deliberate: an unattributed document must be distinguishable
+// from one written by a principal literally named "anonymous" or "system", and
+// inventing a name here would make the two identical forever after.
+type CallerResolver interface {
+	// Caller returns the principal for this request, or "" if there is none.
+	Caller(ctx context.Context) string
+}
+
+// CallerResolverFunc adapts a function to CallerResolver.
+type CallerResolverFunc func(ctx context.Context) string
+
+func (f CallerResolverFunc) Caller(ctx context.Context) string { return f(ctx) }
+
+// callerFromCtx is the single place a writer is derived.
+//
+// Nil-safe on purpose. A deployment that has not wired a resolver keeps working
+// and records no writer, rather than failing every write - attribution is a
+// property worth having, not worth refusing data over. The empty value is then
+// honest about what happened.
+func (r *RestColServiceServerService) callerFromCtx(ctx context.Context) string {
+	if r.callerResolver == nil {
+		return ""
+	}
+	return r.callerResolver.Caller(ctx)
+}

pkg/app/documents.go

@@ -93,8 +93,16 @@ func (r *RestColServiceServerService) CreateDocument(ctx context.Context, req *a
 		}
 	}
 
+	// Both are set on the way in. The upsert then keeps created_by (it is not
+	// in DoUpdates) and overwrites updated_by, so a document replaced by a
+	// different principal reports both of them - which is the question these
+	// two fields exist to answer.
+	caller := r.callerFromCtx(ctx)
+
 	docModel := &documentsmodel.ModelDocument{
 		ID:                docId,
+		CreatedBy:         caller,
+		UpdatedBy:         caller,
 		Data:              documentsmodel.NewModelDocumentData(valueHolder),
 		ModelCollectionID: cid,
 		ModelCollection: collectionsmodel.NewModelCollection(

pkg/models/documents/documents.go

@@ -24,6 +24,26 @@ type ModelDocument struct {
 	UpdatedAt time.Time      `gorm:"column:updated_at"`
 	DeletedAt gorm.DeletedAt `gorm:"column:deleted_at"`
 
+	// CreatedBy is the principal that first wrote the document; UpdatedBy is
+	// the one that wrote it last.
+	//
+	// Two columns because writing an existing documentId is an UPSERT: with
+	// only a creator, a document overwritten by a DIFFERENT principal would
+	// still report the original author and the second write would leave no
+	// trace of who made it.
+	//
+	// They must move independently, and the upsert in DocumentCURD.Write is
+	// what makes that true: updated_by is in its DoUpdates list and created_by
+	// is deliberately NOT, exactly as with updated_at and created_at. If
+	// created_by ever joined that list, both fields would report the most
+	// recent writer and the pair would answer nothing.
+	//
+	// Empty means unattributed - a deployment with no caller resolver wired
+	// records no writer, which is a different thing from a writer named
+	// "anonymous".
+	CreatedBy string `gorm:"column:created_by"`
+	UpdatedBy string `gorm:"column:updated_by"`
+
 	Data *ModelDocumentData `gorm:"column:data;type:jsonb"`
 
 	ModelCollectionID modelcollections.CollectionID `gorm:"column:model_collection_id;primaryKey;index:docScope,priority:2"` // foreign key to model collection

pkg/models/documents/dto.go

@@ -27,6 +27,10 @@ func NewPbDocumentMetadata(md *ModelDocument) *apppb.DataMetadata {
 		// content and metadata. gorm has always maintained the column; only the
 		// API was hiding it.
 		XUpdatedAt: timestamppb.New(md.UpdatedAt),
+		// Empty when no caller resolver is wired - which says "unattributed"
+		// rather than naming a principal that does not exist.
+		XCreatedBy: md.CreatedBy,
+		XUpdatedBy: md.UpdatedBy,
 		XDeletedAt: nil,
 	}
 	if deletedAt, _ := md.DeletedAt.Value(); deletedAt != nil {

pkg/models/documents/dto_test.go

@@ -94,3 +94,52 @@ func TestDeletedAtAndUpdatedAtCoexist(t *testing.T) {
 	assert.Equal(t, updated.UTC(), pb.XUpdatedAt.AsTime().UTC())
 	assert.Equal(t, created.UTC(), pb.XCreatedAt.AsTime().UTC())
 }
+
+// Attribution must survive the DTO. The columns are useless if the API does not
+// carry them, and the failure would be silent: the database would hold the
+// right answer while every caller saw nothing.
+func TestWriterAttributionIsSurfaced(t *testing.T) {
+	const alice = "grandturk:apikey:42:alice-key"
+	const bob = "grandturk:apikey:42:bob-key"
+
+	md := fixture(time.Now(), time.Now())
+	md.CreatedBy = alice
+	md.UpdatedBy = bob
+
+	pb := NewPbDocumentMetadata(md)
+
+	assert.Equal(t, alice, pb.XCreatedBy)
+	assert.Equal(t, bob, pb.XUpdatedBy)
+}
+
+// The distinguishing case, through the API: a document overwritten by someone
+// else must be tellable from one written once by its creator.
+func TestOverwrittenByAnotherPrincipalIsVisibleThroughTheApi(t *testing.T) {
+	const alice = "grandturk:apikey:42:alice-key"
+	const bob = "grandturk:apikey:42:bob-key"
+	at := time.Now()
+
+	own := fixture(at, at)
+	own.CreatedBy, own.UpdatedBy = alice, alice
+
+	replaced := fixture(at, at.Add(time.Hour))
+	replaced.CreatedBy, replaced.UpdatedBy = alice, bob
+
+	ownPb := NewPbDocumentMetadata(own)
+	replacedPb := NewPbDocumentMetadata(replaced)
+
+	assert.Equal(t, ownPb.XCreatedBy, replacedPb.XCreatedBy,
+		"both were created by the same principal")
+	assert.NotEqual(t, ownPb.XUpdatedBy, replacedPb.XUpdatedBy,
+		"but only one was last written by somebody else - if these compared "+
+			"equal the API could not tell an overwrite from a first write")
+}
+
+// Empty means unattributed, and must not become a placeholder on the way out.
+func TestUnattributedStaysEmptyThroughTheApi(t *testing.T) {
+	pb := NewPbDocumentMetadata(fixture(time.Now(), time.Now()))
+
+	assert.Empty(t, pb.XCreatedBy,
+		"an unattributed document must not acquire a writer in the DTO")
+	assert.Empty(t, pb.XUpdatedBy)
+}

pkg/storage/documents/documents.go

@@ -35,15 +35,15 @@ func (c *DocumentCURD) AutoMigrate() error {
 func (c *DocumentCURD) Write(ctx context.Context, tableName string, record *appmodeldocuments.ModelDocument) error {
 	err := c.With(ctx, tableName).Clauses(clause.OnConflict{
 		Columns:   []clause.Column{{Name: "id"}, {Name: "model_collection_id"}, {Name: "model_project_id"}},
-		DoUpdates: clause.AssignmentColumns([]string{"updated_at", "deleted_at", "data"}),
+		DoUpdates: clause.AssignmentColumns([]string{"updated_at", "updated_by", "deleted_at", "data"}),
 	}).Create(record).Error
 	return storage.WrapStorageError(err)
 }
 
 func (c *DocumentCURD) BatchWrite(ctx context.Context, tableName string, records []*appmodeldocuments.ModelDocument) error {
 	err := c.With(ctx, tableName).Clauses(clause.OnConflict{
 		Columns:   []clause.Column{{Name: "id"}, {Name: "model_collection_id"}, {Name: "model_project_id"}},
-		DoUpdates: clause.AssignmentColumns([]string{"updated_at", "deleted_at", "data"}),
+		DoUpdates: clause.AssignmentColumns([]string{"updated_at", "updated_by", "deleted_at", "data"}),
 	}).Create(&records).Error
 	return storage.WrapStorageError(err)
 }

pkg/storage/documents/documents_test.go

@@ -406,3 +406,104 @@ func TestUpsertAdvancesUpdatedAtAndPreservesCreatedAt(t *testing.T) {
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, count, "upsert must not append a second row")
 }
+
+// The scenario this pair of columns exists for: one principal creates a
+// document, a DIFFERENT one overwrites it, and both are recoverable.
+//
+// Writing an existing documentId is an upsert and there is no Update rpc, so
+// before created_by/updated_by there was no way to tell that a second party had
+// replaced someone else's data - not from the content, not from the metadata.
+//
+// It pins the asymmetry that makes the pair meaningful: updated_by is in the
+// upsert's DoUpdates and created_by is not. If created_by ever joined that
+// list, BOTH fields would report the most recent writer, the pair would answer
+// nothing, and every other assertion here would still pass.
+func TestUpsertByADifferentPrincipalKeepsCreatorAndMovesUpdater(t *testing.T) {
+	if testing.Short() {
+		t.Skip("needs postgres")
+		return
+	}
+	ctx := context.Background()
+	postgrescli, err := storagetestutils.NewTestPostgresCli(logger.NewLogger(false))
+	assert.NoError(t, err)
+
+	regularProject, _, err := storageprojects.TestProjectSuite(postgrescli)
+	assert.Nil(t, err)
+	collection, err := storagecollectionstestutils.TestCollectionSuite(postgrescli, regularProject)
+	assert.NoError(t, err)
+
+	dcrud := NewDocumentCURD(postgrescli)
+	assert.Nil(t, dcrud.AutoMigrate())
+
+	docID := appmodeldocuments.NewDocumentID()
+	write := func(principal, value string) {
+		assert.NoError(t, dcrud.Write(ctx, "", &appmodeldocuments.ModelDocument{
+			ID:                docID,
+			CreatedBy:         principal,
+			UpdatedBy:         principal,
+			Data:              appmodeldocuments.NewModelDocumentData(map[string]interface{}{"v": value}),
+			ModelCollectionID: collection.ID,
+			ModelProjectID:    regularProject.ID,
+		}))
+	}
+
+	const alice = "grandturk:apikey:42:alice-key"
+	const bob = "grandturk:apikey:42:bob-key"
+
+	write(alice, "first")
+	first, err := dcrud.Get(ctx, "", regularProject.ID, collection.ID, docID)
+	assert.NoError(t, err)
+	assert.Equal(t, alice, first.CreatedBy)
+	assert.Equal(t, alice, first.UpdatedBy,
+		"on a document written once, the creator is also the last writer")
+
+	write(bob, "second")
+	second, err := dcrud.Get(ctx, "", regularProject.ID, collection.ID, docID)
+	assert.NoError(t, err)
+
+	assert.Equal(t, alice, second.CreatedBy,
+		"the creator must survive an overwrite by someone else - this is the "+
+			"half that fails if created_by joins the upsert's DoUpdates")
+	assert.Equal(t, bob, second.UpdatedBy,
+		"the last writer must be the principal that actually overwrote it")
+	assert.Equal(t, "second", second.Data.MapValue["v"],
+		"and the overwrite must really have replaced the payload")
+
+	count, err := dcrud.CountByCollection(ctx, "", regularProject.ID, collection.ID)
+	assert.NoError(t, err)
+	assert.EqualValues(t, 1, count, "an upsert must not append a second row")
+}
+
+// An unattributed write records an empty writer rather than a placeholder.
+// A deployment with no caller resolver must be distinguishable from one whose
+// documents were genuinely written by a principal called "system".
+func TestUnattributedWritesRecordNoPrincipal(t *testing.T) {
+	if testing.Short() {
+		t.Skip("needs postgres")
+		return
+	}
+	ctx := context.Background()
+	postgrescli, err := storagetestutils.NewTestPostgresCli(logger.NewLogger(false))
+	assert.NoError(t, err)
+
+	regularProject, _, err := storageprojects.TestProjectSuite(postgrescli)
+	assert.Nil(t, err)
+	collection, err := storagecollectionstestutils.TestCollectionSuite(postgrescli, regularProject)
+	assert.NoError(t, err)
+
+	dcrud := NewDocumentCURD(postgrescli)
+	assert.Nil(t, dcrud.AutoMigrate())
+
+	docID := appmodeldocuments.NewDocumentID()
+	assert.NoError(t, dcrud.Write(ctx, "", &appmodeldocuments.ModelDocument{
+		ID:                docID,
+		Data:              appmodeldocuments.NewModelDocumentData(map[string]interface{}{"v": "x"}),
+		ModelCollectionID: collection.ID,
+		ModelProjectID:    regularProject.ID,
+	}))
+
+	got, err := dcrud.Get(ctx, "", regularProject.ID, collection.ID, docID)
+	assert.NoError(t, err)
+	assert.Empty(t, got.CreatedBy, "an unattributed document must not invent a writer")
+	assert.Empty(t, got.UpdatedBy)
+}