Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Make trusted device cookie toggleable #345

Merged
merged 1 commit into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions config/openconext/parameters.yaml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ parameters:
# Should be one of the strings in: \Surfnet\Tiqr\Service\TrustedDevice\Http\CookieSameSite: 'none', 'lax', 'strict'
trusted_device_cookie_same_site: 'none'

# Enable/Disable enforcement of MFA fatigue prevention
# The trusted device cookie ensures no push notifications are sent when logging in on devices
# To prevent everyone having to scan qr codes all of a sudden, enforcement can be temporarily disabled so trusted devices can build up
trusted_device_enforcement_enabled: true

# The secret key is used for the authenticated encryption (AE) of the trusted-device cookies, it is stored in the
# parameters.yaml of Stepup-Tiqr. This secret may only be used for the AE of the trusted device cookies.
# The secret key size is fixed, it must be 256 bits (32 bytes (64 hex digits))
Expand Down
1 change: 1 addition & 0 deletions config/services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ services:
$appSecret: '%app_secret%'
$sessionOptions: '%session.storage.options%'
$correlationIdSalt: '%correlation_id_salt%'
$trustedDeviceCookieEnforcementEnabled: '%trusted_device_enforcement_enabled%'

# makes classes in src/ available to be used as services
# this creates a service per class whose id is the fully-qualified class name
Expand Down
55 changes: 33 additions & 22 deletions src/Controller/AuthenticationNotificationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Surfnet\GsspBundle\Service\StateHandlerInterface;
use Surfnet\Tiqr\Attribute\RequiresActiveSession;
use Surfnet\Tiqr\Service\TrustedDevice\TrustedDeviceService;
use Surfnet\Tiqr\Service\TrustedDeviceHelper;
use Surfnet\Tiqr\Tiqr\Exception\UserNotExistsException;
use Surfnet\Tiqr\Tiqr\TiqrServiceInterface;
use Surfnet\Tiqr\Tiqr\TiqrUserRepositoryInterface;
Expand All @@ -49,6 +50,7 @@ public function __construct(
private readonly TiqrUserRepositoryInterface $userRepository,
private readonly LoggerInterface $logger,
private readonly TrustedDeviceService $trustedDeviceService,
private readonly TrustedDeviceHelper $trustedDeviceHelper,
) {
}

Expand Down Expand Up @@ -96,31 +98,11 @@ public function __invoke(Request $request): Response
return $this->generateNotificationResponse('no-device');
}

$cookie = $this->trustedDeviceService->read($request);
if ($cookie === null) {
$this->logger->notice(
sprintf(
'No trusted device cookie stored for notification address "%s" and user "%s". No notification was sent',
$notificationAddress,
$nameId
)
);
if ($this->trustedDeviceHelper->trustedDeviceCookieEnforcementEnabled()
&& !$this->isTrustedDevice($request, $notificationAddress, $nameId)) {
return $this->generateNotificationResponse('no-trusted-device');
}

if ($this->trustedDeviceService->isTrustedDevice($cookie, $notificationAddress) === false) {
$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');
}


$this->logger->notice(sprintf(
'Sending push notification for user "%s" with type "%s" and (untranslated) address "%s"',
$nameId,
Expand Down Expand Up @@ -182,4 +164,33 @@ private function generateNotificationResponse(string $status): JsonResponse
{
return new JsonResponse($status);
}

private function isTrustedDevice(Request $request, string $notificationAddress, string $nameId): bool
{
$cookie = $this->trustedDeviceService->read($request);
if ($cookie === null) {
$this->logger->notice(
sprintf(
'No trusted device cookie stored for notification address "%s" and user "%s". No notification was sent',
$notificationAddress,
$nameId
)
);
return false;
}

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

return false;
}

return true;
}
}
6 changes: 6 additions & 0 deletions src/Service/TrustedDeviceHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
public function __construct(
private TrustedDeviceService $cookieService,
private LoggerInterface $logger,
private bool $trustedDeviceCookieEnforcementEnabled,
) {
}

Expand All @@ -51,4 +52,9 @@ public function handleRegisterTrustedDevice(
$this->logger->warning('Could not register trusted device on registration', ['exception' => $e]);
}
}

public function trustedDeviceCookieEnforcementEnabled(): bool
{
return $this->trustedDeviceCookieEnforcementEnabled === true;
}
}
99 changes: 99 additions & 0 deletions tests/Unit/Controller/AuthenticationNotificationControllerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php
/**
* Copyright 2025 SURFnet B.V.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

namespace Unit\Controller;

use PHPUnit\Framework\TestCase;
use Psr\Log\NullLogger;
use Surfnet\GsspBundle\Service\AuthenticationService;
use Surfnet\GsspBundle\Service\StateHandlerInterface;
use Surfnet\Tiqr\Controller\AuthenticationNotificationController;
use Surfnet\Tiqr\Service\TrustedDevice\TrustedDeviceService;
use Surfnet\Tiqr\Service\TrustedDeviceHelper;
use Surfnet\Tiqr\Tiqr\Legacy\TiqrUser;
use Surfnet\Tiqr\Tiqr\TiqrServiceInterface;
use Surfnet\Tiqr\Tiqr\TiqrUserRepositoryInterface;
use Symfony\Component\HttpFoundation\Request;

class AuthenticationNotificationControllerTest extends TestCase
{
private AuthenticationService $authService;
private StateHandlerInterface $stateHandler;
private TiqrServiceInterface $tiqrService;
private TiqrUserRepositoryInterface $userRepository;
private TrustedDeviceService $trustedDeviceService;

public function __construct(?string $name = null, array $data = [], $dataName = '')
{
$this->authService = $this->createMock(AuthenticationService::class);
$this->stateHandler = $this->createMock(StateHandlerInterface::class);
$this->tiqrService = $this->createMock(TiqrServiceInterface::class);
$this->userRepository = $this->createMock(TiqrUserRepositoryInterface::class);
$this->trustedDeviceService = $this->createMock(TrustedDeviceService::class);

parent::__construct($name, $data, $dataName);
}

public function provideTrustedDeviceCookieEnforcementEnabledScenarios(): array
{
return [
[false, '"success"'],
[true, '"no-trusted-device"'],
];
}

/**
* @dataProvider provideTrustedDeviceCookieEnforcementEnabledScenarios
*/
public function testTrustedDeviceCookieEnforcement(bool $trustedDeviceCookieEnforcementEnabled, string $expectedResponse): void
{
$controller = $this->makeController($trustedDeviceCookieEnforcementEnabled);
$this->authService->method('authenticationRequired')->willReturn(true);

$user = $this->mockUser('ACN', '01011001');

$this->userRepository->method('getUser')->willReturn($user);
$this->trustedDeviceService->method('read')->willReturn(null);

$request = $this->createMock(Request::class);

$response = $controller->__invoke($request);

$this->assertSame($expectedResponse, $response->getContent());
}

private function makeController(bool $trustedDeviceCookieEnforcementEnabled): AuthenticationNotificationController
{
return new AuthenticationNotificationController(
$this->authService,
$this->stateHandler,
$this->tiqrService,
$this->userRepository,
new NullLogger(),
$this->trustedDeviceService,
new TrustedDeviceHelper($this->trustedDeviceService, new NullLogger(), $trustedDeviceCookieEnforcementEnabled),
);
}

private function mockUser(string $notificationType, string $notificationAddress): TiqrUser
{
$user = $this->createMock(TiqrUser::class);
$user->expects($this->once())->method('getNotificationType')->willReturn($notificationType);
$user->method('getNotificationAddress')->willReturn($notificationAddress);
return $user;
}
}