Skip to content

Test/test for session timeout bug #1

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

Open
wants to merge 2 commits into
base: bugfix/session-timeout_validate-session
Choose a base branch
from

Conversation

PedroAugustoRamalhoDuarte
Copy link
Collaborator

Hello @jankoegel,

I haven't been able to determine whether the issue with the test or the code. However, with remember_me module enabled after session timeout test is failing.

I believe this may be due to line 72 in the lib/sorcery/controller.rb file within the logout method. In the logged_in function, we call logout, and because of this, we are unable to reach the before_logout hook.

Let me know if you need further clarification or assistance.

@jankoegel
Copy link
Owner

Thanks for preparing this.
I had trouble running the Sorcery specs as well.

When I run the specs of this PR with Ruby 3.2.2 I get 20 failures [1] in total. The most relevant ones for us are:

rspec ./spec/controllers/controller_session_timeout_spec.rb:132 # SorceryController with session timeout features with 'session_timeout_from_last_action' does not logout if there was activity
rspec ./spec/controllers/controller_session_timeout_spec.rb:194 # SorceryController with session timeout features with remember_me module enable after session timeout resets session and remember_me

Is 132 also failing for you or only 194?

194's error for me is:

Failure/Error: expect(cookies['remember_me_token']).to be_nil

       expected: nil
            got: "BAh..."

Which is weird because I also test resetting og the remember_me_token after invalidation in my app's specs and there it correctly gets reset to "".


[1]

479 examples, 20 failures, 2 pending

Failed examples:

rspec ./spec/controllers/controller_oauth2_spec.rb:31 # SorceryController using create_from creates a new user
rspec ./spec/controllers/controller_oauth2_spec.rb:39 # SorceryController using create_from supports nested attributes
rspec ./spec/controllers/controller_oauth2_spec.rb:47 # SorceryController using create_from does not crash on missing nested attributes
rspec ./spec/controllers/controller_oauth2_spec.rb:57 # SorceryController using create_from with a block does not create user
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:1:2]' # SorceryController OAuth with session timeout features when facebook resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:2:2]' # SorceryController OAuth with session timeout features when github resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:3:2]' # SorceryController OAuth with session timeout features when google resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:4:2]' # SorceryController OAuth with session timeout features when liveid resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:5:2]' # SorceryController OAuth with session timeout features when vk resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:6:2]' # SorceryController OAuth with session timeout features when salesforce resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:7:2]' # SorceryController OAuth with session timeout features when slack resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:8:2]' # SorceryController OAuth with session timeout features when discord resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:9:2]' # SorceryController OAuth with session timeout features when battlenet resets session after session timeout
rspec ./spec/controllers/controller_oauth_spec.rb:136 # SorceryController  using 'create_from' creates a new user
rspec ./spec/controllers/controller_oauth_spec.rb:144 # SorceryController  using 'create_from' supports nested attributes
rspec ./spec/controllers/controller_oauth_spec.rb:152 # SorceryController  using 'create_from' does not crash on missing nested attributes
rspec ./spec/controllers/controller_oauth_spec.rb:172 # SorceryController  using 'create_from' with a block does not create user
rspec ./spec/controllers/controller_session_timeout_spec.rb:132 # SorceryController with session timeout features with 'session_timeout_from_last_action' does not logout if there was activity
rspec ./spec/controllers/controller_session_timeout_spec.rb:194 # SorceryController with session timeout features with remember_me module enable after session timeout resets session and remember_me
rspec ./spec/controllers/controller_spec.rb:67 # SorceryController when activated with sorcery #login when succeeds sets csrf token in session

@jankoegel
Copy link
Owner

re:
https://github.com/Sorcery/sorcery/blob/d9dc0bd80a3d5689398baea4489b14ed78e6c42d/lib/sorcery/controller.rb#L72

What do you mean by:

In the logged_in function, we call logout, and because of this, we are unable to reach the before_logout hook.

Do you mean the logged_in? function?

@PedroAugustoRamalhoDuarte
Copy link
Collaborator Author

re: https://github.com/Sorcery/sorcery/blob/d9dc0bd80a3d5689398baea4489b14ed78e6c42d/lib/sorcery/controller.rb#L72

What do you mean by:

In the logged_in function, we call logout, and because of this, we are unable to reach the before_logout hook.

Do you mean the logged_in? function?

Yes, now we logout inside validate_session inside logged_in? só will return false then would not call before_logout.

@PedroAugustoRamalhoDuarte
Copy link
Collaborator Author

The others test are failling due to upgrade from rspec-rails: Sorcery#358 (comment)

I can fixes then later too

@PedroAugustoRamalhoDuarte
Copy link
Collaborator Author

Thanks for preparing this. I had trouble running the Sorcery specs as well.

When I run the specs of this PR with Ruby 3.2.2 I get 20 failures [1] in total. The most relevant ones for us are:

rspec ./spec/controllers/controller_session_timeout_spec.rb:132 # SorceryController with session timeout features with 'session_timeout_from_last_action' does not logout if there was activity
rspec ./spec/controllers/controller_session_timeout_spec.rb:194 # SorceryController with session timeout features with remember_me module enable after session timeout resets session and remember_me

Is 132 also failing for you or only 194?

194's error for me is:

Failure/Error: expect(cookies['remember_me_token']).to be_nil

       expected: nil
            got: "BAh..."

Which is weird because I also test resetting og the remember_me_token after invalidation in my app's specs and there it correctly gets reset to "".

[1]

479 examples, 20 failures, 2 pending

Failed examples:

rspec ./spec/controllers/controller_oauth2_spec.rb:31 # SorceryController using create_from creates a new user
rspec ./spec/controllers/controller_oauth2_spec.rb:39 # SorceryController using create_from supports nested attributes
rspec ./spec/controllers/controller_oauth2_spec.rb:47 # SorceryController using create_from does not crash on missing nested attributes
rspec ./spec/controllers/controller_oauth2_spec.rb:57 # SorceryController using create_from with a block does not create user
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:1:2]' # SorceryController OAuth with session timeout features when facebook resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:2:2]' # SorceryController OAuth with session timeout features when github resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:3:2]' # SorceryController OAuth with session timeout features when google resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:4:2]' # SorceryController OAuth with session timeout features when liveid resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:5:2]' # SorceryController OAuth with session timeout features when vk resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:6:2]' # SorceryController OAuth with session timeout features when salesforce resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:7:2]' # SorceryController OAuth with session timeout features when slack resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:8:2]' # SorceryController OAuth with session timeout features when discord resets session after session timeout
rspec './spec/controllers/controller_oauth2_spec.rb[1:5:9:2]' # SorceryController OAuth with session timeout features when battlenet resets session after session timeout
rspec ./spec/controllers/controller_oauth_spec.rb:136 # SorceryController  using 'create_from' creates a new user
rspec ./spec/controllers/controller_oauth_spec.rb:144 # SorceryController  using 'create_from' supports nested attributes
rspec ./spec/controllers/controller_oauth_spec.rb:152 # SorceryController  using 'create_from' does not crash on missing nested attributes
rspec ./spec/controllers/controller_oauth_spec.rb:172 # SorceryController  using 'create_from' with a block does not create user
rspec ./spec/controllers/controller_session_timeout_spec.rb:132 # SorceryController with session timeout features with 'session_timeout_from_last_action' does not logout if there was activity
rspec ./spec/controllers/controller_session_timeout_spec.rb:194 # SorceryController with session timeout features with remember_me module enable after session timeout resets session and remember_me
rspec ./spec/controllers/controller_spec.rb:67 # SorceryController when activated with sorcery #login when succeeds sets csrf token in session

Yes the 132 is failling too, i will investigate in depth in the weekend.

@jankoegel
Copy link
Owner

Yes, now we logout inside validate_session inside logged_in? só will return false then would not call before_logout.

I still don't follow what the problem is.

You're right that validate_session gets called pretty much at the start of the controller's before actions.
if the sessiong has been invalidated, with this PR, we now call logout.

Logout manually calls the before_ and after_logout! hooks: https://github.com/Sorcery/sorcery/blob/d9dc0bd80a3d5689398baea4489b14ed78e6c42d/lib/sorcery/controller.rb#L75


side note

  • perhaps you have a similar problem

  • we had to add a workaround in our code - basically a before_action that runs before validate_session since we're retroactively adding the session_timeout submodule to an existing app. All our users' current sessions have no login_time and no last_action_time.
    Do you understand why there's a fallback to Time.now here?
    https://github.com/Sorcery/sorcery/blob/d9dc0bd80a3d5689398baea4489b14ed78e6c42d/lib/sorcery/controller/submodules/session_timeout.rb#L71

    It basically means that all legacy sessions (created before the session_timeout feature was activated) cannot be invalidated.

    Our custom before_action works around this by setting the session's [:login_time] to Jan 1, 2023 - a date that's guaranteed to be earlier than our rollout date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants