Skip to content

Forget user when deleting an associated active session #80

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

Merged
merged 1 commit into from
Feb 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 121 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
> - 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`.
3 changes: 3 additions & 0 deletions app/controllers/active_sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ class ActiveSessionsController < ApplicationController
before_action :authenticate_user!

def destroy
user = current_user
@active_session = current_user.active_sessions.find(params[:id])

@active_session.destroy

if current_user
redirect_to account_path, notice: "Session deleted."
else
forget(user)
reset_session
redirect_to root_path, notice: "Signed out."
end
Expand All @@ -17,6 +19,7 @@ def destroy
def destroy_all
current_user

forget(current_user)
current_user.active_sessions.destroy_all
reset_session

Expand Down
10 changes: 5 additions & 5 deletions app/controllers/concerns/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions app/models/active_session.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
class ActiveSession < ApplicationRecord
belongs_to :user

has_secure_token :remember_token
end
1 change: 0 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions test/controllers/active_sessions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand All @@ -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
7 changes: 6 additions & 1 deletion test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down