From b2e18aeb72c69ac21dd178b20dad0cb6a5710d79 Mon Sep 17 00:00:00 2001 From: Andy Zhao Date: Tue, 12 Nov 2019 11:10:34 -0500 Subject: [PATCH] Use Redis for session storage (#4004) [deploy] * WIP use redis-session-store for sessions * Use Redis as session storage * Add redis-server to Travis * Use integer for SESSION_EXPIRY_SECONDS env variable Co-Authored-By: rhymes * Remove sessions b/c no custom sessions logic * Use ApplicationConfig instead * Rename default value * Remove rememberable module * Persist the user for the test * Remove remember_me related tests * Revert 'undo remember_me' commits * Add redis to procfile * Cleanup devise configuration * Move REDIS configuration in the basic configuration section * Make sure the expiration time can't zero * Restore old order and remove useless comment line * Delete legacy session cookie after login Once deployed the server will start using the new session cookie, this makes sure the legacy one is deleted on the user's browser * Remove redis from Procfile * Add signed, secure and httponly to the Redis session cookie --- .travis.yml | 2 ++ Envfile | 3 ++- app/controllers/async_info_controller.rb | 1 - app/controllers/omniauth_callbacks_controller.rb | 6 ++++++ app/models/user.rb | 4 ++-- config/initializers/devise.rb | 8 +++----- config/initializers/session_store.rb | 15 ++++++++++++++- config/routes.rb | 1 - spec/requests/api_secrets_create_spec.rb | 2 +- 9 files changed, 30 insertions(+), 12 deletions(-) diff --git a/.travis.yml b/.travis.yml index b7e3d757d..e3eafe280 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,6 +10,8 @@ rvm: addons: postgresql: '9.6' chrome: 'stable' +services: + - redis-server env: global: - RAILS_ENV=test diff --git a/Envfile b/Envfile index 9dd4d2b0a..0fd4a4173 100644 --- a/Envfile +++ b/Envfile @@ -37,6 +37,8 @@ variable :REDIS_URL, :String, default: "redis://localhost:6379" # Redis sessions storage variable :REDIS_SESSIONS_URL, :String, default: "redis://localhost:6379" +variable :SESSION_KEY, :String, default: "_Dev_Community_Session" +variable :SESSION_EXPIRY_SECONDS, :Integer, default: 1209600 # two weeks in seconds ################################################ ############## 3rd Party Services ############## @@ -218,5 +220,4 @@ group :production do # Heroku variable :HEROKU_APP_URL, :String # practicaldev.herokuapp.com - end diff --git a/app/controllers/async_info_controller.rb b/app/controllers/async_info_controller.rb index 56f5a64f3..8cb4df79b 100644 --- a/app/controllers/async_info_controller.rb +++ b/app/controllers/async_info_controller.rb @@ -17,7 +17,6 @@ class AsyncInfoController < ApplicationController remember_me(current_user) end @user = current_user.decorate - # Updates article analytics periodically: occasionally_update_analytics respond_to do |format| format.json do diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 1f77f0b59..ef1bf1c4f 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -1,4 +1,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController + LEGACY_COOKIE_NAME = "_PracticalDeveloper_session".freeze + # Don't need a policy for this since this is our sign up/in route include Devise::Controllers::Rememberable @@ -27,7 +29,11 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController def callback_for(provider) cta_variant = request.env["omniauth.params"]["state"].to_s @user = AuthorizationService.new(request.env["omniauth.auth"], current_user, cta_variant).get_user + if persisted_and_valid? + # delete legacy session based cookie once the user has logged in again + request.cookie_jar.delete(LEGACY_COOKIE_NAME) if request.cookies[LEGACY_COOKIE_NAME] + remember_me(@user) sign_in_and_redirect @user, event: :authentication set_flash_message(:notice, :success, kind: provider.to_s.capitalize) if is_navigational_format? diff --git a/app/models/user.rb b/app/models/user.rb index e32d3cf27..878c15043 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -54,8 +54,8 @@ class User < ApplicationRecord mount_uploader :profile_image, ProfileImageUploader - devise :omniauthable, :rememberable, - :registerable, :database_authenticatable, :confirmable + devise :omniauthable, :registerable, :database_authenticatable, :confirmable, :rememberable + validates :email, length: { maximum: 50 }, email: true, diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index d51d549da..b07e1f643 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -127,15 +127,13 @@ Devise.setup do |config| # ==> Configuration for :rememberable # The time the user will be remembered without asking for credentials again. - config.remember_for = 26.weeks - config.extend_remember_period = true - config.timeout_in = 20.weeks + config.remember_for = 26.weeks # 6 months and a half # Invalidates all the remember me tokens when the user signs out. - config.expire_all_remember_me_on_sign_out = false + config.expire_all_remember_me_on_sign_out = true # If true, extends the user's remember period when remembered via cookie. - # config.extend_remember_period = false + config.extend_remember_period = true # Options to be passed to the created cookie. For instance, you can set # secure: true in order to force SSL only cookies. diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index cb4d19f53..5d39a79b9 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -1,3 +1,16 @@ # Be sure to restart your server when you modify this file. -Rails.application.config.session_store :cookie_store, key: "_PracticalDeveloper_session" +# we want a default in case the expiration is not set or set to 0 +# because 0 is an invalid value +app_config_expires_after = ApplicationConfig["SESSION_EXPIRY_SECONDS"].to_i +expires_after = app_config_expires_after.positive? ? app_config_expires_after : 2.weeks.to_i + +# see for configuration options, +# httponly has been added in +Rails.application.config.session_store :redis_store, + key: ApplicationConfig["SESSION_KEY"], + servers: ApplicationConfig["REDIS_URL"], + expire_after: expires_after, + signed: true, + secure: Rails.env.production?, + httponly: true diff --git a/config/routes.rb b/config/routes.rb index a3f43ca26..3158bd728 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -7,7 +7,6 @@ Rails.application.routes.draw do devise_for :users, controllers: { omniauth_callbacks: "omniauth_callbacks", - session: "sessions", registrations: "registrations" } diff --git a/spec/requests/api_secrets_create_spec.rb b/spec/requests/api_secrets_create_spec.rb index 005d1b294..6a5cf8664 100644 --- a/spec/requests/api_secrets_create_spec.rb +++ b/spec/requests/api_secrets_create_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" RSpec.describe "ApiSecretsCreate", type: :request do - let(:user) { build(:user) } + let(:user) { create(:user) } before { sign_in user }