diff --git a/README.md b/README.md index 64aeceb..856456a 100644 --- a/README.md +++ b/README.md @@ -1595,4 +1595,124 @@ end > **What's Going On Here?** > -> - This is a very subtle change, but we've added a [safe navigation operator](https://ruby-doc.org/core-2.6/doc/syntax/calling_methods_rdoc.html#label-Safe+navigation+operator) via the `&.user` call. This is because `ActiveSession.find_by(id: session[:current_active_session_id])` can now return `nil` since we're able to delete other `active_session` records. \ No newline at end of file +> - This is a very subtle change, but we've added a [safe navigation operator](https://ruby-doc.org/core-2.6/doc/syntax/calling_methods_rdoc.html#label-Safe+navigation+operator) via the `&.user` call. This is because `ActiveSession.find_by(id: session[:current_active_session_id])` can now return `nil` since we're able to delete other `active_session` records. + +## Step 21: Refactor Remember Logic + +Since we're now associating our sessions with an `active_session` and not a `user`, we'll want to remove the `remember_token` token from the `users` table and onto the `active_sessions`. + +1. Move remember_token column from users to active_sessions table. + +```bash +rails g migration move_remember_token_from_users_to_active_sessions +``` + +```ruby +# db/migrate/[timestamp]_move_remember_token_from_users_to_active_sessions.rb +class MoveRememberTokenFromUsersToActiveSessions < ActiveRecord::Migration[6.1] + def change + remove_column :users, :remember_token + add_column :active_sessions, :remember_token, :string, null: false + + add_index :active_sessions, :remember_token, unique: true + end +end +``` + +> **What's Going On Here?** +> +> - We add `null: false` to ensure this column always has a value. +> - We add a [unique index](https://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/Table.html#method-i-index) to ensure this column has unique data. + +2. Update User Model. + +```diff + class User < ApplicationRecord + ... +- has_secure_password + ... + end +``` + +3. Update Active Session Model. + +```ruby +# app/models/active_session.rb +class ActiveSession < ApplicationRecord + ... + has_secure_token :remember_token +end +``` + +> **What's Going On Here?** +> +> - We call [has_secure_token](https://api.rubyonrails.org/classes/ActiveRecord/SecureToken/ClassMethods.html#method-i-has_secure_token) on the `remember_token`. This ensures that the value for this column will be set when the record is created. This value will be used later to securely identify the user. +> - Note that we remove this from the `user` model. + +4. Refactor the Authentication Concern. + +```ruby +# app/controllers/concerns/authentication.rb +module Authentication + ... + def login(user) + reset_session + active_session = user.active_sessions.create!(user_agent: request.user_agent, ip_address: request.ip) + session[:current_active_session_id] = active_session.id + + active_session + end + + def forget(user) + cookies.delete :remember_token + end + ... + def remember(active_session) + cookies.permanent.encrypted[:remember_token] = active_session.remember_token + end + ... + private + + def current_user + Current.user = if session[:current_active_session_id].present? + ActiveSession.find_by(id: session[:current_active_session_id])&.user + elsif cookies.permanent.encrypted[:remember_token].present? + ActiveSession.find_by(remember_token: cookies.permanent.encrypted[:remember_token])&.user + end + end + ... +end +``` + +> **What's Going On Here?** +> +> - The `login` method now returns the `active_session`. This will be used later when calling `SessionsController#create`. +> - The `forget` method simply deletes the `cookie`. We don't need to call `active_session.regenerate_remember_token` since the `active_session` will be deleted, and therefor cannot be referenced again. +> - The `remember` method now accepts an `active_session` and not a `user`. We do not need to call `active_session.regenerate_remember_token` since a new `active_session` record will be created each time a user logs in. Note that we now save `active_session.remember_token` to the cookie. +> - The `current_user` method now finds the `active_session` record if the `remember_token` is present and returns the user via the [safe navigation operator](https://ruby-doc.org/core-2.6/doc/syntax/calling_methods_rdoc.html#label-Safe+navigation+operator). + +5. Refactor the Sessions Controller. + +```ruby +# app/controllers/sessions_controller.rb +class SessionsController < ApplicationController + def create + ... + if @user + if @user.unconfirmed? + ... + else + ... + active_session = login @user + remember(active_session) if params[:user][:remember_me] == "1" + end + else + ... + end + end +end +``` + +> **What's Going On Here?** +> +> - Since the `login` method now returns an `active_session`, we can take that value and pass it to `remember`. \ No newline at end of file diff --git a/app/controllers/active_sessions_controller.rb b/app/controllers/active_sessions_controller.rb index d3f14db..61e05a2 100644 --- a/app/controllers/active_sessions_controller.rb +++ b/app/controllers/active_sessions_controller.rb @@ -2,6 +2,7 @@ class ActiveSessionsController < ApplicationController before_action :authenticate_user! def destroy + user = current_user @active_session = current_user.active_sessions.find(params[:id]) @active_session.destroy @@ -9,6 +10,7 @@ def destroy if current_user redirect_to account_path, notice: "Session deleted." else + forget(user) reset_session redirect_to root_path, notice: "Signed out." end @@ -17,6 +19,7 @@ def destroy def destroy_all current_user + forget(current_user) current_user.active_sessions.destroy_all reset_session diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index a8f57db..3c45ea8 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -16,11 +16,12 @@ def login(user) reset_session active_session = user.active_sessions.create!(user_agent: request.user_agent, ip_address: request.ip) session[:current_active_session_id] = active_session.id + + active_session end def forget(user) cookies.delete :remember_token - user.regenerate_remember_token end def logout @@ -33,9 +34,8 @@ def redirect_if_authenticated redirect_to root_path, alert: "You are already logged in." if user_signed_in? end - def remember(user) - user.regenerate_remember_token - cookies.permanent.encrypted[:remember_token] = user.remember_token + def remember(active_session) + cookies.permanent.encrypted[:remember_token] = active_session.remember_token end def store_location @@ -48,7 +48,7 @@ def current_user Current.user = if session[:current_active_session_id].present? ActiveSession.find_by(id: session[:current_active_session_id])&.user elsif cookies.permanent.encrypted[:remember_token].present? - User.find_by(remember_token: cookies.permanent.encrypted[:remember_token]) + ActiveSession.find_by(remember_token: cookies.permanent.encrypted[:remember_token])&.user end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 91743b5..9c4b52c 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -9,8 +9,8 @@ def create redirect_to new_confirmation_path, alert: "Incorrect email or password." else after_login_path = session[:user_return_to] || root_path - login @user - remember(@user) if params[:user][:remember_me] == "1" + active_session = login @user + remember(active_session) if params[:user][:remember_me] == "1" redirect_to after_login_path, notice: "Signed in." end else diff --git a/app/models/active_session.rb b/app/models/active_session.rb index 4e570f2..27d939a 100644 --- a/app/models/active_session.rb +++ b/app/models/active_session.rb @@ -1,3 +1,5 @@ class ActiveSession < ApplicationRecord belongs_to :user + + has_secure_token :remember_token end diff --git a/app/models/user.rb b/app/models/user.rb index 3fdb708..578e289 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -6,7 +6,6 @@ class User < ApplicationRecord attr_accessor :current_password has_secure_password - has_secure_token :remember_token has_many :active_sessions, dependent: :destroy diff --git a/db/migrate/20220204201046_move_remember_token_from_users_to_active_sessions.rb b/db/migrate/20220204201046_move_remember_token_from_users_to_active_sessions.rb new file mode 100644 index 0000000..3fdd0ec --- /dev/null +++ b/db/migrate/20220204201046_move_remember_token_from_users_to_active_sessions.rb @@ -0,0 +1,10 @@ +# TODO: Remove comment +# rails g migration move_remember_token_from_users_to_active_sessions +class MoveRememberTokenFromUsersToActiveSessions < ActiveRecord::Migration[6.1] + def change + remove_column :users, :remember_token + add_column :active_sessions, :remember_token, :string, null: false + + add_index :active_sessions, :remember_token, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 0cbb61a..c94db43 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_02_01_102359) do +ActiveRecord::Schema.define(version: 2022_02_04_201046) do create_table "active_sessions", force: :cascade do |t| t.integer "user_id", null: false @@ -18,6 +18,8 @@ t.datetime "updated_at", precision: 6, null: false t.string "user_agent" t.string "ip_address" + t.string "remember_token", null: false + t.index ["remember_token"], name: "index_active_sessions_on_remember_token", unique: true t.index ["user_id"], name: "index_active_sessions_on_user_id" end @@ -28,9 +30,7 @@ t.datetime "confirmed_at" t.string "password_digest", null: false t.string "unconfirmed_email" - t.string "remember_token", null: false t.index ["email"], name: "index_users_on_email", unique: true - t.index ["remember_token"], name: "index_users_on_remember_token", unique: true end add_foreign_key "active_sessions", "users", on_delete: :cascade diff --git a/test/controllers/active_sessions_controller_test.rb b/test/controllers/active_sessions_controller_test.rb index 3696444..9222c18 100644 --- a/test/controllers/active_sessions_controller_test.rb +++ b/test/controllers/active_sessions_controller_test.rb @@ -18,6 +18,18 @@ class ActiveSessionsControllerTest < ActionDispatch::IntegrationTest assert_not_nil flash[:notice] end + test "should destroy all active sessions and forget active sessions" do + login @confirmed_user, remember_user: true + @confirmed_user.active_sessions.create! + + assert_difference("ActiveSession.count", -2) do + delete destroy_all_active_sessions_path + end + + assert_nil current_user + assert cookies[:remember_token].blank? + end + test "should destroy another session" do login @confirmed_user @confirmed_user.active_sessions.create! @@ -42,4 +54,15 @@ class ActiveSessionsControllerTest < ActionDispatch::IntegrationTest assert_nil current_user assert_not_nil flash[:notice] end + + test "should destroy current session and forget current active session" do + login @confirmed_user, remember_user: true + + assert_difference("ActiveSession.count", -1) do + delete active_session_path(@confirmed_user.active_sessions.last) + end + + assert_nil current_user + assert cookies[:remember_token].blank? + end end diff --git a/test/test_helper.rb b/test/test_helper.rb index dbc9d51..7f1b170 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -11,7 +11,12 @@ class ActiveSupport::TestCase # Add more helper methods to be used by all tests here... def current_user - session[:current_active_session_id] && ActiveSession.find_by(id: session[:current_active_session_id])&.user + if session[:current_active_session_id].present? + ActiveSession.find_by(id: session[:current_active_session_id])&.user + else + cookies[:remember_token].present? + ActiveSession.find_by(remember_token: cookies[:remember_token])&.user + end end def login(user, remember_user: nil)