From e6b6f3f16981e61557feef29ec82d231420c34bd Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Mon, 29 Mar 2021 04:47:27 +0200 Subject: [PATCH] Allow users to manually decline auth request even when an application was authenticated before --- api/modules/oauth/models/OauthProcess.php | 31 +++++++++++---------- api/tests/functional/oauth/AuthCodeCest.php | 2 +- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/api/modules/oauth/models/OauthProcess.php b/api/modules/oauth/models/OauthProcess.php index 16cac16..0aad3de 100644 --- a/api/modules/oauth/models/OauthProcess.php +++ b/api/modules/oauth/models/OauthProcess.php @@ -25,10 +25,7 @@ class OauthProcess { P::OBTAIN_ACCOUNT_EMAIL => 'account_email', ]; - /** - * @var AuthorizationServer - */ - private $server; + private AuthorizationServer $server; public function __construct(AuthorizationServer $server) { $this->server = $server; @@ -96,19 +93,22 @@ class OauthProcess { /** @var OauthClient $client */ $client = $this->findClient($authRequest->getClient()->getIdentifier()); - $approved = $this->canAutoApprove($account, $client, $authRequest); - if (!$approved) { - Yii::$app->statsd->inc('oauth.complete.approve_required'); + $canBeAutoApproved = $this->canBeAutoApproved($account, $client, $authRequest); + $acceptParam = ((array)$request->getParsedBody())['accept'] ?? null; + if ($acceptParam === null && !$canBeAutoApproved) { + throw $this->createAcceptRequiredException(); + } - $acceptParam = ((array)$request->getParsedBody())['accept'] ?? null; - if ($acceptParam === null) { - throw $this->createAcceptRequiredException(); - } + Yii::$app->statsd->inc('oauth.complete.approve_required'); + if ($acceptParam === null && $canBeAutoApproved) { + $approved = true; + } else { $approved = in_array($acceptParam, [1, '1', true, 'true'], true); - if ($approved) { - $this->storeOauthSession($account, $client, $authRequest); - } + } + + if ($approved) { + $this->storeOauthSession($account, $client, $authRequest); } $authRequest->setUser(new UserEntity($account->id)); @@ -123,6 +123,7 @@ class OauthProcess { Yii::$app->statsd->inc('oauth.complete.success'); } catch (OAuthServerException $e) { if ($e->getErrorType() === 'accept_required') { + // TODO: revoke access if there previously was an oauth session? Yii::$app->statsd->inc('oauth.complete.fail'); } @@ -213,7 +214,7 @@ class OauthProcess { * * @return bool */ - private function canAutoApprove(Account $account, OauthClient $client, AuthorizationRequest $request): bool { + private function canBeAutoApproved(Account $account, OauthClient $client, AuthorizationRequest $request): bool { if ($client->is_trusted) { return true; } diff --git a/api/tests/functional/oauth/AuthCodeCest.php b/api/tests/functional/oauth/AuthCodeCest.php index 425ba23..2f141c0 100644 --- a/api/tests/functional/oauth/AuthCodeCest.php +++ b/api/tests/functional/oauth/AuthCodeCest.php @@ -134,8 +134,8 @@ class AuthCodeCest { 'error' => 'access_denied', 'parameter' => null, 'statusCode' => 401, + 'redirectUri' => 'http://ely.by?&error=access_denied&error_description=The+resource+owner+or+authorization+server+denied+the+request.&hint=The+user+denied+the+request&message=The+resource+owner+or+authorization+server+denied+the+request.', ]); - $I->canSeeResponseJsonMatchesJsonPath('$.redirectUri'); } public function invalidClientId(FunctionalTester $I) {