Merge pull request #13595 from nextcloud/enh/do_not_leak_user_existence
Generic message on password reset
This commit is contained in:
commit
a32577d048
|
@ -39,6 +39,7 @@ use OCP\AppFramework\Utility\ITimeFactory;
|
||||||
use OCP\Defaults;
|
use OCP\Defaults;
|
||||||
use OCP\Encryption\IEncryptionModule;
|
use OCP\Encryption\IEncryptionModule;
|
||||||
use OCP\Encryption\IManager;
|
use OCP\Encryption\IManager;
|
||||||
|
use OCP\ILogger;
|
||||||
use \OCP\IURLGenerator;
|
use \OCP\IURLGenerator;
|
||||||
use \OCP\IRequest;
|
use \OCP\IRequest;
|
||||||
use \OCP\IL10N;
|
use \OCP\IL10N;
|
||||||
|
@ -80,6 +81,8 @@ class LostController extends Controller {
|
||||||
protected $timeFactory;
|
protected $timeFactory;
|
||||||
/** @var ICrypto */
|
/** @var ICrypto */
|
||||||
protected $crypto;
|
protected $crypto;
|
||||||
|
/** @var ILogger */
|
||||||
|
private $logger;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param string $appName
|
* @param string $appName
|
||||||
|
@ -108,7 +111,8 @@ class LostController extends Controller {
|
||||||
IManager $encryptionManager,
|
IManager $encryptionManager,
|
||||||
IMailer $mailer,
|
IMailer $mailer,
|
||||||
ITimeFactory $timeFactory,
|
ITimeFactory $timeFactory,
|
||||||
ICrypto $crypto) {
|
ICrypto $crypto,
|
||||||
|
ILogger $logger) {
|
||||||
parent::__construct($appName, $request);
|
parent::__construct($appName, $request);
|
||||||
$this->urlGenerator = $urlGenerator;
|
$this->urlGenerator = $urlGenerator;
|
||||||
$this->userManager = $userManager;
|
$this->userManager = $userManager;
|
||||||
|
@ -121,6 +125,7 @@ class LostController extends Controller {
|
||||||
$this->mailer = $mailer;
|
$this->mailer = $mailer;
|
||||||
$this->timeFactory = $timeFactory;
|
$this->timeFactory = $timeFactory;
|
||||||
$this->crypto = $crypto;
|
$this->crypto = $crypto;
|
||||||
|
$this->logger = $logger;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -236,10 +241,11 @@ class LostController extends Controller {
|
||||||
// FIXME: use HTTP error codes
|
// FIXME: use HTTP error codes
|
||||||
try {
|
try {
|
||||||
$this->sendEmail($user);
|
$this->sendEmail($user);
|
||||||
} catch (\Exception $e){
|
} catch (\Exception $e) {
|
||||||
$response = new JSONResponse($this->error($e->getMessage()));
|
// Ignore the error since we do not want to leak this info
|
||||||
$response->throttle();
|
$this->logger->logException($e, [
|
||||||
return $response;
|
'level' => ILogger::WARN
|
||||||
|
]);
|
||||||
}
|
}
|
||||||
|
|
||||||
$response = new JSONResponse($this->success());
|
$response = new JSONResponse($this->success());
|
||||||
|
|
|
@ -2,7 +2,7 @@
|
||||||
OC.Lostpassword = {
|
OC.Lostpassword = {
|
||||||
sendErrorMsg : t('core', 'Couldn\'t send reset email. Please contact your administrator.'),
|
sendErrorMsg : t('core', 'Couldn\'t send reset email. Please contact your administrator.'),
|
||||||
|
|
||||||
sendSuccessMsg : t('core', 'The link to reset your password has been sent to your email. If you do not receive it within a reasonable amount of time, check your spam/junk folders.<br>If it is not there ask your local administrator.'),
|
sendSuccessMsg : t('core', 'We have send a password reset e-mail to the e-mail address known to us for this account. If you do not receive it within a reasonable amount of time, check your spam/junk folders.<br>If it is not there ask your local administrator.'),
|
||||||
|
|
||||||
encryptedMsg : t('core', "Your files are encrypted. There will be no way to get your data back after your password is reset.<br />If you are not sure what to do, please contact your administrator before you continue. <br />Do you really want to continue?")
|
encryptedMsg : t('core', "Your files are encrypted. There will be no way to get your data back after your password is reset.<br />If you are not sure what to do, please contact your administrator before you continue. <br />Do you really want to continue?")
|
||||||
+ ('<br /><input type="checkbox" id="encrypted-continue" class="checkbox checkbox--white" value="Yes" />')
|
+ ('<br /><input type="checkbox" id="encrypted-continue" class="checkbox checkbox--white" value="Yes" />')
|
||||||
|
|
|
@ -31,6 +31,7 @@ use OCP\Encryption\IEncryptionModule;
|
||||||
use OCP\Encryption\IManager;
|
use OCP\Encryption\IManager;
|
||||||
use OCP\IConfig;
|
use OCP\IConfig;
|
||||||
use OCP\IL10N;
|
use OCP\IL10N;
|
||||||
|
use OCP\ILogger;
|
||||||
use OCP\IRequest;
|
use OCP\IRequest;
|
||||||
use OCP\IURLGenerator;
|
use OCP\IURLGenerator;
|
||||||
use OCP\IUser;
|
use OCP\IUser;
|
||||||
|
@ -74,6 +75,8 @@ class LostControllerTest extends \Test\TestCase {
|
||||||
private $request;
|
private $request;
|
||||||
/** @var ICrypto|\PHPUnit_Framework_MockObject_MockObject */
|
/** @var ICrypto|\PHPUnit_Framework_MockObject_MockObject */
|
||||||
private $crypto;
|
private $crypto;
|
||||||
|
/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
|
||||||
|
private $logger;
|
||||||
|
|
||||||
protected function setUp() {
|
protected function setUp() {
|
||||||
parent::setUp();
|
parent::setUp();
|
||||||
|
@ -124,6 +127,7 @@ class LostControllerTest extends \Test\TestCase {
|
||||||
->method('isEnabled')
|
->method('isEnabled')
|
||||||
->willReturn(true);
|
->willReturn(true);
|
||||||
$this->crypto = $this->createMock(ICrypto::class);
|
$this->crypto = $this->createMock(ICrypto::class);
|
||||||
|
$this->logger = $this->createMock(ILogger::class);
|
||||||
$this->lostController = new LostController(
|
$this->lostController = new LostController(
|
||||||
'Core',
|
'Core',
|
||||||
$this->request,
|
$this->request,
|
||||||
|
@ -137,7 +141,8 @@ class LostControllerTest extends \Test\TestCase {
|
||||||
$this->encryptionManager,
|
$this->encryptionManager,
|
||||||
$this->mailer,
|
$this->mailer,
|
||||||
$this->timeFactory,
|
$this->timeFactory,
|
||||||
$this->crypto
|
$this->crypto,
|
||||||
|
$this->logger
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -265,6 +270,9 @@ class LostControllerTest extends \Test\TestCase {
|
||||||
array(false, $nonExistingUser)
|
array(false, $nonExistingUser)
|
||||||
)));
|
)));
|
||||||
|
|
||||||
|
$this->logger->expects($this->exactly(2))
|
||||||
|
->method('logException');
|
||||||
|
|
||||||
$this->userManager
|
$this->userManager
|
||||||
->method('getByEmail')
|
->method('getByEmail')
|
||||||
->willReturn([]);
|
->willReturn([]);
|
||||||
|
@ -272,8 +280,7 @@ class LostControllerTest extends \Test\TestCase {
|
||||||
// With a non existing user
|
// With a non existing user
|
||||||
$response = $this->lostController->email($nonExistingUser);
|
$response = $this->lostController->email($nonExistingUser);
|
||||||
$expectedResponse = new JSONResponse([
|
$expectedResponse = new JSONResponse([
|
||||||
'status' => 'error',
|
'status' => 'success',
|
||||||
'msg' => 'Couldn\'t send reset email. Please make sure your username is correct.'
|
|
||||||
]);
|
]);
|
||||||
$expectedResponse->throttle();
|
$expectedResponse->throttle();
|
||||||
$this->assertEquals($expectedResponse, $response);
|
$this->assertEquals($expectedResponse, $response);
|
||||||
|
@ -286,8 +293,7 @@ class LostControllerTest extends \Test\TestCase {
|
||||||
->will($this->returnValue(null));
|
->will($this->returnValue(null));
|
||||||
$response = $this->lostController->email($existingUser);
|
$response = $this->lostController->email($existingUser);
|
||||||
$expectedResponse = new JSONResponse([
|
$expectedResponse = new JSONResponse([
|
||||||
'status' => 'error',
|
'status' => 'success',
|
||||||
'msg' => 'Couldn\'t send reset email. Please make sure your username is correct.'
|
|
||||||
]);
|
]);
|
||||||
$expectedResponse->throttle();
|
$expectedResponse->throttle();
|
||||||
$this->assertEquals($expectedResponse, $response);
|
$this->assertEquals($expectedResponse, $response);
|
||||||
|
@ -511,8 +517,11 @@ class LostControllerTest extends \Test\TestCase {
|
||||||
$this->equalTo('test@example.comSECRET')
|
$this->equalTo('test@example.comSECRET')
|
||||||
)->willReturn('encryptedToken');
|
)->willReturn('encryptedToken');
|
||||||
|
|
||||||
|
$this->logger->expects($this->exactly(1))
|
||||||
|
->method('logException');
|
||||||
|
|
||||||
$response = $this->lostController->email('ExistingUser');
|
$response = $this->lostController->email('ExistingUser');
|
||||||
$expectedResponse = new JSONResponse(['status' => 'error', 'msg' => 'Couldn\'t send reset email. Please contact your administrator.']);
|
$expectedResponse = new JSONResponse(['status' => 'success']);
|
||||||
$expectedResponse->throttle();
|
$expectedResponse->throttle();
|
||||||
$this->assertEquals($expectedResponse, $response);
|
$this->assertEquals($expectedResponse, $response);
|
||||||
}
|
}
|
||||||
|
@ -708,8 +717,11 @@ class LostControllerTest extends \Test\TestCase {
|
||||||
->with('ExistingUser')
|
->with('ExistingUser')
|
||||||
->willReturn($user);
|
->willReturn($user);
|
||||||
|
|
||||||
|
$this->logger->expects($this->exactly(1))
|
||||||
|
->method('logException');
|
||||||
|
|
||||||
$response = $this->lostController->email('ExistingUser');
|
$response = $this->lostController->email('ExistingUser');
|
||||||
$expectedResponse = new JSONResponse(['status' => 'error', 'msg' => 'Could not send reset email because there is no email address for this username. Please contact your administrator.']);
|
$expectedResponse = new JSONResponse(['status' => 'success']);
|
||||||
$expectedResponse->throttle();
|
$expectedResponse->throttle();
|
||||||
$this->assertEquals($expectedResponse, $response);
|
$this->assertEquals($expectedResponse, $response);
|
||||||
}
|
}
|
||||||
|
@ -790,12 +802,14 @@ class LostControllerTest extends \Test\TestCase {
|
||||||
->method('getByEmail')
|
->method('getByEmail')
|
||||||
->willReturn([$user1, $user2]);
|
->willReturn([$user1, $user2]);
|
||||||
|
|
||||||
|
$this->logger->expects($this->exactly(1))
|
||||||
|
->method('logException');
|
||||||
|
|
||||||
// request password reset for test@example.com
|
// request password reset for test@example.com
|
||||||
$response = $this->lostController->email('test@example.com');
|
$response = $this->lostController->email('test@example.com');
|
||||||
|
|
||||||
$expectedResponse = new JSONResponse([
|
$expectedResponse = new JSONResponse([
|
||||||
'status' => 'error',
|
'status' => 'success'
|
||||||
'msg' => 'Couldn\'t send reset email. Please make sure your username is correct.'
|
|
||||||
]);
|
]);
|
||||||
$expectedResponse->throttle();
|
$expectedResponse->throttle();
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue