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.

Sign up for GitHub

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

Jump to bottom

Fix constant session regeneration #19113

Merged
merged 3 commits into from
Dec 29, 2021
Merged

Fix constant session regeneration #19113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
413ce02
Fix constant session regeneration
Dec 27, 2021
736d160
Add tests for session ID, always regenerating session ID for switchin…
Dec 28, 2021
b9a6fbe
Merge branch 'master' into 19067-session-id-always-regenerated
samdark Dec 29, 2021
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
11 changes: 4 additions & 7 deletions framework/web/User.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -655,9 +655,7 @@ public function switchIdentity($identity, $duration = 0)
}

$session = Yii::$app->getSession();
if (!YII_ENV_TEST) {
samdark marked this conversation as resolved.
Show resolved Hide resolved
$session->regenerateID(true);
}
$session->regenerateID(true);
$session->remove($this->idParam);
$session->remove($this->authTimeoutParam);
$session->remove($this->authKeyParam);
samdark marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -698,6 +696,9 @@ protected function renewAuthStatus()
/* @var $class IdentityInterface */
$class = $this->identityClass;
$identity = $class::findIdentity($id);
if ($identity === null) {
$this->switchIdentity(null);
}
}

if ($identity !== null) {
Expand Down Expand Up @@ -728,10 +729,6 @@ protected function renewAuthStatus()
$this->renewIdentityCookie();
}
}

if ($this->getIdentity(false) === null) {
$this->switchIdentity(null);
}
}

/**
Expand Down
53 changes: 48 additions & 5 deletions tests/framework/web/UserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ public function testLoginExpires()
'authManager' => [
'class' => PhpManager::className(),
'itemFile' => '@runtime/user_test_rbac_items.php',
'assignmentFile' => '@runtime/user_test_rbac_assignments.php',
'ruleFile' => '@runtime/user_test_rbac_rules.php',
'assignmentFile' => '@runtime/user_test_rbac_assignments.php',
'ruleFile' => '@runtime/user_test_rbac_rules.php',
],
],
];
Expand Down Expand Up @@ -169,23 +169,29 @@ public function testCookieCleanup()
];

$this->mockWebApplication($appConfig);
$id1 = Yii::$app->session->id;
Yii::$app->session->removeAll();

$cookie = new Cookie(Yii::$app->user->identityCookie);
$cookie->value = 'junk';
$cookiesMock->add($cookie);
Yii::$app->user->getIdentity();
$this->assertEquals(strlen($cookiesMock->getValue(Yii::$app->user->identityCookie['name'])), 0);
$this->assertSame($id1, Yii::$app->session->id);

Yii::$app->user->login(UserIdentity::findIdentity('user1'), 3600);
$this->assertFalse(Yii::$app->user->isGuest);
$this->assertSame(Yii::$app->user->id, 'user1');
$this->assertNotEquals(strlen($cookiesMock->getValue(Yii::$app->user->identityCookie['name'])), 0);
$id2 = Yii::$app->session->id;
$this->assertNotSame($id1, $id2);

Yii::$app->user->login(UserIdentity::findIdentity('user2'), 0);
$this->assertFalse(Yii::$app->user->isGuest);
$this->assertSame(Yii::$app->user->id, 'user2');
$this->assertEquals(strlen($cookiesMock->getValue(Yii::$app->user->identityCookie['name'])), 0);
$id3 = Yii::$app->session->id;
$this->assertNotSame($id2, $id3);
}

/**
Expand Down Expand Up @@ -225,7 +231,7 @@ public function testLoginRequired()
],
];
$this->mockWebApplication($appConfig);

$id = Yii::$app->session->id;

$user = Yii::$app->user;

Expand All @@ -234,6 +240,7 @@ public function testLoginRequired()
$user->loginRequired();
$this->assertEquals('normal', $user->getReturnUrl());
$this->assertTrue(Yii::$app->response->getIsRedirection());
$this->assertSame($id, Yii::$app->session->id);


$this->reset();
Expand All @@ -242,33 +249,37 @@ public function testLoginRequired()

$user->loginRequired();
$this->assertEquals(Yii::$app->getHomeUrl(), $user->getReturnUrl());
// AJAX requests don't update returnUrl but they do cause redirection.
// AJAX requests don't update returnUrl, but they do cause redirection.
$this->assertTrue(Yii::$app->response->getIsRedirection());

$user->loginRequired(false);
$this->assertEquals('ajax', $user->getReturnUrl());
$this->assertTrue(Yii::$app->response->getIsRedirection());
$this->assertSame($id, Yii::$app->session->id);

$this->reset();
Yii::$app->request->setUrl('json-only');
$_SERVER['HTTP_ACCEPT'] = 'Accept: text/json, q=0.1';
$user->loginRequired(true, false);
$this->assertEquals('json-only', $user->getReturnUrl());
$this->assertTrue(Yii::$app->response->getIsRedirection());
$this->assertSame($id, Yii::$app->session->id);

$this->reset();
Yii::$app->request->setUrl('json-only');
$_SERVER['HTTP_ACCEPT'] = 'text/json,q=0.1';
$user->loginRequired(true, false);
$this->assertEquals('json-only', $user->getReturnUrl());
$this->assertTrue(Yii::$app->response->getIsRedirection());
$this->assertSame($id, Yii::$app->session->id);

$this->reset();
Yii::$app->request->setUrl('accept-all');
$_SERVER['HTTP_ACCEPT'] = '*/*;q=0.1';
$user->loginRequired();
$this->assertEquals('accept-all', $user->getReturnUrl());
$this->assertTrue(Yii::$app->response->getIsRedirection());
$this->assertSame($id, Yii::$app->session->id);

$this->reset();
Yii::$app->request->setUrl('json-and-accept-all');
Expand All @@ -278,34 +289,39 @@ public function testLoginRequired()
} catch (ForbiddenHttpException $e) {
}
$this->assertFalse(Yii::$app->response->getIsRedirection());
$this->assertSame($id, Yii::$app->session->id);

$this->reset();
Yii::$app->request->setUrl('accept-html-json');
$_SERVER['HTTP_ACCEPT'] = 'text/json; q=1, text/html; q=0.1';
$user->loginRequired();
$this->assertEquals('accept-html-json', $user->getReturnUrl());
$this->assertTrue(Yii::$app->response->getIsRedirection());
$this->assertSame($id, Yii::$app->session->id);

$this->reset();
Yii::$app->request->setUrl('accept-html-json');
$_SERVER['HTTP_ACCEPT'] = 'text/json;q=1,application/xhtml+xml;q=0.1';
$user->loginRequired();
$this->assertEquals('accept-html-json', $user->getReturnUrl());
$this->assertTrue(Yii::$app->response->getIsRedirection());
$this->assertSame($id, Yii::$app->session->id);

$this->reset();
$_SERVER['REQUEST_METHOD'] = 'POST';
Yii::$app->request->setUrl('dont-set-return-url-on-post-request');
Yii::$app->getSession()->set($user->returnUrlParam, null);
$user->loginRequired();
$this->assertNull(Yii::$app->getSession()->get($user->returnUrlParam));
$this->assertSame($id, Yii::$app->session->id);

$this->reset();
$_SERVER['REQUEST_METHOD'] = 'GET';
Yii::$app->request->setUrl('set-return-url-on-get-request');
Yii::$app->getSession()->set($user->returnUrlParam, null);
$user->loginRequired();
$this->assertEquals('set-return-url-on-get-request', Yii::$app->getSession()->get($user->returnUrlParam));
$this->assertSame($id, Yii::$app->session->id);

// Confirm that returnUrl is not set.
$this->reset();
Expand All @@ -316,11 +332,13 @@ public function testLoginRequired()
} catch (ForbiddenHttpException $e) {
}
$this->assertNotEquals('json-only', $user->getReturnUrl());
$this->assertSame($id, Yii::$app->session->id);

$this->reset();
$_SERVER['HTTP_ACCEPT'] = 'text/json;q=0.1';
$this->expectException('yii\\web\\ForbiddenHttpException');
$user->loginRequired();
$this->assertSame($id, Yii::$app->session->id);
}

public function testLoginRequiredException1()
Expand Down Expand Up @@ -482,8 +500,10 @@ public function testSessionAuthWithMissingKey()
$this->mockWebApplication($appConfig);

Yii::$app->session->set('__id', 'user1');
$id = Yii::$app->session->id;

$this->assertNotNull(Yii::$app->user->getIdentity());
$this->assertSame($id, Yii::$app->session->id);
}

public function testSessionAuthWithInvalidKey()
Expand All @@ -497,12 +517,13 @@ public function testSessionAuthWithInvalidKey()
];

$this->mockWebApplication($appConfig);
$id = Yii::$app->session->id;

Yii::$app->session->set('__id', 'user1');
Yii::$app->session->set('__authKey', 'invalid');


$this->assertNull(Yii::$app->user->getIdentity());
$this->assertSame($id, Yii::$app->session->id);
}

public function testSessionAuthWithValidKey()
Expand All @@ -516,11 +537,33 @@ public function testSessionAuthWithValidKey()
];

$this->mockWebApplication($appConfig);
$id = Yii::$app->session->id;

Yii::$app->session->set('__id', 'user1');
Yii::$app->session->set('__authKey', 'ABCD1234');

$this->assertNotNull(Yii::$app->user->getIdentity());
$this->assertSame($id, Yii::$app->session->id);
}

public function testSessionAuthWhenIdentityReturnsNull()
{
$appConfig = [
'components' => [
'user' => [
'identityClass' => UserIdentity::className(),
],
],
];

$this->mockWebApplication($appConfig);
$id = Yii::$app->session->id;

Yii::$app->session->set('__id', 'user999999');
Yii::$app->session->set('__authKey', 'ABCD1234');

$this->assertNull(Yii::$app->user->getIdentity());
$this->assertNotSame($id, Yii::$app->session->id);
}
}

Expand Down