From a233e8a78c1d1a03cb0457596881c8796f83bb74 Mon Sep 17 00:00:00 2001 From: Henry Tsai Date: Thu, 15 Aug 2024 15:39:58 -0700 Subject: [PATCH] Disallowed write after delete (#796) --------- Co-authored-by: Liran Cohen --- src/core/dwn-error.ts | 2 +- src/handlers/records-write.ts | 18 +++-- tests/handlers/records-write.spec.ts | 103 +++++++++------------------ 3 files changed, 44 insertions(+), 79 deletions(-) diff --git a/src/core/dwn-error.ts b/src/core/dwn-error.ts index 981029378..a8d5b5283 100644 --- a/src/core/dwn-error.ts +++ b/src/core/dwn-error.ts @@ -138,9 +138,9 @@ export enum DwnErrorCode { RecordsWriteMissingSigner = 'RecordsWriteMissingSigner', RecordsWriteMissingDataInPrevious = 'RecordsWriteMissingDataInPrevious', RecordsWriteMissingEncodedDataInPrevious = 'RecordsWriteMissingEncodedDataInPrevious', - RecordsWriteMissingDataStream = 'RecordsWriteMissingDataStream', RecordsWriteMissingProtocol = 'RecordsWriteMissingProtocol', RecordsWriteMissingSchema = 'RecordsWriteMissingSchema', + RecordsWriteNotAllowedAfterDelete = 'RecordsWriteNotAllowedAfterDelete', RecordsWriteOwnerAndTenantMismatch = 'RecordsWriteOwnerAndTenantMismatch', RecordsWriteSignAsOwnerDelegateUnknownAuthor = 'RecordsWriteSignAsOwnerDelegateUnknownAuthor', RecordsWriteSignAsOwnerUnknownAuthor = 'RecordsWriteSignAsOwnerUnknownAuthor', diff --git a/src/handlers/records-write.ts b/src/handlers/records-write.ts index 47f8a1c39..f4fcd6a26 100644 --- a/src/handlers/records-write.ts +++ b/src/handlers/records-write.ts @@ -96,6 +96,13 @@ export class RecordsWriteHandler implements MethodHandler { } try { + if (newestExistingMessage?.descriptor.method === DwnMethodName.Delete) { + throw new DwnError( + DwnErrorCode.RecordsWriteNotAllowedAfterDelete, + 'RecordsWrite is not allowed after a RecordsDelete.' + ); + } + // NOTE: We want to perform additional validation before storing the RecordsWrite. // This is necessary for core DWN RecordsWrite that needs additional processing and allows us to fail before the storing and post processing. // @@ -116,15 +123,6 @@ export class RecordsWriteHandler implements MethodHandler { } else { // else data stream is NOT provided - if (newestExistingMessage?.descriptor.method === DwnMethodName.Delete) { - throw new DwnError( - DwnErrorCode.RecordsWriteMissingDataStream, - 'No data stream was provided with the previous message being a delete' - ); - } - - // at this point we know that newestExistingMessage exists is not a Delete - // if the incoming message is not an initial write, and no dataStream is provided, we would allow it provided it passes validation // processMessageWithoutDataStream() abstracts that logic if (!newMessageIsInitialWrite) { @@ -149,7 +147,7 @@ export class RecordsWriteHandler implements MethodHandler { if (e.code !== undefined) { if (e.code === DwnErrorCode.RecordsWriteMissingEncodedDataInPrevious || e.code === DwnErrorCode.RecordsWriteMissingDataInPrevious || - e.code === DwnErrorCode.RecordsWriteMissingDataStream || + e.code === DwnErrorCode.RecordsWriteNotAllowedAfterDelete || e.code === DwnErrorCode.RecordsWriteDataCidMismatch || e.code === DwnErrorCode.RecordsWriteDataSizeMismatch || e.code.startsWith('PermissionsProtocolValidate') || diff --git a/tests/handlers/records-write.spec.ts b/tests/handlers/records-write.spec.ts index 17c6acce1..2f5084676 100644 --- a/tests/handlers/records-write.spec.ts +++ b/tests/handlers/records-write.spec.ts @@ -629,70 +629,6 @@ export function testRecordsWriteHandler(): void { }); }); - it('should return 400 for if dataStream is not present for a write after a delete', async () => { - const { message, author, dataStream, dataBytes } = await TestDataGenerator.generateRecordsWrite({ - data : TestDataGenerator.randomBytes(DwnConstant.maxDataSizeAllowedToBeEncoded), - published : false - }); - const tenant = author.did; - - TestStubGenerator.stubDidResolver(didResolver, [author]); - - const initialWriteReply = await dwn.processMessage(tenant, message, { dataStream }); - expect(initialWriteReply.status.code).to.equal(202); - - const recordsDelete = await RecordsDelete.create({ - recordId : message.recordId, - signer : Jws.createSigner(author), - }); - const deleteReply = await dwn.processMessage(tenant, recordsDelete.message); - expect(deleteReply.status.code).to.equal(202); - - const write = await RecordsWrite.createFrom({ - recordsWriteMessage : message, - signer : Jws.createSigner(author), - }); - - const withoutDataReply = await dwn.processMessage(tenant, write.message); - expect(withoutDataReply.status.code).to.equal(400); - expect(withoutDataReply.status.detail).to.contain(DwnErrorCode.RecordsWriteMissingDataStream); - const updatedWriteData = DataStream.fromBytes(dataBytes!); - const withoutDataReply2 = await dwn.processMessage(tenant, write.message, { dataStream: updatedWriteData }); - expect(withoutDataReply2.status.code).to.equal(202); - }); - - it('should return 400 for if dataStream is not present for a write after a delete with data above the threshold', async () => { - const { message, author, dataStream, dataBytes } = await TestDataGenerator.generateRecordsWrite({ - data : TestDataGenerator.randomBytes(DwnConstant.maxDataSizeAllowedToBeEncoded + 1), - published : false - }); - const tenant = author.did; - - TestStubGenerator.stubDidResolver(didResolver, [author]); - - const initialWriteReply = await dwn.processMessage(tenant, message, { dataStream }); - expect(initialWriteReply.status.code).to.equal(202); - - const recordsDelete = await RecordsDelete.create({ - recordId : message.recordId, - signer : Jws.createSigner(author), - }); - const deleteReply = await dwn.processMessage(tenant, recordsDelete.message); - expect(deleteReply.status.code).to.equal(202); - - const write = await RecordsWrite.createFrom({ - recordsWriteMessage : message, - signer : Jws.createSigner(author), - }); - - const withoutDataReply = await dwn.processMessage(tenant, write.message); - expect(withoutDataReply.status.code).to.equal(400); - expect(withoutDataReply.status.detail).to.contain(DwnErrorCode.RecordsWriteMissingDataStream); - const updatedWriteData = DataStream.fromBytes(dataBytes!); - const withoutDataReply2 = await dwn.processMessage(tenant, write.message, { dataStream: updatedWriteData }); - expect(withoutDataReply2.status.code).to.equal(202); - }); - it('should return 400 for data CID mismatch with both dataStream and `dataSize` larger than encodedData threshold', async () => { const alice = await TestDataGenerator.generateDidKeyPersona(); const { message } = await TestDataGenerator.generateRecordsWrite({ @@ -4059,9 +3995,9 @@ export function testRecordsWriteHandler(): void { }); }); - it('should 400 if dataStream is not provided and dataStore does not contain dataCid', async () => { - // scenario: A sync writes a pruned initial RecordsWrite, without a `dataStream`. Alice does another regular - // RecordsWrite for the same record, referencing the same `dataCid` but omitting the `dataStream`. + it('should return 400 if dataStream is not provided and dataStore does not contain dataCid', async () => { + // scenario: A sync writes a pruned initial RecordsWrite, without a `dataStream`. Alice does another regular + // RecordsWrite for the same record, referencing the same `dataCid` but omitting the `dataStream`. // Pruned RecordsWrite // Data large enough to use the DataStore @@ -4087,7 +4023,7 @@ export function testRecordsWriteHandler(): void { expect(recordsWriteReply.status.detail).to.contain(DwnErrorCode.RecordsWriteMissingDataInPrevious); }); - it('should 400 if dataStream is not provided and previous message does not contain encodedData', async () => { + it('should return 400 if dataStream is not provided and previous message does not contain encodedData', async () => { // scenario: A sync writes a pruned initial RecordsWrite, without a `dataStream`. Alice does another regular // RecordsWrite for the same record, referencing the same `dataCid` but omitting the `dataStream`. @@ -4115,6 +4051,37 @@ export function testRecordsWriteHandler(): void { expect(recordsWriteReply.status.detail).to.contain(DwnErrorCode.RecordsWriteMissingEncodedDataInPrevious); }); + it('should return 400 if attempting a write after a delete', async () => { + const { message, author, dataStream } = await TestDataGenerator.generateRecordsWrite({ + data : TestDataGenerator.randomBytes(DwnConstant.maxDataSizeAllowedToBeEncoded), + published : false + }); + const tenant = author.did; + + TestStubGenerator.stubDidResolver(didResolver, [author]); + + const initialWriteReply = await dwn.processMessage(tenant, message, { dataStream }); + expect(initialWriteReply.status.code).to.equal(202); + + const recordsDelete = await RecordsDelete.create({ + recordId : message.recordId, + signer : Jws.createSigner(author), + }); + const deleteReply = await dwn.processMessage(tenant, recordsDelete.message); + expect(deleteReply.status.code).to.equal(202); + + const newDataBytes = TestDataGenerator.randomBytes(100); + const newInvalidWrite = await RecordsWrite.createFrom({ + recordsWriteMessage : message, + signer : Jws.createSigner(author), + data : newDataBytes + }); + + const newInvalidWriteReply = await dwn.processMessage(tenant, newInvalidWrite.message, { dataStream: DataStream.fromBytes(newDataBytes) }); + expect(newInvalidWriteReply.status.code).to.equal(400); + expect(newInvalidWriteReply.status.detail).to.contain(DwnErrorCode.RecordsWriteNotAllowedAfterDelete); + }); + it('should not allow referencing data across tenants', async () => { const alice = await TestDataGenerator.generateDidKeyPersona(); const bob = await TestDataGenerator.generateDidKeyPersona();