Add OAuth state to session

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

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);

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);
 		}