From 7cee0eb8c1a78acd92ed48f5b5c91e6cd6eb7a43 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 13 May 2022 10:50:30 +0200 Subject: [PATCH 1/2] Show user account on grant loginflow step Signed-off-by: Joas Schilling --- core/Controller/ClientFlowLoginController.php | 6 ++++++ core/Controller/ClientFlowLoginV2Controller.php | 11 +++++++++++ core/templates/loginflow/grant.php | 8 +++++++- core/templates/loginflowv2/grant.php | 10 ++++++++-- 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index d67a065a14eff..6a88705c6e475 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -49,6 +49,7 @@ use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; +use OCP\IUser; use OCP\IUserSession; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; @@ -250,10 +251,15 @@ public function grantPage(string $stateToken = '', $csp->addAllowedFormActionDomain('nc://*'); } + /** @var IUser $user */ + $user = $this->userSession->getUser(); + $response = new StandaloneTemplateResponse( $this->appName, 'loginflow/grant', [ + 'userId' => $user->getUID(), + 'userDisplayName' => $user->getDisplayName(), 'client' => $clientName, 'clientIdentifier' => $clientIdentifier, 'instanceName' => $this->defaults->getName(), diff --git a/core/Controller/ClientFlowLoginV2Controller.php b/core/Controller/ClientFlowLoginV2Controller.php index 205c1ff8a1c74..f2b9399301f42 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -41,6 +41,8 @@ use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; +use OCP\IUser; +use OCP\IUserSession; use OCP\Security\ISecureRandom; class ClientFlowLoginV2Controller extends Controller { @@ -53,6 +55,8 @@ class ClientFlowLoginV2Controller extends Controller { private $urlGenerator; /** @var ISession */ private $session; + /** @var IUserSession */ + private $userSession; /** @var ISecureRandom */ private $random; /** @var Defaults */ @@ -67,6 +71,7 @@ public function __construct(string $appName, LoginFlowV2Service $loginFlowV2Service, IURLGenerator $urlGenerator, ISession $session, + IUserSession $userSession, ISecureRandom $random, Defaults $defaults, ?string $userId, @@ -75,6 +80,7 @@ public function __construct(string $appName, $this->loginFlowV2Service = $loginFlowV2Service; $this->urlGenerator = $urlGenerator; $this->session = $session; + $this->userSession = $userSession; $this->random = $random; $this->defaults = $defaults; $this->userId = $userId; @@ -160,10 +166,15 @@ public function grantPage(string $stateToken): StandaloneTemplateResponse { return $this->loginTokenForbiddenResponse(); } + /** @var IUser $user */ + $user = $this->userSession->getUser(); + return new StandaloneTemplateResponse( $this->appName, 'loginflowv2/grant', [ + 'userId' => $user->getUID(), + 'userDisplayName' => $user->getDisplayName(), 'client' => $flow->getClientName(), 'instanceName' => $this->defaults->getName(), 'urlGenerator' => $this->urlGenerator, diff --git a/core/templates/loginflow/grant.php b/core/templates/loginflow/grant.php index c537c47ea6488..04fdced1c62b3 100644 --- a/core/templates/loginflow/grant.php +++ b/core/templates/loginflow/grant.php @@ -29,6 +29,12 @@

t('Account access')) ?>

+

+ t('Currently logged in as %1$s (%2$s).', [ + $_['userDisplayName'], + $_['userId'], + ])) ?> +

t('You are about to grant %1$s access to your %2$s account.', [ '' . \OCP\Util::sanitizeHTML($_['client']) . '', @@ -44,7 +50,7 @@ - +

diff --git a/core/templates/loginflowv2/grant.php b/core/templates/loginflowv2/grant.php index b036d33ad7c92..19005a20e2c1a 100644 --- a/core/templates/loginflowv2/grant.php +++ b/core/templates/loginflowv2/grant.php @@ -29,6 +29,12 @@

t('Account access')) ?>

+

+ t('Currently logged in as %1$s (%2$s).', [ + $_['userDisplayName'], + $_['userId'], + ])) ?> +

t('You are about to grant %1$s access to your %2$s account.', [ '' . \OCP\Util::sanitizeHTML($_['client']) . '', @@ -41,10 +47,10 @@

- +
-
+

From e432abdb1ba65bb07274e31f2005b5481233fc9e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 16 May 2022 10:33:30 +0200 Subject: [PATCH 2/2] Extend tests Signed-off-by: Joas Schilling --- .../ClientFlowLoginControllerTest.php | 69 ++++++++++--------- .../ClientFlowLoginV2ControllerTest.php | 14 ++++ 2 files changed, 50 insertions(+), 33 deletions(-) diff --git a/tests/Core/Controller/ClientFlowLoginControllerTest.php b/tests/Core/Controller/ClientFlowLoginControllerTest.php index c60c89407bdd7..f704f29c4871a 100644 --- a/tests/Core/Controller/ClientFlowLoginControllerTest.php +++ b/tests/Core/Controller/ClientFlowLoginControllerTest.php @@ -134,15 +134,15 @@ public function testShowAuthPickerPageNoClientOrOauthRequest() { public function testShowAuthPickerPageWithOcsHeader() { $this->request - ->expects($this->at(0)) ->method('getHeader') - ->with('USER_AGENT') - ->willReturn('Mac OS X Sync Client'); - $this->request - ->expects($this->at(1)) - ->method('getHeader') - ->with('OCS-APIREQUEST') - ->willReturn('true'); + ->withConsecutive( + ['USER_AGENT'], + ['OCS-APIREQUEST'] + ) + ->willReturnMap([ + ['USER_AGENT', 'Mac OS X Sync Client'], + ['OCS-APIREQUEST', 'true'], + ]); $this->random ->expects($this->once()) ->method('generate') @@ -195,10 +195,15 @@ public function testShowAuthPickerPageWithOcsHeader() { public function testShowAuthPickerPageWithOauth() { $this->request - ->expects($this->at(0)) ->method('getHeader') - ->with('USER_AGENT') - ->willReturn('Mac OS X Sync Client'); + ->withConsecutive( + ['USER_AGENT'], + ['OCS-APIREQUEST'] + ) + ->willReturnMap([ + ['USER_AGENT', 'Mac OS X Sync Client'], + ['OCS-APIREQUEST', 'false'], + ]); $client = new Client(); $client->setName('My external service'); $client->setRedirectUri('https://example.com/redirect.php'); @@ -411,23 +416,21 @@ public function testGeneratePasswordWithPassword() { */ public function testGeneratePasswordWithPasswordForOauthClient($redirectUri, $redirectUrl) { $this->session - ->expects($this->at(0)) ->method('get') - ->with('client.flow.state.token') - ->willReturn('MyStateToken'); - $this->session - ->expects($this->at(1)) - ->method('remove') - ->with('client.flow.state.token'); - $this->session - ->expects($this->at(3)) - ->method('get') - ->with('oauth.state') - ->willReturn('MyOauthState'); + ->withConsecutive( + ['client.flow.state.token'], + ['oauth.state'] + ) + ->willReturnMap([ + ['client.flow.state.token', 'MyStateToken'], + ['oauth.state', 'MyOauthState'], + ]); $this->session - ->expects($this->at(4)) ->method('remove') - ->with('oauth.state'); + ->withConsecutive( + ['client.flow.state.token'], + ['oauth.state'] + ); $this->session ->expects($this->once()) ->method('getId') @@ -448,15 +451,15 @@ public function testGeneratePasswordWithPasswordForOauthClient($redirectUri, $re ->with($myToken, 'SessionId') ->willReturn('MyPassword'); $this->random - ->expects($this->at(0)) ->method('generate') - ->with(72) - ->willReturn('MyGeneratedToken'); - $this->random - ->expects($this->at(1)) - ->method('generate') - ->with(128) - ->willReturn('MyAccessCode'); + ->withConsecutive( + [72], + [128] + ) + ->willReturnMap([ + [72, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS, 'MyGeneratedToken'], + [128, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS, 'MyAccessCode'], + ]); $user = $this->createMock(IUser::class); $user ->expects($this->once()) diff --git a/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php b/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php index 1e35dc71c3f0b..53d5f392ac647 100644 --- a/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php +++ b/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php @@ -36,6 +36,8 @@ use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; +use OCP\IUser; +use OCP\IUserSession; use OCP\Security\ISecureRandom; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -50,6 +52,8 @@ class ClientFlowLoginV2ControllerTest extends TestCase { private $urlGenerator; /** @var ISession|MockObject */ private $session; + /** @var IUserSession|MockObject */ + private $userSession; /** @var ISecureRandom|MockObject */ private $random; /** @var Defaults|MockObject */ @@ -66,6 +70,7 @@ protected function setUp(): void { $this->loginFlowV2Service = $this->createMock(LoginFlowV2Service::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->session = $this->createMock(ISession::class); + $this->userSession = $this->createMock(IUserSession::class); $this->random = $this->createMock(ISecureRandom::class); $this->defaults = $this->createMock(Defaults::class); $this->l = $this->createMock(IL10N::class); @@ -75,6 +80,7 @@ protected function setUp(): void { $this->loginFlowV2Service, $this->urlGenerator, $this->session, + $this->userSession, $this->random, $this->defaults, 'user', @@ -224,6 +230,14 @@ public function testGrantPageValid() { return null; }); + $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn('uid'); + $user->method('getDisplayName') + ->willReturn('display name'); + $this->userSession->method('getUser') + ->willReturn($user); + $flow = new LoginFlowV2(); $this->loginFlowV2Service->method('getByLoginToken') ->with('loginToken')