Skip to content

Commit

Permalink
Auth when reading deleted records (#820)
Browse files Browse the repository at this point in the history
This addresses #819.
  • Loading branch information
andresuribe87 authored Oct 15, 2024
1 parent 0a043f2 commit 7012f5d
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 0 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ Our preferred code style has been codified into `eslint` rules. Feel free to tak
| command | description |
| --------------------------------- | ------------------------------------------------------------------------------------------------------------------ |
| `npm run test:node` | runs tests and type checking |
| `npm run test:node-grep` | runs specific tests matching a pattern. Requires the -g option. For example: `npm run test:node-grep -g "RecordsReadHandler.handle"` |
| `npm run test:browser` | runs tests against browser bundles in headless browser |
| `npm run test:browser-debug` | runs tests against browser bundles in debug-ready Chrome |
| `npm run build` | transpiles `ts` -> `js` as `esm` and `cjs`, generates `esm` and `umd` bundles, and generates all type declarations |
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@
"lint:fix": "eslint . --fix",
"circular-dependency-check": "depcruise src",
"test:node": "npm run compile-validators && tsc && c8 node --enable-source-maps node_modules/.bin/mocha \"dist/esm/tests/**/*.spec.js\"",
"test:node-grep": "npm run compile-validators && tsc && c8 node --enable-source-maps node_modules/.bin/mocha \"dist/esm/tests/**/*.spec.js\" -- --grep $npm_config_grep",
"test:browser": "npm run compile-validators && cross-env karma start karma.conf.cjs",
"test:browser-debug": "npm run compile-validators && cross-env karma start karma.conf.debug.cjs",
"license-check": "license-report --only=prod > license-report.json && node ./build/license-check.cjs",
Expand Down
1 change: 1 addition & 0 deletions src/core/dwn-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,5 @@ export enum DwnErrorCode {
UrlProtocolNotNormalized = 'UrlProtocolNotNormalized',
UrlProtocolNotNormalizable = 'UrlProtocolNotNormalizable',
UrlSchemaNotNormalized = 'UrlSchemaNotNormalized',
RecordsReadInitialWriteNotFound = 'RecordsReadInitialWriteNotFound',
};
16 changes: 16 additions & 0 deletions src/handlers/records-read.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,22 @@ export class RecordsReadHandler implements MethodHandler {
if (matchedMessage.descriptor.method === DwnMethodName.Delete) {
const recordsDeleteMessage = matchedMessage as RecordsDeleteMessage;
const initialWrite = await RecordsWrite.fetchInitialRecordsWriteMessage(this.messageStore, tenant, recordsDeleteMessage.descriptor.recordId);

if (initialWrite === undefined) {
return messageReplyFromError(new DwnError(
DwnErrorCode.RecordsReadInitialWriteNotFound,
'Initial write for deleted record not found'
), 400);
}

// Perform authorization before returning the delete and initial write messages
const parsedInitialWrite = await RecordsWrite.parse(initialWrite);
try {
await RecordsReadHandler.authorizeRecordsRead(tenant, recordsRead, parsedInitialWrite, this.messageStore);
} catch (error) {
return messageReplyFromError(error, 401);
}

return {
status : { code: 404, detail: 'Not Found' },
entry : {
Expand Down
94 changes: 94 additions & 0 deletions tests/handlers/records-read.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,100 @@ export function testRecordsReadHandler(): void {
expect(ArrayUtility.byteArraysEqual(dataFetched, dataBytes!)).to.be.true;
});

it('should return 400 when fetching initial write for a deleted record fails', async () => {
const alice = await TestDataGenerator.generateDidKeyPersona();

// Write a record
const { message: writeMessage, dataStream } = await TestDataGenerator.generateRecordsWrite({ author: alice });
const writeReply = await dwn.processMessage(alice.did, writeMessage, { dataStream });
expect(writeReply.status.code).to.equal(202);

// Delete the record
const recordsDelete = await RecordsDelete.create({
signer : Jws.createSigner(alice),
recordId : writeMessage.recordId
});
const deleteReply = await dwn.processMessage(alice.did, recordsDelete.message);
expect(deleteReply.status.code).to.equal(202);

// Stub the messageStore.query method to simulate failure in fetching initial write
const queryStub = sinon.stub(dwn['messageStore'], 'query');
queryStub.onFirstCall().resolves({ messages: [recordsDelete.message] });
queryStub.onSecondCall().resolves({ messages: [] }); // Simulate no initial write found

// Attempt to read the deleted record
const recordsRead = await RecordsRead.create({
filter : { recordId: writeMessage.recordId },
signer : Jws.createSigner(alice)
});
const readReply = await dwn.processMessage(alice.did, recordsRead.message);

// Verify the response
expect(readReply.status.code).to.equal(400);
expect(readReply.status.detail).to.contain(DwnErrorCode.RecordsReadInitialWriteNotFound);

// Restore the original messageStore.query method
queryStub.restore();
});

it('should return 401 when a non-author attempts to read the initial write of a deleted record', async () => {
const alice = await TestDataGenerator.generateDidKeyPersona();
const bob = await TestDataGenerator.generateDidKeyPersona();
const carol = await TestDataGenerator.generateDidKeyPersona();

// Alice installs a protocol that allows anyone to write
const protocolDefinition: ProtocolDefinition = {
published : true,
protocol : 'https://example.com/foo',
types : {
foo: {}
},
structure: {
foo: {
$actions: [{
who : 'anyone',
can : ['create', 'delete']
}]
}
}
};

const configureProtocol = await TestDataGenerator.generateProtocolsConfigure({
author : alice,
protocolDefinition : protocolDefinition,
});
const configureProtocolReply = await dwn.processMessage(alice.did, configureProtocol.message);
expect(configureProtocolReply.status.code).to.equal(202);

// Bob writes a record to Alice's DWN
const { message: writeMessage, dataStream } = await TestDataGenerator.generateRecordsWrite({
author : bob,
protocol : protocolDefinition.protocol,
protocolPath : 'foo'
});
const writeReply = await dwn.processMessage(alice.did, writeMessage, { dataStream });
expect(writeReply.status.code).to.equal(202);

// Bob deletes the record
const recordsDelete = await RecordsDelete.create({
signer : Jws.createSigner(bob),
recordId : writeMessage.recordId
});
const deleteReply = await dwn.processMessage(alice.did, recordsDelete.message);
expect(deleteReply.status.code).to.equal(202);

// Carol attempts to read the deleted record
const recordsRead = await RecordsRead.create({
filter : { recordId: writeMessage.recordId },
signer : Jws.createSigner(carol)
});
const readReply = await dwn.processMessage(alice.did, recordsRead.message);

// Verify the response
expect(readReply.status.code).to.equal(401);
expect(readReply.status.detail).to.contain(DwnErrorCode.ProtocolAuthorizationActionNotAllowed);
});

it('should allow a non-tenant to read RecordsRead data they have authored', async () => {
const alice = await TestDataGenerator.generateDidKeyPersona();
const bob = await TestDataGenerator.generateDidKeyPersona();
Expand Down

0 comments on commit 7012f5d

Please sign in to comment.