remove authPickerPage (v1 and v2)

Signed-off-by: Sergey Nikolaev <kinolaev@gmail.com>
nextcloud · Sep 19, 2020 · feee428 · feee428
1 parent 0a38f4e
commit feee428
Show file tree

Hide file tree

Showing 26 changed files with 229 additions and 443 deletions.
diff --git a/apps/oauth2/lib/Controller/LoginRedirectorController.php b/apps/oauth2/lib/Controller/LoginRedirectorController.php
@@ -101,7 +101,7 @@ public function authorize($client_id,
 		$this->session->set('oauth.state', $state);
 
 		$targetUrl = $this->urlGenerator->linkToRouteAbsolute(
-			'core.ClientFlowLogin.showAuthPickerPage',
+			'core.ClientFlowLogin.grantPage',
 			[
 				'clientIdentifier' => $client->getClientIdentifier(),
 			]

diff --git a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php
@@ -90,7 +90,7 @@ public function testAuthorize() {
 			->expects($this->once())
 			->method('linkToRouteAbsolute')
 			->with(
-				'core.ClientFlowLogin.showAuthPickerPage',
+				'core.ClientFlowLogin.grantPage',
 				[
 					'clientIdentifier' => 'MyClientIdentifier',
 				]

diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php
@@ -41,7 +41,6 @@
 use OCA\OAuth2\Db\ClientMapper;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
-use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Http\StandaloneTemplateResponse;
 use OCP\Defaults;
@@ -161,101 +160,21 @@ private function stateTokenForbiddenResponse() {
 	}
 
 	/**
-	 * @PublicPage
+	 * @NoAdminRequired
 	 * @NoCSRFRequired
+	 * @NoSameSiteCookieRequired
 	 * @UseSession
 	 *
 	 * @param string $clientIdentifier
-	 *
-	 * @return StandaloneTemplateResponse|RedirectResponse
+	 * @return StandaloneTemplateResponse
 	 */
-	public function showAuthPickerPage($clientIdentifier = '') {
-		$clientName = $this->getClientName();
-		$client = null;
-		if ($clientIdentifier !== '') {
-			$client = $this->clientMapper->getByIdentifier($clientIdentifier);
-			$clientName = $client->getName();
-		}
-
-		// 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 StandaloneTemplateResponse(
-				$this->appName,
-				'error',
-				[
-					'errors' =>
-					[
-						[
-							'error' => 'Access Forbidden',
-							'hint' => 'Invalid request',
-						],
-					],
-				],
-				'guest'
-			);
-		}
-
+	public function grantPage($clientIdentifier = '') {
 		$stateToken = $this->random->generate(
 			64,
 			ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_DIGITS
 		);
 		$this->session->set(self::STATE_NAME, $stateToken);
 
-		$oauthState = $this->session->get('oauth.state');
-		if (!empty($oauthState)) {
-			$targetUrl = $this->urlGenerator->linkToRoute(
-				'core.ClientFlowLogin.grantPage',
-				[
-					'stateToken' => $stateToken,
-					'clientIdentifier' => $clientIdentifier,
-					'oauthState' => $oauthState
-				]
-			);
-			return new RedirectResponse($targetUrl);
-		}
-
-		$csp = new Http\ContentSecurityPolicy();
-		if ($client) {
-			$csp->addAllowedFormActionDomain($client->getRedirectUri());
-		} else {
-			$csp->addAllowedFormActionDomain('nc://*');
-		}
-
-		$response = new StandaloneTemplateResponse(
-			$this->appName,
-			'loginflow/authpicker',
-			[
-				'client' => $clientName,
-				'clientIdentifier' => $clientIdentifier,
-				'instanceName' => $this->defaults->getName(),
-				'urlGenerator' => $this->urlGenerator,
-				'stateToken' => $stateToken,
-				'serverHost' => $this->getServerPath(),
-			],
-			'guest'
-		);
-
-		$response->setContentSecurityPolicy($csp);
-		return $response;
-	}
-
-	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 * @NoSameSiteCookieRequired
-	 * @UseSession
-	 *
-	 * @param string $stateToken
-	 * @param string $clientIdentifier
-	 * @return StandaloneTemplateResponse
-	 */
-	public function grantPage($stateToken = '',
-								 $clientIdentifier = '') {
-		if (!$this->isValidToken($stateToken)) {
-			return $this->stateTokenForbiddenResponse();
-		}
-
 		$clientName = $this->getClientName();
 		$client = null;
 		if ($clientIdentifier !== '') {
@@ -386,11 +305,7 @@ public function generateAppPassword($stateToken,
 	/**
 	 * @PublicPage
 	 */
-	public function apptokenRedirect(string $stateToken, string $user, string $password) {
-		if (!$this->isValidToken($stateToken)) {
-			return $this->stateTokenForbiddenResponse();
-		}
-
+	public function apptokenRedirect(string $user, string $password) {
 		try {
 			$token = $this->tokenProvider->getToken($password);
 			if ($token->getLoginName() !== $user) {
@@ -410,7 +325,24 @@ public function apptokenRedirect(string $stateToken, string $user, string $passw
 		}
 
 		$redirectUri = 'nc://login/server:' . $this->getServerPath() . '&user:' . urlencode($user) . '&password:' . urlencode($password);
-		return new Http\RedirectResponse($redirectUri);
+
+		$csp = new Http\ContentSecurityPolicy();
+		$csp->addAllowedFormActionDomain('nc://*');
+
+		$response = new StandaloneTemplateResponse(
+			$this->appName,
+			'loginflow/grant',
+			[
+				'client' => $token->getName(),
+				'instanceName' => $this->defaults->getName(),
+				'urlGenerator' => $this->urlGenerator,
+				'redirectUri' => $redirectUri,
+			],
+			'guest'
+		);
+
+		$response->setContentSecurityPolicy($csp);
+		return $response;
 	}
 
 	private function getServerPath(): string {

diff --git a/core/Controller/ClientFlowLoginV2Controller.php b/core/Controller/ClientFlowLoginV2Controller.php
@@ -109,16 +109,17 @@ public function landing(string $token): Response {
 		$this->session->set(self::TOKEN_NAME, $token);
 
 		return new RedirectResponse(
-			$this->urlGenerator->linkToRouteAbsolute('core.ClientFlowLoginV2.showAuthPickerPage')
+			$this->urlGenerator->linkToRouteAbsolute('core.ClientFlowLoginV2.grantPage')
 		);
 	}
 
 	/**
-	 * @NoCSRFRequired
-	 * @PublicPage
+	 * @NoAdminRequired
 	 * @UseSession
+	 * @NoCSRFRequired
+	 * @NoSameSiteCookieRequired
 	 */
-	public function showAuthPickerPage(): StandaloneTemplateResponse {
+	public function grantPage(): StandaloneTemplateResponse {
 		try {
 			$flow = $this->getFlowByLoginToken();
 		} catch (LoginFlowV2NotFoundException $e) {
@@ -131,36 +132,6 @@ public function showAuthPickerPage(): StandaloneTemplateResponse {
 		);
 		$this->session->set(self::STATE_NAME, $stateToken);
 
-		return new StandaloneTemplateResponse(
-			$this->appName,
-			'loginflowv2/authpicker',
-			[
-				'client' => $flow->getClientName(),
-				'instanceName' => $this->defaults->getName(),
-				'urlGenerator' => $this->urlGenerator,
-				'stateToken' => $stateToken,
-			],
-			'guest'
-		);
-	}
-
-	/**
-	 * @NoAdminRequired
-	 * @UseSession
-	 * @NoCSRFRequired
-	 * @NoSameSiteCookieRequired
-	 */
-	public function grantPage(string $stateToken): StandaloneTemplateResponse {
-		if (!$this->isValidStateToken($stateToken)) {
-			return $this->stateTokenForbiddenResponse();
-		}
-
-		try {
-			$flow = $this->getFlowByLoginToken();
-		} catch (LoginFlowV2NotFoundException $e) {
-			return $this->loginTokenForbiddenResponse();
-		}
-
 		return new StandaloneTemplateResponse(
 			$this->appName,
 			'loginflowv2/grant',

diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php
@@ -36,6 +36,7 @@
 use OC\Authentication\Login\Chain;
 use OC\Authentication\Login\LoginData;
 use OC\Authentication\WebAuthn\Manager as WebAuthnManager;
+use OC\Core\Service\LoginFlowV2Service;
 use OC\Security\Bruteforce\Throttler;
 use OC\User\Session;
 use OC_App;
@@ -86,6 +87,8 @@ class LoginController extends Controller {
 	private $webAuthnManager;
 	/** @var ClientMapper */
 	private $clientMapper;
+	/** @var LoginFlowV2Service */
+	private $loginFlowV2Service;
 
 	public function __construct(?string $appName,
 								IRequest $request,
@@ -100,7 +103,8 @@ public function __construct(?string $appName,
 								Chain $loginChain,
 								IInitialStateService $initialStateService,
 								WebAuthnManager $webAuthnManager,
-								ClientMapper $clientMapper) {
+								ClientMapper $clientMapper,
+								LoginFlowV2Service $loginFlowV2Service) {
 		parent::__construct($appName, $request);
 		$this->userManager = $userManager;
 		$this->config = $config;
@@ -114,6 +118,7 @@ public function __construct(?string $appName,
 		$this->initialStateService = $initialStateService;
 		$this->webAuthnManager = $webAuthnManager;
 		$this->clientMapper = $clientMapper;
+		$this->loginFlowV2Service = $loginFlowV2Service;
 	}
 
 	/**
@@ -182,19 +187,11 @@ public function showLoginForm(string $user = null, string $redirect_url = null):
 		if (!empty($redirect_url)) {
 			$this->initialStateService->provideInitialState('core', 'loginRedirectUrl', $redirect_url);
 
-			$grant_url = $this->urlGenerator->linkToRoute('core.ClientFlowLogin.grantPage');
-			if (strpos($redirect_url, $grant_url) === 0) {
-				parse_str(parse_url($redirect_url, PHP_URL_QUERY), $grant_query);
-				if (empty($grant_query['clientIdentifier'])) {
-					$userAgent = $this->request->getHeader('USER_AGENT');
-					$clientName = $userAgent !== '' ? $userAgent : 'unknown';
-				} else {
-					$client = $this->clientMapper->getByIdentifier($grant_query['clientIdentifier']);
-					$clientName = $client->getName();
-				}
+			$clientName = $this->getClientName($redirect_url);
+			if ($clientName) {
 				$this->initialStateService->provideInitialState('core', 'loginGrantParams', [
 					'client' => Util::sanitizeHTML($clientName),
-					'instanceName' => Util::sanitizeHTML($this->defaults->getName())
+					'instanceName' => Util::sanitizeHTML($this->defaults->getName()),
 				]);
 			}
 		}
@@ -393,4 +390,26 @@ public function confirmPassword($password) {
 		$this->session->set('last-password-confirm', $confirmTimestamp);
 		return new DataResponse(['lastLogin' => $confirmTimestamp], Http::STATUS_OK);
 	}
+
+	/**
+	 * @param string $redirect_url
+	 * @return null|string
+	 */
+	private function getClientName($redirect_url) {
+		if (strpos($redirect_url, $this->urlGenerator->linkToRoute('core.ClientFlowLogin.grantPage')) === 0) {
+			parse_str(parse_url($redirect_url, PHP_URL_QUERY), $clientFlowQuery);
+			if (isset($clientFlowQuery['clientIdentifier'])) {
+				$client = $this->clientMapper->getByIdentifier($clientFlowQuery['clientIdentifier']);
+				return $client->getName();
+			} else {
+				$userAgent = $this->request->getHeader('USER_AGENT');
+				return $userAgent !== '' ? $userAgent : 'unknown';
+			}
+		} elseif (strpos($redirect_url, $this->urlGenerator->linkToRoute('core.ClientFlowLoginV2.grantPage')) === 0) {
+			$flowV2Token = $this->session->get(ClientFlowLoginV2Controller::TOKEN_NAME);
+			$flowV2 = $this->loginFlowV2Service->getByLoginToken($flowV2Token);
+			return $flowV2->getClientName();
+		}
+		return null;
+	}
 }
diff --git a/core/js/dist/install.js b/core/js/dist/install.js
diff --git a/core/js/dist/install.js.map b/core/js/dist/install.js.map
diff --git a/core/js/dist/login.js b/core/js/dist/login.js
diff --git a/core/js/dist/login.js.map b/core/js/dist/login.js.map
diff --git a/core/js/dist/main.js b/core/js/dist/main.js
diff --git a/core/js/dist/main.js.map b/core/js/dist/main.js.map
diff --git a/core/js/dist/maintenance.js b/core/js/dist/maintenance.js
diff --git a/core/js/dist/maintenance.js.map b/core/js/dist/maintenance.js.map
diff --git a/core/js/dist/recommendedapps.js b/core/js/dist/recommendedapps.js
diff --git a/core/js/dist/recommendedapps.js.map b/core/js/dist/recommendedapps.js.map
diff --git a/core/js/dist/unified-search.js b/core/js/dist/unified-search.js
diff --git a/core/js/dist/unified-search.js.map b/core/js/dist/unified-search.js.map
diff --git a/core/routes.php b/core/routes.php
@@ -54,15 +54,13 @@
 		['name' => 'login#showLoginForm', 'url' => '/login', 'verb' => 'GET'],
 		['name' => 'login#logout', 'url' => '/logout', 'verb' => 'GET'],
 		// Original login flow used by all clients
-		['name' => 'ClientFlowLogin#showAuthPickerPage', 'url' => '/login/flow', 'verb' => 'GET'],
+		['name' => 'ClientFlowLogin#grantPage', 'url' => '/login/flow', 'verb' => 'GET'],
 		['name' => 'ClientFlowLogin#generateAppPassword', 'url' => '/login/flow', 'verb' => 'POST'],
-		['name' => 'ClientFlowLogin#grantPage', 'url' => '/login/flow/grant', 'verb' => 'GET'],
 		['name' => 'ClientFlowLogin#apptokenRedirect', 'url' => '/login/flow/apptoken', 'verb' => 'POST'],
 		// NG login flow used by desktop client in case of Kerberos/fancy 2fa (smart cards for example)
 		['name' => 'ClientFlowLoginV2#poll', 'url' => '/login/v2/poll', 'verb' => 'POST'],
-		['name' => 'ClientFlowLoginV2#showAuthPickerPage', 'url' => '/login/v2/flow', 'verb' => 'GET'],
+		['name' => 'ClientFlowLoginV2#grantPage', 'url' => '/login/v2/flow', 'verb' => 'GET'],
 		['name' => 'ClientFlowLoginV2#landing', 'url' => '/login/v2/flow/{token}', 'verb' => 'GET'],
-		['name' => 'ClientFlowLoginV2#grantPage', 'url' => '/login/v2/grant', 'verb' => 'GET'],
 		['name' => 'ClientFlowLoginV2#generateAppPassword', 'url' => '/login/v2/grant', 'verb' => 'POST'],
 		['name' => 'ClientFlowLoginV2#init', 'url' => '/login/v2', 'verb' => 'POST'],
 		['name' => 'TwoFactorChallenge#selectChallenge', 'url' => '/login/selectchallenge', 'verb' => 'GET'],