-
Notifications
You must be signed in to change notification settings - Fork 116
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
Allow SAML secrets to be rotated #1977
Conversation
Gemfile
Outdated
@@ -42,7 +42,7 @@ gem 'redis-session-store', github: '18F/redis-session-store', branch: 'master' | |||
gem 'rqrcode' | |||
gem 'ruby-progressbar' | |||
gem 'ruby-saml' | |||
gem 'saml_idp', git: 'https://github.com/18F/saml_idp.git', tag: 'v0.5.0-18f' | |||
gem 'saml_idp', git: 'https://github.com/18F/saml_idp.git', branch: 'jmhooper-idp-cert-and-secret' |
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.
We will want to switch this to a new tag once this 18F/saml_idp#15 is merged. Until that happens, this is blocked.
Note to self: once #1976 is merged, I'll need to merge the changes to the dockerfile and add the new saml2018 cert and key there. |
The update to the |
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.
LGTM. Let's make sure the necessary Figaro configs are added to dev and qa before merging. Note that dev and qa are currently broken because of the webpacker change. We will have to wait until RC 51 is deployed before we can deploy the webpacker fixes to the lower envs.
There shouldn't be any required configs for this. We'll only need to flip the feature flag and set the other configs when we want to start rotating certs. |
a1fe185
to
d82d2e7
Compare
**Why**: Because our SAML certificate is expiring soon and we should also have the ability to occasionally rotate our secrets **How**: This commit allows for the app to be put into a saml secret rotation mode by setting the following env vars in `application.yml`: - saml_secret_rotation_certificate - saml_secret_rotation_enabled - saml_secret_rotation_path_suffix - saml_secret_rotation_secret_key - saml_secret_rotation_secret_key_password A new key and certificate will also need to be added to the apps. Once the app is in secret rotation mode, new saml endpoint will be available with the suffic specified by `saml_secret_rotation_path_suffix`. For example, if the suffix is `2018`, `/api/saml/metadata2018`, `/api/saml/logout2018`, and `/api/saml/auth2018` will be made available. These endpoints will use the new secrets. SAML SPs will be able to begin using these endpoints. Once all SPs have moved to the new endpoints or the old cert expires, the old cert can be replaced by the new certs and the SPs can begin to switch back to the original SAML endpoints and the app can be taken out of saml secret rotation mode.
Why: Because our SAML certificate is expiring soon and we should
also have the ability to occasionally rotate our secrets
How: This commit allows for the app to be put into a saml secret
rotation mode by setting the following env vars in
application.yml
:A new key and certificate will also need to be added to the apps.
Once the app is in secret rotation mode, new saml endpoint will be
available with the suffic specified by
saml_secret_rotation_path_suffix
. For example, if the suffix is2018
,/api/saml/metadata2018
,/api/saml/logout2018
, and/api/saml/auth2018
will be made available. These endpoints will usethe new secrets. SAML SPs will be able to begin using these endpoints.
Once all SPs have moved to the new endpoints or the old cert expires,
the old cert can be replaced by the new certs and the SPs can begin to
switch back to the original SAML endpoints and the app can be taken out
of saml secret rotation mode.
Hi! Before submitting your PR for review, and/or before merging it, please
go through the following checklist:
For DB changes, check for missing indexes, check to see if the changes
affect other apps (such as the dashboard), make sure the DB columns in the
various environments are properly populated, coordinate with devops, plan
migrations in separate steps.
For route changes, make sure GET requests don't change state or result in
destructive behavior. GET requests should only result in information being
read, not written.
For encryption changes, make sure it is compatible with data that was
encrypted with the old code.
For secrets changes, make sure to update the S3 secrets bucket with the
new configs in all environments.
Do not disable Rubocop or Reek offenses unless you are absolutely sure
they are false positives. If you're not sure how to fix the offense, please
ask a teammate.
When reading data, write tests for nil values, empty strings,
and invalid formats.
When calling
redirect_to
in a controller, use_url
, not_path
.When adding user data to the session, use the
user_session
helperinstead of the
session
helper so the data does not persist beyond the user'ssession.
When adding a new controller that requires the user to be fully
authenticated, make sure to add
before_action :confirm_two_factor_authenticated
.