Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix/session timeout #304

Closed
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
1 change: 1 addition & 0 deletions lib/sorcery/controller/submodules/session_timeout.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def register_login_time(_user, _credentials = nil)
def validate_session
session_to_use = Config.session_timeout_from_last_action ? session[:last_action_time] : session[:login_time]
if (session_to_use && sorcery_session_expired?(session_to_use.to_time)) || sorcery_session_invalidated?
forget_me! if current_user.present? && Config.submodules.include?(:remember_me)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make more sense to just call logout here?
There might be other before/after logout hooks configured and logout already takes care of calling all of those.
Just discovered this PR after I ran into the same problem and opened my PR with logout here:
#358

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @jankoegel i saw you PR, yes is better then mine. Makes more sense to call logout. Can i add the tests in your PR?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I invited you as a collaborator to my fork. Let me know if that works for you & thanks for preparing the specs!

reset_sorcery_session
remove_instance_variable :@current_user if defined? @current_user
else
Expand Down
57 changes: 56 additions & 1 deletion spec/controllers/controller_session_timeout_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'spec_helper'

describe SorceryController, type: :controller do
let!(:user) { double('user', id: 42) }
let!(:user) { double('user', id: 42, password: 'secret') }

# ----------------- SESSION TIMEOUT -----------------------
context 'with session timeout features' do
Expand Down Expand Up @@ -36,6 +36,61 @@
expect(response).to be_a_redirect
end

context 'with remember_me module' do
before(:example) do
if SORCERY_ORM == :active_record
MigrationHelper.migrate("#{Rails.root}/db/migrate/remember_me")
User.reset_column_information
end

sorcery_reload!([:remember_me, :session_timeout])

allow(user).to receive(:remember_me_token)
allow(user).to receive(:forget_me!)
allow(user).to receive(:remember_me_token_expires_at)
allow(user).to receive_message_chain(:sorcery_config, :remember_me_token_attribute_name).and_return(:remember_me_token)
allow(user).to receive_message_chain(:sorcery_config, :remember_me_token_expires_at_attribute_name).and_return(:remember_me_token_expires_at)
end

after(:example) do
if SORCERY_ORM == :active_record
MigrationHelper.rollback("#{Rails.root}/db/migrate/remember_me")
end

sorcery_reload!([:session_timeout])
end

it 'resets session and remember_me cookies after session timeout' do
expect(User).to receive(:authenticate).with('bla@bla.com', 'secret', '1') { |&block| block.call(user, nil) }
expect(user).to receive(:remember_me!)
expect(user).to receive(:remember_me_token).and_return('abracadabra')

post :test_login_with_remember_in_login, params: { email: 'bla@bla.com', password: 'secret', remember: '1' }

Timecop.travel(Time.now.in_time_zone + 0.6)

get :test_login_from_cookie

expect(cookies['remember_me_token']).to be_nil
expect(session[:user_id]).to be_nil
end

it 'does not resets session and remember_me cookies after session timeout' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this test supposed to be "does not reset session and remember_me cookies before session timeout"?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i will change

expect(User).to receive(:authenticate).with('bla@bla.com', 'secret', '1') { |&block| block.call(user, nil) }
expect(user).to receive(:remember_me!)
expect(user).to receive(:remember_me_token).and_return('abracadabra')

post :test_login_with_remember_in_login, params: { email: 'bla@bla.com', password: 'secret', remember: '1' }

Timecop.travel(Time.now.in_time_zone + 0.4)

get :test_login_from_cookie

expect(cookies['remember_me_token']).to be_present
expect(session[:user_id]).to be_present
end
end

context "with 'invalidate_active_sessions_enabled'" do
it 'does not reset the session if invalidate_sessions_before is nil' do
sorcery_controller_property_set(:session_timeout_invalidate_active_sessions_enabled, true)
Expand Down