From 821a5f85fda588de0442c57e6eb5447727a85cb3 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Wed, 25 May 2022 13:05:40 +0200 Subject: [PATCH 1/2] Add option to invalidate tokens on user logout --- appinfo/Migrations/Version20220525140622.php | 19 ++++++++++++++ js/settings-admin.js | 1 + lib/AppInfo/Application.php | 2 ++ lib/Commands/AddClient.php | 12 +++++++++ lib/Commands/ListClients.php | 1 + lib/Commands/ModifyClient.php | 8 ++++-- lib/Controller/PageController.php | 2 +- lib/Controller/SettingsController.php | 3 +++ lib/Db/Client.php | 11 ++++++++ lib/Db/ClientMapper.php | 11 ++++++++ lib/Hooks/UserHooks.php | 27 ++++++++++++++++++++ templates/client.part.php | 5 ++++ templates/settings-admin.php | 3 +++ 13 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 appinfo/Migrations/Version20220525140622.php diff --git a/appinfo/Migrations/Version20220525140622.php b/appinfo/Migrations/Version20220525140622.php new file mode 100644 index 00000000..bcea0881 --- /dev/null +++ b/appinfo/Migrations/Version20220525140622.php @@ -0,0 +1,19 @@ +getTable("{$prefix}oauth2_clients"); + if (!$table->hasColumn('invalidate_on_logout')) { + $table->addColumn('invalidate_on_logout', Types::BOOLEAN, [ + 'notnull' => true, + 'default' => false, + ]); + } + } +} diff --git a/js/settings-admin.js b/js/settings-admin.js index 85c6c657..0975e1e9 100644 --- a/js/settings-admin.js +++ b/js/settings-admin.js @@ -38,6 +38,7 @@ $(document).ready(function () { $('#oauth2 input[name="name"]').val(''); $('#oauth2 input[name="redirect_uri"]').val(''); $('#oauth2 input[name="allow_subdomains"]').prop('checked', false); + $('#oauth2 input[name="invalidate_on_logout"]').prop('checked', false); } } ); diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index b073320d..6e355c7c 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -57,7 +57,9 @@ public function __construct(array $urlParams = []) { $c->query('OCA\OAuth2\Db\AuthorizationCodeMapper'), $c->query('OCA\OAuth2\Db\AccessTokenMapper'), $c->query('OCA\OAuth2\Db\RefreshTokenMapper'), + $c->query('OCA\OAuth2\Db\ClientMapper'), $c->query('Logger'), + \OC::$server->getUserSession(), $c->query('AppName') ); }); diff --git a/lib/Commands/AddClient.php b/lib/Commands/AddClient.php index 3cb85fd1..ea4d6286 100644 --- a/lib/Commands/AddClient.php +++ b/lib/Commands/AddClient.php @@ -84,6 +84,12 @@ protected function configure() { 'Trust the client even if the redirect-url is localhost.', 'false' ) + ->addArgument( + 'invalidate-on-logout', + InputArgument::OPTIONAL, + 'Invalidate the oauth token when logging out of the ownCloud web client', + 'false' + ) ; } @@ -101,6 +107,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { $allowSubDomains = $input->getArgument('allow-sub-domains'); $trusted = $input->getArgument('trusted'); $forceTrust = $input->getArgument('force-trust'); + $invalidateOnLogout = $input->getArgument('invalidate-on-logout'); if (\strlen($id) < 32) { throw new \InvalidArgumentException('The client id should be at least 32 characters long'); @@ -117,6 +124,9 @@ protected function execute(InputInterface $input, OutputInterface $output) { if (!\in_array($trusted, ['true', 'false'])) { throw new \InvalidArgumentException('Please enter true or false for trusted.'); } + if (!\in_array($invalidateOnLogout, ['true', 'false'])) { + throw new \InvalidArgumentException('Please enter true or false for invalidate-on-logout.'); + } try { // the name should be uniq $this->clientMapper->findByName($name); @@ -139,6 +149,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { $allowSubDomains = \filter_var($allowSubDomains, FILTER_VALIDATE_BOOLEAN); $client->setAllowSubdomains((bool)$allowSubDomains); $trusted = \filter_var($trusted, FILTER_VALIDATE_BOOLEAN); + $invalidateOnLogout = \filter_var($invalidateOnLogout, FILTER_VALIDATE_BOOLEAN); $forceTrust = \filter_var($forceTrust, FILTER_VALIDATE_BOOLEAN); $rURI = new URL(Utilities::removeWildcardPort($url)); if ($trusted && !$forceTrust && ($rURI->hostname === 'localhost' || $rURI->hostname === '127.0.0.1')) { @@ -146,6 +157,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { return 1; } $client->setTrusted((bool)$trusted); + $client->setInvalidateOnLogout((bool)$invalidateOnLogout); $this->clientMapper->insert($client); } diff --git a/lib/Commands/ListClients.php b/lib/Commands/ListClients.php index 407d76bc..8e04809a 100644 --- a/lib/Commands/ListClients.php +++ b/lib/Commands/ListClients.php @@ -64,6 +64,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { 'client-secret' => $client->getSecret(), 'allow-sub-domains' => $client->getAllowSubdomains(), 'trusted' => $client->getTrusted(), + 'invalidate-on-logout' => $client->getInvalidateOnLogout(), ]; } parent::writeArrayInOutputFormat($input, $output, $clientsOutput, self::DEFAULT_OUTPUT_PREFIX, true); diff --git a/lib/Commands/ModifyClient.php b/lib/Commands/ModifyClient.php index 4db84d64..b2161381 100644 --- a/lib/Commands/ModifyClient.php +++ b/lib/Commands/ModifyClient.php @@ -53,7 +53,7 @@ protected function configure() { ->addArgument( 'key', InputArgument::REQUIRED, - 'Key to be changed. Valid keys are : name, client-id, client-secret, redirect-url, allow-sub-domains, trusted' + 'Key to be changed. Valid keys are : name, client-id, client-secret, redirect-url, allow-sub-domains, trusted, invalidate-on-logout' ) ->addArgument( 'value', @@ -89,6 +89,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { 'redirect-url' => 'setRedirectUri', 'allow-sub-domains' => 'setAllowSubdomains', 'trusted' => 'setTrusted', + 'invalidate-on-logout' => 'setInvalidateOnLogout', ]; if (!\array_key_exists($key, $funcMapper)) { @@ -111,8 +112,11 @@ protected function execute(InputInterface $input, OutputInterface $output) { if ($key === 'trusted' && !\in_array($value, ['true', 'false'])) { throw new \InvalidArgumentException('Please enter true or false for trusted.'); } + if ($key === 'invalidate-on-logout' && !\in_array($value, ['true', 'false'])) { + throw new \InvalidArgumentException('Please enter true or false for invalidate-on-logout.'); + } - if ($key === 'trusted' || $key == 'allow-sub-domains') { + if ($key === 'trusted' || $key == 'allow-sub-domains' || $key == 'invalidate-on-logout') { $value = \filter_var($value, FILTER_VALIDATE_BOOLEAN); } diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index c3989b73..a1591535 100755 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -354,7 +354,7 @@ public function logout( ); } // logout the current user - $this->userSession->logout(); +// $this->userSession->logout(); $redirectUrl = $this->urlGenerator->linkToRoute('oauth2.page.authorize', [ 'response_type' => $response_type, diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 29e92e71..67d1fdbe 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -137,6 +137,9 @@ public function addClient(): JSONResponse { } $client->setTrusted($trusted); + $invalidateOnLogout = $this->request->getParam('invalidate_on_logout', null) !== null; + $client->setInvalidateOnLogout($invalidateOnLogout); + $this->clientMapper->insert($client); $this->logger->info('The client "' . $client->getName() . '" has been added.', ['app' => $this->appName]); diff --git a/lib/Db/Client.php b/lib/Db/Client.php index 822d99c1..aa5667c6 100644 --- a/lib/Db/Client.php +++ b/lib/Db/Client.php @@ -33,6 +33,7 @@ * @method void setName(string $name) * @method boolean getAllowSubdomains() * @method boolean getTrusted() + * @method boolean getInvalidateOnLogout() */ class Client extends Entity { protected $identifier; @@ -41,6 +42,7 @@ class Client extends Entity { protected $name; protected $allowSubdomains; protected $trusted; + protected $invalidateOnLogout; /** * Client constructor. @@ -53,6 +55,7 @@ public function __construct() { $this->addType('name', 'string'); $this->addType('allow_subdomains', 'boolean'); $this->addType('trusted', 'boolean'); + $this->addType('invalidateOnLogout', 'boolean'); } public function setAllowSubdomains(bool $value): void { @@ -70,4 +73,12 @@ public function setTrusted(bool $value): void { $this->setter('trusted', [$value]); } } + + public function setInvalidateOnLogout(bool $value): void { + if (\OC::$server->getDatabaseConnection()->getDatabasePlatform() instanceof OraclePlatform) { + $this->setter('invalidateOnLogout', [$value ? 1 : 0]); + } else { + $this->setter('invalidateOnLogout', [$value]); + } + } } diff --git a/lib/Db/ClientMapper.php b/lib/Db/ClientMapper.php index 10e85349..3a7dd95a 100644 --- a/lib/Db/ClientMapper.php +++ b/lib/Db/ClientMapper.php @@ -124,4 +124,15 @@ public function deleteAll() { $stmt = $this->execute($sql, []); $stmt->closeCursor(); } + + /** + * Selects clients whose tokens should be invalidated on logout. + * + * @return Entity[] The client entities. + */ + public function findInvalidateOnLogout() { + $sql = 'SELECT * FROM `' . $this->tableName . '` ' + . 'WHERE `invalidate_on_logout` = ?'; + return $this->findEntities($sql, [1], null, null); + } } diff --git a/lib/Hooks/UserHooks.php b/lib/Hooks/UserHooks.php index 1a53c959..3eae5a83 100644 --- a/lib/Hooks/UserHooks.php +++ b/lib/Hooks/UserHooks.php @@ -23,8 +23,11 @@ use OC\User\User; use OCA\OAuth2\Db\AccessTokenMapper; use OCA\OAuth2\Db\AuthorizationCodeMapper; +use OCA\OAuth2\Db\Client; +use OCA\OAuth2\Db\ClientMapper; use OCA\OAuth2\Db\RefreshTokenMapper; use OCP\ILogger; +use OCP\IUserSession; class UserHooks { @@ -40,12 +43,18 @@ class UserHooks { /** @var RefreshTokenMapper */ private $refreshTokenMapper; + /** @var ClientMapper */ + private $clientMapper; + /** @var ILogger */ private $logger; /** @var string */ private $appName; + /** @var IUserSession */ + private $userSession; + /** * UserHooks constructor. * @@ -53,7 +62,9 @@ class UserHooks { * @param AuthorizationCodeMapper $authorizationCodeMapper The authorization code mapper. * @param AccessTokenMapper $accessTokenMapper The access token mapper. * @param RefreshTokenMapper $refreshTokenMapper The refresh token mapper. + * @param ClientMapper $clientMapper The client mapper. * @param ILogger $logger The logger. + * @param IUserSession $userSession The user session * @param string $AppName The app's name. */ public function __construct( @@ -61,14 +72,18 @@ public function __construct( AuthorizationCodeMapper $authorizationCodeMapper, AccessTokenMapper $accessTokenMapper, RefreshTokenMapper $refreshTokenMapper, + ClientMapper $clientMapper, ILogger $logger, + IUserSession $userSession, $AppName ) { $this->userManager = $userManager; $this->authorizationCodeMapper = $authorizationCodeMapper; $this->accessTokenMapper = $accessTokenMapper; $this->refreshTokenMapper = $refreshTokenMapper; + $this->clientMapper = $clientMapper; $this->logger = $logger; + $this->userSession = $userSession; $this->appName = $AppName; } @@ -91,5 +106,17 @@ public function register() { }; /** @phan-suppress-next-line PhanUndeclaredMethod */ $this->userManager->listen('\OC\User', 'preDelete', $callback); + $this->userManager->listen('\OC\User', 'preLogout', function () { + $user = $this->userSession->getUser(); + $clientsWithInvalidate = $this->clientMapper->findInvalidateOnLogout(); + + foreach ($clientsWithInvalidate as $client) { +// $this->authorizationCodeMapper->deleteByClientUser($client->getId(), $user->getUID()); +// $this->accessTokenMapper->deleteByClientUser($client->getId(), $user->getUID()); +// $this->refreshTokenMapper->deleteByClientUser($client->getId(), $user->getUID()); + } + + return null; + }); } } diff --git a/templates/client.part.php b/templates/client.part.php index aeb71708..249a606a 100644 --- a/templates/client.part.php +++ b/templates/client.part.php @@ -18,6 +18,11 @@ + getInvalidateOnLogout()): ?> + + + + diff --git a/templates/settings-admin.php b/templates/settings-admin.php index 1e16d6fb..00e2da8d 100644 --- a/templates/settings-admin.php +++ b/templates/settings-admin.php @@ -47,6 +47,7 @@ t('Secret')); ?> t('Subdomains allowed')); ?> t('Trusted client')); ?> + t('Invalidate on logout')); ?>   @@ -67,6 +68,8 @@ + + From ae628da9f4ab02d51cf5cf4d8525bf8888b4d6e0 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Wed, 25 May 2022 13:12:15 +0200 Subject: [PATCH 2/2] Add option to invalidate tokens on user logout --- lib/Hooks/UserHooks.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Hooks/UserHooks.php b/lib/Hooks/UserHooks.php index 3eae5a83..1e64d858 100644 --- a/lib/Hooks/UserHooks.php +++ b/lib/Hooks/UserHooks.php @@ -111,9 +111,9 @@ public function register() { $clientsWithInvalidate = $this->clientMapper->findInvalidateOnLogout(); foreach ($clientsWithInvalidate as $client) { -// $this->authorizationCodeMapper->deleteByClientUser($client->getId(), $user->getUID()); -// $this->accessTokenMapper->deleteByClientUser($client->getId(), $user->getUID()); -// $this->refreshTokenMapper->deleteByClientUser($client->getId(), $user->getUID()); + $this->authorizationCodeMapper->deleteByClientUser($client->getId(), $user->getUID()); + $this->accessTokenMapper->deleteByClientUser($client->getId(), $user->getUID()); + $this->refreshTokenMapper->deleteByClientUser($client->getId(), $user->getUID()); } return null;