OpenID HTTPS redirect fix #91
Conversation
|
This should all be handled on the nginx side of things too -- it should not accept any http traffic or rather redirect it all to https |
megascatterbomb
force-pushed
the
openid400fix
branch
from
68d8d47
to
431cf8f
Compare
megascatterbomb
force-pushed
the
openid400fix
branch
from
431cf8f
to
3e3fefc
Compare
|
Latest commits fix #92 |
| if not fake_ip.startswith("169.254"): | ||
| if ":" not in fake_ip: | ||
| fake_ip = f"{fake_ip}:27015" |
There was a problem hiding this comment.
what with this custom port?
There was a problem hiding this comment.
that is the port TF2 assumes if none is specified.
masterbase/app.py
Outdated
| dev_mode = os.getenv("DEVELOPMENT") | ||
| if dev_mode is not None and dev_mode.lower() == "true": | ||
| if base_url.startswith("https://"): | ||
| base_url = base_url.replace("https://", "http://") | ||
| else: | ||
| base_url = "http://" + base_url | ||
| else: | ||
| if base_url.startswith("http://"): | ||
| base_url = base_url.replace("http://", "https://") | ||
| else: | ||
| base_url = "https://" + base_url |
There was a problem hiding this comment.
nice, maybe worth making this a function somewhere else and calling here
There was a problem hiding this comment.
This function seems a bit buggy..
If dev_mode is enabled and the base_url starts with http://, it will get modified to be http://http://<base url>.
If the base_url starts with https://, then it will be set to http://<base url> as expected.
Same goes for the branch when dev_mode is disabled (except with https:// instead).
You can also simplify the actual string replacement, since calling replace on "https://" on a string with "http://" is just a no-op and returns the original string.
# Default a protocol if the base_url doesn't start with one
if not base_url.startswith("http"):
base_url = "https://" + base_url
if os.getenv('DEVELOPMENT', 'false').lower() == 'true':
base_url = base_url.replace("https://", "http://")
else:
base_url = base_url.replace("http://", "https://") There was a problem hiding this comment.
Alternatively, you can do some pythonic splitting...
proto = "http://" if os.getenv('DEVELOPMENT', 'false').lower() == 'true' else "https://"
base_url = proto + base_url.split("//")[-1]There was a problem hiding this comment.
Implemented your pythonic method with an additional check for None on os.getenv
There was a problem hiding this comment.
Nevermind, didn't realise the extra parameter on getenv handled that case.
|
Been running this branch in production for weeks without issue, going to merge to main. |
Fixes some edge cases where the openid config wasn't properly forcing HTTPS.