From b07a0f51bacc65cc55982172301599ec12fdc235 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 18 May 2017 15:43:14 +0200 Subject: [PATCH] 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 1a2e00ef5d..9237b4b1b3 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 b41a29dc1c..cafcc16442 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); }