From 581d3007fb23731665e01276232ff3ba518c907b Mon Sep 17 00:00:00 2001 From: Scott Shambarger Date: Thu, 30 Apr 2020 16:32:35 -0700 Subject: [PATCH 1/3] Skip session validation during app load (Fixes #20756) Adds parameter to User\Session:getUser() to skip validation, and modifies OC_App::getEnabledApps() to use this parameter to skip session validation when loading apps. Signed-off-by: Scott Shambarger --- lib/private/User/Session.php | 14 ++++++++++---- lib/private/legacy/OC_App.php | 3 ++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 817dcbf4c3..9fec8eb08b 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -223,9 +223,10 @@ class Session implements IUserSession, Emitter { /** * get the current active user * + * @param bool $validate whether to validate session * @return IUser|null Current user, otherwise null */ - public function getUser() { + public function getUser($validate = true) { // FIXME: This is a quick'n dirty work-around for the incognito mode as // described at https://github.com/owncloud/core/pull/12912#issuecomment-67391155 if (OC_User::isIncognitoMode()) { @@ -236,11 +237,16 @@ class Session implements IUserSession, Emitter { if (is_null($uid)) { return null; } - $this->activeUser = $this->manager->get($uid); - if (is_null($this->activeUser)) { + // UserManager will cache user for later validation... + $user = $this->manager->get($uid); + if (is_null($user)) { return null; } - $this->validateSession(); + if ($validate === true) { + // only set activeUser when validating... + $this->activeUser = $user; + $this->validateSession(); + } } return $this->activeUser; } diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php index 2454d6be4f..337ccda3a4 100644 --- a/lib/private/legacy/OC_App.php +++ b/lib/private/legacy/OC_App.php @@ -348,7 +348,8 @@ class OC_App { if ($all) { $user = null; } else { - $user = \OC::$server->getUserSession()->getUser(); + // getUser but don't validate session yet + $user = \OC::$server->getUserSession()->getUser(false); } if (is_null($user)) { From 1885d41460ca80b3c2cca0d0d5cc6dc24e8a7e81 Mon Sep 17 00:00:00 2001 From: Scott Shambarger Date: Mon, 4 May 2020 14:00:29 -0700 Subject: [PATCH 2/3] Revert 581d3007f --- lib/private/User/Session.php | 14 ++++---------- lib/private/legacy/OC_App.php | 3 +-- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 9fec8eb08b..817dcbf4c3 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -223,10 +223,9 @@ class Session implements IUserSession, Emitter { /** * get the current active user * - * @param bool $validate whether to validate session * @return IUser|null Current user, otherwise null */ - public function getUser($validate = true) { + public function getUser() { // FIXME: This is a quick'n dirty work-around for the incognito mode as // described at https://github.com/owncloud/core/pull/12912#issuecomment-67391155 if (OC_User::isIncognitoMode()) { @@ -237,16 +236,11 @@ class Session implements IUserSession, Emitter { if (is_null($uid)) { return null; } - // UserManager will cache user for later validation... - $user = $this->manager->get($uid); - if (is_null($user)) { + $this->activeUser = $this->manager->get($uid); + if (is_null($this->activeUser)) { return null; } - if ($validate === true) { - // only set activeUser when validating... - $this->activeUser = $user; - $this->validateSession(); - } + $this->validateSession(); } return $this->activeUser; } diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php index 337ccda3a4..2454d6be4f 100644 --- a/lib/private/legacy/OC_App.php +++ b/lib/private/legacy/OC_App.php @@ -348,8 +348,7 @@ class OC_App { if ($all) { $user = null; } else { - // getUser but don't validate session yet - $user = \OC::$server->getUserSession()->getUser(false); + $user = \OC::$server->getUserSession()->getUser(); } if (is_null($user)) { From ff8695fdb43aa303f5cccd2d9975c762b44726ae Mon Sep 17 00:00:00 2001 From: Scott Shambarger Date: Mon, 4 May 2020 14:03:27 -0700 Subject: [PATCH 3/3] Modify loadApps to always load all protected apps (Fixes #20756) Adds a new AppManager method which identifies when all app types are protected, loads all apps (without checking user filters). This has the intended benefit of avoiding calls to User\Session:getUser() for these app types, and likely invalidating the session before user backends are loaded. Signed-off-by: Scott Shambarger --- lib/private/App/AppManager.php | 15 +++++++++++++++ lib/private/legacy/OC_App.php | 6 +++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index f756664e45..40b329d249 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -350,6 +350,21 @@ class AppManager implements IAppManager { return !empty($protectedTypes); } + /** + * Whether a list of types contains only protected app types + * + * @param string[] $types + * @return bool + */ + public function hasOnlyProtectedAppTypes($types) { + if (empty($types)) { + return false; + } + + $unprotectedTypes = array_diff($types, $this->protectedAppTypes); + return empty($unprotectedTypes); + } + /** * Enable an app only for specific groups * diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php index 2454d6be4f..23712312e1 100644 --- a/lib/private/legacy/OC_App.php +++ b/lib/private/legacy/OC_App.php @@ -110,8 +110,12 @@ class OC_App { if ((bool) \OC::$server->getSystemConfig()->getValue('maintenance', false)) { return false; } + // If only protected types, don't filter by user (prevents + // session invalidation when loading prelogin/authentication + // types). + $all = \OC::$server->getAppManager()->hasOnlyProtectedAppTypes($types); // Load the enabled apps here - $apps = self::getEnabledApps(); + $apps = self::getEnabledApps(false, $all); // Add each apps' folder as allowed class path foreach ($apps as $app) {