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

Avoid session fixation by regenerating session id on user promotion #414

Merged
merged 6 commits into from
Dec 6, 2018
Merged
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
6 changes: 3 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
- Adds Parse Server Health Check (#366)
- Adds the ability to upgrade to a revocable session (#368)
- Adds ability to Request Verification Emails (#369)
- Adds the ability to set/save in `ParseConfig` (#371)
- Adds the ability to set/save in `ParseConfig` (#371)
- Adds `ParseLogs` (#370)
- Adds `ParseAudience` (#372)
- Adds jobs to `ParseCloud` (#373)
Expand All @@ -40,7 +40,7 @@

- Updates to make the sdk friendly with `phpdoc`
- Added **Getting Started** section to README
- Removed the default server and mount path for `api.parse.com`
- Removed the default server and mount path for `api.parse.com`
- Setup `phpdoc` style enforcing and autodeploy from most recent `master` for our [api ref](http://parseplatform.org/parse-php-sdk/namespaces/Parse.html)
- **jms/serializer** pinned to **1.7.1** for testing as mentioned in #336 (for phpdoc)
- Added **ParsePolygon** type and `polygonContains` to **ParseQuery** (thanks to [Diamond Lewis](https://github.com/dplewis))
Expand Down Expand Up @@ -266,4 +266,4 @@
- Updated visibility of `ParseObject::_isDirty` to `protected` (thanks to [Fosco Marotto](https://github.com/gfosco))

### 1.0.0
- Initial release! (thanks to [Fosco Marotto](https://github.com/gfosco))
- Initial release! (thanks to [Fosco Marotto](https://github.com/gfosco))
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Please note that this documentation contains the latest changes that may as of y
- [Use Declarations](#use-declarations)
- [Parse Objects](#parse-objects)
- [Users](#users)
- [Session Id and Session Fixation](#session-id-and-session-fixation)
- [Verification Emails](#verification-emails)
- [ACLs/Security](#acls)
- [Queries](#queries)
Expand Down Expand Up @@ -288,6 +289,10 @@ try {
// Current user
$user = ParseUser::getCurrentUser();
```
#### Session Id and Session Fixation
Copy link
Contributor

Choose a reason for hiding this comment

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

Text looks good 👍 . Should probably add a link to this above 'Verification Emails' in the table of contents and we should be good.

Copy link
Member

Choose a reason for hiding this comment

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

@acinader Can we get this in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think I have just added what you're asking. :).

Copy link
Member

Choose a reason for hiding this comment

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

I think he was referring Table of Contents in the ReadME

Copy link
Contributor Author

Choose a reason for hiding this comment

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

riiight. thanks for spoon feeding me.

In an attempt to avoid [session fixation exploits](https://www.owasp.org/index.php/Session_fixation), the PHP SDK will call [`session_regenerate_id()`](http://php.net/manual/en/function.session-regenerate-id.php) when a session's permissions are elevated (since 1.5.0). In practice this means that `session_regenerate_id()` will be called when a session goes from no user to anonymous user or from no user / anonymous user to registered user.

Changing the PHP session id should have no impact on the contents of the session and state should be maintained for a user that was anonymous and becomes registered.

#### Verification Emails

Expand Down
4 changes: 4 additions & 0 deletions src/Parse/ParseUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,10 @@ protected function handleSaveResult($makeCurrent = false)
unset($this->serverData['sessionToken']);
}
if ($makeCurrent) {
if (session_id()) {
// see: https://www.owasp.org/index.php/Session_fixation
session_regenerate_id();
}
static::$currentUser = $this;
static::saveCurrentUser();
}
Expand Down
66 changes: 66 additions & 0 deletions tests/Parse/ParseSessionFixationTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php
namespace Parse\Test;

use Parse\ParseClient;
use Parse\ParseUser;
use Parse\ParseSession;

class ParseSessionFixationTest extends \PHPUnit_Framework_TestCase
{

public static function setUpBeforeClass()
{
Helper::clearClass(ParseUser::$parseClassName);
Helper::clearClass(ParseSession::$parseClassName);
ParseUser::logout();
ParseClient::_unsetStorage();

// indicate we should not use cookies
ini_set("session.use_cookies", 0);
// indicate we can use something other than cookies
ini_set("session.use_only_cookies", 0);
// enable transparent sid support, for url based sessions
ini_set("session.use_trans_sid", 1);
// clear cache control for session pages
ini_set("session.cache_limiter", "");
session_start();
Helper::setUp();
}

public function tearDown()
{
Helper::tearDown();
Helper::clearClass(ParseUser::$parseClassName);
Helper::clearClass(ParseSession::$parseClassName);
ParseUser::logout();
}

public static function tearDownAfterClass()
{
session_destroy();
}

public function testCookieIdChangedForAnonymous()
{
ParseClient::getStorage()->set('test', 'hi');
$noUserSessionId = session_id();
$user = ParseUser::loginWithAnonymous();
$anonymousSessionId = session_id();
$this->assertNotEquals($noUserSessionId, $anonymousSessionId);
$this->assertEquals(ParseClient::getStorage()->get('test'), 'hi');
}

public function testCookieIdChangedForAnonymousToRegistered()
{
$user = ParseUser::loginWithAnonymous();
$anonymousSessionId = session_id();
ParseClient::getStorage()->set('test', 'hi');
$user->setUsername('testy');
$user->setPassword('testy');
$user->save();
$user->login('testy', 'testy');
$registeredSessionId = session_id();
$this->assertNotEquals($anonymousSessionId, $registeredSessionId);
$this->assertEquals(ParseClient::getStorage()->get('test'), 'hi');
}
}