-
Notifications
You must be signed in to change notification settings - Fork 47
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
Be more explicit about configs and where they come from #535
Conversation
2c23b5a
to
a35f124
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I love it! ❤️
Ou, actually, I found an issue. I'll re-do bits of this (specifically concerning the I'll mark this PR as a DO NOT MERGE for now. |
Should be resolved now -- I removed the bits in |
This allows us to remove a dependency (`everett`) while being clearer about where configs come from. If something fails with a `KeyError` (or similar), we can `grep` the codebase and find that value. Jira: IAM-1443
We don't need these since the discovery [is handled by Flask-pyoidc for us][pyoidc]. Jira: IAM-1443 [pyoidc]: https://github.com/zamzterz/Flask-pyoidc/blob/26b123572cba0b3fa84482c6c0270900042a73c9/src/flask_pyoidc/provider_configuration.py#L142-L144
Jira: IAM-1443
Jira: IAM-1443
Jira: IAM-1443
I was confusing what this configuration value was used for. Turns out, we don't make explicit use of this, though internally Flask-pyoidc does. To clear up some confusion here I removed the bit of code from TokenVerification. Jira: IAM-1443
8eef740
to
784997c
Compare
This PR adds/removes a couple of things:
env.sample
env.sample
, now shows the full keys we expect, along with some docs about where to get themeverett
) -- this one did a bit of magic which I removed and implemented ourself (as a result it should be painfully clear what's missing on startup)OIDC_REDIRECT_URI
-- we set this environment variable but never use itSee individual commits for a better review story.