Skip to content

Commit

Permalink
Forget user when deleting an associated active session
Browse files Browse the repository at this point in the history
When deleting an active session I want to ensure the associated session is forgotten so that I can ensure my account it safe.

Issues
------
- Closes #78
  • Loading branch information
stevepolitodesign committed Feb 5, 2022
1 parent e68248c commit f1ad3c7
Show file tree
Hide file tree
Showing 10 changed files with 175 additions and 13 deletions.
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

0 comments on commit f1ad3c7

Please sign in to comment.