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

Fix cookie extraction during OAuth for REST based AWS API GW + Lambda app #451

Merged
merged 1 commit into from Aug 28, 2021

Conversation

naveensan1
Copy link
Contributor

Summary:

This pull request is related to #379.

I too deployed a Bolt-python app to AWS Lambda, integrated with REST (format v1.0) based API GW. In the process of the verification of oauth redirect, the app could not be installed properly. Taking a look at the CloudWatch logs I could see the multivalueheader in the request had the key "cookie" instead of "Cookie" as is expected by "to_bolt_request".

   if cookies is None or len(cookies) == 0:
        # In the case of format v1
        multiValueHeaders = event.get("multiValueHeaders", {})
        cookies = multiValueHeaders.get("Cookie", [])

After modifying the following line to account for lower case "cookie", everything worked fine:

     cookies = multiValueHeaders.get("cookie", [])

I think the comment about to_bolt_request not supporting REST API can now be removed because I tried and it works

Category (place an x in each of the [ ])

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Adapters in slack_bolt.adapter
  • Document pages under /docs
  • Others

Requirements (place an x in each [ ])

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run ./scripts/install_all_and_run_tests.sh after making the changes.

@CLAassistant
Copy link

CLAassistant commented Aug 28, 2021

CLA assistant check
All committers have signed the CLA.

@naveensan1 naveensan1 changed the title Fix cookie extraction during OAuth for REST API based AWS API GW + Lambda app Fix cookie extraction during OAuth for REST based AWS API GW + Lambda app Aug 28, 2021
@seratch seratch added area:adapter bug Something isn't working labels Aug 28, 2021
@codecov
Copy link

codecov bot commented Aug 28, 2021

Codecov Report

Merging #451 (ee2c378) into main (4ec4444) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head ee2c378 differs from pull request most recent head cefccf6. Consider uploading reports for the commit cefccf6 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #451   +/-   ##
=======================================
  Coverage   91.34%   91.34%           
=======================================
  Files         167      167           
  Lines        5499     5501    +2     
=======================================
+ Hits         5023     5025    +2     
  Misses        476      476           
Impacted Files Coverage Δ
slack_bolt/adapter/aws_lambda/handler.py 95.83% <100.00%> (+0.11%) ⬆️

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 4ec4444...cefccf6. Read the comment docs.

@seratch seratch added this to the 1.8.1 milestone Aug 28, 2021
Copy link
Member

@seratch seratch 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!

@@ -75,20 +75,17 @@ def handle(self, event, context):


def to_bolt_request(event) -> BoltRequest:
"""Note that this handler supports only the payload format 2.0.
Copy link
Member

Choose a reason for hiding this comment

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

Good catch 👍

cookies = multiValueHeaders.get("Cookie", [])
cookies = multiValueHeaders.get("cookie", [])
if len(cookies) == 0:
# Try using uppercase
Copy link
Member

Choose a reason for hiding this comment

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

👍

@seratch seratch merged commit 05951c6 into slackapi:main Aug 28, 2021
@naveensan1 naveensan1 deleted the handle_cookie_extraction branch August 28, 2021 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:adapter bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants