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

Remove authentication headers from webhook payloads #4983

Merged
merged 5 commits into from
Sep 29, 2020
Merged

Remove authentication headers from webhook payloads #4983

merged 5 commits into from
Sep 29, 2020

Conversation

potato
Copy link
Contributor

@potato potato commented Jul 2, 2020

Authentication headers (along with authentication cookies) shouldn't be stored in the payload of the trigger instance, since authentication data from GET query parameters are not stored either.

Not filtering in _log_request since on DEBUG loglevel those can be useful data.

@pull-request-size pull-request-size bot added the size/S PR that changes 10-29 lines. Very easy to review. label Jul 2, 2020
@CLAassistant
Copy link

CLAassistant commented Jul 2, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

arm4b
arm4b reviewed Jul 2, 2020
View reviewed changes
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Thanks for contributing this. The change looks great!

Please also include a Changelog record for this in CHANGELOG.rst.

@arm4b arm4b added this to the 3.3.0 milestone Jul 2, 2020
@pull-request-size pull-request-size bot added size/M PR that changes 30-99 lines. Good size to review. and removed size/S PR that changes 10-29 lines. Very easy to review. labels Jul 2, 2020
@potato
Copy link
Contributor Author

potato commented Jul 2, 2020

updated the CHANGELOG.rst

nmaludy
nmaludy suggested changes Sep 29, 2020
View reviewed changes
Copy link
Member

@nmaludy nmaludy left a comment

Choose a reason for hiding this comment

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

I like this idea a lot. Probably good to list out what headers are going to be stripped out in the CHANGELOG and probably also in st2docs

@arm4b arm4b requested a review from nmaludy September 29, 2020 19:15
@arm4b
Copy link
Member

arm4b commented Sep 29, 2020

Thanks @potato @knagy for the contribution and @nmaludy for review! 👍

@arm4b arm4b merged commit 0d143d4 into StackStorm:master Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arm4b arm4b arm4b left review comments

@nmaludy nmaludy nmaludy approved these changes

Assignees
No one assigned
Labels
bug security size/M PR that changes 30-99 lines. Good size to review.
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.
4 participants
@potato @CLAassistant @arm4b @nmaludy