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(httpApi): external authorizer #7789

Merged

Conversation

Michsior14
Copy link
Contributor

@Michsior14 Michsior14 commented May 27, 2020

This allows to add external authorizer to httpApi event in similar manner to http event.

Closes: #7598

@codecov-commenter
Copy link

codecov-commenter commented May 27, 2020

Codecov Report

Merging #7789 into master will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7789      +/-   ##
==========================================
+ Coverage   88.13%   88.21%   +0.08%     
==========================================
  Files         245      245              
  Lines        9262     9300      +38     
==========================================
+ Hits         8163     8204      +41     
+ Misses       1099     1096       -3     
Impacted Files Coverage Δ
...lugins/aws/package/compile/events/httpApi/index.js 90.11% <100.00%> (+1.63%) ⬆️
lib/utils/log/log.js 0.00% <0.00%> (ø)
lib/utils/getServerlessConfigFile.js 100.00% <0.00%> (ø)
lib/classes/Utils.js 95.33% <0.00%> (+0.06%) ⬆️
...s/package/compile/events/apiGateway/lib/apiKeys.js 94.73% <0.00%> (+0.14%) ⬆️
.../compile/events/apiGateway/lib/hack/updateStage.js 95.12% <0.00%> (+0.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af3fbf0...15ea146. Read the comment docs.

Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

@Michsior14 great thanks for giving that a spin.

Still, in first place we need to agree on an implementation in scope of an issue (#7598).

There was already an implementation spec proposed there.

lib/plugins/aws/info/getStackInfo.js Outdated Show resolved Hide resolved
lib/plugins/aws/package/compile/events/httpApi/index.js Outdated Show resolved Hide resolved
@Michsior14 Michsior14 requested a review from medikoo May 28, 2020 13:59
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Thank you @Michsior14 It looks really good! I’ve proposed few last minor improvements and we should be good to go

@Michsior14 Michsior14 requested a review from medikoo May 29, 2020 08:33
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

@Michsior14 looks great! Posted just one final suggestion, to keep in-logic validation simpler, and we should be good to go

lib/plugins/aws/package/compile/events/httpApi/index.js Outdated Show resolved Hide resolved
@Michsior14 Michsior14 requested a review from medikoo June 1, 2020 10:42
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Thank you @Michsior14 !

@medikoo medikoo merged commit 4074739 into serverless:master Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow support for external authorizer to an httpApi event
3 participants