Skip to content

Commit

Permalink
Enforce requiring all read actions (read, query, subscribe) to …
Browse files Browse the repository at this point in the history
…exist in a protocol role rule. (#813)

Currently there are some inconsistencies being able to fetch record
data. If a user can only query for something, they are able to get the
data if it is under the encodedData size limit, however if it is not
they are unable to issue a `RecordsRead` for that record.

This change forces the user to include all 3 "read" actions (read,
subscribe, query) when using any one of them in a protocol role-based
action rule.
  • Loading branch information
LiranCohen authored Oct 4, 2024
1 parent 8b6eb13 commit 7b02581
Show file tree
Hide file tree
Showing 7 changed files with 300 additions and 5 deletions.
1 change: 1 addition & 0 deletions src/core/dwn-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export enum DwnErrorCode {
ProtocolsConfigureInvalidTagSchema = 'ProtocolsConfigureInvalidTagSchema',
ProtocolsConfigureRecordNestingDepthExceeded = 'ProtocolsConfigureRecordNestingDepthExceeded',
ProtocolsConfigureRoleDoesNotExistAtGivenPath = 'ProtocolsConfigureRoleDoesNotExistAtGivenPath',
ProtocolsConfigureRoleReadActionMissing = 'ProtocolsConfigureRoleReadActionMissing',
ProtocolsGrantAuthorizationQueryProtocolScopeMismatch = 'ProtocolsGrantAuthorizationQueryProtocolScopeMismatch',
ProtocolsGrantAuthorizationScopeProtocolMismatch = 'ProtocolsGrantAuthorizationScopeProtocolMismatch',
ProtocolsQueryUnauthorized = 'ProtocolsQueryUnauthorized',
Expand Down
10 changes: 10 additions & 0 deletions src/interfaces/protocols-configure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,16 @@ export class ProtocolsConfigure extends AbstractMessage<ProtocolsConfigureMessag
DwnErrorCode.ProtocolsConfigureRoleDoesNotExistAtGivenPath,
`Role in action ${JSON.stringify(actionRule)} for rule set ${ruleSetProtocolPath} does not exist.`
);
} else {
// it is a role record, we ensure that if any of the `can` actions are 'read' type of actions ('read', 'query', 'subscribe'),
// that they are all present.
const readActions = [ProtocolAction.Read, ProtocolAction.Query, ProtocolAction.Subscribe];
if (readActions.find( action => actionRule.can.includes(action)) && !readActions.every(action => actionRule.can.includes(action))) {
throw new DwnError(
DwnErrorCode.ProtocolsConfigureRoleReadActionMissing,
`Role in action ${JSON.stringify(actionRule)} for rule set ${ruleSetProtocolPath} must contain all read actions (${readActions.join(', ')}).`
);
}
}
}

Expand Down
155 changes: 155 additions & 0 deletions tests/handlers/protocols-configure.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,161 @@ export function testProtocolsConfigureHandler(): void {
expect(protocolsConfigureReply.status.detail).to.contain(DwnErrorCode.ProtocolsConfigureDuplicateRoleInRuleSet);
});

it('should reject ProtocolsConfigure with role action rule that contain a read action(`read` || `query` || `subscribe`) but not all of the read actions', async () => {
const alice = await TestDataGenerator.generateDidKeyPersona();

// without 'subscribe' action
const protocolDefinitionWithoutSubscribe: ProtocolDefinition = {
protocol : 'http://foo',
published : true,
types : {
friend : {},
foo : {},
},
structure: {
friend: {
$role: true
},
foo: {
$actions: [
{
role : 'friend',
can : [ProtocolAction.Read, ProtocolAction.Query] // missing `subscribe`
}
]
}
}
};

// manually craft the invalid ProtocolsConfigure message because our library will not let you create an invalid definition
let descriptor: ProtocolsConfigureDescriptor = {
interface : DwnInterfaceName.Protocols,
method : DwnMethodName.Configure,
messageTimestamp : Time.getCurrentTimestamp(),
definition : protocolDefinitionWithoutSubscribe
};

let authorization = await Message.createAuthorization({
descriptor,
signer: Jws.createSigner(alice)
});
let protocolsConfigureMessage = { descriptor, authorization };

const withoutSubscribeResponse = await dwn.processMessage(alice.did, protocolsConfigureMessage);
expect(withoutSubscribeResponse.status.code).to.equal(400);

// without 'query' action
const protocolDefinitionWithoutQuery: ProtocolDefinition = {
protocol : 'http://foo',
published : true,
types : {
friend : {},
foo : {},
},
structure: {
friend: {
$role: true
},
foo: {
$actions: [
{
role : 'friend',
can : [ProtocolAction.Read, ProtocolAction.Subscribe] // missing `query`
}
]
}
}
};

descriptor = {
interface : DwnInterfaceName.Protocols,
method : DwnMethodName.Configure,
messageTimestamp : Time.getCurrentTimestamp(),
definition : protocolDefinitionWithoutQuery
};

authorization = await Message.createAuthorization({
descriptor,
signer: Jws.createSigner(alice)
});
protocolsConfigureMessage = { descriptor, authorization };

const withoutQueryResponse = await dwn.processMessage(alice.did, protocolsConfigureMessage);
expect(withoutQueryResponse.status.code).to.equal(400);

// without 'read' action
const protocolDefinitionWithoutRead: ProtocolDefinition = {
protocol : 'http://foo',
published : true,
types : {
friend : {},
foo : {},
},
structure: {
friend: {
$role: true
},
foo: {
$actions: [
{
role : 'friend',
can : [ProtocolAction.Query, ProtocolAction.Subscribe] // missing `read`
}
]
}
}
};

descriptor = {
interface : DwnInterfaceName.Protocols,
method : DwnMethodName.Configure,
messageTimestamp : Time.getCurrentTimestamp(),
definition : protocolDefinitionWithoutRead
};

authorization = await Message.createAuthorization({
descriptor,
signer: Jws.createSigner(alice)
});

protocolsConfigureMessage = { descriptor, authorization };

const withoutReadResponse = await dwn.processMessage(alice.did, protocolsConfigureMessage);
expect(withoutReadResponse.status.code).to.equal(400);


// sanity, all read actions exist
const protocolDefinitionWithAllReadActions: ProtocolDefinition = {
protocol : 'http://foo',
published : true,
types : {
friend : {},
foo : {},
},
structure: {
friend: {
$role: true
},
foo: {
$actions: [
{
role : 'friend',
can : [ProtocolAction.Read, ProtocolAction.Query, ProtocolAction.Subscribe]
}
]
}
}
};

const withAllReadActions = await TestDataGenerator.generateProtocolsConfigure({
author : alice,
protocolDefinition : protocolDefinitionWithAllReadActions,
});

const withAllReadActionsResponse = await dwn.processMessage(alice.did, withAllReadActions.message);
expect(withAllReadActionsResponse.status.code).to.equal(202);
});

describe('Grant authorization', () => {
it('allows an external party to ProtocolsConfigure only if they have a valid grant', async () => {
// scenario:
Expand Down
125 changes: 124 additions & 1 deletion tests/interfaces/protocols-configure.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import chaiAsPromised from 'chai-as-promised';
import chai, { expect } from 'chai';

import type { ProtocolsConfigureDescriptor, ProtocolsConfigureMessage } from '../../src/index.js';
import type { ProtocolDefinition, ProtocolsConfigureDescriptor, ProtocolsConfigureMessage } from '../../src/index.js';

import dexProtocolDefinition from '../vectors/protocol-definitions/dex.json' assert { type: 'json' };
import { Jws } from '../../src/utils/jws.js';
Expand Down Expand Up @@ -233,6 +233,129 @@ describe('ProtocolsConfigure', () => {
.to.be.rejectedWith(DwnErrorCode.ProtocolsConfigureRoleDoesNotExistAtGivenPath);
});

it('should reject protocol definitions with `role` actions that contain a read action(`read` || `query` || `subscribe`) but not all of the read actions', async () => {
const alice = await TestDataGenerator.generateDidKeyPersona();

// without 'subscribe' action
const protocolDefinitionWithoutSubscribe: ProtocolDefinition = {
protocol : 'http://foo',
published : true,
types : {
friend : {},
foo : {},
},
structure: {
friend: {
$role: true
},
foo: {
$actions: [
{
role : 'friend',
can : [ProtocolAction.Read, ProtocolAction.Query] // missing `subscribe`
}
]
}
}
};

const withoutSubscribePromise = TestDataGenerator.generateProtocolsConfigure({
author : alice,
protocolDefinition : protocolDefinitionWithoutSubscribe,
});

await expect(withoutSubscribePromise).to.be.rejectedWith(DwnErrorCode.ProtocolsConfigureRoleReadActionMissing);

// without 'query' action
const protocolDefinitionWithoutQuery: ProtocolDefinition = {
protocol : 'http://foo',
published : true,
types : {
friend : {},
foo : {},
},
structure: {
friend: {
$role: true
},
foo: {
$actions: [
{
role : 'friend',
can : [ProtocolAction.Read, ProtocolAction.Subscribe] // missing `query`
}
]
}
}
};

const withoutQueryPromise = TestDataGenerator.generateProtocolsConfigure({
author : alice,
protocolDefinition : protocolDefinitionWithoutQuery,
});

await expect(withoutQueryPromise).to.be.rejectedWith(DwnErrorCode.ProtocolsConfigureRoleReadActionMissing);

// without 'read' action
const protocolDefinitionWithoutRead: ProtocolDefinition = {
protocol : 'http://foo',
published : true,
types : {
friend : {},
foo : {},
},
structure: {
friend: {
$role: true
},
foo: {
$actions: [
{
role : 'friend',
can : [ProtocolAction.Query, ProtocolAction.Subscribe] // missing `read`
}
]
}
}
};

const withoutReadPromise = TestDataGenerator.generateProtocolsConfigure({
author : alice,
protocolDefinition : protocolDefinitionWithoutRead,
});

await expect(withoutReadPromise).to.be.rejectedWith(DwnErrorCode.ProtocolsConfigureRoleReadActionMissing);

// sanity, all read actions exist
const protocolDefinitionWithAllReadActions: ProtocolDefinition = {
protocol : 'http://foo',
published : true,
types : {
friend : {},
foo : {},
},
structure: {
friend: {
$role: true
},
foo: {
$actions: [
{
role : 'friend',
can : [ProtocolAction.Read, ProtocolAction.Query, ProtocolAction.Subscribe]
}
]
}
}
};

const withAllReadActions = await TestDataGenerator.generateProtocolsConfigure({
author : alice,
protocolDefinition : protocolDefinitionWithAllReadActions,
});
expect(withAllReadActions.message.descriptor.definition).to.deep.equal(protocolDefinitionWithAllReadActions);
});

it('rejects protocol definitions with actions that contain `of` and `who` is `anyone`', async () => {
const definition = {
published : true,
Expand Down
2 changes: 1 addition & 1 deletion tests/vectors/protocol-definitions/friend-role.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
{
"role": "fan",
"can": [
"read"
"read", "query", "subscribe"
]
},
{
Expand Down
8 changes: 6 additions & 2 deletions tests/vectors/protocol-definitions/slack.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
{
"role": "community/admin",
"can": [
"read"
"read", "query", "subscribe"
]
}
],
Expand Down Expand Up @@ -156,14 +156,16 @@
"can": [
"create",
"update",
"read",
"query",
"subscribe",
"co-delete"
]
},
{
"role": "community/gatedChannel/participant",
"can": [
"read"
"read", "query", "subscribe"
]
}
],
Expand Down Expand Up @@ -203,6 +205,8 @@
"create",
"update",
"query",
"read",
"subscribe",
"co-delete"
]
}
Expand Down
4 changes: 3 additions & 1 deletion tests/vectors/protocol-definitions/thread-role.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
{
"role": "thread/participant",
"can": [
"read"
"read", "query", "subscribe"
]
}
],
Expand All @@ -31,6 +31,8 @@
"role": "thread/participant",
"can": [
"read",
"query",
"subscribe",
"create"
]
}
Expand Down

0 comments on commit 7b02581

Please sign in to comment.