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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve auth decorators #3762

Merged
merged 3 commits into from Sep 19, 2019

Conversation

@raymondfeng
Copy link
Member

raymondfeng commented Sep 18, 2019

This PR improves @authenticate (implements #2460)

  1. Allow @authenticate to be applied at class level
  2. Add @authenticate.skip to skip authentication

It also improves @authorize:

  1. Add @authorize.skip to skip authorization

Checklist

馃憠 Read and sign the CLA (Contributor License Agreement) 馃憟

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

馃憠 Check out how to submit a PR 馃憟

@raymondfeng raymondfeng requested a review from bajtos as a code owner Sep 18, 2019
@raymondfeng raymondfeng force-pushed the improve-auth-decorators branch from 26e43e9 to 9fb240a Sep 18, 2019
Copy link
Contributor

dhmlau left a comment

One minor comment on the docs. The usage and tests LGTM. I'll leave the review of the actual implementation to other maintainers. :)

docs/site/decorators/Decorators_authenticate.md Outdated Show resolved Hide resolved
@raymondfeng raymondfeng force-pushed the improve-auth-decorators branch from 9fb240a to f706947 Sep 19, 2019
@raymondfeng raymondfeng requested a review from jannyHou Sep 19, 2019
Copy link
Contributor

jannyHou left a comment

@raymondfeng The design looks reasonable to me 馃憤 a few nitpicks and will take another round to review the changes of tests.

docs/site/decorators/Decorators_authenticate.md Outdated Show resolved Hide resolved
@@ -40,6 +41,33 @@ export class WhoAmIController {
}
```

To configure a default authentication for all methods within a class,

This comment has been minimized.

Copy link
@jannyHou

jannyHou Sep 19, 2019

Contributor

馃憤 馃憦 The design looks reasonable to me.

@raymondfeng raymondfeng force-pushed the improve-auth-decorators branch from f706947 to 5638f53 Sep 19, 2019
@raymondfeng raymondfeng force-pushed the improve-auth-decorators branch from 5638f53 to 46d2958 Sep 19, 2019
Copy link
Contributor

jannyHou left a comment

:shipit: LGTM

@raymondfeng raymondfeng merged commit 757ee16 into master Sep 19, 2019
4 checks passed
4 checks passed
Travis CI - Pull Request Build Passed
Details
clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage remained the same at 0.0%
Details
@raymondfeng raymondfeng deleted the improve-auth-decorators branch Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.