feat(oauth): Allow to skip grant step for selected applications

apps/oauth2/lib/Controller/LoginRedirectorController.php

@@ -8,6 +8,7 @@
  */
 namespace OCA\OAuth2\Controller;
 
+use OC\Core\Controller\ClientFlowLoginController;
 use OCA\OAuth2\Db\ClientMapper;
 use OCA\OAuth2\Exceptions\ClientNotFoundException;
 use OCP\AppFramework\Controller;
@@ -18,10 +19,12 @@
 use OCP\AppFramework\Http\Attribute\UseSession;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\IAppConfig;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\ISession;
 use OCP\IURLGenerator;
+use OCP\Security\ISecureRandom;
 
 #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
 class LoginRedirectorController extends Controller {
@@ -40,6 +43,8 @@ public function __construct(
 		private ClientMapper $clientMapper,
 		private ISession $session,
 		private IL10N $l,
+		private ISecureRandom $random,
+		private IAppConfig $appConfig,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -78,12 +83,28 @@ public function authorize($client_id,
 
 		$this->session->set('oauth.state', $state);
 
-		$targetUrl = $this->urlGenerator->linkToRouteAbsolute(
-			'core.ClientFlowLogin.showAuthPickerPage',
-			[
-				'clientIdentifier' => $client->getClientIdentifier(),
-			]
-		);
+		if (in_array($client->getName(), $this->appConfig->getValueArray('oauth2', 'skipAuthPickerApplications', []))) {
+			/** @see ClientFlowLoginController::showAuthPickerPage **/
+			$stateToken = $this->random->generate(
+				64,
+				ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS
+			);
+			$this->session->set(ClientFlowLoginController::STATE_NAME, $stateToken);
+			$targetUrl = $this->urlGenerator->linkToRouteAbsolute(
+				'core.ClientFlowLogin.grantPage',
+				[
+					'stateToken' => $stateToken,
+					'clientIdentifier' => $client->getClientIdentifier(),
+				]
+			);
+		} else {
+			$targetUrl = $this->urlGenerator->linkToRouteAbsolute(
+				'core.ClientFlowLogin.showAuthPickerPage',
+				[
+					'clientIdentifier' => $client->getClientIdentifier(),
+				]
+			);
+		}
 		return new RedirectResponse($targetUrl);
 	}
 }

apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php

@@ -5,34 +5,35 @@
  */
 namespace OCA\OAuth2\Tests\Controller;
 
+use OC\Core\Controller\ClientFlowLoginController;
 use OCA\OAuth2\Controller\LoginRedirectorController;
 use OCA\OAuth2\Db\Client;
 use OCA\OAuth2\Db\ClientMapper;
 use OCA\OAuth2\Exceptions\ClientNotFoundException;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\IAppConfig;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\ISession;
 use OCP\IURLGenerator;
+use OCP\Security\ISecureRandom;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 /**
  * @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;
-	/** @var IL10N */
-	private $l;
+	private IRequest&MockObject $request;
+	private IURLGenerator&MockObject $urlGenerator;
+	private ClientMapper&MockObject $clientMapper;
+	private ISession&MockObject $session;
+	private IL10N&MockObject $l;
+	private ISecureRandom&MockObject $random;
+	private IAppConfig&MockObject $appConfig;
+
+	private LoginRedirectorController $loginRedirectorController;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -42,14 +43,18 @@ protected function setUp(): void {
 		$this->clientMapper = $this->createMock(ClientMapper::class);
 		$this->session = $this->createMock(ISession::class);
 		$this->l = $this->createMock(IL10N::class);
+		$this->random = $this->createMock(ISecureRandom::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
 
 		$this->loginRedirectorController = new LoginRedirectorController(
 			'oauth2',
 			$this->request,
 			$this->urlGenerator,
 			$this->clientMapper,
 			$this->session,
-			$this->l
+			$this->l,
+			$this->random,
+			$this->appConfig,
 		);
 	}
 
@@ -80,6 +85,53 @@ public function testAuthorize(): void {
 		$this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code'));
 	}
 
+	public function testAuthorizeSkipPicker(): void {
+		$client = new Client();
+		$client->setName('MyClientName');
+		$client->setClientIdentifier('MyClientIdentifier');
+		$this->clientMapper
+			->expects($this->once())
+			->method('getByIdentifier')
+			->with('MyClientId')
+			->willReturn($client);
+		$this->session
+			->expects(static::exactly(2))
+			->method('set')
+			->willReturnCallback(function (string $key, string $value): void {
+				switch ([$key, $value]) {
+					case ['oauth.state', 'MyState']:
+					case [ClientFlowLoginController::STATE_NAME, 'MyStateToken']:
+						/* Expected */
+						break;
+					default:
+						throw new LogicException();
+				}
+			});
+		$this->appConfig
+			->expects(static::once())
+			->method('getValueArray')
+			->with('oauth2', 'skipAuthPickerApplications', [])
+			->willReturn(['MyClientName']);
+		$this->random
+			->expects(static::once())
+			->method('generate')
+			->willReturn('MyStateToken');
+		$this->urlGenerator
+			->expects($this->once())
+			->method('linkToRouteAbsolute')
+			->with(
+				'core.ClientFlowLogin.grantPage',
+				[
+					'stateToken' => 'MyStateToken',
+					'clientIdentifier' => 'MyClientIdentifier',
+				]
+			)
+			->willReturn('https://example.com/?clientIdentifier=foo');
+
+		$expected = new RedirectResponse('https://example.com/?clientIdentifier=foo');
+		$this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code'));
+	}
+
 	public function testAuthorizeWrongResponseType(): void {
 		$client = new Client();
 		$client->setClientIdentifier('MyClientIdentifier');

core/Controller/ClientFlowLoginController.php

@@ -8,7 +8,6 @@
 use OC\Authentication\Events\AppPasswordCreatedEvent;
 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;
@@ -24,6 +23,7 @@
 use OCP\AppFramework\Http\StandaloneTemplateResponse;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Authentication\Exceptions\InvalidTokenException;
+use OCP\Authentication\Token\IToken;
 use OCP\Defaults;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IL10N;
@@ -157,9 +157,11 @@ public function showAuthPickerPage(string $clientIdentifier = '', string $user =
 	#[NoCSRFRequired]
 	#[UseSession]
 	#[FrontpageRoute(verb: 'GET', url: '/login/flow/grant')]
-	public function grantPage(string $stateToken = '',
+	public function grantPage(
+		string $stateToken = '',
 		string $clientIdentifier = '',
-		int $direct = 0): StandaloneTemplateResponse {
+		int $direct = 0,
+	): Response {
 		if (!$this->isValidToken($stateToken)) {
 			return $this->stateTokenForbiddenResponse();
 		}
@@ -203,14 +205,13 @@ public function grantPage(string $stateToken = '',
 		return $response;
 	}
 
-	/**
-	 * @return Http\RedirectResponse|Response
-	 */
 	#[NoAdminRequired]
 	#[UseSession]
 	#[FrontpageRoute(verb: 'POST', url: '/login/flow')]
-	public function generateAppPassword(string $stateToken,
-		string $clientIdentifier = '') {
+	public function generateAppPassword(
+		string $stateToken,
+		string $clientIdentifier = '',
+	): Response {
 		if (!$this->isValidToken($stateToken)) {
 			$this->session->remove(self::STATE_NAME);
 			return $this->stateTokenForbiddenResponse();

tests/Core/Controller/ClientFlowLoginControllerTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -28,38 +31,25 @@
 use OCP\Security\ICrypto;
 use OCP\Security\ISecureRandom;
 use OCP\Session\Exceptions\SessionNotAvailableException;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class ClientFlowLoginControllerTest extends TestCase {
-	/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */
-	private $request;
-	/** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */
-	private $userSession;
-	/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
-	private $l10n;
-	/** @var Defaults|\PHPUnit\Framework\MockObject\MockObject */
-	private $defaults;
-	/** @var ISession|\PHPUnit\Framework\MockObject\MockObject */
-	private $session;
-	/** @var IProvider|\PHPUnit\Framework\MockObject\MockObject */
-	private $tokenProvider;
-	/** @var ISecureRandom|\PHPUnit\Framework\MockObject\MockObject */
-	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 IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */
-	private $eventDispatcher;
-	/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
-	private $timeFactory;
-
-	/** @var ClientFlowLoginController */
-	private $clientFlowLoginController;
+	private IRequest&MockObject $request;
+	private IUserSession&MockObject $userSession;
+	private IL10N&MockObject $l10n;
+	private Defaults&MockObject $defaults;
+	private ISession&MockObject $session;
+	private IProvider&MockObject $tokenProvider;
+	private ISecureRandom&MockObject $random;
+	private IURLGenerator&MockObject $urlGenerator;
+	private ClientMapper&MockObject $clientMapper;
+	private AccessTokenMapper&MockObject $accessTokenMapper;
+	private ICrypto&MockObject $crypto;
+	private IEventDispatcher&MockObject $eventDispatcher;
+	private ITimeFactory&MockObject $timeFactory;
+
+	private ClientFlowLoginController $clientFlowLoginController;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -98,7 +88,7 @@ protected function setUp(): void {
 			$this->accessTokenMapper,
 			$this->crypto,
 			$this->eventDispatcher,
-			$this->timeFactory
+			$this->timeFactory,
 		);
 	}
 

tests/lib/Config/UserConfigTest.php

@@ -746,9 +746,9 @@ public function providerSearchValuesByUsers(): array {
 	public function testSearchValuesByUsers(
 		string $app,
 		string $key,
-		?ValueType $typedAs = null,
-		?array $userIds = null,
-		array $result = [],
+		?ValueType $typedAs,
+		?array $userIds,
+		array $result,
 	): void {
 		$userConfig = $this->generateUserConfig();
 		$this->assertEqualsCanonicalizing(