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.

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

feat(field-level-auth): Add field level auth support via the @auth directive #1262

Merged
merged 9 commits into from Apr 15, 2019

Conversation

mikeparisstuff
Copy link
Contributor

@mikeparisstuff mikeparisstuff commented Apr 11, 2019

Issue #, if available:

#1043

Description of changes:

Adding support for field level authorization via the @auth directive.

This addresses proposals 1 & 4 in #1043.

Changes:
- Added the 'operations' argument to the AuthRule input used by the @auth directive.
- Made backwards compatible changes such that @auth directives that use the 'operations' argument protect @connection resolvers.
- Added support for field level @auth directives. Protect query resolvers on any type and mutations on @models.
- Many new e2e tests for field level authorization checks.
- Refactor existing tests to make debugging and coverage easier to understand.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ghost ghost assigned mikeparisstuff Apr 11, 2019
@ghost ghost added the review label Apr 11, 2019
Paris added 4 commits April 11, 2019 16:00
Changes:
- Added the 'operations' argument to the AuthRule input used by the @auth directive.
- Made backwards compatible changes such that @auth directives that use the 'operations' argument protect @connection resolvers.
- Added support for field level @auth directives. Protect query resolvers on any type and mutations on @models.
- Many new e2e tests for field level authorization checks.
- Refactor existing tests to make debugging and coverage easier to understand.
…auth support via the @auth dire

Adding support for field level authorization via the @auth directive.

re #1043

Cleaning up tests
@mikeparisstuff mikeparisstuff changed the title Per field auth feat(@auth directive transformer and e2e tests.): Adding field level auth support via the @auth directive Apr 12, 2019
@mikeparisstuff mikeparisstuff changed the title feat(@auth directive transformer and e2e tests.): Adding field level auth support via the @auth directive feat(field-level-auth): Adding field level auth support via the @auth directive Apr 12, 2019
@mikeparisstuff
Copy link
Contributor Author

Implements proposals 1 & 5 from #1043.

@yuth yuth changed the title feat(field-level-auth): Adding field level auth support via the @auth directive feat(field-level-auth): Add field level auth support via the @auth directive Apr 14, 2019
Copy link
Contributor

@yuth yuth left a comment

Choose a reason for hiding this comment

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

Looks like there are merge conflicts. Can you resolve them

).toEqual('AMAZON_COGNITO_USER_POOLS')
expect(
out.resolvers['Query.getPost.res.vtl']
).toContain('Authorization rule:')
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think these would be good candidate for snapshot tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps. Do you mean storing the output and then diffing them between runs?

Copy link
Contributor

@yuth yuth Apr 15, 2019

Choose a reason for hiding this comment

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

Yes. Jest has built in support for that.

expect(out.resolvers['Query.getPost.res.vtl']).toMatchSnapshot();

this should do it.

I think we should have both assertions, since in PR people tend to miss the snapshot if it grows a lot

@sebastienfi
Copy link

sebastienfi commented May 2, 2019

Implements proposals 1 & 5 from #1043.

@mikeparisstuff I think you meant "1 & 4".

AlessandroVol23 pushed a commit to AlessandroVol23/amplify-cli that referenced this pull request May 16, 2019
…rective (aws-amplify#1262)

* feat(@auth directive transformer and e2e tests.): Adding field level auth support via the @auth dire

Changes:
- Added the 'operations' argument to the AuthRule input used by the @auth directive.
- Made backwards compatible changes such that @auth directives that use the 'operations' argument protect @connection resolvers.
- Added support for field level @auth directives. Protect query resolvers on any type and mutations on @models.
- Many new e2e tests for field level authorization checks.
- Refactor existing tests to make debugging and coverage easier to understand.

re #1043
@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants