Skip to content

Commit 5bef2d7

Browse files
come-nccome-nc
committed
fixup! feat(login): Add rememberme checkbox
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
1 parent 8b0a2f4 commit 5bef2d7

2 files changed

Lines changed: 51 additions & 18 deletions

File tree

‎tests/Core/Controller/LoginControllerTest.php‎

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
use OCP\Notification\IManager;
3232
use OCP\Security\Bruteforce\IThrottler;
3333
use OCP\Security\ITrustedDomainHelper;
34+
use PHPUnit\Framework\Attributes\DataProvider;
3435
use PHPUnit\Framework\MockObject\MockObject;
3536
use Test\TestCase;
3637

@@ -277,7 +278,7 @@ public function testShowLoginFormWithErrorsInSession(): void {
277278
'',
278279
]
279280
];
280-
$this->initialState->expects($this->exactly(13))
281+
$this->initialState->expects($this->exactly(14))
281282
->method('provideInitialState')
282283
->willReturnCallback(function () use (&$calls): void {
283284
$expected = array_shift($calls);
@@ -309,12 +310,16 @@ public function testShowLoginFormForFlowAuth(): void {
309310
'loginAutocomplete',
310311
false
311312
],
313+
[
314+
'loginCanRememberme',
315+
false
316+
],
312317
[
313318
'loginRedirectUrl',
314319
'login/flow'
315320
],
316321
];
317-
$this->initialState->expects($this->exactly(14))
322+
$this->initialState->expects($this->exactly(15))
318323
->method('provideInitialState')
319324
->willReturnCallback(function () use (&$calls): void {
320325
$expected = array_shift($calls);
@@ -351,7 +356,7 @@ public static function passwordResetDataProvider(): array {
351356
];
352357
}
353358

354-
#[\PHPUnit\Framework\Attributes\DataProvider('passwordResetDataProvider')]
359+
#[DataProvider('passwordResetDataProvider')]
355360
public function testShowLoginFormWithPasswordResetOption($canChangePassword,
356361
$expectedResult): void {
357362
$this->userSession
@@ -386,13 +391,13 @@ public function testShowLoginFormWithPasswordResetOption($canChangePassword,
386391
'loginUsername',
387392
'LdapUser'
388393
],
389-
[], [], [],
394+
[], [], [], [],
390395
[
391396
'loginCanResetPassword',
392397
$expectedResult
393398
],
394399
];
395-
$this->initialState->expects($this->exactly(13))
400+
$this->initialState->expects($this->exactly(14))
396401
->method('provideInitialState')
397402
->willReturnCallback(function () use (&$calls): void {
398403
$expected = array_shift($calls);
@@ -445,6 +450,10 @@ public function testShowLoginFormForUserNamed0(): void {
445450
'loginAutocomplete',
446451
true
447452
],
453+
[
454+
'loginCanRememberme',
455+
false
456+
],
448457
[],
449458
[
450459
'loginResetPasswordLink',
@@ -455,7 +464,7 @@ public function testShowLoginFormForUserNamed0(): void {
455464
false
456465
],
457466
];
458-
$this->initialState->expects($this->exactly(13))
467+
$this->initialState->expects($this->exactly(14))
459468
->method('provideInitialState')
460469
->willReturnCallback(function () use (&$calls): void {
461470
$expected = array_shift($calls);
@@ -476,7 +485,19 @@ public function testShowLoginFormForUserNamed0(): void {
476485
$this->assertEquals($expectedResponse, $this->loginController->showLoginForm('0', ''));
477486
}
478487

479-
public function testLoginWithInvalidCredentials(): void {
488+
public static function remembermeProvider(): array {
489+
return [
490+
[
491+
true,
492+
],
493+
[
494+
false,
495+
],
496+
];
497+
}
498+
499+
#[DataProvider('remembermeProvider')]
500+
public function testLoginWithInvalidCredentials(bool $rememberme): void {
480501
$user = 'MyUserName';
481502
$password = 'secret';
482503
$loginPageUrl = '/login?redirect_url=/apps/files';
@@ -491,6 +512,7 @@ public function testLoginWithInvalidCredentials(): void {
491512
$this->request,
492513
$user,
493514
$password,
515+
$rememberme,
494516
'/apps/files'
495517
);
496518
$loginResult = LoginResult::failure($loginData, LoginController::LOGIN_MSG_INVALIDPASSWORD);
@@ -509,12 +531,13 @@ public function testLoginWithInvalidCredentials(): void {
509531
$expected = new RedirectResponse($loginPageUrl);
510532
$expected->throttle(['user' => 'MyUserName']);
511533

512-
$response = $this->loginController->tryLogin($loginChain, $trustedDomainHelper, $user, $password, '/apps/files');
534+
$response = $this->loginController->tryLogin($loginChain, $trustedDomainHelper, $user, $password, $rememberme, '/apps/files');
513535

514536
$this->assertEquals($expected, $response);
515537
}
516538

517-
public function testLoginWithValidCredentials(): void {
539+
#[DataProvider('remembermeProvider')]
540+
public function testLoginWithValidCredentials(bool $rememberme): void {
518541
$user = 'MyUserName';
519542
$password = 'secret';
520543
$loginChain = $this->createMock(LoginChain::class);
@@ -527,7 +550,8 @@ public function testLoginWithValidCredentials(): void {
527550
$loginData = new LoginData(
528551
$this->request,
529552
$user,
530-
$password
553+
$password,
554+
$rememberme,
531555
);
532556
$loginResult = LoginResult::success($loginData);
533557
$loginChain->expects($this->once())
@@ -540,10 +564,11 @@ public function testLoginWithValidCredentials(): void {
540564
->willReturn('/default/foo');
541565

542566
$expected = new RedirectResponse('/default/foo');
543-
$this->assertEquals($expected, $this->loginController->tryLogin($loginChain, $trustedDomainHelper, $user, $password));
567+
$this->assertEquals($expected, $this->loginController->tryLogin($loginChain, $trustedDomainHelper, $user, $password, $rememberme));
544568
}
545569

546-
public function testLoginWithoutPassedCsrfCheckAndNotLoggedIn(): void {
570+
#[DataProvider('remembermeProvider')]
571+
public function testLoginWithoutPassedCsrfCheckAndNotLoggedIn(bool $rememberme): void {
547572
/** @var IUser|MockObject $user */
548573
$user = $this->createMock(IUser::class);
549574
$user->expects($this->any())
@@ -567,14 +592,15 @@ public function testLoginWithoutPassedCsrfCheckAndNotLoggedIn(): void {
567592
$this->userSession->expects($this->never())
568593
->method('createRememberMeToken');
569594

570-
$response = $this->loginController->tryLogin($loginChain, $trustedDomainHelper, 'Jane', $password, $originalUrl);
595+
$response = $this->loginController->tryLogin($loginChain, $trustedDomainHelper, 'Jane', $password, $rememberme, $originalUrl);
571596

572597
$expected = new RedirectResponse('');
573598
$expected->throttle(['user' => 'Jane']);
574599
$this->assertEquals($expected, $response);
575600
}
576601

577-
public function testLoginWithoutPassedCsrfCheckAndLoggedIn(): void {
602+
#[DataProvider('remembermeProvider')]
603+
public function testLoginWithoutPassedCsrfCheckAndLoggedIn(bool $rememberme): void {
578604
/** @var IUser|MockObject $user */
579605
$user = $this->createMock(IUser::class);
580606
$user->expects($this->any())
@@ -607,13 +633,14 @@ public function testLoginWithoutPassedCsrfCheckAndLoggedIn(): void {
607633
->with('remember_login_cookie_lifetime')
608634
->willReturn(1234);
609635

610-
$response = $this->loginController->tryLogin($loginChain, $trustedDomainHelper, 'Jane', $password, $originalUrl);
636+
$response = $this->loginController->tryLogin($loginChain, $trustedDomainHelper, 'Jane', $password, $rememberme, $originalUrl);
611637

612638
$expected = new RedirectResponse($redirectUrl);
613639
$this->assertEquals($expected, $response);
614640
}
615641

616-
public function testLoginWithValidCredentialsAndRedirectUrl(): void {
642+
#[DataProvider('remembermeProvider')]
643+
public function testLoginWithValidCredentialsAndRedirectUrl(bool $rememberme): void {
617644
$user = 'MyUserName';
618645
$password = 'secret';
619646
$redirectUrl = 'https://next.cloud/apps/mail';
@@ -628,6 +655,7 @@ public function testLoginWithValidCredentialsAndRedirectUrl(): void {
628655
$this->request,
629656
$user,
630657
$password,
658+
$rememberme,
631659
'/apps/mail'
632660
);
633661
$loginResult = LoginResult::success($loginData);
@@ -644,12 +672,13 @@ public function testLoginWithValidCredentialsAndRedirectUrl(): void {
644672
->willReturn($redirectUrl);
645673
$expected = new RedirectResponse($redirectUrl);
646674

647-
$response = $this->loginController->tryLogin($loginChain, $trustedDomainHelper, $user, $password, '/apps/mail');
675+
$response = $this->loginController->tryLogin($loginChain, $trustedDomainHelper, $user, $password, $rememberme, '/apps/mail');
648676

649677
$this->assertEquals($expected, $response);
650678
}
651679

652-
public function testToNotLeakLoginName(): void {
680+
#[DataProvider('remembermeProvider')]
681+
public function testToNotLeakLoginName(bool $rememberme): void {
653682
$loginChain = $this->createMock(LoginChain::class);
654683
$trustedDomainHelper = $this->createMock(ITrustedDomainHelper::class);
655684
$trustedDomainHelper->method('isTrustedUrl')->willReturn(true);
@@ -662,6 +691,7 @@ public function testToNotLeakLoginName(): void {
662691
$this->request,
663692
'john@doe.com',
664693
'just wrong',
694+
$rememberme,
665695
'/apps/files'
666696
);
667697
$loginResult = LoginResult::failure($loginData, LoginController::LOGIN_MSG_INVALIDPASSWORD);
@@ -688,6 +718,7 @@ public function testToNotLeakLoginName(): void {
688718
$trustedDomainHelper,
689719
'john@doe.com',
690720
'just wrong',
721+
$rememberme,
691722
'/apps/files'
692723
);
693724

‎tests/lib/Authentication/Login/ALoginTestCommand.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ protected function getLoggedInLoginDataWithRedirectUrl(): LoginData {
8383
$this->request,
8484
$this->username,
8585
$this->password,
86+
true,
8687
$this->redirectUrl
8788
);
8889
$data->setUser($this->user);
@@ -94,6 +95,7 @@ protected function getLoggedInLoginDataWithTimezone(): LoginData {
9495
$this->request,
9596
$this->username,
9697
$this->password,
98+
true,
9799
null,
98100
$this->timezone,
99101
$this->timeZoneOffset

0 commit comments

Comments
 (0)