Skip to content

adds ProxyFix for Cloud9 #65

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

Merged
merged 5 commits into from
Dec 29, 2018
Merged

adds ProxyFix for Cloud9 #65

merged 5 commits into from
Dec 29, 2018

Conversation

dmalan
Copy link
Member

@dmalan dmalan commented Dec 16, 2018

This monkey-patches flask.Flask so that app is wrapped with http://werkzeug.pocoo.org/docs/0.14/contrib/fixers/#werkzeug.contrib.fixers.ProxyFix on Cloud9 (online), which ensures flask.redirect won't redirect from HTTPS to HTTP. Sample app in tests/redirect/.

Seems best to monkey-patch rather than add, e.g., http://stackoverflow.com/a/23504684/5156190 to students' own application.py files, lest they reuse that file on a non-proxied setup, which isn't recommended:

Do not use this middleware in non-proxy setups for security reasons.

@dmalan
Copy link
Member Author

dmalan commented Dec 23, 2018

@kzidane mind taking a look?

@@ -22,7 +22,7 @@

# Add support for Cloud9 proxy so that flask.redirect doesn't redirect from HTTPS to HTTP
# http://stackoverflow.com/a/23504684/5156190
if getenv("C9_HOSTNAME") and not getenv("IDE_OFFLINE"):
if getenv("C9_PROJECT") == "ide50" and not getenv("IDE_OFFLINE"):
Copy link
Member Author

Choose a reason for hiding this comment

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

How come better, @kzidane?

Copy link
Member

Choose a reason for hiding this comment

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

C9_HOSTNAME is set offline as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

But that's why we're checking and not getenv("IDE_OFFLINE") too, no?

Copy link
Member

Choose a reason for hiding this comment

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

Well, in that case we could just check for IDE_OFFLINE really since it's not set online. C9_PROJECT is set to a different value online ("ide50") vs offline ("ide50-offline").

Copy link
Member Author

Choose a reason for hiding this comment

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

That'd be different logic. We need the ProxyFix iff using CS50 IDE Online.

kzidane
kzidane previously approved these changes Dec 29, 2018
@kzidane kzidane merged commit 07ca0cb into develop Dec 29, 2018
@kzidane kzidane deleted the ProxyFix branch December 29, 2018 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants