From 9bbd67334c5ee5853aabf5789293eef512f64edc Mon Sep 17 00:00:00 2001 From: mjansen Date: Fri, 15 May 2026 11:25:12 +0200 Subject: [PATCH] [IMPROVEMENT] Auth: Ensure proper session rotation / Impl. `SessionUpdateTimestampHandlerInterface` --- ...class.ilAuthDestroyExpiredSessionsCron.php | 2 +- .../classes/Frontend/class.ilAuthFrontend.php | 4 +- .../classes/class.ilAuthSession.php | 134 ++++++++++++--- .../classes/class.ilSession.php | 53 ++++-- .../classes/class.ilSessionDBHandler.php | 29 +++- .../classes/class.ilSessionStatisticsGUI.php | 2 +- .../src/Domain/SessionRotationPolicy.php | 33 ++++ .../src/Domain/SubmittedSessionId.php | 32 ++++ .../src/Infrastructure/CookieSessionId.php | 48 ++++++ .../Authentication/tests/ilSessionTest.php | 161 ++++++++++++++++-- .../class.ilCalendarRemoteAccessHandler.php | 2 +- .../Container/classes/class.ilContainer.php | 2 +- .../Content/class.ilDclContentExporter.php | 2 +- .../Init/classes/class.ilInitialisation.php | 44 ++--- .../ILIAS/Init/classes/class.ilStartUpGUI.php | 100 ++++++----- .../ILIAS/User/classes/class.ilObjUser.php | 2 +- .../classes/class.ilWebAccessChecker.php | 3 +- .../src/Auth/ILIASAuthenticationCallback.php | 4 +- .../ECS/classes/class.ilAuthProviderECS.php | 4 +- 19 files changed, 519 insertions(+), 142 deletions(-) create mode 100644 components/ILIAS/Authentication/src/Domain/SessionRotationPolicy.php create mode 100644 components/ILIAS/Authentication/src/Domain/SubmittedSessionId.php create mode 100644 components/ILIAS/Authentication/src/Infrastructure/CookieSessionId.php diff --git a/components/ILIAS/Authentication/classes/Cron/class.ilAuthDestroyExpiredSessionsCron.php b/components/ILIAS/Authentication/classes/Cron/class.ilAuthDestroyExpiredSessionsCron.php index 7f38fe762170..1e39509be4eb 100755 --- a/components/ILIAS/Authentication/classes/Cron/class.ilAuthDestroyExpiredSessionsCron.php +++ b/components/ILIAS/Authentication/classes/Cron/class.ilAuthDestroyExpiredSessionsCron.php @@ -86,7 +86,7 @@ public function run(): JobResult $result = new JobResult(); $result->setStatus(JobResult::STATUS_OK); - $num_destroyed_sessions = ilSession::_destroyExpiredSessions(); + $num_destroyed_sessions = ilSession::destroyExpiredSessions(); ilSessionStatistics::aggregateRaw(time()); $result->setMessage('Number of destroyed sessions: ' . $num_destroyed_sessions); diff --git a/components/ILIAS/Authentication/classes/Frontend/class.ilAuthFrontend.php b/components/ILIAS/Authentication/classes/Frontend/class.ilAuthFrontend.php index 472b0723d3ef..c0a64ff932c7 100755 --- a/components/ILIAS/Authentication/classes/Frontend/class.ilAuthFrontend.php +++ b/components/ILIAS/Authentication/classes/Frontend/class.ilAuthFrontend.php @@ -90,7 +90,7 @@ public function resetStatus(): void public function migrateAccount(ilAuthSession $session): bool { - if (!$session->isAuthenticated()) { + if (!$session->isFullyAuthenticated()) { $this->logger->warning('Desired user account is not authenticated'); return false; } @@ -285,7 +285,7 @@ protected function handleAuthenticationSuccess(ilAuthProviderInterface $provider $user->update(); $this->logger->info('Successfully authenticated: ' . ilObjUser::_lookupLogin($this->getStatus()->getAuthenticatedUserId())); - $this->getAuthSession()->setAuthenticated(true, $this->getStatus()->getAuthenticatedUserId()); + $this->getAuthSession()->onLoginSuccess($this->getStatus()->getAuthenticatedUserId()); ilInitialisation::initUserAccount(); diff --git a/components/ILIAS/Authentication/classes/class.ilAuthSession.php b/components/ILIAS/Authentication/classes/class.ilAuthSession.php index af281bc0a855..27704751a0be 100755 --- a/components/ILIAS/Authentication/classes/class.ilAuthSession.php +++ b/components/ILIAS/Authentication/classes/class.ilAuthSession.php @@ -18,6 +18,9 @@ declare(strict_types=1); +use ILIAS\Authentication\Domain\SessionRotationPolicy; +use ILIAS\Authentication\Domain\SubmittedSessionId; + class ilAuthSession { private const string SESSION_AUTH_AUTHENTICATED = '_authsession_authenticated'; @@ -38,12 +41,9 @@ private function __construct(ilLogger $logger) $this->logger = $logger; } - public static function getInstance(ilLogger $logger): ilAuthSession + public static function getInstance(ilLogger $logger): self { - if (self::$instance) { - return self::$instance; - } - return self::$instance = new self($logger); + return self::$instance ??= new self($logger); } protected function getLogger(): ilLogger @@ -54,7 +54,7 @@ protected function getLogger(): ilLogger /** * Start auth session */ - public function init(): bool + public function init(SubmittedSessionId $submitted_session_id): bool { if (session_status() === PHP_SESSION_ACTIVE) { $this->getLogger()->error(__METHOD__ . ' called with active session.'); @@ -65,9 +65,9 @@ public function init(): bool session_start(); $this->setId(session_id()); + $this->rejectForeignSessionIdIfUnknown($submitted_session_id); $user_id = (int) (ilSession::get(self::SESSION_AUTH_USER_ID) ?? ANONYMOUS_USER_ID); - if ($user_id) { $this->getLogger()->debug('Resuming old session for user: ' . $user_id); $this->setUserId($user_id); @@ -81,6 +81,7 @@ public function init(): bool $this->expired = false; $this->authenticated = false; } + return true; } @@ -92,28 +93,55 @@ public function isValid(): bool return !$this->isExpired() && $this->isAuthenticated(); } - /** - * Regenerate id - */ public function regenerateId(): void { - $old_session_id = session_id(); - session_regenerate_id(true); - $this->setId(session_id()); - $this->getLogger()->info('Session regenerate id: [' . substr($old_session_id, 0, 5) . '] -> [' . substr($this->getId(), 0, 5) . ']'); + $this->applySessionRotation(SessionRotationPolicy::Rotate); } /** - * Logout user => stop session + * End the current auth context and establish an anonymous session. + * Callers that must keep the session id use ensureAnonymousContext(). */ public function logout(): void { $this->getLogger()->debug('Logout called for: ' . $this->getUserId()); - session_regenerate_id(true); + + $this->clearSessionPayload(); + session_destroy(); + session_start(); + + $this->setAuthenticated(ANONYMOUS_USER_ID); + $this->applySessionRotation(SessionRotationPolicy::Rotate); + } + + /** + * Prepare the login screen: anonymous auth state in the current session (no destroy, no rotate). + */ + public function ensureAnonymousContext(): void + { + $this->getLogger()->debug( + 'Ensure anonymous context for: ' . $this->getUserId() + ); + $this->establishAnonymousAuthState(); + } - $this->init(); - $this->setAuthenticated(true, ANONYMOUS_USER_ID); + /** + * Establish an authenticated session after successful login. + */ + public function onLoginSuccess(int $user_id): void + { + $this->setAuthenticated($user_id); + $this->applySessionRotation(SessionRotationPolicy::Rotate); + } + + /** + * Transition an expired privileged session to anonymous. + */ + public function onSessionExpired(): void + { + $this->establishAnonymousAuthState(); + $this->applySessionRotation(SessionRotationPolicy::Rotate); } /** @@ -124,19 +152,13 @@ public function isAuthenticated(): bool return $this->authenticated || $this->user_id === ANONYMOUS_USER_ID; } - /** - * Set authenticated - */ - public function setAuthenticated(bool $a_status, int $a_user_id): void + private function setAuthenticated(int $a_user_id): void { - $this->authenticated = $a_status; + $this->authenticated = true; $this->user_id = $a_user_id; - ilSession::set(self::SESSION_AUTH_AUTHENTICATED, $a_status); + ilSession::set(self::SESSION_AUTH_AUTHENTICATED, true); ilSession::set(self::SESSION_AUTH_USER_ID, $a_user_id); $this->setExpired(false); - if ($a_status) { - $this->regenerateId(); - } } public function isFullyAuthenticated(): bool @@ -196,6 +218,7 @@ protected function validateExpiration(): bool $this->setExpired(true); return false; } + return true; } @@ -214,4 +237,61 @@ public function getId(): string { return $this->id; } + + /** + * Anonymous auth flags. A switch away from a logged-in account drops that account's session values. + * An already anonymous session keeps its payload, so the login page can preserve its id and messages. + */ + private function establishAnonymousAuthState(): void + { + if ($this->holdsLoggedInAccount()) { + $this->clearSessionPayload(); + } + + $this->setAuthenticated(ANONYMOUS_USER_ID); + } + + private function holdsLoggedInAccount(): bool + { + return $this->user_id > 0 && $this->user_id !== ANONYMOUS_USER_ID; + } + + private function clearSessionPayload(): void + { + ilSession::purge(); + } + + private function rejectForeignSessionIdIfUnknown(SubmittedSessionId $submitted_session_id): void + { + if (defined('IL_PHPUNIT_TEST')) { + return; + } + + $submitted_id = $submitted_session_id->get(); + if ($submitted_id === '' || $submitted_id !== session_id()) { + return; + } + + if (!ilSession::exists($submitted_id)) { + $this->applySessionRotation(SessionRotationPolicy::RejectForeign); + } + } + + private function applySessionRotation(SessionRotationPolicy $policy): void + { + $old_session_id = session_id(); + + session_regenerate_id( + $policy === SessionRotationPolicy::Rotate + ); + + $this->setId(session_id()); + + $label = $policy === SessionRotationPolicy::RejectForeign + ? 'Session reject foreign id' + : 'Session regenerate id'; + $this->getLogger()->info( + $label . ': [' . substr($old_session_id, 0, 5) . '] -> [' . substr($this->getId(), 0, 5) . ']' + ); + } } diff --git a/components/ILIAS/Authentication/classes/class.ilSession.php b/components/ILIAS/Authentication/classes/class.ilSession.php index c2355c03184c..e818609555ca 100755 --- a/components/ILIAS/Authentication/classes/class.ilSession.php +++ b/components/ILIAS/Authentication/classes/class.ilSession.php @@ -47,7 +47,7 @@ class ilSession * @param string session id * @return string session data */ - public static function _getData(string $a_session_id): string + public static function getData(string $a_session_id): string { if (!$a_session_id) { // fix for php #70520 @@ -89,7 +89,7 @@ public static function lookupExpireTime(string $a_session_id): int return 0; } - public static function _writeData(string $a_session_id, string $a_data): bool + public static function writeData(string $a_session_id, string $a_data): bool { global $DIC; @@ -123,7 +123,7 @@ public static function _writeData(string $a_session_id, string $a_data): bool $fields['remote_addr'] = [ilDBConstants::T_TEXT, $_SERVER['REMOTE_ADDR'] ?? '']; } - if (self::_exists($a_session_id)) { + if (self::exists($a_session_id)) { // note that we do this only when inserting the new record // updating may get us other contexts for the same session, especially ilContextWAC, which we do not want if (class_exists('ilContext') && ilContext::isSessionMainContext()) { @@ -194,7 +194,7 @@ public static function _writeData(string $a_session_id, string $a_data): bool $r = new \Random\Randomizer(); if ($r->getInt(0, 50) === 2) { // get time _before_ destroying expired sessions - self::_destroyExpiredSessions(); + self::destroyExpiredSessions(); ilSessionStatistics::aggregateRaw($now); } } @@ -202,7 +202,7 @@ public static function _writeData(string $a_session_id, string $a_data): bool return true; } - public static function _exists(string $a_session_id): bool + public static function exists(string $a_session_id): bool { if (!$a_session_id) { return false; @@ -224,7 +224,7 @@ public static function _exists(string $a_session_id): bool * @param int|null $a_closing_context closing context * @param int|bool $a_expired_at expired at timestamp */ - public static function _destroy($a_session_id, ?int $a_closing_context = null, $a_expired_at = null): bool + public static function destroy($a_session_id, ?int $a_closing_context = null, $a_expired_at = null): bool { global $DIC; @@ -283,7 +283,7 @@ public static function _destroy($a_session_id, ?int $a_closing_context = null, $ * * @param int user id */ - public static function _destroyByUserId(int $a_user_id): bool + public static function destroyByUserId(int $a_user_id): bool { global $DIC; @@ -300,7 +300,7 @@ public static function _destroyByUserId(int $a_user_id): bool * Destroy expired sessions * @return int The number of deleted sessions on success */ - public static function _destroyExpiredSessions(): int + public static function destroyExpiredSessions(): int { global $DIC; @@ -313,7 +313,7 @@ public static function _destroyExpiredSessions(): int $ids[$row['session_id']] = (int) $row['expires']; } if ($ids !== []) { - self::_destroy($ids, self::SESSION_CLOSE_EXPIRE, true); + self::destroy($ids, self::SESSION_CLOSE_EXPIRE, true); } return count($ids); @@ -325,7 +325,7 @@ public static function _destroyExpiredSessions(): int * @param string session id * @return string new session id */ - public static function _duplicate(string $a_session_id): string + public static function duplicate(string $a_session_id): string { global $DIC; @@ -345,7 +345,7 @@ public static function _duplicate(string $a_session_id): string $res = $ilDB->query($query); if ($row = $ilDB->fetchObject($res)) { - self::_writeData($new_session, $row->data); + self::writeData($new_session, $row->data); return $new_session; } //TODO check if throwing an excpetion might be a better choice @@ -360,6 +360,32 @@ public static function getExpireValue(): int return time() + self::getIdleValue(); } + /** + * Move the idle deadline of an existing session without rewriting its payload. + * Used when PHP calls updateTimestamp because the session data did not change. + */ + public static function updateTimestamp(string $session_id): bool + { + if ($session_id === '' || self::isWebAccessWithoutSessionEnabled()) { + return true; + } + + global $DIC; + + $affected_rows = $DIC['ilDB']->update( + 'usr_session', + [ + 'expires' => [ilDBConstants::T_INTEGER, self::getExpireValue()], + 'ctime' => [ilDBConstants::T_INTEGER, time()], + ], + [ + 'session_id' => [ilDBConstants::T_TEXT, $session_id], + ] + ); + + return $affected_rows > 0; + } + /** * Returns the idle time in seconds */ @@ -408,6 +434,11 @@ public static function clear(string $a_var): void } } + public static function purge(): void + { + $_SESSION = []; + } + /** * The names of all variables currently held in the session. * diff --git a/components/ILIAS/Authentication/classes/class.ilSessionDBHandler.php b/components/ILIAS/Authentication/classes/class.ilSessionDBHandler.php index 9ed00a589eae..90f8796d9e33 100755 --- a/components/ILIAS/Authentication/classes/class.ilSessionDBHandler.php +++ b/components/ILIAS/Authentication/classes/class.ilSessionDBHandler.php @@ -18,7 +18,7 @@ declare(strict_types=1); -class ilSessionDBHandler implements SessionHandlerInterface +class ilSessionDBHandler implements SessionHandlerInterface, SessionUpdateTimestampHandlerInterface { /** * Registers the session save handler @@ -65,7 +65,7 @@ public function close(): bool */ public function read($id): string { - return ilSession::_getData($id); + return ilSession::getData($id); } /** @@ -77,7 +77,7 @@ public function write($id, $data): bool { chdir(IL_INITIAL_WD); - return ilSession::_writeData($id, $data); + return ilSession::writeData($id, $data); } /** @@ -86,7 +86,7 @@ public function write($id, $data): bool */ public function destroy($id): bool { - return ilSession::_destroy($id); + return ilSession::destroy($id); } /** @@ -95,6 +95,25 @@ public function destroy($id): bool */ public function gc(int $max_lifetime): int | false { - return ilSession::_destroyExpiredSessions(); + return ilSession::destroyExpiredSessions(); + } + + public function validateId(string $id): bool + { + return ilSession::exists($id); + } + + /** + * PHP calls this when $_SESSION did not change. + * An expired session row is deleted before shutdown; a timestamp UPDATE then + * matches nothing and would drop the expired flag. Store the payload again. + */ + public function updateTimestamp(string $id, string $data): bool + { + if (ilSession::updateTimestamp($id)) { + return true; + } + + return $this->write($id, $data); } } diff --git a/components/ILIAS/Authentication/classes/class.ilSessionStatisticsGUI.php b/components/ILIAS/Authentication/classes/class.ilSessionStatisticsGUI.php index 890937ba001e..da852a93cf96 100755 --- a/components/ILIAS/Authentication/classes/class.ilSessionStatisticsGUI.php +++ b/components/ILIAS/Authentication/classes/class.ilSessionStatisticsGUI.php @@ -735,7 +735,7 @@ protected function adminSync(): void { // see ilSession::_writeData() $now = time(); - ilSession::_destroyExpiredSessions(); + ilSession::destroyExpiredSessions(); ilSessionStatistics::aggregateRaw($now); $this->tpl->setOnScreenMessage('success', $this->lng->txt('trac_sync_session_stats_success'), true); diff --git a/components/ILIAS/Authentication/src/Domain/SessionRotationPolicy.php b/components/ILIAS/Authentication/src/Domain/SessionRotationPolicy.php new file mode 100644 index 000000000000..710a0b58eb99 --- /dev/null +++ b/components/ILIAS/Authentication/src/Domain/SessionRotationPolicy.php @@ -0,0 +1,33 @@ +cookies->retrieve( + (string) session_name(), + $this->refinery->byTrying([ + $this->refinery->kindlyTo()->string(), + $this->refinery->always('') + ]) + ); + } +} diff --git a/components/ILIAS/Authentication/tests/ilSessionTest.php b/components/ILIAS/Authentication/tests/ilSessionTest.php index 344dccb65800..2e42c67b2925 100755 --- a/components/ILIAS/Authentication/tests/ilSessionTest.php +++ b/components/ILIAS/Authentication/tests/ilSessionTest.php @@ -238,28 +238,28 @@ function ($arg) { ); $result = ''; - ilSession::_writeData('123456', 'Testdata'); - if (ilSession::_exists('123456')) { + ilSession::writeData('123456', 'Testdata'); + if (ilSession::exists('123456')) { $result .= 'exists-'; } - if (ilSession::_getData('123456') === 'Testdata') { + if (ilSession::getData('123456') === 'Testdata') { $result .= 'write-get-'; } - $duplicate = ilSession::_duplicate('123456'); - if (ilSession::_getData($duplicate) === 'Testdata') { + $duplicate = ilSession::duplicate('123456'); + if (ilSession::getData($duplicate) === 'Testdata') { $result .= 'duplicate-'; } - ilSession::_destroy('123456'); - if (!ilSession::_exists('123456')) { + ilSession::destroy('123456'); + if (!ilSession::exists('123456')) { $result .= 'destroy-'; } - ilSession::_destroyExpiredSessions(); - if (ilSession::_exists($duplicate)) { + ilSession::destroyExpiredSessions(); + if (ilSession::exists($duplicate)) { $result .= 'destroyExp-'; } - ilSession::_destroyByUserId(17); - if (!ilSession::_exists($duplicate)) { + ilSession::destroyByUserId(17); + if (!ilSession::exists($duplicate)) { $result .= 'destroyByUser-'; } @@ -387,4 +387,143 @@ public function testPasswordAssisstanceSession(): void $sqlite = null; } + + public function testUpdateTimestampTouchesExpiryWithoutRewritingPayload(): void + { + $client_ini = $this->getMockBuilder(ilIniFile::class)->disableOriginalConstructor()->getMock(); + $client_ini + ->expects($this->once()) + ->method('readVariable') + ->with('session', 'expire') + ->willReturn('1800'); + $this->setGlobalVariable('ilClientIniFile', $client_ini); + + $ilDB = $GLOBALS['DIC']['ilDB']; + + $before = time(); + $ilDB + ->expects($this->once()) + ->method('update') + ->with( + 'usr_session', + $this->callback(static function (array $values) use ($before): bool { + if (array_key_exists('data', $values) || array_key_exists('user_id', $values)) { + return false; + } + + $expires = $values['expires'][1] ?? null; + $ctime = $values['ctime'][1] ?? null; + + return $values['expires'][0] === ilDBConstants::T_INTEGER + && $values['ctime'][0] === ilDBConstants::T_INTEGER + && is_int($expires) + && $expires >= $before + 1800 + && $expires <= time() + 1800 + && is_int($ctime) + && $ctime >= $before + && $ctime <= time(); + }), + ['session_id' => [ilDBConstants::T_TEXT, 'known-session-id']] + ) + ->willReturn(1); + + self::assertTrue(ilSession::updateTimestamp('known-session-id')); + } + + public function testUpdateTimestampSkipsEmptySessionId(): void + { + $ilDB = $GLOBALS['DIC']['ilDB']; + $ilDB->expects($this->never())->method('update'); + + self::assertTrue(ilSession::updateTimestamp('')); + } + + public function testUpdateTimestampSkipsWebAccessWithoutSession(): void + { + $ilDB = $GLOBALS['DIC']['ilDB']; + $ilDB->expects($this->never())->method('update'); + + ilSession::enableWebAccessWithoutSession(true); + try { + self::assertTrue(ilSession::updateTimestamp('wac-session-id')); + } finally { + ilSession::enableWebAccessWithoutSession(false); + } + } + + public function testHandlerKeepsExistingSessionOnTimestampUpdate(): void + { + $client_ini = $this->getMockBuilder(ilIniFile::class)->disableOriginalConstructor()->getMock(); + $client_ini + ->expects($this->once()) + ->method('readVariable') + ->with('session', 'expire') + ->willReturn('1800'); + $this->setGlobalVariable('ilClientIniFile', $client_ini); + + $ilDB = $GLOBALS['DIC']['ilDB']; + $ilDB->expects($this->once())->method('update')->willReturn(1); + $ilDB->expects($this->never())->method('manipulate'); + + $handler = new ilSessionDBHandler(); + + self::assertTrue($handler->updateTimestamp('known-session-id', 'unchanged-payload')); + } + + public function testHandlerStoresPayloadWhenTimestampUpdateMatchesNoRow(): void + { + ilSession::enableWebAccessWithoutSession(false); + + $client_ini = $this->createStub(ilIniFile::class); + $client_ini->method('readVariable')->willReturnCallback( + static function (string $group, string $key): string { + if ($group === 'session' && $key === 'expire') { + return '1800'; + } + + return ''; + } + ); + $this->setGlobalVariable('ilClientIniFile', $client_ini); + + $cron_manager = $this->createMock(\ILIAS\Cron\Job\JobManager::class); + $cron_manager + ->expects($this->once()) + ->method('isJobActive') + ->with('auth_destroy_expired_sessions') + ->willReturn(true); + $this->setGlobalVariable('cron.manager', $cron_manager); + + $ilDB = $GLOBALS['DIC']['ilDB']; + $ilDB->expects($this->once())->method('update')->willReturn(0); + $ilDB->method('quote')->willReturnCallback( + static fn(mixed $value, ?string $type = null): string => '[' . (string) $value . ']' + ); + $ilDB->expects($this->once())->method('query')->willReturn($this->createStub(ilDBStatement::class)); + $ilDB->expects($this->once())->method('numRows')->willReturn(0); + $ilDB + ->expects($this->once()) + ->method('manipulate') + ->with($this->callback(static function (string $sql): bool { + return str_contains($sql, 'INSERT INTO usr_session') + && str_contains($sql, '[expired-session-id]') + && str_contains($sql, '[expired-flag-payload]'); + })) + ->willReturn(1); + + $working_directory = getcwd(); + if (is_string($working_directory) && !defined('IL_INITIAL_WD')) { + define('IL_INITIAL_WD', $working_directory); + } + + try { + $handler = new ilSessionDBHandler(); + + self::assertTrue($handler->updateTimestamp('expired-session-id', 'expired-flag-payload')); + } finally { + if (is_string($working_directory)) { + chdir($working_directory); + } + } + } } diff --git a/components/ILIAS/Calendar/classes/class.ilCalendarRemoteAccessHandler.php b/components/ILIAS/Calendar/classes/class.ilCalendarRemoteAccessHandler.php index 572816ebab3e..f9459b8c9872 100755 --- a/components/ILIAS/Calendar/classes/class.ilCalendarRemoteAccessHandler.php +++ b/components/ILIAS/Calendar/classes/class.ilCalendarRemoteAccessHandler.php @@ -163,7 +163,7 @@ protected function initUser(): bool return false; } - $GLOBALS['DIC']['ilAuthSession']->setAuthenticated(true, $this->getTokenHandler()->getUserId()); + $GLOBALS['DIC']['ilAuthSession']->onLoginSuccess($this->getTokenHandler()->getUserId()); ilInitialisation::initUserAccount(); if (!$DIC->user() instanceof ilObjUser) { diff --git a/components/ILIAS/Container/classes/class.ilContainer.php b/components/ILIAS/Container/classes/class.ilContainer.php index 73bcff11179d..41a64bda3f31 100755 --- a/components/ILIAS/Container/classes/class.ilContainer.php +++ b/components/ILIAS/Container/classes/class.ilContainer.php @@ -548,7 +548,7 @@ public function cloneAllObject( #print_r($options); // Duplicate session to avoid logout problems with backgrounded SOAP calls - $new_session_id = ilSession::_duplicate($session_id); + $new_session_id = ilSession::duplicate($session_id); // Start cloning process using soap call $soap_client = new ilSoapClient(); $soap_client->setResponseTimeout($soap_client->getResponseTimeout()); diff --git a/components/ILIAS/DataCollection/classes/Content/class.ilDclContentExporter.php b/components/ILIAS/DataCollection/classes/Content/class.ilDclContentExporter.php index df7d46d0ec08..c1c748671064 100755 --- a/components/ILIAS/DataCollection/classes/Content/class.ilDclContentExporter.php +++ b/components/ILIAS/DataCollection/classes/Content/class.ilDclContentExporter.php @@ -163,7 +163,7 @@ public function exportAsync(string $format = self::EXPORT_EXCEL, ?string $filepa $soap_params = [$this->dcl->getRefId()]; array_push($soap_params, $this->table_id, $format, $filepath); - $new_session_id = ilSession::_duplicate( + $new_session_id = ilSession::duplicate( $DIC->http()->wrapper()->cookie()->retrieve(session_name(), $DIC->refinery()->kindlyTo()->string()) ); $client_id = $DIC->http()->wrapper()->cookie()->retrieve('ilClientId', $DIC->refinery()->kindlyTo()->string()); diff --git a/components/ILIAS/Init/classes/class.ilInitialisation.php b/components/ILIAS/Init/classes/class.ilInitialisation.php index 29c0320c932d..73fbb819f093 100755 --- a/components/ILIAS/Init/classes/class.ilInitialisation.php +++ b/components/ILIAS/Init/classes/class.ilInitialisation.php @@ -32,6 +32,7 @@ use ILIAS\Mail\Service\MailService; use ILIAS\Init\AllModernComponents; use ILIAS\Scripts\PHPStan\Attributes\AllowSuperglobalWrite; +use ILIAS\Authentication\Infrastructure\CookieSessionId; // needed for slow queries, etc. if (!isset($GLOBALS['ilGlobalStartTime']) || !$GLOBALS['ilGlobalStartTime']) { @@ -471,29 +472,14 @@ protected static function initGlobalCache(): void ); } - /** - * set session handler to db - * Used in Soap - */ public static function setSessionHandler(): void { $db_session_handler = new ilSessionDBHandler(); if (!$db_session_handler->setSaveHandler()) { - self::abortAndDie("Cannot start session handling."); - } - - // Do not accept external session ids - if (!ilSession::_exists(session_id()) && !defined('IL_PHPUNIT_TEST')) { - // php7-todo, correct-with-php5-removal : alex, 1.3.2016: added if, please check - if (function_exists("session_status") && session_status() == PHP_SESSION_ACTIVE) { - session_regenerate_id(); - } + self::abortAndDie('Cannot start session handling.'); } } - /** - * - */ protected static function setCookieConstants(): void { if (\ilAuthFactory::getContext() === \ilAuthFactory::CONTEXT_HTTP) { @@ -510,9 +496,9 @@ protected static function setCookieConstants(): void here it is set to '\'. in both cases a further '/' won't be appended due to the following regex */ - $cookie_path .= (!preg_match("/[\/|\\\\]$/", $cookie_path)) ? "/" : ""; + $cookie_path .= (!preg_match("/[\/|\\\\]$/", $cookie_path)) ? '/' : ''; - if ($cookie_path == "\\") { + if ($cookie_path === "\\") { $cookie_path = '/'; } @@ -770,18 +756,11 @@ public static function goToPublicSection(): void self::abortAndDie("Public Section enabled, but no Anonymous user found."); } - $session_destroyed = false; if ($DIC['ilAuthSession']->isExpired()) { - $session_destroyed = true; ilSession::setClosingContext(ilSession::SESSION_CLOSE_EXPIRE); - } - if (!$DIC['ilAuthSession']->isAuthenticated()) { - $session_destroyed = true; - ilSession::setClosingContext(ilSession::SESSION_CLOSE_PUBLIC); - } - - if ($session_destroyed) { - $GLOBALS['DIC']['ilAuthSession']->setAuthenticated(true, ANONYMOUS_USER_ID); + $DIC['ilAuthSession']->onSessionExpired(); + } elseif (!$DIC['ilAuthSession']->isAuthenticated()) { + $DIC['ilAuthSession']->ensureAnonymousContext(); } self::initUserAccount(); @@ -1036,7 +1015,12 @@ protected static function initSession(): void $auth_session = ilAuthSession::getInstance( $c['ilLoggerFactory']->getLogger('auth') ); - $auth_session->init(); + $auth_session->init( + new CookieSessionId( + $c->http()->wrapper()->cookie(), + $c->refinery() + ) + ); return $auth_session; }; } @@ -1226,7 +1210,7 @@ public static function resumeUserSession(): void $DIC['ilAuthSession']->isExpired() ) { if ($GLOBALS['DIC']['ilAuthSession']->isExpired()) { - ilSession::_destroy($_COOKIE[session_name()], ilSession::SESSION_CLOSE_EXPIRE); + ilSession::destroy($_COOKIE[session_name()], ilSession::SESSION_CLOSE_EXPIRE); } ilLoggerFactory::getLogger('init')->debug('Current session is invalid: ' . $GLOBALS['DIC']['ilAuthSession']->getId()); diff --git a/components/ILIAS/Init/classes/class.ilStartUpGUI.php b/components/ILIAS/Init/classes/class.ilStartUpGUI.php index 0ae745c60889..de6a1e9164f0 100755 --- a/components/ILIAS/Init/classes/class.ilStartUpGUI.php +++ b/components/ILIAS/Init/classes/class.ilStartUpGUI.php @@ -32,6 +32,7 @@ use ILIAS\LegalDocuments\Conductor; use ILIAS\components\Authentication\Pages\AuthPageEditorContext; use ILIAS\AuthApache\AuthFrontendCredentialsApache; +use ILIAS\UICore\GlobalTemplate; /** * @ilCtrl_Calls ilStartUpGUI: ilAccountRegistrationGUI, ilPasswordAssistanceGUI, ilLoginPageGUI, ilDashboardGUI @@ -153,7 +154,6 @@ public function getUnsafeGetCommands(): array public function getSafePostCommands(): array { return [ - 'doStandardAuthentication', ]; } @@ -217,8 +217,7 @@ private function showLoginPageOrStartupPage(): void * To address https://mantis.ilias.de/view.php?id=29991 we assume that we can ignore such requests * to prevent "Logout without CSRF / Denial of Service for Users" and redirect the user to the start page instead. */ - - if ($this->authSession->isValid() && $this->authSession->getUserId() > 0 && !$this->user->isAnonymous()) { + if ($this->authSession->isFullyAuthenticated()) { ilInitialisation::redirectToStartingPage(); } @@ -239,33 +238,14 @@ private function showLoginPageOrStartupPage(): void ) === 'force_login'; if ($is_forced_login) { - // Only allow this for anonymous user, see: showLoginPageOrStartupPage - $this->logger->debug('Force login'); - $messages = []; - if ($this->authSession->isValid()) { - $messages = $this->retrieveMessagesFromSession(); - $this->logger->debug('Valid session -> logout current user'); - ilSession::setClosingContext(ilSession::SESSION_CLOSE_USER); - $this->authSession->logout(); - - $this->eventHandler->raise( - 'components/ILIAS/Authentication', - 'afterLogout', - [ - 'username' => $this->user->getLogin(), - 'is_explicit_logout' => false, - ] - ); - - $this->dic->user()->setId($this->authSession->getUserId()); - $this->dic->user()->read(); - } - - $this->logger->debug('Show login page'); - foreach ($messages as $type => $content) { + $this->logger->debug('Forced login page presentation'); + foreach ($this->retrieveMessagesFromSession() as $type => $content) { $this->mainTemplate->setOnScreenMessage($type, $content); } + $this->prepareAnonymousContextForLoginPage(); + + $this->logger->debug('Show login page'); $this->showLoginPage(); return; } @@ -288,6 +268,14 @@ private function showLoginPage(?ILIAS\UI\Component\Input\Container\Form\Form $fo $this->getLogger()->debug('Showing login page'); + // Rotate expired sessions before form/rtoken generation (same ordering as force_login). + $session_expired_for_login_page = $this->authSession->isExpired(); + if ($session_expired_for_login_page) { + ilSession::setClosingContext(ilSession::SESSION_CLOSE_EXPIRE); + $this->authSession->onSessionExpired(); + $this->syncGlobalUserToAuthSession(); + } + $extUid = $this->http->wrapper()->query()->retrieve( 'ext_uid', $this->refinery->byTrying([$this->refinery->kindlyTo()->string(), $this->refinery->always('')]) @@ -326,7 +314,7 @@ private function showLoginPage(?ILIAS\UI\Component\Input\Container\Form\Form $fo $page_editor_html = $this->purgePlaceholders($page_editor_html); // check expired session and send message - if ($this->authSession->isExpired() || $this->http->wrapper()->query()->has('session_expired')) { + if ($session_expired_for_login_page || $this->http->wrapper()->query()->has('session_expired')) { $this->mainTemplate->setOnScreenMessage('failure', $this->lng->txt('auth_err_expired')); } elseif ($this->http->wrapper()->query()->has('reg_confirmation_msg')) { $this->lng->loadLanguageModule('registration'); @@ -335,7 +323,7 @@ private function showLoginPage(?ILIAS\UI\Component\Input\Container\Form\Form $fo $this->refinery->kindlyTo()->string() ); $message_type = $message_key === 'reg_account_confirmation_successful' ? - ilGlobalTemplateInterface::MESSAGE_TYPE_SUCCESS : ilGlobalTemplateInterface::MESSAGE_TYPE_FAILURE; + GlobalTemplate::MESSAGE_TYPE_SUCCESS : GlobalTemplate::MESSAGE_TYPE_FAILURE; $this->mainTemplate->setOnScreenMessage( $message_type, $this->lng->txt($message_key) @@ -346,13 +334,6 @@ private function showLoginPage(?ILIAS\UI\Component\Input\Container\Form\Form $fo $tpl->setVariable('LPE', $page_editor_html); } - if ($this->authSession->isExpired()) { - // The usr_id is is still the one of the former logged-in user, so we have to unset it - $this->authSession->setAuthenticated(false, ANONYMOUS_USER_ID); - $this->dic->user()->setId($this->authSession->getUserId()); - $this->dic->user()->read(); - } - $this->mainTemplate->setPermanentLink('auth', null, 'login'); self::printToGlobalTemplate($tpl); } @@ -375,10 +356,10 @@ private function retrieveMessagesFromSession(): array { $messages = []; $message_types = [ - ilGlobalTemplateInterface::MESSAGE_TYPE_FAILURE, - ilGlobalTemplateInterface::MESSAGE_TYPE_SUCCESS, - ilGlobalTemplateInterface::MESSAGE_TYPE_INFO, - ilGlobalTemplateInterface::MESSAGE_TYPE_QUESTION + GlobalTemplate::MESSAGE_TYPE_FAILURE, + GlobalTemplate::MESSAGE_TYPE_SUCCESS, + GlobalTemplate::MESSAGE_TYPE_INFO, + GlobalTemplate::MESSAGE_TYPE_QUESTION ]; foreach ($message_types as $message_type) { @@ -390,6 +371,41 @@ private function retrieveMessagesFromSession(): array return $messages; } + /** + * Align ilAuthSession and the global user with anonymous before showing the login page (force_login). + */ + private function prepareAnonymousContextForLoginPage(): void + { + if ($this->authSession->isExpired()) { + $former_login = $this->user->getLogin(); + $this->logger->debug('Expired privileged session -> anonymous with session rotation'); + ilSession::setClosingContext(ilSession::SESSION_CLOSE_EXPIRE); + $this->authSession->onSessionExpired(); + + $this->mainTemplate->setOnScreenMessage('failure', $this->lng->txt('auth_err_expired')); + + $this->eventHandler->raise( + 'components/ILIAS/Authentication', + 'afterLogout', + [ + 'username' => $former_login, + 'is_explicit_logout' => false, + ] + ); + } else { + $this->logger->debug('Ensure anonymous context (preserve session id)'); + $this->authSession->ensureAnonymousContext(); + } + + $this->syncGlobalUserToAuthSession(); + } + + private function syncGlobalUserToAuthSession(): void + { + $this->dic->user()->setId($this->authSession->getUserId()); + $this->dic->user()->read(); + } + private function showCodeForm( ?string $username = null, ?ILIAS\UI\Component\Input\Container\Form\Form $form = null @@ -1187,9 +1203,7 @@ private function doMigration(array $migration_request_data): void private function showLogout(): void { - if (!$this->authSession->isExpired() && - $this->authSession->isAuthenticated() && - !ilObjUser::_isAnonymous($this->authSession->getUserId())) { + if ($this->authSession->isFullyAuthenticated()) { $this->ctrl->redirectToURL(ilUserUtil::getStartingPointAsUrl()); } diff --git a/components/ILIAS/User/classes/class.ilObjUser.php b/components/ILIAS/User/classes/class.ilObjUser.php index 50b6a35a343e..e1b9b155ea6c 100755 --- a/components/ILIAS/User/classes/class.ilObjUser.php +++ b/components/ILIAS/User/classes/class.ilObjUser.php @@ -493,7 +493,7 @@ public function delete(): bool ['usr_id' => $this->getId()] ); - ilSession::_destroyByUserId($this->getId()); + ilSession::destroyByUserId($this->getId()); ilLDAPRoleGroupMapping::_getInstance()->deleteUser($this->getId()); /** diff --git a/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessChecker.php b/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessChecker.php index 868cf1495570..7ce5ecd1d7cc 100755 --- a/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessChecker.php +++ b/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessChecker.php @@ -330,8 +330,7 @@ protected function initAnonymousSession(): void */ $ilAuthSession = $DIC['ilAuthSession']; $ilAuthSession->regenerateId(); - $ilAuthSession->setUserId(ANONYMOUS_USER_ID); - $ilAuthSession->setAuthenticated(false, ANONYMOUS_USER_ID); + $ilAuthSession->ensureAnonymousContext(); $DIC->user()->setId(ANONYMOUS_USER_ID); } } diff --git a/components/ILIAS/WebDAV/src/Auth/ILIASAuthenticationCallback.php b/components/ILIAS/WebDAV/src/Auth/ILIASAuthenticationCallback.php index d6353952fa68..ccd77a777b46 100755 --- a/components/ILIAS/WebDAV/src/Auth/ILIASAuthenticationCallback.php +++ b/components/ILIAS/WebDAV/src/Auth/ILIASAuthenticationCallback.php @@ -89,9 +89,7 @@ public function authenticate(string $a_username, string $a_password): bool return true; } if ($this->isUserAgentSessionAware($this->getUserAgent())) { - if ($this->session->isAuthenticated() - && $this->user->getId() !== 0 - && $this->user->getId() !== ANONYMOUS_USER_ID) { + if ($this->session->isFullyAuthenticated()) { $this->logger->debug('User authenticated through session. UserID = ' . $this->user->getId()); return true; } diff --git a/components/ILIAS/WebServices/ECS/classes/class.ilAuthProviderECS.php b/components/ILIAS/WebServices/ECS/classes/class.ilAuthProviderECS.php index 77a3dec2dbc7..ed17f92cad8f 100755 --- a/components/ILIAS/WebServices/ECS/classes/class.ilAuthProviderECS.php +++ b/components/ILIAS/WebServices/ECS/classes/class.ilAuthProviderECS.php @@ -192,7 +192,7 @@ protected function resumeCurrentSession(): bool $session_user_id = $this->authSession->getUserId(); if (!$session_user_id || $session_user_id === ANONYMOUS_USER_ID) { $this->getLogger()->debug('No valid session found'); - $this->authSession->setAuthenticated(false, ANONYMOUS_USER_ID); + $this->authSession->ensureAnonymousContext(); return false; } $session_ext_account = ilObjUser::_lookupExternalAccount($session_user_id); @@ -201,7 +201,7 @@ protected function resumeCurrentSession(): bool $this->getLogger()->debug('Session external account: ' . $session_ext_account); if (!$session_ext_account || strcmp($user->getLogin(), $session_ext_account) !== 0) { $this->getLogger()->debug('No matching session found. Terminating current user session.'); - $this->authSession->setAuthenticated(false, ANONYMOUS_USER_ID); + $this->authSession->ensureAnonymousContext(); return false; } // assign to ECS global role