Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

Commit

Permalink
Improve & fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@tvdijen
tvdijen committed Oct 23, 2023
1 parent 3d25ef1 commit 044c30b
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 68 deletions.
18 changes: 9 additions & 9 deletions src/Controller/OTP.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use SimpleSAML\{Auth, Configuration, Error, Logger, Module, Session, Utils};
use SimpleSAML\Module\cmdotcom\Utils\OTPClient;
use SimpleSAML\XHTML\Template;
use Symfony\Component\HttpFoundation\{RedirectResponse, Request, StreamedResponse};
use Symfony\Component\HttpFoundation\{RedirectResponse, Request};
use UnexpectedValueException;

/**
Expand Down Expand Up @@ -119,9 +119,9 @@ public function enterCode(Request $request): Template
/**
* Process the entered validation code.
*
* @return \Symfony\Component\HttpFoundation\StreamedResponse
* @return \Symfony\Component\HttpFoundation\Response|\Symfony\Component\HttpFoundation\RedirectResponse
*/
public function validateCode(Request $request): StreamedResponse
public function validateCode(Request $request): Response|RedirectResponse

Check failure on line 124 in src/Controller/OTP.php

View workflow job for this annotation

GitHub Actions / Quality control

UndefinedClass 

src/Controller/OTP.php:124:53: UndefinedClass: Class, interface or enum named SimpleSAML\Module\cmdotcom\Controller\Response does not exist (see https://psalm.dev/019)
{
$id = $request->query->get('AuthState', null);
if ($id === null) {
Expand Down Expand Up @@ -151,7 +151,7 @@ public function validateCode(Request $request): StreamedResponse
$url = Module::getModuleURL('cmdotcom/promptResend');

$this->logger::info("Code for message ID " . $reference . " has expired.");
return new StreamedResponse([$this->httpUtils, 'redirectTrustedURL'], [$url, ['AuthState' => $id]]);
return $this->httpUtils->redirectTrustedURL($url, ['AuthState' => $id]);
}

$otpClient = new OTPClient($this->config);
Expand All @@ -167,15 +167,15 @@ public function validateCode(Request $request): StreamedResponse
$session->setData('bool', 'cmdotcom:authenticated', true);
$session->save();

return new StreamedResponse([Auth\ProcessingChain::class, 'resumeProcessing'], [$state]);
return Auth\ProcessingChain::class::resumeProcessing($state);
} else {
$this->logger::warning("Code for message ID " . $reference . " failed verification!");
$state['cmdotcom:invalid'] = true;

$id = Auth\State::saveState($state, 'cmdotcom:request');
$url = Module::getModuleURL('cmdotcom/enterCode');

return new StreamedResponse([$this->httpUtils, 'redirectTrustedURL'], [$url, ['AuthState' => $id]]);
return $this->httpUtils->redirectTrustedURL($url, ['AuthState' => $id]);
}
}

Expand Down Expand Up @@ -216,9 +216,9 @@ public function promptResend(Request $request): Template
/**
* Send an SMS and redirect to either the validation page or the resend-prompt
*
* @return \Symfony\Component\HttpFoundation\StreamedResponse
* @return \Symfony\Component\HttpFoundation\RedirectResponse
*/
public function sendCode(Request $request): StreamedResponse
public function sendCode(Request $request): RedirectResponse
{
$id = $request->query->get('AuthState', null);
if ($id === null) {
Expand Down Expand Up @@ -264,6 +264,6 @@ public function sendCode(Request $request): StreamedResponse
$url = Module::getModuleURL('cmdotcom/promptResend');
}

return new StreamedResponse([$this->httpUtils, 'redirectTrustedURL'], [$url, ['AuthState' => $id]]);
return $this->httpUtils->redirectTrustedURL($url, ['AuthState' => $id]);
}
}
98 changes: 46 additions & 52 deletions tests/src/Controller/OTPTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use SimpleSAML\Module\cmdotcom\Controller;
use SimpleSAML\Module\cmdotcom\Utils\TextMessage as TextUtils;
use SimpleSAML\XHTML\Template;
use Symfony\Component\HttpFoundation\{Request, StreamedResponse};
use Symfony\Component\HttpFoundation\{Request, RedirectResponse};

/**
* Set of tests for the controllers in the "cmdotcom" module.
Expand All @@ -29,24 +29,34 @@ class OTPTest extends TestCase
public static ?string $phoneNumber = null;

/** @var \SimpleSAML\Configuration */
protected Configuration $config;
protected static Configuration $config;

/** @var \SimpleSAML\Utils\HTTP */
protected Utils\HTTP $httpUtils;
protected static Utils\HTTP $httpUtils;

/** @var \SimpleSAML\Session */
protected Session $session;

/** @var string $otpHash */
protected string $otpHash;
protected static Session $session;


/**
* Set up for each test.
* Set up before class.
*/
protected function setUp(): void
public static function setUpBeforeClass(): void
{
parent::setUp();
parent::setUpBeforeClass();

self::$config = Configuration::loadFromArray(
[
'module.enable' => [
'cmdotcom' => true,
],
],
'[ARRAY]',
'simplesaml',
);

self::$httpUtils = new Utils\HTTP();
self::$session = Session::getSessionFromRequest();

$productToken = getenv('CMDOTCOM_PRODUCT_KEY');
if ($productToken !== false) {
Expand All @@ -59,19 +69,6 @@ protected function setUp(): void
Assert::stringNotEmpty($phoneNumber);
self::$phoneNumber = $phoneNumber;
}

$this->config = Configuration::loadFromArray(
[
'module.enable' => [
'cmdotcom' => true,
],
],
'[ARRAY]',
'simplesaml',
);

$this->session = Session::getSessionFromRequest();
$this->httpUtils = new Utils\HTTP();
}


Expand All @@ -84,7 +81,7 @@ public function testEnterCodeMissingState(): void
'GET',
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$this->expectException(Error\BadRequest::class);
$this->expectExceptionMessage('Missing AuthState parameter.');
Expand All @@ -105,7 +102,7 @@ public function testEnterCode(): void
]
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$c->setAuthState(new class () extends Auth\State {
public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
Expand All @@ -130,7 +127,7 @@ public function testValidateCodeMissingState(): void
'GET',
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$this->expectException(Error\BadRequest::class);
$this->expectExceptionMessage('Missing AuthState parameter.');
Expand All @@ -156,11 +153,11 @@ public function testValidateCodeIncorrect(): void
]
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$c->setHttpUtils($this->httpUtils);
$c->setHttpUtils(self::$httpUtils);
$c->setLogger(new class () extends Logger {
public static function warning(string $str): void
public static function warning(string $string): void
{
// do nothing
}
Expand All @@ -183,17 +180,17 @@ public static function loadState(string $id, string $stage, bool $allowMissing =
});

$response = $c->validateCode($request);
$this->assertInstanceOf(StreamedResponse::class, $response);
$this->assertInstanceOf(RedirectResponse::class, $response);
$this->assertTrue($response->isSuccessful());
$this->assertEquals([$this->httpUtils, 'redirectTrustedURL'], $response->getCallable());
$this->assertEquals('http://localhost/simplesaml/module.php/cmdotcom/enterCode', $response->getArguments()[0]);
$this->assertEquals('http://localhost/simplesaml/module.php/cmdotcom/enterCode', $response->getTargetUrl());
}


/**
*/
public function testValidateCodeExpired(): void
{
$_SERVER['REQUEST_URI'] = '/validateCode?AuthState=someState';
$request = Request::create(
'/validateCode?AuthState=someState',
'POST',
Expand All @@ -202,9 +199,9 @@ public function testValidateCodeExpired(): void
]
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$c->setHttpUtils($this->httpUtils);
$c->setHttpUtils(self::$httpUtils);
$c->setAuthState(new class () extends Auth\State {
public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
{
Expand All @@ -222,12 +219,11 @@ public static function loadState(string $id, string $stage, bool $allowMissing =
});

$response = $c->validateCode($request);
$this->assertInstanceOf(StreamedResponse::class, $response);
$this->assertInstanceOf(RedirectResponse::class, $response);
$this->assertTrue($response->isSuccessful());

Check failure on line 223 in tests/src/Controller/OTPTest.php

View workflow job for this annotation

GitHub Actions / Unit tests, PHP 8.0, ubuntu-latest 

Failed asserting that false is true.

Check failure on line 223 in tests/src/Controller/OTPTest.php

View workflow job for this annotation

GitHub Actions / Unit tests, PHP 8.1, ubuntu-latest 

Failed asserting that false is true.

Check failure on line 223 in tests/src/Controller/OTPTest.php

View workflow job for this annotation

GitHub Actions / Unit tests, PHP 8.2, ubuntu-latest 

Failed asserting that false is true.
$this->assertEquals([$this->httpUtils, 'redirectTrustedURL'], $response->getCallable());
$this->assertEquals(
'http://localhost/simplesaml/module.php/cmdotcom/promptResend',
$response->getArguments()[0]
$response->getTargetUrl()
);
}

Expand All @@ -241,7 +237,7 @@ public function testsendCodeMissingState(): void
'GET',
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$this->expectException(Error\BadRequest::class);
$this->expectExceptionMessage('Missing AuthState parameter.');
Expand All @@ -265,10 +261,10 @@ public function testsendCodeSuccess(): void
[]
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$c->setLogger(new class () extends Logger {
public static function info(string $str): void
public static function info(string $string): void
{
// do nothing
}
Expand All @@ -289,12 +285,11 @@ public static function loadState(string $id, string $stage, bool $allowMissing =
});

$response = $c->sendCode($request);
$this->assertInstanceOf(StreamedResponse::class, $response);
$this->assertInstanceOf(RedirectResponse::class, $response);
$this->assertTrue($response->isSuccessful());
$this->assertEquals([$this->httpUtils, 'redirectTrustedURL'], $response->getCallable());
$this->assertEquals(
'http://localhost/simplesaml/module.php/cmdotcom/enterCode',
$response->getArguments()[0]
$response->getTargetUrl()
);
}

Expand All @@ -309,10 +304,10 @@ public function testsendCodeFailure(): void
[]
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$c->setLogger(new class () extends Logger {
public static function error(string $str): void
public static function error(string $string): void
{
// do nothing
}
Expand All @@ -333,12 +328,11 @@ public static function loadState(string $id, string $stage, bool $allowMissing =
});

$response = $c->sendCode($request);
$this->assertInstanceOf(StreamedResponse::class, $response);
$this->assertInstanceOf(RedirectResponse::class, $response);
$this->assertTrue($response->isSuccessful());
$this->assertEquals([$this->httpUtils, 'redirectTrustedURL'], $response->getCallable());
$this->assertEquals(
'http://localhost/simplesaml/module.php/cmdotcom/promptResend',
$response->getArguments()[0]
$response->getTargetUrl()
);
}

Expand All @@ -352,7 +346,7 @@ public function testPromptResendMissingState(): void
'GET',
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$this->expectException(Error\BadRequest::class);
$this->expectExceptionMessage('Missing AuthState parameter.');
Expand All @@ -373,7 +367,7 @@ public function testPromptResendExpired(): void
]
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$c->setAuthState(new class () extends Auth\State {
public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
Expand Down Expand Up @@ -403,7 +397,7 @@ public function testPromptResendSendFailure(): void
]
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$c->setAuthState(new class () extends Auth\State {
public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
Expand Down Expand Up @@ -433,7 +427,7 @@ public function testPromptResendUnknownReason(): void
]
);

$c = new Controller\OTP($this->config, $this->session);
$c = new Controller\OTP(self::$config, self::$session);

$c->setAuthState(new class () extends Auth\State {
public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
Expand Down
14 changes: 7 additions & 7 deletions tests/src/Utils/PhoneNumberTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@
class PhoneNumberTest extends TestCase
{
/** @var \SimpleSAML\Module\cmdotcom\Utils\PhoneNumber */
private PhoneNumberUtils $phoneNumberUtils;
private static PhoneNumberUtils $phoneNumberUtils;


/**
* Set up for each test.
* Set up before class.
*/
protected function setUp(): void
public static function setUpBeforeClass(): void
{
parent::setUp();
parent::setUpBeforeClass();

$this->phoneNumberUtils = new PhoneNumberUtils();
self::$phoneNumberUtils = new PhoneNumberUtils();
}


Expand All @@ -38,7 +38,7 @@ protected function setUp(): void
*/
public function testValidPhoneNumberIsSanitizedToE164Format(string $input, string $output): void
{
$result = $this->phoneNumberUtils->sanitizePhoneNumber($input);
$result = self::$phoneNumberUtils->sanitizePhoneNumber($input);
$this->assertEquals($result, $output);
}

Expand All @@ -51,7 +51,7 @@ public function testValidPhoneNumberIsSanitizedToE164Format(string $input, strin
public function testInvalidPhoneNumberThrowsAnException(string $input): void
{
$this->expectException(NumberParseException::class);
$this->phoneNumberUtils->sanitizePhoneNumber($input);
self::$phoneNumberUtils->sanitizePhoneNumber($input);
}


Expand Down

0 comments on commit 044c30b

Please sign in to comment.