Skip to content

Commit

Permalink
Add the session id derived id to polling requests
Browse files Browse the repository at this point in the history
The polling async requests should include the correlation id based on
the session id of the user that is placing them.

That way we can correlate between the access  logs and the application
logs.

See: https://www.pivotaltracker.com/story/show/188205232
  • Loading branch information
MKodde authored and johanib committed Nov 21, 2024
1 parent ff9742a commit 303e565
Show file tree
Hide file tree
Showing 23 changed files with 239 additions and 253 deletions.
15 changes: 12 additions & 3 deletions assets/typescript/Client/StatusClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,23 @@ export interface PendingRequest {
}

export class StatusClient {
constructor(private apiUrl: string) {

constructor(
private apiUrl: string,
private correlationLoggingId: string,
) {
}

/**
* Request status form the API.
*/
public request(callback: (status: string) => void, errorCallback: (error: unknown) => void): PendingRequest {
return jQuery.get(this.apiUrl, callback).fail(errorCallback);
let url;
if(this.correlationLoggingId !== ''){
url = this.apiUrl + (this.apiUrl.includes('?') ? '&' : '?') + 'correlation-id=' + this.correlationLoggingId;
}else{
url = this.apiUrl;
}

return jQuery.get(url, callback).fail(errorCallback);
}
}
6 changes: 3 additions & 3 deletions assets/typescript/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import jQuery from 'jquery';

declare global {
interface Window {
bootstrapAuthentication: (statusApiUrl: string, notificationApiUrl: string) => AuthenticationPageService;
bootstrapAuthentication: (statusApiUrl: string, notificationApiUrl: string, correlationLoggingId: string) => AuthenticationPageService;
}
}

window.bootstrapAuthentication = (statusApiUrl: string, notificationApiUrl: string) => {
const statusClient = new StatusClient(statusApiUrl);
window.bootstrapAuthentication = (statusApiUrl: string, notificationApiUrl: string, correlationLoggingId: string) => {
const statusClient = new StatusClient(statusApiUrl, correlationLoggingId);
const notificationClient = new NotificationClient(notificationApiUrl);
const pollingService = new StatusPollService(statusClient);

Expand Down
10 changes: 7 additions & 3 deletions assets/typescript/registration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@ import jQuery from 'jquery';

declare global {
interface Window {
bootstrapRegistration: (statusApiUrl: string, notificationApiUrl: string) => RegistrationStateMachine;
bootstrapRegistration: (
statusApiUrl: string,
notificationApiUrl: string,
correlationLoggingId: string
) => RegistrationStateMachine;
}
}

window.bootstrapRegistration = (statusApiUrl: string, finalizedUrl: string) => {
const statusClient = new StatusClient(statusApiUrl);
window.bootstrapRegistration = (statusApiUrl: string, finalizedUrl: string, correlationLoggingId: string) => {
const statusClient = new StatusClient(statusApiUrl, correlationLoggingId);
const pollingService = new StatusPollService(statusClient);
const machine = new RegistrationStateMachine(
pollingService,
Expand Down
2 changes: 1 addition & 1 deletion ci/qa/phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<server name="KERNEL_CLASS" value="Surfnet\Tiqr\Kernel"/>
<server name="SHELL_VERBOSITY" value="-1"/>
<server name="SYMFONY_PHPUNIT_REMOVE" value=""/>
<server name="SYMFONY_PHPUNIT_VERSION" value="6.5"/>
<server name="SYMFONY_PHPUNIT_VERSION" value="9.6"/>
<env name="SYMFONY_DEPRECATIONS_HELPER" value="weak"/>
</php>
<testsuites>
Expand Down
1 change: 0 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
"khanamiryan/qrcode-detector-decoder": "^2.0",
"league/csv": "^9.13",
"malukenho/docheader": "^1",
"mockery/mockery": "^1.6",
"overtrue/phplint": "*",
"phpmd/phpmd": "^2.15",
"phpstan/phpstan": "^1.10",
Expand Down
136 changes: 1 addition & 135 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 3 additions & 5 deletions config/openconext/parameters.yaml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ parameters:
# A secret key that's used to generate certain security-related tokens
app_secret: ThisTokenIsNotSoSecretChangeIt

# A secret salt used to hash the correlationId for logging based on the session_id
correlation_id_salt: 'changeMeToAtLeast16CharsOfRandomString'

# All locales supported by the application
default_locale: en_GB
locales:
Expand Down Expand Up @@ -41,11 +44,6 @@ parameters:
# PCRE as accepted by preg_match (http://php.net/preg_match).
mobile_app_user_agent_pattern: "/^.*$/"

# When logging authentication related messages, having a reference to the session id of the user
# makes auditing the logs much easier. We do not want to log the session_id for obvious reasons, that's why
# a salt is used to hash a part of the session id. Ensuring we do not disclose sensitive data in the logs.
session_correlation_salt: 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'

# Options for the tiqr library
tiqr_library_options:
general:
Expand Down
3 changes: 2 additions & 1 deletion config/packages/framework.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ framework:
# Remove or comment this section to explicitly disable session support.
session:
handler_id: null
cookie_secure: true
cookie_samesite: none
name: sess_tiqr
cookie_httponly: true
cookie_secure: true
router:
strict_requirements: null
utf8: true
Expand Down
2 changes: 1 addition & 1 deletion config/services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ services:
$tiqrConfiguration: '%tiqr_library_options%'
$appSecret: '%app_secret%'
$sessionOptions: '%session.storage.options%'
$sessionCorrelationSalt: '%session_correlation_salt%'
$correlationIdSalt: '%correlation_id_salt%'

# 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
1 change: 0 additions & 1 deletion dev/Command/AuthenticationCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ protected function decorateResult(string $text): string
protected function readAuthenticationLinkFromFile(string $file, OutputInterface $output): string
{
$qrcode = new QrReader(file_get_contents($file), QrReader::SOURCE_TYPE_BLOB);
/** @phpstan-var mixed $link */
$link = $qrcode->text();

if (!is_string($link)) {
Expand Down
7 changes: 5 additions & 2 deletions src/Controller/AuthenticationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use Surfnet\Tiqr\Exception\UserNotFoundException;
use Surfnet\Tiqr\Exception\UserPermanentlyBlockedException;
use Surfnet\Tiqr\Exception\UserTemporarilyBlockedException;
use Surfnet\Tiqr\Service\SessionCorrelationIdService;
use Surfnet\Tiqr\Tiqr\AuthenticationRateLimitServiceInterface;
use Surfnet\Tiqr\Tiqr\Exception\UserNotExistsException;
use Surfnet\Tiqr\Tiqr\Response\AuthenticationResponse;
Expand All @@ -53,6 +54,7 @@ public function __construct(
private readonly TiqrServiceInterface $tiqrService,
private readonly TiqrUserRepositoryInterface $userRepository,
private readonly AuthenticationRateLimitServiceInterface $authenticationRateLimitService,
private readonly SessionCorrelationIdService $correlationIdService,
private readonly LoggerInterface $logger
) {
}
Expand All @@ -72,7 +74,7 @@ public function __invoke(Request $request): Response

$logger->info('Verifying if there is a pending authentication request from SP');

// Do have a valid sample AuthnRequest?
// Do we have a valid GSSP authentication AuthnRequest in this session?
if (!$this->authenticationService->authenticationRequired()) {
$logger->error('There is no pending authentication request from SP');

Expand Down Expand Up @@ -145,7 +147,8 @@ public function __invoke(Request $request): Response
$logger->info('Return authentication page with QR code');

return $this->render('default/authentication.html.twig', [
'authenticateUrl' => $this->tiqrService->authenticationUrl()
'authenticateUrl' => $this->tiqrService->authenticationUrl(),
'correlationLoggingId' => $this->correlationIdService->generateCorrelationId(),
]);
}

Expand Down
1 change: 0 additions & 1 deletion src/Controller/AuthenticationQrController.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public function __invoke(): Response
$logger = WithContextLogger::from($this->logger, ['nameId' => $nameId, 'sari' => $sari]);
$logger->info('Client request QR image');

// Do have a valid sample AuthnRequest?.
if (!$this->authenticationService->authenticationRequired()) {
$logger->error('There is no pending authentication request from SP');

Expand Down
3 changes: 3 additions & 0 deletions src/Controller/RegistrationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use Surfnet\GsspBundle\Service\StateHandlerInterface;
use Surfnet\Tiqr\Attribute\RequiresActiveSession;
use Surfnet\Tiqr\Exception\NoActiveAuthenrequestException;
use Surfnet\Tiqr\Service\SessionCorrelationIdService;
use Surfnet\Tiqr\Tiqr\Legacy\TiqrService;
use Surfnet\Tiqr\Tiqr\TiqrServiceInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
Expand All @@ -38,6 +39,7 @@ public function __construct(
private readonly RegistrationService $registrationService,
private readonly StateHandlerInterface $stateHandler,
private readonly TiqrServiceInterface $tiqrService,
private readonly SessionCorrelationIdService $correlationIdService,
private readonly LoggerInterface $logger
) {
}
Expand Down Expand Up @@ -81,6 +83,7 @@ public function registration(Request $request): Response
'default/registration.html.twig',
[
'metadataUrl' => sprintf("tiqrenroll://%s", $metadataUrl),
'correlationLoggingId' => $this->correlationIdService->generateCorrelationId(),
'enrollmentKey' => $key
]
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@
private string $sessionName;

public function __construct(
private LoggerInterface $logger,
private LoggerInterface $logger,
private SessionCorrelationIdService $sessionCorrelationIdService,
/** @var array<string, string> */
private array $sessionOptions,
) {
if (!array_key_exists('name', $this->sessionOptions)) {
Expand Down
1 change: 1 addition & 0 deletions src/EventSubscriber/SessionStateListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
public function __construct(
private LoggerInterface $logger,
private SessionCorrelationIdService $sessionCorrelationIdService,
/** @var array<string, string> */
private array $sessionOptions,
) {
if (!array_key_exists('name', $this->sessionOptions)) {
Expand Down
Loading

0 comments on commit 303e565

Please sign in to comment.