Skip to content

Commit

Permalink
f
Browse files Browse the repository at this point in the history
  • Loading branch information
johanib committed Dec 11, 2024
1 parent 5170d03 commit e11170e
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 19 deletions.
10 changes: 9 additions & 1 deletion assets/typescript/__test__/AuthenticationPageService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,19 @@ describe('AuthenticationPageService', () => {
throw new Error('Should have started notification request');
}
const spy = jest.spyOn(context.authenticationPageService, 'switchToNoDevice');
// @TODO add testcase for no-trusted-device
successCallback('no-device');
expect(spy).toBeCalled();
});

it('Should show qr when there is no trusted-device cookie', () => {
if (!successCallback || !errorCallback) {
throw new Error('Should have started notification request');
}
const spy = jest.spyOn(context.authenticationPageService, 'switchToNoDevice');
successCallback('no-trusted-device');
expect(spy).toBeCalled();
});

it('Should handle connection errors', () => {
if (!successCallback || !errorCallback) {
throw new Error('Should have started notification request');
Expand Down
2 changes: 1 addition & 1 deletion ci/qa/phpcs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
xsi:noNamespaceSchemaLocation="../../vendor/squizlabs/php_codesniffer/phpcs.xsd">

<arg name="basepath" value="../../"/>
<arg name="cache" value="../../var/cache/.phpcs-cache"/>
<arg name="cache" value=".phpcs-cache"/>
<arg name="colors"/>
<arg name="extensions" value="php"/>

Expand Down
2 changes: 1 addition & 1 deletion ci/qa/phpunit
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
cd $(dirname $0)/../../

printf "\nStart PHPUnit tests\n"
XDEBUG_MODE=coverage ./vendor/bin/phpunit --configuration=ci/qa/phpunit.xml --cache-result-file=../../var/cache/.phpunit-cache --coverage-text
XDEBUG_MODE=coverage ./vendor/bin/phpunit --configuration=ci/qa/phpunit.xml --coverage-text
4 changes: 0 additions & 4 deletions config/packages/framework.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ when@test:
test: true
session:
storage_factory_id: session.storage.factory.mock_file
handler_id: null
cookie_secure: true
cookie_samesite: none
name: MOCKSESSID
cookie_httponly: true
profiler:
collect: false
8 changes: 2 additions & 6 deletions config/services_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,8 @@ services:
arguments:
- '/^Behat UA$/'

# surfnet_gssp.value_store.service:
# class: Surfnet\GsspBundle\Service\ValueStore\InMemoryValueStore
# public: true

surfnet_gssp.value_store.service:
class: Surfnet\Tiqr\Service\FileValueStore
class: Surfnet\Tiqr\Features\Framework\FileValueStore
public: true
arguments:
$filePath: '/var/www/html/var/store.json'
$filePath: '/var/www/html/var/gssp_store.json'
8 changes: 8 additions & 0 deletions src/Controller/AuthenticationNotificationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,14 @@ public function __invoke(Request $request): Response
}

if (!$this->trustedCookieService->isTrustedDevice($cookie, $nameId, $notificationAddress)) {
$this->logger->notice(
sprintf(
'A trusted device cookie is found for notification address "%s" and user "%s", but has signature mismatch',
$notificationAddress,
$nameId
)
);

return $this->generateNotificationResponse('no-trusted-device');
}

Expand Down
17 changes: 15 additions & 2 deletions src/Features/Context/TiqrContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -592,15 +592,17 @@ public function aPushNotificationIsSent(): void

/**
* @When /^push notification is sent with a trusted\-device cookie with address "([^"]*)"$/
* @When /^push notification is sent with a trusted\-device cookie with address "([^"]*)" and cookie userId "([^"]*)"$/
*/
public function aPushNotificationIsSentWithATrustedDevice(string $notificationAddress): void
public function aPushNotificationIsSentWithATrustedDevice(string $notificationAddress, string $cookieUserId = null): void
{
$userId = $this->metadata->identity->identifier;
$cookieUserId = $cookieUserId ?? $userId;

$config = new Configuration('tiqr-trusted-device', 3600, '000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f', 'none');
$cryptoHelper = new HaliteCryptoHelper($config);

$cookieValue = CookieValue::from($userId, $notificationAddress);
$cookieValue = CookieValue::from($cookieUserId, $notificationAddress);
$helper = new CookieHelper($config, $cryptoHelper, new NullLogger());
$cookieName = $helper->buildCookieName($userId, $notificationAddress);

Expand Down Expand Up @@ -677,6 +679,17 @@ public function theLogsShouldSayNoTrustedCookie(string $address): void
);
}

/**
* @Then /^the logs should mention a signature mismatch for address "([^"]*)"$/
*/
public function theLogsShouldMentionSignatureMismatch(string $address): void
{
$userId = $this->metadata->identity->identifier;
$this->logsContain(
'A trusted device cookie is found for notification address "'.$address.'" and user "'.$userId.'", but has signature mismatch'
);
}

/**
* @Then /^I dump the page$/
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* limitations under the License.
*/

namespace Surfnet\Tiqr\Service;
namespace Surfnet\Tiqr\Features\Framework;

use InvalidArgumentException;
use Surfnet\GsspBundle\Exception\NotFound;
Expand All @@ -32,7 +32,7 @@ public function __construct(string $filePath)
{
$this->filePath = $filePath;
if (!file_exists($this->filePath)) {
file_put_contents($this->filePath, json_encode([]));
file_put_contents($this->filePath, json_encode([], JSON_THROW_ON_ERROR));
chmod($this->filePath, 0666);
}
}
Expand All @@ -47,7 +47,7 @@ private function readValues(): array
throw new InvalidArgumentException(sprintf('Could not read FileValueStore storage file. %s', $this->filePath));
}

$result = json_decode($content, true);
$result = json_decode($content, true, 512, JSON_THROW_ON_ERROR);

return is_array($result) ? $result : [];
}
Expand All @@ -57,7 +57,7 @@ private function readValues(): array
*/
private function writeValues(array $values): void
{
file_put_contents($this->filePath, json_encode($values));
file_put_contents($this->filePath, json_encode($values, JSON_THROW_ON_ERROR));
}

public function set(string $key, mixed $value): self
Expand Down
10 changes: 10 additions & 0 deletions src/Features/mfaFatigueMitigation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,16 @@ Feature: When an user needs to authenticate
When push notification is sent with a trusted-device cookie with address "0000000000111111111122222222223333333333"
Then it should send a notification for the user with type "APNS" and address "0000000000111111111122222222223333333333"

Scenario: When a user tries to authenticates with a trusted cookie, but changes the address, a notification should not be sent
Given I am on "/demo/sp"
And I fill in "NameID" with my identifier
When I press "authenticate"
Then I should see "Log in with tiqr"
And I should be on "/authentication"

When push notification is sent with a trusted-device cookie with address "0000000000111111111122222222223333333333" and cookie userId "abc-1234"
Then the logs should mention a signature mismatch for address "0000000000111111111122222222223333333333"
And it should fail with "no-trusted-device"


# @TODO Add a test somewhere, maybe not here, that tests the cookie get overwritten properly (or appended) if a new scan occurs?
Expand Down

0 comments on commit e11170e

Please sign in to comment.