-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Make OAuth secrets and settings configurable #1540
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
Conversation
liggitt
commented
Apr 1, 2015
- Add structure and validation around session secrets
- Switch session secret from time-based uuid to hash of random uuid
- Generate default signing and encrypting secrets
- Optionally externalize secrets using a file ref to a serialized SessionSecrets object
- Update OAuth config documentation
|
@deads2k @detiber review and critique, please Two ways to configure session secrets: 1 - No config, default generated by 2 - Externalized config: mysecrets.yaml: |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1729/) |
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'm anti-space.
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.
reworked to produce errors like this:
oauthConfig.sessionConfig.sessionSecretsFile: invalid value 'secrets.yaml': secrets[0].encryption: invalid value '*********************************': must be 16, 24, or 32 characters long
|
Nits. lgtm. |
|
Added integration test to make sure we can start with OAuth disabled and cert auth still works. [merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1500/) (Image: devenv-fedora_1226) |
Allow secrets to be filled via envvar Default to generating session secrets on master startup
|
Spurious, re[merge] @ncdc not sure if this is related to image changes... looks like test-cmd.sh failed around this call: |
|
Evaluated for origin up to 4457e6f |