Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Build HTTPS redirect URI when behind LB #156

Merged
merged 1 commit into from
Feb 24, 2021

Conversation

iskandar
Copy link
Contributor

Issue/Feature

When running s3-proxy behind a TLS-terminating Load Balancer, s3-proxy generates a http:// value for
the OIDC redirect_uri. In HTTPS-only environments (or with secure cookies enabled), this causes the OIDC flow to fail.

This PR adds a simple check for the de-facto standard HTTP Header X-Forwarded-Proto. If this header is set and has value https, the GetRequestURI function will use the https scheme instead of http.

Checklist:

  • Verified by team member
  • Tests were added
  • Comments where necessary
  • Documentation changes if necessary

@iskandar
Copy link
Contributor Author

iskandar commented Feb 24, 2021

I guess the 'labeling' GitHub Actions fail because this remote fork doesn't have access to the GitHub Token.

@iskandar
Copy link
Contributor Author

Tests actually pass in CircleCI, but the process fails trying to run Coveralls - again, probably because the fork doesn't have permission to use the token.

@oxyno-zeta
Copy link
Owner

Hello @iskandar !

Yes it is because of all of this. Forks cannot have access to my tokens. I will try to work on a patch to avoid running those steps when comes from a PR.

By the way, the PR will be merged as everything important to me (linter, tests are green, builds are ok) are working.

Thanks for your contribution !

@oxyno-zeta oxyno-zeta self-assigned this Feb 24, 2021
@oxyno-zeta oxyno-zeta added the bug Something isn't working label Feb 24, 2021
@oxyno-zeta oxyno-zeta merged commit 04cd22d into oxyno-zeta:master Feb 24, 2021
@oxyno-zeta
Copy link
Owner

Merged, will be packaged in the next release 👍

@iskandar
Copy link
Contributor Author

Thanks, and thanks for building a useful project :)

@iskandar iskandar deleted the fix-tls-2 branch February 24, 2021 13:25
@oxyno-zeta
Copy link
Owner

Happy to see that this project is used and help some people ! 😉

@oxyno-zeta
Copy link
Owner

🎉 This PR is included in version 3.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants