From 150e58880e2df492f458745d43edfe88bc8978db Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 29 Jul 2020 18:06:05 +0200 Subject: [PATCH] Fix other tests Signed-off-by: Joas Schilling --- lib/Controller/RegisterController.php | 1 - lib/Service/RegistrationService.php | 20 +-- .../Controller/RegisterControllerTest.php | 138 ------------------ .../Service/RegistrationServiceTest.php | 39 +++-- tests/Unit/Controller/ApiControllerTest.php | 4 + .../Controller/RegisterControllerTest.php | 7 + 6 files changed, 45 insertions(+), 164 deletions(-) delete mode 100644 tests/Integration/Controller/RegisterControllerTest.php diff --git a/lib/Controller/RegisterController.php b/lib/Controller/RegisterController.php index 4a4e167..cc5ea0e 100644 --- a/lib/Controller/RegisterController.php +++ b/lib/Controller/RegisterController.php @@ -34,7 +34,6 @@ use OCP\IL10N; use OCP\IRequest; use OCP\IURLGenerator; use OCP\IConfig; -use OCP\IL10N; class RegisterController extends Controller { diff --git a/lib/Service/RegistrationService.php b/lib/Service/RegistrationService.php index a5c4c80..26bf83b 100644 --- a/lib/Service/RegistrationService.php +++ b/lib/Service/RegistrationService.php @@ -215,14 +215,14 @@ class RegistrationService { * @return bool */ public function checkAllowedDomains(string $email): bool { - $allowed_domains = $this->config->getAppValue($this->appName, 'allowed_domains', ''); - if ($allowed_domains !== '') { - $allowed_domains = explode(';', $allowed_domains); + $allowedDomains = $this->config->getAppValue($this->appName, 'allowed_domains', ''); + if ($allowedDomains !== '') { + $allowedDomains = explode(';', $allowedDomains); $allowed = false; - foreach ($allowed_domains as $domain) { - $maildomain = explode("@", $email)[1]; - // valid domain, everythings fine - if ($maildomain === $domain) { + foreach ($allowedDomains as $domain) { + [,$mailDomain] = explode('@', $email, 2); + // valid domain, everything's fine + if ($mailDomain === $domain) { $allowed = true; break; } @@ -236,9 +236,9 @@ class RegistrationService { * @return string[] */ public function getAllowedDomains(): array { - $allowed_domains = $this->config->getAppValue($this->appName, 'allowed_domains', ''); - $allowed_domains = explode(';', $allowed_domains); - return $allowed_domains; + $allowedDomains = $this->config->getAppValue($this->appName, 'allowed_domains', ''); + $allowedDomains = explode(';', $allowedDomains); + return $allowedDomains; } /** diff --git a/tests/Integration/Controller/RegisterControllerTest.php b/tests/Integration/Controller/RegisterControllerTest.php deleted file mode 100644 index b3bb45b..0000000 --- a/tests/Integration/Controller/RegisterControllerTest.php +++ /dev/null @@ -1,138 +0,0 @@ -mailService = $this->createMock(MailService::class); - $this->l10n = $this->createMock(IL10N::class); - $this->urlGenerator = $this->createMock(IURLGenerator::class); - #$this->userManager = $this->createMock(IUserManager::class); - $this->userManager = \OC::$server->getUserManager(); - $this->config = $this->createMock(IConfig::class); - $this->groupManager = \OC::$server->getGroupManager(); - $this->random = \OC::$server->getSecureRandom(); - $this->usersession = $this->createMock(IUserSession::class); - $this->request = $this->createMock(IRequest::class); - $this->logger = $this->createMock(ILogger::class); - $this->session = $this->createMock(ISession::class); - $this->tokenProvider = $this->createMock(IProvider::class); - $this->crypto = $this->createMock(ICrypto::class); - - $this->registrationMapper = new RegistrationMapper( - \OC::$server->getDatabaseConnection(), - $this->random - ); - - $this->registrationService = new RegistrationService( - 'registration', - $this->mailService, - $this->l10n, - $this->urlGenerator, - $this->registrationMapper, - $this->userManager, - $this->config, - $this->groupManager, - $this->random, - $this->usersession, - $this->request, - $this->logger, - $this->session, - $this->tokenProvider, - $this->crypto - ); - - $this->controller = new RegisterController( - 'registration', - $this->request, - $this->l10n, - $this->urlGenerator, - $this->config, - $this->registrationService, - $this->mailService - ); - } - - public function testValidateEmailNormal() { - $email = 'aaaa@example.com'; - - $this->config->expects($this->atLeastOnce()) - ->method('getAppValue') - ->with("registration", 'allowed_domains', '') - ->willReturn(''); - $this->mailService->expects($this->once()) - ->method('sendTokenByMail'); - - $this->assertEquals($this->registrationService->validateEmail($email), true); - - $ret = $this->controller->validateEmail($email); - - $expected = new TemplateResponse('registration', 'message', ['msg' => - $this->l10n->t('Verification email successfully sent.') - ], 'guest'); - - - $this->assertEquals($expected, $ret, print_r($ret, true)); - } -} diff --git a/tests/Integration/Service/RegistrationServiceTest.php b/tests/Integration/Service/RegistrationServiceTest.php index 0ab0aaa..4130ffd 100644 --- a/tests/Integration/Service/RegistrationServiceTest.php +++ b/tests/Integration/Service/RegistrationServiceTest.php @@ -61,6 +61,9 @@ class RegistrationServiceTest extends TestCase { /** @var ICrypto */ private $crypto; + /** @var RegistrationService */ + private $service; + public function setUp(): void { parent::setUp(); $this->mailService = $this->createMock(MailService::class); @@ -107,13 +110,10 @@ class RegistrationServiceTest extends TestCase { $this->config->expects($this->once()) ->method('getAppValue') - ->with("registration", 'allowed_domains', '') + ->with('registration', 'allowed_domains', '') ->willReturn(''); - $ret = $this->service->validateEmail($email); - - //$this->assertInstanceOf(Registration::class, $ret); - $this->assertTrue($ret); + $this->service->validateEmail($email); } public function testValidateNewEmailWithinAllowedDomain() { @@ -121,18 +121,23 @@ class RegistrationServiceTest extends TestCase { $this->config->expects($this->atLeastOnce()) ->method('getAppValue') - ->with("registration", 'allowed_domains', '') + ->with('registration', 'allowed_domains', '') ->willReturn('example.com'); - $ret = $this->service->validateEmail($email); - $this->assertTrue($ret, print_r($ret, true)); + $this->service->validateEmail($email); } + /** * @depends testValidateNewEmailWithinAllowedDomain */ public function testValidateNewEmailNotWithinAllowedDomain() { $email2 = 'bbbb@gmail.com'; + $this->config->expects($this->atLeastOnce()) + ->method('getAppValue') + ->with('registration', 'allowed_domains', '') + ->willReturn('example.com'); + $this->expectException(RegistrationException::class); $this->service->validateEmail($email2); } @@ -143,18 +148,24 @@ class RegistrationServiceTest extends TestCase { $this->config->expects($this->atLeastOnce()) ->method('getAppValue') - ->with("registration", 'allowed_domains', '') + ->with('registration', 'allowed_domains', '') ->willReturn('example.com;gmail.com'); - $this->assertTrue($this->service->validateEmail($email)); - $this->assertTrue($this->service->validateEmail($email2)); + $this->service->validateEmail($email); + $this->service->validateEmail($email2); } + /** * @depends testValidateNewEmailWithinMultipleAllowedDomain */ public function testValidateNewEmailNotWithinMultipleAllowedDomain() { $email2 = 'cccc@yahoo.com'; + $this->config->expects($this->atLeastOnce()) + ->method('getAppValue') + ->with('registration', 'allowed_domains', '') + ->willReturn('example.com;gmail.com'); + $this->expectException(RegistrationException::class); $this->service->validateEmail($email2); } @@ -180,10 +191,8 @@ class RegistrationServiceTest extends TestCase { $email = 'aaaa@example.com'; $this->service->createRegistration($email, 'alice'); - $ret = $this->service->validateEmail($email); - - $this->assertInstanceOf(Registration::class, $ret); - $this->assertEquals($email, $ret->getEmail()); + $this->expectException(RegistrationException::class); + $this->service->validateEmail($email); } public function testCreateAccountWebForm() { diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 2701f42..e37d46a 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -157,7 +157,11 @@ class ApiControllerTest extends TestCase { $registration = new Registration(); $registration->setEmailConfirmed(true); $registration->setClientSecret('mysecret'); + $registration->setUsername('user'); + $registration->setPassword('password'); $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn('user'); $this->registrationService ->method('getRegistrationForSecret') ->with('mysecret') diff --git a/tests/Unit/Controller/RegisterControllerTest.php b/tests/Unit/Controller/RegisterControllerTest.php index f9721f2..896f209 100644 --- a/tests/Unit/Controller/RegisterControllerTest.php +++ b/tests/Unit/Controller/RegisterControllerTest.php @@ -35,6 +35,7 @@ use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\RedirectToDefaultAppResponse; use OCP\AppFramework\Http\StandaloneTemplateResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; use OCP\IURLGenerator; @@ -50,6 +51,8 @@ class RegisterControllerTest extends TestCase { private $l10n; /** @var IURLGenerator|MockObject */ private $urlGenerator; + /** @var IConfig|MockObject */ + private $config; /** @var RegistrationService|MockObject */ private $registrationService; /** @var LoginFlowService|MockObject */ @@ -62,6 +65,7 @@ class RegisterControllerTest extends TestCase { $this->request = $this->createMock(IRequest::class); $this->l10n = $this->createMock(IL10N::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->config = $this->createMock(IConfig::class); $this->registrationService = $this->createMock(RegistrationService::class); $this->loginFlowService = $this->createMock(LoginFlowService::class); $this->mailService = $this->createMock(MailService::class); @@ -84,6 +88,7 @@ class RegisterControllerTest extends TestCase { $this->request, $this->l10n, $this->urlGenerator, + $this->config, $this->registrationService, $this->loginFlowService, $this->mailService @@ -97,6 +102,7 @@ class RegisterControllerTest extends TestCase { $this->request, $this->l10n, $this->urlGenerator, + $this->config, $this->registrationService, $this->loginFlowService, $this->mailService, @@ -446,6 +452,7 @@ class RegisterControllerTest extends TestCase { self::assertSame([ 'email' => $email, + 'email_is_login' => false, 'username' => $username, 'message' => $message, ], $response->getParams());