fix(permissions): add missing permission checks to procedures

src/api/procedures/__tests__/togglePauseRequirements.ts

@@ -134,9 +134,10 @@ describe('togglePauseRequirements procedure', () => {
     test('should return the appropriate roles and permissions', () => {
       const proc = procedureMockUtils.getInstance<Params, SecurityToken>(mockContext);
       const boundFunc = getAuthorization.bind(proc);
-      const args = {
+      const args: Params = {
         ticker,
-      } as Params;
+        pause: true,
+      };
 
       expect(boundFunc(args)).toEqual({
         identityRoles: [{ type: RoleType.TokenOwner, ticker }],
@@ -146,6 +147,17 @@ describe('togglePauseRequirements procedure', () => {
           portfolios: [],
         },
       });
+
+      args.pause = false;
+
+      expect(boundFunc(args)).toEqual({
+        identityRoles: [{ type: RoleType.TokenOwner, ticker }],
+        signerPermissions: {
+          transactions: [TxTags.complianceManager.ResumeAssetCompliance],
+          tokens: [entityMockUtils.getSecurityTokenInstance({ ticker })],
+          portfolios: [],
+        },
+      });
     });
   });
 });

src/api/procedures/consumeJoinIdentityAuthorization.ts

@@ -82,12 +82,13 @@ export async function getAuthorization(
 
   let transactions: TxTag[] = [];
 
-  let paidByThirdParty = false;
+  let bypassSignerPermissions = false;
 
   if (target instanceof Account) {
     const { address } = context.getCurrentAccount();
     condition = address === target.address;
-    paidByThirdParty = condition;
+    // if the current account is joining an identity, it doesn't need (and couldn't possibly have) any permissions
+    bypassSignerPermissions = condition;
   } else {
     did = await fetchDid();
     condition = did === target.did;
@@ -106,7 +107,7 @@ export async function getAuthorization(
 
   const identityRoles = condition && !authRequest.isExpired();
 
-  if (paidByThirdParty) {
+  if (bypassSignerPermissions) {
     return {
       identityRoles,
     };

src/api/procedures/modifySignerPermissions.ts

@@ -2,7 +2,7 @@ import P from 'bluebird';
 
 import { assertSecondaryKeys } from '~/api/procedures/utils';
 import { Procedure } from '~/internal';
-import { PermissionsLike, Signer } from '~/types';
+import { PermissionsLike, Signer, TxTags } from '~/types';
 import { tuple } from '~/types/utils';
 import {
   permissionsLikeToPermissions,
@@ -64,4 +64,10 @@ export async function prepareModifySignerPermissions(
 /**
  * @hidden
  */
-export const modifySignerPermissions = new Procedure(prepareModifySignerPermissions);
+export const modifySignerPermissions = new Procedure(prepareModifySignerPermissions, {
+  signerPermissions: {
+    transactions: [TxTags.identity.SetPermissionToSigner],
+    tokens: [],
+    portfolios: [],
+  },
+});

src/api/procedures/setTokenDocuments.ts

@@ -86,7 +86,7 @@ export async function prepareSetTokenDocuments(
   }
 
   if (rawDocuments.length) {
-    batchArguments(rawDocuments, TxTags.asset.BatchAddDocument).forEach(rawDocumentBatch => {
+    batchArguments(rawDocuments, TxTags.asset.AddDocuments).forEach(rawDocumentBatch => {
       this.addTransaction(
         tx.asset.addDocuments,
         { batchSize: rawDocumentBatch.length },

src/api/procedures/togglePauseRequirements.ts

@@ -54,12 +54,16 @@ export async function prepareTogglePauseRequirements(
  */
 export function getAuthorization(
   this: Procedure<Params, SecurityToken>,
-  { ticker }: Params
+  { ticker, pause }: Params
 ): ProcedureAuthorization {
   return {
     identityRoles: [{ type: RoleType.TokenOwner, ticker }],
     signerPermissions: {
-      transactions: [TxTags.complianceManager.PauseAssetCompliance],
+      transactions: [
+        pause
+          ? TxTags.complianceManager.PauseAssetCompliance
+          : TxTags.complianceManager.ResumeAssetCompliance,
+      ],
       tokens: [new SecurityToken({ ticker }, this.context)],
       portfolios: [],
     },

src/base/Procedure.ts

@@ -54,7 +54,7 @@ export class Procedure<Args extends unknown = void, ReturnValue extends unknown
   private checkAuthorization: (
     this: Procedure<Args, ReturnValue>,
     args: Args
-  ) => Promise<boolean> | boolean | Promise<ProcedureAuthorization> | ProcedureAuthorization;
+  ) => Promise<ProcedureAuthorization> | ProcedureAuthorization;
 
   private transactions: (
     | PolymeshTransaction<unknown[]>
@@ -67,7 +67,7 @@ export class Procedure<Args extends unknown = void, ReturnValue extends unknown
    * @hidden
    *
    * @param prepareTransactions - function that prepares the transaction queue
-   * @param checkRoles - can be an array of roles, a function that returns an array of roles, or a function that returns a boolean that determines whether the procedure can be executed by the current user
+   * @param checkRoles - can be a ProcedureAuthorization object or a function that returns a ProcedureAuthorization object
    */
   constructor(
     prepareTransactions: (
@@ -79,11 +79,9 @@ export class Procedure<Args extends unknown = void, ReturnValue extends unknown
       | ((
           this: Procedure<Args, ReturnValue>,
           args: Args
-        ) =>
-          | Promise<boolean>
-          | boolean
-          | Promise<ProcedureAuthorization>
-          | ProcedureAuthorization) = async (): Promise<boolean> => true
+        ) => Promise<ProcedureAuthorization> | ProcedureAuthorization) = async (): Promise<
+      ProcedureAuthorization
+    > => ({})
   ) {
     this.prepareTransactions = prepareTransactions;
 
@@ -113,36 +111,36 @@ export class Procedure<Args extends unknown = void, ReturnValue extends unknown
     this.context = context;
 
     const checkAuthorizationResult = await this.checkAuthorization(args);
-    let allowed: boolean;
 
-    if (typeof checkAuthorizationResult !== 'boolean') {
-      const { signerPermissions = true, identityRoles = true } = checkAuthorizationResult;
+    const { signerPermissions = true, identityRoles = true } = checkAuthorizationResult;
 
-      let identityAllowed: boolean;
-      if (typeof identityRoles !== 'boolean') {
-        const identity = await context.getCurrentIdentity();
-        identityAllowed = await identity.hasRoles(identityRoles);
-      } else {
-        identityAllowed = identityRoles;
-      }
-
-      let signerAllowed: boolean;
-      if (typeof signerPermissions !== 'boolean') {
-        const account = context.getCurrentAccount();
-        signerAllowed = await account.hasPermissions(signerPermissions);
-      } else {
-        signerAllowed = signerPermissions;
-      }
+    let identityAllowed: boolean;
+    if (typeof identityRoles !== 'boolean') {
+      const identity = await context.getCurrentIdentity();
+      identityAllowed = await identity.hasRoles(identityRoles);
+    } else {
+      identityAllowed = identityRoles;
+    }
 
-      allowed = identityAllowed && signerAllowed;
+    let signerAllowed: boolean;
+    if (typeof signerPermissions !== 'boolean') {
+      const account = context.getCurrentAccount();
+      signerAllowed = await account.hasPermissions(signerPermissions);
     } else {
-      allowed = checkAuthorizationResult;
+      signerAllowed = signerPermissions;
+    }
+
+    if (!signerAllowed) {
+      throw new PolymeshError({
+        code: ErrorCode.NotAuthorized,
+        message: "Current Account doesn't have the required permissions to execute this procedure",
+      });
     }
 
-    if (!allowed) {
+    if (!identityAllowed) {
       throw new PolymeshError({
         code: ErrorCode.NotAuthorized,
-        message: 'Current account is not authorized to execute this procedure',
+        message: "Current Identity doesn't have the required roles to execute this procedure",
       });
     }
 

src/base/__tests__/Procedure.ts

@@ -182,13 +182,7 @@ describe('Procedure class', () => {
       context = dsMockUtils.getContextInstance({ hasRoles: false, hasPermissions: false });
 
       await expect(proc.prepare(procArgs, context)).rejects.toThrow(
-        'Current account is not authorized to execute this procedure'
-      );
-
-      proc = new Procedure(func, { identityRoles: false });
-
-      await expect(proc.prepare(procArgs, context)).rejects.toThrow(
-        'Current account is not authorized to execute this procedure'
+        "Current Identity doesn't have the required roles to execute this procedure"
       );
 
       proc = new Procedure(func, {
@@ -200,21 +194,21 @@ describe('Procedure class', () => {
       });
 
       await expect(proc.prepare(procArgs, context)).rejects.toThrow(
-        'Current account is not authorized to execute this procedure'
+        "Current Account doesn't have the required permissions to execute this procedure"
       );
 
       proc = new Procedure(func, {
         signerPermissions: false,
       });
 
       await expect(proc.prepare(procArgs, context)).rejects.toThrow(
-        'Current account is not authorized to execute this procedure'
+        "Current Account doesn't have the required permissions to execute this procedure"
       );
 
-      proc = new Procedure(func, async () => false);
+      proc = new Procedure(func, async () => ({ identityRoles: false }));
 
       await expect(proc.prepare(procArgs, context)).rejects.toThrow(
-        'Current account is not authorized to execute this procedure'
+        "Current Identity doesn't have the required roles to execute this procedure"
       );
     });
   });

src/utils/__tests__/conversion.ts

@@ -3777,40 +3777,40 @@ describe('portfolioLikeToPortfolioId', () => {
   });
 
   test('should convert a DID string to a PortfolioId', async () => {
-    const result = await portfolioLikeToPortfolioId(did);
+    const result = portfolioLikeToPortfolioId(did);
 
     expect(result).toEqual({ did, number: undefined });
   });
 
   test('should convert an Identity to a PortfolioId', async () => {
     const identity = entityMockUtils.getIdentityInstance({ did });
 
-    const result = await portfolioLikeToPortfolioId(identity);
+    const result = portfolioLikeToPortfolioId(identity);
 
     expect(result).toEqual({ did, number: undefined });
   });
 
   test('should convert a NumberedPortfolio to a PortfolioId', async () => {
     const portfolio = new NumberedPortfolio({ did, id: number }, context);
 
-    const result = await portfolioLikeToPortfolioId(portfolio);
+    const result = portfolioLikeToPortfolioId(portfolio);
 
     expect(result).toEqual({ did, number });
   });
 
   test('should convert a DefaultPortfolio to a PortfolioId', async () => {
     const portfolio = new DefaultPortfolio({ did }, context);
 
-    const result = await portfolioLikeToPortfolioId(portfolio);
+    const result = portfolioLikeToPortfolioId(portfolio);
 
     expect(result).toEqual({ did, number: undefined });
   });
 
   test('should convert a Portfolio identifier object to a PortfolioId', async () => {
-    let result = await portfolioLikeToPortfolioId({ identity: did, id: number });
+    let result = portfolioLikeToPortfolioId({ identity: did, id: number });
     expect(result).toEqual({ did, number });
 
-    result = await portfolioLikeToPortfolioId({
+    result = portfolioLikeToPortfolioId({
       identity: entityMockUtils.getIdentityInstance({ did }),
       id: number,
     });
@@ -3846,12 +3846,12 @@ describe('portfolioLikeToPortfolio', () => {
   });
 
   test('should convert a PortfolioLike to a DefaultPortfolio instance', async () => {
-    const result = await portfolioLikeToPortfolio(did, context);
+    const result = portfolioLikeToPortfolio(did, context);
     expect(result instanceof DefaultPortfolio).toBe(true);
   });
 
   test('should convert a PortfolioLike to a NumberedPortfolio instance', async () => {
-    const result = await portfolioLikeToPortfolio({ identity: did, id }, context);
+    const result = portfolioLikeToPortfolio({ identity: did, id }, context);
     expect(result instanceof NumberedPortfolio).toBe(true);
   });
 });