#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

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"
+                  ]
+                }
               }
             }
           }

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',

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<void> {
     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<void> {
-    if (!inboundMessageRuleSet.$role) {
+    if (!ruleSet.$role) {
       return;
     }
 

src/interfaces/protocols-configure.ts

@@ -144,52 +144,90 @@ export class ProtocolsConfigure extends AbstractMessage<ProtocolsConfigureMessag
       }
     }
 
-    // Validate $actions in the rule set
-    const actions = ruleSet.$actions ?? [];
-    for (const action of actions) {
+    // validate each action rule
+    const actionRules = ruleSet.$actions ?? [];
+    for (let i = 0; i < actionRules.length; i++) {
+      const actionRule = actionRules[i];
+
       // Validate the `role` property of an `action` if exists.
-      if (action.role !== undefined) {
+      if (actionRule.role !== undefined) {
         // make sure the role contains a valid protocol paths to a role record
-        if (!roles.includes(action.role)) {
+        if (!roles.includes(actionRule.role)) {
           throw new DwnError(
             DwnErrorCode.ProtocolsConfigureRoleDoesNotExistAtGivenPath,
-            `Role in action ${JSON.stringify(action)} for rule set ${ruleSetProtocolPath} does not exist.`
+            `Role in action ${JSON.stringify(actionRule)} for rule set ${ruleSetProtocolPath} does not exist.`
           );
         }
       }
 
       // Validate that if `who` is set to `anyone` then `of` is not set
-      if (action.who === 'anyone' && action.of) {
+      if (actionRule.who === 'anyone' && actionRule.of) {
         throw new DwnError(
           DwnErrorCode.ProtocolsConfigureInvalidActionOfNotAllowed,
           `'of' is not allowed at rule set protocol path (${ruleSetProtocolPath})`
         );
       }
 
-      // Validate that if `who === recipient` and `of === undefined`, then `can` is either `co-delete` or `co-update`
-      // We will allow `read`, `write`, or `query` because:
+      // Validate that if `who === recipient` and `of === undefined`, then `can` can only contain `co-delete` and `co-update`
+      // We do not allow `read`, `write`, or `query` in the `can` array because:
       // - `read` - Recipients are always allowed to `read`.
       // - `write` - Entails ability to create and update.
       //             Since `of` is undefined, it implies the recipient of THIS record,
       //             there is no 'recipient' until this record has been created, so it makes no sense to allow recipient to write this record.
       // - `query` - Only authorized using roles, so allowing direct recipients to query is outside the scope.
-      if (action.who === ProtocolActor.Recipient &&
-          action.of === undefined &&
-          ![ProtocolAction.CoUpdate, ProtocolAction.CoDelete].includes(action.can as ProtocolAction)
-      ) {
-        throw new DwnError(
-          DwnErrorCode.ProtocolsConfigureInvalidRecipientOfAction,
-          'Rules for `recipient` without `of` property must have `can` === `co-delete` or `co-update`'
-        );
+      if (actionRule.who === ProtocolActor.Recipient && actionRule.of === undefined) {
+
+        // throw if `can` contains a value that is not `co-update` or `co-delete`
+        if (actionRule.can.some((allowedAction) => ![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