Skip to content

Commit 668f6c5

Browse files
author
Shilen Patel
committed
Adding some initial logging to MetricLogger
1 parent c21bea2 commit 668f6c5

9 files changed

+357
-86
lines changed

‎lib/Controller/LogoutController.php‎

Lines changed: 76 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace SimpleSAML\Module\oidc\Controller;
44

5+
use CirrusIdentity\SSP\Utils\MetricLogger;
56
use Exception;
67
use SimpleSAML\Error\BadRequest;
78
use SimpleSAML\Module\oidc\Factories\TemplateFactory;
@@ -66,66 +67,93 @@ public function __invoke(ServerRequest $request): Response
6667
// (FCL has challenges with User Agents Blocking Access to Third-Party Content:
6768
// https://openid.net/specs/openid-connect-frontchannel-1_0.html#ThirdPartyContent)
6869

69-
$logoutRequest = $this->authorizationServer->validateLogoutRequest($request);
70+
try {
71+
$logoutRequest = $this->authorizationServer->validateLogoutRequest($request);
7072

71-
// Set indication that the logout is initiated using OIDC protocol. This will be checked in the
72-
// logoutHandler() method.
73-
$this->sessionService->setIsOidcInitiatedLogout(true);
73+
// Set indication that the logout is initiated using OIDC protocol. This will be checked in the
74+
// logoutHandler() method.
75+
$this->sessionService->setIsOidcInitiatedLogout(true);
7476

75-
// Indication if any there was a call to logout action on any auth source at all...
76-
$wasLogoutActionCalled = false;
77+
// Indication if any there was a call to logout action on any auth source at all...
78+
$wasLogoutActionCalled = false;
7779

78-
$sidClaim = null;
80+
$sidClaim = null;
7981

80-
// If id_token_hint was provided, resolve session ID
81-
$idTokenHint = $logoutRequest->getIdTokenHint();
82-
if ($idTokenHint !== null) {
83-
$sidClaim = $idTokenHint->claims()->get('sid');
84-
}
82+
// If id_token_hint was provided, resolve session ID
83+
$idTokenHint = $logoutRequest->getIdTokenHint();
84+
if ($idTokenHint !== null) {
85+
$sidClaim = $idTokenHint->claims()->get('sid');
86+
}
8587

86-
// Check if RP is requesting logout for session that previously existed (not this current session).
87-
// Claim 'sid' from 'id_token_hint' logout parameter indicates for which session should logout be
88-
// performed (sid is session ID used when ID token was issued during authn). If the requested
89-
// sid is different from the current session ID, try to find the requested session.
90-
if (
91-
$sidClaim !== null &&
92-
$this->sessionService->getCurrentSession()->getSessionId() !== $sidClaim
93-
) {
94-
try {
95-
if (($sidSession = $this->sessionService->getSessionById($sidClaim)) !== null) {
96-
$sidSessionValidAuthorities = $sidSession->getAuthorities();
97-
98-
if (! empty($sidSessionValidAuthorities)) {
99-
$wasLogoutActionCalled = true;
100-
// Create a SessionLogoutTicket so that the sid is available in the static logoutHandler()
101-
$this->sessionLogoutTicketStoreBuilder->getInstance()->add($sidClaim);
102-
// Initiate logout for every valid auth source for the requested session.
103-
foreach ($sidSessionValidAuthorities as $authSourceId) {
104-
$sidSession->doLogout($authSourceId);
88+
// Check if RP is requesting logout for session that previously existed (not this current session).
89+
// Claim 'sid' from 'id_token_hint' logout parameter indicates for which session should logout be
90+
// performed (sid is session ID used when ID token was issued during authn). If the requested
91+
// sid is different from the current session ID, try to find the requested session.
92+
if (
93+
$sidClaim !== null &&
94+
$this->sessionService->getCurrentSession()->getSessionId() !== $sidClaim
95+
) {
96+
try {
97+
if (($sidSession = $this->sessionService->getSessionById($sidClaim)) !== null) {
98+
$sidSessionValidAuthorities = $sidSession->getAuthorities();
99+
100+
if (! empty($sidSessionValidAuthorities)) {
101+
$wasLogoutActionCalled = true;
102+
// Create a SessionLogoutTicket so that the sid is available in the static logoutHandler()
103+
$this->sessionLogoutTicketStoreBuilder->getInstance()->add($sidClaim);
104+
// Initiate logout for every valid auth source for the requested session.
105+
foreach ($sidSessionValidAuthorities as $authSourceId) {
106+
$sidSession->doLogout($authSourceId);
107+
}
105108
}
106109
}
110+
} catch (Throwable $exception) {
111+
$this->loggerService->warning(
112+
sprintf('Logout: could not get session with ID %s, error: %s', $sidClaim, $exception->getMessage())
113+
);
107114
}
108-
} catch (Throwable $exception) {
109-
$this->loggerService->warning(
110-
sprintf('Logout: could not get session with ID %s, error: %s', $sidClaim, $exception->getMessage())
111-
);
112115
}
113-
}
114116

115-
$currentSessionValidAuthorities = $this->sessionService->getCurrentSession()->getAuthorities();
116-
if (! empty($currentSessionValidAuthorities)) {
117-
$wasLogoutActionCalled = true;
118-
// Initiate logout for every valid auth source for the current session.
119-
foreach ($this->sessionService->getCurrentSession()->getAuthorities() as $authSourceId) {
120-
$this->sessionService->getCurrentSession()->doLogout($authSourceId);
117+
$currentSessionValidAuthorities = $this->sessionService->getCurrentSession()->getAuthorities();
118+
if (! empty($currentSessionValidAuthorities)) {
119+
$wasLogoutActionCalled = true;
120+
// Initiate logout for every valid auth source for the current session.
121+
foreach ($this->sessionService->getCurrentSession()->getAuthorities() as $authSourceId) {
122+
$this->sessionService->getCurrentSession()->doLogout($authSourceId);
123+
}
121124
}
122-
}
123125

124-
// Set indication for OIDC initiated logout back to false, so that the logoutHandler() method does not
125-
// run for other logout initiated actions, like (currently) re-authentication...
126-
$this->sessionService->setIsOidcInitiatedLogout(false);
127-
128-
return $this->resolveResponse($logoutRequest, $wasLogoutActionCalled);
126+
// Set indication for OIDC initiated logout back to false, so that the logoutHandler() method does not
127+
// run for other logout initiated actions, like (currently) re-authentication...
128+
$this->sessionService->setIsOidcInitiatedLogout(false);
129+
130+
MetricLogger::getInstance()->logMetric(
131+
'oidc',
132+
'logout',
133+
[
134+
'sessionId' => $this->sessionService->getCurrentSession()->getSessionId(),
135+
'sessionTrackID' => $this->sessionService->getCurrentSession()->getTrackID(),
136+
'sidClaim' => $sidClaim,
137+
'idTokenHint' => $idTokenHint
138+
]
139+
);
140+
141+
return $this->resolveResponse($logoutRequest, $wasLogoutActionCalled);
142+
} catch (Exception $e) {
143+
MetricLogger::getInstance()->logMetric(
144+
'oidc',
145+
'error',
146+
[
147+
'message' => $e->getMessage(),
148+
'oidc' => [
149+
'endpoint' => 'logout',
150+
]
151+
152+
]
153+
);
154+
155+
throw $e;
156+
}
129157
}
130158

131159
/**

‎lib/Controller/OAuth2AccessTokenController.php‎

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
namespace SimpleSAML\Module\oidc\Controller;
1616

17+
use Exception;
18+
use CirrusIdentity\SSP\Utils\MetricLogger;
1719
use SimpleSAML\Module\oidc\Repositories\AllowedOriginRepository;
1820
use SimpleSAML\Module\oidc\Server\AuthorizationServer;
1921
use Laminas\Diactoros\Response;
@@ -40,9 +42,45 @@ public function __invoke(ServerRequest $request): \Psr\Http\Message\ResponseInte
4042
{
4143
// Check if this is actually a CORS preflight request...
4244
if (strtoupper($request->getMethod()) === 'OPTIONS') {
43-
return $this->handleCors($request);
45+
try {
46+
return $this->handleCors($request);
47+
} catch (OidcServerException $e) {
48+
MetricLogger::getInstance()->logMetric(
49+
'oidc',
50+
'error',
51+
[
52+
'message' => $e->getMessage(),
53+
'errorDescription' => $e->getPayload()["error_description"],
54+
'oidc' => [
55+
'endpoint' => 'token',
56+
]
57+
]
58+
);
59+
60+
throw $e;
61+
}
62+
}
63+
64+
try {
65+
return $this->authorizationServer->respondToAccessTokenRequest($request, new Response());
66+
} catch (Exception $e) {
67+
// TODO log anything else?
68+
MetricLogger::getInstance()->logMetric(
69+
'oidc',
70+
'error',
71+
[
72+
'message' => $e->getMessage(),
73+
'oidc' => [
74+
'endpoint' => 'token',
75+
'clientId' => $this->getClientIdFromTokenRequest($request),
76+
'grantType' => $this->getRequestParameter("grant_type", $request)
77+
]
78+
79+
]
80+
);
81+
82+
throw $e;
4483
}
45-
return $this->authorizationServer->respondToAccessTokenRequest($request, new Response());
4684
}
4785

4886
/**
@@ -75,4 +113,42 @@ protected function handleCors(ServerRequest $request): Response
75113

76114
return new Response('php://memory', 204, $headers);
77115
}
116+
117+
private function getClientIdFromTokenRequest(ServerRequest $request)
118+
{
119+
[$basicAuthUser, $basicAuthPassword] = $this->getBasicAuthCredentials($request);
120+
121+
$clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser);
122+
123+
return $clientId;
124+
}
125+
126+
private function getBasicAuthCredentials(ServerRequest $request)
127+
{
128+
if (!$request->hasHeader('Authorization')) {
129+
return [null, null];
130+
}
131+
132+
$header = $request->getHeader('Authorization')[0];
133+
if (\strpos($header, 'Basic ') !== 0) {
134+
return [null, null];
135+
}
136+
137+
if (!($decoded = \base64_decode(\substr($header, 6)))) {
138+
return [null, null];
139+
}
140+
141+
if (\strpos($decoded, ':') === false) {
142+
return [null, null]; // HTTP Basic header without colon isn't valid
143+
}
144+
145+
return \explode(':', $decoded, 2);
146+
}
147+
148+
private function getRequestParameter($parameter, ServerRequest $request, $default = null)
149+
{
150+
$requestParameters = (array) $request->getParsedBody();
151+
152+
return $requestParameters[$parameter] ?? $default;
153+
}
78154
}

‎lib/Controller/OAuth2AuthorizationController.php‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414

1515
namespace SimpleSAML\Module\oidc\Controller;
1616

17+
use CirrusIdentity\SSP\Utils\MetricLogger;
1718
use Exception;
1819
use League\OAuth2\Server\Exception\OAuthServerException;
1920
use SimpleSAML\Error;
21+
use SimpleSAML\Error\BadRequest;
2022
use Psr\Http\Message\ResponseInterface;
2123
use SimpleSAML\Module\oidc\Server\AuthorizationServer;
2224
use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException;
@@ -65,22 +67,41 @@ public function __construct(
6567
*/
6668
public function __invoke(ServerRequest $request): ResponseInterface
6769
{
68-
$authorizationRequest = $this->authorizationServer->validateAuthorizationRequest($request);
69-
70-
$user = $this->authenticationService->getAuthenticateUser($request);
71-
72-
$authorizationRequest->setUser($user);
73-
$authorizationRequest->setAuthorizationApproved(true);
74-
75-
if ($authorizationRequest instanceof AuthorizationRequest) {
76-
$authorizationRequest->setIsCookieBasedAuthn($this->authenticationService->isCookieBasedAuthn());
77-
$authorizationRequest->setAuthSourceId($this->authenticationService->getAuthSourceId());
78-
$authorizationRequest->setSessionId($this->authenticationService->getSessionId());
79-
80-
$this->validatePostAuthnAuthorizationRequest($authorizationRequest);
70+
try {
71+
$authorizationRequest = $this->authorizationServer->validateAuthorizationRequest($request);
72+
73+
$user = $this->authenticationService->getAuthenticateUser($request);
74+
75+
$authorizationRequest->setUser($user);
76+
$authorizationRequest->setAuthorizationApproved(true);
77+
78+
if ($authorizationRequest instanceof AuthorizationRequest) {
79+
$authorizationRequest->setIsCookieBasedAuthn($this->authenticationService->isCookieBasedAuthn());
80+
$authorizationRequest->setAuthSourceId($this->authenticationService->getAuthSourceId());
81+
$authorizationRequest->setSessionId($this->authenticationService->getSessionId());
82+
83+
$this->validatePostAuthnAuthorizationRequest($authorizationRequest);
84+
}
85+
86+
return $this->authorizationServer->completeAuthorizationRequest($authorizationRequest, new Response());
87+
} catch (Exception $e) {
88+
if (!($e instanceof BadRequest)) {
89+
MetricLogger::getInstance()->logMetric(
90+
'oidc',
91+
'error',
92+
[
93+
'message' => $e->getMessage(),
94+
'oidc' => [
95+
'endpoint' => 'authorize',
96+
]
97+
// authorize endpoint doesn't contain secrets so okay to log all params
98+
+ $request->getQueryParams()
99+
]
100+
);
101+
}
102+
103+
throw $e;
81104
}
82-
83-
return $this->authorizationServer->completeAuthorizationRequest($authorizationRequest, new Response());
84105
}
85106

86107
/**

‎lib/Controller/OpenIdConnectDiscoverConfigurationController.php‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
namespace SimpleSAML\Module\oidc\Controller;
1616

17+
use CirrusIdentity\SSP\Utils\MetricLogger;
18+
use Exception;
1719
use Laminas\Diactoros\Response\JsonResponse;
1820
use Laminas\Diactoros\ServerRequest;
1921
use SimpleSAML\Module\oidc\Services\OidcOpenIdProviderMetadataService;
@@ -33,6 +35,22 @@ public function __construct(
3335

3436
public function __invoke(ServerRequest $serverRequest): JsonResponse
3537
{
36-
return new JsonResponse($this->oidcOpenIdProviderMetadataService->getMetadata());
38+
try {
39+
return new JsonResponse($this->oidcOpenIdProviderMetadataService->getMetadata());
40+
} catch (Exception $e) {
41+
MetricLogger::getInstance()->logMetric(
42+
'oidc',
43+
'error',
44+
[
45+
'message' => $e->getMessage(),
46+
'oidc' => [
47+
'endpoint' => 'configuration',
48+
]
49+
50+
]
51+
);
52+
53+
throw $e;
54+
}
3755
}
3856
}

0 commit comments

Comments
 (0)