Skip to content

Commit d352b2c

Browse files
solracsfsolracsf
committed
refactor(session): Code simplification
Signed-off-by: Git'Fellow <[email protected]>
1 parent f61ef6d commit d352b2c

File tree

2 files changed

+209
-186
lines changed

2 files changed

+209
-186
lines changed

‎lib/private/User/Session.php‎

Lines changed: 74 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,7 @@
6767
class Session implements IUserSession, Emitter {
6868
use TTransactional;
6969

70-
/** @var User $activeUser */
71-
protected $activeUser;
70+
protected User $activeUser;
7271

7372
public function __construct(
7473
private Manager $manager,
@@ -83,17 +82,13 @@ public function __construct(
8382
) {
8483
}
8584

86-
/**
87-
* @param IProvider $provider
88-
*/
8985
public function setTokenProvider(IProvider $provider) {
9086
$this->tokenProvider = $provider;
9187
}
9288

9389
/**
9490
* @param string $scope
9591
* @param string $method
96-
* @param callable $callback
9792
*/
9893
public function listen($scope, $method, callable $callback) {
9994
$this->manager->listen($scope, $method, $callback);
@@ -102,7 +97,6 @@ public function listen($scope, $method, callable $callback) {
10297
/**
10398
* @param string $scope optional
10499
* @param string $method optional
105-
* @param callable $callback optional
106100
*/
107101
public function removeListener($scope = null, $method = null, ?callable $callback = null) {
108102
$this->manager->removeListener($scope, $method, $callback);
@@ -145,7 +139,7 @@ public function setSession(ISession $session) {
145139
* @param IUser|null $user
146140
*/
147141
public function setUser($user) {
148-
if (is_null($user)) {
142+
if ($user === null) {
149143
$this->session->remove('user_id');
150144
} else {
151145
$this->session->set('user_id', $user->getUID());
@@ -173,17 +167,22 @@ public function getUser() {
173167
if (OC_User::isIncognitoMode()) {
174168
return null;
175169
}
176-
if (is_null($this->activeUser)) {
177-
$uid = $this->session->get('user_id');
178-
if (is_null($uid)) {
179-
return null;
180-
}
181-
$this->activeUser = $this->manager->get($uid);
182-
if (is_null($this->activeUser)) {
183-
return null;
184-
}
185-
$this->validateSession();
170+
171+
if ($this->activeUser !== null) {
172+
return $this->activeUser;
173+
}
174+
175+
$uid = $this->session->get('user_id');
176+
if ($uid === null) {
177+
return null;
178+
}
179+
180+
$this->activeUser = $this->manager->get($uid);
181+
if ($this->activeUser === null) {
182+
return null;
186183
}
184+
185+
$this->validateSession();
187186
return $this->activeUser;
188187
}
189188

@@ -194,17 +193,12 @@ public function getUser() {
194193
* - For browsers, the session token validity is checked
195194
*/
196195
protected function validateSession() {
197-
$token = null;
198196
$appPassword = $this->session->get('app_password');
199197

200-
if (is_null($appPassword)) {
201-
try {
202-
$token = $this->session->getId();
203-
} catch (SessionNotAvailableException $ex) {
204-
return;
205-
}
206-
} else {
207-
$token = $appPassword;
198+
try {
199+
$token = $appPassword ?? $this->session->getId();
200+
} catch (SessionNotAvailableException $ex) {
201+
return;
208202
}
209203

210204
if (!$this->validateToken($token)) {
@@ -220,7 +214,7 @@ protected function validateSession() {
220214
*/
221215
public function isLoggedIn() {
222216
$user = $this->getUser();
223-
if (is_null($user)) {
217+
if ($user === null) {
224218
return false;
225219
}
226220

@@ -233,7 +227,7 @@ public function isLoggedIn() {
233227
* @param string|null $loginName for the logged in user
234228
*/
235229
public function setLoginName($loginName) {
236-
if (is_null($loginName)) {
230+
if ($loginName === null) {
237231
$this->session->remove('loginname');
238232
} else {
239233
$this->session->set('loginname', $loginName);
@@ -246,12 +240,12 @@ public function setLoginName($loginName) {
246240
* @return ?string
247241
*/
248242
public function getLoginName() {
249-
if ($this->activeUser) {
243+
if ($this->activeUser !== null) {
250244
return $this->session->get('loginname');
251245
}
252246

253247
$uid = $this->session->get('user_id');
254-
if ($uid) {
248+
if ($uid !== null) {
255249
$this->activeUser = $this->manager->get($uid);
256250
return $this->session->get('loginname');
257251
}
@@ -273,7 +267,6 @@ public function setImpersonatingUserID(bool $useCurrentUser = true): void {
273267
}
274268

275269
$currentUser = $this->getUser();
276-
277270
if ($currentUser === null) {
278271
throw new \OC\User\NoUserException();
279272
}
@@ -348,19 +341,21 @@ public function completeLogin(IUser $user, array $loginDetails, $regenerateSessi
348341
$loginDetails['password'],
349342
$isToken
350343
));
344+
351345
$this->manager->emit('\OC\User', 'postLogin', [
352346
$user,
353347
$loginDetails['loginName'],
354348
$loginDetails['password'],
355349
$isToken,
356350
]);
357-
if ($this->isLoggedIn()) {
358-
$this->prepareUserLogin($firstTimeLogin, $regenerateSessionId);
359-
return true;
360-
}
361351

362-
$message = \OCP\Util::getL10N('lib')->t('Login canceled by app');
363-
throw new LoginException($message);
352+
if (!$this->isLoggedIn()) {
353+
$message = \OCP\Util::getL10N('lib')->t('Login canceled by app');
354+
throw new LoginException($message);
355+
}
356+
357+
$this->prepareUserLogin($firstTimeLogin, $regenerateSessionId);
358+
return true;
364359
}
365360

366361
/**
@@ -458,7 +453,7 @@ private function handleLoginFailed(IThrottler $throttler, int $currentDelay, str
458453
}
459454

460455
protected function supportsCookies(IRequest $request) {
461-
if (!is_null($request->getCookie('cookie_test'))) {
456+
if ($request->getCookie('cookie_test') !== null) {
462457
return true;
463458
}
464459
setcookie('cookie_test', 'test', $this->timeFactory->getTime() + 3600);
@@ -476,7 +471,7 @@ protected function isTwoFactorEnforced($username) {
476471
['uid' => &$username]
477472
);
478473
$user = $this->manager->get($username);
479-
if (is_null($user)) {
474+
if ($user === null) {
480475
$users = $this->manager->getByEmail($username);
481476
if (empty($users)) {
482477
return false;
@@ -619,7 +614,7 @@ private function loginWithToken($token) {
619614
$this->manager->emit('\OC\User', 'preLogin', [$dbToken->getLoginName(), $password]);
620615

621616
$user = $this->manager->get($uid);
622-
if (is_null($user)) {
617+
if ($user === null) {
623618
// user does not exist
624619
return false;
625620
}
@@ -645,7 +640,7 @@ private function loginWithToken($token) {
645640
* @return boolean
646641
*/
647642
public function createSessionToken(IRequest $request, $uid, $loginName, $password = null, $remember = IToken::DO_NOT_REMEMBER) {
648-
if (is_null($this->manager->get($uid))) {
643+
if ($this->manager->get($uid) === null) {
649644
// User does not exist
650645
return false;
651646
}
@@ -675,7 +670,7 @@ public function createSessionToken(IRequest $request, $uid, $loginName, $passwor
675670
* @return string|null the password or null if none was set in the token
676671
*/
677672
private function getPassword($password) {
678-
if (is_null($password)) {
673+
if ($password === null) {
679674
// This is surely no token ;-)
680675
return null;
681676
}
@@ -714,7 +709,7 @@ private function checkTokenCredentials(IToken $dbToken, $token) {
714709
} catch (PasswordlessTokenException $ex) {
715710
// Token has no password
716711

717-
if (!is_null($this->activeUser) && !$this->activeUser->isEnabled()) {
712+
if ($this->activeUser !== null && !$this->activeUser->isEnabled()) {
718713
$this->tokenProvider->invalidateToken($token);
719714
return false;
720715
}
@@ -723,7 +718,7 @@ private function checkTokenCredentials(IToken $dbToken, $token) {
723718
}
724719

725720
// Invalidate token if the user is no longer active
726-
if (!is_null($this->activeUser) && !$this->activeUser->isEnabled()) {
721+
if ($this->activeUser !== null && !$this->activeUser->isEnabled()) {
727722
$this->tokenProvider->invalidateToken($token);
728723
return false;
729724
}
@@ -764,7 +759,7 @@ private function validateToken($token, $user = null) {
764759
return false;
765760
}
766761

767-
if (!is_null($user) && !$this->validateTokenLoginName($user, $dbToken)) {
762+
if ($user !== null && !$this->validateTokenLoginName($user, $dbToken)) {
768763
return false;
769764
}
770765

@@ -869,7 +864,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) {
869864
$this->session->regenerateId();
870865
$this->manager->emit('\OC\User', 'preRememberedLogin', [$uid]);
871866
$user = $this->manager->get($uid);
872-
if (is_null($user)) {
867+
if ($user === null) {
873868
// user does not exist
874869
return false;
875870
}
@@ -938,9 +933,10 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) {
938933
* @param IUser $user
939934
*/
940935
public function createRememberMeToken(IUser $user) {
936+
$uid = $user->getUID():
941937
$token = $this->random->generate(32);
942-
$this->config->setUserValue($user->getUID(), 'login_token', $token, (string)$this->timeFactory->getTime());
943-
$this->setMagicInCookie($user->getUID(), $token);
938+
$this->config->setUserValue($uid, 'login_token', $token, (string)$this->timeFactory->getTime());
939+
$this->setMagicInCookie($uid, $token);
944940
}
945941

946942
/**
@@ -949,6 +945,7 @@ public function createRememberMeToken(IUser $user) {
949945
public function logout() {
950946
$user = $this->getUser();
951947
$this->manager->emit('\OC\User', 'logout', [$user]);
948+
952949
if ($user !== null) {
953950
try {
954951
$token = $this->session->getId();
@@ -957,16 +954,18 @@ public function logout() {
957954
'user' => $user->getUID(),
958955
]);
959956
} catch (SessionNotAvailableException $ex) {
957+
// Session not available, continue logout
960958
}
961959
}
962960
$this->logger->debug('Logging out', [
963-
'user' => $user === null ? null : $user->getUID(),
961+
'user' => $user?->getUID(),
964962
]);
965963
$this->setUser(null);
966964
$this->setLoginName(null);
967965
$this->setToken(null);
968966
$this->unsetMagicInCookie();
969967
$this->session->clear();
968+
970969
$this->manager->emit('\OC\User', 'postLogout', [$user]);
971970
}
972971

@@ -985,6 +984,7 @@ public function setMagicInCookie($username, $token) {
985984
$domain = $this->config->getSystemValueString('cookie_domain');
986985

987986
$maxAge = $this->config->getSystemValueInt('remember_login_cookie_lifetime', 60 * 60 * 24 * 15);
987+
988988
\OC\Http\CookieHelper::setCookie(
989989
'nc_username',
990990
$username,
@@ -995,6 +995,7 @@ public function setMagicInCookie($username, $token) {
995995
true,
996996
\OC\Http\CookieHelper::SAMESITE_LAX
997997
);
998+
998999
\OC\Http\CookieHelper::setCookie(
9991000
'nc_token',
10001001
$token,
@@ -1005,6 +1006,7 @@ public function setMagicInCookie($username, $token) {
10051006
true,
10061007
\OC\Http\CookieHelper::SAMESITE_LAX
10071008
);
1009+
10081010
try {
10091011
\OC\Http\CookieHelper::setCookie(
10101012
'nc_session_id',
@@ -1028,18 +1030,31 @@ public function unsetMagicInCookie() {
10281030
//TODO: DI for cookies and IRequest
10291031
$secureCookie = OC::$server->getRequest()->getServerProtocol() === 'https';
10301032
$domain = $this->config->getSystemValueString('cookie_domain');
1033+
$expireTime = $this->timeFactory->getTime() - 3600;
1034+
1035+
//TODO: DI
1036+
unset($_COOKIE['nc_username'], $_COOKIE['nc_token'], $_COOKIE['nc_session_id']);
1037+
1038+
$cookieOptions = [
1039+
'expires' => $expireTime,
1040+
'path' => OC::$WEBROOT,
1041+
'domain' => $domain,
1042+
'secure' => $secureCookie,
1043+
'httponly' => true,
1044+
'samesite' => 'Lax',
1045+
];
1046+
1047+
// Clear cookies with current path
1048+
setcookie('nc_username', '', $cookieOptions);
1049+
setcookie('nc_token', '', $cookieOptions);
1050+
setcookie('nc_session_id', '', $cookieOptions);
10311051

1032-
unset($_COOKIE['nc_username']); //TODO: DI
1033-
unset($_COOKIE['nc_token']);
1034-
unset($_COOKIE['nc_session_id']);
1035-
setcookie('nc_username', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT, $domain, $secureCookie, true);
1036-
setcookie('nc_token', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT, $domain, $secureCookie, true);
1037-
setcookie('nc_session_id', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT, $domain, $secureCookie, true);
10381052
// old cookies might be stored under /webroot/ instead of /webroot
10391053
// and Firefox doesn't like it!
1040-
setcookie('nc_username', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT . '/', $domain, $secureCookie, true);
1041-
setcookie('nc_token', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT . '/', $domain, $secureCookie, true);
1042-
setcookie('nc_session_id', '', $this->timeFactory->getTime() - 3600, OC::$WEBROOT . '/', $domain, $secureCookie, true);
1054+
$cookieOptions['path'] = OC::$WEBROOT . '/';
1055+
setcookie('nc_username', '', $cookieOptions);
1056+
setcookie('nc_token', '', $cookieOptions);
1057+
setcookie('nc_session_id', '', $cookieOptions);
10431058
}
10441059

10451060
/**

0 commit comments

Comments
 (0)