Skip to content

Commit 93ebfd1

Browse files
n1ru4lcoderabbitai[bot]
authored and
hasparus
committed
feat: organization access token error handling (#6573)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent b0f2bcb commit 93ebfd1

File tree

15 files changed

+288
-79
lines changed

15 files changed

+288
-79
lines changed

‎.changeset/eleven-brooms-happen.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-hive/cli': minor
3+
---
4+
5+
Better error handling for missing `--target` option when required.

‎integration-tests/tests/cli/schema.spec.ts

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* eslint-disable no-process-env */
22
import { createHash } from 'node:crypto';
33
import { ProjectType } from 'testkit/gql/graphql';
4+
import * as GraphQLSchema from 'testkit/gql/graphql';
45
import { createCLI, schemaCheck, schemaPublish } from '../../testkit/cli';
56
import { cliOutputSnapshotSerializer } from '../../testkit/cli-snapshot-serializer';
67
import { initSeed } from '../../testkit/seed';
@@ -497,3 +498,145 @@ test('schema:check gives correct error message for missing `--service` name flag
497498
- Missing service name
498499
`);
499500
});
501+
502+
test('schema:check without `--target` flag fails for organization access token', async ({
503+
expect,
504+
}) => {
505+
const { createOrg } = await initSeed().createOwner();
506+
const { createOrganizationAccessToken } = await createOrg();
507+
const privateKey = await createOrganizationAccessToken({
508+
permissions: ['schemaCheck:create', 'project:describe'],
509+
resources: {
510+
mode: GraphQLSchema.ResourceAssignmentMode.All,
511+
},
512+
});
513+
514+
await expect(
515+
schemaCheck([
516+
'--registry.accessToken',
517+
privateKey,
518+
'--author',
519+
'Kamil',
520+
'fixtures/init-schema.graphql',
521+
]),
522+
).rejects.toMatchInlineSnapshot(`
523+
:::::::::::::::: CLI FAILURE OUTPUT :::::::::::::::
524+
exitCode------------------------------------------:
525+
2
526+
stderr--------------------------------------------:
527+
› Error: Missing 1 required argument:
528+
› TARGET The target on which the action is performed. This can either be a
529+
› slug following the format "$organizationSlug/$projectSlug/$targetSlug"
530+
› (e.g "the-guild/graphql-hive/staging") or an UUID (e.g.
531+
› "a0f4c605-6541-4350-8cfe-b31f21a4bf80"). [102]
532+
› > See https://__URL__ for
533+
› a complete list of error codes and recommended fixes.
534+
› To disable this message set HIVE_NO_ERROR_TIP=1
535+
stdout--------------------------------------------:
536+
__NONE__
537+
`);
538+
});
539+
540+
test('schema:check with `--target` flag succeeds for organization access token', async ({
541+
expect,
542+
}) => {
543+
const { createOrg } = await initSeed().createOwner();
544+
const { createOrganizationAccessToken, createProject, organization } = await createOrg();
545+
const { project, target } = await createProject();
546+
const privateKey = await createOrganizationAccessToken({
547+
permissions: ['schemaCheck:create', 'project:describe'],
548+
resources: {
549+
mode: GraphQLSchema.ResourceAssignmentMode.All,
550+
},
551+
});
552+
553+
await expect(
554+
schemaCheck([
555+
'--registry.accessToken',
556+
privateKey,
557+
'--author',
558+
'Kamil',
559+
'--target',
560+
`${organization.slug}/${project.slug}/${target.slug}`,
561+
'fixtures/init-schema.graphql',
562+
]),
563+
).resolves.toMatchInlineSnapshot(`
564+
:::::::::::::::: CLI SUCCESS OUTPUT :::::::::::::::::
565+
566+
stdout--------------------------------------------:
567+
✔ Schema registry is empty, nothing to compare your schema with.
568+
View full report:
569+
http://__URL__
570+
`);
571+
});
572+
573+
test('schema:publish without `--target` flag fails for organization access token', async ({
574+
expect,
575+
}) => {
576+
const { createOrg } = await initSeed().createOwner();
577+
const { createOrganizationAccessToken } = await createOrg();
578+
const privateKey = await createOrganizationAccessToken({
579+
permissions: ['project:describe', 'schemaVersion:publish'],
580+
resources: {
581+
mode: GraphQLSchema.ResourceAssignmentMode.All,
582+
},
583+
});
584+
585+
await expect(
586+
schemaPublish([
587+
'--registry.accessToken',
588+
privateKey,
589+
'--author',
590+
'Kamil',
591+
592+
'fixtures/init-schema.graphql',
593+
]),
594+
).rejects.toMatchInlineSnapshot(`
595+
:::::::::::::::: CLI FAILURE OUTPUT :::::::::::::::
596+
exitCode------------------------------------------:
597+
2
598+
stderr--------------------------------------------:
599+
› Error: Missing 1 required argument:
600+
› TARGET The target on which the action is performed. This can either be a
601+
› slug following the format "$organizationSlug/$projectSlug/$targetSlug"
602+
› (e.g "the-guild/graphql-hive/staging") or an UUID (e.g.
603+
› "a0f4c605-6541-4350-8cfe-b31f21a4bf80"). [102]
604+
› > See https://__URL__ for
605+
› a complete list of error codes and recommended fixes.
606+
› To disable this message set HIVE_NO_ERROR_TIP=1
607+
stdout--------------------------------------------:
608+
__NONE__
609+
`);
610+
});
611+
612+
test('schema:publish with `--target` flag succeeds for organization access token', async ({
613+
expect,
614+
}) => {
615+
const { createOrg } = await initSeed().createOwner();
616+
const { createOrganizationAccessToken, organization, createProject } = await createOrg();
617+
const { project, target } = await createProject();
618+
const privateKey = await createOrganizationAccessToken({
619+
permissions: ['project:describe', 'schemaVersion:publish'],
620+
resources: {
621+
mode: GraphQLSchema.ResourceAssignmentMode.All,
622+
},
623+
});
624+
625+
await expect(
626+
schemaPublish([
627+
'--registry.accessToken',
628+
privateKey,
629+
'--author',
630+
'Kamil',
631+
'--target',
632+
`${organization.slug}/${project.slug}/${target.slug}`,
633+
'fixtures/init-schema.graphql',
634+
]),
635+
).resolves.toMatchInlineSnapshot(`
636+
:::::::::::::::: CLI SUCCESS OUTPUT :::::::::::::::::
637+
638+
stdout--------------------------------------------:
639+
✔ Published initial schema.
640+
ℹ Available at http://__URL__
641+
`);
642+
});

‎packages/libraries/cli/src/base-command.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,14 @@ export default abstract class BaseCommand<T extends typeof Command> extends Comm
262262
}
263263

264264
if (jsonData.errors && jsonData.errors.length > 0) {
265+
if (jsonData.errors[0].extensions?.code === 'ERR_MISSING_TARGET') {
266+
throw new MissingArgumentsError([
267+
'target',
268+
'The target on which the action is performed.' +
269+
' This can either be a slug following the format "$organizationSlug/$projectSlug/$targetSlug" (e.g "the-guild/graphql-hive/staging")' +
270+
' or an UUID (e.g. "a0f4c605-6541-4350-8cfe-b31f21a4bf80").',
271+
]);
272+
}
265273
if (jsonData.errors[0].message === 'Invalid token provided') {
266274
throw new InvalidRegistryTokenError();
267275
}

‎packages/services/api/src/modules/app-deployments/providers/app-deployments-manager.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Injectable, Scope } from 'graphql-modules';
22
import * as GraphQLSchema from '../../../__generated__/types';
33
import { Target } from '../../../shared/entities';
44
import { batch } from '../../../shared/helpers';
5-
import { InsufficientPermissionError, Session } from '../../auth/lib/authz';
5+
import { Session } from '../../auth/lib/authz';
66
import { IdTranslator } from '../../shared/providers/id-translator';
77
import { Logger } from '../../shared/providers/logger';
88
import { TargetManager } from '../../target/providers/target-manager';
@@ -64,11 +64,12 @@ export class AppDeploymentsManager {
6464
}) {
6565
const selector = await this.idTranslator.resolveTargetReference({
6666
reference: args.reference,
67-
onError() {
68-
throw new InsufficientPermissionError('appDeployment:create');
69-
},
7067
});
7168

69+
if (!selector) {
70+
this.session.raise('appDeployment:create');
71+
}
72+
7273
await this.session.assertPerformAction({
7374
action: 'appDeployment:create',
7475
organizationId: selector.organizationId,
@@ -100,11 +101,12 @@ export class AppDeploymentsManager {
100101
}) {
101102
const selector = await this.idTranslator.resolveTargetReference({
102103
reference: args.reference,
103-
onError() {
104-
throw new InsufficientPermissionError('appDeployment:create');
105-
},
106104
});
107105

106+
if (!selector) {
107+
this.session.raise('appDeployment:create');
108+
}
109+
108110
await this.session.assertPerformAction({
109111
action: 'appDeployment:create',
110112
organizationId: selector.organizationId,
@@ -134,11 +136,12 @@ export class AppDeploymentsManager {
134136
}) {
135137
const selector = await this.idTranslator.resolveTargetReference({
136138
reference: args.reference,
137-
onError() {
138-
throw new InsufficientPermissionError('appDeployment:publish');
139-
},
140139
});
141140

141+
if (!selector) {
142+
this.session.raise('appDeployment:publish');
143+
}
144+
142145
await this.session.assertPerformAction({
143146
action: 'appDeployment:publish',
144147
organizationId: selector.organizationId,

‎packages/services/api/src/modules/auth/lib/authz.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { NoopLogger } from '../../shared/providers/logger';
33
import { AuthorizationPolicyStatement, Session } from './authz';
44

55
class TestSession extends Session {
6+
id = 'test-session';
67
policyStatements: Array<AuthorizationPolicyStatement>;
78
constructor(policyStatements: Array<AuthorizationPolicyStatement>) {
89
super({ logger: new NoopLogger() });

‎packages/services/api/src/modules/auth/lib/authz.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ export abstract class Session {
7272
organizationId: string,
7373
): Promise<Array<AuthorizationPolicyStatement>> | Array<AuthorizationPolicyStatement>;
7474

75+
abstract readonly id: string;
76+
7577
/** Retrieve the current viewer. Implementations of the session need to implement this function */
7678
public getViewer(): Promise<User> {
7779
throw new AccessError('Authorization token is missing', 'UNAUTHENTICATED');
@@ -130,6 +132,15 @@ export abstract class Session {
130132
return await result;
131133
}
132134

135+
/**
136+
* Raise an insufficient permission error.
137+
* Useful in situations where a resource can not be identified and it should be treated
138+
* as having insufficient permissions.
139+
*/
140+
public raise<TAction extends keyof typeof actionDefinitions>(action: TAction): never {
141+
throw new InsufficientPermissionError(action);
142+
}
143+
133144
/**
134145
* Check whether a session is allowed to perform a specific action.
135146
* Throws a AccessError if the action is not allowed.
@@ -185,7 +196,7 @@ export abstract class Session {
185196
args.organizationId,
186197
args.params,
187198
);
188-
throw new InsufficientPermissionError(args.action);
199+
this.raise(args.action);
189200
} else {
190201
isAllowed = true;
191202
}
@@ -201,7 +212,7 @@ export abstract class Session {
201212
args.params,
202213
);
203214

204-
throw new InsufficientPermissionError(args.action);
215+
this.raise(args.action);
205216
}
206217
}
207218

@@ -486,6 +497,7 @@ class UnauthenticatedSession extends Session {
486497
): Promise<Array<AuthorizationPolicyStatement>> | Array<AuthorizationPolicyStatement> {
487498
return [];
488499
}
500+
id = 'noop';
489501
}
490502

491503
/**

‎packages/services/api/src/modules/auth/lib/organization-access-token-strategy.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ function hashToken(token: string) {
1313
export class OrganizationAccessTokenSession extends Session {
1414
public readonly organizationId: string;
1515
private policies: Array<AuthorizationPolicyStatement>;
16+
readonly id: string;
1617

1718
constructor(
1819
args: {
20+
id: string;
1921
organizationId: string;
2022
policies: Array<AuthorizationPolicyStatement>;
2123
},
@@ -24,6 +26,7 @@ export class OrganizationAccessTokenSession extends Session {
2426
},
2527
) {
2628
super({ logger: deps.logger });
29+
this.id = args.id;
2730
this.organizationId = args.organizationId;
2831
this.policies = args.policies;
2932
}
@@ -106,6 +109,7 @@ export class OrganizationAccessTokenStrategy extends AuthNStrategy<OrganizationA
106109

107110
return new OrganizationAccessTokenSession(
108111
{
112+
id: organizationAccessToken.id,
109113
organizationId: organizationAccessToken.organizationId,
110114
policies: organizationAccessToken.authorizationPolicyStatements,
111115
},

‎packages/services/api/src/modules/auth/lib/supertokens-strategy.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ export class SuperTokensCookieBasedSession extends Session {
2525
this.storage = deps.storage;
2626
}
2727

28+
get id(): string {
29+
return this.superTokensUserId;
30+
}
31+
2832
protected async loadPolicyStatementsForOrganization(
2933
organizationId: string,
3034
): Promise<Array<AuthorizationPolicyStatement>> {

‎packages/services/api/src/modules/auth/lib/target-access-token-strategy.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ export class TargetAccessTokenSession extends Session {
4343
return this.policies;
4444
}
4545

46+
get id(): string {
47+
return this.token;
48+
}
49+
4650
public getLegacySelector() {
4751
return {
4852
token: this.token,

‎packages/services/api/src/modules/schema/providers/schema-manager.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { HiveError } from '../../../shared/errors';
2626
import { atomic, cache, stringifySelector } from '../../../shared/helpers';
2727
import { isUUID } from '../../../shared/is-uuid';
2828
import { parseGraphQLSource } from '../../../shared/schema';
29-
import { InsufficientPermissionError, Session } from '../../auth/lib/authz';
29+
import { Session } from '../../auth/lib/authz';
3030
import { GitHubIntegrationManager } from '../../integrations/providers/github-integration-manager';
3131
import { ProjectManager } from '../../project/providers/project-manager';
3232
import { CryptoProvider } from '../../shared/providers/crypto';
@@ -123,11 +123,12 @@ export class SchemaManager {
123123

124124
const selector = await this.idTranslator.resolveTargetReference({
125125
reference: input.target ?? null,
126-
onError() {
127-
throw new InsufficientPermissionError('schema:compose');
128-
},
129126
});
130127

128+
if (!selector) {
129+
this.session.raise('schema:compose');
130+
}
131+
131132
trace.getActiveSpan()?.setAttributes({
132133
'hive.organization.id': selector.organizationId,
133134
'hive.target.id': selector.targetId,
@@ -1002,11 +1003,12 @@ export class SchemaManager {
10021003
}) {
10031004
const selector = await this.idTranslator.resolveTargetReference({
10041005
reference: args.target,
1005-
onError() {
1006-
throw new InsufficientPermissionError('project:describe');
1007-
},
10081006
});
10091007

1008+
if (!selector) {
1009+
this.session.raise('project:describe');
1010+
}
1011+
10101012
this.logger.debug('Fetch schema version by action id. (args=%o)', {
10111013
projectId: selector.projectId,
10121014
targetId: selector.targetId,

0 commit comments

Comments
 (0)