Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallowed write after delete #796

Merged
merged 3 commits into from
Aug 15, 2024
Merged

Disallowed write after delete #796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f430896
Disallowed write after delete
thehenrytsai Aug 15, 2024
96cadbd
lint
thehenrytsai Aug 15, 2024
75118b6
Update tests/handlers/records-write.spec.ts
thehenrytsai Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/core/dwn-error.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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