Correct Auth lifecycle, token, and cache behavior - #473
Merged
Conversation
Record the approved Auth work unit before its implementation changes. The plan defines the validated TokenGuard, Gate, password-broker, Eloquent cache, configuration, metadata, and sensitive-parameter boundaries together with their focused regression coverage. It also carries the audit anti-overengineering rules, Laravel API compatibility requirements, coroutine ownership model, performance constraints, rejected alternatives, verification gates, and final audit-record routing so the implementation can be reviewed against one authoritative design.
Read query, request-body, bearer, and basic-password token sources in their documented order while accepting only non-empty strings and preserving the valid string zero. Invalid values now fall through to later sources instead of shadowing them or leaking through a mixed boundary. Make explicit credential validation honor the configured input key, storage key, and hashing mode. Add focused regressions for ordered lazy access, malformed values, zero tokens, custom keys, hashed storage, and sensitive credential metadata.
Normalize callable reflection through Closure::fromCallable so invokable objects and callable strings receive the same guest-access analysis as closures and array callables. Cache object-callable results by weak identity and restore the nullable lazy-cache sentinel during framework-state cleanup. Avoid constructing and dispatching GateEvaluated when the active dispatcher has no listener, while preserving Event fakes through the repository hasListeners convention. Add regressions for every callable family, before and after callbacks, listener and fake paths, weak-cache release, and flushState cleanup.
Restore a user model timestamp setting in a finally block around remember-token writes so successful and exceptional saves cannot leak temporary state. Keep the original database failure unchanged and preserve models that began with timestamps disabled. Use the configured model as the single cache-key authority, register each active model keyspace when setModel changes it, retain only the store and prefix needed by invalidation descriptors, and keep old keyspaces invalidatable until expiry. Add unit and real-cache regressions for timestamp restoration, model switching, descriptor deduplication, and both old and new keyspaces.
Give verification expiry, password confirmation timeout, authentication timebox duration, and password rehashing one shipped configuration owner. Remove redundant consumer defaults where the setting is required while retaining the intentional nested verification fallback for replaced application configuration. Use typed configuration reads at each boundary, update the public verification guide, and add regressions for environment coercion, signed-link expiry, nested replacement behavior, missing canonical settings, and middleware construction from the shipped values.
Construct and dispatch PasswordResetLinkSent only when the active dispatcher observes that event, preserving Event fakes while avoiding unused observational work. Add a narrow dispatcher replacement method for the worker-lifetime broker so container event rebinding can update an already-resolved instance. Cover no-listener, real-listener, and replacement behavior directly while leaving reset-link callback ordering and the public password-broker contract unchanged.
Refresh already-resolved concrete password brokers when the event dispatcher binding changes without resolving an unused manager or replacing custom broker contracts. This keeps worker-lifetime brokers aligned with Event::fake, fakeFor restoration, and ordinary container rebinding while preserving the protected custom resolution extension point. Accept unit and backed enums across broker names, guard-to-broker lookup, default broker selection, and guard-owned cache clearing, including integer-backed zero without falsey fallback. Update contracts and generated facade metadata, consume canonical timebox settings, and cover broker cache identity, dispatcher restoration, unused-manager behavior, custom brokers, session guards, and real forgot-password wiring.
Mark SessionGuard credential arrays and hash keys as sensitive at every public and internal propagation boundary, and protect password-reset repository hash keys the same way. TokenGuard validation carries the corresponding attribute from its earlier correctness change. Add reflection-based coverage for the complete eleven-parameter surface so future signature edits cannot silently expose credentials or application key material in stack traces.
Exercise static and invokable custom guard creators through AuthManager using Hypervel contracts and return types. These focused parity cases protect the existing Laravel-style extension API without changing production behavior or introducing additional runtime machinery.
Add an end-to-end Auth integration regression proving logoutOtherDevices rehashes the persisted password through the configured user provider and hashing service. The test exercises real request, session, guard, database, and hash wiring rather than replacing the behavior with mocks.
Declare the split Auth package direct runtime dependencies and both discoverable providers instead of relying on the monorepo root to mask missing package edges. Keep constraints aligned with the root replacement and dependency policy. Add executable metadata coverage for dependencies, provider discovery, and the generated enum-aware Auth and Password facade signatures so package installs, IDE metadata, and static-analysis surfaces cannot drift independently of the concrete APIs.
Mark Auth complete in the core package checklist and route every carried or cross-package dependency through the audit index, including the Fortify consumer revalidated by canonical Auth configuration. Record final findings, ownership, rejected concerns, coroutine and worker-lifecycle boundaries, Laravel-facing compatibility, performance characteristics, regression coverage, authoritative validation, independent review, and the absence of deferred Auth work or TODOs.
# Conflicts: # docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md # docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md
|
Warning Review limit reached
Next review available in: 34 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 (2)
📝 WalkthroughWalkthroughThe Auth package now covers stricter token validation, generalized Gate callbacks, dispatcher rebinding, enum identifiers, model-cache consistency, configuration ownership, sensitive parameters, package dependencies, and expanded unit and integration tests. ChangesAuth correctness and lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant PasswordResetServiceProvider
participant PasswordBrokerManager
participant PasswordBroker
participant EventDispatcher
Application->>PasswordResetServiceProvider: Rebind events
PasswordResetServiceProvider->>PasswordBrokerManager: Refresh resolved brokers
PasswordBrokerManager->>PasswordBroker: Replace dispatcher
PasswordBroker->>EventDispatcher: Check listeners before reset-link event
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 SummaryThis PR corrects authentication token handling, authorization callable reflection, password-broker event lifecycle, and Eloquent user-cache ownership while expanding Laravel-compatible enum and configuration support.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/auth/src/TokenGuard.php | Token extraction now accepts only non-empty strings in documented precedence order, while explicit validation honors configured keys and hashing. |
| src/auth/src/Access/Gate.php | Gate normalizes callable reflection, weakly caches object-callable metadata, resets static state, and avoids unobserved evaluation events. |
| src/auth/src/EloquentUserProvider.php | Remember-token writes restore timestamp settings and model-specific cache descriptors preserve coherent lookup and invalidation keyspaces. |
| src/auth/src/Passwords/PasswordBroker.php | Reset-link events are constructed only when observed, and resolved brokers can receive dispatcher replacements. |
| src/auth/src/Passwords/PasswordBrokerManager.php | Broker identifiers now support enums and resolved concrete brokers follow dispatcher rebinding. |
| src/auth/src/Passwords/PasswordResetServiceProvider.php | Event rebinding refreshes already-resolved password managers without resolving otherwise-unused services. |
| src/auth/src/AuthManager.php | Guard cache clearing accepts enum identifiers while preserving coroutine-local guard selection. |
| src/foundation/config/auth.php | Foundation now owns canonical verification-expiry and authentication-timebox defaults. |
| src/contracts/src/Auth/PasswordBrokerFactory.php | The password-broker contract exposes additive enum-aware identifier signatures. |
| src/auth/composer.json | The split Auth package declares its direct runtime dependencies and provider discovery metadata. |
Reviews (2): Last reviewed commit: "Clarify Auth validation and dispatcher l..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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-05-1615-auth-correctness-lifecycle-and-current-parity.md`:
- Line 198: Update the hashing-cost statement in section 8 to acknowledge that
TokenGuard::user() also hashes request tokens when configured with $hash = true,
and state that explicit validate() uses the same configured hashing path as
normal request authentication.
- Around line 576-577: Update the final validation steps in the plan to use only
`composer fix` from the worktree root as the authoritative aggregate command.
Remove the separate PHPStan and PHP-CS-Fixer commands while preserving the
instruction not to weaken, skip, or rewrite tests.
In `@src/auth/src/Passwords/PasswordBroker.php`:
- Around line 200-209: Update the PHPDoc for PasswordBroker::setDispatcher() and
PasswordBrokerManager::refreshEventDispatcher() to state that request-time
mutation can replace the dispatcher seen by concurrent requests, causing events
to use an incorrect or fake dispatcher. Preserve the existing boot/tests-only
lifecycle guidance in both methods.
🪄 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: e493ac7e-78e4-4b19-87ac-77e3a34e2962
📒 Files selected for processing (42)
docs/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-05-1615-auth-correctness-lifecycle-and-current-parity.mdsrc/auth/composer.jsonsrc/auth/src/Access/Gate.phpsrc/auth/src/AuthManager.phpsrc/auth/src/EloquentUserProvider.phpsrc/auth/src/Notifications/VerifyEmail.phpsrc/auth/src/PasswordConfirmation.phpsrc/auth/src/Passwords/CacheTokenRepository.phpsrc/auth/src/Passwords/DatabaseTokenRepository.phpsrc/auth/src/Passwords/PasswordBroker.phpsrc/auth/src/Passwords/PasswordBrokerManager.phpsrc/auth/src/Passwords/PasswordResetServiceProvider.phpsrc/auth/src/SessionGuard.phpsrc/auth/src/TokenGuard.phpsrc/boost/docs/verification.mdsrc/contracts/src/Auth/PasswordBrokerFactory.phpsrc/fortify/src/Actions/RedirectIfTwoFactorAuthenticatable.phpsrc/foundation/config/auth.phpsrc/support/src/Facades/Auth.phpsrc/support/src/Facades/Password.phptests/Auth/AuthAccessGateTest.phptests/Auth/AuthConfigTest.phptests/Auth/AuthEloquentUserProviderCacheTest.phptests/Auth/AuthEloquentUserProviderTest.phptests/Auth/AuthManagerTest.phptests/Auth/AuthPasswordBrokerManagerTest.phptests/Auth/AuthPasswordBrokerTest.phptests/Auth/AuthPasswordResetServiceProviderTest.phptests/Auth/AuthTokenGuardTest.phptests/Auth/AuthenticateMiddlewareTest.phptests/Auth/PackageMetadataTest.phptests/Auth/PasswordConfirmationTest.phptests/Auth/RequirePasswordMiddlewareTest.phptests/Auth/SensitiveParameterTest.phptests/Auth/VerifyEmailNotificationTest.phptests/Integration/Auth/AuthenticationTest.phptests/Integration/Auth/EloquentUserProviderCacheTest.phptests/Integration/Auth/Fixtures/AuthTestUser.phptests/Integration/Auth/ForgotPasswordTest.phptests/Integration/Auth/RehashOnLogoutOtherDevicesTest.php
Correct the implementation plan to distinguish TokenGuard hashing already performed by normal request authentication from the hashing newly applied by explicit validation. Replace duplicated partial tool commands with composer fix as the single authoritative aggregate gate. Document that refreshing every resolved password broker mutates worker-lifetime dispatcher state and races across coroutines when used per request. Keep the single-broker setter wording unchanged because it already states its worker-wide effect and matches established Auth mutators.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR corrects Auth token validation, callable handling, password-broker lifecycle behavior, and Eloquent user-cache ownership. It also brings the audited Auth API and integration surface up to date with current Laravel where those behaviors fit Hypervel's coroutine model.
The result keeps request state coroutine-local and worker-lifetime services reusable. It does not add locks, retries, network calls, serialization layers, or compatibility wrappers.
For the complete design and decision record, see
docs/plans/2026-08-05-1615-auth-correctness-lifecycle-and-current-parity.md.Token and authorization behavior
Password broker lifecycle
Event::fake()andEvent::fakeFor()restoration correct without resolving unused managers or replacing custom broker contracts.Eloquent provider correctness
Configuration, metadata, and diagnostics
Compatibility and performance
Supported Laravel-facing Auth signatures, guard and broker APIs, protected resolution hooks, event behavior, and facade entry points remain intact. Enum support is additive. Hypervel-specific corrections reject malformed native token inputs or fix behavior that already contradicted configured cache and token contracts.
Hot paths gain only bounded local type checks or existing listener lookups. The event changes avoid unused allocations and dispatch. No request path gains additional I/O, locking, polling, container-resolution loops, or unbounded retained state.
Verification
composer fix.Summary by CodeRabbit
New Features
Bug Fixes
Documentation