ADR-027: OAuth token requests use the secure HTTP client 

Table of contents

Status 

Accepted — documents the unified state after PRs #145, #146, and the LOW-followups PR #148 land. The cache-key extension to clientSecretSecret and the redactCredentials helper are specifically introduced in #148; the rest is on main as of PR #146.

Date 

2026-05-22

Context 

Classes/Http/OAuth/OAuthTokenManager issues token-endpoint requests for adapters that need OAuth client-credentials / refresh-token flows. Until PR #145 the token request path used a ClientInterface injected via constructor and did not go through SecureHttpClientFactory. Two real consequences:

  1. SSRF protections were missing on the token endpoint. The isHostAllowed / isDangerousIpLiteral checks documented in ADR-010: Secure Outbound inside nr-vault ran on the adapter's secret-fetch URL but not on its OAuth token URL — an attacker who could write the oauth.tokenEndpoint config could pivot to internal infrastructure even though the secret URL itself was hardened.
  2. Cache-key collision risk. The token cache key originally hashed tokenEndpoint + clientIdSecret + grantType + scopes. Two adapters that shared a tokenEndpoint + clientId + scope but used different client secrets (e.g., a rotation in progress) would collide and serve the wrong cached token.

A separate finding (multi-axis review) was that error-path log messages and audit-log payloads could quote $exception->getMessage() which — for token failures — frequently echoes back the request body, leaking client_secret=, refresh_token=, and Authorization: Bearer values into warm storage.

Decision 

Three converging changes packaged as PRs #145 and #146:

  1. Mandatory hardened client. OAuthTokenManager's ClientInterface $httpClient constructor parameter no longer defaults to new GuzzleHttp\\Client(...). Callers MUST inject a hardened client — in practice every production caller threads one built by SecureHttpClientFactory::create(), so the SSRF middleware (see ADR-026: DNS-rebinding defence via CURLOPT_RESOLVE) protects the token endpoint just like every other outbound request. Removing the default closes the only path that constructed a raw Guzzle client at runtime. The architectural lock added in ADR-028: PHPat architectural lock for HTTP client construction enforces that no other code re-introduces one.
  2. tokenEndpoint isHostAllowed gate. OAuthTokenManager additionally accepts a SecureHttpClientFactory solely to gate the configured tokenEndpoint host through isHostAllowed() before any token request fires. The DNS-pin middleware on the injected client rejects dangerous resolved IPs at request time, but it does NOT apply the $GLOBALS['TYPO3_CONF_VARS']['HTTP']['allowed_hosts'] allowlist admins use to restrict outbound calls to a known set of partner hostnames. Without this extra gate, an attacker-controlled config pointing at http://169.254.169.254 would fail per-request DNS validation but the host-allowlist failure mode (a hard, audit-logged refusal pre-flight) was missing.

  3. Cache key fragmentation by secret. The OAuth token-cache key now includes $config->clientSecretSecret (the vault handle, not the secret value) so a credential rotation invalidates the cache cleanly:

    hash('xxh128', implode(':', [
    $config->tokenEndpoint,
    $config->clientIdSecret,
    $config->clientSecretSecret,  // new
    $config->grantType,
    $config->getScopesString(),
]));
    Copied!
  4. Credential redaction in error paths. A redactCredentials helper replaces client_secret=..., refresh_token=..., and Authorization: Bearer ... / Basic ... patterns before the original exception message reaches the logger or the audit log. Four call-sites cover both the synchronous Guzzle exception path and the async retry path.

Consequences 

Positive 

  • Token endpoints get the full DNS-rebinding + isHostAllowed defence that secret URLs already had.
  • Credential rotation no longer needs a manual cache-bust.
  • Token-endpoint failures leave no credential material in logs or audit rows — replayable secrets stay in the vault.
  • Tested via fuzz suite (regression added: malformed token responses no longer leak secrets via the exception chain).

Negative 

  • Constructor signature change is a positional BC break for callers using positional arguments: OAuthTokenManager now takes `VaultServiceInterface, ClientInterface, SecureHttpClientFactory, ?LoggerInterface, ?RequestFactoryInterface, ?StreamFactoryInterface, ?AuditLogServiceInterface`. The new required third parameter (SecureHttpClientFactory) shifts every parameter after it. Callers using named arguments are unaffected; positional callers must update — DI containers were updated in lockstep. Optional params are confined to the trailing positions (no required parameter follows an optional one) so adding future optional dependencies stays additive.
  • Adapter authors must invalidate their own caches if they cache OAuthTokenManager instances across config changes. (Most adapters fetch the manager from DI per request, so this is a non-issue in practice.)

Verified 

References