diff --git a/lib/OC.php b/lib/OC.php index 725e945219d47..ab61904ecb99b 100644 --- a/lib/OC.php +++ b/lib/OC.php @@ -494,7 +494,7 @@ public static function initSession(): void { if (isset($_COOKIE[session_name()])) { setcookie(session_name(), '', -1, self::$WEBROOT ? : '/'); } - Server::get(IUserSession::class)->logout(); + Server::get(\OC\User\Session::class)->endInactiveSession(); } if (!self::hasSessionRelaxedExpiry()) { @@ -1298,10 +1298,13 @@ public static function handleLogin(OCP\IRequest $request): bool { if ($userSession->tryTokenLogin($request)) { return true; } - if (isset($_COOKIE['nc_username']) - && isset($_COOKIE['nc_token']) - && isset($_COOKIE['nc_session_id']) - && $userSession->loginWithCookie($_COOKIE['nc_username'], $_COOKIE['nc_token'], $_COOKIE['nc_session_id'])) { + // Read from $_COOKIE, where a logout or the Apache auth earlier in this request removes them. Clients can send + // the cookies as arrays. + $username = $_COOKIE['nc_username'] ?? null; + $token = $_COOKIE['nc_token'] ?? null; + $sessionId = $_COOKIE['nc_session_id'] ?? null; + if (is_string($username) && is_string($token) && is_string($sessionId) + && $userSession->loginWithCookie($username, $token, $sessionId)) { return true; } if ($userSession->tryBasicAuthLogin($request, Server::get(IThrottler::class))) { diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 49a7b0608d561..46b657fdd906b 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -905,7 +905,7 @@ public function tryTokenLogin(IRequest $request, bool $allowOcmAccessToken = fal * @param string $oldSessionId * @return bool */ - public function loginWithCookie($uid, $currentToken, $oldSessionId) { + public function loginWithCookie(string $uid, string $currentToken, string $oldSessionId) { $this->dispatcher->dispatchTyped(new BeforeUserLoggedInWithCookieEvent($uid)); $user = $this->manager->get($uid); if (is_null($user)) { @@ -913,10 +913,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { return false; } - // get stored tokens - $tokens = $this->config->getUserKeys($uid, 'login_token'); - // test cookies token against stored tokens - if (!in_array($currentToken, $tokens, true)) { + if (!$this->isStoredRememberMeToken($uid, $currentToken)) { $this->logger->info('Tried to log in but could not verify token', [ 'app' => 'core', 'user' => $uid, @@ -1035,6 +1032,66 @@ public function logout() { $this->dispatcher->dispatchTyped(new UserLoggedOutEvent($user)); } + /** + * End a session that was inactive for longer than session_lifetime. A request that proves a remembered + * login only loses its session data: its session token moves to the new session id. + */ + public function endInactiveSession(): void { + $cookies = $this->rememberedLoginCookies(Server::get(IRequest::class)); + if ($cookies === null) { + $this->logout(); + return; + } + + $this->logger->debug('Ending inactive session, keeping the remembered login', [ + 'app' => 'core', + 'user' => $cookies['username'], + ]); + try { + $oldSessionId = $this->session->getId(); + $this->setVolatileActiveUser(null); + $this->session->clear(); + $this->tokenProvider->renewSessionToken($oldSessionId, $this->session->getId()); + $this->renewMagicSessionId($oldSessionId); + } catch (InvalidTokenException|SessionNotAvailableException) { + // Without the session token the request continues as an anonymous one + } + } + + /** + * The cookies of a request that proves a remembered login for the session token of the current session + * + * @return ?array{username: string, token: string} + */ + private function rememberedLoginCookies(IRequest $request): ?array { + if ($this->tokenProvider === null) { + return null; + } + try { + $sessionId = $this->session->getId(); + } catch (SessionNotAvailableException) { + return null; + } + $cookies = $this->rememberMeCookiesFor($request, $sessionId); + if ($cookies === null) { + return null; + } + + try { + $token = $this->tokenProvider->getToken($sessionId); + } catch (InvalidTokenException) { + return null; + } + // No user backend is set up yet when a session times out, so the session token tells whose session it is + if ($token->getType() !== IToken::TEMPORARY_TOKEN + || $token->getRemember() !== IToken::REMEMBER + || $token->getUID() !== $cookies['username'] + || !$this->isStoredRememberMeToken($token->getUID(), $cookies['token'])) { + return null; + } + return $cookies; + } + /** * Set cookie value to use in next page load * @@ -1091,14 +1148,29 @@ public function setMagicInCookie($username, $token) { * login can still find the token that was renewed along with it. */ public function renewMagicSessionId(string $oldSessionId): void { - $request = Server::get(IRequest::class); + $cookies = $this->rememberMeCookiesFor(Server::get(IRequest::class), $oldSessionId); + if ($cookies === null) { + return; + } + $this->setMagicInCookie($cookies['username'], $cookies['token']); + } + + private function isStoredRememberMeToken(string $uid, string $token): bool { + return in_array($token, $this->config->getUserKeys($uid, 'login_token'), true); + } + + /** + * The remember-me cookies of a request, if they point at the session $sessionId + * + * @return ?array{username: string, token: string} + */ + private function rememberMeCookiesFor(IRequest $request, string $sessionId): ?array { $username = $request->getCookie('nc_username'); $token = $request->getCookie('nc_token'); - $sessionId = $request->getCookie('nc_session_id'); - if ($username === null || $token === null || $sessionId !== $oldSessionId) { - return; + if (!is_string($username) || !is_string($token) || $request->getCookie('nc_session_id') !== $sessionId) { + return null; } - $this->setMagicInCookie($username, $token); + return ['username' => $username, 'token' => $token]; } /** diff --git a/tests/lib/OCHandleLoginTest.php b/tests/lib/OCHandleLoginTest.php new file mode 100644 index 0000000000000..2016ac035ee23 --- /dev/null +++ b/tests/lib/OCHandleLoginTest.php @@ -0,0 +1,87 @@ + 'foo', 'nc_token' => 'fooToken', 'nc_session_id' => 'sess123']; + + private Session&MockObject $userSession; + private IRequest&MockObject $request; + private array $cookiesBefore; + + #[\Override] + protected function setUp(): void { + parent::setUp(); + + $this->cookiesBefore = $_COOKIE; + $this->userSession = $this->createMock(Session::class); + $this->overwriteService(Session::class, $this->userSession); + $this->overwriteService(IThrottler::class, $this->createMock(IThrottler::class)); + $this->request = $this->createMock(IRequest::class); + $this->request->method('getHeader')->willReturn(''); + // The cookies as the request came in + $this->request->method('getCookie')->willReturnCallback(fn (string $name) => self::COOKIES[$name] ?? null); + } + + #[\Override] + protected function tearDown(): void { + $_COOKIE = $this->cookiesBefore; + + parent::tearDown(); + } + + public function testLoginWithRememberMeCookies(): void { + $_COOKIE = self::COOKIES; + + $this->userSession->expects($this->once()) + ->method('loginWithCookie') + ->with('foo', 'fooToken', 'sess123') + ->willReturn(true); + + $this->assertTrue(\OC::handleLogin($this->request)); + } + + public static function cookieAsArrayData(): array { + return [ + 'username' => ['name' => 'nc_username'], + 'token' => ['name' => 'nc_token'], + 'session id' => ['name' => 'nc_session_id'], + ]; + } + + #[DataProvider(methodName: 'cookieAsArrayData')] + public function testRememberMeCookieAsArray(string $name): void { + $_COOKIE = [$name => [self::COOKIES[$name]]] + self::COOKIES; + + $this->userSession->expects($this->never()) + ->method('loginWithCookie'); + + $this->assertFalse(\OC::handleLogin($this->request)); + } + + public function testRememberMeCookiesRemovedEarlierInTheRequest(): void { + // A logout or the Apache auth removed them from $_COOKIE, but the request still carries them + $_COOKIE = []; + + $this->userSession->expects($this->never()) + ->method('loginWithCookie'); + + $this->assertFalse(\OC::handleLogin($this->request)); + } +} diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 3272036198c91..ed6597118d2f8 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -35,6 +35,7 @@ use OCP\Lockdown\ILockdownManager; use OCP\Security\Bruteforce\IThrottler; use OCP\Security\ISecureRandom; +use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\User\Events\BeforeUserLoggedInEvent; use OCP\User\Events\PostLoginEvent; use OCP\User\Events\UserLoggedInEvent; @@ -1271,6 +1272,7 @@ public static function renewMagicSessionIdData(): array { 'cookie holds the old session id' => [['nc_username' => 'u', 'nc_token' => 't', 'nc_session_id' => 'old'], true], 'cookie holds another session id' => [['nc_username' => 'u', 'nc_token' => 't', 'nc_session_id' => 'other'], false], 'no remember-me cookies' => [[], false], + 'remember-me token cookie is an array' => [['nc_username' => 'u', 'nc_token' => ['t'], 'nc_session_id' => 'old'], false], ]; } @@ -1287,6 +1289,213 @@ public function testRenewMagicSessionId(array $cookies, bool $expectRenewal): vo $this->userSession->renewMagicSessionId('old'); } + public static function endInactiveSessionData(): array { + $cookies = ['nc_username' => 'foo', 'nc_token' => 'fooToken', 'nc_session_id' => 'sess123']; + return [ + 'remembered login' => [ + 'cookies' => $cookies, + 'remember' => IToken::REMEMBER, + 'keepsSessionToken' => true, + ], + 'username in another case' => [ + 'cookies' => ['nc_username' => 'FOO'] + $cookies, + 'remember' => IToken::REMEMBER, + 'keepsSessionToken' => false, + ], + 'login without remember me' => [ + 'cookies' => $cookies, + 'remember' => IToken::DO_NOT_REMEMBER, + 'keepsSessionToken' => false, + ], + 'no remember-me cookies' => [ + 'cookies' => [], + 'remember' => IToken::REMEMBER, + 'keepsSessionToken' => false, + ], + 'cookies point at another session' => [ + 'cookies' => ['nc_session_id' => 'other'] + $cookies, + 'remember' => IToken::REMEMBER, + 'keepsSessionToken' => false, + ], + 'cookies of another user' => [ + 'cookies' => ['nc_username' => 'bar', 'nc_token' => 'barToken'] + $cookies, + 'remember' => IToken::REMEMBER, + 'keepsSessionToken' => false, + ], + 'remember-me token of another user' => [ + 'cookies' => ['nc_token' => 'barToken'] + $cookies, + 'remember' => IToken::REMEMBER, + 'keepsSessionToken' => false, + ], + 'remember-me token is not stored' => [ + 'cookies' => ['nc_token' => 'guessed'] + $cookies, + 'remember' => IToken::REMEMBER, + 'keepsSessionToken' => false, + ], + 'remember-me token cookie is an array' => [ + 'cookies' => ['nc_token' => ['fooToken']] + $cookies, + 'remember' => IToken::REMEMBER, + 'keepsSessionToken' => false, + ], + 'session token is an app password' => [ + 'cookies' => $cookies, + 'remember' => IToken::REMEMBER, + 'keepsSessionToken' => false, + 'type' => IToken::PERMANENT_TOKEN, + ], + 'session token missing' => [ + 'cookies' => $cookies, + 'remember' => null, + 'keepsSessionToken' => false, + ], + ]; + } + + #[DataProvider(methodName: 'endInactiveSessionData')] + public function testEndInactiveSession(array $cookies, ?int $remember, bool $keepsSessionToken, int $type = IToken::TEMPORARY_TOKEN): void { + $userSession = $this->createUserSessionForTimeout($this->tokenProvider, ['getUser']); + $userSession->method('getUser')->willReturn($this->createMock(IUser::class)); + $sessionId = 'sess123'; + $this->session->method('getId')->willReturnCallback(function () use (&$sessionId): string { + return $sessionId; + }); + $this->session->expects($this->once()) + ->method('clear') + ->willReturnCallback(function () use (&$sessionId): void { + $sessionId = 'sess456'; + }); + $this->mockRememberedLogin($cookies, $remember, $type); + + $this->tokenProvider->expects($keepsSessionToken ? $this->once() : $this->never()) + ->method('renewSessionToken') + ->with('sess123', 'sess456'); + $userSession->expects($keepsSessionToken ? $this->once() : $this->never()) + ->method('setMagicInCookie') + ->with($cookies['nc_username'] ?? null, 'fooToken'); + $this->tokenProvider->expects($keepsSessionToken ? $this->never() : $this->once()) + ->method('invalidateToken') + ->with('sess123'); + $userSession->expects($keepsSessionToken ? $this->never() : $this->once()) + ->method('unsetMagicInCookie'); + $this->dispatcher->expects($keepsSessionToken ? $this->never() : $this->atLeastOnce()) + ->method('dispatchTyped'); + + $userSession->endInactiveSession(); + } + + public static function endInactiveSessionWithoutTokenLookupData(): array { + return [ + 'no token provider' => ['hasTokenProvider' => false, 'hasSessionId' => true], + 'no session id' => ['hasTokenProvider' => true, 'hasSessionId' => false], + ]; + } + + #[DataProvider(methodName: 'endInactiveSessionWithoutTokenLookupData')] + public function testEndInactiveSessionWithoutTokenLookupLogsOut(bool $hasTokenProvider, bool $hasSessionId): void { + $userSession = $this->createUserSessionForTimeout($hasTokenProvider ? $this->tokenProvider : null, ['getUser']); + $userSession->method('getUser')->willReturn(null); + if ($hasSessionId) { + $this->session->method('getId')->willReturn('sess123'); + } else { + $this->session->method('getId')->willThrowException(new SessionNotAvailableException()); + } + $this->mockRememberedLogin(['nc_username' => 'foo', 'nc_token' => 'fooToken', 'nc_session_id' => 'sess123'], IToken::REMEMBER); + + $userSession->expects($this->once()) + ->method('unsetMagicInCookie'); + $this->session->expects($this->once()) + ->method('clear'); + + $userSession->endInactiveSession(); + } + + public function testEndInactiveSessionForgetsTheActiveUser(): void { + $user = $this->createMock(IUser::class); + $this->manager->method('get')->with('foo')->willReturn($user); + $sessionData = ['user_id' => 'foo']; + $this->session->method('get')->willReturnCallback(function (string $key) use (&$sessionData) { + return $sessionData[$key] ?? null; + }); + $this->session->method('clear')->willReturnCallback(function () use (&$sessionData): void { + $sessionData = []; + }); + $this->session->method('getId')->willReturn('sess123'); + $this->mockRememberedLogin(['nc_username' => 'foo', 'nc_token' => 'fooToken', 'nc_session_id' => 'sess123'], IToken::REMEMBER); + $userSession = $this->createUserSessionForTimeout($this->tokenProvider, ['validateSession']); + $this->assertSame($user, $userSession->getUser()); + + $userSession->endInactiveSession(); + + $this->assertNull($userSession->getUser()); + } + + public function testEndInactiveSessionWithAVanishedSessionToken(): void { + $userSession = $this->createUserSessionForTimeout($this->tokenProvider, ['getUser']); + $userSession->method('getUser')->willReturn($this->createMock(IUser::class)); + $this->session->method('getId')->willReturn('sess123'); + $this->mockRememberedLogin(['nc_username' => 'foo', 'nc_token' => 'fooToken', 'nc_session_id' => 'sess123'], IToken::REMEMBER); + $this->tokenProvider->method('renewSessionToken')->willThrowException(new InvalidTokenException()); + + $userSession->expects($this->never()) + ->method('setMagicInCookie'); + + $userSession->endInactiveSession(); + } + + public function testEndInactiveSessionWithoutANewSessionId(): void { + $userSession = $this->createUserSessionForTimeout($this->tokenProvider, ['getUser']); + $userSession->method('getUser')->willReturn($this->createMock(IUser::class)); + $lookups = 0; + $this->session->method('getId')->willReturnCallback(function () use (&$lookups): string { + // The cleared session has no id to move the token to + if (++$lookups > 2) { + throw new SessionNotAvailableException(); + } + return 'sess123'; + }); + $this->mockRememberedLogin(['nc_username' => 'foo', 'nc_token' => 'fooToken', 'nc_session_id' => 'sess123'], IToken::REMEMBER); + + $this->tokenProvider->expects($this->never()) + ->method('renewSessionToken'); + $userSession->expects($this->never()) + ->method('setMagicInCookie'); + + $userSession->endInactiveSession(); + } + + /** + * A user session whose cookie handling and $mockedMethods are mocked + */ + private function createUserSessionForTimeout(?IProvider $tokenProvider, array $mockedMethods): Session&MockObject { + return $this->getMockBuilder(Session::class) + ->setConstructorArgs([$this->manager, $this->session, $this->timeFactory, $tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->onlyMethods(['unsetMagicInCookie', 'setMagicInCookie', ...$mockedMethods]) + ->getMock(); + } + + /** + * Remember-me tokens fooToken for user foo and barToken for user bar, a session token sess123 of foo with the + * remember flag $remember (missing if null) and the type $type, and a request with $cookies. The user manager + * knows no user, as no user backend is set up yet when a session times out. + */ + private function mockRememberedLogin(array $cookies, ?int $remember, int $type = IToken::TEMPORARY_TOKEN): void { + $request = $this->createMock(IRequest::class); + $request->method('getCookie')->willReturnCallback(fn (string $key) => $cookies[$key] ?? null); + $this->overwriteService(IRequest::class, $request); + $storedTokens = ['foo' => ['fooToken'], 'bar' => ['barToken']]; + $this->config->method('getUserKeys') + ->willReturnCallback(fn (string $uid, string $app) => $app === 'login_token' ? ($storedTokens[$uid] ?? []) : []); + if ($remember === null) { + $this->tokenProvider->method('getToken')->willThrowException(new InvalidTokenException()); + return; + } + $token = new PublicKeyToken(); + $token->setUid('foo'); + $token->setRemember($remember); + $token->setType($type); + $this->tokenProvider->method('getToken')->with('sess123')->willReturn($token); + } + public function testTryBasicAuthLoginValid(): void { $request = $this->createMock(Request::class); $request->method('__get')