Socialite: harden provider lifecycles and first-party extensibility - #485
Conversation
Keep the manager configuration repository aligned with its active container when tests or rebinding replace the application instance. This removes the need for package-specific rebinding workarounds and adds coverage proving cached managers resolve configuration from the replacement container.
Alias the concrete pool manager and recycler to their canonical contracts so concrete and contract consumers use the same worker-lifetime registries. Preserve application overrides while adding focused provider coverage for shared pool and recycler state.
Alias the array channel manager to the channel-manager contract when the application has not supplied its own binding. This keeps concrete and contract resolutions on one worker-local repository while preserving both early and late application overrides.
Give each cached provider a monotonic process-lifetime context namespace, move request ownership into coroutine context, and refresh the active request whenever a cached driver is resolved. Unify concrete manager and Factory resolution without breaking Factory-only fakes, remove duplicate rebinding behavior, protect raw provider-context internals, regenerate facade metadata, and make manager and fake return contracts truthful. The regressions cover tenant isolation, request reuse failures, enum drivers, config key edge cases, and manager rebinding.
Parse token responses through protected access-token, refresh-token, expiry, scope, and whole-response seams shared by login and refresh flows. Preserve unrotated refresh tokens, accept bounded protocol digit strings, publish complete token responses on returned users, and cache authenticated users only after mapping and decoration succeed. Restore the current OAuth 2 user fake and cover parser overrides, transactional memoization, direct token lookup, response publication, and fake-manager separation.
Use one exact-URL, bounded JWKS implementation for generic OIDC, Google, and Facebook, with cache-control expiry, atomic publication, and one throttled rotation retry. Correct issuer, audience, nonce, discovery, and failure-class behavior; preserve complete token-response mapping without worker-retained response state; and remove the obsolete phpseclib dependency. The coverage exercises tenant URL switching, malformed metadata, cache directives, failed refresh cooldowns, key rotation, disabled nonce flows, scalar and list audiences, and provider-specific validation.
Send Bitbucket, GitLab, and generic provider credentials through their correct Bearer transport, update GitLab user lookup to the current endpoint, and make LinkedIn image mapping tolerate missing optional nodes. Mark secret-bearing provider frames with SensitiveParameter while keeping non-secret diagnostics visible. Add exact request-shape, optional-image, and derived provider-surface reflection coverage so new first-party providers inherit the same security contract.
Document custom OAuth 2 and OpenID Connect providers, protected response parsers, full token responses, request access, trusted audiences, testing fakes, and provider registration in Laravel-style prose. Clarify boot-time versus request-local configuration, require tenant configuration on both redirect and callback requests, explain session requirements for PKCE and nonce validation, and record the intentional OAuth 1 and legacy Twitter differences without exposing internal lifecycle machinery.
Mark Socialite complete in the package checklist and routing index, record all accepted and rejected findings, and capture the final lifecycle, security, performance, compatibility, and validation outcomes. Record Support, Object Pool, and Reverb corrections at their owning package entries, add their dependency-index routes, and retain the completed Sanctum records merged from the latest 0.4 branch.
Document the final Socialite architecture, accepted findings, rejected complexity, implementation boundaries, regression strategy, and performance and compatibility gates. The plan captures the current Laravel and SocialiteProviders research, OAuth and OpenID Connect protocol decisions, coroutine and worker ownership model, shared JWKS design, cross-package corrections, and completion review used for this work.
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughSocialite now isolates provider state per coroutine, exposes typed OAuth contracts, normalizes token responses, validates OIDC tokens, caches JWKS data, uses Bearer token transport, and supports custom providers and fake users. Related lifecycle bindings, dependencies, documentation, and tests were updated. ChangesSocialite correctness and lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SocialiteManager
participant OAuthProvider
participant JWKS
Client->>SocialiteManager: request provider
SocialiteManager->>OAuthProvider: create or reuse provider
Client->>OAuthProvider: exchange code and retrieve user
OAuthProvider->>JWKS: fetch or refresh signing keys
JWKS-->>OAuthProvider: return JWKS response
OAuthProvider-->>Client: return validated User
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThe PR hardens Socialite’s coroutine-local provider state, OAuth/OIDC validation, token handling, and bounded JWKS reuse while unifying several worker-lifetime container identities.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/socialite/src/Two/Concerns/InteractsWithJwks.php | Introduces the shared JWKS cache and now bounds responses without usable cache directives to the configured fallback lifetime. |
| src/socialite/src/Two/AbstractProvider.php | Centralizes token-response parsing and publishes memoized users only after mapping and token metadata assignment complete. |
| src/socialite/src/Concerns/HasProviderContext.php | Gives cached provider instances monotonic namespaces while keeping mutable provider state in coroutine context. |
| src/socialite/src/Two/OpenIdProvider.php | Consolidates OIDC decoding and validates nonce, audience, and issuer claims through the shared provider boundaries. |
| src/object-pool/src/ObjectPoolServiceProvider.php | Unifies concrete and contract resolutions around the same worker-lifetime pool manager and recycler owners. |
| src/reverb/src/ReverbServiceProvider.php | Unifies concrete and contract channel-manager resolution while preserving application binding precedence. |
| src/support/src/Manager.php | Refreshes the retained configuration repository when a cached manager is moved to another container. |
Reviews (3): Last reviewed commit: "Resolve Socialite plan approval state" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
src/socialite/src/Two/AbstractProvider.php (1)
296-302: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueDeclare the failure mode when
access_tokenis missing.
parseAccessTokendeclares astringreturn type.Arr::getreturnsnullwhen the provider returns an error body withoutaccess_token. PHP then throws aTypeErrorfrom inside the parser. The message does not identify the provider or the response. Consider throwing a dedicated Socialite exception so callers can handle failed token exchanges.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/socialite/src/Two/AbstractProvider.php` around lines 296 - 302, Update AbstractProvider::parseAccessToken to explicitly detect a missing access_token before returning it, and throw the established dedicated Socialite exception with provider context and the token response details instead of allowing a TypeError. Preserve returning the access token unchanged when present.src/socialite/src/Two/User.php (1)
40-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
new staticso subclasses get their own fake instance.
fake()returnsnew self, so a subclass ofUserreceives a baseUserinstance. If you want provider-specific user subclasses to supportfake(), usenew staticand declare the return type asstatic.♻️ Proposed change
- public static function fake(#[SensitiveParameter] array $attributes = []): self + public static function fake(#[SensitiveParameter] array $attributes = []): static { @@ - return (new self)->setRaw($attributes)->map($attributes) + return (new static)->setRaw($attributes)->map($attributes)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/socialite/src/Two/User.php` around lines 40 - 61, Update User::fake() to instantiate the late-static-bound class with new static instead of new self, and change its return type from self to static so subclasses receive their own fake instance while preserving the existing attribute setup.src/socialite/src/Two/OpenIdProvider.php (1)
148-173: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffSingle-entry caches thrash when one provider instance serves several tenants. Both caches hold one entry in an instance property. The
SocialiteManagercaches the provider per driver and worker, while the effective URL comes fromsetConfig, which stores the value in coroutine context. When concurrent coroutines use different tenants, each request replaces the other's entry and triggers a new HTTP fetch. The check-and-return is atomic under cooperative scheduling, so no wrong-tenant data is returned; the cost is repeated network calls on the login path.
src/socialite/src/Two/OpenIdProvider.php#L148-L173: key$openidConfigby URL as a bounded map instead of a single['url' => ..., 'config' => ...]entry, so tenant A and tenant B do not evict each other.src/socialite/src/Two/Concerns/InteractsWithJwks.php#L61-L102: apply the same bounded per-URL map to$jwksand$jwksRefreshAttempt, and keep the existing expiry and cooldown handling per entry.Set an explicit maximum entry count so a hostile or misconfigured tenant list cannot grow the map without bound.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/socialite/src/Two/OpenIdProvider.php` around lines 148 - 173, In src/socialite/src/Two/OpenIdProvider.php lines 148-173, update getOpenIdConfig to cache configurations in a URL-keyed bounded map so tenant URLs retain separate entries; define and enforce an explicit maximum entry count with an eviction policy. In src/socialite/src/Two/Concerns/InteractsWithJwks.php lines 61-102, apply the same bounded URL-keyed map approach to jwks and jwksRefreshAttempt, preserving each entry’s existing expiry and refresh-cooldown behavior.tests/Socialite/LinkedInProviderTest.php (1)
111-124: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMake the avatar selection rule unambiguous.
The 800px image is both the widest and the last valid element. The assertion passes whether the provider picks the widest image or the last image. Place the widest image before a smaller one so the test pins down the intended rule.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Socialite/LinkedInProviderTest.php` around lines 111 - 124, Reorder the valid image fixtures in the profilePicture test data so the 800px avatar-original image appears before the 100px avatar image, while retaining the existing assertions for getAvatar() and avatar_original. This ensures the test distinguishes widest-image selection from simply choosing the last valid element.tests/Socialite/GitlabProviderTest.php (1)
17-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the custom host path.
This test only exercises the default
https://gitlab.comhost.GitlabProvider::setHost()and thehostconfiguration key both feedgetHost()and change the request URL. Add a case that sets a custom host and asserts the trailing slash is trimmed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Socialite/GitlabProviderTest.php` around lines 17 - 42, Extend testUserRequestUsesTheVersionedApiAndBearerAuthorization to configure GitlabProvider with a custom host via setHost, then assert the mocked request uses that host without a trailing slash while preserving the /api/v4/user path and bearer header expectations.tests/Socialite/OpenIdProviderTest.php (1)
809-849: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated JWKS test helpers. Three test files now carry byte-identical
createRsaKeyPair,jwks, andbase64UrlEncodeimplementations, plus near-identical JWKS response builders that differ only by URL. The shared root cause is the absence of a common JWKS test helper, so every new OIDC provider test copies the same code and all copies must be updated together when the JWKS contract changes.Move these helpers into one trait or fixture class under
tests/Socialite/Fixtures/, then have each test file use it. Also consider caching the generated key pairs perkidinside that helper. Each call tocreateRsaKeyPairgenerates a fresh 2048-bit RSA key, and the suite now performs many such generations per run.
tests/Socialite/OpenIdProviderTest.php#L809-L849: remove the localcreateRsaKeyPair,jwks, andbase64UrlEncodemethods and use the shared helper.tests/Socialite/GoogleProviderIdTokenTest.php#L231-L267: remove the same three local helpers and use the shared helper.tests/Socialite/FacebookProviderTest.php#L183-L219: remove the same three local helpers and use the shared helper.As per coding guidelines: "Put standalone test support files under a capitalized
Fixtures/directory."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Socialite/OpenIdProviderTest.php` around lines 809 - 849, Extract createRsaKeyPair, jwks, and base64UrlEncode into one shared trait or fixture class under tests/Socialite/Fixtures/, optionally caching generated keys by kid, and update imports/usages accordingly. Remove the duplicated helpers from tests/Socialite/OpenIdProviderTest.php lines 809-849, tests/Socialite/GoogleProviderIdTokenTest.php lines 231-267, and tests/Socialite/FacebookProviderTest.php lines 183-219; each site requires the same removal and delegation to the shared helper.Source: Coding guidelines
tests/Socialite/GoogleProviderIdTokenTest.php (1)
79-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
setConfigfor per-request trusted audience override.
setConfig()merges into the current request configuration, whilewithConfig()sets the provider baseline for the worker lifetime.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Socialite/GoogleProviderIdTokenTest.php` around lines 79 - 93, Update testItAcceptsConfiguredTrustedAudiences to apply trusted_audiences through the per-request setConfig path, preserving the test’s existing audience and user ID assertions; do not use withConfig, which establishes a worker-lifetime baseline.tests/Socialite/Fixtures/OpenIdTestProviderStub.php (1)
32-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unnecessary pass-through override.
OpenIdProvideralready declaresprotected function getUserByToken(#[SensitiveParameter] string $token): array. The stub can keepgetProviderUserByTokenand call inheritedgetUserByToken, or remove that public wrapper too. If you keep the wrapper, useparent::getUserByToken($token)so the fixture can call the stubbed base provider implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Socialite/Fixtures/OpenIdTestProviderStub.php` around lines 32 - 35, Remove the unnecessary getUserByToken override from OpenIdTestProviderStub, since OpenIdProvider already provides it. Preserve getProviderUserByToken only if needed, ensuring it calls the inherited parent::getUserByToken($token) implementation.tests/Socialite/FacebookProviderTest.php (1)
103-114: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a stable assertion for the JWK error message.
firebase/php-jwtowns"kid" invalid, unable to lookup correct key, so this exact-mismatch test can fail across dependency updates. UseassertStringContainsString('"kid" invalid', $e->getMessage())like the matching OpenID test instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Socialite/FacebookProviderTest.php` around lines 103 - 114, Update testAnUnknownKidRaisesTheLibraryAuthenticationFailure to capture the expected UnexpectedValueException and assert that its message contains '"kid" invalid' using assertStringContainsString, rather than requiring the dependency-owned full error message. Keep the existing unknown-key token setup and exception type assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/object-pool/src/ObjectPoolServiceProvider.php`:
- Around line 20-22: Register PoolManager as a singleton before aliasing it to
Factory, and apply the same concrete-singleton-plus-contract-alias pattern to
ArrayChannelManager/ChannelManager in
src/reverb/src/ReverbServiceProvider.php:93 and SocialiteManager/Factory in
src/socialite/src/SocialiteServiceProvider.php:17; update the anchor in
src/object-pool/src/ObjectPoolServiceProvider.php:20-22 accordingly.
In `@src/socialite/README.md`:
- Around line 4-14: Reduce the src/socialite/README.md content to a brief
package description and the existing documentation link, removing the detailed
compatibility and provider-behavior bullets. Retain or move that guidance in the
canonical socialite documentation at src/boost/docs/socialite.md.
In `@src/socialite/src/Socialite.php`:
- Around line 14-22: Update the annotations in Socialite to use imported short
type names instead of fully qualified names: add use statements for Provider,
AbstractProvider, SocialiteManager, and Container, then replace their qualified
references in the `@method` declarations while preserving the existing signatures.
In `@src/socialite/src/Two/OpenIdProvider.php`:
- Around line 184-190: Update getUserByTokenResponse to validate that response
contains a string id_token before calling getUserByOIDCToken. If missing or
invalid, throw the appropriate Socialite exception with a clear cause-specific
message, using a dedicated exception type if ConfigurationFetchingException is
not suitable.
In `@tests/Socialite/AbstractProviderTest.php`:
- Around line 164-187: Update testRecycledObjectIdsCannotReuseProviderContext to
avoid requiring PHP to recycle the object ID within 1000 allocations: either
make the marker-key validation deterministic or skip the assertion when no
matching replacement is found, while retaining the existing validation when a
recycled ID is obtained.
In `@tests/Socialite/OpenIdProviderTest.php`:
- Around line 278-306: Guard the id_token access in
OpenIdProvider::getUserByTokenResponse by validating that it exists and is a
string, then throw the named MissingIdTokenException when validation fails
before calling getUserByOIDCToken. Update
testMissingIdTokenFailsAtTheRequiredResponseBoundary to assert
MissingIdTokenException and remove the set_error_handler, warning capture, and
TypeError-specific assertions.
---
Nitpick comments:
In `@src/socialite/src/Two/AbstractProvider.php`:
- Around line 296-302: Update AbstractProvider::parseAccessToken to explicitly
detect a missing access_token before returning it, and throw the established
dedicated Socialite exception with provider context and the token response
details instead of allowing a TypeError. Preserve returning the access token
unchanged when present.
In `@src/socialite/src/Two/OpenIdProvider.php`:
- Around line 148-173: In src/socialite/src/Two/OpenIdProvider.php lines
148-173, update getOpenIdConfig to cache configurations in a URL-keyed bounded
map so tenant URLs retain separate entries; define and enforce an explicit
maximum entry count with an eviction policy. In
src/socialite/src/Two/Concerns/InteractsWithJwks.php lines 61-102, apply the
same bounded URL-keyed map approach to jwks and jwksRefreshAttempt, preserving
each entry’s existing expiry and refresh-cooldown behavior.
In `@src/socialite/src/Two/User.php`:
- Around line 40-61: Update User::fake() to instantiate the late-static-bound
class with new static instead of new self, and change its return type from self
to static so subclasses receive their own fake instance while preserving the
existing attribute setup.
In `@tests/Socialite/FacebookProviderTest.php`:
- Around line 103-114: Update
testAnUnknownKidRaisesTheLibraryAuthenticationFailure to capture the expected
UnexpectedValueException and assert that its message contains '"kid" invalid'
using assertStringContainsString, rather than requiring the dependency-owned
full error message. Keep the existing unknown-key token setup and exception type
assertion.
In `@tests/Socialite/Fixtures/OpenIdTestProviderStub.php`:
- Around line 32-35: Remove the unnecessary getUserByToken override from
OpenIdTestProviderStub, since OpenIdProvider already provides it. Preserve
getProviderUserByToken only if needed, ensuring it calls the inherited
parent::getUserByToken($token) implementation.
In `@tests/Socialite/GitlabProviderTest.php`:
- Around line 17-42: Extend
testUserRequestUsesTheVersionedApiAndBearerAuthorization to configure
GitlabProvider with a custom host via setHost, then assert the mocked request
uses that host without a trailing slash while preserving the /api/v4/user path
and bearer header expectations.
In `@tests/Socialite/GoogleProviderIdTokenTest.php`:
- Around line 79-93: Update testItAcceptsConfiguredTrustedAudiences to apply
trusted_audiences through the per-request setConfig path, preserving the test’s
existing audience and user ID assertions; do not use withConfig, which
establishes a worker-lifetime baseline.
In `@tests/Socialite/LinkedInProviderTest.php`:
- Around line 111-124: Reorder the valid image fixtures in the profilePicture
test data so the 800px avatar-original image appears before the 100px avatar
image, while retaining the existing assertions for getAvatar() and
avatar_original. This ensures the test distinguishes widest-image selection from
simply choosing the last valid element.
In `@tests/Socialite/OpenIdProviderTest.php`:
- Around line 809-849: Extract createRsaKeyPair, jwks, and base64UrlEncode into
one shared trait or fixture class under tests/Socialite/Fixtures/, optionally
caching generated keys by kid, and update imports/usages accordingly. Remove the
duplicated helpers from tests/Socialite/OpenIdProviderTest.php lines 809-849,
tests/Socialite/GoogleProviderIdTokenTest.php lines 231-267, and
tests/Socialite/FacebookProviderTest.php lines 183-219; each site requires the
same removal and delegation to the shared helper.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d6818157-7adb-429d-ad4f-cf1c129fa254
📒 Files selected for processing (59)
composer.jsondocs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.mddocs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.mddocs/plans/2026-08-07-1416-socialite-correctness-first-party-extensibility-and-lifecycle.mdsrc/boost/docs/socialite.mdsrc/object-pool/src/ObjectPoolServiceProvider.phpsrc/reverb/src/ReverbServiceProvider.phpsrc/socialite/README.mdsrc/socialite/composer.jsonsrc/socialite/src/AbstractProvider.phpsrc/socialite/src/Concerns/HasProviderContext.phpsrc/socialite/src/Contracts/Factory.phpsrc/socialite/src/HasProviderContext.phpsrc/socialite/src/Socialite.phpsrc/socialite/src/SocialiteManager.phpsrc/socialite/src/SocialiteServiceProvider.phpsrc/socialite/src/Testing/SocialiteFake.phpsrc/socialite/src/Two/AbstractProvider.phpsrc/socialite/src/Two/BitbucketProvider.phpsrc/socialite/src/Two/Concerns/InteractsWithJwks.phpsrc/socialite/src/Two/Exceptions/ConfigurationFetchingException.phpsrc/socialite/src/Two/Exceptions/InvalidUserInfoUrlException.phpsrc/socialite/src/Two/FacebookProvider.phpsrc/socialite/src/Two/GithubProvider.phpsrc/socialite/src/Two/GitlabProvider.phpsrc/socialite/src/Two/GoogleProvider.phpsrc/socialite/src/Two/LinkedInOpenIdProvider.phpsrc/socialite/src/Two/LinkedInProvider.phpsrc/socialite/src/Two/OpenIdProvider.phpsrc/socialite/src/Two/SlackOpenIdProvider.phpsrc/socialite/src/Two/SlackProvider.phpsrc/socialite/src/Two/Token.phpsrc/socialite/src/Two/TwitchProvider.phpsrc/socialite/src/Two/User.phpsrc/socialite/src/Two/XProvider.phpsrc/support/src/Manager.phptests/ObjectPool/ObjectPoolServiceProviderTest.phptests/Reverb/ReverbServiceProviderTest.phptests/Socialite/AbstractProviderTest.phptests/Socialite/BitbucketProviderTest.phptests/Socialite/FacebookProviderTest.phptests/Socialite/Fixtures/GenericTestProviderStub.phptests/Socialite/Fixtures/OAuthTwoTestProviderStub.phptests/Socialite/Fixtures/OpenIdTestProviderStub.phptests/Socialite/Fixtures/VerifyingOpenIdTestProviderStub.phptests/Socialite/GitlabProviderTest.phptests/Socialite/GoogleProviderIdTokenTest.phptests/Socialite/GoogleProviderTest.phptests/Socialite/LinkedInOpenIdProviderTest.phptests/Socialite/LinkedInProviderTest.phptests/Socialite/OAuthTwoTest.phptests/Socialite/OpenIdProviderTest.phptests/Socialite/PackageMetadataTest.phptests/Socialite/SensitiveParameterTest.phptests/Socialite/SlackOpenIdProviderTest.phptests/Socialite/SlackProviderTest.phptests/Socialite/SocialiteFakeTest.phptests/Socialite/SocialiteManagerTest.phptests/Support/ManagerTest.php
💤 Files with no reviewable changes (3)
- src/socialite/src/HasProviderContext.php
- composer.json
- src/socialite/composer.json
…tness-extensibility-lifecycle
Apply a five-minute fallback when an identity provider omits usable cache directives so revoked signing keys cannot remain trusted for the worker lifetime. Keep explicit no-cache, no-store, and max-age directives authoritative, tighten the cached expiry shape, and retain the existing forced-refresh cooldown behavior.\n\nAdd deterministic coverage for both fallback caching and expiry, consolidate the repeated RSA/JWK test fixtures without caching key material, stabilize the Facebook unknown-key assertion, and remove the no-op OpenID test override. Document the observable fallback cadence for custom OpenID Connect providers.
Construct OAuth 2 fake users with late static binding so ecosystem-specific User subclasses receive instances of the called class instead of the base Socialite user. Cover the inherited factory directly.\n\nStrengthen LinkedIn avatar mapping coverage with a later unrelated image so the regression continues to prove exact 100px and 800px selection rather than accidentally accepting the final image.
Replace the superseded indefinite-headerless JWKS design with the reviewed five-minute provider fallback in the Socialite plan and audit ledger. Record the non-null cache shape, deterministic regression boundary, performance effect, and late-static User fake divergence without retaining rejected reasoning or decision history.
|
@greptileai @coderabbitai Thanks for the review. The follow-up is pushed. The JWKS finding was valid. Responses without a usable cache directive now expire after five minutes instead of remaining fresh for the worker lifetime. Explicit The same pass also:
I left the other suggestions unchanged after tracing them against the repository contracts. Hypervel aliases already converge on the auto-singleton concrete, the README section is the required concise Validation is green across the Socialite suite, static analysis, the full repository gate, and package-mode checks. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Implementation outline =='
ast-grep outline src/socialite/src/Two/Concerns/InteractsWithJwks.php --items all
echo '== Cache directive and TTL implementation =='
rg -n -C 8 --glob 'InteractsWithJwks.php' \
'max-age|no-cache|no-store|Cache-Control|cache|ttl|TTL|300|five' \
src/socialite/src/Two/Concerns/InteractsWithJwks.php
echo '== Relevant JWKS cache regression references =='
rg -n -C 5 --glob '*Test.php' \
'max-age|no-cache|no-store|Cache-Control|cache|expiry|expire|JWKS|Jwks' \
tests/SocialiteLength of output: 60898 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== JWKS expiration calculation =='
sed -n '110,155p' src/socialite/src/Two/Concerns/InteractsWithJwks.php
echo '== Test-only default TTL configuration =='
rg -n -C 6 --glob '*.php' \
'setJwksDefaultTtlSeconds\s*\(' \
src/socialite tests/Socialite
echo '== Missing-directives and malformed-directives tests =='
sed -n '585,627p' tests/Socialite/OpenIdProviderTest.phpLength of output: 5564
Static inspection confirms that
I also noted the repository contracts you described for aliases, generated facade annotations, strict token parsing, and recycled-object-ID coverage. ᕱ⑅ᕱ ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
|
The JWKS fix in
The cooldown guard on forced refreshes pairs well with the 5-minute default — a rotation burst won't hammer the endpoint, and stale keys won't persist longer than a predictable window. The rationale for the unchanged items is sound. The alias-to-concrete convergence, generated facade annotations, strict native boundary for missing token fields, and the recycled-object-ID regression are all repository-specific constraints that aren't visible from the diff alone. Thanks for tracing and documenting each one. This looks good to merge. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@docs/plans/2026-08-07-1416-socialite-correctness-first-party-extensibility-and-lifecycle.md`:
- Line 466: Resolve the contradiction between the final decisions in the early
plan section and the approval requirement in the paragraph containing “Before
source implementation.” Record owner approval and remove the pending gate for
the listed API changes, or consistently mark those decisions as pending and
state that implementation is blocked until approval is recorded.
- Line 74: Update the JWKS hot path around driver() so mutable state such as
$jwks and $jwksRefreshAttempt is isolated per coroutine or stored in a
coroutine-safe bounded cache keyed by the exact JWKS URL, rather than on the
shared cached provider instance. Preserve the refresh, cooldown, and
rotation-retry behavior, and add an interleaved coroutine test proving
concurrent requests cannot overwrite each other’s keys or refresh state.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fbe633c1-4b4b-41bd-aa56-fc76842c1368
📒 Files selected for processing (15)
composer.jsondocs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.mddocs/plans/2026-08-07-1416-socialite-correctness-first-party-extensibility-and-lifecycle.mdsrc/boost/docs/socialite.mdsrc/reverb/src/ReverbServiceProvider.phpsrc/socialite/src/Two/Concerns/InteractsWithJwks.phpsrc/socialite/src/Two/User.phptests/Socialite/FacebookProviderTest.phptests/Socialite/Fixtures/CreatesJwksFixtures.phptests/Socialite/Fixtures/OpenIdTestProviderStub.phptests/Socialite/Fixtures/VerifyingOpenIdTestProviderStub.phptests/Socialite/GoogleProviderIdTokenTest.phptests/Socialite/LinkedInProviderTest.phptests/Socialite/OpenIdProviderTest.phptests/Socialite/SocialiteFakeTest.php
💤 Files with no reviewable changes (1)
- tests/Socialite/Fixtures/OpenIdTestProviderStub.php
🚧 Files skipped from review as they are similar to previous changes (9)
- src/reverb/src/ReverbServiceProvider.php
- composer.json
- tests/Socialite/LinkedInProviderTest.php
- tests/Socialite/FacebookProviderTest.php
- src/socialite/src/Two/User.php
- tests/Socialite/Fixtures/VerifyingOpenIdTestProviderStub.php
- tests/Socialite/OpenIdProviderTest.php
- src/boost/docs/socialite.md
- tests/Socialite/GoogleProviderIdTokenTest.php
Replace the stale pre-implementation owner gate with the final intentional API decisions now present in the package. Keep the compatibility rationale and Laravel-facing contract summary while removing procedural text that contradicted the completed implementation.
1 participant
Summary
This PR completes Socialite's OAuth 2 and OpenID Connect correctness work while preserving Hypervel's cached-provider design.
The main changes are:
The PR also fixes three service-identity issues found while tracing Socialite's container behavior: Support managers now refresh their configuration repository when swapping containers, Object Pool uses one manager and recycler owner, and Reverb uses one array channel repository.
Motivation
Hypervel keeps Socialite providers alive for the worker lifetime. That is useful for stable metadata and HTTP/JWKS reuse, but mutable request state cannot live on those shared objects. The old provider context used recyclable object IDs, retained request ownership indirectly, and allowed separately resolved manager identities to disagree about drivers and configuration.
The provider implementations also had several protocol-level gaps. Some credentials were sent in query strings, generic discovery and key caches could be reused across changing tenant URLs, nonce validation did not honor providers that disable nonce protection, and an exception after partial user construction could leave a cached user behind.
These issues are fixed at their owning boundaries without cloning providers, adding locks, retaining per-tenant maps, or introducing a second provider registry.
Provider lifecycle
Provider instances now receive a monotonic process-lifetime context namespace. Request, credentials, scopes, HTTP client, state, and user memoization remain coroutine-local. Cached driver resolution refreshes the current request, while direct cross-coroutine reuse without a request fails clearly instead of reading stale state.
SocialiteManageris the canonical worker-lifetime service and the Factory contract aliases to it. Application bindings and Factory-only fake swaps still take precedence.OAuth responses and extension points
The base OAuth 2 provider now owns protected parsers for access tokens, refresh tokens, expiry, approved scopes, and whole-response user lookup. Login and refresh use the same parsing rules. Missing refresh-token rotation preserves the submitted token, valid zero-padded expiry values remain valid, and malformed advisory expiry values become
null.Returned users expose the complete token response. Providers retain no response property or context slot. User memoization happens only after mapping and every setter succeeds.
These hooks, along with request access and generic
OpenIdProvider, provide the first-party surface needed by external provider packages and tenant-driven applications without the separate SocialiteProviders manager layer.OIDC and JWKS
Generic OIDC, Google, and Facebook now use one shared JWKS concern. It retains one parsed key set and one refresh-attempt timestamp per cached provider, keys entries by exact URL, honors useful Cache-Control directives, and retries decoding once after supported rotation failures. A short cooldown prevents repeated refresh traffic while matching cached keys remain available.
Discovery publishes URL and metadata atomically. Audience validation accepts scalar and list claims, requires the configured client ID, and rejects untrusted extra audiences. Google issuer checks retain only the documented issuer forms. Nonces are consumed once and required only when the provider enables nonce protection.
The shared Firebase JWT path removes Socialite's manual RSA construction and the direct phpseclib dependency.
Compatibility and performance
Supported Laravel OAuth 2 APIs, named arguments, protected extension points, and provider ergonomics remain intact. Hypervel continues to omit OAuth 1 and legacy Twitter support and retains the
xdriver. Additions such as complete token responses, parser hooks, trusted audiences, andUser::fake()are additive.Ordinary non-Socialite requests are unchanged. Provider construction adds one local integer increment. Callback paths add bounded local type and array checks beside existing network and JWT work. JWKS reuse removes repeated network requests. There is no new lock, timer, background job, unbounded tenant cache, provider clone, serialization layer, or ordinary network round trip.
Verification
The changes were verified with focused Socialite, Support, Object Pool, and Reverb coverage, root and split Composer validation, facade and documentation checks, stale-symbol scans, formatting, both PHPStan configurations, the complete parallel components suite, Testbench package mode, dogfood, and diff checks.
For more details, see:
docs/plans/2026-08-07-1416-socialite-correctness-first-party-extensibility-and-lifecycle.md.Summary by CodeRabbit