Skip to content

Commit

Permalink
feat: separate touching & validating auth sessions
Browse files Browse the repository at this point in the history
* Validation will happen, as always, at the start of the request
* Touching will happen after the request
  • Loading branch information
adamcooke committed May 2, 2022
1 parent fd7f70e commit e688762
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 16 deletions.
19 changes: 13 additions & 6 deletions lib/authie/controller_delegate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,25 @@ def set_browser_id
proposed_browser_id
end

# Touch the session on each request to ensure that it is validated and all last activity
# information is updated. This will return the session if one has been touched otherwise
# it will reteurn false if there is no session/not logged in. It is safe to run this on
# all requests even if there is no session.
# Validate the auth session to ensure that it is current validate and raise an error if it
# is not suitable for use.
#
# @return [Authie::Session, false]
def touch_auth_session
return auth_session.touch if logged_in?
def validate_auth_session
return auth_session.validate if logged_in?

false
end

# Touch the session to update details on the latest activity.
#
# @return [Authie::Session, false]
def touch_auth_session
yield if block_given?
ensure
auth_session.touch if logged_in?
end

# Return the user for the currently logged in user or nil if no user is logged in
#
# @return [ActiveRecord::Base, nil]
Expand Down
4 changes: 3 additions & 1 deletion lib/authie/controller_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ module ControllerExtension
class << self
def included(base)
base.helper_method :logged_in?, :current_user, :auth_session
base.before_action :set_browser_id, :touch_auth_session
base.before_action :set_browser_id, :validate_auth_session
base.around_action :touch_auth_session

base.delegate :set_browser_id, to: :auth_session_delegate
base.delegate :validate_auth_session, to: :auth_session_delegate
base.delegate :touch_auth_session, to: :auth_session_delegate
base.delegate :current_user, to: :auth_session_delegate
base.delegate :create_auth_session, to: :auth_session_delegate
Expand Down
1 change: 0 additions & 1 deletion lib/authie/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ def invalidate
# @raises [ActiveRecord::RecordInvalid]
# @return [Authie::Session]
def touch
validate
@session.last_activity_at = Time.now
@session.last_activity_ip = @controller.request.ip
@session.last_activity_path = @controller.request.path
Expand Down
4 changes: 4 additions & 0 deletions spec/dummy/app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,8 @@ def logged_in
render plain: 'Not logged in'
end
end

def error
1 / 0
end
end
1 change: 1 addition & 0 deletions spec/dummy/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
get '/authenticated', to: 'pages#authenticated'
get '/logged_in', to: 'pages#logged_in'
get '/request_count', to: 'pages#request_count'
get '/error', to: 'pages#error'
end
13 changes: 12 additions & 1 deletion spec/integration/controller_extension_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,21 @@
setup_session
3.times do |i|
get :request_count
expect(response.body).to eq "Count: #{i + 1}"
expect(response.body).to eq "Count: #{i}"
end
end

it 'touches the session even if there is an error' do
session = setup_session
time = Time.new(2022, 2, 4, 2, 11)
Timecop.freeze(time) do
expect { get :error }.to raise_error ZeroDivisionError
end
session.reload
expect(session.last_activity_path).to eq '/error'
expect(session.last_activity_at).to eq time
end

it 'raises an error if the browser ID mismatches' do
setup_session { |s| s.browser_id = 'abc' }
expect { get(:authenticated) }.to raise_error Authie::Session::BrowserMismatch
Expand Down
20 changes: 15 additions & 5 deletions spec/lib/controller_delegate_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,19 @@
end

describe '#touch_auth_session' do
it 'will call the touch method on the underlying session and return its result' do
expect(delegate.auth_session).to receive(:touch).and_return 1234
expect(delegate.touch_auth_session).to eq 1234
it 'will call the touch method on the underlying' do
expect(delegate.auth_session).to receive(:touch)
delegate.touch_auth_session
end

it 'will call a block before running touch' do
count = 0
delegate.touch_auth_session { count += 1 }
expect(count).to eq 1
end

it 'will return the return value of the executed block' do
expect(delegate.touch_auth_session { 1234 }).to eq 1234
end
end
end
Expand All @@ -70,8 +80,8 @@
end

describe '#touch_auth_session' do
it 'will return false' do
expect(delegate.touch_auth_session).to be false
it 'will execute the block and return the value' do
expect(delegate.touch_auth_session { 'abcdef' }).to eq 'abcdef'
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/session_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@
end

describe '#touch' do
it 'calls the validate method' do
expect(session).to receive(:validate).and_return true
it 'not call the validate method as it previously did' do
expect(session).to_not receive(:validate)
session.touch
end

Expand Down

0 comments on commit e688762

Please sign in to comment.