From e316beabc84239a56b6259157c8dba305d5d88c2 Mon Sep 17 00:00:00 2001 From: Henry Tsai Date: Fri, 8 Mar 2024 10:09:43 -0800 Subject: [PATCH] #700 - Changed protocol 'can' property to an array (#702) 1. Changed protocol 'can' property to an array 2. Disallowed protocol update action without create 3. Disallowed duplicated actors or roles within a protocol rule set 4. Various renames to improve code readability --- .../interface-methods/protocol-rule-set.json | 48 +++--- src/core/dwn-error.ts | 3 + src/core/protocol-authorization.ts | 59 +++---- src/interfaces/protocols-configure.ts | 74 ++++++--- src/types/protocols-types.ts | 10 +- tests/features/protocol-create-action.spec.ts | 8 +- tests/features/protocol-update-action.spec.ts | 100 ++++++++---- tests/handlers/protocols-configure.spec.ts | 148 +++++++++++++++++- tests/interfaces/protocols-configure.spec.ts | 18 +-- .../protocols/protocols-configure.spec.ts | 6 +- .../anyone-collaborate.json | 14 +- .../protocol-definitions/author-can.json | 14 +- tests/vectors/protocol-definitions/chat.json | 26 ++- .../contribution-reward.json | 10 +- .../credential-issuance.json | 10 +- tests/vectors/protocol-definitions/dex.json | 14 +- tests/vectors/protocol-definitions/email.json | 26 ++- .../protocol-definitions/free-for-all.json | 14 +- .../protocol-definitions/friend-role.json | 32 ++-- .../vectors/protocol-definitions/message.json | 4 +- .../protocol-definitions/post-comment.json | 17 +- .../protocol-definitions/recipient-can.json | 19 +-- tests/vectors/protocol-definitions/slack.json | 130 ++++++++------- .../protocol-definitions/social-media.json | 33 ++-- .../protocol-definitions/thread-role.json | 45 +++--- 25 files changed, 575 insertions(+), 307 deletions(-) diff --git a/json-schemas/interface-methods/protocol-rule-set.json b/json-schemas/interface-methods/protocol-rule-set.json index 0c4784b29..a66256394 100644 --- a/json-schemas/interface-methods/protocol-rule-set.json +++ b/json-schemas/interface-methods/protocol-rule-set.json @@ -41,15 +41,19 @@ "type": "string" }, "can": { - "type": "string", - "enum": [ - "co-delete", - "co-update", - "create", - "read", - "update", - "write" - ] + "type": "array", + "minItems": 1, + "items": { + "type": "string", + "enum": [ + "co-delete", + "co-update", + "create", + "read", + "update", + "write" + ] + } } } }, @@ -64,17 +68,21 @@ "type": "string" }, "can": { - "type": "string", - "enum": [ - "co-delete", - "co-update", - "create", - "query", - "subscribe", - "read", - "update", - "write" - ] + "type": "array", + "minItems": 1, + "items": { + "type": "string", + "enum": [ + "co-delete", + "co-update", + "create", + "query", + "subscribe", + "read", + "update", + "write" + ] + } } } } diff --git a/src/core/dwn-error.ts b/src/core/dwn-error.ts index a757d7957..e6937fee1 100644 --- a/src/core/dwn-error.ts +++ b/src/core/dwn-error.ts @@ -76,9 +76,12 @@ export enum DwnErrorCode { ProtocolAuthorizationProtocolNotFound = 'ProtocolAuthorizationProtocolNotFound', ProtocolAuthorizationQueryWithoutRole = 'ProtocolAuthorizationQueryWithoutRole', ProtocolAuthorizationRoleMissingRecipient = 'ProtocolAuthorizationRoleMissingRecipient', + ProtocolsConfigureDuplicateActorInRuleSet = 'ProtocolsConfigureDuplicateActorInRuleSet', + ProtocolsConfigureDuplicateRoleInRuleSet = 'ProtocolsConfigureDuplicateRoleInRuleSet', ProtocolsConfigureInvalidSize = 'ProtocolsConfigureInvalidSize', ProtocolsConfigureInvalidActionMissingOf = 'ProtocolsConfigureInvalidActionMissingOf', ProtocolsConfigureInvalidActionOfNotAllowed = 'ProtocolsConfigureInvalidActionOfNotAllowed', + ProtocolsConfigureInvalidActionUpdateWithoutCreate = 'ProtocolsConfigureInvalidActionUpdateWithoutCreate', ProtocolsConfigureInvalidRecipientOfAction = 'ProtocolsConfigureInvalidRecipientOfAction', ProtocolsConfigureInvalidRuleSetRecordType = 'ProtocolsConfigureInvalidRuleSetRecordType', ProtocolsConfigureQueryNotAllowed = 'ProtocolsConfigureQueryNotAllowed', diff --git a/src/core/protocol-authorization.ts b/src/core/protocol-authorization.ts index bf88f917a..eddde0383 100644 --- a/src/core/protocol-authorization.ts +++ b/src/core/protocol-authorization.ts @@ -46,7 +46,7 @@ export class ProtocolAuthorization { ); // get the rule set for the inbound message - const inboundMessageRuleSet = ProtocolAuthorization.getRuleSet( + const ruleSet = ProtocolAuthorization.getRuleSet( incomingMessage.message.descriptor.protocolPath!, protocolDefinition, ); @@ -55,12 +55,12 @@ export class ProtocolAuthorization { await ProtocolAuthorization.verifyAsRoleRecordIfNeeded( tenant, incomingMessage, - inboundMessageRuleSet, + ruleSet, messageStore, ); // Verify size limit - ProtocolAuthorization.verifySizeLimit(incomingMessage, inboundMessageRuleSet); + ProtocolAuthorization.verifySizeLimit(incomingMessage, ruleSet); } /** @@ -84,7 +84,7 @@ export class ProtocolAuthorization { ); // get the rule set for the inbound message - const inboundMessageRuleSet = ProtocolAuthorization.getRuleSet( + const ruleSet = ProtocolAuthorization.getRuleSet( incomingMessage.message.descriptor.protocolPath!, protocolDefinition, ); @@ -99,11 +99,11 @@ export class ProtocolAuthorization { messageStore, ); - // verify method invoked against the allowed actions - await ProtocolAuthorization.verifyAllowedActions( + // verify method invoked against the allowed actions in the rule set + await ProtocolAuthorization.authorizeAgainstAllowedActions( tenant, incomingMessage, - inboundMessageRuleSet, + ruleSet, ancestorMessageChain, messageStore, ); @@ -133,7 +133,7 @@ export class ProtocolAuthorization { ); // get the rule set for the inbound message - const inboundMessageRuleSet = ProtocolAuthorization.getRuleSet( + const ruleSet = ProtocolAuthorization.getRuleSet( newestRecordsWrite.message.descriptor.protocolPath!, protocolDefinition, ); @@ -148,11 +148,11 @@ export class ProtocolAuthorization { messageStore, ); - // verify method invoked against the allowed actions - await ProtocolAuthorization.verifyAllowedActions( + // verify method invoked against the allowed actions in the rule set + await ProtocolAuthorization.authorizeAgainstAllowedActions( tenant, incomingMessage, - inboundMessageRuleSet, + ruleSet, ancestorMessageChain, messageStore, ); @@ -173,7 +173,7 @@ export class ProtocolAuthorization { ); // get the rule set for the inbound message - const inboundMessageRuleSet = ProtocolAuthorization.getRuleSet( + const ruleSet = ProtocolAuthorization.getRuleSet( protocolPath!, // presence of `protocolPath` is verified in `parse()` protocolDefinition, ); @@ -188,11 +188,11 @@ export class ProtocolAuthorization { messageStore, ); - // verify method invoked against the allowed actions - await ProtocolAuthorization.verifyAllowedActions( + // verify method invoked against the allowed actions in the rule set + await ProtocolAuthorization.authorizeAgainstAllowedActions( tenant, incomingMessage, - inboundMessageRuleSet, + ruleSet, [], // ancestor chain is not relevant to subscriptions messageStore, ); @@ -217,7 +217,7 @@ export class ProtocolAuthorization { ); // get the rule set for the inbound message - const inboundMessageRuleSet = ProtocolAuthorization.getRuleSet( + const ruleSet = ProtocolAuthorization.getRuleSet( newestRecordsWrite.message.descriptor.protocolPath!, protocolDefinition, ); @@ -232,11 +232,11 @@ export class ProtocolAuthorization { messageStore, ); - // verify method invoked against the allowed actions - await ProtocolAuthorization.verifyAllowedActions( + // verify method invoked against the allowed actions in the rule set + await ProtocolAuthorization.authorizeAgainstAllowedActions( tenant, incomingMessage, - inboundMessageRuleSet, + ruleSet, ancestorMessageChain, messageStore, ); @@ -355,6 +355,7 @@ export class ProtocolAuthorization { `Declared protocol path '${declaredProtocolPath}' is not valid for records with no parentId'.` ); } + return; } @@ -557,20 +558,20 @@ export class ProtocolAuthorization { } /** - * Verifies the action (e.g. read/write) specified in the given message matches the allowed actions in the rule set. + * Verifies the given message is authorized by one of the action rules in the given protocol rule set. * @throws {Error} if action not allowed. */ - private static async verifyAllowedActions( + private static async authorizeAgainstAllowedActions( tenant: string, incomingMessage: RecordsDelete | RecordsQuery | RecordsRead | RecordsSubscribe | RecordsWrite, - inboundMessageRuleSet: ProtocolRuleSet, + ruleSet: ProtocolRuleSet, ancestorMessageChain: RecordsWriteMessage[], messageStore: MessageStore, ): Promise { const incomingMessageMethod = incomingMessage.message.descriptor.method; const actionsSeekingARuleMatch = await ProtocolAuthorization.getActionsSeekingARuleMatch(tenant, incomingMessage, messageStore); const author = incomingMessage.author; - const actionRules = inboundMessageRuleSet.$actions; + const actionRules = ruleSet.$actions; // We have already checked that the message is not from tenant, owner, or permissionsGrant if (actionRules === undefined) { @@ -583,7 +584,9 @@ export class ProtocolAuthorization { const invokedRole = incomingMessage.signaturePayload?.protocolRole; for (const actionRule of actionRules) { - if (!actionsSeekingARuleMatch.includes(actionRule.can as ProtocolAction)) { + // If the action rule does not have an allowed action that matches an action that can authorize the message, skip to evaluate next action rule. + const ruleHasAMatchingAllowedAction = actionRule.can.some(allowedAction => actionsSeekingARuleMatch.includes(allowedAction as ProtocolAction)); + if (!ruleHasAMatchingAllowedAction) { continue; } @@ -634,9 +637,9 @@ export class ProtocolAuthorization { */ private static verifySizeLimit( incomingMessage: RecordsWrite, - inboundMessageRuleSet: ProtocolRuleSet + ruleSet: ProtocolRuleSet ): void { - const { min = 0, max } = inboundMessageRuleSet.$size || {}; + const { min = 0, max } = ruleSet.$size || {}; const dataSize = incomingMessage.message.descriptor.dataSize; @@ -662,10 +665,10 @@ export class ProtocolAuthorization { private static async verifyAsRoleRecordIfNeeded( tenant: string, incomingMessage: RecordsWrite, - inboundMessageRuleSet: ProtocolRuleSet, + ruleSet: ProtocolRuleSet, messageStore: MessageStore, ): Promise { - if (!inboundMessageRuleSet.$role) { + if (!ruleSet.$role) { return; } diff --git a/src/interfaces/protocols-configure.ts b/src/interfaces/protocols-configure.ts index b8123f389..3da922842 100644 --- a/src/interfaces/protocols-configure.ts +++ b/src/interfaces/protocols-configure.ts @@ -144,52 +144,90 @@ export class ProtocolsConfigure extends AbstractMessage ![ProtocolAction.CoUpdate, ProtocolAction.CoDelete].includes(allowedAction as ProtocolAction))) { + throw new DwnError( + DwnErrorCode.ProtocolsConfigureInvalidRecipientOfAction, + 'Rules for `recipient` without `of` property must have `can` containing only `co-update` or `co-delete`' + ); + } } // Validate that if `who` is set to `author` then `of` is set - if (action.who === ProtocolActor.Author && !action.of) { + if (actionRule.who === ProtocolActor.Author && !actionRule.of) { throw new DwnError( DwnErrorCode.ProtocolsConfigureInvalidActionMissingOf, `'of' is required when 'author' is specified as 'who'` ); } + + // validate that if can contains `update`, it must also contain `create` + if (actionRule.can !== undefined) { + if (actionRule.can.includes(ProtocolAction.Update) && !actionRule.can.includes(ProtocolAction.Create)) { + throw new DwnError( + DwnErrorCode.ProtocolsConfigureInvalidActionUpdateWithoutCreate, + `Action rule ${JSON.stringify(actionRule)} contains 'update' action but missing the required 'create' action.` + ); + } + } + + // Validate that there are no duplicate actors or roles in the remaining action rules: + // ie. no two action rules can have the same combination of `who` + `of` or `role`. + // NOTE: we only need to check the remaining action rules that have yet to go through action rule validation loop, as a perf shortcut. + for (let j = i + 1; j < actionRules.length; j++) { + const otherActionRule = actionRules[j]; + + if (actionRule.who !== undefined) { + if (actionRule.who === otherActionRule.who && actionRule.of === otherActionRule.of) { + throw new DwnError( + DwnErrorCode.ProtocolsConfigureDuplicateActorInRuleSet, + `More than one action rule per actor ${actionRule.who} of ${actionRule.of} not allowed within a rule set: ${JSON.stringify(actionRule)}` + ); + } + } else { + // else implicitly a role-based action rule + + if (actionRule.role === otherActionRule.role) { + throw new DwnError( + DwnErrorCode.ProtocolsConfigureDuplicateRoleInRuleSet, + `More than one action rule per role ${actionRule.role} not allowed within a rule set: ${JSON.stringify(actionRule)}` + ); + } + } + } } // Validate nested rule sets diff --git a/src/types/protocols-types.ts b/src/types/protocols-types.ts index e557eeefb..8543516ad 100644 --- a/src/types/protocols-types.ts +++ b/src/types/protocols-types.ts @@ -53,7 +53,7 @@ export enum ProtocolAction { * 1. Anyone can write. * { * who: 'anyone', - * can: 'write + * can: ['write'] * } * * 2. Author of protocolPath can write; OR @@ -61,13 +61,13 @@ export enum ProtocolAction { * { * who: 'recipient' * of: 'requestForQuote', - * can: 'write' + * can: ['write'] * } * * 3. Role can write. * { * role: 'friend', - * can: 'write' + * can: ['write'] * } */ export type ProtocolActionRule = { @@ -91,11 +91,11 @@ export type ProtocolActionRule = { of?: string; /** - * Action that the actor can perform. + * Array of actions that the actor/role can perform. * See {ProtocolAction} for possible values. * 'query' and 'subscribe' are only supported for `role` rules. */ - can: string; + can: string[]; }; /** * Config for protocol-path encryption scheme. diff --git a/tests/features/protocol-create-action.spec.ts b/tests/features/protocol-create-action.spec.ts index 0ce2cb7aa..22261bcdd 100644 --- a/tests/features/protocol-create-action.spec.ts +++ b/tests/features/protocol-create-action.spec.ts @@ -89,7 +89,7 @@ export function testProtocolCreateAction(): void { $actions: [ { role : 'admin', - can : ProtocolAction.Create + can : [ProtocolAction.Create] }, ], bar: { @@ -97,19 +97,19 @@ export function testProtocolCreateAction(): void { { who : 'author', of : 'foo', - can : ProtocolAction.Create + can : [ProtocolAction.Create] }, { who : 'recipient', of : 'foo', - can : ProtocolAction.Create + can : [ProtocolAction.Create] } ], baz: { $actions: [ { who : 'anyone', - can : ProtocolAction.Create + can : [ProtocolAction.Create] } ], } diff --git a/tests/features/protocol-update-action.spec.ts b/tests/features/protocol-update-action.spec.ts index 98659cf0c..b77dd1c2a 100644 --- a/tests/features/protocol-update-action.spec.ts +++ b/tests/features/protocol-update-action.spec.ts @@ -1,6 +1,6 @@ import type { EventStream } from '../../src/types/subscriptions.js'; -import type { ProtocolDefinition } from '../../src/types/protocols-types.js'; import type { DataStore, EventLog, MessageStore } from '../../src/index.js'; +import type { ProtocolDefinition, ProtocolsConfigureDescriptor } from '../../src/types/protocols-types.js'; import chaiAsPromised from 'chai-as-promised'; import sinon from 'sinon'; @@ -17,7 +17,7 @@ import { RecordsWrite } from '../../src/interfaces/records-write.js'; import { TestDataGenerator } from '../utils/test-data-generator.js'; import { TestEventStream } from '../test-event-stream.js'; import { TestStores } from '../test-stores.js'; -import { DwnErrorCode, ProtocolsConfigure } from '../../src/index.js'; +import { DwnErrorCode, DwnInterfaceName, DwnMethodName, Message, ProtocolsConfigure, Time } from '../../src/index.js'; chai.use(chaiAsPromised); @@ -88,11 +88,7 @@ export function testProtocolUpdateAction(): void { $actions: [ { role : 'user', - can : ProtocolAction.Create - }, - { - role : 'user', - can : ProtocolAction.Update + can : [ProtocolAction.Create, ProtocolAction.Update] } ] } @@ -250,12 +246,7 @@ export function testProtocolUpdateAction(): void { { who : 'recipient', of : 'foo', - can : ProtocolAction.Create - }, - { - who : 'recipient', - of : 'foo', - can : ProtocolAction.Update + can : [ProtocolAction.Create, ProtocolAction.Update] } ], baz: { @@ -263,12 +254,7 @@ export function testProtocolUpdateAction(): void { { who : 'author', of : 'foo/bar', - can : ProtocolAction.Create - }, - { - who : 'author', - of : 'foo/bar', - can : ProtocolAction.Update + can : [ProtocolAction.Create, ProtocolAction.Update] } ] } @@ -476,11 +462,7 @@ export function testProtocolUpdateAction(): void { $actions: [ { who : 'anyone', - can : ProtocolAction.Create - }, - { - who : 'anyone', - can : ProtocolAction.Update + can : [ProtocolAction.Create, ProtocolAction.Update] } ] } @@ -565,10 +547,72 @@ export function testProtocolUpdateAction(): void { expect(bobUnauthorizedFooUpdateReply.status.code).to.equal(401); expect(bobUnauthorizedFooUpdateReply.status.detail).to.contain(DwnErrorCode.ProtocolAuthorizationActionNotAllowed); }); - }); - xit('should not allow `update` action without `create` action', async () => { - // TODO: pending https://github.com/TBD54566975/dwn-sdk-js/issues/700: - // Change the can property type in protocol definition schema to an array #700 + it('should not allow creation of a protocol definition with action rule containing `update` without `create`', async () => { + const protocolDefinition: ProtocolDefinition = { + protocol : 'foo', + published : true, + types : { + foo: {}, + }, + structure: { + foo: { + $actions: [ + { + who : 'anyone', + can : [ProtocolAction.Update] // intentionally missing `create` action + } + ] + } + } + }; + + const alice = await TestDataGenerator.generateDidKeyPersona(); + const protocolsConfigureCreatePromise = ProtocolsConfigure.create({ + definition : protocolDefinition, + signer : Jws.createSigner(alice) + }); + + await expect(protocolsConfigureCreatePromise).to.be.rejectedWith(DwnErrorCode.ProtocolsConfigureInvalidActionUpdateWithoutCreate); + }); + + it('should reject ProtocolsConfigure with action rule containing `update` action without `create` during processing', async () => { + const protocolDefinition: ProtocolDefinition = { + protocol : 'http://foo', + published : true, + types : { + foo: {}, + }, + structure: { + foo: { + $actions: [ + { + who : 'anyone', + can : [ProtocolAction.Update] // intentionally missing `create` action + } + ] + } + } + }; + + // manually craft the invalid ProtocolsConfigure message because our library will not let you create an invalid definition + const descriptor: ProtocolsConfigureDescriptor = { + interface : DwnInterfaceName.Protocols, + method : DwnMethodName.Configure, + messageTimestamp : Time.getCurrentTimestamp(), + definition : protocolDefinition + }; + + const alice = await TestDataGenerator.generateDidKeyPersona(); + const authorization = await Message.createAuthorization({ + descriptor, + signer: Jws.createSigner(alice) + }); + const protocolsConfigureMessage = { descriptor, authorization }; + + const protocolsConfigureReply = await dwn.processMessage(alice.did, protocolsConfigureMessage); + expect(protocolsConfigureReply.status.code).to.equal(400); + expect(protocolsConfigureReply.status.detail).to.contain(DwnErrorCode.ProtocolsConfigureInvalidActionUpdateWithoutCreate); + }); }); } \ No newline at end of file diff --git a/tests/handlers/protocols-configure.spec.ts b/tests/handlers/protocols-configure.spec.ts index 972305588..83c0462aa 100644 --- a/tests/handlers/protocols-configure.spec.ts +++ b/tests/handlers/protocols-configure.spec.ts @@ -4,7 +4,9 @@ import type { GenerateProtocolsConfigureOutput } from '../utils/test-data-genera import type { DataStore, EventLog, - MessageStore + MessageStore, + ProtocolDefinition, + ProtocolsConfigureDescriptor } from '../../src/index.js'; import chaiAsPromised from 'chai-as-promised'; @@ -17,6 +19,7 @@ import minimalProtocolDefinition from '../vectors/protocol-definitions/minimal.j import { GeneralJwsBuilder } from '../../src/jose/jws/general/builder.js'; import { lexicographicalCompare } from '../../src/utils/string.js'; import { Message } from '../../src/core/message.js'; +import { ProtocolAction } from '../../src/types/protocols-types.js'; import { TestDataGenerator } from '../utils/test-data-generator.js'; import { TestEventStream } from '../test-event-stream.js'; import { TestStores } from '../test-stores.js'; @@ -24,7 +27,7 @@ import { TestStubGenerator } from '../utils/test-stub-generator.js'; import { Time } from '../../src/utils/time.js'; import { DidKey, DidResolver } from '@web5/dids'; -import { Dwn, DwnErrorCode, Encoder, Jws } from '../../src/index.js'; +import { Dwn, DwnErrorCode, DwnInterfaceName, DwnMethodName, Encoder, Jws } from '../../src/index.js'; chai.use(chaiAsPromised); @@ -289,6 +292,147 @@ export function testProtocolsConfigureHandler(): void { expect(protocolsConfigureReply.status.detail).to.contain(DwnErrorCode.AuthorizationAuthorNotOwner); }); + it('should reject ProtocolsConfigure with action rule containing duplicated actor (`who` or `who` + `of` combination) within a rule set', async () => { + const alice = await TestDataGenerator.generateDidKeyPersona(); + + const protocolDefinition: ProtocolDefinition = { + protocol : 'http://foo-bar', + published : true, + types : { + foo: {}, + }, + structure: { + foo: { + $actions: [ + { + who : 'anyone', + can : [ProtocolAction.Create] + }, + // duplicated `who` value + { + who : 'anyone', + can : [ProtocolAction.Update] + } + ] + } + } + }; + + // manually craft the invalid ProtocolsConfigure message because our library will not let you create an invalid definition + const descriptor: ProtocolsConfigureDescriptor = { + interface : DwnInterfaceName.Protocols, + method : DwnMethodName.Configure, + messageTimestamp : Time.getCurrentTimestamp(), + definition : protocolDefinition + }; + + const authorization = await Message.createAuthorization({ + descriptor, + signer: Jws.createSigner(alice) + }); + const protocolsConfigureMessage = { descriptor, authorization }; + + const protocolsConfigureReply = await dwn.processMessage(alice.did, protocolsConfigureMessage); + expect(protocolsConfigureReply.status.code).to.equal(400); + expect(protocolsConfigureReply.status.detail).to.contain(DwnErrorCode.ProtocolsConfigureDuplicateActorInRuleSet); + + + // similar test as above but with `of` property + const protocolDefinition2: ProtocolDefinition = { + protocol : 'http://foo-bar', + published : true, + types : { + foo : {}, + bar : {}, + }, + structure: { + foo: { + bar: { + $actions: [ + { + who : 'recipient', + of : 'foo', + can : [ProtocolAction.Create] + }, + // duplicated `who` value + { + who : 'recipient', + of : 'foo', + can : [ProtocolAction.Update] + } + ] + } + } + } + }; + + const descriptor2: ProtocolsConfigureDescriptor = { + interface : DwnInterfaceName.Protocols, + method : DwnMethodName.Configure, + messageTimestamp : Time.getCurrentTimestamp(), + definition : protocolDefinition2 + }; + + const authorization2 = await Message.createAuthorization({ + descriptor : descriptor2, + signer : Jws.createSigner(alice) + }); + const protocolsConfigureMessage2 = { descriptor: descriptor2, authorization: authorization2 }; + + const protocolsConfigure2Reply = await dwn.processMessage(alice.did, protocolsConfigureMessage2); + expect(protocolsConfigure2Reply.status.code).to.equal(400); + expect(protocolsConfigure2Reply.status.detail).to.contain(DwnErrorCode.ProtocolsConfigureDuplicateActorInRuleSet); + }); + + it('should reject ProtocolsConfigure with action rule containing duplicated role within a rule set', async () => { + const alice = await TestDataGenerator.generateDidKeyPersona(); + + const protocolDefinition: ProtocolDefinition = { + protocol : 'http://foo', + published : true, + types : { + user : {}, + foo : {}, + }, + structure: { + user: { + $role: true + }, + foo: { + $actions: [ + { + role : 'user', + can : [ProtocolAction.Create] + }, + // duplicated `role` value + { + role : 'user', + can : [ProtocolAction.Update] + } + ] + } + } + }; + + // manually craft the invalid ProtocolsConfigure message because our library will not let you create an invalid definition + const descriptor: ProtocolsConfigureDescriptor = { + interface : DwnInterfaceName.Protocols, + method : DwnMethodName.Configure, + messageTimestamp : Time.getCurrentTimestamp(), + definition : protocolDefinition + }; + + const authorization = await Message.createAuthorization({ + descriptor, + signer: Jws.createSigner(alice) + }); + const protocolsConfigureMessage = { descriptor, authorization }; + + const protocolsConfigureReply = await dwn.processMessage(alice.did, protocolsConfigureMessage); + expect(protocolsConfigureReply.status.code).to.equal(400); + expect(protocolsConfigureReply.status.detail).to.contain(DwnErrorCode.ProtocolsConfigureDuplicateRoleInRuleSet); + }); + describe('event log', () => { it('should add event for ProtocolsConfigure', async () => { const alice = await TestDataGenerator.generateDidKeyPersona(); diff --git a/tests/interfaces/protocols-configure.spec.ts b/tests/interfaces/protocols-configure.spec.ts index a6cc5cc71..dfc2eb5a3 100644 --- a/tests/interfaces/protocols-configure.spec.ts +++ b/tests/interfaces/protocols-configure.spec.ts @@ -143,14 +143,14 @@ describe('ProtocolsConfigure', () => { secondLevel : { $actions: [{ role : 'rootRole', // valid because 'rootRole` has $role: true - can : 'write' + can : ['write'] }] } }, firstLevel: { $actions: [{ role : 'rootRole', // valid because 'rootRole` has $role: true - can : 'write' + can : ['write'] }] } } @@ -183,7 +183,7 @@ describe('ProtocolsConfigure', () => { chat: { $actions: [{ role : 'thread/participant', // valid because 'thread/participant` has $role: true - can : 'write' + can : ['write'] }] } } @@ -215,7 +215,7 @@ describe('ProtocolsConfigure', () => { bar: { $actions: [{ role : 'foo', // foo is not a role - can : 'read' + can : ['read'] }] } } @@ -244,7 +244,7 @@ describe('ProtocolsConfigure', () => { $actions: [{ who : 'anyone', of : 'message', // Not allowed - can : 'read' + can : ['read'] }] } } @@ -272,7 +272,7 @@ describe('ProtocolsConfigure', () => { message: { $actions: [{ who : 'recipient', - can : 'read' // not allowed, should be either delete or update + can : ['read'] // not allowed, should be either delete or update }] } } @@ -301,7 +301,7 @@ describe('ProtocolsConfigure', () => { $actions: [{ who : 'author', // of : 'message', // Intentionally missing - can : 'read' + can : ['read'] }] } } @@ -318,7 +318,7 @@ describe('ProtocolsConfigure', () => { .to.be.rejectedWith(DwnErrorCode.ProtocolsConfigureInvalidActionMissingOf); }); - it('rejects protocol definitions with `can: query` in non-role rules', async () => { + it('rejects protocol definitions with `can query` in non-role rules', async () => { const definition = { published : true, protocol : 'http://example.com', @@ -330,7 +330,7 @@ describe('ProtocolsConfigure', () => { $actions: [{ who : 'author', of : 'message', - can : 'query' + can : ['query'] }] } } diff --git a/tests/validation/json-schemas/protocols/protocols-configure.spec.ts b/tests/validation/json-schemas/protocols/protocols-configure.spec.ts index 05bbdc1f3..1378a06d2 100644 --- a/tests/validation/json-schemas/protocols/protocols-configure.spec.ts +++ b/tests/validation/json-schemas/protocols/protocols-configure.spec.ts @@ -22,7 +22,7 @@ describe('ProtocolsConfigure schema definition', () => { $actions: [ { who : 'unknown', - can : 'write' + can : ['write'] } ] } @@ -50,7 +50,7 @@ describe('ProtocolsConfigure schema definition', () => { $actions: [{ who : 'author', of : 'thread', - // can: 'read' // intentionally missing + // can: ['read'] // intentionally missing }] }; @@ -58,7 +58,7 @@ describe('ProtocolsConfigure schema definition', () => { $actions: [{ who : 'recipient', of : 'thread', - // can: 'read' // intentionally missing + // can: ['read'] // intentionally missing }] }; diff --git a/tests/vectors/protocol-definitions/anyone-collaborate.json b/tests/vectors/protocol-definitions/anyone-collaborate.json index 6fee0d6c0..31c1a159a 100644 --- a/tests/vectors/protocol-definitions/anyone-collaborate.json +++ b/tests/vectors/protocol-definitions/anyone-collaborate.json @@ -9,15 +9,11 @@ "$actions": [ { "who": "anyone", - "can": "co-update" - }, - { - "who": "anyone", - "can": "read" - }, - { - "who": "anyone", - "can": "co-delete" + "can": [ + "read", + "co-update", + "co-delete" + ] } ] } diff --git a/tests/vectors/protocol-definitions/author-can.json b/tests/vectors/protocol-definitions/author-can.json index 728127d97..db635c5fe 100644 --- a/tests/vectors/protocol-definitions/author-can.json +++ b/tests/vectors/protocol-definitions/author-can.json @@ -10,7 +10,9 @@ "$actions": [ { "who": "anyone", - "can": "write" + "can": [ + "write" + ] } ], "comment": { @@ -18,12 +20,10 @@ { "who": "author", "of": "post", - "can": "co-update" - }, - { - "who": "author", - "of": "post", - "can": "co-delete" + "can": [ + "co-update", + "co-delete" + ] } ] } diff --git a/tests/vectors/protocol-definitions/chat.json b/tests/vectors/protocol-definitions/chat.json index 5fad29c03..14f72f0ec 100644 --- a/tests/vectors/protocol-definitions/chat.json +++ b/tests/vectors/protocol-definitions/chat.json @@ -20,37 +20,49 @@ "$actions": [ { "who": "anyone", - "can": "write" + "can": [ + "write" + ] }, { "who": "author", "of": "thread", - "can": "read" + "can": [ + "read" + ] }, { "who": "recipient", "of": "thread", - "can": "read" + "can": [ + "read" + ] } ], "message": { "$actions": [ { "who": "anyone", - "can": "write" + "can": [ + "write" + ] }, { "who": "author", "of": "thread/message", - "can": "read" + "can": [ + "read" + ] }, { "who": "recipient", "of": "thread/message", - "can": "read" + "can": [ + "read" + ] } ] } } } -} +} \ No newline at end of file diff --git a/tests/vectors/protocol-definitions/contribution-reward.json b/tests/vectors/protocol-definitions/contribution-reward.json index 1549db0fb..ef5c2df22 100644 --- a/tests/vectors/protocol-definitions/contribution-reward.json +++ b/tests/vectors/protocol-definitions/contribution-reward.json @@ -20,7 +20,9 @@ "$actions": [ { "who": "anyone", - "can": "write" + "can": [ + "write" + ] } ] }, @@ -29,9 +31,11 @@ { "who": "author", "of": "contribution", - "can": "read" + "can": [ + "read" + ] } ] } } -} +} \ No newline at end of file diff --git a/tests/vectors/protocol-definitions/credential-issuance.json b/tests/vectors/protocol-definitions/credential-issuance.json index eecd37407..fe7354622 100644 --- a/tests/vectors/protocol-definitions/credential-issuance.json +++ b/tests/vectors/protocol-definitions/credential-issuance.json @@ -20,7 +20,9 @@ "$actions": [ { "who": "anyone", - "can": "write" + "can": [ + "write" + ] } ], "credentialResponse": { @@ -28,10 +30,12 @@ { "who": "recipient", "of": "credentialApplication", - "can": "write" + "can": [ + "write" + ] } ] } } } -} +} \ No newline at end of file diff --git a/tests/vectors/protocol-definitions/dex.json b/tests/vectors/protocol-definitions/dex.json index b52c62e32..05f9b7c22 100644 --- a/tests/vectors/protocol-definitions/dex.json +++ b/tests/vectors/protocol-definitions/dex.json @@ -26,7 +26,9 @@ "$actions": [ { "who": "anyone", - "can": "write" + "can": [ + "write" + ] } ], "offer": { @@ -34,7 +36,9 @@ { "who": "recipient", "of": "ask", - "can": "write" + "can": [ + "write" + ] } ], "fulfillment": { @@ -42,11 +46,13 @@ { "who": "recipient", "of": "ask/offer", - "can": "write" + "can": [ + "write" + ] } ] } } } } -} +} \ No newline at end of file diff --git a/tests/vectors/protocol-definitions/email.json b/tests/vectors/protocol-definitions/email.json index 9e8061b96..1def38059 100644 --- a/tests/vectors/protocol-definitions/email.json +++ b/tests/vectors/protocol-definitions/email.json @@ -14,37 +14,49 @@ "$actions": [ { "who": "anyone", - "can": "write" + "can": [ + "write" + ] }, { "who": "author", "of": "email", - "can": "read" + "can": [ + "read" + ] }, { "who": "recipient", "of": "email", - "can": "read" + "can": [ + "read" + ] } ], "email": { "$actions": [ { "who": "anyone", - "can": "write" + "can": [ + "write" + ] }, { "who": "author", "of": "email/email", - "can": "read" + "can": [ + "read" + ] }, { "who": "recipient", "of": "email/email", - "can": "read" + "can": [ + "read" + ] } ] } } } -} +} \ No newline at end of file diff --git a/tests/vectors/protocol-definitions/free-for-all.json b/tests/vectors/protocol-definitions/free-for-all.json index 074b1ec22..41372dea0 100644 --- a/tests/vectors/protocol-definitions/free-for-all.json +++ b/tests/vectors/protocol-definitions/free-for-all.json @@ -14,15 +14,11 @@ "$actions": [ { "who": "anyone", - "can": "write" - }, - { - "who": "anyone", - "can": "co-delete" - }, - { - "who": "anyone", - "can": "read" + "can": [ + "write", + "read", + "co-delete" + ] } ] } diff --git a/tests/vectors/protocol-definitions/friend-role.json b/tests/vectors/protocol-definitions/friend-role.json index 8fd190232..d3eb55620 100644 --- a/tests/vectors/protocol-definitions/friend-role.json +++ b/tests/vectors/protocol-definitions/friend-role.json @@ -21,31 +21,25 @@ "$actions": [ { "role": "fan", - "can": "read" + "can": [ + "read" + ] }, { "role": "friend", - "can": "write" - }, - { - "role": "friend", - "can": "read" - }, - { - "role": "friend", - "can": "query" - }, - { - "role": "friend", - "can": "subscribe" - }, - { - "role": "admin", - "can": "co-update" + "can": [ + "write", + "read", + "query", + "subscribe" + ] }, { "role": "admin", - "can": "co-delete" + "can": [ + "co-update", + "co-delete" + ] } ] } diff --git a/tests/vectors/protocol-definitions/message.json b/tests/vectors/protocol-definitions/message.json index 262197712..d2a066448 100644 --- a/tests/vectors/protocol-definitions/message.json +++ b/tests/vectors/protocol-definitions/message.json @@ -14,7 +14,9 @@ "$actions": [ { "who": "anyone", - "can": "write" + "can": [ + "write" + ] } ] } diff --git a/tests/vectors/protocol-definitions/post-comment.json b/tests/vectors/protocol-definitions/post-comment.json index ba47c8364..71f0153d1 100644 --- a/tests/vectors/protocol-definitions/post-comment.json +++ b/tests/vectors/protocol-definitions/post-comment.json @@ -20,23 +20,26 @@ "$actions": [ { "who": "anyone", - "can": "read" + "can": [ + "read" + ] } ], "comment": { "$actions": [ { "who": "anyone", - "can": "read" - }, - { - "who": "anyone", - "can": "write" + "can": [ + "read", + "write" + ] }, { "who": "author", "of": "post", - "can": "co-delete" + "can": [ + "co-delete" + ] } ] } diff --git a/tests/vectors/protocol-definitions/recipient-can.json b/tests/vectors/protocol-definitions/recipient-can.json index 668455f7d..8f931b6ca 100644 --- a/tests/vectors/protocol-definitions/recipient-can.json +++ b/tests/vectors/protocol-definitions/recipient-can.json @@ -10,11 +10,10 @@ "$actions": [ { "who": "recipient", - "can": "co-update" - }, - { - "who": "recipient", - "can": "co-delete" + "can": [ + "co-update", + "co-delete" + ] } ], "tag": { @@ -22,12 +21,10 @@ { "who": "recipient", "of": "post", - "can": "co-update" - }, - { - "who": "recipient", - "of": "post", - "can": "co-delete" + "can": [ + "co-update", + "co-delete" + ] } ] } diff --git a/tests/vectors/protocol-definitions/slack.json b/tests/vectors/protocol-definitions/slack.json index 6578a6bc0..8411c4660 100644 --- a/tests/vectors/protocol-definitions/slack.json +++ b/tests/vectors/protocol-definitions/slack.json @@ -56,7 +56,9 @@ "$actions": [ { "role": "community/admin", - "can": "read" + "can": [ + "read" + ] } ], "admin": { @@ -65,20 +67,17 @@ { "who": "author", "of": "community", - "can": "write" - }, - { - "who": "author", - "of": "community", - "can": "co-delete" - }, - { - "role": "community/admin", - "can": "write" + "can": [ + "write", + "co-delete" + ] }, { "role": "community/admin", - "can": "co-delete" + "can": [ + "write", + "co-delete" + ] } ] }, @@ -87,11 +86,10 @@ "$actions": [ { "role": "community/admin", - "can": "write" - }, - { - "role": "community/admin", - "can": "co-delete" + "can": [ + "write", + "co-delete" + ] } ] }, @@ -99,11 +97,10 @@ "$actions": [ { "role": "community/admin", - "can": "write" - }, - { - "role": "community/admin", - "can": "co-delete" + "can": [ + "write", + "co-delete" + ] } ], "message": { @@ -111,15 +108,16 @@ { "who": "recipient", "of": "community/openChannel/message", - "can": "write" + "can": [ + "write" + ] }, { "role": "community/member", - "can": "write" - }, - { - "role": "community/member", - "can": "co-delete" + "can": [ + "write", + "co-delete" + ] } ], "media": { @@ -127,7 +125,9 @@ { "who": "author", "of": "community/openChannel/message", - "can": "write" + "can": [ + "write" + ] } ] }, @@ -135,11 +135,10 @@ "$actions": [ { "role": "community/member", - "can": "write" - }, - { - "role": "community/member", - "can": "co-delete" + "can": [ + "write", + "co-delete" + ] } ] } @@ -149,15 +148,16 @@ "$actions": [ { "role": "community/admin", - "can": "write" - }, - { - "role": "community/admin", - "can": "co-delete" + "can": [ + "write", + "co-delete" + ] }, { "role": "community/gatedChannel/participant", - "can": "read" + "can": [ + "read" + ] } ], "participant": { @@ -166,20 +166,17 @@ { "who": "author", "of": "community/gatedChannel", - "can": "write" - }, - { - "who": "author", - "of": "community/gatedChannel", - "can": "co-delete" + "can": [ + "write", + "co-delete" + ] }, { "role": "community/gatedChannel/participant", - "can": "write" - }, - { - "role": "community/gatedChannel/participant", - "can": "co-delete" + "can": [ + "write", + "co-delete" + ] } ] }, @@ -188,19 +185,17 @@ { "who": "recipient", "of": "community/gatedChannel/message", - "can": "write" - }, - { - "role": "community/gatedChannel/participant", - "can": "write" + "can": [ + "write" + ] }, { "role": "community/gatedChannel/participant", - "can": "query" - }, - { - "role": "community/gatedChannel/participant", - "can": "co-delete" + "can": [ + "write", + "query", + "co-delete" + ] } ], "media": { @@ -208,7 +203,9 @@ { "who": "author", "of": "community/gatedChannel/message", - "can": "write" + "can": [ + "write" + ] } ] }, @@ -216,11 +213,10 @@ "$actions": [ { "role": "community/gatedChannel/participant", - "can": "write" - }, - { - "role": "community/gatedChannel/participant", - "can": "co-delete" + "can": [ + "write", + "co-delete" + ] } ] } diff --git a/tests/vectors/protocol-definitions/social-media.json b/tests/vectors/protocol-definitions/social-media.json index 5b28d8d63..dde6ab909 100644 --- a/tests/vectors/protocol-definitions/social-media.json +++ b/tests/vectors/protocol-definitions/social-media.json @@ -34,7 +34,9 @@ "$actions": [ { "who": "anyone", - "can": "write" + "can": [ + "write" + ] } ], "reply": { @@ -42,7 +44,9 @@ { "who": "recipient", "of": "message", - "can": "write" + "can": [ + "write" + ] } ] } @@ -51,23 +55,26 @@ "$actions": [ { "who": "anyone", - "can": "read" - }, - { - "who": "anyone", - "can": "write" + "can": [ + "read", + "write" + ] } ], "caption": { "$actions": [ { "who": "anyone", - "can": "read" + "can": [ + "read" + ] }, { "who": "author", "of": "image", - "can": "write" + "can": [ + "write" + ] } ] }, @@ -76,12 +83,16 @@ { "who": "author", "of": "image", - "can": "read" + "can": [ + "read" + ] }, { "who": "recipient", "of": "image", - "can": "write" + "can": [ + "write" + ] } ] } diff --git a/tests/vectors/protocol-definitions/thread-role.json b/tests/vectors/protocol-definitions/thread-role.json index 7b1f756e4..6e2dddbe9 100644 --- a/tests/vectors/protocol-definitions/thread-role.json +++ b/tests/vectors/protocol-definitions/thread-role.json @@ -16,7 +16,9 @@ "$actions": [ { "role": "thread/participant", - "can": "read" + "can": [ + "read" + ] } ], "admin": { @@ -27,11 +29,10 @@ "$actions": [ { "role": "thread/participant", - "can": "read" - }, - { - "role": "thread/participant", - "can": "write" + "can": [ + "read", + "write" + ] } ] }, @@ -39,31 +40,25 @@ "$actions": [ { "role": "thread/participant", - "can": "read" - }, - { - "role": "thread/participant", - "can": "write" - }, - { - "role": "thread/participant", - "can": "query" - }, - { - "role": "thread/participant", - "can": "subscribe" - }, - { - "role": "thread/admin", - "can": "co-update" + "can": [ + "read", + "write", + "query", + "subscribe" + ] }, { "role": "thread/admin", - "can": "co-delete" + "can": [ + "co-update", + "co-delete" + ] }, { "role": "globalAdmin", - "can": "co-delete" + "can": [ + "co-delete" + ] } ] }