Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(amplify-category-api): change auth directive type and fix codegen bug #8639

Merged
merged 1 commit into from
Nov 3, 2021
Merged

fix(amplify-category-api): change auth directive type and fix codegen bug #8639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...gory-api/src/__tests__/provider-utils/awscloudformation/utils/global-sandbox-mode.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ describe('global sandbox mode GraphQL directive', () => {
it('returns input AMPLIFY with code comment', () => {
expect(defineGlobalSandboxMode()).toEqual(`# This "input" configures a global authorization rule to enable public access to
# all models in this schema. Learn more about authorization rules here: https://docs.amplify.aws/cli/graphql-transformer/auth
input AMPLIFY { global_auth_rule: AuthorizationRule = { allow: public } } # FOR TESTING ONLY!\n
input AMPLIFY { global_auth_rule: AuthRule = { allow: public } } # FOR TESTING ONLY!\n
`);
});
});
2 changes: 1 addition & 1 deletion ...es/amplify-category-api/src/provider-utils/awscloudformation/utils/global-sandbox-mode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export function defineGlobalSandboxMode(): string {
return `# This "input" configures a global authorization rule to enable public access to
# all models in this schema. Learn more about authorization rules here: https://docs.amplify.aws/cli/graphql-transformer/auth
input AMPLIFY { global_auth_rule: AuthorizationRule = { allow: public } } # FOR TESTING ONLY!\n
input AMPLIFY { global_auth_rule: AuthRule = { allow: public } } # FOR TESTING ONLY!\n
`;
}
2 changes: 1 addition & 1 deletion packages/amplify-e2e-tests/schemas/model_with_sandbox_mode.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
input AMPLIFY {
global_auth_rule: AuthorizationRule = { allow: public }
global_auth_rule: AuthRule = { allow: public }
}

type Todo @model {
Expand Down
4 changes: 4 additions & 0 deletions packages/amplify-e2e-tests/src/__tests__/global_sandbox.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
updateApiSchema,
apiGqlCompile,
amplifyPush,
generateModels,
} from 'amplify-e2e-core';
import { createNewProjectDir, deleteProjectDir } from 'amplify-e2e-core';

Expand All @@ -29,19 +30,22 @@ describe('global sandbox mode', () => {
it('compiles schema with one model and pushes to cloud', async () => {
await addApiWithOneModel(projectDir);
await apiGqlCompile(projectDir, true);
await generateModels(projectDir);
await amplifyPush(projectDir, true);
});

it.skip('compiles schema with three models and pushes to cloud', async () => {
await addApiWithThreeModels(projectDir);
await apiGqlCompile(projectDir, true);
await generateModels(projectDir);
await amplifyPush(projectDir, true);
});

it('compiles schema user-added schema and pushes to cloud', async () => {
await addApiWithoutSchema(projectDir, { apiName });
updateApiSchema(projectDir, apiName, 'model_with_sandbox_mode.graphql');
await apiGqlCompile(projectDir, true);
await generateModels(projectDir);
await amplifyPush(projectDir, true);
});
});
9 changes: 7 additions & 2 deletions packages/amplify-graphql-transformer-core/src/transformation/validation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ scalar AWSPhone
scalar AWSIPAddress
scalar BigInt
scalar Double
scalar AuthorizationRule
`);

export const EXTRA_DIRECTIVES_DOCUMENT = parse(`
Expand Down Expand Up @@ -160,7 +159,13 @@ export const validateAuthModes = (authConfig: AppSyncAuthConfiguration) => {
for (let i = 0; i < authModes.length; i++) {
const mode = authModes[i];

if (mode !== 'API_KEY' && mode !== 'AMAZON_COGNITO_USER_POOLS' && mode !== 'AWS_IAM' && mode !== 'OPENID_CONNECT' && mode !== 'AWS_LAMBDA') {
if (
mode !== 'API_KEY' &&
mode !== 'AMAZON_COGNITO_USER_POOLS' &&
mode !== 'AWS_IAM' &&
mode !== 'OPENID_CONNECT' &&
mode !== 'AWS_LAMBDA'
) {
throw new Error(`Invalid auth mode ${mode}`);
}
}
Comment on lines 159 to 171
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I know this was just a formatting change, but this could be simplified a bit to:

const allowedAuthModes = ['API_KEY', 'AWS_IAM', ...];
const invalidMode = authModes.find(mode => !allowedAuthModes.includes(mode));
if (!!invalidMode) {
  throw new Error(`Invalid auth mode ${mode}`)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to go this route, please hoist the array out of the function so that the same array can be reused. It might also make sense to use a Set instead of an array (not sure if there are enough elements to make it worth it though).

Expand Down
10 changes: 5 additions & 5 deletions packages/amplify-provider-awscloudformation/src/__tests__/utils/sandbox-mode-helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('sandbox mode helpers', () => {
expect(prompts.printer.info).toBeCalledWith(
`
⚠️ WARNING: Global Sandbox Mode has been enabled, which requires a valid API key. If
you'd like to disable, remove ${chalk.green('"input AMPLIFY { global_auth_rule: AuthorizationRule = { allow: public } }"')}
you'd like to disable, remove ${chalk.green('"input AMPLIFY { global_auth_rule: AuthRule = { allow: public } }"')}
from your GraphQL schema and run 'amplify push' again. If you'd like to proceed with
sandbox mode disabled, do not create an API Key.
`,
Expand All @@ -62,7 +62,7 @@ sandbox mode disabled, do not create an API Key.
describe('schemaHasSandboxModeEnabled', () => {
it('parses sandbox AMPLIFY input on schema', () => {
const schema = `
input AMPLIFY { global_auth_rule: AuthorizationRule = { allow: public } }
input AMPLIFY { global_auth_rule: AuthRule = { allow: public } }
`;

expect(schemaHasSandboxModeEnabled(schema)).toEqual(true);
Expand Down Expand Up @@ -90,7 +90,7 @@ sandbox mode disabled, do not create an API Key.
);
});

it('guards for AuthorizationRule', () => {
it('guards for AuthRule', () => {
const schema = `
input AMPLIFY { global_auth_rule: AuthenticationRule = { allow: public } }
`;
Expand All @@ -104,7 +104,7 @@ sandbox mode disabled, do not create an API Key.

it('checks for "allow" field name', () => {
const schema = `
input AMPLIFY { global_auth_rule: AuthorizationRule = { allows: public } }
input AMPLIFY { global_auth_rule: AuthRule = { allows: public } }
`;

expect(() => schemaHasSandboxModeEnabled(schema)).toThrow(
Expand All @@ -116,7 +116,7 @@ sandbox mode disabled, do not create an API Key.

it('checks for "public" value from "allow" field', () => {
const schema = `
input AMPLIFY { global_auth_rule: AuthorizationRule = { allow: private } }
input AMPLIFY { global_auth_rule: AuthRule = { allow: private } }
`;

expect(() => schemaHasSandboxModeEnabled(schema)).toThrowError(
Expand Down
4 changes: 2 additions & 2 deletions packages/amplify-provider-awscloudformation/src/utils/sandbox-mode-helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { parse } from 'graphql';

const AMPLIFY = 'AMPLIFY';
const GLOBAL_AUTH_RULE = 'global_auth_rule';
const AUTHORIZATION_RULE = 'AuthorizationRule';
const AUTHORIZATION_RULE = 'AuthRule';
const ALLOW = 'allow';
const PUBLIC = 'public';

Expand All @@ -15,7 +15,7 @@ export async function showSandboxModePrompts(context: $TSContext): Promise<any>
printer.info(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could use printer.warn here. If you switch it, remove the leading ⚠️ as this will be prepended by warn internally

`
⚠️ WARNING: Global Sandbox Mode has been enabled, which requires a valid API key. If
you'd like to disable, remove ${chalk.green('"input AMPLIFY { global_auth_rule: AuthorizationRule = { allow: public } }"')}
you'd like to disable, remove ${chalk.green('"input AMPLIFY { global_auth_rule: AuthRule = { allow: public } }"')}
from your GraphQL schema and run 'amplify push' again. If you'd like to proceed with
sandbox mode disabled, do not create an API Key.
`,
Expand Down