blaissao : apply suggested fixs (code refactoring)

Author: laissaouibrahim

messages/delete_sandbox.md

@@ -57,8 +57,13 @@ Unable to determine the username of the org to delete. Specify the username with
 
 # error.insufficientPermissions
 
-You do not have the required permission to delete this sandbox: %s. Contact your administrator to assign you the "DeleteSandbox" PermissionSet.
+You don't have the required "DeleteSandbox" Permission Set assigned in the production org to delete sandbox "%s".
 
-# warning.couldNotVerifyPermissions
+# error.insufficientPermissions.actions
 
-Could not verify permissions in sandbox: %s. The delete operation will proceed, but may fail if you do not have the required permissions.
+- Ask your administrator to create a Permission Set named "DeleteSandbox" in the production org and assign it to your user.
+- Re-authenticate with the production org and try again.
+
+# error.insufficientAccess
+
+You don't have permission to delete this sandbox. Ask your Salesforce admin to grant you the "Manage Sandboxes" system permission on your profile or a permission set in the production org.

src/commands/org/delete/sandbox.ts

@@ -61,43 +61,27 @@ export default class DeleteSandbox extends SfCommand<SandboxDeleteResponse> {
       throw messages.createError('error.unknownSandbox', [username]);
     }
 
-    // Check if user has the DeleteSandbox PermissionSet in the sandbox
-    try {
-      const sandboxOrg = await Org.create({ aliasOrUsername: username });
-      const hasDeleteSandboxPermission = await this.hasPermission(sandboxOrg, 'DeleteSandbox');
-      this.debug('hasDeleteSandboxPermission %s ', hasDeleteSandboxPermission);
-      if (!hasDeleteSandboxPermission) {
-        throw messages.createError('error.insufficientPermissions', [username]);
-      }
-    } catch (error) {
-      // If it's a permission error we created, re-throw it
-      if (error instanceof SfError) {
-        const errorMessage = error.message || '';
-        if (errorMessage.includes('required permission') || errorMessage.includes('DeleteSandbox')) {
-          throw error;
-        }
-      }
-      // For other errors (e.g., org connection issues), log a warning but continue
-      // The actual delete operation will fail if permissions are truly insufficient
-      if (error instanceof Error) {
-        this.warn(messages.getMessage('warning.couldNotVerifyPermissions', [username]));
-      }
-    }
+    await this.verifyDeletePermission(stateAggregator, orgId, username);
+
+    const org = await Org.create({ aliasOrUsername: username });
 
     if (flags['no-prompt'] || (await this.confirm({ message: messages.getMessage('prompt.confirm', [username]) }))) {
       try {
-        const org = await Org.create({ aliasOrUsername: username });
         await org.delete();
         this.logSuccess(messages.getMessage('success', [username]));
       } catch (e) {
         if (e instanceof Error && e.name === 'DomainNotFoundError') {
-          // the org has expired, so remote operations won't work
-          // let's clean up the files locally
           const authRemover = await AuthRemover.create();
           await authRemover.removeAuth(username);
           this.logSuccess(messages.getMessage('success.Idempotent', [username]));
         } else if (e instanceof Error && e.name === 'SandboxNotFound') {
           this.logSuccess(messages.getMessage('success.Idempotent', [username]));
+        } else if (
+          e instanceof Error &&
+          'errorCode' in e &&
+          (e as { errorCode: string }).errorCode === 'INSUFFICIENT_ACCESS_OR_READONLY'
+        ) {
+          throw messages.createError('error.insufficientAccess');
         } else {
           throw e;
         }
@@ -107,42 +91,39 @@ export default class DeleteSandbox extends SfCommand<SandboxDeleteResponse> {
   }
 
   /**
-   * Checks if the current user has a PermissionSet with the specified name assigned.
-   *
-   * @param org The org to check permissions in
-   * @param permissionSetName The name of the PermissionSet to check (e.g., 'DeleteSandbox')
-   * @returns True if the user has the PermissionSet assigned, false otherwise
+   * Verifies the current user has the 'DeleteSandbox' PermissionSet assigned
+   * in the production org that owns the sandbox.
    */
   // eslint-disable-next-line class-methods-use-this
-  private async hasPermission(org: Org, permissionSetName: string): Promise<boolean> {
-    try {
-      const connection = org.getConnection();
-      await org.refreshAuth();
-      // try to get it from Identity API  
-      const identity = await connection.identity();
-      const userId = identity.user_id;
-
-      if (!userId) {
-        return false;
-      }
+  private async verifyDeletePermission(
+    stateAggregator: StateAggregator,
+    orgId: string,
+    sandboxUsername: string
+  ): Promise<void> {
+    const sandboxConfig = await stateAggregator.sandboxes.read(orgId);
+    const prodOrgUsername = sandboxConfig?.prodOrgUsername;
 
-      // Check if user has the PermissionSet assigned
-      const permissionSetAssignmentQuery = `
-      SELECT Id
-      FROM PermissionSetAssignment
-      WHERE AssigneeId = '${userId.replace(/'/g, "\\'")}'
-      AND PermissionSet.Name = '${permissionSetName.replace(/'/g, "\\'")}'
-      `;
+    if (!prodOrgUsername) {
+      return;
+    }
 
-      try {
-        const permissionSetResult = await connection.query(permissionSetAssignmentQuery);
-        return permissionSetResult.totalSize > 0;
-      } catch {
-        // If query fails, return false
-        return false;
-      }
-    } catch {
-      return false;
+    const prodOrg = await Org.create({ aliasOrUsername: prodOrgUsername });
+    const connection = prodOrg.getConnection();
+    await prodOrg.refreshAuth();
+
+    const identity = await connection.identity();
+    const userId = identity.user_id;
+
+    if (!userId) {
+      throw messages.createError('error.insufficientPermissions', [sandboxUsername]);
+    }
+
+    const result = await connection.query(
+      `SELECT Id FROM PermissionSetAssignment WHERE AssigneeId = '${userId}' AND PermissionSet.Name = 'DeleteSandbox'`
+    );
+
+    if (result.totalSize === 0) {
+      throw messages.createError('error.insufficientPermissions', [sandboxUsername]);
     }
   }
 }

test/unit/org/delete.test.ts

@@ -5,7 +5,7 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
-import { AuthInfo, Messages, Org, SfError } from '@salesforce/core';
+import { AuthInfo, Connection, Messages, Org, SfError } from '@salesforce/core';
 import { MockTestOrgData, TestContext } from '@salesforce/core/testSetup';
 import { SinonStub } from 'sinon';
 import { config, expect } from 'chai';
@@ -38,6 +38,12 @@ describe('org delete', () => {
   });
 
   describe('sandbox', () => {
+    let sandboxReadStub: SinonStub;
+
+    beforeEach(() => {
+      sandboxReadStub = $$.SANDBOX.stub(SandboxAccessor.prototype, 'read').resolves(null);
+    });
+
     it('will throw an error when no org provided', async () => {
       await $$.stubConfig({});
       try {
@@ -128,6 +134,61 @@ describe('org delete', () => {
         JSON.stringify(sfCommandUxStubs.logSuccess.getCalls().flatMap((call) => call.args))
       ).to.deep.include(sbxOrgMessages.getMessage('success.Idempotent', [testOrg.username]));
     });
+
+    it('will throw a clean SfError when the user lacks Manage Sandboxes permission', async () => {
+      $$.SANDBOX.stub(SandboxAccessor.prototype, 'hasFile').resolves(true);
+      orgDeleteStub.restore();
+      const insufficientAccessError = Object.assign(new Error('INSUFFICIENT_ACCESS_OR_READONLY'), {
+        errorCode: 'INSUFFICIENT_ACCESS_OR_READONLY',
+      });
+      $$.SANDBOX.stub(Org.prototype, 'delete').throws(insufficientAccessError);
+      try {
+        await DeleteSandbox.run(['--no-prompt', '--target-org', testOrg.username]);
+        expect.fail('should have thrown InsufficientAccessError');
+      } catch (e) {
+        const err = e as SfError;
+        expect(err.name).to.equal('InsufficientAccessError');
+        expect(err.message).to.equal(sbxOrgMessages.getMessage('error.insufficientAccess'));
+      }
+    });
+
+    describe('DeleteSandbox permission check', () => {
+      let identityStub: SinonStub;
+      let queryStub: SinonStub;
+
+      beforeEach(() => {
+        $$.SANDBOX.stub(SandboxAccessor.prototype, 'hasFile').resolves(true);
+        $$.SANDBOX.stub(Org.prototype, 'refreshAuth').resolves();
+        identityStub = $$.SANDBOX.stub(Connection.prototype, 'identity');
+        queryStub = $$.SANDBOX.stub(Connection.prototype, 'query');
+      });
+
+      it('will allow deletion when user has DeleteSandbox permission set', async () => {
+        sandboxReadStub.resolves({ sandboxOrgId: testOrg.orgId, prodOrgUsername: testHub.username });
+        // eslint-disable-next-line camelcase
+        identityStub.resolves({ user_id: '005xx000001X' });
+        queryStub.resolves({ totalSize: 1, records: [{ Id: '0Pa000000000001' }] });
+        const res = await DeleteSandbox.run(['--no-prompt', '--target-org', testOrg.username]);
+        expect(sfCommandUxStubs.logSuccess.callCount).to.equal(1);
+        expect(res).to.deep.equal({ orgId: testOrg.orgId, username: testOrg.username });
+      });
+
+      it('will block deletion when user lacks DeleteSandbox permission set', async () => {
+        sandboxReadStub.resolves({ sandboxOrgId: testOrg.orgId, prodOrgUsername: testHub.username });
+        // eslint-disable-next-line camelcase
+        identityStub.resolves({ user_id: '005xx000001X' });
+        queryStub.resolves({ totalSize: 0, records: [] });
+        try {
+          await DeleteSandbox.run(['--no-prompt', '--target-org', testOrg.username]);
+          expect.fail('should have thrown InsufficientPermissionsError');
+        } catch (e) {
+          const err = e as SfError;
+          expect(err.name).to.equal('InsufficientPermissionsError');
+          expect(err.message).to.include(testOrg.username);
+          expect(err.message).to.include('DeleteSandbox');
+        }
+      });
+    });
   });
 
   describe('scratch', () => {