feat(config): improve configuration validation on invalid org id (#128)

https://coveord.atlassian.net/browse/CDX-208

packages/cli/package.json

@@ -8,7 +8,7 @@
   },
   "dependencies": {
     "@angular/cli": "^11.1.4",
-    "@coveord/platform-client": "^9.29.0",
+    "@coveord/platform-client": "^18.2.1",
     "@oclif/command": "^1",
     "@oclif/config": "^1",
     "@oclif/plugin-help": "^3",

packages/cli/src/commands/auth/login.spec.ts

@@ -9,45 +9,36 @@ import {mocked} from 'ts-jest/utils';
 import {Config} from '../../lib/config/config';
 import {OAuth} from '../../lib/oauth/oauth';
 import {AuthenticatedClient} from '../../lib/platform/authenticatedClient';
-import {PlatformClient} from '@coveord/platform-client';
 const mockedOAuth = mocked(OAuth, true);
 const mockedConfig = mocked(Config, true);
 const mockedAuthenticatedClient = mocked(AuthenticatedClient);
-const mockedPlatformClient = mocked(PlatformClient);
 
 describe('auth:login', () => {
   const mockConfigSet = jest.fn();
-  const mockConfigGet = jest.fn();
-  const mockListOrgs = jest.fn();
 
-  beforeEach(() => {
-    mockListOrgs.mockReturnValue(Promise.resolve([{id: 'foo'}]));
-
-    mockConfigGet.mockReturnValue(
-      Promise.resolve({
-        region: 'us-east-1',
-        organization: 'foo',
-        environment: 'prod',
-      })
-    );
+  const mockConfigGet = jest.fn().mockReturnValue(
+    Promise.resolve({
+      region: 'us-east-1',
+      organization: 'foo',
+      environment: 'prod',
+    })
+  );
 
-    mockedPlatformClient.mockImplementation(
-      () =>
-        ({
-          organization: {list: mockListOrgs} as unknown,
-        } as PlatformClient)
-    );
+  const mockListOrgs = jest
+    .fn()
+    .mockReturnValue(Promise.resolve([{id: 'foo'}]));
+
+  const mockGetHasAccessToOrg = jest
+    .fn()
+    .mockReturnValue(Promise.resolve(true));
+
+  beforeEach(() => {
     mockedAuthenticatedClient.mockImplementation(
       () =>
-        ({
-          getClient: () =>
-            Promise.resolve(
-              mockedPlatformClient.getMockImplementation()!({
-                accessToken: 'theToken',
-                organizationId: 'foo',
-              })
-            ),
-        } as AuthenticatedClient)
+        (({
+          getAllOrgsUserHasAccessTo: mockListOrgs,
+          getUserHasAccessToOrg: mockGetHasAccessToOrg,
+        } as unknown) as AuthenticatedClient)
     );
     mockedOAuth.mockImplementationOnce(
       () =>
@@ -135,6 +126,9 @@ describe('auth:login', () => {
 
   test
     .stdout()
+    .do(() => {
+      mockGetHasAccessToOrg.mockReturnValueOnce(Promise.resolve(false));
+    })
     .command(['auth:login', '-o', 'this_is_not_a_valid_org'])
     .exit(2)
     .it('fails when organization flag is invalid');

packages/cli/src/commands/auth/login.ts

@@ -114,34 +114,32 @@ export default class Login extends Command {
   }
 
   private async pickFirstAvailableOrganization() {
-    const orgs = await this.getAllOrgsUserHasAccessTo();
+    const orgs = await new AuthenticatedClient().getAllOrgsUserHasAccessTo();
     return orgs[0]?.id;
   }
 
   private get flags() {
     const {flags} = this.parse(Login);
     return flags;
   }
-  private async getAllOrgsUserHasAccessTo() {
-    const orgs = await (
-      await new AuthenticatedClient().getClient()
-    ).organization.list();
-    return (orgs as unknown) as OrganizationModel[];
-  }
 
   private async verifyOrganization() {
     const flags = this.flags;
-    const orgs = await this.getAllOrgsUserHasAccessTo();
+    const authenticatedClient = new AuthenticatedClient();
 
     if (flags.organization) {
-      const found = orgs.find((o) => o.id === flags.organization);
-      if (!found) {
+      const hasAccess = await authenticatedClient.getUserHasAccessToOrg(
+        flags.organization
+      );
+      if (!hasAccess) {
         this.error(
           `You either do not have access to organization ${flags.organization}, or it does not exists.`
         );
       }
     }
 
+    const orgs = await authenticatedClient.getAllOrgsUserHasAccessTo();
+
     if (orgs.length === 0) {
       this.error(`
       You do not have access to any Coveo organization in this region and environment.

packages/cli/src/commands/config/get.spec.ts

@@ -6,7 +6,7 @@ import {Config} from '../../lib/config/config';
 import {test} from '@oclif/test';
 const mockedConfig = mocked(Config);
 
-describe('config', () => {
+describe('config:get', () => {
   const mockGet = jest.fn();
   const mockSet = jest.fn();
 

packages/cli/src/commands/config/set.spec.ts

@@ -0,0 +1,114 @@
+jest.mock('../../lib/config/config');
+jest.mock('../../hooks/analytics/analytics');
+jest.mock('../../hooks/prerun/prerun');
+jest.mock('../../lib/platform/authenticatedClient');
+
+import {mocked} from 'ts-jest/utils';
+import {Config} from '../../lib/config/config';
+import {test} from '@oclif/test';
+import {AuthenticatedClient} from '../../lib/platform/authenticatedClient';
+const mockedConfig = mocked(Config);
+const mockedClient = mocked(AuthenticatedClient);
+
+describe('config:set', () => {
+  const mockSet = jest.fn();
+  const mockGetHasAccessToOrg = jest.fn();
+
+  mockedConfig.mockImplementation(
+    () =>
+      (({
+        set: mockSet,
+      } as unknown) as Config)
+  );
+
+  mockedClient.mockImplementation(
+    () =>
+      (({
+        getUserHasAccessToOrg: mockGetHasAccessToOrg,
+      } as unknown) as AuthenticatedClient)
+  );
+
+  test
+    .stdout()
+    .command(['config:set'])
+    .it(
+      'allows to call set without any flags and should not modify configuration',
+      () => {
+        expect(mockSet).not.toHaveBeenCalled();
+      }
+    );
+
+  ['dev', 'qa', 'prod', 'hipaa'].forEach((environment) => {
+    test
+      .stdout()
+      .command(['config:set', '-e', environment])
+      .it(`allows to modify environment ${environment}`, () => {
+        expect(mockSet).toHaveBeenCalledWith('environment', environment);
+      });
+  });
+
+  test
+    .stdout()
+    .command(['config:set', '-e', 'foo'])
+    .catch(/Expected --environment=foo/)
+    .it('fails when trying to set an invalid environment', () => {
+      expect(mockSet).not.toHaveBeenCalled();
+    });
+
+  [
+    'us-east-1',
+    'eu-west-1',
+    'eu-west-3',
+    'ap-southeast-2',
+    'us-west-2',
+  ].forEach((region) => {
+    test
+      .stdout()
+      .command(['config:set', '-r', region])
+      .it(`allows to modify region ${region}`, () => {
+        expect(mockSet).toHaveBeenCalledWith('region', region);
+      });
+  });
+
+  test
+    .stdout()
+    .command(['config:set', '-r', 'foo'])
+    .catch(/Expected --region=foo/)
+    .it('fails when trying to set an invalid region', () => {
+      expect(mockSet).not.toHaveBeenCalled();
+    });
+
+  test
+    .stdout()
+    .do(() => {
+      mockGetHasAccessToOrg.mockReturnValueOnce(Promise.resolve(true));
+    })
+    .command(['config:set', '-o', 'the_org'])
+    .it(
+      'allows to modify the organization of the user has access to it',
+      () => {
+        expect(mockSet).toHaveBeenCalledWith('organization', 'the_org');
+      }
+    );
+
+  test
+    .stdout()
+    .do(() => {
+      mockGetHasAccessToOrg.mockReturnValueOnce(Promise.resolve(false));
+    })
+    .command(['config:set', '-o', 'the_org'])
+    .catch(/do not have access to organization the_org/)
+    .it(
+      'fails when trying to set to an invalid organzation the user does not have access to',
+      () => {
+        expect(mockSet).not.toHaveBeenCalled();
+      }
+    );
+
+  test
+    .stdout()
+    .command(['config:set', '-a', 'y'])
+    .it('allows to modify the analytics configuration', () => {
+      expect(mockSet).toHaveBeenCalledWith('analyticsEnabled', true);
+    });
+});

packages/cli/src/commands/config/set.ts

@@ -4,6 +4,11 @@ import {
   buildAnalyticsSuccessHook,
 } from '../../hooks/analytics/analytics';
 import {Config} from '../../lib/config/config';
+import {
+  IsAuthenticated,
+  Preconditions,
+} from '../../lib/decorators/preconditions';
+import {AuthenticatedClient} from '../../lib/platform/authenticatedClient';
 import {
   PlatformEnvironment,
   PlatformRegion,
@@ -44,13 +49,15 @@ export default class Set extends Command {
     }),
   };
 
+  @Preconditions(IsAuthenticated())
   async run() {
     const {flags} = this.parse(Set);
     const cfg = new Config(this.config.configDir, this.error);
     if (flags.environment) {
       cfg.set('environment', flags.environment as PlatformEnvironment);
     }
     if (flags.organization) {
+      await this.verifyOrganization(flags.organization);
       cfg.set('organization', flags.organization);
     }
     if (flags.region) {
@@ -73,4 +80,15 @@ export default class Set extends Command {
     );
     throw err;
   }
+
+  private async verifyOrganization(org: string) {
+    const hasAccess = await new AuthenticatedClient().getUserHasAccessToOrg(
+      org
+    );
+    if (!hasAccess) {
+      this.error(
+        `You either do not have access to organization ${org}, or it does not exists.`
+      );
+    }
+  }
 }

packages/cli/src/lib/platform/authenticatedClient.ts

@@ -61,6 +61,16 @@ export class AuthenticatedClient {
       console.error('Currently not logged in.');
     }
   }
+
+  async getAllOrgsUserHasAccessTo() {
+    const platformClient = await this.getClient();
+    return platformClient.organization.list();
+  }
+
+  async getUserHasAccessToOrg(org: string) {
+    const orgs = await this.getAllOrgsUserHasAccessTo();
+    return orgs.some((o) => o.id === org);
+  }
 }
 
 export enum AuthenticationStatus {