Skip to content

Resolve #251 by correcting a duplicated response header - #253

Merged
seratch merged 1 commit into
slackapi:mainfrom
seratch:issue-251-cloud-run-oauth
Mar 6, 2021
Merged

seratch merged 1 commit into
slackapi:mainfrom
seratch:issue-251-cloud-run-oauth

Conversation

@seratch

@seratch seratch commented Mar 6, 2021 •

Copy link
Copy Markdown
Contributor

This pull request resolves #251 by correcting the set of response headers for OAuth flows.

Before the changes in this PR, this library's OAuth flow handler has been having the content-length header twice in a response. In actual fact, the response works with major browsers anyway. However, Google Cloud Run's server-side has a bit strict format validation policies before delivering an HTTP response. Due to the validation, the OAuth flow fails in the way described in the issue.

If I remember correctly, there is no specific need to manually craft the header but I added the logic just for easily ensuring the compatibility with various adapters (sadly, it was not easy in this case...). As most web frameworks generates the header under the hood.

For this reason, I'm sure it's safe enough to delete the one created by this library. Also, I've already verified all the adapters including the AWS Lambda adapter work with this change.

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.

@seratch seratch added bug Something isn't working area:async area:sync labels Mar 6, 2021
@seratch seratch added this to the 1.4.3 milestone Mar 6, 2021
@codecov

codecov Bot commented Mar 6, 2021

Copy link
Copy Markdown

Codecov Report

Merging #253 (09b4580) into main (1efd5cd) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #253   +/-   ##
=======================================
  Coverage   91.37%   91.37%           
=======================================
  Files         160      160           
  Lines        4973     4973           
=======================================
  Hits         4544     4544           
  Misses        429      429           
Impacted Files Coverage Δ
slack_bolt/oauth/async_oauth_flow.py 90.75% <ø> (ø)
slack_bolt/oauth/internals.py 97.77% <ø> (ø)
slack_bolt/oauth/oauth_flow.py 90.43% <ø> (ø)

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 1efd5cd...72476fe. Read the comment docs.

@seratch
seratch merged commit f0080b2 into slackapi:main Mar 6, 2021
@seratch
seratch deleted the issue-251-cloud-run-oauth branch March 6, 2021 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:async area:sync bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to run OAuth flow URLs in Google Cloud Run

1 participant