feat: improve error handling of missing resource input when required

n1ru4l · n1ru4l · commit c20fda241b90 · 2025-03-04T10:42:23.000+08:00
diff --git a/integration-tests/tests/cli/schema.spec.ts b/integration-tests/tests/cli/schema.spec.ts
@@ -1,6 +1,7 @@
 /* eslint-disable no-process-env */
 import { createHash } from 'node:crypto';
 import { ProjectType } from 'testkit/gql/graphql';
+import * as GraphQLSchema from 'testkit/gql/graphql';
 import { createCLI, schemaCheck, schemaPublish } from '../../testkit/cli';
 import { cliOutputSnapshotSerializer } from '../../testkit/cli-snapshot-serializer';
 import { initSeed } from '../../testkit/seed';
@@ -497,3 +498,139 @@ test('schema:check gives correct error message for missing `--service` name flag
        - Missing service name
   `);
 });
+
+test('schema:check without `--target` flag fails for organization access token', async ({
+  expect,
+}) => {
+  const { createOrg } = await initSeed().createOwner();
+  const { createOrganizationAccessToken } = await createOrg();
+  const privateKey = await createOrganizationAccessToken({
+    permissions: ['schemaCheck:create', 'project:describe'],
+    resources: {
+      mode: GraphQLSchema.ResourceAssignmentMode.All,
+    },
+  });
+
+  await expect(
+    schemaCheck([
+      '--registry.accessToken',
+      privateKey,
+      '--author',
+      'Kamil',
+      'fixtures/init-schema.graphql',
+    ]),
+  ).rejects.toMatchInlineSnapshot(`
+    :::::::::::::::: CLI FAILURE OUTPUT :::::::::::::::
+    exitCode------------------------------------------:
+    2
+    stderr--------------------------------------------:
+     ›   Error: A target is required to perform the action. Please provide the
+     ›   "--target" flag.  [109]
+     ›   > See https://__URL__ for
+     ›    a complete list of error codes and recommended fixes.
+     ›   To disable this message set HIVE_NO_ERROR_TIP=1
+    stdout--------------------------------------------:
+    __NONE__
+  `);
+});
+
+test('schema:check with `--target` flag succeeds for organization access token', async ({
+  expect,
+}) => {
+  const { createOrg } = await initSeed().createOwner();
+  const { createOrganizationAccessToken, createProject, organization } = await createOrg();
+  const { project, target } = await createProject();
+  const privateKey = await createOrganizationAccessToken({
+    permissions: ['schemaCheck:create', 'project:describe'],
+    resources: {
+      mode: GraphQLSchema.ResourceAssignmentMode.All,
+    },
+  });
+
+  await expect(
+    schemaCheck([
+      '--registry.accessToken',
+      privateKey,
+      '--author',
+      'Kamil',
+      '--target',
+      `${organization.slug}/${project.slug}/${target.slug}`,
+      'fixtures/init-schema.graphql',
+    ]),
+  ).resolves.toMatchInlineSnapshot(`
+    :::::::::::::::: CLI SUCCESS OUTPUT :::::::::::::::::
+
+    stdout--------------------------------------------:
+    ✔ Schema registry is empty, nothing to compare your schema with.
+    View full report:
+    http://__URL__
+  `);
+});
+
+test('schema:publish without `--target` flag fails for organization access token', async ({
+  expect,
+}) => {
+  const { createOrg } = await initSeed().createOwner();
+  const { createOrganizationAccessToken } = await createOrg();
+  const privateKey = await createOrganizationAccessToken({
+    permissions: ['project:describe', 'schemaVersion:publish'],
+    resources: {
+      mode: GraphQLSchema.ResourceAssignmentMode.All,
+    },
+  });
+
+  await expect(
+    schemaPublish([
+      '--registry.accessToken',
+      privateKey,
+      '--author',
+      'Kamil',
+
+      'fixtures/init-schema.graphql',
+    ]),
+  ).rejects.toMatchInlineSnapshot(`
+    :::::::::::::::: CLI FAILURE OUTPUT :::::::::::::::
+    exitCode------------------------------------------:
+    2
+    stderr--------------------------------------------:
+     ›   Error: A target is required to perform the action. Please provide the
+     ›   "--target" flag.  [109]
+     ›   > See https://__URL__ for
+     ›    a complete list of error codes and recommended fixes.
+     ›   To disable this message set HIVE_NO_ERROR_TIP=1
+    stdout--------------------------------------------:
+    __NONE__
+  `);
+});
+
+test('schema:publish with `--target` flag succeeds for organization access token', async ({
+  expect,
+}) => {
+  const { createOrg } = await initSeed().createOwner();
+  const { createOrganizationAccessToken, organization, createProject } = await createOrg();
+  const { project, target } = await createProject();
+  const privateKey = await createOrganizationAccessToken({
+    permissions: ['project:describe', 'schemaVersion:publish'],
+    resources: {
+      mode: GraphQLSchema.ResourceAssignmentMode.All,
+    },
+  });
+
+  await expect(
+    schemaPublish([
+      '--registry.accessToken',
+      privateKey,
+      '--author',
+      'Kamil',
+      '--target',
+      `${organization.slug}/${project.slug}/${target.slug}`,
+      'fixtures/init-schema.graphql',
+    ]),
+  ).resolves.toMatchInlineSnapshot(`
+    :::::::::::::::: CLI SUCCESS OUTPUT :::::::::::::::::
+
+    stdout--------------------------------------------:
+    ✔ Published initial schema.
+    ℹ Available at http://__URL__
+  `);
+});
diff --git a/packages/libraries/cli/src/base-command.ts b/packages/libraries/cli/src/base-command.ts
@@ -14,6 +14,7 @@ import {
   InvalidRegistryTokenError,
   isAggregateError,
   MissingArgumentsError,
+  MissingTargetError,
   NetworkError,
 } from './helpers/errors';
 import { Texture } from './helpers/texture/texture';
@@ -262,6 +263,9 @@ export default abstract class BaseCommand<T extends typeof Command> extends Comm
         }
 
         if (jsonData.errors && jsonData.errors.length > 0) {
+          if (jsonData.errors[0].extensions?.code === 'ERR_MISSING_TARGET') {
+            throw new MissingTargetError();
+          }
           if (jsonData.errors[0].message === 'Invalid token provided') {
             throw new InvalidRegistryTokenError();
           }
diff --git a/packages/libraries/cli/src/helpers/errors.ts b/packages/libraries/cli/src/helpers/errors.ts
@@ -137,6 +137,16 @@ export class MissingCdnEndpointError extends HiveCLIError {
   }
 }
 
+export class MissingTargetError extends HiveCLIError {
+  constructor() {
+    super(
+      ExitCode.ERROR,
+      errorCode(ErrorCategory.GENERIC, 9),
+      `A target is required to perform the action. Please provide the "--target" flag.`,
+    );
+  }
+}
+
 export class MissingEnvironmentError extends HiveCLIError {
   constructor(...requiredVars: Array<[string, string]>) {
     const varsStr = requiredVars.map(a => `\t${a[0]} \t${a[1]}`).join('\n');
diff --git a/packages/services/api/src/modules/app-deployments/providers/app-deployments-manager.ts b/packages/services/api/src/modules/app-deployments/providers/app-deployments-manager.ts
@@ -2,7 +2,7 @@ import { Injectable, Scope } from 'graphql-modules';
 import * as GraphQLSchema from '../../../__generated__/types';
 import { Target } from '../../../shared/entities';
 import { batch } from '../../../shared/helpers';
-import { InsufficientPermissionError, Session } from '../../auth/lib/authz';
+import { Session } from '../../auth/lib/authz';
 import { IdTranslator } from '../../shared/providers/id-translator';
 import { Logger } from '../../shared/providers/logger';
 import { TargetManager } from '../../target/providers/target-manager';
@@ -64,11 +64,12 @@ export class AppDeploymentsManager {
   }) {
     const selector = await this.idTranslator.resolveTargetReference({
       reference: args.reference,
-      onError() {
-        throw new InsufficientPermissionError('appDeployment:create');
-      },
     });
 
+    if (!selector) {
+      this.session.raise('appDeployment:create');
+    }
+
     await this.session.assertPerformAction({
       action: 'appDeployment:create',
       organizationId: selector.organizationId,
@@ -100,11 +101,12 @@ export class AppDeploymentsManager {
   }) {
     const selector = await this.idTranslator.resolveTargetReference({
       reference: args.reference,
-      onError() {
-        throw new InsufficientPermissionError('appDeployment:create');
-      },
     });
 
+    if (!selector) {
+      this.session.raise('appDeployment:create');
+    }
+
     await this.session.assertPerformAction({
       action: 'appDeployment:create',
       organizationId: selector.organizationId,
@@ -134,11 +136,12 @@ export class AppDeploymentsManager {
   }) {
     const selector = await this.idTranslator.resolveTargetReference({
       reference: args.reference,
-      onError() {
-        throw new InsufficientPermissionError('appDeployment:publish');
-      },
     });
 
+    if (!selector) {
+      this.session.raise('appDeployment:publish');
+    }
+
     await this.session.assertPerformAction({
       action: 'appDeployment:publish',
       organizationId: selector.organizationId,
diff --git a/packages/services/api/src/modules/auth/lib/authz.ts b/packages/services/api/src/modules/auth/lib/authz.ts
@@ -72,6 +72,8 @@ export abstract class Session {
     organizationId: string,
   ): Promise<Array<AuthorizationPolicyStatement>> | Array<AuthorizationPolicyStatement>;
 
+  abstract readonly id: string;
+
   /** Retrieve the current viewer. Implementations of the session need to implement this function */
   public getViewer(): Promise<User> {
     throw new AccessError('Authorization token is missing', 'UNAUTHENTICATED');
@@ -130,6 +132,15 @@ export abstract class Session {
     return await result;
   }
 
+  /**
+   * Raise an insufficient permission error.
+   * Useful in situations where a resource can not be identified and it should be treated
+   * as having insufficient permissions.
+   */
+  public raise<TAction extends keyof typeof actionDefinitions>(action: TAction): never {
+    throw new InsufficientPermissionError(action);
+  }
+
   /**
    * Check whether a session is allowed to perform a specific action.
    * Throws a AccessError if the action is not allowed.
@@ -185,7 +196,7 @@ export abstract class Session {
               args.organizationId,
               args.params,
             );
-            throw new InsufficientPermissionError(args.action);
+            this.raise(args.action);
           } else {
             isAllowed = true;
           }
@@ -201,7 +212,7 @@ export abstract class Session {
         args.params,
       );
 
-      throw new InsufficientPermissionError(args.action);
+      this.raise(args.action);
     }
   }
 
diff --git a/packages/services/api/src/modules/auth/lib/organization-access-token-strategy.ts b/packages/services/api/src/modules/auth/lib/organization-access-token-strategy.ts
@@ -13,9 +13,11 @@ function hashToken(token: string) {
 export class OrganizationAccessTokenSession extends Session {
   public readonly organizationId: string;
   private policies: Array<AuthorizationPolicyStatement>;
+  readonly id: string;
 
   constructor(
     args: {
+      id: string;
       organizationId: string;
       policies: Array<AuthorizationPolicyStatement>;
     },
@@ -24,6 +26,7 @@ export class OrganizationAccessTokenSession extends Session {
     },
   ) {
     super({ logger: deps.logger });
+    this.id = args.id;
     this.organizationId = args.organizationId;
     this.policies = args.policies;
   }
@@ -105,6 +108,7 @@ export class OrganizationAccessTokenStrategy extends AuthNStrategy<OrganizationA
 
     return new OrganizationAccessTokenSession(
       {
+        id: organizationAccessToken.id,
         organizationId: organizationAccessToken.organizationId,
         policies: organizationAccessToken.authorizationPolicyStatements,
       },
diff --git a/packages/services/api/src/modules/auth/lib/supertokens-strategy.ts b/packages/services/api/src/modules/auth/lib/supertokens-strategy.ts
@@ -25,6 +25,10 @@ export class SuperTokensCookieBasedSession extends Session {
     this.storage = deps.storage;
   }
 
+  get id(): string {
+    return this.superTokensUserId;
+  }
+
   protected async loadPolicyStatementsForOrganization(
     organizationId: string,
   ): Promise<Array<AuthorizationPolicyStatement>> {
diff --git a/packages/services/api/src/modules/auth/lib/target-access-token-strategy.ts b/packages/services/api/src/modules/auth/lib/target-access-token-strategy.ts
@@ -43,6 +43,10 @@ export class TargetAccessTokenSession extends Session {
     return this.policies;
   }
 
+  get id(): string {
+    return this.token;
+  }
+
   public getLegacySelector() {
     return {
       token: this.token,
diff --git a/packages/services/api/src/modules/schema/providers/schema-manager.ts b/packages/services/api/src/modules/schema/providers/schema-manager.ts
@@ -22,11 +22,12 @@ import {
   ProjectType,
   Target,
 } from '../../../shared/entities';
-import { HiveError } from '../../../shared/errors';
+import { HiveError, MissingTargetError } from '../../../shared/errors';
 import { atomic, cache, stringifySelector } from '../../../shared/helpers';
 import { isUUID } from '../../../shared/is-uuid';
 import { parseGraphQLSource } from '../../../shared/schema';
 import { InsufficientPermissionError, Session } from '../../auth/lib/authz';
+import { TargetAccessTokenSession } from '../../auth/lib/target-access-token-strategy';
 import { GitHubIntegrationManager } from '../../integrations/providers/github-integration-manager';
 import { ProjectManager } from '../../project/providers/project-manager';
 import { CryptoProvider } from '../../shared/providers/crypto';
@@ -123,11 +124,12 @@ export class SchemaManager {
 
     const selector = await this.idTranslator.resolveTargetReference({
       reference: input.target ?? null,
-      onError() {
-        throw new InsufficientPermissionError('schema:compose');
-      },
     });
 
+    if (!selector) {
+      this.session.raise('schema:compose');
+    }
+
     trace.getActiveSpan()?.setAttributes({
       'hive.organization.id': selector.organizationId,
       'hive.target.id': selector.targetId,
@@ -1002,11 +1004,12 @@ export class SchemaManager {
   }) {
     const selector = await this.idTranslator.resolveTargetReference({
       reference: args.target,
-      onError() {
-        throw new InsufficientPermissionError('project:describe');
-      },
     });
 
+    if (!selector) {
+      this.session.raise('project:describe');
+    }
+
     this.logger.debug('Fetch schema version by action id. (args=%o)', {
       projectId: selector.projectId,
       targetId: selector.targetId,
diff --git a/packages/services/api/src/modules/schema/providers/schema-publisher.ts b/packages/services/api/src/modules/schema/providers/schema-publisher.ts
diff --git a/packages/services/api/src/modules/shared/providers/id-translator.ts b/packages/services/api/src/modules/shared/providers/id-translator.ts
diff --git a/packages/services/api/src/shared/errors.ts b/packages/services/api/src/shared/errors.ts
