Skip to content

Commit

Permalink
Merge pull request #345 from OpenConext/feature/343-allow-trusted-dev…
Browse files Browse the repository at this point in the history
…ice-cookie-suspend

Feature: Make trusted device cookie toggleable
  • Loading branch information
johanib authored Jan 30, 2025
2 parents 7c3a75b + a740f35 commit de256fd
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 22 deletions.
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;
}
}

0 comments on commit de256fd

Please sign in to comment.