Skip to content
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

Session set issue after user login action #1932

Closed
afbora opened this issue Jul 19, 2019 · 6 comments

Comments

@afbora
Copy link

commented Jul 19, 2019

I created a login page for customers. Like sample:
https://getkirby.com/docs/cookbook/security/access-restriction

I just want to set session message like "You logged successfully!" after login action, like following and you can reproduce with that:

controllers/login.php

...    
$user->login(get('password'));
// When you remove the following line, login success
$kirby->session()->set('myplugin.messages', 'You logged successfully!');
go('/');

controllers/home.php

var_dump(kirby()->user()); // Returns NULL

Additional info
Another very interesting situation. Normally there are 1 file in the sessions folder as guest session. After login, there are two. But in the case I explained above, there are 3 files after redirected homepage.

Kirby Version
3.2.2

Desktop

  • Windows 10
  • Chrome 75.0.3770.142 64 Bit
@afbora

This comment has been minimized.

Copy link
Author

commented Jul 20, 2019

I have tested on several different scenarios.

Scenario 1. Failed: Same page access

If you try to access the same page, both data and user sessions do not come.

$kirby->session()->set('myplugin.before', 'Before message!');
$user = $kirby->user("john@doe.com");
$user->login("12345678"); // Or with > $user->loginPasswordless();
$kirby->session()->set('myplugin.after', 'After message!');

Output:

var_dump($kirby->session());

["data":protected] =>
object(Kirby\Session\SessionData)#291 (2) {
    ["session":protected]=>
    *RECURSION*
    ["data":protected]=>
    array(0) {
    }
}

var_dump($kirby->user());

NULL

Scenario 2. Failed: Access after redirect

If access is provided on the redirected page, only the session set after the login action myplugin.after is received, no user session is received.

$kirby->session()->set('myplugin.before', 'Before message!');
$user = $kirby->user("john@doe.com");
$user->login("12345678"); // Or with > $user->loginPasswordless();
$kirby->session()->set('myplugin.after', 'After message!');
go('/');

Output:

var_dump($kirby->session());

["data":protected]=>
    object(Kirby\Session\SessionData)#292 (2) {
    ["session":protected]=>
    *RECURSION*
    ["data":protected]=>
    array(1) {
      ["myplugin.after"]=>
      string(14) "After message!"
    }
}

var_dump($kirby->user());

NULL

Scenario 3. Success: Run codes with no session created

This scenario is a little different.
In the above scenarios the codes worked when there was an existing session.
In this scenario, the method was run directly after all sessions were deleted.

Same codes work with both senario as access on same page and redirected page.

Ouput:

var_dump($kirby->session());

["data":protected]=>
  object(Kirby\Session\SessionData)#292 (2) {
    ["session":protected]=>
    *RECURSION*
    ["data":protected]=>
    array(3) {
      ["myplugin.before"]=>
      string(15) "Before message!"
      ["user.id"]=>
      string(8) "lHXp9r3E"
      ["myplugin.after"]=>
      string(14) "After message!"
    }
}

var_dump($kirby->user());

object(Kirby\Cms\User)#298 (7) {
  ["content"]=>
  object(Kirby\Cms\Content)#445 (14) {
    ...
  },
  ["email"]=>
  string(16) "john@doe.com"
  ["id"]=>
  string(8) "lHXp9r3E"
  ["language"]=>
  string(2) "tr"
  ...
}
@afbora

This comment has been minimized.

Copy link
Author

commented Jul 20, 2019

Also I'm going to ask you a question about something I'm curious about:

Why new session created after the user login and logout for both methods on frontend or panel?
So why the current session not saved on login?

Session files (/site/sessions)

One session file before login as guest

> xxx.file.sess
6b8823ab56b1edd5a905f6f54e07e5f2a48209ce82fdce7de8ff715f4a52d353
a:7:{s:9:"startTime";i:1563614873;s:10:"expiryTime";i:1563622073;s:8:"duration";i:7200;s:7:"timeout";i:1800;s:12:"lastActivity";i:1563614873;s:9:"renewable";b:1;s:4:"data";a:1:{s:4:"csrf";s:64:"4ed63adb302c78f54777c8afab8c679e3467a3c62e020f270469a6ca89fb1fd6";}}

Two session files after login:

> xxx.file.sess
943c65f6d24eb6c8a2323afbce2cd00ad3c6664ec5f557a0c5f4e7674bb853e2
a:3:{s:9:"startTime";i:1563617343;s:10:"expiryTime";i:1563617386;s:10:"newSession";s:31:"1563624543.b6a884b577b703fb18d8";}


> yyy.file.sess
22292d4dacb640461270c80c680550b27c88662605c62c1f0a3076abec995a4c
a:7:{s:9:"startTime";i:1563617343;s:10:"expiryTime";i:1563624543;s:8:"duration";i:7200;s:7:"timeout";i:1800;s:12:"lastActivity";i:1563617343;s:9:"renewable";b:1;s:4:"data";a:2:{s:4:"csrf";s:64:"cb8cb48bb50f537aa27c460a5f6aa651eb0f5892892c0a399eecbbccc9e29bdf";s:7:"user.id";s:8:"0H1gJMWt";}}

Three session files after logout:

> xxx.file.sess
943c65f6d24eb6c8a2323afbce2cd00ad3c6664ec5f557a0c5f4e7674bb853e2
a:3:{s:9:"startTime";i:1563617343;s:10:"expiryTime";i:1563617386;s:10:"newSession";s:31:"1563624543.b6a884b577b703fb18d8";}


> yyy.file.sess
b6005cfe0810fb5c5de7b3442e6b034189a1c97f72e4b43af86500e10495da3c
a:3:{s:9:"startTime";i:1563617343;s:10:"expiryTime";i:1563617423;s:10:"newSession";s:31:"1563624543.846da6a80d6fe1e06e20";}

> zzz.file.sess
cb72202474c5d792db451a04283550363a8ab942273bc47a418ee1fa48614559
a:7:{s:9:"startTime";i:1563617343;s:10:"expiryTime";i:1563624543;s:8:"duration";i:7200;s:7:"timeout";i:1800;s:12:"lastActivity";i:1563617343;s:9:"renewable";b:1;s:4:"data";a:1:{s:4:"csrf";s:64:"cb8cb48bb50f537aa27c460a5f6aa651eb0f5892892c0a399eecbbccc9e29bdf";}}

Four session files after re-login.. as it goes stretching 🤔

@lukasbestle lukasbestle modified the milestones: 3.2.4, 3.2.3 Jul 20, 2019

lukasbestle added a commit that referenced this issue Jul 20, 2019

@lukasbestle lukasbestle referenced this issue Jul 20, 2019
4 of 4 tasks complete
@lukasbestle

This comment has been minimized.

Copy link
Contributor

commented Jul 20, 2019

Why new session created after the user login and logout for both methods on frontend or panel?
So why the current session not saved on login?

That's a security measure to make so-called session fixation attacks harder to pull off. Each login and logout is a privilege level change, which require a clear separation of the sessions used. You can read more about this in the OWASP recommendations for session management.

Don't worry about the session files though: They are automatically garbage-collected once they have expired. The reason why they are still kept is to ensure that requests that are sent around the session token regeneration don't fail. The old session tokens stop working 30 seconds after the token regeneration, but are kept until their regular expiry time (two hours by default) has passed. After that, they are deleted by Kirby on the next run of the garbage collector.


About the issue: Thanks for the detailed information! ❤️

This bug occurred because you retrieved the session instance in the same request where the session token was regenerated. Because the cookie was already updated by the session token regeneration, our code that retrieves the session instance then already tried to find the session instance with the new token, which failed in the same request. I have fixed this issue with #1934. Please test if it works for you. :)

@afbora

This comment has been minimized.

Copy link
Author

commented Jul 22, 2019

You're welcome. I try to give all the details and implement the tests and scenarios that come to mind in order to explain the issue more clearly and to solve the issue more quickly.

As for the solution, it works great in all scenarios. Great! Thank you 👍

@lukasbestle

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

Thanks for testing. Glad we got it fixed. :)

bastianallgeier added a commit that referenced this issue Jul 29, 2019

bastianallgeier added a commit that referenced this issue Jul 29, 2019

@bastianallgeier

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.