diff --git a/public/typo3conf/ext/ep_products/Classes/MyEP/ApiClient.php b/public/typo3conf/ext/ep_products/Classes/MyEP/ApiClient.php index edb82c03..6b3ad8e2 100644 --- a/public/typo3conf/ext/ep_products/Classes/MyEP/ApiClient.php +++ b/public/typo3conf/ext/ep_products/Classes/MyEP/ApiClient.php @@ -49,8 +49,22 @@ class ApiClient implements LoggerAwareInterface */ private const ACCESS_TOKEN_DEFAULT_TTL = 300; + /** + * Status codes that mean the token was rejected rather than the request being wrong. + */ + private const UNAUTHORIZED_STATUS_CODES = [Response::HTTP_UNAUTHORIZED, Response::HTTP_FORBIDDEN]; + private static ?AccessToken $accessToken = null; + /** + * Shared on purpose. Symfony's CurlHttpClient keeps its connection pool and TLS session + * cache in a CurlClientState owned by the instance, so building a client per call throws + * both away and forces a fresh DNS lookup, TCP connect and TLS handshake every time - + * TravelinfoController alone makes two calls per page render. The bearer token is passed + * per request rather than as a client default so one instance can outlive one token. + */ + private static ?HttpClientInterface $httpClient = null; + private ?FrontendInterface $cache = null; private ?array $configuration = null; @@ -103,6 +117,7 @@ class ApiClient implements LoggerAwareInterface // A conflict means the address is already registered, which is the normal answer to a // resubmitted contact form and not worth reporting. $this->assertStatusCode($response, [Response::HTTP_OK, Response::HTTP_CREATED, Response::HTTP_ACCEPTED, Response::HTTP_NO_CONTENT, Response::HTTP_CONFLICT], 'POST', 'contactform'); + $this->drainResponse($response); } /** @@ -117,6 +132,7 @@ class ApiClient implements LoggerAwareInterface // A conflict means the submission is already known, which is the normal answer to a // double-clicked submit button and not worth reporting. $this->assertStatusCode($response, [Response::HTTP_OK, Response::HTTP_CREATED, Response::HTTP_ACCEPTED, Response::HTTP_NO_CONTENT, Response::HTTP_CONFLICT], 'POST', 'applications'); + $this->drainResponse($response); } /** @@ -199,6 +215,30 @@ class ApiClient implements LoggerAwareInterface */ private function request(string $method, string $uri, array $options = []): ResponseInterface { + $response = $this->sendRequest($method, $uri, $options); + + if (false === in_array($this->readStatusCode($response, $method, $uri), self::UNAUTHORIZED_STATUS_CODES, true)) { + return $response; + } + + // The token outlives a single PHP request, so one the API has stopped accepting - after + // a MyEP restart or a revoked client - would keep failing every call for the rest of its + // cached lifetime unless it is actively discarded here. + $this->logWarning(sprintf('MyEP API rejected the cached access token for %s %s, re-authenticating once', $method, $uri)); + $this->discardAccessToken(); + + return $this->sendRequest($method, $uri, $options); + } + + /** + * @throws ApiException + */ + private function sendRequest(string $method, string $uri, array $options): ResponseInterface + { + // Resolved here rather than baked into the client's default options so that the shared + // client survives a token change, and so the retry above picks up the fresh token. + $options['auth_bearer'] = $this->getAccessToken($this->getConfiguration())->getToken(); + try { return $this->getHttpClient()->request($method, $uri, $options); } catch (TransportExceptionInterface $e) { @@ -215,13 +255,7 @@ class ApiClient implements LoggerAwareInterface */ private function assertStatusCode(ResponseInterface $response, array $expectedStatusCodes, string $method, string $uri): void { - try { - $statusCode = $response->getStatusCode(); - } catch (TransportExceptionInterface $e) { - $message = sprintf('MyEP API transport error while reading status for %s %s: %s', $method, $uri, $e->getMessage()); - $this->logError($message, ['exception' => $e]); - throw new ApiException($message, 0, $e); - } + $statusCode = $this->readStatusCode($response, $method, $uri); if (true === in_array($statusCode, $expectedStatusCodes, true)) { return; @@ -233,6 +267,39 @@ class ApiClient implements LoggerAwareInterface throw new ApiException($message, $statusCode); } + /** + * Reading the status completes the transfer, so this is also what turns the lazy response + * returned by the client into a finished request. Symfony keeps the status on the response, + * so calling this twice costs nothing. + * + * @throws ApiException + */ + private function readStatusCode(ResponseInterface $response, string $method, string $uri): int + { + try { + return $response->getStatusCode(); + } catch (TransportExceptionInterface $e) { + $message = sprintf('MyEP API transport error while reading status for %s %s: %s', $method, $uri, $e->getMessage()); + $this->logError($message, ['exception' => $e]); + throw new ApiException($message, 0, $e); + } + } + + /** + * Reads and discards a body nobody else is going to look at. Destroying an unconsumed + * response cancels the transfer, which closes the connection instead of returning it to + * the pool, so the next call would have to handshake again. + */ + private function drainResponse(ResponseInterface $response): void + { + try { + $response->getContent(false); + } catch (TransportExceptionInterface $e) { + // The status has already been asserted, so a failure here costs nothing but the + // connection reuse this method exists for. + } + } + /** * @throws ApiException */ @@ -253,13 +320,13 @@ class ApiClient implements LoggerAwareInterface private function getHttpClient(): HttpClientInterface { + if (null !== static::$httpClient) { + return static::$httpClient; + } + $config = $this->getConfiguration(); - static::$accessToken = $this->getAccessToken($config); - - $options = [ - 'auth_bearer' => static::$accessToken->getToken(), - ]; + $options = []; // Without these the client falls back to PHP's default_socket_timeout (commonly 60s), // so a stalled API pins a PHP worker for a minute per request. The defaults live in @@ -277,7 +344,7 @@ class ApiClient implements LoggerAwareInterface $options['max_duration'] = $maxDuration; } - return HttpClient::createForBaseUri($config['myEpApiBaseUrl'], $options); + return static::$httpClient = HttpClient::createForBaseUri($config['myEpApiBaseUrl'], $options); } /** @@ -312,7 +379,7 @@ class ApiClient implements LoggerAwareInterface */ private function getAccessToken(array $config): AccessToken { - if (null !== static::$accessToken && false === static::$accessToken->hasExpired()) { + if (true === $this->isTokenUsable(static::$accessToken)) { return static::$accessToken; } @@ -321,7 +388,7 @@ class ApiClient implements LoggerAwareInterface $cachedToken = $this->getCache()->get(self::ACCESS_TOKEN_CACHE_IDENTIFIER); if (is_array($cachedToken) && isset($cachedToken['access_token'])) { $token = new AccessToken($cachedToken); - if (false === $token->hasExpired()) { + if (true === $this->isTokenUsable($token)) { static::$accessToken = $token; return static::$accessToken; @@ -329,7 +396,7 @@ class ApiClient implements LoggerAwareInterface } try { - static::$accessToken = $this->getProvider($config)->getAccessToken('client_credentials'); + static::$accessToken = $this->normalizeToken($this->getProvider($config)->getAccessToken('client_credentials')); $this->cacheAccessToken(static::$accessToken); } catch (IdentityProviderException $e) { $message = sprintf('MyEP OAuth token request failed: %s', $e->getMessage()); @@ -348,16 +415,53 @@ class ApiClient implements LoggerAwareInterface return static::$accessToken; } - private function cacheAccessToken(AccessToken $token): void + /** + * AccessToken::hasExpired() throws for a token that reports no expiry, so it cannot be used + * as the check - a token without one is simply treated as unusable and replaced. The same + * margin as the cache lifetime is applied on read as well, so a token cannot lapse between + * this check and the API call that uses it, whichever node wrote the cache entry. + */ + private function isTokenUsable(?AccessToken $token): bool { + if (null === $token) { + return false; + } + $expires = $token->getExpires(); if (null === $expires) { - $lifetime = self::ACCESS_TOKEN_DEFAULT_TTL; - } else { - $lifetime = $expires - time() - self::ACCESS_TOKEN_EXPIRY_MARGIN; + return false; } + return $expires - self::ACCESS_TOKEN_EXPIRY_MARGIN > time(); + } + + /** + * A token that reports no expiry cannot be validated on read - isTokenUsable() rejects it + * and AccessToken::hasExpired() throws for it - so it is given the fallback lifetime here, + * once, and the rest of the class can treat every token the same way. + */ + private function normalizeToken(AccessToken $token): AccessToken + { + if (null !== $token->getExpires()) { + return $token; + } + + return new AccessToken($token->jsonSerialize() + ['expires_in' => self::ACCESS_TOKEN_DEFAULT_TTL]); + } + + private function discardAccessToken(): void + { + static::$accessToken = null; + $this->getCache()->remove(self::ACCESS_TOKEN_CACHE_IDENTIFIER); + } + + private function cacheAccessToken(AccessToken $token): void + { + // normalizeToken() has already given a token without a reported expiry the fallback + // lifetime, so there is always a real expiry to subtract the margin from here. + $lifetime = $token->getExpires() - time() - self::ACCESS_TOKEN_EXPIRY_MARGIN; + if ($lifetime <= 0) { return; }