fix: Require signatures for ssl_check request verification#1495
Conversation
|
Thanks for the contribution! Before we can merge this, we need @homanp to sign the Salesforce Inc. Contributor License Agreement. |
|
CLA was signed but not showing up here. |
|
@WilliamBergamin found this as well, not sure if this was "by-design" or not but worth taking a look. |
WilliamBergamin
left a comment
There was a problem hiding this comment.
This is another awesome find 💯 🚀
Reproducing this locally and testing it out took me longer then I want to admit, but changes look good to me 🥇 thanks for adding these unit tests, they will prevent us form introducing similar behavior in the future
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1495 +/- ##
=======================================
Coverage 91.32% 91.32%
=======================================
Files 228 228
Lines 7262 7262
=======================================
Hits 6632 6632
Misses 630 630 ☔ View full report in Codecov by Sentry. |
|
This might of been by design to allows users that disable User that want to handle their own ssl check logic are able to do so in a global middleware |
Summary
RequestVerificationfrom skipping HMAC validation solely because a form body containsssl_check=1Why
When
ssl_check_enabled=False, the SSL-check middleware does not interceptssl_check=1requests. Request verification still needs to authenticate those HTTP requests before they can reach normal middleware or slash command listeners.Fixes #1494.
Testing
./scripts/run_tests.sh "tests/slack_bolt/middleware/request_verification/test_request_verification.py tests/slack_bolt_async/middleware/request_verification/test_request_verification.py tests/scenario_tests/test_slash_command.py tests/scenario_tests_async/test_slash_command.py tests/adapter_tests/wsgi/test_wsgi_http.py"./scripts/lint.sh --no-install./scripts/run_mypy.sh --no-install