mirror of
https://github.com/nextcloud/server.git
synced 2026-04-22 14:50:17 -04:00
fix(2fa-backupcodes): Don't remember disabled and deleted users over and over again
Signed-off-by: Joas Schilling <coding@schilljs.com>
This commit is contained in:
parent
158aedb549
commit
b80dd1f6db
4 changed files with 98 additions and 6 deletions
|
|
@ -58,6 +58,10 @@ class CheckBackupCodes extends QueuedJob {
|
|||
|
||||
protected function run($argument) {
|
||||
$this->userManager->callForSeenUsers(function (IUser $user) {
|
||||
if (!$user->isEnabled()) {
|
||||
return;
|
||||
}
|
||||
|
||||
$providers = $this->registry->getProviderStates($user);
|
||||
$isTwoFactorAuthenticated = $this->twofactorManager->isTwoFactorAuthenticated($user);
|
||||
|
||||
|
|
|
|||
|
|
@ -70,6 +70,7 @@ class RememberBackupCodesJob extends TimedJob {
|
|||
|
||||
if ($user === null) {
|
||||
// We can't run with an invalid user
|
||||
$this->jobList->remove(self::class, $argument);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -98,6 +99,11 @@ class RememberBackupCodesJob extends TimedJob {
|
|||
->setSubject('create_backupcodes');
|
||||
$this->notificationManager->markProcessed($notification);
|
||||
|
||||
if (!$user->isEnabled()) {
|
||||
// Don't recreate a notification for a user that can not read it
|
||||
$this->jobList->remove(self::class, $argument);
|
||||
return;
|
||||
}
|
||||
$notification->setDateTime($date);
|
||||
$this->notificationManager->notify($notification);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -82,6 +82,9 @@ class CheckBackupCodeTest extends TestCase {
|
|||
}
|
||||
|
||||
public function testRunAlreadyGenerated() {
|
||||
$this->user->method('isEnabled')
|
||||
->willReturn(true);
|
||||
|
||||
$this->registry->method('getProviderStates')
|
||||
->with($this->user)
|
||||
->willReturn(['backup_codes' => true]);
|
||||
|
|
@ -97,6 +100,8 @@ class CheckBackupCodeTest extends TestCase {
|
|||
public function testRun() {
|
||||
$this->user->method('getUID')
|
||||
->willReturn('myUID');
|
||||
$this->user->method('isEnabled')
|
||||
->willReturn(true);
|
||||
|
||||
$this->registry->expects($this->once())
|
||||
->method('getProviderStates')
|
||||
|
|
@ -117,7 +122,26 @@ class CheckBackupCodeTest extends TestCase {
|
|||
$this->invokePrivate($this->checkBackupCodes, 'run', [[]]);
|
||||
}
|
||||
|
||||
public function testRunDisabledUser() {
|
||||
$this->user->method('getUID')
|
||||
->willReturn('myUID');
|
||||
$this->user->method('isEnabled')
|
||||
->willReturn(false);
|
||||
|
||||
$this->registry->expects($this->never())
|
||||
->method('getProviderStates')
|
||||
->with($this->user);
|
||||
|
||||
$this->jobList->expects($this->never())
|
||||
->method('add');
|
||||
|
||||
$this->invokePrivate($this->checkBackupCodes, 'run', [[]]);
|
||||
}
|
||||
|
||||
public function testRunNoProviders() {
|
||||
$this->user->method('isEnabled')
|
||||
->willReturn(true);
|
||||
|
||||
$this->registry->expects($this->once())
|
||||
->method('getProviderStates')
|
||||
->with($this->user)
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ use OCP\IUser;
|
|||
use OCP\IUserManager;
|
||||
use OCP\Notification\IManager;
|
||||
use OCP\Notification\INotification;
|
||||
use OCP\Server;
|
||||
use Test\TestCase;
|
||||
|
||||
class RememberBackupCodesJobTest extends TestCase {
|
||||
|
|
@ -82,16 +83,25 @@ class RememberBackupCodesJobTest extends TestCase {
|
|||
|
||||
$this->notificationManager->expects($this->never())
|
||||
->method($this->anything());
|
||||
$this->jobList->expects($this->once())
|
||||
->method('remove')
|
||||
->with(
|
||||
RememberBackupCodesJob::class,
|
||||
['uid' => 'invalidUID']
|
||||
);
|
||||
$this->jobList->expects($this->never())
|
||||
->method($this->anything());
|
||||
->method('add');
|
||||
|
||||
$this->invokePrivate($this->job, 'run', [['uid' => 'invalidUID']]);
|
||||
self::invokePrivate($this->job, 'run', [['uid' => 'invalidUID']]);
|
||||
}
|
||||
|
||||
public function testBackupCodesGenerated() {
|
||||
$user = $this->createMock(IUser::class);
|
||||
$user->method('getUID')
|
||||
->willReturn('validUID');
|
||||
$user->method('isEnabled')
|
||||
->willReturn(true);
|
||||
|
||||
$this->userManager->method('get')
|
||||
->with('validUID')
|
||||
->willReturn($user);
|
||||
|
|
@ -112,7 +122,7 @@ class RememberBackupCodesJobTest extends TestCase {
|
|||
$this->notificationManager->expects($this->never())
|
||||
->method($this->anything());
|
||||
|
||||
$this->invokePrivate($this->job, 'run', [['uid' => 'validUID']]);
|
||||
self::invokePrivate($this->job, 'run', [['uid' => 'validUID']]);
|
||||
}
|
||||
|
||||
public function testNoActiveProvider() {
|
||||
|
|
@ -140,13 +150,15 @@ class RememberBackupCodesJobTest extends TestCase {
|
|||
$this->notificationManager->expects($this->never())
|
||||
->method($this->anything());
|
||||
|
||||
$this->invokePrivate($this->job, 'run', [['uid' => 'validUID']]);
|
||||
self::invokePrivate($this->job, 'run', [['uid' => 'validUID']]);
|
||||
}
|
||||
|
||||
public function testNotificationSend() {
|
||||
$user = $this->createMock(IUser::class);
|
||||
$user->method('getUID')
|
||||
->willReturn('validUID');
|
||||
$user->method('isEnabled')
|
||||
->willReturn(true);
|
||||
$this->userManager->method('get')
|
||||
->with('validUID')
|
||||
->willReturn($user);
|
||||
|
|
@ -165,7 +177,7 @@ class RememberBackupCodesJobTest extends TestCase {
|
|||
$date->setTimestamp($this->time->getTime());
|
||||
|
||||
$this->notificationManager->method('createNotification')
|
||||
->willReturn(\OC::$server->query(IManager::class)->createNotification());
|
||||
->willReturn(Server::get(IManager::class)->createNotification());
|
||||
|
||||
$this->notificationManager->expects($this->once())
|
||||
->method('notify')
|
||||
|
|
@ -178,6 +190,52 @@ class RememberBackupCodesJobTest extends TestCase {
|
|||
$n->getSubject() === 'create_backupcodes';
|
||||
}));
|
||||
|
||||
$this->invokePrivate($this->job, 'run', [['uid' => 'validUID']]);
|
||||
self::invokePrivate($this->job, 'run', [['uid' => 'validUID']]);
|
||||
}
|
||||
|
||||
public function testNotificationNotSendForDisabledUser() {
|
||||
$user = $this->createMock(IUser::class);
|
||||
$user->method('getUID')
|
||||
->willReturn('validUID');
|
||||
$user->method('isEnabled')
|
||||
->willReturn(false);
|
||||
$this->userManager->method('get')
|
||||
->with('validUID')
|
||||
->willReturn($user);
|
||||
|
||||
$this->registry->method('getProviderStates')
|
||||
->with($user)
|
||||
->willReturn([
|
||||
'backup_codes' => false,
|
||||
'foo' => true,
|
||||
]);
|
||||
|
||||
$this->jobList->expects($this->once())
|
||||
->method('remove')
|
||||
->with(
|
||||
RememberBackupCodesJob::class,
|
||||
['uid' => 'validUID']
|
||||
);
|
||||
|
||||
$date = new \DateTime();
|
||||
$date->setTimestamp($this->time->getTime());
|
||||
|
||||
$this->notificationManager->method('createNotification')
|
||||
->willReturn(Server::get(IManager::class)->createNotification());
|
||||
|
||||
$this->notificationManager->expects($this->once())
|
||||
->method('markProcessed')
|
||||
->with($this->callback(function (INotification $n) {
|
||||
return $n->getApp() === 'twofactor_backupcodes' &&
|
||||
$n->getUser() === 'validUID' &&
|
||||
$n->getObjectType() === 'create' &&
|
||||
$n->getObjectId() === 'codes' &&
|
||||
$n->getSubject() === 'create_backupcodes';
|
||||
}));
|
||||
|
||||
$this->notificationManager->expects($this->never())
|
||||
->method('notify');
|
||||
|
||||
self::invokePrivate($this->job, 'run', [['uid' => 'validUID']]);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue