Skip to content

Commit 81c1894

Browse files
Forget user when deleting an associated active session
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
1 parent 2b74b3e commit 81c1894

10 files changed

+175
-13
lines changed

README.md

+121-1
Original file line numberDiff line numberDiff line change
@@ -1595,4 +1595,124 @@ end
15951595

15961596
> **What's Going On Here?**
15971597
>
1598-
> - 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.
1598+
> - 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.
1599+
1600+
## Step 21: Refactor Remember Logic
1601+
1602+
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`.
1603+
1604+
1. Move remember_token column from users to active_sessions table.
1605+
1606+
```bash
1607+
rails g migration move_remember_token_from_users_to_active_sessions
1608+
```
1609+
1610+
```ruby
1611+
# db/migrate/[timestamp]_move_remember_token_from_users_to_active_sessions.rb
1612+
class MoveRememberTokenFromUsersToActiveSessions < ActiveRecord::Migration[6.1]
1613+
def change
1614+
remove_column :users, :remember_token
1615+
add_column :active_sessions, :remember_token, :string, null: false
1616+
1617+
add_index :active_sessions, :remember_token, unique: true
1618+
end
1619+
end
1620+
```
1621+
1622+
> **What's Going On Here?**
1623+
>
1624+
> - We add `null: false` to ensure this column always has a value.
1625+
> - We add a [unique index](https://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/Table.html#method-i-index) to ensure this column has unique data.
1626+
1627+
2. Update User Model.
1628+
1629+
```diff
1630+
class User < ApplicationRecord
1631+
...
1632+
- has_secure_password
1633+
...
1634+
end
1635+
```
1636+
1637+
3. Update Active Session Model.
1638+
1639+
```ruby
1640+
# app/models/active_session.rb
1641+
class ActiveSession < ApplicationRecord
1642+
...
1643+
has_secure_token :remember_token
1644+
end
1645+
```
1646+
1647+
> **What's Going On Here?**
1648+
>
1649+
> - 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.
1650+
> - Note that we remove this from the `user` model.
1651+
1652+
4. Refactor the Authentication Concern.
1653+
1654+
```ruby
1655+
# app/controllers/concerns/authentication.rb
1656+
module Authentication
1657+
...
1658+
def login(user)
1659+
reset_session
1660+
active_session = user.active_sessions.create!(user_agent: request.user_agent, ip_address: request.ip)
1661+
session[:current_active_session_id] = active_session.id
1662+
1663+
active_session
1664+
end
1665+
1666+
def forget(user)
1667+
cookies.delete :remember_token
1668+
end
1669+
...
1670+
def remember(active_session)
1671+
cookies.permanent.encrypted[:remember_token] = active_session.remember_token
1672+
end
1673+
...
1674+
private
1675+
1676+
def current_user
1677+
Current.user = if session[:current_active_session_id].present?
1678+
ActiveSession.find_by(id: session[:current_active_session_id])&.user
1679+
elsif cookies.permanent.encrypted[:remember_token].present?
1680+
ActiveSession.find_by(remember_token: cookies.permanent.encrypted[:remember_token])&.user
1681+
end
1682+
end
1683+
...
1684+
end
1685+
```
1686+
1687+
> **What's Going On Here?**
1688+
>
1689+
> - The `login` method now returns the `active_session`. This will be used later when calling `SessionsController#create`.
1690+
> - 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.
1691+
> - 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.
1692+
> - 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).
1693+
1694+
5. Refactor the Sessions Controller.
1695+
1696+
```ruby
1697+
# app/controllers/sessions_controller.rb
1698+
class SessionsController < ApplicationController
1699+
def create
1700+
...
1701+
if @user
1702+
if @user.unconfirmed?
1703+
...
1704+
else
1705+
...
1706+
active_session = login @user
1707+
remember(active_session) if params[:user][:remember_me] == "1"
1708+
end
1709+
else
1710+
...
1711+
end
1712+
end
1713+
end
1714+
```
1715+
1716+
> **What's Going On Here?**
1717+
>
1718+
> - Since the `login` method now returns an `active_session`, we can take that value and pass it to `remember`.

app/controllers/active_sessions_controller.rb

+3
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ class ActiveSessionsController < ApplicationController
22
before_action :authenticate_user!
33

44
def destroy
5+
user = current_user
56
@active_session = current_user.active_sessions.find(params[:id])
67

78
@active_session.destroy
89

910
if current_user
1011
redirect_to account_path, notice: "Session deleted."
1112
else
13+
forget(user)
1214
reset_session
1315
redirect_to root_path, notice: "Signed out."
1416
end
@@ -17,6 +19,7 @@ def destroy
1719
def destroy_all
1820
current_user
1921

22+
forget(current_user)
2023
current_user.active_sessions.destroy_all
2124
reset_session
2225

app/controllers/concerns/authentication.rb

+5-5
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@ def login(user)
1616
reset_session
1717
active_session = user.active_sessions.create!(user_agent: request.user_agent, ip_address: request.ip)
1818
session[:current_active_session_id] = active_session.id
19+
20+
active_session
1921
end
2022

2123
def forget(user)
2224
cookies.delete :remember_token
23-
user.regenerate_remember_token
2425
end
2526

2627
def logout
@@ -33,9 +34,8 @@ def redirect_if_authenticated
3334
redirect_to root_path, alert: "You are already logged in." if user_signed_in?
3435
end
3536

36-
def remember(user)
37-
user.regenerate_remember_token
38-
cookies.permanent.encrypted[:remember_token] = user.remember_token
37+
def remember(active_session)
38+
cookies.permanent.encrypted[:remember_token] = active_session.remember_token
3939
end
4040

4141
def store_location
@@ -48,7 +48,7 @@ def current_user
4848
Current.user = if session[:current_active_session_id].present?
4949
ActiveSession.find_by(id: session[:current_active_session_id])&.user
5050
elsif cookies.permanent.encrypted[:remember_token].present?
51-
User.find_by(remember_token: cookies.permanent.encrypted[:remember_token])
51+
ActiveSession.find_by(remember_token: cookies.permanent.encrypted[:remember_token])&.user
5252
end
5353
end
5454

app/controllers/sessions_controller.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ def create
99
redirect_to new_confirmation_path, alert: "Incorrect email or password."
1010
else
1111
after_login_path = session[:user_return_to] || root_path
12-
login @user
13-
remember(@user) if params[:user][:remember_me] == "1"
12+
active_session = login @user
13+
remember(active_session) if params[:user][:remember_me] == "1"
1414
redirect_to after_login_path, notice: "Signed in."
1515
end
1616
else

app/models/active_session.rb

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
class ActiveSession < ApplicationRecord
22
belongs_to :user
3+
4+
has_secure_token :remember_token
35
end

app/models/user.rb

-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ class User < ApplicationRecord
66
attr_accessor :current_password
77

88
has_secure_password
9-
has_secure_token :remember_token
109

1110
has_many :active_sessions, dependent: :destroy
1211

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# TODO: Remove comment
2+
# rails g migration move_remember_token_from_users_to_active_sessions
3+
class MoveRememberTokenFromUsersToActiveSessions < ActiveRecord::Migration[6.1]
4+
def change
5+
remove_column :users, :remember_token
6+
add_column :active_sessions, :remember_token, :string, null: false
7+
8+
add_index :active_sessions, :remember_token, unique: true
9+
end
10+
end

db/schema.rb

+3-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/controllers/active_sessions_controller_test.rb

+23
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,18 @@ class ActiveSessionsControllerTest < ActionDispatch::IntegrationTest
1818
assert_not_nil flash[:notice]
1919
end
2020

21+
test "should destroy all active sessions and forget active sessions" do
22+
login @confirmed_user, remember_user: true
23+
@confirmed_user.active_sessions.create!
24+
25+
assert_difference("ActiveSession.count", -2) do
26+
delete destroy_all_active_sessions_path
27+
end
28+
29+
assert_nil current_user
30+
assert cookies[:remember_token].blank?
31+
end
32+
2133
test "should destroy another session" do
2234
login @confirmed_user
2335
@confirmed_user.active_sessions.create!
@@ -42,4 +54,15 @@ class ActiveSessionsControllerTest < ActionDispatch::IntegrationTest
4254
assert_nil current_user
4355
assert_not_nil flash[:notice]
4456
end
57+
58+
test "should destroy current session and forget current active session" do
59+
login @confirmed_user, remember_user: true
60+
61+
assert_difference("ActiveSession.count", -1) do
62+
delete active_session_path(@confirmed_user.active_sessions.last)
63+
end
64+
65+
assert_nil current_user
66+
assert cookies[:remember_token].blank?
67+
end
4568
end

test/test_helper.rb

+6-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ class ActiveSupport::TestCase
1111

1212
# Add more helper methods to be used by all tests here...
1313
def current_user
14-
session[:current_active_session_id] && ActiveSession.find_by(id: session[:current_active_session_id])&.user
14+
if session[:current_active_session_id].present?
15+
ActiveSession.find_by(id: session[:current_active_session_id])&.user
16+
else
17+
cookies[:remember_token].present?
18+
ActiveSession.find_by(remember_token: cookies[:remember_token])&.user
19+
end
1520
end
1621

1722
def login(user, remember_user: nil)

0 commit comments

Comments
 (0)