-
Notifications
You must be signed in to change notification settings - Fork 11
Switch to the default CookieJar session store #162
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
Switch to the default CookieJar session store #162
Conversation
580f6e3 to
b0e44ca
Compare
|
Merge #161 first |
386ac96 to
60d8d77
Compare
Deploying this will log everyone out, so we may want to consider an intermediate migration step.
60d8d77 to
ae97aae
Compare
|
Tested by logging in ticking remember me, stopping the rails server, exiting my browser. change to this branch, start rails, start browser and verify still logged in. The cookie works fine for the non-session remember me, but if you don't click remember me your session will be lost regardless of the browser, etc. |
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
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
|
Merge #195 first, to skip the legacy signed-but-not-encrypted cookie store mode. |
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 decided over Discord not to bother with a migration or a notice about being signed out (if they didn't tick "remember me").
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
We forgot to update Gemfile.lock in #162.
We forgot to update Gemfile.lock in #162.
|
This has been deployed just now, so all users that didn't tick "remember me" will be signed out. |
|
It appears that users might get signed out even if they ticked "remember me". We are not quite sure why. |
|
It turns out that the sign-out issue is because by default Devise sets the "remember me" cookie to expire after 2 weeks, which is very short. In future we should probably increase this duration, and/or set |
Deploying this will log everyone out, so we may want to consider an intermediate migration step.