From 5f71805c35d04e585ea6d4227254b11204413dfd Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 4 May 2017 23:46:59 +0200 Subject: [PATCH 01/35] Add basic implementation for OAuth 2.0 Authorization Code Flow Signed-off-by: Lukas Reschke --- apps/dav/lib/Connector/Sabre/Auth.php | 13 +++ apps/oauth2/appinfo/database.xml | 79 ++++++++++++++++ apps/oauth2/appinfo/info.xml | 18 ++++ apps/oauth2/appinfo/routes.php | 46 ++++++++++ .../Controller/LoginRedirectorController.php | 78 ++++++++++++++++ .../lib/Controller/OauthApiController.php | 88 ++++++++++++++++++ .../lib/Controller/SettingsController.php | 86 +++++++++++++++++ apps/oauth2/lib/Db/AccessToken.php | 53 +++++++++++ apps/oauth2/lib/Db/AccessTokenMapper.php | 49 ++++++++++ apps/oauth2/lib/Db/Client.php | 53 +++++++++++ apps/oauth2/lib/Db/ClientMapper.php | 61 ++++++++++++ apps/oauth2/lib/Settings/Admin.php | 67 ++++++++++++++ apps/oauth2/templates/admin.php | 70 ++++++++++++++ core/Controller/ClientFlowLoginController.php | 92 +++++++++++++++---- core/templates/loginflow/authpicker.php | 2 +- core/templates/loginflow/redirect.php | 2 + lib/private/User/Session.php | 4 +- 17 files changed, 838 insertions(+), 23 deletions(-) create mode 100644 apps/oauth2/appinfo/database.xml create mode 100644 apps/oauth2/appinfo/info.xml create mode 100644 apps/oauth2/appinfo/routes.php create mode 100644 apps/oauth2/lib/Controller/LoginRedirectorController.php create mode 100644 apps/oauth2/lib/Controller/OauthApiController.php create mode 100644 apps/oauth2/lib/Controller/SettingsController.php create mode 100644 apps/oauth2/lib/Db/AccessToken.php create mode 100644 apps/oauth2/lib/Db/AccessTokenMapper.php create mode 100644 apps/oauth2/lib/Db/Client.php create mode 100644 apps/oauth2/lib/Db/ClientMapper.php create mode 100644 apps/oauth2/lib/Settings/Admin.php create mode 100644 apps/oauth2/templates/admin.php diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php index bdaf73d46e7..7ddbb70530a 100644 --- a/apps/dav/lib/Connector/Sabre/Auth.php +++ b/apps/dav/lib/Connector/Sabre/Auth.php @@ -210,6 +210,19 @@ class Auth extends AbstractBasic { */ private function auth(RequestInterface $request, ResponseInterface $response) { $forcedLogout = false; + + $authHeader = $request->getHeader('Authorization'); + if (strpos($authHeader, 'Bearer ') !== false) { + if($this->userSession->tryTokenLogin($this->request)) { + $this->session->set(self::DAV_AUTHENTICATED, $this->userSession->getUser()->getUID()); + $user = $this->userSession->getUser()->getUID(); + \OC_Util::setupFS($user); + $this->currentUser = $user; + $this->session->close(); + return [true, $this->principalPrefix . $user]; + } + } + if(!$this->request->passesCSRFCheck() && $this->requiresCSRFCheck()) { // In case of a fail with POST we need to recheck the credentials diff --git a/apps/oauth2/appinfo/database.xml b/apps/oauth2/appinfo/database.xml new file mode 100644 index 00000000000..2d7e3502db2 --- /dev/null +++ b/apps/oauth2/appinfo/database.xml @@ -0,0 +1,79 @@ + + + *dbname* + true + false + utf8 + + *dbprefix*oauth2_clients + + + id + integer + true + true + true + true + + + name + text + true + 64 + + + redirect_uri + text + true + 2000 + + + client_identifier + text + true + 64 + + + secret + text + true + 64 + + +
+ + *dbprefix*oauth2_access_tokens + + + id + integer + true + true + true + true + + + token_id + integer + true + + + client_id + integer + true + + + hashed_code + text + true + 128 + + + encrypted_token + text + true + 255 + + +
+ diff --git a/apps/oauth2/appinfo/info.xml b/apps/oauth2/appinfo/info.xml new file mode 100644 index 00000000000..ebead97eb72 --- /dev/null +++ b/apps/oauth2/appinfo/info.xml @@ -0,0 +1,18 @@ + + + oauth2 + OAuth 2.0 + The OAuth2 app allows administrators to configure the built-in authentication workflow to also allow OAuth2 compatible authentication from other web applications. + AGPL + Lukas Reschke + OAuth2 + 1.0.3 + + + + + + + OCA\OAuth2\Settings\Admin + + diff --git a/apps/oauth2/appinfo/routes.php b/apps/oauth2/appinfo/routes.php new file mode 100644 index 00000000000..b088dff0d48 --- /dev/null +++ b/apps/oauth2/appinfo/routes.php @@ -0,0 +1,46 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +return [ + 'routes' => [ + [ + 'name' => 'Settings#addClient', + 'url' => '/settings', + 'verb' => 'POST', + ], + [ + 'name' => 'Settings#deleteClient', + 'url' => '/clients/{id}/delete', + 'verb' => 'POST' + ], + [ + 'name' => 'LoginRedirector#authorize', + 'url' => '/authorize', + 'verb' => 'GET', + ], + [ + 'name' => 'OauthApi#getToken', + 'url' => '/api/v1/token', + // TODO: POST! + 'verb' => 'GET' + ], + ], +]; \ No newline at end of file diff --git a/apps/oauth2/lib/Controller/LoginRedirectorController.php b/apps/oauth2/lib/Controller/LoginRedirectorController.php new file mode 100644 index 00000000000..1a2e00ef5dc --- /dev/null +++ b/apps/oauth2/lib/Controller/LoginRedirectorController.php @@ -0,0 +1,78 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Controller; + +use OCA\OAuth2\Db\ClientMapper; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\RedirectResponse; +use OCP\IRequest; +use OCP\IURLGenerator; + +class LoginRedirectorController extends Controller { + /** @var IURLGenerator */ + private $urlGenerator; + /** @var ClientMapper */ + private $clientMapper; + + /** + * @param string $appName + * @param IRequest $request + * @param IURLGenerator $urlGenerator + * @param ClientMapper $clientMapper + */ + public function __construct($appName, + IRequest $request, + IURLGenerator $urlGenerator, + ClientMapper $clientMapper) { + parent::__construct($appName, $request); + $this->urlGenerator = $urlGenerator; + $this->clientMapper = $clientMapper; + } + + /** + * @PublicPage + * @NoCSRFRequired + * + * @param string $client_id + * @param string $redirect_uri + * @param string $state + * @return RedirectResponse + */ + public function authorize($client_id, + $redirect_uri, + $state) { + $client = $this->clientMapper->getByIdentifier($client_id); + + if($client->getRedirectUri() !== $redirect_uri) { + throw new \Exception('Redirect URI does not match'); + } + + $targetUrl = $this->urlGenerator->linkToRouteAbsolute( + 'core.ClientFlowLogin.showAuthPickerPage', + [ + 'clientIdentifier' => $client->getClientIdentifier(), + 'oauthState' => $state, + ] + ); + return new RedirectResponse($targetUrl); + } +} diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php new file mode 100644 index 00000000000..8432830bce3 --- /dev/null +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -0,0 +1,88 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Controller; + +use OC\Authentication\Token\DefaultTokenMapper; +use OCA\OAuth2\Db\AccessTokenMapper; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\JSONResponse; +use OCP\IRequest; +use OCP\Security\ICrypto; +use OCP\Security\ISecureRandom; + +class OauthApiController extends Controller { + /** @var AccessTokenMapper */ + private $accessTokenMapper; + /** @var ICrypto */ + private $crypto; + /** @var DefaultTokenMapper */ + private $defaultTokenMapper; + /** @var ISecureRandom */ + private $secureRandom; + + /** + * @param string $appName + * @param IRequest $request + * @param ICrypto $crypto + * @param AccessTokenMapper $accessTokenMapper + * @param DefaultTokenMapper $defaultTokenMapper + * @param ISecureRandom $secureRandom + */ + public function __construct($appName, + IRequest $request, + ICrypto $crypto, + AccessTokenMapper $accessTokenMapper, + DefaultTokenMapper $defaultTokenMapper, + ISecureRandom $secureRandom) { + parent::__construct($appName, $request); + $this->crypto = $crypto; + $this->accessTokenMapper = $accessTokenMapper; + $this->defaultTokenMapper = $defaultTokenMapper; + $this->secureRandom = $secureRandom; + } + + /** + * @PublicPage + * @NoCSRFRequired + * + * @param string $code + * @return JSONResponse + */ + public function getToken($code) { + $accessToken = $this->accessTokenMapper->getByCode($code); + $decryptedToken = $this->crypto->decrypt($accessToken->getEncryptedToken(), $code); + $newCode = $this->secureRandom->generate(128); + $accessToken->setHashedCode(hash('sha512', $newCode)); + $accessToken->setEncryptedToken($this->crypto->encrypt($decryptedToken, $newCode)); + $this->accessTokenMapper->update($accessToken); + + return new JSONResponse( + [ + 'access_token' => $decryptedToken, + 'token_type' => 'token', + 'expires_in' => 3600, + 'refresh_token' => $newCode, + 'user_id' => ($this->defaultTokenMapper->getTokenById($accessToken->getTokenId()))->getUID(), + ] + ); + } +} diff --git a/apps/oauth2/lib/Controller/SettingsController.php b/apps/oauth2/lib/Controller/SettingsController.php new file mode 100644 index 00000000000..1d376694f5a --- /dev/null +++ b/apps/oauth2/lib/Controller/SettingsController.php @@ -0,0 +1,86 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Controller; + +use OCA\OAuth2\Db\Client; +use OCA\OAuth2\Db\ClientMapper; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\RedirectResponse; +use OCP\IRequest; +use OCP\IURLGenerator; +use OCP\Security\ISecureRandom; + +class SettingsController extends Controller { + /** @var IURLGenerator */ + private $urlGenerator; + /** @var ClientMapper */ + private $clientMapper; + /** @var ISecureRandom */ + private $secureRandom; + + const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; + + /** + * @param string $appName + * @param IRequest $request + * @param IURLGenerator $urlGenerator + * @param ClientMapper $clientMapper + * @param ISecureRandom $secureRandom + */ + public function __construct($appName, + IRequest $request, + IURLGenerator $urlGenerator, + ClientMapper $clientMapper, + ISecureRandom $secureRandom) { + parent::__construct($appName, $request); + $this->urlGenerator = $urlGenerator; + $this->secureRandom = $secureRandom; + $this->clientMapper = $clientMapper; + } + + /** + * @param string $name + * @param string $redirectUri + * @return RedirectResponse + */ + public function addClient($name, + $redirectUri) { + $client = new Client(); + $client->setName($name); + $client->setRedirectUri($redirectUri); + $client->setSecret($this->secureRandom->generate(64, self::validChars)); + $client->setClientIdentifier($this->secureRandom->generate(64, self::validChars)); + $this->clientMapper->insert($client); + return new RedirectResponse($this->urlGenerator->getAbsoluteURL('/index.php/settings/admin/security')); + } + + /** + * @param int $id + * @return RedirectResponse + */ + public function deleteClient($id) { + $client = new Client(); + $client->setId($id); + $this->clientMapper->delete($client); + return new RedirectResponse($this->urlGenerator->getAbsoluteURL('/index.php/settings/admin/security')); + } +} diff --git a/apps/oauth2/lib/Db/AccessToken.php b/apps/oauth2/lib/Db/AccessToken.php new file mode 100644 index 00000000000..8266a9a0068 --- /dev/null +++ b/apps/oauth2/lib/Db/AccessToken.php @@ -0,0 +1,53 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Db; + +use OCP\AppFramework\Db\Entity; + +/** + * @method int getTokenId() + * @method void setTokenId(int $identifier) + * @method int getClientId() + * @method void setClientId(int $identifier) + * @method string getEncryptedToken() + * @method void setEncryptedToken(string $token) + * @method string getHashedCode() + * @method void setHashedCode(string $token) + */ +class AccessToken extends Entity { + /** @var int */ + protected $tokenId; + /** @var int */ + protected $clientId; + /** @var string */ + protected $hashedCode; + /** @var string */ + protected $encryptedToken; + + public function __construct() { + $this->addType('id', 'int'); + $this->addType('token_id', 'int'); + $this->addType('client_id', 'int'); + $this->addType('hashed_code', 'string'); + $this->addType('encrypted_token', 'string'); + } +} diff --git a/apps/oauth2/lib/Db/AccessTokenMapper.php b/apps/oauth2/lib/Db/AccessTokenMapper.php new file mode 100644 index 00000000000..0400ad6366c --- /dev/null +++ b/apps/oauth2/lib/Db/AccessTokenMapper.php @@ -0,0 +1,49 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Db; + +use OCP\AppFramework\Db\Mapper; +use OCP\IDBConnection; + +class AccessTokenMapper extends Mapper { + + /** + * @param IDBConnection $db + */ + public function __construct(IDBConnection $db) { + parent::__construct($db, 'oauth2_access_tokens'); + } + + /** + * @param string $code + * @return AccessToken + */ + public function getByCode($code) { + $qb = $this->db->getQueryBuilder(); + $qb + ->select('*') + ->from($this->tableName) + ->where($qb->expr()->eq('hashed_code', $qb->createParameter('hashedCode'))); + + return $this->findEntity($qb->getSQL(), [hash('sha512', $code)]); + } +} diff --git a/apps/oauth2/lib/Db/Client.php b/apps/oauth2/lib/Db/Client.php new file mode 100644 index 00000000000..85c1630cb15 --- /dev/null +++ b/apps/oauth2/lib/Db/Client.php @@ -0,0 +1,53 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Db; + +use OCP\AppFramework\Db\Entity; + +/** + * @method string getClientIdentifier() + * @method void setClientIdentifier(string $identifier) + * @method string getSecret() + * @method void setSecret(string $secret) + * @method string getRedirectUri() + * @method void setRedirectUri(string $redirectUri) + * @method string getName() + * @method void setName(string $name) + */ +class Client extends Entity { + /** @var string */ + protected $name; + /** @var string */ + protected $redirectUri; + /** @var string */ + protected $clientIdentifier; + /** @var string */ + protected $secret; + + public function __construct() { + $this->addType('id', 'int'); + $this->addType('name', 'string'); + $this->addType('redirect_uri', 'string'); + $this->addType('client_identifier', 'string'); + $this->addType('secret', 'string'); + } +} diff --git a/apps/oauth2/lib/Db/ClientMapper.php b/apps/oauth2/lib/Db/ClientMapper.php new file mode 100644 index 00000000000..d3c09ac5c69 --- /dev/null +++ b/apps/oauth2/lib/Db/ClientMapper.php @@ -0,0 +1,61 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Db; + +use OCP\AppFramework\Db\Mapper; +use OCP\IDBConnection; + +class ClientMapper extends Mapper { + + /** + * @param IDBConnection $db + */ + public function __construct(IDBConnection $db) { + parent::__construct($db, 'oauth2_clients'); + } + + /** + * @param string $clientIdentifier + * @return Client + */ + public function getByIdentifier($clientIdentifier) { + $qb = $this->db->getQueryBuilder(); + $qb + ->select('*') + ->from($this->tableName) + ->where($qb->expr()->eq('client_identifier', $qb->createParameter('clientId'))); + + return $this->findEntity($qb->getSQL(), [$clientIdentifier]); + } + + /** + * @return Client[] + */ + public function getClients() { + $qb = $this->db->getQueryBuilder(); + $qb + ->select('*') + ->from($this->tableName); + + return $this->findEntities($qb->getSQL()); + } +} diff --git a/apps/oauth2/lib/Settings/Admin.php b/apps/oauth2/lib/Settings/Admin.php new file mode 100644 index 00000000000..aa120bcb7d7 --- /dev/null +++ b/apps/oauth2/lib/Settings/Admin.php @@ -0,0 +1,67 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Settings; + +use OCA\OAuth2\Db\ClientMapper; +use OCP\AppFramework\Http\TemplateResponse; +use OCP\IConfig; +use OCP\Settings\ISettings; + +class Admin implements ISettings { + /** @var ClientMapper */ + private $clientMapper; + + /** + * @param ClientMapper $clientMapper + */ + public function __construct(ClientMapper $clientMapper) { + $this->clientMapper = $clientMapper; + } + + /** + * @return TemplateResponse + */ + public function getForm() { + return new TemplateResponse( + 'oauth2', + 'admin', + [ + 'clients' => $this->clientMapper->getClients(), + ], + '' + ); + } + + /** + * {@inheritdoc} + */ + public function getSection() { + return 'security'; + } + + /** + * {@inheritdoc} + */ + public function getPriority() { + return 0; + } +} diff --git a/apps/oauth2/templates/admin.php b/apps/oauth2/templates/admin.php new file mode 100644 index 00000000000..f5b8532e6b1 --- /dev/null +++ b/apps/oauth2/templates/admin.php @@ -0,0 +1,70 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +$urlGenerator = \OC::$server->getURLGenerator(); +$themingDefaults = \OC::$server->getThemingDefaults(); + +/** @var array $_ */ +/** @var \OCA\OAuth2\Db\Client[] $clients */ +$clients = $_['clients']; +?> + +
+

t('OAuth 2.0 clients')); ?>

+

t('OAuth 2.0 allows external services to request access to your %s.', [$themingDefaults->getName()])); ?>

+ + + + + + + + + + + + + + + + + + + + + + +
t('Name')); ?>t('Redirection URI')); ?>t('Client Identifier')); ?>t('Secret')); ?> 
getName()); ?>getRedirectUri()); ?>getClientIdentifier()); ?>getSecret()); ?> +
+ + +
+
+ +
+

t('Add client')); ?>

+
+ + + + +
+
\ No newline at end of file diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 8c2c121d5b2..45287f3048c 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -25,6 +25,9 @@ use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; +use OCA\OAuth2\Db\AccessToken; +use OCA\OAuth2\Db\AccessTokenMapper; +use OCA\OAuth2\Db\ClientMapper; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Response; @@ -35,6 +38,7 @@ use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; use OCP\IUserSession; +use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; use OCP\Session\Exceptions\SessionNotAvailableException; @@ -53,6 +57,12 @@ class ClientFlowLoginController extends Controller { private $random; /** @var IURLGenerator */ private $urlGenerator; + /** @var ClientMapper */ + private $clientMapper; + /** @var AccessTokenMapper */ + private $accessTokenMapper; + /** @var ICrypto */ + private $crypto; const stateName = 'client.flow.state.token'; @@ -66,6 +76,9 @@ class ClientFlowLoginController extends Controller { * @param IProvider $tokenProvider * @param ISecureRandom $random * @param IURLGenerator $urlGenerator + * @param ClientMapper $clientMapper + * @param AccessTokenMapper $accessTokenMapper + * @param ICrypto $crypto */ public function __construct($appName, IRequest $request, @@ -75,7 +88,10 @@ class ClientFlowLoginController extends Controller { ISession $session, IProvider $tokenProvider, ISecureRandom $random, - IURLGenerator $urlGenerator) { + IURLGenerator $urlGenerator, + ClientMapper $clientMapper, + AccessTokenMapper $accessTokenMapper, + ICrypto $crypto) { parent::__construct($appName, $request); $this->userSession = $userSession; $this->l10n = $l10n; @@ -84,6 +100,9 @@ class ClientFlowLoginController extends Controller { $this->tokenProvider = $tokenProvider; $this->random = $random; $this->urlGenerator = $urlGenerator; + $this->clientMapper = $clientMapper; + $this->accessTokenMapper = $accessTokenMapper; + $this->crypto = $crypto; } /** @@ -126,31 +145,31 @@ class ClientFlowLoginController extends Controller { * @NoCSRFRequired * @UseSession * + * @param string $clientIdentifier + * * @return TemplateResponse */ - public function showAuthPickerPage() { - if($this->userSession->isLoggedIn()) { - return new TemplateResponse( - $this->appName, - '403', - [ - 'file' => $this->l10n->t('Auth flow can only be started unauthenticated.'), - ], - 'guest' - ); - } - + public function showAuthPickerPage($clientIdentifier = '', + $oauthState = '') { $stateToken = $this->random->generate( 64, ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_DIGITS ); $this->session->set(self::stateName, $stateToken); + $clientName = $this->getClientName(); + if($clientIdentifier !== '') { + $client = $this->clientMapper->getByIdentifier($clientIdentifier); + $clientName = $client->getName(); + } + return new TemplateResponse( $this->appName, 'loginflow/authpicker', [ - 'client' => $this->getClientName(), + 'client' => $clientName, + 'clientIdentifier' => $clientIdentifier, + 'oauthState' => $oauthState, 'instanceName' => $this->defaults->getName(), 'urlGenerator' => $this->urlGenerator, 'stateToken' => $stateToken, @@ -166,9 +185,13 @@ class ClientFlowLoginController extends Controller { * @UseSession * * @param string $stateToken + * @param string $clientIdentifier + * @param string $oauthState * @return TemplateResponse */ - public function redirectPage($stateToken = '') { + public function redirectPage($stateToken = '', + $clientIdentifier = '', + $oauthState = '') { if(!$this->isValidToken($stateToken)) { return $this->stateTokenForbiddenResponse(); } @@ -179,6 +202,8 @@ class ClientFlowLoginController extends Controller { [ 'urlGenerator' => $this->urlGenerator, 'stateToken' => $stateToken, + 'clientIdentifier' => $clientIdentifier, + 'oauthState' => $oauthState, ], 'empty' ); @@ -189,9 +214,15 @@ class ClientFlowLoginController extends Controller { * @UseSession * * @param string $stateToken + * @param string $clientIdentifier + * @param string $state + * @param string $oauthState * @return Http\RedirectResponse|Response */ - public function generateAppPassword($stateToken) { + public function generateAppPassword($stateToken, + $clientIdentifier = '', + $state = '', + $oauthState = '') { if(!$this->isValidToken($stateToken)) { $this->session->remove(self::stateName); return $this->stateTokenForbiddenResponse(); @@ -222,9 +253,10 @@ class ClientFlowLoginController extends Controller { } $token = $this->random->generate(72); - $this->tokenProvider->generateToken( + $uid = $this->userSession->getUser()->getUID(); + $generatedToken = $this->tokenProvider->generateToken( $token, - $this->userSession->getUser()->getUID(), + $uid, $loginName, $password, $this->getClientName(), @@ -232,7 +264,27 @@ class ClientFlowLoginController extends Controller { IToken::DO_NOT_REMEMBER ); - return new Http\RedirectResponse('nc://login/server:' . $this->request->getServerHost() . '&user:' . urlencode($loginName) . '&password:' . urlencode($token)); - } + if($clientIdentifier !== '') { + $client = $this->clientMapper->getByIdentifier($clientIdentifier); + $code = $this->random->generate(128); + $accessToken = new AccessToken(); + $accessToken->setClientId($client->getId()); + $accessToken->setEncryptedToken($this->crypto->encrypt($token, $code)); + $accessToken->setHashedCode(hash('sha512', $code)); + $accessToken->setTokenId($generatedToken->getId()); + $this->accessTokenMapper->insert($accessToken); + + $redirectUri = sprintf( + '%s?state=%s&code=%s', + $client->getRedirectUri(), + urlencode($oauthState), + urlencode($code) + ); + } else { + $redirectUri = 'nc://login/server:' . $this->request->getServerHost() . '&user:' . urlencode($loginName) . '&password:' . urlencode($token); + } + + return new Http\RedirectResponse($redirectUri); + } } diff --git a/core/templates/loginflow/authpicker.php b/core/templates/loginflow/authpicker.php index c5eb6cb316d..127a1156a68 100644 --- a/core/templates/loginflow/authpicker.php +++ b/core/templates/loginflow/authpicker.php @@ -35,7 +35,7 @@ $urlGenerator = $_['urlGenerator'];
diff --git a/core/templates/loginflow/redirect.php b/core/templates/loginflow/redirect.php index 7ef0184f61f..1c6e105b88d 100644 --- a/core/templates/loginflow/redirect.php +++ b/core/templates/loginflow/redirect.php @@ -31,7 +31,9 @@ $urlGenerator = $_['urlGenerator'];
+ +
diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index f818666c374..0291c1baecb 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -725,7 +725,7 @@ class Session implements IUserSession, Emitter { */ public function tryTokenLogin(IRequest $request) { $authHeader = $request->getHeader('Authorization'); - if (strpos($authHeader, 'token ') === false) { + if (strpos($authHeader, 'Bearer ') === false) { // No auth header, let's try session id try { $token = $this->session->getId(); @@ -733,7 +733,7 @@ class Session implements IUserSession, Emitter { return false; } } else { - $token = substr($authHeader, 6); + $token = substr($authHeader, 7); } if (!$this->loginWithToken($token)) { From a5ddd65c10ff8812f6b49bc9703a2858a6c323e6 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 4 May 2017 23:55:38 +0200 Subject: [PATCH 02/35] Use query builder Signed-off-by: Lukas Reschke --- apps/oauth2/lib/Db/AccessTokenMapper.php | 10 +++++++--- apps/oauth2/lib/Db/ClientMapper.php | 10 +++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/apps/oauth2/lib/Db/AccessTokenMapper.php b/apps/oauth2/lib/Db/AccessTokenMapper.php index 0400ad6366c..0b54d5c3da4 100644 --- a/apps/oauth2/lib/Db/AccessTokenMapper.php +++ b/apps/oauth2/lib/Db/AccessTokenMapper.php @@ -42,8 +42,12 @@ class AccessTokenMapper extends Mapper { $qb ->select('*') ->from($this->tableName) - ->where($qb->expr()->eq('hashed_code', $qb->createParameter('hashedCode'))); - - return $this->findEntity($qb->getSQL(), [hash('sha512', $code)]); + ->where($qb->expr()->eq('hashed_code', $qb->createNamedParameter(hash('sha512', $code)))); + $result = $qb->execute(); + $rows = $result->fetchAll(); + $result->closeCursor(); + return array_map(function ($row) { + return AccessToken::fromRow($row); + }, $rows); } } diff --git a/apps/oauth2/lib/Db/ClientMapper.php b/apps/oauth2/lib/Db/ClientMapper.php index d3c09ac5c69..8d1f26ce471 100644 --- a/apps/oauth2/lib/Db/ClientMapper.php +++ b/apps/oauth2/lib/Db/ClientMapper.php @@ -42,9 +42,13 @@ class ClientMapper extends Mapper { $qb ->select('*') ->from($this->tableName) - ->where($qb->expr()->eq('client_identifier', $qb->createParameter('clientId'))); - - return $this->findEntity($qb->getSQL(), [$clientIdentifier]); + ->where($qb->expr()->eq('client_identifier', $qb->createNamedParameter($clientIdentifier))); + $result = $qb->execute(); + $rows = $result->fetchAll(); + $result->closeCursor(); + return array_map(function ($row) { + return Client::fromRow($row); + }, $rows); } /** From 0a2b57c93fb8231615dbbf2aebba369d78700541 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 4 May 2017 23:58:45 +0200 Subject: [PATCH 03/35] Get a single row Signed-off-by: Lukas Reschke --- apps/oauth2/lib/Db/AccessTokenMapper.php | 6 ++---- apps/oauth2/lib/Db/ClientMapper.php | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/apps/oauth2/lib/Db/AccessTokenMapper.php b/apps/oauth2/lib/Db/AccessTokenMapper.php index 0b54d5c3da4..9702e2c58fd 100644 --- a/apps/oauth2/lib/Db/AccessTokenMapper.php +++ b/apps/oauth2/lib/Db/AccessTokenMapper.php @@ -44,10 +44,8 @@ class AccessTokenMapper extends Mapper { ->from($this->tableName) ->where($qb->expr()->eq('hashed_code', $qb->createNamedParameter(hash('sha512', $code)))); $result = $qb->execute(); - $rows = $result->fetchAll(); + $row = $result->fetchAll(); $result->closeCursor(); - return array_map(function ($row) { - return AccessToken::fromRow($row); - }, $rows); + return AccessToken::fromRow($row); } } diff --git a/apps/oauth2/lib/Db/ClientMapper.php b/apps/oauth2/lib/Db/ClientMapper.php index 8d1f26ce471..8531f6d007c 100644 --- a/apps/oauth2/lib/Db/ClientMapper.php +++ b/apps/oauth2/lib/Db/ClientMapper.php @@ -44,11 +44,9 @@ class ClientMapper extends Mapper { ->from($this->tableName) ->where($qb->expr()->eq('client_identifier', $qb->createNamedParameter($clientIdentifier))); $result = $qb->execute(); - $rows = $result->fetchAll(); + $row = $result->fetch(); $result->closeCursor(); - return array_map(function ($row) { - return Client::fromRow($row); - }, $rows); + return Client::fromRow($row); } /** From c5ad3c9213830abb6d76a7a98d1b308c7ffa187f Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 5 May 2017 00:00:32 +0200 Subject: [PATCH 04/35] Increase length of encrypted token in database Signed-off-by: Lukas Reschke --- apps/oauth2/appinfo/database.xml | 2 +- apps/oauth2/appinfo/info.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/oauth2/appinfo/database.xml b/apps/oauth2/appinfo/database.xml index 2d7e3502db2..48de162299c 100644 --- a/apps/oauth2/appinfo/database.xml +++ b/apps/oauth2/appinfo/database.xml @@ -72,7 +72,7 @@ encrypted_token text true - 255 + 786 diff --git a/apps/oauth2/appinfo/info.xml b/apps/oauth2/appinfo/info.xml index ebead97eb72..8b24d4581fb 100644 --- a/apps/oauth2/appinfo/info.xml +++ b/apps/oauth2/appinfo/info.xml @@ -6,7 +6,7 @@ AGPL Lukas Reschke OAuth2 - 1.0.3 + 1.0.4 From 4d96cd3df7e092aa32e1212e26aaad614ecbd54e Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 5 May 2017 00:02:18 +0200 Subject: [PATCH 05/35] Change to POST Signed-off-by: Lukas Reschke --- apps/oauth2/appinfo/routes.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/oauth2/appinfo/routes.php b/apps/oauth2/appinfo/routes.php index b088dff0d48..808f2a22395 100644 --- a/apps/oauth2/appinfo/routes.php +++ b/apps/oauth2/appinfo/routes.php @@ -39,8 +39,7 @@ return [ [ 'name' => 'OauthApi#getToken', 'url' => '/api/v1/token', - // TODO: POST! - 'verb' => 'GET' + 'verb' => 'POST' ], ], ]; \ No newline at end of file From 1470ec95ca1deb3e38be633e7a4a74303fbaa64b Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 5 May 2017 00:08:23 +0200 Subject: [PATCH 06/35] Fetch signle row Signed-off-by: Lukas Reschke --- apps/oauth2/lib/Db/AccessTokenMapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/oauth2/lib/Db/AccessTokenMapper.php b/apps/oauth2/lib/Db/AccessTokenMapper.php index 9702e2c58fd..2751302522c 100644 --- a/apps/oauth2/lib/Db/AccessTokenMapper.php +++ b/apps/oauth2/lib/Db/AccessTokenMapper.php @@ -44,7 +44,7 @@ class AccessTokenMapper extends Mapper { ->from($this->tableName) ->where($qb->expr()->eq('hashed_code', $qb->createNamedParameter(hash('sha512', $code)))); $result = $qb->execute(); - $row = $result->fetchAll(); + $row = $result->fetch(); $result->closeCursor(); return AccessToken::fromRow($row); } From 4b4d3bb1c2aed09c288350822e6677426594a7ea Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 5 May 2017 00:19:28 +0200 Subject: [PATCH 07/35] It's a bearer Signed-off-by: Lukas Reschke --- apps/oauth2/lib/Controller/OauthApiController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 8432830bce3..97f8185149a 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -78,7 +78,7 @@ class OauthApiController extends Controller { return new JSONResponse( [ 'access_token' => $decryptedToken, - 'token_type' => 'token', + 'token_type' => 'Bearer', 'expires_in' => 3600, 'refresh_token' => $newCode, 'user_id' => ($this->defaultTokenMapper->getTokenById($accessToken->getTokenId()))->getUID(), From e86749121c626db9d6937c182634096de6a015fe Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 5 May 2017 00:40:23 +0200 Subject: [PATCH 08/35] Remove special characters Signed-off-by: Lukas Reschke --- core/Controller/ClientFlowLoginController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 45287f3048c..70cf8e8cebc 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -252,7 +252,7 @@ class ClientFlowLoginController extends Controller { return $response; } - $token = $this->random->generate(72); + $token = $this->random->generate(72, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS); $uid = $this->userSession->getUser()->getUID(); $generatedToken = $this->tokenProvider->generateToken( $token, From 3775b14c4c6889a4dbffa633f2cc3eef9f3ea624 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 12 May 2017 12:00:24 +0200 Subject: [PATCH 09/35] remove 'Alternative login using app token' in case of oauth login Signed-off-by: Bjoern Schiessle --- core/templates/loginflow/authpicker.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/templates/loginflow/authpicker.php b/core/templates/loginflow/authpicker.php index 127a1156a68..c427d657e4a 100644 --- a/core/templates/loginflow/authpicker.php +++ b/core/templates/loginflow/authpicker.php @@ -54,4 +54,6 @@ $urlGenerator = $_['urlGenerator']; + t('Alternative login using app token')) ?> + From 1a8965b488e436099bf6e1bbd025d652e9791fe7 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 12 May 2017 12:43:22 +0200 Subject: [PATCH 10/35] handle case if no valid client identifier is given Signed-off-by: Bjoern Schiessle --- apps/oauth2/lib/Db/ClientMapper.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/oauth2/lib/Db/ClientMapper.php b/apps/oauth2/lib/Db/ClientMapper.php index 8531f6d007c..38751a2e5cf 100644 --- a/apps/oauth2/lib/Db/ClientMapper.php +++ b/apps/oauth2/lib/Db/ClientMapper.php @@ -46,6 +46,11 @@ class ClientMapper extends Mapper { $result = $qb->execute(); $row = $result->fetch(); $result->closeCursor(); + + if (!is_array($row)) { + $row = []; + } + return Client::fromRow($row); } From a74d67b69c986f1703567bc5986daed9f82f4571 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 12 May 2017 12:44:22 +0200 Subject: [PATCH 11/35] show error page if no valid client identifier is given and if it is not a API request Signed-off-by: Bjoern Schiessle --- core/Controller/ClientFlowLoginController.php | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 70cf8e8cebc..996ae34b0f2 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -151,18 +151,37 @@ class ClientFlowLoginController extends Controller { */ public function showAuthPickerPage($clientIdentifier = '', $oauthState = '') { + + + $clientName = $this->getClientName(); + $client = null; + if($clientIdentifier !== '') { + $client = $this->clientMapper->getByIdentifier($clientIdentifier); + $clientName = $client->getName(); + } + + $validClient = $client !== null && $client->getClientIdentifier() !== null; + $cookieCheckSuccessful = $this->request->passesStrictCookieCheck(); + + // no valid clientIdentifier given and no valid API Request (APIRequest header not set) + if ($cookieCheckSuccessful === false && $validClient === false) { + return new TemplateResponse( + $this->appName, + 'error', + ['errors' => + [ + ['error' => 'Access Forbidden', 'hint' => 'Invalid request'] + ] + ] + ); + } + $stateToken = $this->random->generate( 64, ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_DIGITS ); $this->session->set(self::stateName, $stateToken); - $clientName = $this->getClientName(); - if($clientIdentifier !== '') { - $client = $this->clientMapper->getByIdentifier($clientIdentifier); - $clientName = $client->getName(); - } - return new TemplateResponse( $this->appName, 'loginflow/authpicker', From baa8490f44b18ef10c46a117f2143b80f18455bb Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 12 May 2017 13:00:43 +0200 Subject: [PATCH 12/35] add some spacing between the logo and the content of the page Signed-off-by: Bjoern Schiessle --- core/css/login/authpicker.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/css/login/authpicker.css b/core/css/login/authpicker.css index 85016ee6a0e..3603a7906e4 100644 --- a/core/css/login/authpicker.css +++ b/core/css/login/authpicker.css @@ -1,7 +1,7 @@ .picker-window { display: block; padding: 10px; - margin-bottom: 20px; + margin: 20px 0; background-color: rgba(0,0,0,.3); color: #fff; border-radius: 3px; From bb19b3709706b49cdae65c77d28ab89ad3679951 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 12 May 2017 14:55:30 +0200 Subject: [PATCH 13/35] hide client secret by default Signed-off-by: Bjoern Schiessle --- apps/oauth2/css/setting-admin.css | 5 +++++ apps/oauth2/js/setting-admin.js | 16 ++++++++++++++++ apps/oauth2/templates/admin.php | 7 +++++-- 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 apps/oauth2/css/setting-admin.css create mode 100644 apps/oauth2/js/setting-admin.js diff --git a/apps/oauth2/css/setting-admin.css b/apps/oauth2/css/setting-admin.css new file mode 100644 index 00000000000..a57a56bb976 --- /dev/null +++ b/apps/oauth2/css/setting-admin.css @@ -0,0 +1,5 @@ +.show-oauth-credentials { + padding-left: 10px; + opacity: 0.3; + cursor: pointer; +} diff --git a/apps/oauth2/js/setting-admin.js b/apps/oauth2/js/setting-admin.js new file mode 100644 index 00000000000..be774fd720a --- /dev/null +++ b/apps/oauth2/js/setting-admin.js @@ -0,0 +1,16 @@ + +$(document).ready(function () { + + $('.show-oauth-credentials').click(function() { + var row = $(this).parent(); + var code = $(row).find('code'); + if(code.text() === '****') { + code.text(row.data('value')); + $(this).css('opacity', 0.9); + } else { + code.text('****'); + $(this).css('opacity', 0.3); + } + }) + +}); diff --git a/apps/oauth2/templates/admin.php b/apps/oauth2/templates/admin.php index f5b8532e6b1..9c09499add3 100644 --- a/apps/oauth2/templates/admin.php +++ b/apps/oauth2/templates/admin.php @@ -22,6 +22,9 @@ $urlGenerator = \OC::$server->getURLGenerator(); $themingDefaults = \OC::$server->getThemingDefaults(); +script('oauth2', 'setting-admin'); +style('oauth2', 'setting-admin'); + /** @var array $_ */ /** @var \OCA\OAuth2\Db\Client[] $clients */ $clients = $_['clients']; @@ -47,7 +50,7 @@ $clients = $_['clients']; getName()); ?> getRedirectUri()); ?> getClientIdentifier()); ?> - getSecret()); ?> + ****
@@ -67,4 +70,4 @@ $clients = $_['clients'];
- \ No newline at end of file + From 23b296b66eaf674d8eac4b00b044c1dfeda53014 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 12 May 2017 16:41:12 +0200 Subject: [PATCH 14/35] use name of oauth app to identify auth token Signed-off-by: Bjoern Schiessle --- core/Controller/ClientFlowLoginController.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 996ae34b0f2..b41a29dc1c4 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -271,6 +271,14 @@ class ClientFlowLoginController extends Controller { return $response; } + $clientName = $this->getClientName(); + $oAuthClient = false; + if($clientIdentifier !== '') { + $client = $this->clientMapper->getByIdentifier($clientIdentifier); + $clientName = $client->getName(); + $oAuthClient = true; + } + $token = $this->random->generate(72, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS); $uid = $this->userSession->getUser()->getUID(); $generatedToken = $this->tokenProvider->generateToken( @@ -278,12 +286,12 @@ class ClientFlowLoginController extends Controller { $uid, $loginName, $password, - $this->getClientName(), + $clientName, IToken::PERMANENT_TOKEN, IToken::DO_NOT_REMEMBER ); - if($clientIdentifier !== '') { + if($oAuthClient) { $client = $this->clientMapper->getByIdentifier($clientIdentifier); $code = $this->random->generate(128); From 1eb7f4956b1cdc99d0345600047cd6137051790f Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 12 May 2017 16:14:32 +0200 Subject: [PATCH 15/35] delete auth token when client gets deleted Signed-off-by: Bjoern Schiessle --- .../lib/Controller/SettingsController.php | 20 +++++++++++--- apps/oauth2/lib/Db/AccessTokenMapper.php | 14 ++++++++++ apps/oauth2/lib/Db/ClientMapper.php | 22 ++++++++++++++++ apps/oauth2/lib/Settings/Admin.php | 1 - .../Token/DefaultTokenMapper.php | 26 ++++++++++++++----- settings/personal.php | 2 +- .../Token/DefaultTokenMapperTest.php | 12 ++++----- 7 files changed, 79 insertions(+), 18 deletions(-) diff --git a/apps/oauth2/lib/Controller/SettingsController.php b/apps/oauth2/lib/Controller/SettingsController.php index 1d376694f5a..f9ded6c0968 100644 --- a/apps/oauth2/lib/Controller/SettingsController.php +++ b/apps/oauth2/lib/Controller/SettingsController.php @@ -21,6 +21,8 @@ namespace OCA\OAuth2\Controller; +use OC\Authentication\Token\DefaultTokenMapper; +use OCA\OAuth2\Db\AccessTokenMapper; use OCA\OAuth2\Db\Client; use OCA\OAuth2\Db\ClientMapper; use OCP\AppFramework\Controller; @@ -36,6 +38,10 @@ class SettingsController extends Controller { private $clientMapper; /** @var ISecureRandom */ private $secureRandom; + /** @var AccessTokenMapper */ + private $accessTokenMapper; + /** @var DefaultTokenMapper */ + private $defaultTokenMapper; const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; @@ -45,16 +51,23 @@ class SettingsController extends Controller { * @param IURLGenerator $urlGenerator * @param ClientMapper $clientMapper * @param ISecureRandom $secureRandom + * @param AccessTokenMapper $accessTokenMapper + * @param DefaultTokenMapper $defaultTokenMapper */ public function __construct($appName, IRequest $request, IURLGenerator $urlGenerator, ClientMapper $clientMapper, - ISecureRandom $secureRandom) { + ISecureRandom $secureRandom, + AccessTokenMapper $accessTokenMapper, + DefaultTokenMapper $defaultTokenMapper + ) { parent::__construct($appName, $request); $this->urlGenerator = $urlGenerator; $this->secureRandom = $secureRandom; $this->clientMapper = $clientMapper; + $this->accessTokenMapper = $accessTokenMapper; + $this->defaultTokenMapper = $defaultTokenMapper; } /** @@ -78,8 +91,9 @@ class SettingsController extends Controller { * @return RedirectResponse */ public function deleteClient($id) { - $client = new Client(); - $client->setId($id); + $client = $this->clientMapper->getByUid($id); + $this->accessTokenMapper->deleteByClientId($id); + $this->defaultTokenMapper->deleteByName($client->getName()); $this->clientMapper->delete($client); return new RedirectResponse($this->urlGenerator->getAbsoluteURL('/index.php/settings/admin/security')); } diff --git a/apps/oauth2/lib/Db/AccessTokenMapper.php b/apps/oauth2/lib/Db/AccessTokenMapper.php index 2751302522c..51b97bf8d7a 100644 --- a/apps/oauth2/lib/Db/AccessTokenMapper.php +++ b/apps/oauth2/lib/Db/AccessTokenMapper.php @@ -22,6 +22,7 @@ namespace OCA\OAuth2\Db; use OCP\AppFramework\Db\Mapper; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; class AccessTokenMapper extends Mapper { @@ -48,4 +49,17 @@ class AccessTokenMapper extends Mapper { $result->closeCursor(); return AccessToken::fromRow($row); } + + /** + * delete all access token from a given client + * + * @param int $id + */ + public function deleteByClientId($id) { + $qb = $this->db->getQueryBuilder(); + $qb + ->delete($this->tableName) + ->where($qb->expr()->eq('client_id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))); + $qb->execute(); + } } diff --git a/apps/oauth2/lib/Db/ClientMapper.php b/apps/oauth2/lib/Db/ClientMapper.php index 38751a2e5cf..cf00afacb70 100644 --- a/apps/oauth2/lib/Db/ClientMapper.php +++ b/apps/oauth2/lib/Db/ClientMapper.php @@ -22,6 +22,7 @@ namespace OCA\OAuth2\Db; use OCP\AppFramework\Db\Mapper; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; class ClientMapper extends Mapper { @@ -54,6 +55,27 @@ class ClientMapper extends Mapper { return Client::fromRow($row); } + /** + * @param string $uid internal uid of the client + * @return Client + */ + public function getByUid($uid) { + $qb = $this->db->getQueryBuilder(); + $qb + ->select('*') + ->from($this->tableName) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($uid, IQueryBuilder::PARAM_INT))); + $result = $qb->execute(); + $row = $result->fetch(); + $result->closeCursor(); + + if (!is_array($row)) { + $row = []; + } + + return Client::fromRow($row); + } + /** * @return Client[] */ diff --git a/apps/oauth2/lib/Settings/Admin.php b/apps/oauth2/lib/Settings/Admin.php index aa120bcb7d7..07c3fe733ad 100644 --- a/apps/oauth2/lib/Settings/Admin.php +++ b/apps/oauth2/lib/Settings/Admin.php @@ -23,7 +23,6 @@ namespace OCA\OAuth2\Settings; use OCA\OAuth2\Db\ClientMapper; use OCP\AppFramework\Http\TemplateResponse; -use OCP\IConfig; use OCP\Settings\ISettings; class Admin implements ISettings { diff --git a/lib/private/Authentication/Token/DefaultTokenMapper.php b/lib/private/Authentication/Token/DefaultTokenMapper.php index 8848cd3ec56..a69e9e940cd 100644 --- a/lib/private/Authentication/Token/DefaultTokenMapper.php +++ b/lib/private/Authentication/Token/DefaultTokenMapper.php @@ -31,7 +31,7 @@ use OCP\IUser; class DefaultTokenMapper extends Mapper { public function __construct(IDBConnection $db) { - parent::__construct($db, 'authtoken'); + parent::__construct($db, 'AuthToken'); } /** @@ -42,7 +42,7 @@ class DefaultTokenMapper extends Mapper { public function invalidate($token) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $qb->delete('authtoken') + $qb->delete('AuthToken') ->where($qb->expr()->eq('token', $qb->createParameter('token'))) ->setParameter('token', $token) ->execute(); @@ -55,7 +55,7 @@ class DefaultTokenMapper extends Mapper { public function invalidateOld($olderThan, $remember = IToken::DO_NOT_REMEMBER) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $qb->delete('authtoken') + $qb->delete('AuthToken') ->where($qb->expr()->lt('last_activity', $qb->createNamedParameter($olderThan, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('type', $qb->createNamedParameter(IToken::TEMPORARY_TOKEN, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('remember', $qb->createNamedParameter($remember, IQueryBuilder::PARAM_INT))) @@ -73,7 +73,7 @@ class DefaultTokenMapper extends Mapper { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check', 'scope') - ->from('authtoken') + ->from('AuthToken') ->where($qb->expr()->eq('token', $qb->createNamedParameter($token))) ->execute(); @@ -97,7 +97,7 @@ class DefaultTokenMapper extends Mapper { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity', 'last_check', 'scope') - ->from('authtoken') + ->from('AuthToken') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) ->execute(); @@ -122,7 +122,7 @@ class DefaultTokenMapper extends Mapper { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check', 'scope') - ->from('authtoken') + ->from('AuthToken') ->where($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID()))) ->setMaxResults(1000); $result = $qb->execute(); @@ -143,10 +143,22 @@ class DefaultTokenMapper extends Mapper { public function deleteById(IUser $user, $id) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $qb->delete('authtoken') + $qb->delete('AuthToken') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) ->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID()))); $qb->execute(); } + /** + * delete all auth token which belong to a specific client if the client was deleted + * + * @param string $name + */ + public function deleteByName($name) { + $qb = $this->db->getQueryBuilder(); + $qb->delete('AuthToken') + ->where($qb->expr()->eq('name', $qb->createNamedParameter($name))); + $qb->execute(); + } + } diff --git a/settings/personal.php b/settings/personal.php index 86ac4f753f4..7f99dc3259b 100644 --- a/settings/personal.php +++ b/settings/personal.php @@ -49,7 +49,7 @@ $config = \OC::$server->getConfig(); $urlGenerator = \OC::$server->getURLGenerator(); // Highlight navigation entry -OC_Util::addScript('settings', 'authtoken'); +OC_Util::addScript('settings', 'AuthToken'); OC_Util::addScript('settings', 'authtoken_collection'); OC_Util::addScript('settings', 'authtoken_view'); OC_Util::addScript('settings', 'usersettings'); diff --git a/tests/lib/Authentication/Token/DefaultTokenMapperTest.php b/tests/lib/Authentication/Token/DefaultTokenMapperTest.php index 8fe0762daad..650031b6d30 100644 --- a/tests/lib/Authentication/Token/DefaultTokenMapperTest.php +++ b/tests/lib/Authentication/Token/DefaultTokenMapperTest.php @@ -58,8 +58,8 @@ class DefaultTokenMapperTest extends TestCase { private function resetDatabase() { $qb = $this->dbConnection->getQueryBuilder(); - $qb->delete('authtoken')->execute(); - $qb->insert('authtoken')->values([ + $qb->delete('AuthToken')->execute(); + $qb->insert('AuthToken')->values([ 'uid' => $qb->createNamedParameter('user1'), 'login_name' => $qb->createNamedParameter('User1'), 'password' => $qb->createNamedParameter('a75c7116460c082912d8f6860a850904|3nz5qbG1nNSLLi6V|c55365a0e54cfdfac4a175bcf11a7612aea74492277bba6e5d96a24497fa9272488787cb2f3ad34d8b9b8060934fce02f008d371df3ff3848f4aa61944851ff0'), @@ -69,7 +69,7 @@ class DefaultTokenMapperTest extends TestCase { 'last_activity' => $qb->createNamedParameter($this->time - 120, IQueryBuilder::PARAM_INT), // Two minutes ago 'last_check' => $this->time - 60 * 10, // 10mins ago ])->execute(); - $qb->insert('authtoken')->values([ + $qb->insert('AuthToken')->values([ 'uid' => $qb->createNamedParameter('user2'), 'login_name' => $qb->createNamedParameter('User2'), 'password' => $qb->createNamedParameter('971a337057853344700bbeccf836519f|UwOQwyb34sJHtqPV|036d4890f8c21d17bbc7b88072d8ef049a5c832a38e97f3e3d5f9186e896c2593aee16883f617322fa242728d0236ff32d163caeb4bd45e14ca002c57a88665f'), @@ -79,7 +79,7 @@ class DefaultTokenMapperTest extends TestCase { 'last_activity' => $qb->createNamedParameter($this->time - 60 * 60 * 24 * 3, IQueryBuilder::PARAM_INT), // Three days ago 'last_check' => $this->time - 10, // 10secs ago ])->execute(); - $qb->insert('authtoken')->values([ + $qb->insert('AuthToken')->values([ 'uid' => $qb->createNamedParameter('user1'), 'login_name' => $qb->createNamedParameter('User1'), 'password' => $qb->createNamedParameter('063de945d6f6b26862d9b6f40652f2d5|DZ/z520tfdXPtd0T|395f6b89be8d9d605e409e20b9d9abe477fde1be38a3223f9e508f979bf906e50d9eaa4dca983ca4fb22a241eb696c3f98654e7775f78c4caf13108f98642b53'), @@ -94,7 +94,7 @@ class DefaultTokenMapperTest extends TestCase { private function getNumberOfTokens() { $qb = $this->dbConnection->getQueryBuilder(); $result = $qb->select($qb->createFunction('count(*) as `count`')) - ->from('authtoken') + ->from('AuthToken') ->execute() ->fetch(); return (int) $result['count']; @@ -211,7 +211,7 @@ class DefaultTokenMapperTest extends TestCase { $user = $this->createMock(IUser::class); $qb = $this->dbConnection->getQueryBuilder(); $qb->select('id') - ->from('authtoken') + ->from('AuthToken') ->where($qb->expr()->eq('token', $qb->createNamedParameter('9c5a2e661482b65597408a6bb6c4a3d1af36337381872ac56e445a06cdb7fea2b1039db707545c11027a4966919918b19d875a8b774840b18c6cbb7ae56fe206'))); $result = $qb->execute(); $id = $result->fetch()['id']; From 9d91ebf8e022821db8af15cb4bc0463df7146491 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 14:36:30 +0200 Subject: [PATCH 16/35] Add XSD definitions Signed-off-by: Lukas Reschke --- apps/oauth2/appinfo/database.xml | 2 +- apps/oauth2/appinfo/info.xml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/oauth2/appinfo/database.xml b/apps/oauth2/appinfo/database.xml index 48de162299c..de3df4091a6 100644 --- a/apps/oauth2/appinfo/database.xml +++ b/apps/oauth2/appinfo/database.xml @@ -1,5 +1,5 @@ - + *dbname* true false diff --git a/apps/oauth2/appinfo/info.xml b/apps/oauth2/appinfo/info.xml index 8b24d4581fb..8992b87daaa 100644 --- a/apps/oauth2/appinfo/info.xml +++ b/apps/oauth2/appinfo/info.xml @@ -1,9 +1,9 @@ - + oauth2 OAuth 2.0 The OAuth2 app allows administrators to configure the built-in authentication workflow to also allow OAuth2 compatible authentication from other web applications. - AGPL + agpl Lukas Reschke OAuth2 1.0.4 From 88afd8b22466e4dfab8e136f81440b160ee84acb Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 15:16:50 +0200 Subject: [PATCH 17/35] Cleanup code Signed-off-by: Lukas Reschke --- apps/oauth2/lib/Controller/OauthApiController.php | 2 +- apps/oauth2/templates/admin.php | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 97f8185149a..b97d85ae3e6 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -81,7 +81,7 @@ class OauthApiController extends Controller { 'token_type' => 'Bearer', 'expires_in' => 3600, 'refresh_token' => $newCode, - 'user_id' => ($this->defaultTokenMapper->getTokenById($accessToken->getTokenId()))->getUID(), + 'user_id' => $this->defaultTokenMapper->getTokenById($accessToken->getTokenId())->getUID(), ] ); } diff --git a/apps/oauth2/templates/admin.php b/apps/oauth2/templates/admin.php index 9c09499add3..d2e34e08db8 100644 --- a/apps/oauth2/templates/admin.php +++ b/apps/oauth2/templates/admin.php @@ -45,12 +45,15 @@ $clients = $_['clients']; - + imagePath('core', 'actions/toggle.svg'); + foreach ($clients as $client) { + ?> getName()); ?> getRedirectUri()); ?> getClientIdentifier()); ?> - **** + ****
From b07a0f51bacc65cc55982172301599ec12fdc235 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 15:43:14 +0200 Subject: [PATCH 18/35] Add OAuth state to session Signed-off-by: Lukas Reschke --- .../Controller/LoginRedirectorController.php | 17 ++++---- core/Controller/ClientFlowLoginController.php | 42 ++++++++----------- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/apps/oauth2/lib/Controller/LoginRedirectorController.php b/apps/oauth2/lib/Controller/LoginRedirectorController.php index 1a2e00ef5dc..9237b4b1b3c 100644 --- a/apps/oauth2/lib/Controller/LoginRedirectorController.php +++ b/apps/oauth2/lib/Controller/LoginRedirectorController.php @@ -25,6 +25,7 @@ use OCA\OAuth2\Db\ClientMapper; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\RedirectResponse; use OCP\IRequest; +use OCP\ISession; use OCP\IURLGenerator; class LoginRedirectorController extends Controller { @@ -32,45 +33,45 @@ class LoginRedirectorController extends Controller { private $urlGenerator; /** @var ClientMapper */ private $clientMapper; + /** @var ISession */ + private $session; /** * @param string $appName * @param IRequest $request * @param IURLGenerator $urlGenerator * @param ClientMapper $clientMapper + * @param ISession $session */ public function __construct($appName, IRequest $request, IURLGenerator $urlGenerator, - ClientMapper $clientMapper) { + ClientMapper $clientMapper, + ISession $session) { parent::__construct($appName, $request); $this->urlGenerator = $urlGenerator; $this->clientMapper = $clientMapper; + $this->session = $session; } /** * @PublicPage * @NoCSRFRequired + * @UseSession * * @param string $client_id - * @param string $redirect_uri * @param string $state * @return RedirectResponse */ public function authorize($client_id, - $redirect_uri, $state) { $client = $this->clientMapper->getByIdentifier($client_id); - - if($client->getRedirectUri() !== $redirect_uri) { - throw new \Exception('Redirect URI does not match'); - } + $this->session->set('oauth.state', $state); $targetUrl = $this->urlGenerator->linkToRouteAbsolute( 'core.ClientFlowLogin.showAuthPickerPage', [ 'clientIdentifier' => $client->getClientIdentifier(), - 'oauthState' => $state, ] ); return new RedirectResponse($targetUrl); diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index b41a29dc1c4..cafcc16442a 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -149,10 +149,7 @@ class ClientFlowLoginController extends Controller { * * @return TemplateResponse */ - public function showAuthPickerPage($clientIdentifier = '', - $oauthState = '') { - - + public function showAuthPickerPage($clientIdentifier = '') { $clientName = $this->getClientName(); $client = null; if($clientIdentifier !== '') { @@ -160,19 +157,22 @@ class ClientFlowLoginController extends Controller { $clientName = $client->getName(); } - $validClient = $client !== null && $client->getClientIdentifier() !== null; - $cookieCheckSuccessful = $this->request->passesStrictCookieCheck(); - - // no valid clientIdentifier given and no valid API Request (APIRequest header not set) - if ($cookieCheckSuccessful === false && $validClient === false) { + // No valid clientIdentifier given and no valid API Request (APIRequest header not set) + $clientRequest = $this->request->getHeader('OCS-APIREQUEST'); + if ($clientRequest !== 'true' && $client === null) { return new TemplateResponse( $this->appName, 'error', - ['errors' => + [ + 'errors' => [ - ['error' => 'Access Forbidden', 'hint' => 'Invalid request'] - ] - ] + [ + 'error' => 'Access Forbidden', + 'hint' => 'Invalid request', + ], + ], + ], + 'guest' ); } @@ -188,7 +188,6 @@ class ClientFlowLoginController extends Controller { [ 'client' => $clientName, 'clientIdentifier' => $clientIdentifier, - 'oauthState' => $oauthState, 'instanceName' => $this->defaults->getName(), 'urlGenerator' => $this->urlGenerator, 'stateToken' => $stateToken, @@ -205,12 +204,10 @@ class ClientFlowLoginController extends Controller { * * @param string $stateToken * @param string $clientIdentifier - * @param string $oauthState * @return TemplateResponse */ public function redirectPage($stateToken = '', - $clientIdentifier = '', - $oauthState = '') { + $clientIdentifier = '') { if(!$this->isValidToken($stateToken)) { return $this->stateTokenForbiddenResponse(); } @@ -222,7 +219,7 @@ class ClientFlowLoginController extends Controller { 'urlGenerator' => $this->urlGenerator, 'stateToken' => $stateToken, 'clientIdentifier' => $clientIdentifier, - 'oauthState' => $oauthState, + 'oauthState' => $this->session->get('oauth.state'), ], 'empty' ); @@ -234,14 +231,10 @@ class ClientFlowLoginController extends Controller { * * @param string $stateToken * @param string $clientIdentifier - * @param string $state - * @param string $oauthState * @return Http\RedirectResponse|Response */ public function generateAppPassword($stateToken, - $clientIdentifier = '', - $state = '', - $oauthState = '') { + $clientIdentifier = '') { if(!$this->isValidToken($stateToken)) { $this->session->remove(self::stateName); return $this->stateTokenForbiddenResponse(); @@ -305,9 +298,10 @@ class ClientFlowLoginController extends Controller { $redirectUri = sprintf( '%s?state=%s&code=%s', $client->getRedirectUri(), - urlencode($oauthState), + urlencode($this->session->get('oauth.state')), urlencode($code) ); + $this->session->remove('oauth.state'); } else { $redirectUri = 'nc://login/server:' . $this->request->getServerHost() . '&user:' . urlencode($loginName) . '&password:' . urlencode($token); } From a4116220cb77eddbbee8e4fa4469bda1ed177f8a Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 15:53:09 +0200 Subject: [PATCH 19/35] Add app to autoenabled provisioning API scenario Signed-off-by: Lukas Reschke --- build/integration/features/provisioning-v1.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/build/integration/features/provisioning-v1.feature b/build/integration/features/provisioning-v1.feature index 555960b8a55..84e2e30baa1 100644 --- a/build/integration/features/provisioning-v1.feature +++ b/build/integration/features/provisioning-v1.feature @@ -342,6 +342,7 @@ Feature: provisioning | updatenotification | | workflowengine | | files_external | + | oauth2 | Scenario: get app info Given As an "admin" From 26ee889fecc1ad2781d0e2cb5372059685d48d47 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 16:31:14 +0200 Subject: [PATCH 20/35] Add tests for ClientFlowLoginController Signed-off-by: Lukas Reschke --- core/Controller/ClientFlowLoginController.php | 10 +- .../ClientFlowLoginControllerTest.php | 204 ++++++++++++++++-- 2 files changed, 187 insertions(+), 27 deletions(-) diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index cafcc16442a..bec81a89d53 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -109,7 +109,8 @@ class ClientFlowLoginController extends Controller { * @return string */ private function getClientName() { - return $this->request->getHeader('USER_AGENT') !== null ? $this->request->getHeader('USER_AGENT') : 'unknown'; + $userAgent = $this->request->getHeader('USER_AGENT'); + return $userAgent !== null ? $userAgent : 'unknown'; } /** @@ -265,11 +266,10 @@ class ClientFlowLoginController extends Controller { } $clientName = $this->getClientName(); - $oAuthClient = false; + $client = false; if($clientIdentifier !== '') { $client = $this->clientMapper->getByIdentifier($clientIdentifier); $clientName = $client->getName(); - $oAuthClient = true; } $token = $this->random->generate(72, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS); @@ -284,9 +284,7 @@ class ClientFlowLoginController extends Controller { IToken::DO_NOT_REMEMBER ); - if($oAuthClient) { - $client = $this->clientMapper->getByIdentifier($clientIdentifier); - + if($client) { $code = $this->random->generate(128); $accessToken = new AccessToken(); $accessToken->setClientId($client->getId()); diff --git a/tests/Core/Controller/ClientFlowLoginControllerTest.php b/tests/Core/Controller/ClientFlowLoginControllerTest.php index 7a98e5c26c6..1132c0f540c 100644 --- a/tests/Core/Controller/ClientFlowLoginControllerTest.php +++ b/tests/Core/Controller/ClientFlowLoginControllerTest.php @@ -26,6 +26,9 @@ use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; use OC\Core\Controller\ClientFlowLoginController; +use OCA\OAuth2\Db\AccessTokenMapper; +use OCA\OAuth2\Db\Client; +use OCA\OAuth2\Db\ClientMapper; use OCP\AppFramework\Http; use OCP\AppFramework\Http\TemplateResponse; use OCP\Defaults; @@ -35,6 +38,7 @@ use OCP\ISession; use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserSession; +use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; use OCP\Session\Exceptions\SessionNotAvailableException; use Test\TestCase; @@ -56,6 +60,13 @@ class ClientFlowLoginControllerTest extends TestCase { private $random; /** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */ private $urlGenerator; + /** @var ClientMapper|\PHPUnit_Framework_MockObject_MockObject */ + private $clientMapper; + /** @var AccessTokenMapper|\PHPUnit_Framework_MockObject_MockObject */ + private $accessTokenMapper; + /** @var ICrypto|\PHPUnit_Framework_MockObject_MockObject */ + private $crypto; + /** @var ClientFlowLoginController */ private $clientFlowLoginController; @@ -76,6 +87,9 @@ class ClientFlowLoginControllerTest extends TestCase { $this->tokenProvider = $this->createMock(IProvider::class); $this->random = $this->createMock(ISecureRandom::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->clientMapper = $this->createMock(ClientMapper::class); + $this->accessTokenMapper = $this->createMock(AccessTokenMapper::class); + $this->crypto = $this->createMock(ICrypto::class); $this->clientFlowLoginController = new ClientFlowLoginController( 'core', @@ -86,32 +100,43 @@ class ClientFlowLoginControllerTest extends TestCase { $this->session, $this->tokenProvider, $this->random, - $this->urlGenerator + $this->urlGenerator, + $this->clientMapper, + $this->accessTokenMapper, + $this->crypto ); } - public function testShowAuthPickerPageNotAuthenticated() { - $this->userSession - ->expects($this->once()) - ->method('isLoggedIn') - ->willReturn(true); - + public function testShowAuthPickerPageNoClientOrOauthRequest() { $expected = new TemplateResponse( 'core', - '403', + 'error', [ - 'file' => 'Auth flow can only be started unauthenticated.', + 'errors' => + [ + [ + 'error' => 'Access Forbidden', + 'hint' => 'Invalid request', + ], + ], ], 'guest' ); + $this->assertEquals($expected, $this->clientFlowLoginController->showAuthPickerPage()); } - public function testShowAuthPickerPage() { - $this->userSession - ->expects($this->once()) - ->method('isLoggedIn') - ->willReturn(false); + public function testShowAuthPickerPageWithOcsHeader() { + $this->request + ->expects($this->at(0)) + ->method('getHeader') + ->with('USER_AGENT') + ->willReturn('Mac OS X Sync Client'); + $this->request + ->expects($this->at(1)) + ->method('getHeader') + ->with('OCS-APIREQUEST') + ->willReturn('true'); $this->random ->expects($this->once()) ->method('generate') @@ -124,11 +149,6 @@ class ClientFlowLoginControllerTest extends TestCase { ->expects($this->once()) ->method('set') ->with('client.flow.state.token', 'StateToken'); - $this->request - ->expects($this->exactly(2)) - ->method('getHeader') - ->with('USER_AGENT') - ->willReturn('Mac OS X Sync Client'); $this->defaults ->expects($this->once()) ->method('getName') @@ -143,6 +163,7 @@ class ClientFlowLoginControllerTest extends TestCase { 'loginflow/authpicker', [ 'client' => 'Mac OS X Sync Client', + 'clientIdentifier' => '', 'instanceName' => 'ExampleCloud', 'urlGenerator' => $this->urlGenerator, 'stateToken' => 'StateToken', @@ -153,6 +174,56 @@ class ClientFlowLoginControllerTest extends TestCase { $this->assertEquals($expected, $this->clientFlowLoginController->showAuthPickerPage()); } + public function testShowAuthPickerPageWithOauth() { + $this->request + ->expects($this->at(0)) + ->method('getHeader') + ->with('USER_AGENT') + ->willReturn('Mac OS X Sync Client'); + $client = new Client(); + $client->setName('My external service'); + $this->clientMapper + ->expects($this->once()) + ->method('getByIdentifier') + ->with('MyClientIdentifier') + ->willReturn($client); + $this->random + ->expects($this->once()) + ->method('generate') + ->with( + 64, + ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_DIGITS + ) + ->willReturn('StateToken'); + $this->session + ->expects($this->once()) + ->method('set') + ->with('client.flow.state.token', 'StateToken'); + $this->defaults + ->expects($this->once()) + ->method('getName') + ->willReturn('ExampleCloud'); + $this->request + ->expects($this->once()) + ->method('getServerHost') + ->willReturn('example.com'); + + $expected = new TemplateResponse( + 'core', + 'loginflow/authpicker', + [ + 'client' => 'My external service', + 'clientIdentifier' => 'MyClientIdentifier', + 'instanceName' => 'ExampleCloud', + 'urlGenerator' => $this->urlGenerator, + 'stateToken' => 'StateToken', + 'serverHost' => 'example.com', + ], + 'guest' + ); + $this->assertEquals($expected, $this->clientFlowLoginController->showAuthPickerPage('MyClientIdentifier')); + } + public function testRedirectPageWithInvalidToken() { $this->session ->expects($this->once()) @@ -193,10 +264,15 @@ class ClientFlowLoginControllerTest extends TestCase { public function testRedirectPage() { $this->session - ->expects($this->once()) + ->expects($this->at(0)) ->method('get') ->with('client.flow.state.token') ->willReturn('MyStateToken'); + $this->session + ->expects($this->at(1)) + ->method('get') + ->with('oauth.state') + ->willReturn('MyOauthStateToken'); $expected = new TemplateResponse( 'core', @@ -204,10 +280,12 @@ class ClientFlowLoginControllerTest extends TestCase { [ 'urlGenerator' => $this->urlGenerator, 'stateToken' => 'MyStateToken', + 'clientIdentifier' => 'Identifier', + 'oauthState' => 'MyOauthStateToken', ], 'empty' ); - $this->assertEquals($expected, $this->clientFlowLoginController->redirectPage('MyStateToken')); + $this->assertEquals($expected, $this->clientFlowLoginController->redirectPage('MyStateToken', 'Identifier')); } public function testGenerateAppPasswordWithInvalidToken() { @@ -342,6 +420,90 @@ class ClientFlowLoginControllerTest extends TestCase { $this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken')); } + public function testGeneratePasswordWithPasswordForOauthClient() { + $this->session + ->expects($this->at(0)) + ->method('get') + ->with('client.flow.state.token') + ->willReturn('MyStateToken'); + $this->session + ->expects($this->at(1)) + ->method('remove') + ->with('client.flow.state.token'); + $this->session + ->expects($this->at(3)) + ->method('get') + ->with('oauth.state') + ->willReturn('MyOauthState'); + $this->session + ->expects($this->at(4)) + ->method('remove') + ->with('oauth.state'); + $this->session + ->expects($this->once()) + ->method('getId') + ->willReturn('SessionId'); + $myToken = $this->createMock(IToken::class); + $myToken + ->expects($this->once()) + ->method('getLoginName') + ->willReturn('MyLoginName'); + $this->tokenProvider + ->expects($this->once()) + ->method('getToken') + ->with('SessionId') + ->willReturn($myToken); + $this->tokenProvider + ->expects($this->once()) + ->method('getPassword') + ->with($myToken, 'SessionId') + ->willReturn('MyPassword'); + $this->random + ->expects($this->at(0)) + ->method('generate') + ->with(72) + ->willReturn('MyGeneratedToken'); + $this->random + ->expects($this->at(1)) + ->method('generate') + ->with(128) + ->willReturn('MyAccessCode'); + $user = $this->createMock(IUser::class); + $user + ->expects($this->once()) + ->method('getUID') + ->willReturn('MyUid'); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($user); + $token = $this->createMock(IToken::class); + $this->tokenProvider + ->expects($this->once()) + ->method('generateToken') + ->with( + 'MyGeneratedToken', + 'MyUid', + 'MyLoginName', + 'MyPassword', + 'My OAuth client', + IToken::PERMANENT_TOKEN, + IToken::DO_NOT_REMEMBER + ) + ->willReturn($token); + $client = new Client(); + $client->setName('My OAuth client'); + $client->setRedirectUri('https://example.com/redirect.php'); + $this->clientMapper + ->expects($this->once()) + ->method('getByIdentifier') + ->with('MyClientIdentifier') + ->willReturn($client); + + $expected = new Http\RedirectResponse('https://example.com/redirect.php?state=MyOauthState&code=MyAccessCode'); + $this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken', 'MyClientIdentifier')); + } + public function testGeneratePasswordWithoutPassword() { $this->session ->expects($this->once()) From 77827ebf112f7ce1bad5ce5912c7b3900e399ce5 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 16:36:50 +0200 Subject: [PATCH 21/35] Rename table back to lowercase Signed-off-by: Lukas Reschke --- .../Authentication/Token/DefaultTokenMapper.php | 16 ++++++++-------- .../Token/DefaultTokenMapperTest.php | 16 ++++++++++------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/lib/private/Authentication/Token/DefaultTokenMapper.php b/lib/private/Authentication/Token/DefaultTokenMapper.php index a69e9e940cd..44bc553a92e 100644 --- a/lib/private/Authentication/Token/DefaultTokenMapper.php +++ b/lib/private/Authentication/Token/DefaultTokenMapper.php @@ -31,7 +31,7 @@ use OCP\IUser; class DefaultTokenMapper extends Mapper { public function __construct(IDBConnection $db) { - parent::__construct($db, 'AuthToken'); + parent::__construct($db, 'authtoken'); } /** @@ -42,7 +42,7 @@ class DefaultTokenMapper extends Mapper { public function invalidate($token) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $qb->delete('AuthToken') + $qb->delete('authtoken') ->where($qb->expr()->eq('token', $qb->createParameter('token'))) ->setParameter('token', $token) ->execute(); @@ -55,7 +55,7 @@ class DefaultTokenMapper extends Mapper { public function invalidateOld($olderThan, $remember = IToken::DO_NOT_REMEMBER) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $qb->delete('AuthToken') + $qb->delete('authtoken') ->where($qb->expr()->lt('last_activity', $qb->createNamedParameter($olderThan, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('type', $qb->createNamedParameter(IToken::TEMPORARY_TOKEN, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('remember', $qb->createNamedParameter($remember, IQueryBuilder::PARAM_INT))) @@ -73,7 +73,7 @@ class DefaultTokenMapper extends Mapper { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check', 'scope') - ->from('AuthToken') + ->from('authtoken') ->where($qb->expr()->eq('token', $qb->createNamedParameter($token))) ->execute(); @@ -97,7 +97,7 @@ class DefaultTokenMapper extends Mapper { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity', 'last_check', 'scope') - ->from('AuthToken') + ->from('authtoken') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) ->execute(); @@ -122,7 +122,7 @@ class DefaultTokenMapper extends Mapper { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check', 'scope') - ->from('AuthToken') + ->from('authtoken') ->where($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID()))) ->setMaxResults(1000); $result = $qb->execute(); @@ -143,7 +143,7 @@ class DefaultTokenMapper extends Mapper { public function deleteById(IUser $user, $id) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $qb->delete('AuthToken') + $qb->delete('authtoken') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) ->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID()))); $qb->execute(); @@ -156,7 +156,7 @@ class DefaultTokenMapper extends Mapper { */ public function deleteByName($name) { $qb = $this->db->getQueryBuilder(); - $qb->delete('AuthToken') + $qb->delete('authtoken') ->where($qb->expr()->eq('name', $qb->createNamedParameter($name))); $qb->execute(); } diff --git a/tests/lib/Authentication/Token/DefaultTokenMapperTest.php b/tests/lib/Authentication/Token/DefaultTokenMapperTest.php index 650031b6d30..13427f7cb97 100644 --- a/tests/lib/Authentication/Token/DefaultTokenMapperTest.php +++ b/tests/lib/Authentication/Token/DefaultTokenMapperTest.php @@ -58,8 +58,8 @@ class DefaultTokenMapperTest extends TestCase { private function resetDatabase() { $qb = $this->dbConnection->getQueryBuilder(); - $qb->delete('AuthToken')->execute(); - $qb->insert('AuthToken')->values([ + $qb->delete('authtoken')->execute(); + $qb->insert('authtoken')->values([ 'uid' => $qb->createNamedParameter('user1'), 'login_name' => $qb->createNamedParameter('User1'), 'password' => $qb->createNamedParameter('a75c7116460c082912d8f6860a850904|3nz5qbG1nNSLLi6V|c55365a0e54cfdfac4a175bcf11a7612aea74492277bba6e5d96a24497fa9272488787cb2f3ad34d8b9b8060934fce02f008d371df3ff3848f4aa61944851ff0'), @@ -69,7 +69,7 @@ class DefaultTokenMapperTest extends TestCase { 'last_activity' => $qb->createNamedParameter($this->time - 120, IQueryBuilder::PARAM_INT), // Two minutes ago 'last_check' => $this->time - 60 * 10, // 10mins ago ])->execute(); - $qb->insert('AuthToken')->values([ + $qb->insert('authtoken')->values([ 'uid' => $qb->createNamedParameter('user2'), 'login_name' => $qb->createNamedParameter('User2'), 'password' => $qb->createNamedParameter('971a337057853344700bbeccf836519f|UwOQwyb34sJHtqPV|036d4890f8c21d17bbc7b88072d8ef049a5c832a38e97f3e3d5f9186e896c2593aee16883f617322fa242728d0236ff32d163caeb4bd45e14ca002c57a88665f'), @@ -79,7 +79,7 @@ class DefaultTokenMapperTest extends TestCase { 'last_activity' => $qb->createNamedParameter($this->time - 60 * 60 * 24 * 3, IQueryBuilder::PARAM_INT), // Three days ago 'last_check' => $this->time - 10, // 10secs ago ])->execute(); - $qb->insert('AuthToken')->values([ + $qb->insert('authtoken')->values([ 'uid' => $qb->createNamedParameter('user1'), 'login_name' => $qb->createNamedParameter('User1'), 'password' => $qb->createNamedParameter('063de945d6f6b26862d9b6f40652f2d5|DZ/z520tfdXPtd0T|395f6b89be8d9d605e409e20b9d9abe477fde1be38a3223f9e508f979bf906e50d9eaa4dca983ca4fb22a241eb696c3f98654e7775f78c4caf13108f98642b53'), @@ -94,7 +94,7 @@ class DefaultTokenMapperTest extends TestCase { private function getNumberOfTokens() { $qb = $this->dbConnection->getQueryBuilder(); $result = $qb->select($qb->createFunction('count(*) as `count`')) - ->from('AuthToken') + ->from('authtoken') ->execute() ->fetch(); return (int) $result['count']; @@ -190,6 +190,7 @@ class DefaultTokenMapperTest extends TestCase { } public function testGetTokenByUser() { + /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ $user = $this->createMock(IUser::class); $user->expects($this->once()) ->method('getUID') @@ -199,6 +200,7 @@ class DefaultTokenMapperTest extends TestCase { } public function testGetTokenByUserNotFound() { + /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ $user = $this->createMock(IUser::class); $user->expects($this->once()) ->method('getUID') @@ -208,10 +210,11 @@ class DefaultTokenMapperTest extends TestCase { } public function testDeleteById() { + /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ $user = $this->createMock(IUser::class); $qb = $this->dbConnection->getQueryBuilder(); $qb->select('id') - ->from('AuthToken') + ->from('authtoken') ->where($qb->expr()->eq('token', $qb->createNamedParameter('9c5a2e661482b65597408a6bb6c4a3d1af36337381872ac56e445a06cdb7fea2b1039db707545c11027a4966919918b19d875a8b774840b18c6cbb7ae56fe206'))); $result = $qb->execute(); $id = $result->fetch()['id']; @@ -224,6 +227,7 @@ class DefaultTokenMapperTest extends TestCase { } public function testDeleteByIdWrongUser() { + /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ $user = $this->createMock(IUser::class); $id = 33; $user->expects($this->once()) From d90eba3f851711892983aecd643fca431b97397e Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 16:39:28 +0200 Subject: [PATCH 22/35] Fix style issues pointed out in review Signed-off-by: Lukas Reschke --- apps/oauth2/appinfo/routes.php | 2 +- apps/oauth2/js/setting-admin.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/oauth2/appinfo/routes.php b/apps/oauth2/appinfo/routes.php index 808f2a22395..84b1336e37e 100644 --- a/apps/oauth2/appinfo/routes.php +++ b/apps/oauth2/appinfo/routes.php @@ -42,4 +42,4 @@ return [ 'verb' => 'POST' ], ], -]; \ No newline at end of file +]; diff --git a/apps/oauth2/js/setting-admin.js b/apps/oauth2/js/setting-admin.js index be774fd720a..53163be1148 100644 --- a/apps/oauth2/js/setting-admin.js +++ b/apps/oauth2/js/setting-admin.js @@ -1,4 +1,3 @@ - $(document).ready(function () { $('.show-oauth-credentials').click(function() { From 59e968977c64e95fea7a7a96a77a892de5a23d7d Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 16:43:29 +0200 Subject: [PATCH 23/35] Add test for DefaultTokenMapper Signed-off-by: Lukas Reschke --- .../Authentication/Token/DefaultTokenMapperTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/lib/Authentication/Token/DefaultTokenMapperTest.php b/tests/lib/Authentication/Token/DefaultTokenMapperTest.php index 13427f7cb97..b5d24a7ab5e 100644 --- a/tests/lib/Authentication/Token/DefaultTokenMapperTest.php +++ b/tests/lib/Authentication/Token/DefaultTokenMapperTest.php @@ -238,4 +238,15 @@ class DefaultTokenMapperTest extends TestCase { $this->assertEquals(3, $this->getNumberOfTokens()); } + public function testDeleteByName() { + $qb = $this->dbConnection->getQueryBuilder(); + $qb->select('name') + ->from('authtoken') + ->where($qb->expr()->eq('token', $qb->createNamedParameter('9c5a2e661482b65597408a6bb6c4a3d1af36337381872ac56e445a06cdb7fea2b1039db707545c11027a4966919918b19d875a8b774840b18c6cbb7ae56fe206'))); + $result = $qb->execute(); + $name = $result->fetch()['name']; + $this->mapper->deleteByName($name); + $this->assertEquals(2, $this->getNumberOfTokens()); + } + } From 691646bdaedca81dc7100337f48a384ca80b5950 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 19:09:59 +0200 Subject: [PATCH 24/35] Add tests for OAuth2 app Signed-off-by: Lukas Reschke --- apps/oauth2/lib/Db/AccessTokenMapper.php | 5 + apps/oauth2/lib/Db/ClientMapper.php | 15 +- .../LoginRedirectorControllerTest.php | 91 ++++++++++++ .../Controller/OauthApiControllerTest.php | 106 +++++++++++++ .../Controller/SettingsControllerTest.php | 139 ++++++++++++++++++ .../oauth2/tests/Db/AccessTokenMapperTest.php | 69 +++++++++ apps/oauth2/tests/Db/ClientMapperTest.php | 79 ++++++++++ apps/oauth2/tests/Settings/AdminTest.php | 66 +++++++++ tests/phpunit-autotest.xml | 1 + 9 files changed, 563 insertions(+), 8 deletions(-) create mode 100644 apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php create mode 100644 apps/oauth2/tests/Controller/OauthApiControllerTest.php create mode 100644 apps/oauth2/tests/Controller/SettingsControllerTest.php create mode 100644 apps/oauth2/tests/Db/AccessTokenMapperTest.php create mode 100644 apps/oauth2/tests/Db/ClientMapperTest.php create mode 100644 apps/oauth2/tests/Settings/AdminTest.php diff --git a/apps/oauth2/lib/Db/AccessTokenMapper.php b/apps/oauth2/lib/Db/AccessTokenMapper.php index 51b97bf8d7a..2661c853372 100644 --- a/apps/oauth2/lib/Db/AccessTokenMapper.php +++ b/apps/oauth2/lib/Db/AccessTokenMapper.php @@ -21,6 +21,7 @@ namespace OCA\OAuth2\Db; +use OCA\OAuth2\Exceptions\AccessTokenNotFoundException; use OCP\AppFramework\Db\Mapper; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; @@ -37,6 +38,7 @@ class AccessTokenMapper extends Mapper { /** * @param string $code * @return AccessToken + * @throws AccessTokenNotFoundException */ public function getByCode($code) { $qb = $this->db->getQueryBuilder(); @@ -47,6 +49,9 @@ class AccessTokenMapper extends Mapper { $result = $qb->execute(); $row = $result->fetch(); $result->closeCursor(); + if($row === false) { + throw new AccessTokenNotFoundException(); + } return AccessToken::fromRow($row); } diff --git a/apps/oauth2/lib/Db/ClientMapper.php b/apps/oauth2/lib/Db/ClientMapper.php index cf00afacb70..9df07e2789f 100644 --- a/apps/oauth2/lib/Db/ClientMapper.php +++ b/apps/oauth2/lib/Db/ClientMapper.php @@ -21,6 +21,7 @@ namespace OCA\OAuth2\Db; +use OCA\OAuth2\Exceptions\ClientNotFoundException; use OCP\AppFramework\Db\Mapper; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; @@ -37,6 +38,7 @@ class ClientMapper extends Mapper { /** * @param string $clientIdentifier * @return Client + * @throws ClientNotFoundException */ public function getByIdentifier($clientIdentifier) { $qb = $this->db->getQueryBuilder(); @@ -47,17 +49,16 @@ class ClientMapper extends Mapper { $result = $qb->execute(); $row = $result->fetch(); $result->closeCursor(); - - if (!is_array($row)) { - $row = []; + if($row === false) { + throw new ClientNotFoundException(); } - return Client::fromRow($row); } /** * @param string $uid internal uid of the client * @return Client + * @throws ClientNotFoundException */ public function getByUid($uid) { $qb = $this->db->getQueryBuilder(); @@ -68,11 +69,9 @@ class ClientMapper extends Mapper { $result = $qb->execute(); $row = $result->fetch(); $result->closeCursor(); - - if (!is_array($row)) { - $row = []; + if($row === false) { + throw new ClientNotFoundException(); } - return Client::fromRow($row); } diff --git a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php new file mode 100644 index 00000000000..b33d3379be4 --- /dev/null +++ b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php @@ -0,0 +1,91 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Tests\Controller; + +use OCA\Files_Sharing\Tests\TestCase; +use OCA\OAuth2\Controller\LoginRedirectorController; +use OCA\OAuth2\Db\Client; +use OCA\OAuth2\Db\ClientMapper; +use OCP\AppFramework\Http\RedirectResponse; +use OCP\IRequest; +use OCP\ISession; +use OCP\IURLGenerator; + +/** + * @group DB + */ +class LoginRedirectorControllerTest extends TestCase { + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ + private $request; + /** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */ + private $urlGenerator; + /** @var ClientMapper|\PHPUnit_Framework_MockObject_MockObject */ + private $clientMapper; + /** @var ISession|\PHPUnit_Framework_MockObject_MockObject */ + private $session; + /** @var LoginRedirectorController */ + private $loginRedirectorController; + + public function setUp() { + parent::setUp(); + + $this->request = $this->createMock(IRequest::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->clientMapper = $this->createMock(ClientMapper::class); + $this->session = $this->createMock(ISession::class); + + $this->loginRedirectorController = new LoginRedirectorController( + 'oauth2', + $this->request, + $this->urlGenerator, + $this->clientMapper, + $this->session + ); + } + + public function testAuthorize() { + $client = new Client(); + $client->setClientIdentifier('MyClientIdentifier'); + $this->clientMapper + ->expects($this->once()) + ->method('getByIdentifier') + ->with('MyClientId') + ->willReturn($client); + $this->session + ->expects($this->once()) + ->method('set') + ->with('oauth.state', 'MyState'); + $this->urlGenerator + ->expects($this->once()) + ->method('linkToRouteAbsolute') + ->with( + 'core.ClientFlowLogin.showAuthPickerPage', + [ + 'clientIdentifier' => 'MyClientIdentifier', + ] + ) + ->willReturn('https://example.com/?clientIdentifier=foo'); + + $expected = new RedirectResponse('https://example.com/?clientIdentifier=foo'); + $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState')); + } +} diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php new file mode 100644 index 00000000000..c90e2bf711f --- /dev/null +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -0,0 +1,106 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Tests\Controller; + +use OC\Authentication\Token\DefaultToken; +use OC\Authentication\Token\DefaultTokenMapper; +use OCA\OAuth2\Controller\OauthApiController; +use OCA\OAuth2\Db\AccessToken; +use OCA\OAuth2\Db\AccessTokenMapper; +use OCP\AppFramework\Http\JSONResponse; +use OCP\IRequest; +use OCP\Security\ICrypto; +use OCP\Security\ISecureRandom; +use Test\TestCase; + +class OauthApiControllerTest extends TestCase { + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ + private $request; + /** @var ICrypto|\PHPUnit_Framework_MockObject_MockObject */ + private $crypto; + /** @var AccessTokenMapper|\PHPUnit_Framework_MockObject_MockObject */ + private $accessTokenMapper; + /** @var DefaultTokenMapper|\PHPUnit_Framework_MockObject_MockObject */ + private $defaultTokenMapper; + /** @var ISecureRandom|\PHPUnit_Framework_MockObject_MockObject */ + private $secureRandom; + /** @var OauthApiController */ + private $oauthApiController; + + public function setUp() { + parent::setUp(); + + $this->request = $this->createMock(IRequest::class); + $this->crypto = $this->createMock(ICrypto::class); + $this->accessTokenMapper = $this->createMock(AccessTokenMapper::class); + $this->defaultTokenMapper = $this->createMock(DefaultTokenMapper::class); + $this->secureRandom = $this->createMock(ISecureRandom::class); + + $this->oauthApiController = new OauthApiController( + 'oauth2', + $this->request, + $this->crypto, + $this->accessTokenMapper, + $this->defaultTokenMapper, + $this->secureRandom + ); + } + + public function testGetToken() { + $accessToken = new AccessToken(); + $accessToken->setEncryptedToken('MyEncryptedToken'); + $accessToken->setTokenId(123); + $this->accessTokenMapper + ->expects($this->once()) + ->method('getByCode') + ->willReturn($accessToken); + $this->crypto + ->expects($this->once()) + ->method('decrypt') + ->with('MyEncryptedToken', 'MySecretCode') + ->willReturn('MyDecryptedToken'); + $this->secureRandom + ->expects($this->once()) + ->method('generate') + ->with(128) + ->willReturn('NewToken'); + $token = new DefaultToken(); + $token->setUid('JohnDoe'); + $this->defaultTokenMapper + ->expects($this->once()) + ->method('getTokenById') + ->with(123) + ->willReturn($token); + + $expected = new JSONResponse( + [ + 'access_token' => 'MyDecryptedToken', + 'token_type' => 'Bearer', + 'expires_in' => 3600, + 'refresh_token' => 'NewToken', + 'user_id' => 'JohnDoe', + ] + ); + $this->assertEquals($expected, $this->oauthApiController->getToken('MySecretCode')); + } + +} diff --git a/apps/oauth2/tests/Controller/SettingsControllerTest.php b/apps/oauth2/tests/Controller/SettingsControllerTest.php new file mode 100644 index 00000000000..a6c036949ed --- /dev/null +++ b/apps/oauth2/tests/Controller/SettingsControllerTest.php @@ -0,0 +1,139 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Tests\Controller; + +use OC\Authentication\Token\DefaultTokenMapper; +use OCA\OAuth2\Controller\SettingsController; +use OCA\OAuth2\Db\AccessTokenMapper; +use OCA\OAuth2\Db\Client; +use OCA\OAuth2\Db\ClientMapper; +use OCP\AppFramework\Http\RedirectResponse; +use OCP\IRequest; +use OCP\IURLGenerator; +use OCP\Security\ISecureRandom; +use Test\TestCase; + +class SettingsControllerTest extends TestCase { + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ + private $request; + /** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */ + private $urlGenerator; + /** @var ClientMapper|\PHPUnit_Framework_MockObject_MockObject */ + private $clientMapper; + /** @var ISecureRandom|\PHPUnit_Framework_MockObject_MockObject */ + private $secureRandom; + /** @var AccessTokenMapper|\PHPUnit_Framework_MockObject_MockObject */ + private $accessTokenMapper; + /** @var DefaultTokenMapper|\PHPUnit_Framework_MockObject_MockObject */ + private $defaultTokenMapper; + /** @var SettingsController */ + private $settingsController; + + public function setUp() { + parent::setUp(); + + $this->request = $this->createMock(IRequest::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->clientMapper = $this->createMock(ClientMapper::class); + $this->secureRandom = $this->createMock(ISecureRandom::class); + $this->accessTokenMapper = $this->createMock(AccessTokenMapper::class); + $this->defaultTokenMapper = $this->createMock(DefaultTokenMapper::class); + + $this->settingsController = new SettingsController( + 'oauth2', + $this->request, + $this->urlGenerator, + $this->clientMapper, + $this->secureRandom, + $this->accessTokenMapper, + $this->defaultTokenMapper + ); + } + + public function testAddClient() { + $this->secureRandom + ->expects($this->at(0)) + ->method('generate') + ->with(64, 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789') + ->willReturn('MySecret'); + $this->secureRandom + ->expects($this->at(1)) + ->method('generate') + ->with(64, 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789') + ->willReturn('MyClientIdentifier'); + + $client = new Client(); + $client->setName('My Client Name'); + $client->setRedirectUri('https://example.com/'); + $client->setSecret('MySecret'); + $client->setClientIdentifier('MyClientIdentifier'); + + $this->clientMapper + ->expects($this->once()) + ->method('insert') + ->with($client); + + $this->urlGenerator + ->expects($this->once()) + ->method('getAbsoluteURL') + ->with('/index.php/settings/admin/security') + ->willReturn('https://example.com/index.php/settings/admin/security'); + + $expected = new RedirectResponse('https://example.com/index.php/settings/admin/security'); + $this->assertEquals($expected, $this->settingsController->addClient('My Client Name', 'https://example.com/')); + } + + public function testDeleteClient() { + $client = new Client(); + $client->setName('My Client Name'); + $client->setRedirectUri('https://example.com/'); + $client->setSecret('MySecret'); + $client->setClientIdentifier('MyClientIdentifier'); + + $this->clientMapper + ->expects($this->at(0)) + ->method('getByUid') + ->with(123) + ->willReturn($client); + $this->accessTokenMapper + ->expects($this->once()) + ->method('deleteByClientId') + ->with(123); + $this->defaultTokenMapper + ->expects($this->once()) + ->method('deleteByName') + ->with('My Client Name'); + $this->clientMapper + ->expects($this->at(1)) + ->method('delete') + ->with($client); + + $this->urlGenerator + ->expects($this->once()) + ->method('getAbsoluteURL') + ->with('/index.php/settings/admin/security') + ->willReturn('https://example.com/index.php/settings/admin/security'); + + $expected = new RedirectResponse('https://example.com/index.php/settings/admin/security'); + $this->assertEquals($expected, $this->settingsController->deleteClient(123)); + } +} diff --git a/apps/oauth2/tests/Db/AccessTokenMapperTest.php b/apps/oauth2/tests/Db/AccessTokenMapperTest.php new file mode 100644 index 00000000000..58b36b1c240 --- /dev/null +++ b/apps/oauth2/tests/Db/AccessTokenMapperTest.php @@ -0,0 +1,69 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Tests\Db; + +use OCA\OAuth2\Db\AccessToken; +use OCA\OAuth2\Db\AccessTokenMapper; +use Test\TestCase; + +/** + * @group DB + */ +class AccessTokenMapperTest extends TestCase { + /** @var AccessTokenMapper */ + private $accessTokenMapper; + + public function setUp() { + parent::setUp(); + $this->accessTokenMapper = new AccessTokenMapper(\OC::$server->getDatabaseConnection()); + } + + public function testGetByCode() { + $this->accessTokenMapper->deleteByClientId(1234); + $token = new AccessToken(); + $token->setClientId(1234); + $token->setTokenId((string)time()); + $token->setEncryptedToken('MyEncryptedToken'); + $token->setHashedCode(hash('sha512', 'MyAwesomeToken')); + $this->accessTokenMapper->insert($token); + $token->resetUpdatedFields(); + + $result = $this->accessTokenMapper->getByCode('MyAwesomeToken'); + $this->assertEquals($token, $result); + } + + /** + * @expectedException \OCA\OAuth2\Exceptions\AccessTokenNotFoundException + */ + public function testDeleteByClientId() { + $this->accessTokenMapper->deleteByClientId('TestId'); + $token = new AccessToken(); + $token->setClientId(1234); + $token->setTokenId((string)time()); + $token->setEncryptedToken('MyEncryptedToken'); + $token->setHashedCode(hash('sha512', 'MyAwesomeToken')); + $this->accessTokenMapper->insert($token); + $token->resetUpdatedFields(); + $this->accessTokenMapper->deleteByClientId(1234); + $this->accessTokenMapper->getByCode('MyAwesomeToken'); + } +} diff --git a/apps/oauth2/tests/Db/ClientMapperTest.php b/apps/oauth2/tests/Db/ClientMapperTest.php new file mode 100644 index 00000000000..80d69c3b1b8 --- /dev/null +++ b/apps/oauth2/tests/Db/ClientMapperTest.php @@ -0,0 +1,79 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Tests\Db; + +use OCA\OAuth2\Db\Client; +use OCA\OAuth2\Db\ClientMapper; +use Test\TestCase; + +/** + * @group DB + */ +class ClientMapperTest extends TestCase { + /** @var ClientMapper */ + private $clientMapper; + + public function setUp() { + parent::setUp(); + $this->clientMapper = new ClientMapper(\OC::$server->getDatabaseConnection()); + } + + public function testGetByIdentifier() { + $client = new Client(); + $client->setClientIdentifier('MyAwesomeClientIdentifier'); + $client->setName('Client Name'); + $client->setRedirectUri('https://example.com/'); + $client->setSecret('TotallyNotSecret'); + $this->clientMapper->insert($client); + $client->resetUpdatedFields(); + $this->assertEquals($client, $this->clientMapper->getByIdentifier('MyAwesomeClientIdentifier')); + } + + /** + * @expectedException \OCA\OAuth2\Exceptions\ClientNotFoundException + */ + public function testGetByIdentifierNotExisting() { + $this->clientMapper->getByIdentifier('MyTotallyNotExistingClient'); + } + + public function testGetByUid() { + $client = new Client(); + $client->setClientIdentifier('MyNewClient'); + $client->setName('Client Name'); + $client->setRedirectUri('https://example.com/'); + $client->setSecret('TotallyNotSecret'); + $this->clientMapper->insert($client); + $client->resetUpdatedFields(); + $this->assertEquals($client, $this->clientMapper->getByUid($client->getId())); + } + + /** + * @expectedException \OCA\OAuth2\Exceptions\ClientNotFoundException + */ + public function testGetByUidNotExisting() { + $this->clientMapper->getByUid(1234); + } + + public function testGetClients() { + $this->assertSame('array', gettype($this->clientMapper->getClients())); + } +} diff --git a/apps/oauth2/tests/Settings/AdminTest.php b/apps/oauth2/tests/Settings/AdminTest.php new file mode 100644 index 00000000000..9c3d5ed1449 --- /dev/null +++ b/apps/oauth2/tests/Settings/AdminTest.php @@ -0,0 +1,66 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Tests\Settings; + +use OCA\OAuth2\Db\ClientMapper; +use OCA\OAuth2\Settings\Admin; +use OCP\AppFramework\Http\TemplateResponse; +use Test\TestCase; + +class AdminTest extends TestCase { + /** @var ClientMapper|\PHPUnit_Framework_MockObject_MockObject */ + private $clientMapper; + /** @var Admin|\PHPUnit_Framework_MockObject_MockObject */ + private $admin; + + public function setUp() { + parent::setUp(); + + $this->clientMapper = $this->createMock(ClientMapper::class); + $this->admin = new Admin($this->clientMapper); + } + + public function testGetForm() { + $this->clientMapper + ->expects($this->once()) + ->method('getClients') + ->willReturn(['MyClients']); + + $expected = new TemplateResponse( + 'oauth2', + 'admin', + [ + 'clients' => ['MyClients'], + ], + '' + ); + $this->assertEquals($expected, $this->admin->getForm()); + } + + public function testGetSection() { + $this->assertSame('security', $this->admin->getSection()); + } + + public function testGetPriority() { + $this->assertSame(0, $this->admin->getPriority()); + } +} diff --git a/tests/phpunit-autotest.xml b/tests/phpunit-autotest.xml index 9a9c9c957e3..34166a09e2e 100644 --- a/tests/phpunit-autotest.xml +++ b/tests/phpunit-autotest.xml @@ -30,6 +30,7 @@ ../apps/files_sharing/tests ../apps/files_trashbin/tests ../apps/files_versions/tests + ../apps/oauth2/tests ../apps/provisioning_api/tests ../apps/systemtags/tests ../apps/theming/tests From 30552090bccc0d77446d57f5b94ae27ad2d81186 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 20:09:03 +0200 Subject: [PATCH 25/35] Don't ignore OAuth2 app Signed-off-by: Lukas Reschke --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 6a8e6723376..9d9f09c2da9 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,7 @@ !/apps/files_versions !/apps/lookup_server_connector !/apps/user_ldap +!/apps/oauth2 !/apps/provisioning_api !/apps/systemtags !/apps/testing From df3909a7c360f079d3e47401414a38dc7f3494cf Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 20:09:51 +0200 Subject: [PATCH 26/35] Use Bearer backend for SabreDAV Signed-off-by: Lukas Reschke --- apps/dav/lib/Connector/Sabre/Auth.php | 12 --- apps/dav/lib/Connector/Sabre/BearerAuth.php | 76 ++++++++++++++++ apps/dav/lib/Server.php | 8 ++ .../unit/Connector/Sabre/BearerAuthTest.php | 88 +++++++++++++++++++ .../AccessTokenNotFoundException.php | 24 +++++ .../Exceptions/ClientNotFoundException.php | 24 +++++ 6 files changed, 220 insertions(+), 12 deletions(-) create mode 100644 apps/dav/lib/Connector/Sabre/BearerAuth.php create mode 100644 apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php create mode 100644 apps/oauth2/lib/Exceptions/AccessTokenNotFoundException.php create mode 100644 apps/oauth2/lib/Exceptions/ClientNotFoundException.php diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php index 7ddbb70530a..9147e79594c 100644 --- a/apps/dav/lib/Connector/Sabre/Auth.php +++ b/apps/dav/lib/Connector/Sabre/Auth.php @@ -211,18 +211,6 @@ class Auth extends AbstractBasic { private function auth(RequestInterface $request, ResponseInterface $response) { $forcedLogout = false; - $authHeader = $request->getHeader('Authorization'); - if (strpos($authHeader, 'Bearer ') !== false) { - if($this->userSession->tryTokenLogin($this->request)) { - $this->session->set(self::DAV_AUTHENTICATED, $this->userSession->getUser()->getUID()); - $user = $this->userSession->getUser()->getUID(); - \OC_Util::setupFS($user); - $this->currentUser = $user; - $this->session->close(); - return [true, $this->principalPrefix . $user]; - } - } - if(!$this->request->passesCSRFCheck() && $this->requiresCSRFCheck()) { // In case of a fail with POST we need to recheck the credentials diff --git a/apps/dav/lib/Connector/Sabre/BearerAuth.php b/apps/dav/lib/Connector/Sabre/BearerAuth.php new file mode 100644 index 00000000000..dd0a1bac292 --- /dev/null +++ b/apps/dav/lib/Connector/Sabre/BearerAuth.php @@ -0,0 +1,76 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\DAV\Connector\Sabre; + +use OCP\IRequest; +use OCP\ISession; +use OCP\IUserSession; +use Sabre\DAV\Auth\Backend\AbstractBearer; + +class BearerAuth extends AbstractBearer { + /** @var IUserSession */ + private $userSession; + /** @var ISession */ + private $session; + /** @var IRequest */ + private $request; + /** @var string */ + private $principalPrefix; + + /** + * @param IUserSession $userSession + * @param ISession $session + * @param string $principalPrefix + * @param IRequest $request + */ + public function __construct(IUserSession $userSession, + ISession $session, + IRequest $request, + $principalPrefix = 'principals/users/') { + $this->userSession = $userSession; + $this->session = $session; + $this->request = $request; + $this->principalPrefix = $principalPrefix; + } + + private function setupUserFs($userId) { + \OC_Util::setupFS($userId); + $this->session->close(); + return $this->principalPrefix . $userId; + } + + /** + * {@inheritdoc} + */ + public function validateBearerToken($bearerToken) { + \OC_Util::setupFS(); + + if(!$this->userSession->isLoggedIn()) { + $this->userSession->tryTokenLogin($this->request); + } + if($this->userSession->isLoggedIn()) { + return $this->setupUserFs($this->userSession->getUser()->getUID()); + } + + return false; + } +} diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index df5b0ea05b6..994ac04033a 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -33,6 +33,7 @@ use OCA\DAV\CardDAV\ImageExportPlugin; use OCA\DAV\CardDAV\PhotoCache; use OCA\DAV\Comments\CommentsPlugin; use OCA\DAV\Connector\Sabre\Auth; +use OCA\DAV\Connector\Sabre\BearerAuth; use OCA\DAV\Connector\Sabre\BlockLegacyClientPlugin; use OCA\DAV\Connector\Sabre\CommentPropertiesPlugin; use OCA\DAV\Connector\Sabre\CopyEtagHeaderPlugin; @@ -52,6 +53,7 @@ use OCP\SabrePluginEvent; use Sabre\CardDAV\VCFExportPlugin; use Sabre\DAV\Auth\Plugin; use OCA\DAV\Connector\Sabre\TagsPlugin; +use Sabre\HTTP\Auth\Bearer; use SearchDAV\DAV\SearchPlugin; class Server { @@ -100,6 +102,12 @@ class Server { $event = new SabrePluginEvent($this->server); $dispatcher->dispatch('OCA\DAV\Connector\Sabre::authInit', $event); + $bearerAuthBackend = new BearerAuth( + \OC::$server->getUserSession(), + \OC::$server->getSession(), + \OC::$server->getRequest() + ); + $authPlugin->addBackend($bearerAuthBackend); // because we are throwing exceptions this plugin has to be the last one $authPlugin->addBackend($authBackend); diff --git a/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php b/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php new file mode 100644 index 00000000000..5eae75eb8e9 --- /dev/null +++ b/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php @@ -0,0 +1,88 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\DAV\Tests\unit\Connector\Sabre; + +use OC\Authentication\TwoFactorAuth\Manager; +use OC\Security\Bruteforce\Throttler; +use OC\User\Session; +use OCA\DAV\Connector\Sabre\BearerAuth; +use OCP\IRequest; +use OCP\ISession; +use OCP\IUser; +use OCP\IUserSession; +use Sabre\HTTP\RequestInterface; +use Sabre\HTTP\ResponseInterface; +use Test\TestCase; + +/** + * @group DB + */ +class BearerAuthTest extends TestCase { + /** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */ + private $userSession; + /** @var ISession|\PHPUnit_Framework_MockObject_MockObject */ + private $session; + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ + private $request; + /** @var BearerAuth */ + private $bearerAuth; + + public function setUp() { + parent::setUp(); + + $this->userSession = $this->createMock(\OC\User\Session::class); + $this->session = $this->createMock(ISession::class); + $this->request = $this->createMock(IRequest::class); + + $this->bearerAuth = new BearerAuth( + $this->userSession, + $this->session, + $this->request + ); + } + + public function testValidateBearerTokenNotLoggedIn() { + $this->assertFalse($this->bearerAuth->validateBearerToken('Token')); + } + + public function testValidateBearerToken() { + $this->userSession + ->expects($this->at(0)) + ->method('isLoggedIn') + ->willReturn(false); + $this->userSession + ->expects($this->at(2)) + ->method('isLoggedIn') + ->willReturn(true); + $user = $this->createMock(IUser::class); + $user + ->expects($this->once()) + ->method('getUID') + ->willReturn('admin'); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($user); + + $this->assertSame('principals/users/admin', $this->bearerAuth->validateBearerToken('Token')); + } +} diff --git a/apps/oauth2/lib/Exceptions/AccessTokenNotFoundException.php b/apps/oauth2/lib/Exceptions/AccessTokenNotFoundException.php new file mode 100644 index 00000000000..a1eb632a9eb --- /dev/null +++ b/apps/oauth2/lib/Exceptions/AccessTokenNotFoundException.php @@ -0,0 +1,24 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Exceptions; + +class AccessTokenNotFoundException extends \Exception {} diff --git a/apps/oauth2/lib/Exceptions/ClientNotFoundException.php b/apps/oauth2/lib/Exceptions/ClientNotFoundException.php new file mode 100644 index 00000000000..b2395c7bc9e --- /dev/null +++ b/apps/oauth2/lib/Exceptions/ClientNotFoundException.php @@ -0,0 +1,24 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\OAuth2\Exceptions; + +class ClientNotFoundException extends \Exception {} From f2a01e1b0881e6a198a75d3e9b08c229d39f1e17 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 20:32:38 +0200 Subject: [PATCH 27/35] Use a standardized Bearer now Signed-off-by: Lukas Reschke --- build/integration/features/bootstrap/Auth.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/integration/features/bootstrap/Auth.php b/build/integration/features/bootstrap/Auth.php index 7addcab5f97..ae411cc7ab3 100644 --- a/build/integration/features/bootstrap/Auth.php +++ b/build/integration/features/bootstrap/Auth.php @@ -187,7 +187,7 @@ trait Auth { * @param string $method */ public function requestingWithUsingAnUnrestrictedClientToken($url, $method) { - $this->sendRequest($url, $method, 'token ' . $this->unrestrictedClientToken); + $this->sendRequest($url, $method, 'Bearer ' . $this->unrestrictedClientToken); } /** @@ -197,7 +197,7 @@ trait Auth { * @param string $method */ public function requestingWithUsingARestrictedClientToken($url, $method) { - $this->sendRequest($url, $method, 'token ' . $this->restrictedClientToken); + $this->sendRequest($url, $method, 'Bearer ' . $this->restrictedClientToken); } /** From 538112181fb2264bddb8898e2e52587b90d885f5 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 20:34:48 +0200 Subject: [PATCH 28/35] Add additional test for accessing DAV using Bearer Auth Signed-off-by: Lukas Reschke --- build/integration/features/auth.feature | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build/integration/features/auth.feature b/build/integration/features/auth.feature index b9f423a9e93..edcca4bcd4e 100644 --- a/build/integration/features/auth.feature +++ b/build/integration/features/auth.feature @@ -53,6 +53,10 @@ Feature: auth When requesting "/remote.php/webdav" with "PROPFIND" using restricted basic token auth Then the HTTP status code should be "207" + Scenario: using WebDAV with restricted basic token auth + When requesting "/remote.php/webdav" with "PROPFIND" using an unrestricted client token + Then the HTTP status code should be "207" + Scenario: using WebDAV with browser session Given a new browser session is started When requesting "/remote.php/webdav" with "PROPFIND" using browser session From 7927aed991a73a7147f9d388af97725123456179 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 20:36:02 +0200 Subject: [PATCH 29/35] Adjust token name Signed-off-by: Lukas Reschke --- tests/lib/User/SessionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 1bcc6ce3a4d..fcff4f64726 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -956,7 +956,7 @@ class SessionTest extends \Test\TestCase { $request->expects($this->once()) ->method('getHeader') ->with('Authorization') - ->will($this->returnValue('token xxxxx')); + ->will($this->returnValue('Bearer xxxxx')); $this->tokenProvider->expects($this->once()) ->method('getToken') ->with('xxxxx') From fa6ec47a5c064f85e52d06a1c88f5ac13a3b8074 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 20:47:41 +0200 Subject: [PATCH 30/35] Add indexes Signed-off-by: Lukas Reschke --- apps/oauth2/appinfo/database.xml | 21 +++++++++++++++++++++ apps/oauth2/appinfo/info.xml | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/apps/oauth2/appinfo/database.xml b/apps/oauth2/appinfo/database.xml index de3df4091a6..db32e0cf97d 100644 --- a/apps/oauth2/appinfo/database.xml +++ b/apps/oauth2/appinfo/database.xml @@ -39,6 +39,13 @@ true 64 + + oauth2_client_id_idx + false + + client_identifier + + @@ -74,6 +81,20 @@ true786 + + oauth2_access_hash_idx + true + + hashed_code + + + + oauth2_access_client_id_idx + false + + client_id + +
diff --git a/apps/oauth2/appinfo/info.xml b/apps/oauth2/appinfo/info.xml index 8992b87daaa..5e9e8dae06a 100644 --- a/apps/oauth2/appinfo/info.xml +++ b/apps/oauth2/appinfo/info.xml @@ -6,7 +6,7 @@ agpl Lukas Reschke OAuth2 - 1.0.4 + 1.0.5 From ba7b6bd97336646942649a4411c58d94b5753f2f Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 20:54:42 +0200 Subject: [PATCH 31/35] Delete token after usage in test Signed-off-by: Lukas Reschke --- apps/oauth2/tests/Db/AccessTokenMapperTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/oauth2/tests/Db/AccessTokenMapperTest.php b/apps/oauth2/tests/Db/AccessTokenMapperTest.php index 58b36b1c240..7a7c5fd7b39 100644 --- a/apps/oauth2/tests/Db/AccessTokenMapperTest.php +++ b/apps/oauth2/tests/Db/AccessTokenMapperTest.php @@ -49,6 +49,7 @@ class AccessTokenMapperTest extends TestCase { $result = $this->accessTokenMapper->getByCode('MyAwesomeToken'); $this->assertEquals($token, $result); + $this->accessTokenMapper->delete($token); } /** From f93db724d7905d9858af2d2d4cf083c20b9c28de Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 21:19:39 +0200 Subject: [PATCH 32/35] Make legacy DAV backend use the BearerAuth backend as well Signed-off-by: Lukas Reschke --- apps/dav/appinfo/v1/publicwebdav.php | 3 ++- apps/dav/appinfo/v1/webdav.php | 10 +++++++++- apps/dav/lib/Connector/Sabre/ServerFactory.php | 7 ++++--- .../Connector/Sabre/RequestTest/RequestTestCase.php | 3 ++- build/integration/features/auth.feature | 6 +++++- 5 files changed, 22 insertions(+), 7 deletions(-) diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php index 95fb71032d5..3ef1c2e62a5 100644 --- a/apps/dav/appinfo/v1/publicwebdav.php +++ b/apps/dav/appinfo/v1/publicwebdav.php @@ -42,6 +42,7 @@ $authBackend = new OCA\DAV\Connector\PublicAuth( \OC::$server->getShareManager(), \OC::$server->getSession() ); +$authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); $serverFactory = new OCA\DAV\Connector\Sabre\ServerFactory( \OC::$server->getConfig(), @@ -59,7 +60,7 @@ $requestUri = \OC::$server->getRequest()->getRequestUri(); $linkCheckPlugin = new \OCA\DAV\Files\Sharing\PublicLinkCheckPlugin(); $filesDropPlugin = new \OCA\DAV\Files\Sharing\FilesDropPlugin(); -$server = $serverFactory->createServer($baseuri, $requestUri, $authBackend, function (\Sabre\DAV\Server $server) use ($authBackend, $linkCheckPlugin, $filesDropPlugin) { +$server = $serverFactory->createServer($baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($authBackend, $linkCheckPlugin, $filesDropPlugin) { $isAjax = (isset($_SERVER['HTTP_X_REQUESTED_WITH']) && $_SERVER['HTTP_X_REQUESTED_WITH'] === 'XMLHttpRequest'); $federatedSharingApp = new \OCA\FederatedFileSharing\AppInfo\Application(); $federatedShareProvider = $federatedSharingApp->getFederatedShareProvider(); diff --git a/apps/dav/appinfo/v1/webdav.php b/apps/dav/appinfo/v1/webdav.php index 32f93b27760..a1ad4ab489d 100644 --- a/apps/dav/appinfo/v1/webdav.php +++ b/apps/dav/appinfo/v1/webdav.php @@ -52,9 +52,17 @@ $authBackend = new \OCA\DAV\Connector\Sabre\Auth( \OC::$server->getBruteForceThrottler(), 'principals/' ); +$authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); +$bearerAuthPlugin = new \OCA\DAV\Connector\Sabre\BearerAuth( + \OC::$server->getUserSession(), + \OC::$server->getSession(), + \OC::$server->getRequest() +); +$authPlugin->addBackend($bearerAuthPlugin); + $requestUri = \OC::$server->getRequest()->getRequestUri(); -$server = $serverFactory->createServer($baseuri, $requestUri, $authBackend, function() { +$server = $serverFactory->createServer($baseuri, $requestUri, $authPlugin, function() { // use the view for the logged in user return \OC\Files\Filesystem::getView(); }); diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index f04362dfc08..329aa335ea4 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -40,6 +40,7 @@ use OCP\IRequest; use OCP\ITagManager; use OCP\IUserSession; use Sabre\DAV\Auth\Backend\BackendInterface; +use Sabre\DAV\Auth\Plugin; class ServerFactory { /** @var IConfig */ @@ -92,13 +93,13 @@ class ServerFactory { /** * @param string $baseUri * @param string $requestUri - * @param BackendInterface $authBackend + * @param Plugin $authPlugin * @param callable $viewCallBack callback that should return the view for the dav endpoint * @return Server */ public function createServer($baseUri, $requestUri, - BackendInterface $authBackend, + Plugin $authPlugin, callable $viewCallBack) { // Fire up server $objectTree = new \OCA\DAV\Connector\Sabre\ObjectTree(); @@ -110,7 +111,7 @@ class ServerFactory { // Load plugins $server->addPlugin(new \OCA\DAV\Connector\Sabre\MaintenancePlugin($this->config)); $server->addPlugin(new \OCA\DAV\Connector\Sabre\BlockLegacyClientPlugin($this->config)); - $server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend)); + $server->addPlugin($authPlugin); // FIXME: The following line is a workaround for legacy components relying on being able to send a GET to / $server->addPlugin(new \OCA\DAV\Connector\Sabre\DummyGetResponsePlugin()); $server->addPlugin(new \OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin('webdav', $this->logger)); diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php index 50e228b7e84..58a729e18ec 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php @@ -138,8 +138,9 @@ abstract class RequestTestCase extends TestCase { */ protected function getSabreServer(View $view, $user, $password, ExceptionPlugin $exceptionPlugin) { $authBackend = new Auth($user, $password); + $authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); - $server = $this->serverFactory->createServer('/', 'dummy', $authBackend, function () use ($view) { + $server = $this->serverFactory->createServer('/', 'dummy', $authPlugin, function () use ($view) { return $view; }); $server->addPlugin($exceptionPlugin); diff --git a/build/integration/features/auth.feature b/build/integration/features/auth.feature index edcca4bcd4e..679b2465659 100644 --- a/build/integration/features/auth.feature +++ b/build/integration/features/auth.feature @@ -53,10 +53,14 @@ Feature: auth When requesting "/remote.php/webdav" with "PROPFIND" using restricted basic token auth Then the HTTP status code should be "207" - Scenario: using WebDAV with restricted basic token auth + Scenario: using old WebDAV endpoint with unrestricted client token When requesting "/remote.php/webdav" with "PROPFIND" using an unrestricted client token Then the HTTP status code should be "207" + Scenario: using new WebDAV endpoint with unrestricted client token + When requesting "/remote.php/dav/" with "PROPFIND" using an unrestricted client token + Then the HTTP status code should be "207" + Scenario: using WebDAV with browser session Given a new browser session is started When requesting "/remote.php/webdav" with "PROPFIND" using browser session From 639ba526d0eb763be360e5c57d194758291c3ff0 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 21:38:55 +0200 Subject: [PATCH 33/35] Adjust realm from SabreDAV to Nextcloud Signed-off-by: Lukas Reschke --- apps/dav/lib/Connector/Sabre/BearerAuth.php | 4 ++++ build/integration/features/webdav-related.feature | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/BearerAuth.php b/apps/dav/lib/Connector/Sabre/BearerAuth.php index dd0a1bac292..f0e0f389c33 100644 --- a/apps/dav/lib/Connector/Sabre/BearerAuth.php +++ b/apps/dav/lib/Connector/Sabre/BearerAuth.php @@ -50,6 +50,10 @@ class BearerAuth extends AbstractBearer { $this->session = $session; $this->request = $request; $this->principalPrefix = $principalPrefix; + + // setup realm + $defaults = new \OCP\Defaults(); + $this->realm = $defaults->getName(); } private function setupUserFs($userId) { diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index b8ed1c4a778..4279a155ceb 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -8,7 +8,7 @@ Feature: webdav-related Then the HTTP status code should be "401" And there are no duplicate headers And The following headers should be set - |WWW-Authenticate|Basic realm="Nextcloud"| + |WWW-Authenticate|Basic realm="Nextcloud", Bearer realm="Nextcloud"| Scenario: Unauthenticated call new dav path Given using new dav path @@ -16,7 +16,7 @@ Feature: webdav-related Then the HTTP status code should be "401" And there are no duplicate headers And The following headers should be set - |WWW-Authenticate|Basic realm="Nextcloud"| + |WWW-Authenticate|Basic realm="Nextcloud", Bearer realm="Nextcloud"| Scenario: Moving a file Given using old dav path From b8de3f40ee8822cdd7e9d6fbf3f9983cc214d2e1 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 21:57:07 +0200 Subject: [PATCH 34/35] Bearer comes first on the new endpoint Signed-off-by: Lukas Reschke --- build/integration/features/webdav-related.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index 4279a155ceb..b4fd0511356 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -16,7 +16,7 @@ Feature: webdav-related Then the HTTP status code should be "401" And there are no duplicate headers And The following headers should be set - |WWW-Authenticate|Basic realm="Nextcloud", Bearer realm="Nextcloud"| + |WWW-Authenticate|Bearer realm="Nextcloud", Basic realm="Nextcloud"| Scenario: Moving a file Given using old dav path From f4189699e7348615eeb0e528bc5395d818d301ea Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 21:59:22 +0200 Subject: [PATCH 35/35] Function accepts only integers Signed-off-by: Lukas Reschke --- apps/oauth2/tests/Db/AccessTokenMapperTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/oauth2/tests/Db/AccessTokenMapperTest.php b/apps/oauth2/tests/Db/AccessTokenMapperTest.php index 7a7c5fd7b39..ebc6b55a382 100644 --- a/apps/oauth2/tests/Db/AccessTokenMapperTest.php +++ b/apps/oauth2/tests/Db/AccessTokenMapperTest.php @@ -56,7 +56,7 @@ class AccessTokenMapperTest extends TestCase { * @expectedException \OCA\OAuth2\Exceptions\AccessTokenNotFoundException */ public function testDeleteByClientId() { - $this->accessTokenMapper->deleteByClientId('TestId'); + $this->accessTokenMapper->deleteByClientId(1234); $token = new AccessToken(); $token->setClientId(1234); $token->setTokenId((string)time());