From 4b88ed2698b1fec5929ea9c9978ce76449400e13 Mon Sep 17 00:00:00 2001 From: Elias Luhr Date: Thu, 1 Oct 2026 13:29:37 +0200 Subject: [PATCH] fix(auth): accept refresh responses without refresh_token [*] Launchpad's legacy `type=refresh` response carries only `access_token` (see the "legacy format refresh" case in basecamp/basecamp-sdk). The client required `refresh_token` and `expires_in` as well, so every refresh failed with "BC4 token exchange failed (status=200)" once the first access token expired, and the integration needed a manual re-authorization every two weeks. Only `access_token` is required now. Without a rotated refresh_token the current one is kept; without `expires_in` the documented two-week lifetime is assumed. Co-Authored-By: Claude Opus 5.5 --- src/Authentication/OAuth2Authentication.php | 20 +++++-- .../OAuth2AuthenticationTest.php | 60 +++++++++++++++++++ 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/src/Authentication/OAuth2Authentication.php b/src/Authentication/OAuth2Authentication.php index 4624b66..5fad5d4 100644 --- a/src/Authentication/OAuth2Authentication.php +++ b/src/Authentication/OAuth2Authentication.php @@ -16,8 +16,11 @@ * - Proactive refresh when `expires_at < now + 60s` * - Reactive refresh on `401` (handled by {@see Bc4Client::request()} via * {@see refresh()}) - * - Refresh-token rotation: every refresh yields a new refresh_token that - * replaces the previous one in storage + * - Refresh-token rotation when Launchpad sends one: a new refresh_token + * replaces the previous one in storage. The legacy `type=refresh` + * response usually carries only `access_token` (see the "legacy format + * refresh" case in basecamp/basecamp-sdk); the current refresh_token + * then stays valid and is kept * - On `400 invalid_grant`: storage is marked `requires_reauth` and an * {@see InvalidGrantException} is thrown * @@ -28,6 +31,12 @@ class OAuth2Authentication implements AuthenticationInterface { private const LAUNCHPAD_TOKEN_URL = 'https://launchpad.37signals.com/authorization/token'; + /** + * Access-token lifetime documented by 37signals ("2 week lifetime, + * currently"), used when a refresh response omits `expires_in`. + */ + private const DEFAULT_EXPIRES_IN = 1209600; + private ?string $cachedAccessToken = null; private ?string $cachedExpiresAt = null; private ?HttpClientInterface $launchpadClient = null; @@ -138,14 +147,15 @@ private function exchangeRefreshToken(string $refreshToken): array throw new InvalidGrantException('Refresh token rejected by Launchpad (invalid_grant)'); } - if ($status >= 400 || !is_array($body) || !isset($body['access_token'], $body['refresh_token'], $body['expires_in'])) { + if ($status >= 400 || !is_array($body) || empty($body['access_token'])) { throw new \RuntimeException(sprintf('BC4 token exchange failed (status=%d)', $status)); } return [ 'access_token' => (string) $body['access_token'], - 'refresh_token' => (string) $body['refresh_token'], - 'expires_in' => (int) $body['expires_in'], + // no rotation: the current refresh_token stays valid + 'refresh_token' => !empty($body['refresh_token']) ? (string) $body['refresh_token'] : $refreshToken, + 'expires_in' => isset($body['expires_in']) ? (int) $body['expires_in'] : self::DEFAULT_EXPIRES_IN, ]; } diff --git a/tests/Authentication/OAuth2AuthenticationTest.php b/tests/Authentication/OAuth2AuthenticationTest.php index aab1378..573331b 100644 --- a/tests/Authentication/OAuth2AuthenticationTest.php +++ b/tests/Authentication/OAuth2AuthenticationTest.php @@ -53,6 +53,66 @@ public function testProactiveRefreshOnExpiredAccessToken(): void $this->assertSame(1, $launchpad->getRequestsCount(), 'Exactly one Launchpad call'); } + public function testRefreshWithoutRotationKeepsCurrentRefreshToken(): void + { + // Legacy `type=refresh` response: access_token only, no rotation and + // no expires_in (cf. "legacy format refresh" in basecamp/basecamp-sdk). + $launchpad = new MockHttpClient([ + new MockResponse(json_encode(['access_token' => 'access-2']), ['http_code' => 200]), + ]); + + $exchanged = null; + $storage = $this->expiredTokenStorage(function (callable $exchange) use (&$exchanged) { + $exchanged = $exchange('refresh-1'); + + return [ + 'access_token' => $exchanged['access_token'], + 'refresh_token' => $exchanged['refresh_token'], + 'expires_at' => '2030-01-01T00:00:00+00:00', + ]; + }); + + $auth = new OAuth2Authentication('cid', 'secret', 'TestApp', 't@e.de', $storage, $launchpad); + + $this->assertSame('access-2', $auth->getAccessToken()); + $this->assertSame([ + 'access_token' => 'access-2', + 'refresh_token' => 'refresh-1', + 'expires_in' => 1209600, + ], $exchanged); + } + + public function testRefreshResponseWithoutAccessTokenFails(): void + { + $launchpad = new MockHttpClient([ + new MockResponse(json_encode(['expires_in' => 1209600]), ['http_code' => 200]), + ]); + $storage = $this->expiredTokenStorage(fn (callable $exchange) => $exchange('refresh-1')); + + $auth = new OAuth2Authentication('cid', 'secret', 'TestApp', 't@e.de', $storage, $launchpad); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('BC4 token exchange failed (status=200)'); + $auth->getAccessToken(); + } + + private function expiredTokenStorage(callable $refreshAndPersist): TokenStorageInterface + { + $storage = $this->createStub(TokenStorageInterface::class); + $storage->method('isFresh')->willReturnCallback( + static fn (string $iso) => (new \DateTimeImmutable($iso))->getTimestamp() > time() + 60, + ); + $storage->method('loadTokens')->willReturn([ + 'access_token' => null, + 'refresh_token' => 'refresh-1', + 'expires_at' => '2020-01-01T00:00:00+00:00', + 'requires_reauth' => false, + ]); + $storage->method('refreshAndPersist')->willReturnCallback($refreshAndPersist); + + return $storage; + } + public function testInvalidGrantMarksRequiresReauth(): void { $launchpad = new MockHttpClient([