Conversation
secret_key_base was added in Rails 4 and replaces secret_token. It is used by CookieStore to encrypt cookies (we are not currently using CookieStore, but we will switch to it in #162). The Rails 4 convention for setting secret_key_base is to use config/secrets.yml and the environment variable SECRET_KEY_BASE (see [1]). We don't use this approach, because secrets.yml will be deprecated in favour of credentials.yml.enc in Rails 5.2 [2] and because setting the environment variable is awkward. Instead, we generate a random secret the first time the app runs and store it in config/secret_key_base.#{env}.txt. We also support the environment variable SECRET_KEY_BASE to match the Rails convention; we don't currently use this environment variable, but it may be useful (e.g. in situations such as build steps where we don't want to generate the file, or in environments such as Docker where environment variables are often more convenient than files). We don't store the secret in the database (which is how the existing secret_token is implemented) because it may be useful to start the app without a database connection. Also, it's probably more secure to keep the secret out of the database (and its backups). This commit also adds a rule to .gitignore to make sure the secrets aren't accidentally committed. [1]: https://guides.rubyonrails.org/v4.1.8/upgrading_ruby_on_rails.html#config-secrets-yml [2]: https://www.github.com/rails/rails/pull/30067
It has been replaced by secret_key_base, and doesn't appear to be used anywhere because we are not using CookieStore yet. Also rename the file because it is no longer related to secret_token. The setting sessions/secret_token should eventually be deleted manually using the admin web interface, but note that this setting is still used in config/initializers/devise.rb at the moment.
tom93
force-pushed
the
rails-4-secret-key-base
branch
from
46095ef to
1f71cc8
Compare
Contributor
|
tested this fairly throughly in the |
Contributor
Author
|
Thanks! |
bagedevimo
previously approved these changes
Dec 21, 2023
Holmes98
previously approved these changes
Dec 21, 2023
Member
There was a problem hiding this comment.
Note that this invalidates Devise remember tokens, so we should deploy at the same time as #162 (which invalidates sessions) on second thought, I don't think this matters (but it does appear to invalidate remember tokens).
I was wrong, we *are* using CookieStore (just not for the session store). We need to keep secret_token for now, to allow automatic migration from the legacy signed-but-not-encrypted cookies to the new store encrypted using secret_key_base. The cookies are used for the "remember me" functionality for example.
Contributor
Author
|
Oops. As per the chat, I made a mistake -- we were already using CookieJar, just not for the session. So removing secret_token breaks e.g. the "remember me" feature because the signed cookie can't be verified without the old secret_token. I reverted that commit, to fix the issue and allow automatic migration. We should keep secret_token for a while to give users a chance to open the site and get migrated. So this should be good to merge now (let me squash and edit the commit message to exclude the secret_token stuff). |
tom93
changed the title
Holmes98
approved these changes
Dec 21, 2023
Holmes98
pushed a commit
to Holmes98/nztrain
that referenced
this pull request
Dec 21, 2023
secret_key_base was added in Rails 4 and replaces secret_token. It is used by CookieStore to encrypt cookies (we are not currently using CookieStore, but we will switch to it in NZOI#162). The Rails 4 convention for setting secret_key_base is to use config/secrets.yml and the environment variable SECRET_KEY_BASE (see [1]). We don't use this approach, because secrets.yml will be deprecated in favour of credentials.yml.enc in Rails 5.2 [2] and because setting the environment variable is awkward. Instead, we generate a random secret the first time the app runs and store it in config/secret_key_base.#{env}.txt. We also support the environment variable SECRET_KEY_BASE to match the Rails convention; we don't currently use this environment variable, but it may be useful (e.g. in situations such as build steps where we don't want to generate the file, or in environments such as Docker where environment variables are often more convenient than files). We don't store the secret in the database (which is how the existing secret_token is implemented) because it may be useful to start the app without a database connection. Also, it's probably more secure to keep the secret out of the database (and its backups). This commit also adds a rule to .gitignore to make sure the secrets aren't accidentally committed. [1]: https://guides.rubyonrails.org/v4.1.8/upgrading_ruby_on_rails.html#config-secrets-yml [2]: https://www.github.com/rails/rails/pull/30067
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
secret_key_base was added in Rails 4 and replaces secret_token.
This PR is intended to merge using the "Rebase" strategy.Update: I reverted the second commit (see below), so now should use squash & merge and clean up the commit message.