Skip to content

Improve logging in auth flows #348

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.

Sign up for GitHub

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

Merged
cicnavi merged 1 commit into from
Jun 30, 2026
Merged

Improve logging in auth flows #348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions src/Factories/Grant/ImplicitGrantFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use SimpleSAML\Module\oidc\Server\Grants\ImplicitGrant;
use SimpleSAML\Module\oidc\Server\RequestRules\RequestRulesManager;
use SimpleSAML\Module\oidc\Services\IdTokenBuilder;
use SimpleSAML\Module\oidc\Services\LoggerService;
use SimpleSAML\Module\oidc\Utils\RequestParamsResolver;

class ImplicitGrantFactory
Expand All @@ -32,6 +33,7 @@ public function __construct(
private readonly AccessTokenRepository $accessTokenRepository,
private readonly RequestParamsResolver $requestParamsResolver,
private readonly AccessTokenEntityFactory $accessTokenEntityFactory,
private readonly LoggerService $loggerService,
) {
}

Expand All @@ -44,6 +46,7 @@ public function build(): ImplicitGrant
$this->requestRulesManager,
$this->requestParamsResolver,
$this->accessTokenEntityFactory,
$this->loggerService,
);
}
}
3 changes: 3 additions & 0 deletions src/Factories/Grant/RefreshTokenGrantFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use SimpleSAML\Module\oidc\Repositories\RefreshTokenRepository;
use SimpleSAML\Module\oidc\Server\Grants\RefreshTokenGrant;
use SimpleSAML\Module\oidc\Server\TokenIssuers\RefreshTokenIssuer;
use SimpleSAML\Module\oidc\Services\LoggerService;
use SimpleSAML\Module\oidc\Utils\AuthenticatedOAuth2ClientResolver;

class RefreshTokenGrantFactory
Expand All @@ -30,6 +31,7 @@ public function __construct(
private readonly AccessTokenEntityFactory $accessTokenEntityFactory,
private readonly RefreshTokenIssuer $refreshTokenIssuer,
private readonly AuthenticatedOAuth2ClientResolver $authenticatedOAuth2ClientResolver,
private readonly LoggerService $loggerService,
) {
}

Expand All @@ -40,6 +42,7 @@ public function build(): RefreshTokenGrant
$this->accessTokenEntityFactory,
$this->refreshTokenIssuer,
$this->authenticatedOAuth2ClientResolver,
$this->loggerService,
);

$refreshTokenGrant->setRefreshTokenTTL($this->moduleConfig->getRefreshTokenDuration());
Expand Down
129 changes: 125 additions & 4 deletions src/Server/Grants/AuthCodeGrant.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,10 @@ public function completeOidcAuthorizationRequest(
?? $this->getAuthorizationRequestClientRedirectUri($authorizationRequest);

if ($authorizationRequest->isAuthorizationApproved() !== true) {
$this->loggerService->notice(
'Authorization request denied by the user.',
['client_id' => $authorizationRequest->getClient()->getIdentifier()],
);
// The user denied the client, redirect them back with an error
throw OidcServerException::accessDenied(
'The user denied the request',
Expand All @@ -239,6 +243,11 @@ public function completeOidcAuthorizationRequest(
$authorizationRequest,
);

$this->loggerService->notice(
'Authorization approved; authorization code issued.',
['client_id' => $authCode->getClient()->getIdentifier(), 'auth_code_id' => $authCode->getIdentifier()],
);

$payload = [
'client_id' => $authCode->getClient()->getIdentifier(),
'redirect_uri' => $authCode->getRedirectUri(),
Expand Down Expand Up @@ -370,7 +379,7 @@ public function respondToAccessTokenRequest(
$encryptedAuthCode = $this->getRequestParameter('code', $request);

if ($encryptedAuthCode === null) {
$this->loggerService->debug('Code parameter not provided.');
$this->loggerService->notice('Token request rejected: `code` parameter not provided.');
throw OAuthServerException::invalidRequest('code');
}

Expand All @@ -381,10 +390,15 @@ public function respondToAccessTokenRequest(
*/
$authCodePayload = json_decode($this->decrypt($encryptedAuthCode), null, 512, JSON_THROW_ON_ERROR);
} catch (LogicException $e) {
$this->loggerService->warning(
'Token request rejected: could not decrypt the authorization code.',
['exception' => $e->getMessage()],
);
throw OAuthServerException::invalidRequest('code', 'Cannot decrypt the authorization code', $e);
}

if (!property_exists($authCodePayload, 'auth_code_id')) {
$this->loggerService->notice('Token request rejected: authorization code is malformed (no auth_code_id).');
throw OAuthServerException::invalidRequest('code', 'Authorization code malformed');
}

Expand All @@ -395,6 +409,10 @@ public function respondToAccessTokenRequest(
$storedAuthCodeEntity = $this->authCodeRepository->findById($authCodePayload->auth_code_id);

if ($storedAuthCodeEntity === null) {
$this->loggerService->notice(
'Token request rejected: authorization code not found in storage.',
['auth_code_id' => $authCodePayload->auth_code_id],
);
throw OAuthServerException::invalidGrant('Authorization code not found');
}

Expand Down Expand Up @@ -441,10 +459,22 @@ public function respondToAccessTokenRequest(
// client_id below. If an authentication scheme for non-registered clients is introduced later (e.g.
// attestation), this can be relaxed the same way it was for registered clients.
if (! $clientId) {
$this->loggerService->notice(
'Token request rejected: generic (non-registered) client did not provide required `client_id`.',
['auth_code_id' => $authCodePayload->auth_code_id],
);
throw OidcServerException::invalidRequest('client_id');
}

if ($clientId !== $storedAuthCodeEntity->getBoundClientId()) {
$this->loggerService->warning(
'Token request rejected: `client_id` does not match the one the authorization code was bound to.',
[
'auth_code_id' => $authCodePayload->auth_code_id,
'client_id' => $clientId,
'bound_client_id' => $storedAuthCodeEntity->getBoundClientId(),
],
);
throw OAuthServerException::invalidGrant('Authorization code not intended for this client_id.');
}

Expand All @@ -455,10 +485,24 @@ public function respondToAccessTokenRequest(
);

if (! $redirectUri) {
$this->loggerService->notice(
'Token request rejected: generic (non-registered) client did not provide required `redirect_uri`.',
['auth_code_id' => $authCodePayload->auth_code_id, 'client_id' => $clientId],
);
throw OidcServerException::invalidRequest(ParamsEnum::RedirectUri->value);
}

if ($redirectUri !== $storedAuthCodeEntity->getBoundRedirectUri()) {
$this->loggerService->warning(
'Token request rejected: `redirect_uri` does not match the one the authorization code ' .
'was bound to.',
[
'auth_code_id' => $authCodePayload->auth_code_id,
'client_id' => $clientId,
'redirect_uri' => $redirectUri,
'bound_redirect_uri' => $storedAuthCodeEntity->getBoundRedirectUri(),
],
);
throw OAuthServerException::invalidGrant('Authorization code not intended for this redirect_uri.');
}

Expand Down Expand Up @@ -490,6 +534,13 @@ public function respondToAccessTokenRequest(
$registeredGrantTypes !== [] &&
!in_array(GrantTypesEnum::AuthorizationCode->value, $registeredGrantTypes, true)
) {
$this->loggerService->warning(
'Token request rejected: client is not authorized to use the authorization_code grant type.',
[
'client_id' => $client->getIdentifier(),
'registered_grant_types' => $registeredGrantTypes,
],
);
throw OidcServerException::unauthorizedClient(
'The client is not authorized to use the authorization_code grant type.',
);
Expand All @@ -516,6 +567,11 @@ public function respondToAccessTokenRequest(
}

if (empty($utilizedClientAuthenticationParams)) {
$this->loggerService->warning(
'Token request rejected: client authentication not performed (no client authentication ' .
'method and no PKCE code_verifier presented).',
['client_id' => $client->getIdentifier(), 'auth_code_id' => $authCodePayload->auth_code_id],
);
throw OidcServerException::accessDenied('Client authentication not performed.');
}

Expand Down Expand Up @@ -547,6 +603,11 @@ public function respondToAccessTokenRequest(

// If a code challenge isn't present but a code verifier is, reject the request to block PKCE downgrade attack
if (empty($authCodePayload->code_challenge) && $codeVerifier !== null) {
$this->loggerService->warning(
'Token request rejected: `code_verifier` received but the authorization request had no ' .
'`code_challenge` (possible PKCE downgrade attempt).',
['client_id' => $client->getIdentifier(), 'auth_code_id' => $authCodePayload->auth_code_id],
);
throw OAuthServerException::invalidRequest(
'code_challenge',
'code_verifier received when no code_challenge is present',
Expand All @@ -556,6 +617,11 @@ public function respondToAccessTokenRequest(
// Validate code challenge
if (!empty($authCodePayload->code_challenge)) {
if ($codeVerifier === null) {
$this->loggerService->notice(
'Token request rejected: `code_verifier` is missing while a `code_challenge` was used ' .
'in the authorization request.',
['client_id' => $client->getIdentifier(), 'auth_code_id' => $authCodePayload->auth_code_id],
);
throw OAuthServerException::invalidRequest('code_verifier');
}

Expand All @@ -570,22 +636,42 @@ public function respondToAccessTokenRequest(
// }

if (property_exists($authCodePayload, 'code_challenge_method')) {
if (isset($this->codeChallengeVerifiers[$authCodePayload->code_challenge_method])) {
$codeChallengeVerifier = $this->codeChallengeVerifiers[$authCodePayload->code_challenge_method];
$codeChallengeMethod = isset($authCodePayload->code_challenge_method) ?
$authCodePayload->code_challenge_method :
'';
if (isset($this->codeChallengeVerifiers[$codeChallengeMethod])) {
$codeChallengeVerifier = $this->codeChallengeVerifiers[$codeChallengeMethod];

if (
$codeChallengeVerifier->verifyCodeChallenge(
$codeVerifier,
$authCodePayload->code_challenge,
) === false
) {
$this->loggerService->warning(
'Token request rejected: PKCE `code_verifier` failed verification against the ' .
'stored `code_challenge`.',
[
'client_id' => $client->getIdentifier(),
'auth_code_id' => $authCodePayload->auth_code_id,
'code_challenge_method' => $codeChallengeMethod,
],
);
throw OAuthServerException::invalidGrant('Failed to verify `code_verifier`.');
}
} else {
$this->loggerService->error(
'Token request failed: unsupported code challenge method stored on authorization code.',
[
'client_id' => $client->getIdentifier(),
'auth_code_id' => $authCodePayload->auth_code_id,
'code_challenge_method' => $codeChallengeMethod,
],
);
throw OAuthServerException::serverError(
sprintf(
'Unsupported code challenge method `%s`',
($authCodePayload->code_challenge_method ?? ''),
$codeChallengeMethod,
),
);
}
Expand Down Expand Up @@ -661,6 +747,11 @@ public function respondToAccessTokenRequest(
// Revoke used auth code
$this->authCodeRepository->revokeAuthCode($authCodePayload->auth_code_id);

$this->loggerService->notice(
'Authorization code redeemed; access token issued.',
['client_id' => $client->getIdentifier(), 'auth_code_id' => $authCodePayload->auth_code_id],
);

return $responseType;
}

Expand Down Expand Up @@ -693,27 +784,57 @@ protected function validateAuthorizationCode(
}

if (time() > $authCodePayload->expire_time) {
$this->loggerService->notice(
'Token request rejected: authorization code has expired.',
['client_id' => $client->getIdentifier(), 'auth_code_id' => $authCodePayload->auth_code_id],
);
throw OAuthServerException::invalidGrant('Authorization code has expired');
}

if ($storedAuthCodeEntity->isRevoked()) {
$this->loggerService->warning(
'Token request rejected: authorization code has been revoked (likely reused). Revoking all ' .
'related access and refresh tokens.',
['client_id' => $client->getIdentifier(), 'auth_code_id' => $authCodePayload->auth_code_id],
);
// Code is reused, all related tokens must be revoked, per https://tools.ietf.org/html/rfc6749#section-4.1.2
$this->accessTokenRepository->revokeByAuthCodeId($authCodePayload->auth_code_id);
$this->refreshTokenRepository->revokeByAuthCodeId($authCodePayload->auth_code_id);
throw OAuthServerException::invalidGrant('Authorization code has been revoked');
}

if ($authCodePayload->client_id !== $client->getIdentifier()) {
$this->loggerService->warning(
'Token request rejected: authorization code was not issued to the authenticated client.',
[
'client_id' => $client->getIdentifier(),
'auth_code_client_id' => $authCodePayload->client_id,
'auth_code_id' => $authCodePayload->auth_code_id,
],
);
throw OAuthServerException::invalidRequest('code', 'Authorization code was not issued to this client');
}

// The redirect URI is required in this request
$redirectUri = $this->getRequestParameter('redirect_uri', $request);
if (empty($authCodePayload->redirect_uri) === false && $redirectUri === null) {
$this->loggerService->notice(
'Token request rejected: `redirect_uri` parameter is required but was not provided.',
['client_id' => $client->getIdentifier(), 'auth_code_id' => $authCodePayload->auth_code_id],
);
throw OAuthServerException::invalidRequest('redirect_uri');
}

if ($authCodePayload->redirect_uri !== $redirectUri) {
$this->loggerService->warning(
'Token request rejected: `redirect_uri` does not match the one from the authorization request.',
[
'client_id' => $client->getIdentifier(),
'auth_code_id' => $authCodePayload->auth_code_id,
'redirect_uri' => $redirectUri,
'authorization_redirect_uri' => $authCodePayload->redirect_uri,
],
);
throw OAuthServerException::invalidRequest(
'redirect_uri',
'Invalid redirect URI or not the same as in authorization request',
Expand Down
28 changes: 28 additions & 0 deletions src/Server/Grants/ImplicitGrant.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
use SimpleSAML\Module\oidc\Server\RequestTypes\AuthorizationRequest;
use SimpleSAML\Module\oidc\Server\ResponseModes\FragmentResponseMode;
use SimpleSAML\Module\oidc\Services\IdTokenBuilder;
use SimpleSAML\Module\oidc\Services\LoggerService;
use SimpleSAML\Module\oidc\Utils\RequestParamsResolver;
use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum;

Expand All @@ -58,6 +59,7 @@ public function __construct(
protected RequestRulesManager $requestRulesManager,
protected RequestParamsResolver $requestParamsResolver,
AccessTokenEntityFactory $accessTokenEntityFactory,
protected LoggerService $loggerService,
) {
parent::__construct($accessTokenTTL);

Expand Down Expand Up @@ -105,6 +107,10 @@ public function completeAuthorizationRequest(
return $this->completeOidcAuthorizationRequest($authorizationRequest);
}

$this->loggerService->error(
'Implicit grant failed: unexpected authorization request type.',
['type' => $authorizationRequest::class],
);
throw new LogicException('Unexpected OAuth2AuthorizationRequest type.');
}

Expand Down Expand Up @@ -200,12 +206,20 @@ private function completeOidcAuthorizationRequest(AuthorizationRequest $authoriz
$user = $authorizationRequest->getUser();

if ($user instanceof UserEntity === false) {
$this->loggerService->error(
'Implicit grant failed: no authenticated user set on the authorization request.',
['client_id' => $authorizationRequest->getClient()->getIdentifier()],
);
throw new LogicException('An instance of UserEntityInterface should be set on the AuthorizationRequest');
}

$redirectUrl = $this->getRedirectUrl($authorizationRequest);

if ($authorizationRequest->isAuthorizationApproved() !== true) {
$this->loggerService->notice(
'Implicit grant: authorization request denied by the user.',
['client_id' => $authorizationRequest->getClient()->getIdentifier()],
);
throw OidcServerException::accessDenied(
'The user denied the request',
$redirectUrl,
Expand Down Expand Up @@ -237,9 +251,18 @@ private function completeOidcAuthorizationRequest(AuthorizationRequest $authoriz
);

if ($accessToken instanceof EntityStringRepresentationInterface === false) {
$this->loggerService->error(
'Implicit grant failed: issued access token does not implement ' .
EntityStringRepresentationInterface::class . '.',
['client_id' => $authorizationRequest->getClient()->getIdentifier()],
);
throw new RuntimeException('AccessToken must implement ' . EntityStringRepresentationInterface::class);
}
if ($accessToken instanceof AccessTokenEntity === false) {
$this->loggerService->error(
'Implicit grant failed: issued access token is not an instance of ' . AccessTokenEntity::class . '.',
['client_id' => $authorizationRequest->getClient()->getIdentifier()],
);
throw new RuntimeException('AccessToken must be ' . AccessTokenEntity::class);
}

Expand Down Expand Up @@ -271,6 +294,11 @@ private function completeOidcAuthorizationRequest(AuthorizationRequest $authoriz
$responseParams,
);

$this->loggerService->notice(
'Implicit grant: authorization approved; ID token issued.',
['client_id' => $authorizationRequest->getClient()->getIdentifier()],
);

return $response;
}

Expand Down
Loading
Loading