Skip to content

Commit

Permalink
Disallowed write after delete (#796)
Browse files Browse the repository at this point in the history
---------

Co-authored-by: Liran Cohen <[email protected]>
  • Loading branch information
thehenrytsai and LiranCohen authored Aug 15, 2024
1 parent 7491616 commit a233e8a
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 79 deletions.
2 changes: 1 addition & 1 deletion src/core/dwn-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
18 changes: 8 additions & 10 deletions src/handlers/records-write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand All @@ -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) {
Expand All @@ -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') ||
Expand Down
103 changes: 35 additions & 68 deletions tests/handlers/records-write.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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
Expand All @@ -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`.

Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit a233e8a

Please sign in to comment.