Skip to content

Add cached OAuth tokens and connected account renewal to Saloon - #72

Merged
binaryfire merged 8 commits into
0.4from
feature/saloon-token-management
Oct 24, 2026
Merged

binaryfire merged 8 commits into
0.4from
feature/saloon-token-management

Conversation

@binaryfire

@binaryfire binaryfire commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

Saloon can request OAuth tokens, but applications currently have to build token reuse, account persistence and coordinated refresh themselves. This adds two opt-in paths: cached authentication for grants that can be acquired again, and connected OAuth accounts for user grants with rotating refresh tokens.

Cached access tokens

Client-credentials connectors can return a reusable authenticator:

$request->authenticate($oauth->cachedAccessToken(store: 'redis'));

The cache key distinguishes the connector, resolved token endpoint, client credentials and requested scopes. Credentials are hashed before they enter the key. An optional key contribution covers provider-specific inputs such as an audience. Fresh connector instances can reuse the same cached token, while concurrent callers in one worker share an active acquisition and its result or failure.

CachedTokenAuthenticator also accepts a resolver for other token endpoints. It stores the actual authenticator, preserving custom authentication behavior, and calculates a fixed renewal deadline when a token is acquired. There is no worker-wide registry of completed tokens. Calling forget() clears that instance and its shared cache entry; it does not revoke the provider token or clear other live instances.

Connected OAuth accounts

Applications that need durable user grants can generate the oauth_accounts migration and add HasOAuthAccounts to an owning model. Access and refresh tokens use encrypted casts and are hidden from model serialization. The migration is not installed automatically, and credential caching is disabled by default.

$account = $user->oauthAccounts()->create([
    'provider' => 'example',
    'access_token' => $token->getAccessToken(),
    'refresh_token' => $token->getRefreshToken(),
    'expires_at' => $token->getExpiresAt(),
    'scopes' => $token->getScopes(),
]);

$request->authenticate($oauth->accountAuthenticator($account));

Account authentication reads current credentials rather than keeping a private token copy. Optional shared caching avoids database reads on ordinary calls. Saves and deletes invalidate committed entries, including when application context is restored before a transaction commits. Custom account models retain their connection, table, scopes and query builder.

A boot-time partition resolver supports applications that divide accounts by tenant or another ownership boundary. Account identity includes the partition, and authentication checks the active context before reading or using credentials. The application still owns its model's query scoping.

Renewal and failure handling

Renewal combines existing cache locks with a conditional database claim. The replacement is stored before it is used. Credential revisions and claim ownership prevent a late response from overwriting a reconnect or a newer attempt. Idle database connections are released before provider calls and waits, and renewal is rejected inside a caller transaction on the account connection.

Transient provider failures receive a small, bounded retry budget. Each attempt rereads current credentials. Longer Retry-After delays are left to the caller, local persistence failures do not trigger another provider call, and cancellation preserves the original exception even if cleanup fails. An expired token without a refresh token throws OAuthAccountReconnectRequiredException; rejected OAuth grants retain the provider's exception and response.

These measures do not promise exactly one remote request after a crash or an ambiguous network failure. Abandoned attempts can be recovered, and a provider that rejects reuse of a rotated refresh token can still require reconnection.

Other changes

  • Add reported scopes to OAuthAuthenticator, AccessTokenAuthenticator and both grant factories. Custom implementations must provide getScopes(): ?array and accept the factory's new optional scopes argument. null preserves unreported scope metadata; an explicit empty array replaces it.
  • Accept every declared callable form for OAuth request modifiers, including array callables.
  • Share the existing Retry-After parser between OAuth renewal and rate limiting without changing rate-limit policy.
  • Document token reuse, account storage, custom OAuth clients, partitioning and failure handling.
  • Temporarily pin development PHPStan to 2.3.1. Version 2.3.2 reports a false generic-type error in the unchanged Data collection cast; the same cast and standalone reproduction pass on 2.3.1. A tracked follow-up covers removing the pin after a fixed release is verified.

Validation

Formatting, full source and type-fixture analysis, the Saloon suite and Redis integration pass. Coverage includes serialized token reuse, grant identity, concurrent partitions, committed invalidation, cancellation, connection-pool release, reconnects and late refresh results. The Redis tests exercise lost cache locks and abandoned database claims and are included in the Redis, Redis Cluster and Valkey workflows.

Note

Add cached OAuth tokens and connected account renewal to Saloon

  • Adds CachedTokenAuthenticator for client-credentials flows: tokens are cached locally and in a shared cache store, keyed by connector, endpoint, client credentials, scopes, and an optional caller key. Concurrent workers share one token acquisition through the new TokenAcquisition coordinator.
  • Adds connected OAuth accounts: a UUID-keyed OAuthAccount model with encrypted credentials, the HasOAuthAccounts trait, the make:oauth-accounts-table migration command, and OAuthAccountAuthenticator. The OAuthAccounts service handles token retrieval, lock-coordinated renewal with credential revisions, refresh-claim fencing, bounded provider retries, and optional Redis-backed credential caching (disabled by default).
  • OAuth authenticators now carry provider-reported scopes (getScopes() on the OAuthAuthenticator contract), and grant factories receive an optional scopes argument. Request modifiers now accept any callable form, including array callables. Retry-After parsing moved from HasRateLimits to a shared RetryAfter class with no behavior change.
  • Behavioral Change: OAuthAuthenticator implementations must add a getScopes() method, and grant-specific authenticator factories receive a new optional scopes parameter. Retry-After parsing moved out of HasRateLimits.resolveRateLimitCooldown, which retains the 60-second fallback and maximum-cooldown check.

Macroscope summarized f4b8272.

Define cached token acquisition and optional encrypted OAuth account storage using the existing authentication and cache APIs. Specify expiry, credential identity, scope metadata, shared renewal claims, conditional persistence, and bounded recovery from transient provider failures.

Document partition hooks, model customization, transaction and cancellation behavior, database-load expectations, and focused verification. Keep provider-side replay limits explicit and retain application control of credentials and business-request retries.
PHPStan 2.3.2 rejects a correctly narrowed generic value in the existing Data collection cast. The unchanged cast and an isolated reproducer pass on 2.3.1 and fail on 2.3.2.

Pin the development requirement so fresh installs and CI use the verified analyzer. Keep the Data implementation and its type contracts intact, and track removal of the pin once a fixed release passes the source and type-fixture analyses.

Validation: the full source analysis and type fixtures pass with PHPStan 2.3.1.
Move the existing Retry-After parser into the HTTP namespace so account renewal can honor provider delays without duplicating the rate-limit implementation.

Preserve numeric delays, strict HTTP-date parsing, obsolete date formats and leap-second handling. Keep the rate limiter limit and its missing-or-invalid default in the rate-limit caller; the shared parser returns the provider delay without applying that policy.

Validation: formatting, full static analysis and the Saloon suite, including the existing rate-limit cases, pass.
Provide cached authentication for independently reacquirable grants and
durable account authentication for rotating user grants. Keep provider
credentials, request context and OAuth client selection application-owned.

CachedTokenAuthenticator retains the actual authenticator and a fixed early
renewal deadline. ClientCredentialsGrant builds a hashed identity from the
connector, resolved endpoint, credentials, scopes and caller contribution.
Concurrent callers share active acquisitions and failures within a worker;
completed tokens use the selected cache without a worker-wide token registry.

Add an opt-in OAuth accounts migration, encrypted model, owner relation and
authenticator. Shared credential caching uses committed invalidation and
preserves custom models, query builders and connection identities. A boot-time
partition resolver isolates storage and cache identity without retaining the
current account or partition in worker state. Deferred invalidation uses the
captured key even when application context is restored before commit.

Coordinate renewal with existing locks and conditional database claims.
Revision and claim checks prevent late responses from overwriting reconnects
or successor attempts. Store replacements before use, preserve omitted scopes
and refresh tokens, and surface an explicit reconnect-required exception for
expired credentials that cannot renew. Retry only classified provider failures
within a small bounded budget; local persistence failures never repeat the
provider request. Release idle database connections before provider calls and
waits, reject renewal inside caller transactions, and preserve cancellation
when cleanup or reporting also fails.

Extend OAuth metadata and grant factories with nullable reported scopes. Fix
request modifiers to accept all declared callable forms. Register cache
serialization classes, migration command discovery and testing-state cleanup.

Cover serialized token reuse, grant identity, concurrent acquisition and
partition isolation, committed cache invalidation, encryption, bounded retries,
cancellation, database-pool release and late-result ownership. Exercise lost
Redis leases and abandoned database claims with real stores, and include these
tests in the existing Redis, Redis Cluster and Valkey jobs.

Validation: formatting, full PHPStan source and type-fixture analysis, the
complete Saloon suite and the real Redis integration tests pass.
Replace token acquisition on every request with examples for cached client credentials and custom username/password token endpoints. Explain grant identity, store selection, fixed renewal deadlines, custom authenticator serialization and the limits of explicit invalidation.

Document opt-in account persistence, encrypted credentials, custom models, Socialite adapters, cache invalidation, partition context and coordinated renewal. Describe reconnect-required errors, bounded retries, transaction restrictions, cancellation and ambiguous remote refresh outcomes.

Record the scope metadata contract and factory signature change in the porting guide and package README. Mark the implementation plan verified and ready for a framework PR, retaining the separate integration boundary.
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add cached OAuth tokens and connected account renewal to Saloon

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Cache reacquirable OAuth tokens across connector instances and share in-flight acquisitions within
 a worker.
• Persist encrypted OAuth accounts and coordinate refreshes with locks, database claims, and bounded
 retries.
• Document the new APIs, expand coverage, and add scope metadata to OAuth authenticators.
Diagram

graph TD
  A["Authenticated Request"] --> B["Cached Token Auth"] --> C["Token Acquisition"] --> D[("Token Cache")]
  A --> E["Account Auth"] --> F["OAuthAccounts Service"] --> C
  F --> G[("Account Database")]
  F --> H["Provider Refresh"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Hold a database row lock through provider refresh
  • ➕ Uses one coordination primitive instead of a cache lock and durable claim.
  • ➖ Holds a database connection and transaction across network I/O.
  • ➖ Conflicts with the PR's pooled-connection and rollback safeguards.
2. Renew accounts through background jobs
  • ➕ Moves slow provider calls off the request path.
  • ➖ Requires a queue and additional lifecycle management.
  • ➖ Does not remove the need to fence late results or handle on-demand authentication.

Recommendation: Keep the layered approach: worker-local sharing reduces duplicate work, while the conditional database claim survives a lost cache lock. Review claim ownership, revision fencing, and failure recovery closely; the design does not promise exactly-once provider calls.

Files changed (40) +3373 / -163

Enhancement (19) +1294 / -11
OAuthAccountsTableCommand.phpGenerate an OAuth accounts migration +45/-0

Generate an OAuth accounts migration

• Adds 'make:oauth-accounts-table' and its 'oauth-accounts:table' alias. The migration is generated on demand, not installed automatically.

src/saloon/src/Console/Commands/OAuthAccountsTableCommand.php

oauth-accounts.stubDefine the optional OAuth accounts schema +41/-0

Define the optional OAuth accounts schema

• Provides a migration with owner, provider, credential, scope, renewal-deadline, revision, and refresh-claim columns.

src/saloon/src/Console/Commands/stubs/oauth-accounts.stub

OAuthAuthenticator.phpRequire reported scope metadata +7/-0

Require reported scope metadata

• Adds 'getScopes(): ?array' to the OAuth authenticator contract; null represents unreported scopes.

src/saloon/src/Contracts/OAuthAuthenticator.php

CachedToken.phpStore an authenticator with a fixed renewal deadline +36/-0

Store an authenticator with a fixed renewal deadline

• Wraps the actual authenticator, rejects an already-expired acquisition, and records when the cached entry stops being usable.

src/saloon/src/Data/CachedToken.php

OAuthAccountIdentity.phpCapture partition-aware account identity +53/-0

Capture partition-aware account identity

• Hashes model, connection, table, ID, and partition into a storage key. Builds scoped account queries without retaining a mutable model.

src/saloon/src/Data/OAuthAccountIdentity.php

TokenAcquisitionResult.phpHold an in-flight acquisition outcome +20/-0

Hold an in-flight acquisition outcome

• Carries a completed value or exception to callers waiting on the same acquisition.

src/saloon/src/Data/TokenAcquisitionResult.php

OAuthAccountReconnectRequiredException.phpSignal an expired account requiring reconnection +18/-0

Signal an expired account requiring reconnection

• Adds a specific exception for an expired account token without a refresh token.

src/saloon/src/Exceptions/OAuthAccountReconnectRequiredException.php

AccessTokenAuthenticator.phpExpose scopes on access-token authenticators +13/-0

Expose scopes on access-token authenticators

• Adds optional scope metadata to the constructor and implements 'getScopes()'.

src/saloon/src/Http/Auth/AccessTokenAuthenticator.php

CachedTokenAuthenticator.phpReuse independently reacquirable tokens +97/-0

Reuse independently reacquirable tokens

• Reads and stores authenticators through the selected cache store, sharing active acquisitions within a worker. 'forget()' clears its instance value and shared entry.

src/saloon/src/Http/Auth/CachedTokenAuthenticator.php

OAuthAccountAuthenticator.phpAuthenticate with current persisted account credentials +41/-0

Authenticate with current persisted account credentials

• Captures the account's storage identity, obtains current or renewed credentials on use, and checks the active partition before applying them.

src/saloon/src/Http/Auth/OAuthAccountAuthenticator.php

TokenAcquisition.phpShare active token acquisitions within a worker +65/-0

Share active token acquisitions within a worker

• Uses keyed coroutine locks to share an active operation's result or failure. Cancellation releases owner coordination without canceling its waiters.

src/saloon/src/Http/Auth/TokenAcquisition.php

TokenExpiry.phpCalculate fixed early-renewal times +26/-0

Calculate fixed early-renewal times

• Calculates a deadline before expiry using an allowance capped at 30 seconds and half the token's remaining lifetime.

src/saloon/src/Http/Auth/TokenExpiry.php

OAuthAccount.phpAdd the encrypted connected-account model +119/-0

Add the encrypted connected-account model

• Stores hidden, encrypted credentials and computes revisions and renewal deadlines on ordinary saves. Model events trigger committed cache invalidation after saves and deletes.

src/saloon/src/OAuthAccount.php

OAuthAccounts.phpCoordinate account reads and safe renewal +577/-0

Coordinate account reads and safe renewal

• Adds partition-aware account identity, optional shared caching, and committed invalidation. Renewal combines a cache lock with a conditional database claim, bounded provider retries, and revision-fenced persistence.

src/saloon/src/OAuthAccounts.php

SaloonServiceProvider.phpRegister migration command and serializable classes +16/-2

Register migration command and serializable classes

• Registers the OAuth accounts command and allows built-in token, date, and account model classes through the cache serialization policy.

src/saloon/src/SaloonServiceProvider.php

HasOAuthAccounts.phpAdd an owner-to-accounts relation +26/-0

Add an owner-to-accounts relation

• Provides a polymorphic 'oauthAccounts()' relation using the configured account model.

src/saloon/src/Traits/HasOAuthAccounts.php

AuthorizationCodeGrant.phpConnect authorization-code grants to stored accounts +34/-5

Connect authorization-code grants to stored accounts

• Adds an account authenticator backed by the existing refresh method. Passes reported scopes to factories and accepts array-callable request modifiers.

src/saloon/src/Traits/OAuth2/AuthorizationCodeGrant.php

ClientCredentialsGrant.phpAdd cached client-credentials authentication +40/-4

Add cached client-credentials authentication

• Adds 'cachedAccessToken()' keyed by grant identity and optional provider-specific input. Passes scopes to authenticator factories and accepts array-callable modifiers.

src/saloon/src/Traits/OAuth2/ClientCredentialsGrant.php

ParsesOAuthTokenResponses.phpParse provider-reported scopes +20/-0

Parse provider-reported scopes

• Converts reported space-delimited scopes to a list, preserves missing scope metadata as null, and rejects invalid types.

src/saloon/src/Traits/OAuth2/ParsesOAuthTokenResponses.php

Refactor (2) +124 / -122
RetryAfter.phpExtract reusable Retry-After parsing +121/-0

Extract reusable Retry-After parsing

• Moves HTTP-date and seconds parsing into a helper shared by OAuth renewal and rate limiting.

src/saloon/src/Http/RetryAfter.php

HasRateLimits.phpUse shared Retry-After parsing for cooldowns +3/-122

Use shared Retry-After parsing for cooldowns

• Removes the trait's private parser and delegates parsing to 'RetryAfter', retaining the rate limiter's maximum-delay fallback.

src/saloon/src/RateLimit/Traits/HasRateLimits.php

Tests (11) +1477 / -6
AfterEachTestSubscriber.phpReset OAuth account configuration between tests +1/-0

Reset OAuth account configuration between tests

• Flushes the account service's static model and partition configuration after each test.

src/testing/src/PHPUnit/AfterEachTestSubscriber.php

OAuthAccountRenewalTest.phpTest renewal after lost Redis locks +163/-0

Test renewal after lost Redis locks

• Exercises fresh and abandoned database claims across simulated workers. Verifies that a late result cannot overwrite a replacement.

tests/Integration/Saloon/Redis/OAuthAccountRenewalTest.php

AuthCodeFlowConnectorTest.phpTest scope propagation through authorization-code grants +16/-0

Test scope propagation through authorization-code grants

• Checks that exchange and refresh responses pass reported scopes to custom authenticator factories.

tests/Saloon/Feature/Oauth2/AuthCodeFlowConnectorTest.php

ClientCredentialsFlowConnectorTest.phpTest client-credentials scopes and callable modifiers +40/-4

Test client-credentials scopes and callable modifiers

• Covers reported, missing, and invalid scopes plus an array-callable modifier. Updates the custom factory fixture for scopes.

tests/Saloon/Feature/Oauth2/ClientCredentialsFlowConnectorTest.php

CustomOAuthAuthenticator.phpAdd scopes to the custom authenticator fixture +1/-0

Add scopes to the custom authenticator fixture

• Adds a nullable scopes property for tests of custom OAuth authenticators.

tests/Saloon/Fixtures/Authenticators/CustomOAuthAuthenticator.php

CustomResponseOAuth2Connector.phpPass scopes through the custom connector fixture +2/-2

Pass scopes through the custom connector fixture

• Updates the overridden factory signature and forwards scopes to the custom authenticator.

tests/Saloon/Fixtures/Connectors/CustomResponseOAuth2Connector.php

OAuthAccountTest.phpCover account storage, renewal, and failure handling +899/-0

Cover account storage, renewal, and failure handling

• Tests encryption, cache invalidation, partition isolation, conditional writes, concurrent renewal, retries, cancellation, transaction rejection, and reconnect behavior.

tests/Saloon/OAuthAccountTest.php

SaloonServiceProviderTest.phpAssert OAuth account configuration defaults +3/-0

Assert OAuth account configuration defaults

• Checks the default credential-cache TTL and key prefix.

tests/Saloon/SaloonServiceProviderTest.php

SensitiveParameterTest.phpCheck cached-token key sensitivity +2/-0

Check cached-token key sensitivity

• Adds the cached authenticator constructor's grant key to sensitive-parameter coverage.

tests/Saloon/SensitiveParameterTest.php

CachedTokenAuthenticatorTest.phpTest cached-token reuse and acquisition +260/-0

Test cached-token reuse and acquisition

• Covers serialized custom tokens, shared results and failures, fixed deadlines, forgetting, grant identity, and idle-connection release.

tests/Saloon/Unit/Oauth2/CachedTokenAuthenticatorTest.php

TokenAcquisitionTest.phpTest acquisition cancellation semantics +90/-0

Test acquisition cancellation semantics

• Verifies that canceling a waiter does not disturb its owner and that waiters can proceed after owner cancellation.

tests/Saloon/Unit/Oauth2/TokenAcquisitionTest.php

Documentation (5) +452 / -23
2026-10-24-1738-saloon-oauth-token-management.mdRecord the token-management design +250/-0

Record the token-management design

• Documents the intended caching, account persistence, renewal, partitioning, failure handling, and verification strategy.

docs/plans/2026-10-24-1738-saloon-oauth-token-management.md

todo.mdTrack removal of the PHPStan pin +1/-0

Track removal of the PHPStan pin

• Adds a follow-up to restore the version range after verifying a fixed analyzer release.

docs/todo.md

porting-from-laravel.mdExplain the OAuth scope contract change +2/-0

Explain the OAuth scope contract change

• Notes that custom OAuth authenticators need 'getScopes()' and grant factory overrides receive an optional scopes argument.

src/docs/porting-from-laravel.md

saloon.mdDocument cached tokens and connected accounts +198/-22

Document cached tokens and connected accounts

• Adds examples and guidance for token reuse, account migration and storage, caching, renewal, custom clients, and partitioning. Replaces the boot-time token-fetch example.

src/docs/saloon.md

README.mdDocument OAuth API differences +1/-1

Document OAuth API differences

• Describes the new scope contract and optional grant factory argument alongside existing differences from upstream Saloon.

src/saloon/README.md

Other (3) +26 / -1
redis.ymlRun Saloon renewal tests in shared-store CI +3/-0

Run Saloon renewal tests in shared-store CI

• Adds the Saloon Redis integration tests to Redis, Redis Cluster, and Valkey workflows.

.github/workflows/redis.yml

composer.jsonPin development PHPStan to 2.3.1 +1/-1

Pin development PHPStan to 2.3.1

• Pins PHPStan pending a fix for a reported generic-type analysis regression in 2.3.2.

composer.json

saloon.phpConfigure optional OAuth account caching +22/-0

Configure optional OAuth account caching

• Adds disabled-by-default credential caching settings and an optional renewal lock store.

src/saloon/config/saloon.php

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. A saved account can move to another tenant ✓ Resolved
Description
OAuthAccount validates an existing account through identity(), which checks its original
partition but never rejects a changed partition attribute. If an account is loaded in one partition
and its partition column is changed before save(), the write can move the row into another
partition despite the active context still being the first.
Code

src/saloon/src/OAuthAccount.php[R65-67]

+            } else {
+                $accounts->identity($account);
+            }
Evidence
The saving hook calls identity() for existing models, but that method compares only
getRawOriginal() with the active partition. It does not inspect the value that the model save will
write.

src/saloon/src/OAuthAccount.php[60-67]
src/saloon/src/OAuthAccounts.php[175-195]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Existing-account saves validate the persisted partition but can write a different partition supplied on the model.
## Fix Focus Areas
- src/saloon/src/OAuthAccount.php[60-67]
- src/saloon/src/OAuthAccounts.php[175-195]
## Recommended Fix
During the saving hook, reject a dirty partition column whose current value differs from the account's persisted partition. Keep identity capture based on the persisted value for cache invalidation, and add a test that attempts to move an existing account across partitions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Renewal deadline shifts in non-UTC apps ✓ Resolved
Description
The saving hook builds refresh_at with CarbonImmutable::createFromTimestampUTC(). Eloquent's
fromDateTime() stores that value's UTC wall-clock time without converting it to the app timezone,
but asDateTime() reads the stored string back in the app timezone
(date_default_timezone_set(app.timezone)). When app.timezone isn't UTC, the deadline moves by
the timezone offset. East of UTC, renewal comes hours early: a one-hour token looks due on every
call, so every request triggers a provider refresh. West of UTC, renewal comes hours late, after the
access token has expired. The renewal write at OAuthAccounts::renew() line 386 uses the same
conversion, so the wrong deadline is written again after every refresh.
Code

src/saloon/src/OAuthAccount.php[R76-77]

+                $refreshAt = TokenExpiry::refreshAt($account->expires_at);
+                $account->refresh_at = $refreshAt === null ? null : CarbonImmutable::createFromTimestampUTC($refreshAt);
Evidence
asDateTime() keeps the timezone of an incoming DateTimeInterface (here UTC), and fromDateTime()
formats it as wall-clock time without a zone. On read, asDateTime() parses the string with
Date::createFromFormat in the default timezone, which the framework sets from app.timezone.
expires_at is consistent because resolveOAuthExpiry uses Date::now() in the app timezone, but
refresh_at is not.

src/database/src/Eloquent/Concerns/HasAttributes.php[1474-1538]
src/foundation/src/Bootstrap/LoadConfiguration.php[78-82]
src/saloon/src/OAuthAccounts.php[380-391]
src/saloon/src/OAuthAccounts.php[317-319]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`refresh_at` is built with `CarbonImmutable::createFromTimestampUTC()`. Eloquent stores that value's UTC wall-clock time and reads it back in the app timezone, so the renewal deadline shifts by the timezone offset whenever `app.timezone` is not UTC.
## Fix Focus Areas
- src/saloon/src/OAuthAccount.php[76-77]
- src/saloon/src/OAuthAccounts.php[380-386]
## Recommended Fix
Build the deadline in the default timezone, for example `CarbonImmutable::createFromTimestamp($refreshAt, date_default_timezone_get())` or `Date::createFromTimestamp($refreshAt)`, in both places. Add a test that sets a non-UTC `app.timezone` and checks that the `refresh_at` read back has the expected Unix timestamp.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. A changed context hides a grant rejection 🐞 Bug ☼ Reliability
Description
OAuthAccounts::renew() calls ensurePartition() in its invalid_grant recovery branch before it
can rethrow the provider exception. If a refresh callback changes the active partition and then
throws an invalid-grant response, claim cleanup can succeed, but that check replaces the original
provider exception with a partition LogicException.
Code

src/saloon/src/OAuthAccounts.php[R415-416]

+            if ($oauthError === 'invalid_grant') {
+                $this->ensurePartition($identity);
Evidence
The callback exception is classified as an OAuth error, cleanup uses the captured identity, and the
invalid-grant branch then checks the current partition before reaching the original rethrow.
ensurePartition() throws when that context differs.

src/saloon/src/OAuthAccounts.php[366-371]
src/saloon/src/OAuthAccounts.php[399-426]
src/saloon/src/OAuthAccounts.php[210-215]
tests/Saloon/OAuthAccountTest.php[765-793]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An invalid-grant response can be masked when the refresh callback has changed the active partition.
## Fix Focus Areas
- src/saloon/src/OAuthAccounts.php[399-426]
## Recommended Fix
After claim cleanup, skip context-dependent invalid-grant recovery and rethrow the original provider exception if the active partition no longer matches the captured identity. Add a test combining an invalid-grant response with a partition change during the callback.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. A rotated refresh token is discarded ✓ Resolved
Description
renew() throws UnexpectedValueException when $renewed->hasExpired() is true. At that point the
provider call has already succeeded, so the new refresh token it returned is never stored, and the
outer catch only clears the claim. With providers that rotate refresh tokens, the stored token has
already been invalidated, so one reply with expires_in: 0 (which resolveOAuthExpiry accepts)
leaves the account needing reconnection.
Code

src/saloon/src/OAuthAccounts.php[R374-378]

+            $this->ensurePartition($identity);
+
+            if ($renewed->hasExpired()) {
+                throw new UnexpectedValueException('The refreshed OAuth token has already expired.');
+            }
Evidence
The expiry check runs after $refreshUsing($current) returns and before the conditional write. Its
exception goes to the catch block, which only clears this attempt's claim and rethrows, so the new
refresh token is lost. resolveOAuthExpiry accepts expires_in of 0, which gives an expiry of now.

src/saloon/src/OAuthAccounts.php[366-428]
src/saloon/src/Traits/OAuth2/ParsesOAuthTokenResponses.php[60-89]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When the refreshed access token is already expired, `renew()` throws before persisting anything. The provider may have rotated the refresh token, and that new token is lost.
## Fix Focus Areas
- src/saloon/src/OAuthAccounts.php[374-398]
## Recommended Fix
If `$renewed->getRefreshToken()` is non-null and differs from the stored one, write it with the same conditional update (matching revision and claim, with a new revision and a cleared claim) before throwing the expired-token exception, then invalidate the cache. The next operation can then renew with the valid refresh token.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can choose which labels appear on a finding, and whether they show icons or text

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/saloon/src/OAuthAccount.php Outdated
Comment thread src/saloon/src/OAuthAccounts.php
Comment thread src/saloon/src/OAuthAccount.php Outdated
Comment thread src/saloon/src/OAuthAccounts.php Outdated
@greptile-apps

greptile-apps Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5 Tier: plus

[High impact] The PR appears safe to merge, with all four previous findings addressed and no new actionable issues found.

Summary

Adds reusable OAuth tokens and connected accounts with encrypted credentials and coordinated renewal.

  • Client-credentials requests can reuse tokens from a shared cache.
  • Applications can store user OAuth grants and renew their credentials on demand.
  • OAuth tokens now keep the scopes that the provider reports.
  • OAuth request modifiers accept array callables as well as closures.
  • OAuth renewal and rate limiting share Retry-After parsing.

No new actionable issues were found. Runtime tests were not run during this review.

Reviews (2) · Last reviewed commit: "Clarify OAuth account transaction and re..." · Reviewed by Greptile

Comment thread src/saloon/src/OAuthAccount.php Outdated
Comment thread src/saloon/src/OAuthAccounts.php
Comment thread src/saloon/src/OAuthAccounts.php
Comment thread src/saloon/src/OAuthAccounts.php Outdated
Pin the Database and Foundation development requirements to PHPStan 2.3.1, matching the root manifest while the 2.3.2 generic loop-narrowing regression remains unresolved.

Record all three manifests in the removal task so the ranges are restored together. Keep the package manifest consistency check intact; its failure exposed the incomplete pin.

Validation: Composer manifest checks and full source and type-fixture analysis pass.
Validate the current partition before normal saves and deletes, and reject partition changes on existing accounts. Keep application deleting observers active. Normalize write aliases only in cache and renewal identity while preserving the requested connection for transaction ownership.

Store computed renewal deadlines and callback expiry instants through the normal numeric date casts so non-UTC applications do not shift them. Check caller transactions before joining a renewal; allow unclaimed, unexpired tokens to remain usable during the early-renewal window without contacting the provider.

Preserve the original provider rejection when an account is concurrently deleted. Conditionally persist rotated credentials even when the returned access token has already expired, then reject its use without an immediate retry. Late results still cannot overwrite a reconnect or a replacement claim.

Add regressions for partition mutations, model observers, write-alias invalidation, non-UTC dates, transactional waiters, deferred early renewal, concurrent deletion and expired rotated results. Formatting, full source and type analysis, the Saloon suite and shared Redis renewal integration pass.
Document use of unexpired, unclaimed credentials inside caller transactions and rejection before waiting when renewal is required. Explain why an expired access result is not used even though its rotated refresh credentials are retained, including the exception applications receive.

Describe partition checks on normal saves and deletes and the prohibition on moving an existing account between partitions. Align the implementation plan with date-cast conventions, write-alias identity, conditional credential persistence and the current PR review state.
@binaryfire

Copy link
Copy Markdown
Member Author

The latest commits address the account isolation, expiry and renewal findings:

  • Saves and deletes validate the partition before changing a row. Existing accounts cannot move between partitions.
  • Write aliases share credential invalidation and renewal coordination while retaining their own transaction connection. Renewal dates preserve their intended instant in non-UTC applications.
  • Transaction checks run before waiting. Unexpired, unclaimed tokens remain usable during the early-renewal window without starting a refresh.
  • Concurrent deletion preserves the provider rejection. Expired refresh results retain rotated credentials for a later attempt, but the expired access token is never used.

The split-package PHPStan constraints now match the root pin, fixing the manifest consistency failure. Formatting, source and type analysis, the Saloon suite, manifest checks and Redis renewal integration pass locally. CI is running on the updated branch.

Each inline finding has a reply explaining the change or why the existing behavior remains intentional.

@binaryfire
binaryfire merged commit 9b59049 into 0.4 Oct 24, 2026
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant