Merge pull request #3078 from nextcloud/2fa-backup-codes-entropy

Increase 2fa backup codes entropy
This commit is contained in:
Lukas Reschke 2017-01-18 16:08:59 +01:00 committed by GitHub
commit 29d2ca5991
2 changed files with 28 additions and 20 deletions

View File

@ -33,6 +33,8 @@ use OCP\Security\ISecureRandom;
class BackupCodeStorage { class BackupCodeStorage {
private static $CODE_LENGTH = 16;
/** @var BackupCodeMapper */ /** @var BackupCodeMapper */
private $mapper; private $mapper;
@ -48,6 +50,13 @@ class BackupCodeStorage {
/** @var ILogger */ /** @var ILogger */
private $logger; private $logger;
/**
* @param BackupCodeMapper $mapper
* @param ISecureRandom $random
* @param IHasher $hasher
* @param IManager $activityManager
* @param ILogger $logger
*/
public function __construct(BackupCodeMapper $mapper, ISecureRandom $random, IHasher $hasher, public function __construct(BackupCodeMapper $mapper, ISecureRandom $random, IHasher $hasher,
IManager $activityManager, ILogger $logger) { IManager $activityManager, ILogger $logger) {
$this->mapper = $mapper; $this->mapper = $mapper;
@ -69,7 +78,7 @@ class BackupCodeStorage {
$uid = $user->getUID(); $uid = $user->getUID();
foreach (range(1, min([$number, 20])) as $i) { foreach (range(1, min([$number, 20])) as $i) {
$code = $this->random->generate(10, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'); $code = $this->random->generate(self::$CODE_LENGTH, ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS);
$dbCode = new BackupCode(); $dbCode = new BackupCode();
$dbCode->setUserId($uid); $dbCode->setUserId($uid);

View File

@ -31,23 +31,24 @@ use OCP\ILogger;
use OCP\IUser; use OCP\IUser;
use OCP\Security\IHasher; use OCP\Security\IHasher;
use OCP\Security\ISecureRandom; use OCP\Security\ISecureRandom;
use PHPUnit_Framework_MockObject_MockObject;
use Test\TestCase; use Test\TestCase;
class BackupCodeStorageTest extends TestCase { class BackupCodeStorageTest extends TestCase {
/** @var BackupCodeMapper|\PHPUnit_Framework_MockObject_MockObject */ /** @var BackupCodeMapper|PHPUnit_Framework_MockObject_MockObject */
private $mapper; private $mapper;
/** @var ISecureRandom|\PHPUnit_Framework_MockObject_MockObject */ /** @var ISecureRandom|PHPUnit_Framework_MockObject_MockObject */
private $random; private $random;
/** @var IHasher|\PHPUnit_Framework_MockObject_MockObject */ /** @var IHasher|PHPUnit_Framework_MockObject_MockObject */
private $hasher; private $hasher;
/** @var IManager|\PHPUnit_Framework_MockObject_MockObject */ /** @var IManager|PHPUnit_Framework_MockObject_MockObject */
private $activityManager; private $activityManager;
/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ /** @var ILogger|PHPUnit_Framework_MockObject_MockObject */
private $logger; private $logger;
/** @var BackupCodeStorage */ /** @var BackupCodeStorage */
@ -56,11 +57,9 @@ class BackupCodeStorageTest extends TestCase {
protected function setUp() { protected function setUp() {
parent::setUp(); parent::setUp();
$this->mapper = $this->getMockBuilder(BackupCodeMapper::class) $this->mapper = $this->createMock(BackupCodeMapper::class);
->disableOriginalConstructor() $this->random = $this->createMock(ISecureRandom::class);
->getMock(); $this->hasher = $this->createMock(IHasher::class);
$this->random = $this->getMockBuilder(ISecureRandom::class)->getMock();
$this->hasher = $this->getMockBuilder(IHasher::class)->getMock();
$this->activityManager = $this->createMock(IManager::class); $this->activityManager = $this->createMock(IManager::class);
$this->logger = $this->createMock(ILogger::class); $this->logger = $this->createMock(ILogger::class);
@ -68,7 +67,7 @@ class BackupCodeStorageTest extends TestCase {
} }
public function testCreateCodes() { public function testCreateCodes() {
$user = $this->getMockBuilder(IUser::class)->getMock(); $user = $this->createMock(IUser::class);
$number = 5; $number = 5;
$event = $this->createMock(IEvent::class); $event = $this->createMock(IEvent::class);
@ -77,7 +76,7 @@ class BackupCodeStorageTest extends TestCase {
->will($this->returnValue('fritz')); ->will($this->returnValue('fritz'));
$this->random->expects($this->exactly($number)) $this->random->expects($this->exactly($number))
->method('generate') ->method('generate')
->with(10, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789') ->with(16, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789')
->will($this->returnValue('CODEABCDEF')); ->will($this->returnValue('CODEABCDEF'));
$this->hasher->expects($this->exactly($number)) $this->hasher->expects($this->exactly($number))
->method('hash') ->method('hash')
@ -121,7 +120,7 @@ class BackupCodeStorageTest extends TestCase {
} }
public function testHasBackupCodes() { public function testHasBackupCodes() {
$user = $this->getMockBuilder(IUser::class)->getMock(); $user = $this->createMock(IUser::class);
$codes = [ $codes = [
new BackupCode(), new BackupCode(),
new BackupCode(), new BackupCode(),
@ -136,7 +135,7 @@ class BackupCodeStorageTest extends TestCase {
} }
public function testHasBackupCodesNoCodes() { public function testHasBackupCodesNoCodes() {
$user = $this->getMockBuilder(IUser::class)->getMock(); $user = $this->createMock(IUser::class);
$codes = []; $codes = [];
$this->mapper->expects($this->once()) $this->mapper->expects($this->once())
@ -148,7 +147,7 @@ class BackupCodeStorageTest extends TestCase {
} }
public function testGetBackupCodeState() { public function testGetBackupCodeState() {
$user = $this->getMockBuilder(IUser::class)->getMock(); $user = $this->createMock(IUser::class);
$code1 = new BackupCode(); $code1 = new BackupCode();
$code1->setUsed(1); $code1->setUsed(1);
@ -173,7 +172,7 @@ class BackupCodeStorageTest extends TestCase {
} }
public function testGetBackupCodeDisabled() { public function testGetBackupCodeDisabled() {
$user = $this->getMockBuilder(IUser::class)->getMock(); $user = $this->createMock(IUser::class);
$codes = []; $codes = [];
@ -191,7 +190,7 @@ class BackupCodeStorageTest extends TestCase {
} }
public function testValidateCode() { public function testValidateCode() {
$user = $this->getMockBuilder(IUser::class)->getMock(); $user = $this->createMock(IUser::class);
$code = new BackupCode(); $code = new BackupCode();
$code->setUsed(0); $code->setUsed(0);
$code->setCode('HASHEDVALUE'); $code->setCode('HASHEDVALUE');
@ -217,7 +216,7 @@ class BackupCodeStorageTest extends TestCase {
} }
public function testValidateUsedCode() { public function testValidateUsedCode() {
$user = $this->getMockBuilder(IUser::class)->getMock(); $user = $this->createMock(IUser::class);
$code = new BackupCode(); $code = new BackupCode();
$code->setUsed('1'); $code->setUsed('1');
$code->setCode('HASHEDVALUE'); $code->setCode('HASHEDVALUE');
@ -238,7 +237,7 @@ class BackupCodeStorageTest extends TestCase {
} }
public function testValidateCodeWithWrongHash() { public function testValidateCodeWithWrongHash() {
$user = $this->getMockBuilder(IUser::class)->getMock(); $user = $this->createMock(IUser::class);
$code = new BackupCode(); $code = new BackupCode();
$code->setUsed(0); $code->setUsed(0);
$code->setCode('HASHEDVALUE'); $code->setCode('HASHEDVALUE');