Skip to content

Add native session persistence and user session management - #17

Open
binaryfire wants to merge 14 commits into
0.4from
feature/session-management
Open

Add native session persistence and user session management#17
binaryfire wants to merge 14 commits into
0.4from
feature/session-management

Conversation

@binaryfire

@binaryfire binaryfire commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR replaces cache-backed Redis session persistence with a native Redis handler and adds a driver-neutral API for listing and invalidating a user's active sessions.

The database and Redis drivers now implement the same management contract. Other drivers continue to work normally and report that user-session management is unsupported.

Motivation

Routing Redis sessions through the cache package constrained the storage layout and made session-specific optimization difficult. It also coupled Redis session configuration to a cache store that had no role in file, cookie, array, null, or database persistence.

Session persistence now owns its storage directly. This gives the Redis driver a one-command hot path when user tracking is disabled and allows tracked mutations to use session-specific Lua scripts and Redis hash-field expiration.

Public API

The session manager exposes a capability probe and a user-scoped repository:

if (Session::supportsUserSessionManagement()) {
    $sessions = Session::forUser($user)->all();
}

Session::forUser($user)->invalidate($sessionId);
Session::forUser($user)->invalidateOthers($currentSessionId);
Session::forUser($user)->invalidateAll();

forUser uses the currently selected authentication guard by default. A guard name or enum may be supplied explicitly without changing the selected guard.

Each returned UserSession contains the session ID, IP address, user agent, last activity time, and derived expiration time.

Ownership model

Managed sessions are scoped by both authentication provider and normalized user ID. This has a few useful properties:

  • Guards backed by different providers cannot list or invalidate each other's sessions, even when their user IDs overlap.
  • Guards backed by the same provider intentionally share the same account namespace.
  • Integer, UUID, ULID, and application-defined string identifiers use one storage contract.
  • Providerless custom guards can still persist ordinary sessions, but their sessions are explicitly unmanaged.

Session writes distinguish resolved ownership, unknown ownership, and known-unowned state. Unknown writes preserve a previously proven owner, which matters when a public route selects a different unauthenticated guard. Invalidated replacement sessions are marked unowned in coroutine-local state so a guard's cached user cannot associate the empty replacement during response-end persistence.

Deleting a managed current session requires the storage driver to prove ownership before the in-memory session is flushed and rotated. This prevents an administrator managing another user from changing their own session and prevents response-end persistence from recreating an invalidated session ID.

Database driver

The database handler now stores auth_provider and user_id as one ownership value and exposes active listing and scoped deletion through direct set-based queries.

  • Listing, individual invalidation, and handler-level bulk invalidation use one query.
  • Listing reads metadata columns only and does not read session payloads.
  • Bulk repository invalidation performs one additional scoped delete only when it must prove whether the current stored session belongs to the target user.
  • Expiration uses one consistent exclusive active boundary across reads, listing, invalidation, and garbage collection.
  • Duplicate-insert races update the current payload without carrying the session ID into the update or preserving stale ownership.

The generated sessions table now uses a nullable indexed string user_id, a nullable auth_provider, and the semantic ipAddress column type. Existing applications need an explicit migration before using managed database sessions with these identifiers.

Redis driver

With tracking disabled, Redis persistence performs exactly one native command per operation:

  • GET for reads
  • SETEX for writes
  • DEL for deletions

This path does not resolve authentication state, encode metadata, execute Lua, or require Redis hash-field expiration.

Tracked sessions keep the existing session-ID payload key and add a versioned ownership envelope plus one per-user hash index. Index fields carry compact metadata and expire with the payload through HSETEX.

On standalone Redis and Sentinel deployments, tracked mutations use SHA-cached Lua scripts. A warmed mutation is one client round trip and is atomic at the Redis server. Healthy listing is one HGETALL and does not perform per-session payload reads.

Redis Cluster uses an explicit cross-slot branch because session payloads and user indexes cannot share a reliable slot without making ordinary session reads owner-dependent. Payloads remain authoritative. Scoped deletion rechecks the envelope owner, and any stale index entry is harmless and bounded by its field TTL.

The handler owns raw Redis bytes while a pooled connection is pinned, temporarily disabling phpredis serialization and compression and restoring both options afterward. Logical session prefixes and phpredis connection prefixes remain distinct so keys are neither omitted nor double-prefixed.

User-session listing and bulk invalidation are O(n) in that user's active session count. Returning or deleting n sessions requires that work; the implementation keeps it to one server-side operation on standalone Redis and one set-based query for the database driver instead of adding per-session network requests.

Configuration and package boundaries

  • SESSION_STORE and the cache-backed session handler are removed.
  • SESSION_CONNECTION selects the native database or Redis connection.
  • SESSION_PREFIX continues to namespace Redis session keys.
  • SESSION_TRACK_USER_SESSIONS enables Redis user-session tracking and defaults to false.
  • Tracking requires phpredis 6.3 or later and Redis 8 or Valkey 9 or later.

The session package still requires the cache package because session blocking and $session->cache() are first-class APIs. Session-scoped cache values remain inside the normal session payload, while blocking continues to use cache locks. Persistence itself no longer delegates through cache.

Correctness and security

Coverage includes:

  • selected and explicit guard behavior, including coroutine isolation;
  • provider isolation when scalar user IDs overlap;
  • current-session invalidation, logout, remember-me, retry, and response-end save behavior;
  • ownership reassignment and providerless authenticated guards;
  • malformed Redis envelopes, metadata, indexes, and script results;
  • Redis partial failures and Cluster cleanup ordering;
  • stale and wrong-owner indexes being unable to delete authoritative payloads;
  • serializer, compression, prefix, payload TTL, and field TTL behavior;
  • database race handling, expiration boundaries, query counts, and portable identifier storage.

The database handler runs through the MariaDB, MySQL, PostgreSQL, and SQLite integration matrix. Redis behavior is exercised against Redis and Valkey, including real stored values and expiration state. The full formatter, static analysis, unit, integration, and testbench checks pass.

Additional cleanup

The Redis hash-field-expiration test concern is renamed around the capability it checks so cache, auth, and session integrations share one accurate test boundary.

The PR also fixes an existing test fixture declaration order that prevented AttributeInheritanceTest from being run directly by PHPUnit.

Summary by CodeRabbit

  • New Features

    • Added native Redis session persistence, including standalone and cluster support.
    • Added user-session management to list, invalidate, and bulk-invalidate active sessions.
    • Added provider- and guard-aware session ownership tracking.
    • Added configurable Redis session tracking via SESSION_TRACK_USER_SESSIONS.
  • Documentation

    • Expanded session, authentication, upgrade, and package documentation with storage and user-session guidance.
  • Bug Fixes

    • Improved session expiration, ownership isolation, invalidation, and stale-session cleanup.

Rename the any-tag-specific Redis capability concern around the shared hash-field-expiration requirement so cache, auth, and session integrations can use one accurate test boundary.

Update every existing consumer and its unit coverage to the new terminology. Remove the unused dual-tag-mode helper while touching the shared cache integration base.
Add a guard-aware provider-name accessor that accepts string and enum guard identifiers while defaulting to the coroutine-selected guard without mutating that selection.

Preserve the Laravel getDefaultUserProvider API by delegating it to the new source of truth. Document the Hypervel guard semantics, regenerate facade metadata, and cover explicit selection, empty providers, and concurrent coroutine isolation.
Introduce a shared session identifier utility and a coroutine-local identity value that distinguishes resolved ownership, unknown ownership, and explicitly unowned replacement sessions.

Mark the new identifier produced by Store::invalidate as unowned so a selected guard's cached user cannot associate the empty replacement during response-end persistence. Keep ordinary regeneration eligible for later authenticated ownership and give CSRF tokens their own length constant.

Cover identifier validation, provider-qualified identity resolution, suppression persistence, login-time rotation, and coroutine isolation.
Add the driver capability contract for listing and destroying provider-qualified user sessions together with the immutable storage-neutral UserSession value object.

Centralize direct-handler provider and session-list validation in one session concern, while keeping session identifier errors owned by SessionId. Add the immutable Carbon addMinutes annotation used to derive expiry values without storing duplicate timestamps.
Add a Laravel-style UserSessions repository that lists active sessions and invalidates one, other, or all records through the storage capability contract.

Snapshot the current Store identifier per operation, require storage proof before rotating it, and preserve both the caller exception and the original current identifier during bulk deletion. Rotate and suppress a proven deleted current session before later storage work so response-end persistence cannot resurrect it.

Cover reuse after rotation, logged-out invalidation, other-user isolation, unstarted stores, invalid identifiers, retry failures, and exact handler arguments and counts.
Teach the database handler to persist provider-qualified ownership and expose active listing, scoped deletion, and bulk deletion through set-based queries without payload reads or N+1 work.

Track existence and expiration independently in coroutine state, align the active cutoff across reads and management operations, and handle concurrent duplicate inserts without carrying the session identifier into fallback updates or preserving stale ownership.

Update generated and testbench schemas for string user identifiers, nullable auth providers, and semantic IP addresses. Replace the single SQLite test with a shared MariaDB, MySQL, PostgreSQL, and SQLite matrix covering query counts, ownership transitions, expiration boundaries, races, and portable identifiers.
Replace cache-oriented Redis assumptions with a dedicated raw Redis session handler whose tracking-disabled hot path is one GET, SETEX, or DEL and never performs identity, metadata, or hash-field-expiration work.

Add opt-in provider-qualified user tracking with a versioned single-key payload envelope, per-field-expiring user indexes, strict metadata parsing, SHA-cached Lua mutations for standalone atomicity, and an explicit cross-slot Redis Cluster branch with ownership-safe failure ordering.

Cover wire formats, serializer and compression restoration, prefix composition, identity transitions, operation counts, malformed data, authoritative ownership checks, partial failures, cluster cleanup, bulk error chaining, and every return-shape guard through transport mocks.
Add Redis 8 and Valkey 9 integration coverage for plain and tracked session persistence, provider isolation, identity-less keepalives, invalidation, ownership reassignment, corruption handling, and tracking flag transitions.

Assert the physical envelope and index state, synchronized payload TTL and field HTTL, phpredis serializer and compression restoration, connection and session prefix composition, stale-index safety, and deliberate Lua partial-failure ordering against a real server.

Include the session Redis integration directory in both Redis and Valkey workflow jobs while leaving tracking-disabled coverage runnable on older servers.
Wire Redis sessions directly to RedisSessionHandler and remove the cache-backed persistence handler, its store selector, and the tests and testing fixture that depended on that obsolete abstraction.

Expose capability probing and guard-aware Session::forUser repositories through SessionManager. Qualify users by the selected or explicit guard provider, preserve selection, validate Eloquent model/provider matches, normalize scalar identifiers, and surface disabled or unsupported drivers consistently.

Add the Redis package dependency, tracking configuration and environment default while retaining cache as a required dependency for session blocking and session-scoped cache. Regenerate facade metadata and cover native construction, capabilities, encrypted stores, provider isolation, enum guards, and invalid inputs.
Add request-level SQLite coverage that composes auth guards, the session manager, database persistence, middleware saving, and exception retries instead of testing those layers only in isolation.

Prove that current invalidation cannot resurrect its old identifier or associate its empty replacement, later login escapes suppression, provider-qualified guards with overlapping identifiers remain siloed, administrator revocation leaves the administrator untouched, and authentication side effects occur only through explicit Auth operations.
Document direct Redis persistence, the distinction from cache-backed blocking and session-scoped cache, and the opt-in Redis user-tracking platform requirements.

Describe capability probing, guard-aware user repositories, DTO fields, listing and invalidation flows, provider-qualified multi-guard behavior, logout and remember-me boundaries, Redis Cluster drift, and the deliberate per-user O(n) cardinality contract.

Add the database schema and existing-application migration guidance, remove SESSION_STORE guidance, record the lasting Laravel driver and schema differences, and link the split package to the full session documentation.
Declare the inline abstract parent fixture before the child test class so PHPUnit can require the file directly without resolving an as-yet undeclared parent.

Keep a concise comment on the otherwise unusual helper order because the full suite did not expose the original load failure and moving the fixture back below the test would silently restore it.
Capture the final standalone persistence architecture, provider-qualified identity model, resolved and unresolved ownership transitions, current-session lifecycle guarantees, and capability-gated public API.

Specify database query and schema behavior, Redis envelope and index formats, standalone Lua and Cluster failure ordering, round-trip and cardinality constraints, configuration, documentation, migration, and multi-driver testing requirements.

Keep the reviewed plan as the authoritative compact reference for the completed implementation, including the shared validation ownership and final failure-path coverage.
…ment

# Conflicts:
#	src/support/src/Facades/Auth.php
#	src/support/src/Facades/Session.php
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The session package replaces cache-backed Redis persistence with direct database and Redis handlers. It adds provider-qualified user-session listing and invalidation, shared session identity types, lifecycle safeguards, configuration changes, documentation, and comprehensive tests.

Changes

Session contracts and identity model

Layer / File(s) Summary
Session contracts and identity model
src/session/src/SessionId.php, src/session/src/UserSession.php, src/session/src/UserSessionIdentity.php, src/session/src/Contracts/CanManageUserSessions.php, src/session/src/UserSessions.php, src/auth/src/CreatesUserProviders.php, src/support/src/Facades/Auth.php, src/support/src/Facades/Session.php
Adds shared session ID validation, immutable session values, coroutine-scoped identity suppression, provider lookup by guard, and capability-gated user-session APIs.

Database session ownership and management

Layer / File(s) Summary
Database session ownership and management
src/session/src/DatabaseSessionHandler.php, src/session/src/Console/stubs/database.stub, src/testbench/hypervel/migrations/*, tests/Integration/Session/Database/*
The database handler stores provider-qualified ownership, tracks expiration separately, lists active sessions, and supports targeted and bulk deletion. Database-specific integration tests cover supported database engines and lifecycle behavior.

Redis persistence and indexed ownership

Layer / File(s) Summary
Redis persistence and indexed ownership
src/session/src/RedisSessionHandler.php, src/session/composer.json, tests/Session/RedisSessionHandlerTest.php, tests/Integration/Session/Redis/RedisSessionHandlerTest.php, .github/workflows/redis.yml
Adds direct Redis session persistence with versioned envelopes, user indexes, Lua-backed atomic operations, Redis Cluster handling, metadata validation, stale-index cleanup, and standalone and integration test coverage.

Session lifecycle and configuration

Layer / File(s) Summary
Session lifecycle and configuration
src/session/src/SessionManager.php, src/session/src/Store.php, src/foundation/config/session.php, src/testbench/hypervel/.env.example, tests/Session/SessionManagerTest.php, tests/Session/SessionStoreTest.php, tests/Session/SessionConfigTest.php
Session manager construction now uses RedisSessionHandler. Session IDs use the shared format. Invalidation suppresses replacement ownership and rotates the current session. Configuration adds SESSION_TRACK_USER_SESSIONS and removes SESSION_STORE.

Documentation and validation coverage

Layer / File(s) Summary
Documentation and validation coverage
src/docs/session.md, src/docs/upgrade.md, src/session/README.md, docs/plans/*, src/foundation/src/Testing/Concerns/*, tests/Auth/*, tests/Foundation/Testing/Concerns/*, tests/Integration/Auth/*, tests/Integration/Cache/*, tests/Integration/Generators/*, tests/Session/UserSessionIdentityTest.php, tests/Session/UserSessionsTest.php, tests/Session/PackageMetadataTest.php
Documents direct session storage, schema changes, provider-aware management, Redis requirements, and upgrades. Tests cover provider resolution, identity suppression, user-session lifecycle behavior, configuration, package metadata, and renamed Redis capability checks.

Estimated code review effort: 5 (Critical) | ~120 minutes

Mergeability Score: 🟠 High · up to af322

The PR adds user-scoped session listing and invalidation, but the current Redis ownership token can permit cross-account session access or deletion if attacker-controlled identifiers collide; merge should be blocked until the ownership hashing and migration behavior are corrected.

Sequence Diagram(s)

sequenceDiagram
  participant Application
  participant SessionManager
  participant UserSessions
  participant SessionHandler
  participant DatabaseOrRedis
  Application->>SessionManager: forUser(user, guard)
  SessionManager->>SessionHandler: check management capability
  SessionManager->>UserSessions: create provider-qualified repository
  UserSessions->>SessionHandler: list or invalidate sessions
  SessionHandler->>DatabaseOrRedis: query or delete owned sessions
  SessionHandler-->>UserSessions: return sessions or count
  UserSessions-->>Application: return management result
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.93% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main changes: native session persistence and user-session management.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/session-management

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.

@binaryfire

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown

Greptile Summary

The PR replaces cache-backed Redis sessions with native persistence and introduces provider-scoped user-session listing and invalidation for database and Redis drivers.

  • Adds driver-neutral user-session repositories, ownership identities, metadata value objects, and capability probing.
  • Extends database persistence with provider-qualified ownership, active-session queries, scoped deletion, and portable string identifiers.
  • Adds native Redis persistence with optional tracking, Lua-backed standalone mutations, and explicit Redis Cluster behavior.
  • Updates session configuration, migrations, documentation, CI, and integration coverage.

Confidence Score: 3/5

The PR should not merge until Redis Cluster tracked writes can no longer leave live sessions outside user-scoped discovery and revocation after an index-update failure.

Cluster persistence commits the authoritative session payload before a separately fallible index mutation, while listing and bulk invalidation rely entirely on that index, creating a temporary revocation bypass.

Files Needing Attention: src/session/src/RedisSessionHandler.php

Security Review

A Redis Cluster partial write can commit a live owner-tagged session without adding it to the owner index. Because listing and bulk revocation are index-driven, that session can remain usable while evading user-scoped discovery and invalidation until another successful write repairs the index or the payload expires.

Important Files Changed

Filename Overview
src/session/src/RedisSessionHandler.php Implements native Redis persistence and user tracking, but Cluster writes can leave a committed live payload undiscoverable and unrevocable after the separate owner-index update fails.
src/session/src/DatabaseSessionHandler.php Adds provider-qualified ownership, consistent active-session boundaries, scoped set-based management queries, and duplicate-insert handling without an accepted defect.
src/session/src/UserSessions.php Adds the user-scoped management repository and ownership-proven current-session rotation.
src/session/src/UserSessionIdentity.php Resolves selected-guard ownership and tracks explicitly unowned replacement session IDs in coroutine-local state.
src/session/src/SessionManager.php Adds capability probing and provider-scoped repository construction while switching Redis sessions to the native handler.
src/session/src/Store.php Marks invalidated replacement session IDs as unowned to prevent response-end persistence from reassociating them.
src/session/src/Console/stubs/database.stub Updates generated session tables for provider-qualified string user identifiers and semantic IP-address storage.

Fix All in Greploop

Fix All in Claude Code Fix All in Codex

Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/0.4..." | Re-trigger Greptile

Comment on lines +431 to +434

if ($result !== 1) {
return false;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Cluster write leaves unindexed sessions

If the separate owner-index update fails after the Cluster payload mutation succeeds, writeCluster() leaves the live owner-tagged payload committed while returning failure. Because listing and bulk invalidation enumerate only the user index, the session then evades all(), invalidateAll(), and invalidateOthers() until another successful write repairs the index or the payload expires.

How this was verified: The Cluster path commits the payload before the fallible index update, while both listing and bulk invalidation derive their candidates exclusively from that index.

Fix in Claude Code Fix in Codex

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
.github/workflows/redis.yml (1)

74-74: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Optional: add an explicit least-privilege permissions block.

Static analysis reports that these jobs inherit the default GITHUB_TOKEN permissions. These jobs only check out code and run tests, so contents: read is sufficient. Add a top-level permissions block if you want to close this gap in the same change.

🤖 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 @.github/workflows/redis.yml at line 74, Add a top-level GitHub Actions
permissions block in the Redis workflow that grants only contents: read,
covering the checkout and test jobs without inheriting broader GITHUB_TOKEN
permissions.

Source: Linters/SAST tools

src/session/src/RedisSessionHandler.php (1)

181-211: 🚀 Performance & Scalability | 🔵 Trivial

Consider bounding the bulk invalidation loop.

DESTROY_USER_SESSIONS_SCRIPT reads every field of the user index and then issues GET, DEL, and HDEL per session inside one Lua script. Redis executes the whole script on one thread without preemption. For a user with a very large index this blocks all other clients for the duration.

Operationally, consider one of the following:

  • Chunk the work with HSCAN plus repeated bounded script calls.
  • Cap the index size at write time, or add a metric/alert on user index cardinality.

The same shape exists in destroyUserSessionsInCluster, which already iterates in PHP and is therefore interruptible.

🤖 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/session/src/RedisSessionHandler.php` around lines 181 - 211, Bound bulk
invalidation in DESTROY_USER_SESSIONS_SCRIPT so a single Redis Lua invocation
processes only a limited number of sessions, preferably by replacing HKEYS with
bounded HSCAN-based batches and coordinating repeated calls from the surrounding
destroy-user-sessions flow. Preserve exception handling, owner validation,
payload deletion, and index cleanup across batches; apply equivalent bounded
iteration to destroyUserSessionsInCluster if needed for consistent behavior.
src/session/src/UserSessionIdentity.php (1)

29-36: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Suppression entries persist for the process lifetime outside coroutines.

Inside a coroutine, the map dies with the coroutine context. Outside a coroutine, CoroutineContext::set() writes to a process-static array, so every suppressed session ID stays until the process exits. Long-running console commands or workers that invalidate many sessions accumulate entries.

Consider adding a way to drop an entry once the replacement session is written, or bounding the map size.

🤖 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/session/src/UserSessionIdentity.php` around lines 29 - 36, Add cleanup
for entries managed by UserSessionIdentity::suppress by introducing a way to
remove a session ID from the UNOWNED_CONTEXT_KEY map after its replacement
session is written. Ensure cleanup updates the coroutine/process context without
affecting other suppression entries, and use the cleanup at the
replacement-session write path.
src/session/src/Console/stubs/database.stub (1)

18-20: 🚀 Performance & Scalability | 🔵 Trivial

Consider a composite index for the user-session queries.

The handler always filters on auth_provider, user_id, and last_activity together (userSessions(), destroyUserSession(), destroyUserSessions() in src/session/src/DatabaseSessionHandler.php). The stub indexes only user_id. A single-column index on user_id is usually selective enough, so this is not a defect. If you expect many sessions per user, an index on (user_id, auth_provider, last_activity) lets the database satisfy the listing and bulk-delete predicates plus the ordering from one index.

Applications that publish this stub cannot easily change the index later without a follow-up migration, so choosing the shape now is cheaper.

🤖 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/session/src/Console/stubs/database.stub` around lines 18 - 20, Update the
session table definition to add a composite index covering user_id,
auth_provider, and last_activity for the queries in userSessions(),
destroyUserSession(), and destroyUserSessions(). Replace or supplement the
existing user_id-only index so listing, deletion, and ordering can use the
composite index.
🤖 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/docs/session.md`:
- Around line 85-87: Update the Redis command-count statement near the session
payload description to qualify the one-command behavior as applying when
SESSION_TRACK_USER_SESSIONS is disabled. Document the additional user-index
operations for tracked sessions, distinguishing standalone Redis from Redis
Cluster writes as needed.

In `@src/session/src/DatabaseSessionHandler.php`:
- Around line 157-158: Document the getDefaultPayload() signature change in the
repository’s upgrade notes, noting that external subclasses overriding this
protected method must update their signature to match the new string $sessionId
and string $data parameters and array return type.

In `@src/session/src/RedisSessionHandler.php`:
- Around line 758-761: Update RedisSessionHandler::ownerDigest to derive the
32-character owner token from a cryptographic SHA-256 digest while preserving
the existing length and lowercase hexadecimal format required by
OWNER_DIGEST_LENGTH, ENVELOPE_HEADER_LENGTH, and the Lua validation. Handle the
migration impact by documenting that existing index keys and envelopes no longer
match, or by bumping ENVELOPE_VERSION so stale envelopes are rejected.

---

Nitpick comments:
In @.github/workflows/redis.yml:
- Line 74: Add a top-level GitHub Actions permissions block in the Redis
workflow that grants only contents: read, covering the checkout and test jobs
without inheriting broader GITHUB_TOKEN permissions.

In `@src/session/src/Console/stubs/database.stub`:
- Around line 18-20: Update the session table definition to add a composite
index covering user_id, auth_provider, and last_activity for the queries in
userSessions(), destroyUserSession(), and destroyUserSessions(). Replace or
supplement the existing user_id-only index so listing, deletion, and ordering
can use the composite index.

In `@src/session/src/RedisSessionHandler.php`:
- Around line 181-211: Bound bulk invalidation in DESTROY_USER_SESSIONS_SCRIPT
so a single Redis Lua invocation processes only a limited number of sessions,
preferably by replacing HKEYS with bounded HSCAN-based batches and coordinating
repeated calls from the surrounding destroy-user-sessions flow. Preserve
exception handling, owner validation, payload deletion, and index cleanup across
batches; apply equivalent bounded iteration to destroyUserSessionsInCluster if
needed for consistent behavior.

In `@src/session/src/UserSessionIdentity.php`:
- Around line 29-36: Add cleanup for entries managed by
UserSessionIdentity::suppress by introducing a way to remove a session ID from
the UNOWNED_CONTEXT_KEY map after its replacement session is written. Ensure
cleanup updates the coroutine/process context without affecting other
suppression entries, and use the cleanup at the replacement-session write path.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d6dc811-d1b5-4824-8dcc-9c5484b32e16

📥 Commits

Reviewing files that changed from the base of the PR and between 08d5e27 and af3221a.

📒 Files selected for processing (52)
  • .github/workflows/redis.yml
  • docs/plans/2026-08-12-1400-native-session-persistence-and-user-session-management.md
  • src/auth/README.md
  • src/auth/src/CreatesUserProviders.php
  • src/docs/session.md
  • src/docs/upgrade.md
  • src/foundation/config/session.php
  • src/foundation/src/Testing/Concerns/RequiresHashFieldExpiration.php
  • src/session/README.md
  • src/session/composer.json
  • src/session/src/CacheBasedSessionHandler.php
  • src/session/src/Concerns/ValidatesUserSessionArguments.php
  • src/session/src/Console/stubs/database.stub
  • src/session/src/Contracts/CanManageUserSessions.php
  • src/session/src/DatabaseSessionHandler.php
  • src/session/src/RedisSessionHandler.php
  • src/session/src/SessionId.php
  • src/session/src/SessionManager.php
  • src/session/src/Store.php
  • src/session/src/UserSession.php
  • src/session/src/UserSessionIdentity.php
  • src/session/src/UserSessions.php
  • src/support/src/CarbonImmutable.php
  • src/support/src/Facades/Auth.php
  • src/support/src/Facades/Session.php
  • src/testbench/hypervel/.env.example
  • src/testbench/hypervel/migrations/0001_01_01_000002_testbench_create_sessions_table.php
  • tests/Auth/AuthManagerTest.php
  • tests/Foundation/Testing/Concerns/AttributeInheritanceTest.php
  • tests/Foundation/Testing/Concerns/InteractsWithSessionTest.php
  • tests/Foundation/Testing/Concerns/RequiresHashFieldExpirationTest.php
  • tests/Integration/Auth/Redis/EloquentUserProviderCacheTagsTest.php
  • tests/Integration/Cache/Redis/ClusterFallbackIntegrationTest.php
  • tests/Integration/Cache/Redis/PrefixHandlingIntegrationTest.php
  • tests/Integration/Cache/Redis/RedisCacheIntegrationTestCase.php
  • tests/Integration/Generators/SessionTableCommandTest.php
  • tests/Integration/Session/Database/DatabaseSessionHandlerTestCase.php
  • tests/Integration/Session/Database/MariaDb/DatabaseSessionHandlerTest.php
  • tests/Integration/Session/Database/MySql/DatabaseSessionHandlerTest.php
  • tests/Integration/Session/Database/Postgres/DatabaseSessionHandlerTest.php
  • tests/Integration/Session/Database/Sqlite/DatabaseSessionHandlerTest.php
  • tests/Integration/Session/Database/Sqlite/UserSessionLifecycleTest.php
  • tests/Integration/Session/DatabaseSessionHandlerTest.php
  • tests/Integration/Session/Redis/RedisSessionHandlerTest.php
  • tests/Session/CacheBasedSessionHandlerTest.php
  • tests/Session/PackageMetadataTest.php
  • tests/Session/RedisSessionHandlerTest.php
  • tests/Session/SessionConfigTest.php
  • tests/Session/SessionManagerTest.php
  • tests/Session/SessionStoreTest.php
  • tests/Session/UserSessionIdentityTest.php
  • tests/Session/UserSessionsTest.php
💤 Files with no reviewable changes (4)
  • src/session/src/CacheBasedSessionHandler.php
  • tests/Foundation/Testing/Concerns/InteractsWithSessionTest.php
  • tests/Integration/Session/DatabaseSessionHandlerTest.php
  • tests/Session/CacheBasedSessionHandlerTest.php

Comment thread src/docs/session.md
Comment on lines +85 to +87
Redis stores session payloads directly rather than routing them through a cache store. An ordinary session read, write, or deletion therefore uses one Redis command.

To list and invalidate a user's Redis sessions, enable `SESSION_TRACK_USER_SESSIONS`. This feature requires phpredis 6.3.0 or later and Redis 8.0 or Valkey 9.0 or later. You may leave this option disabled if your application does not provide session-management controls.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Qualify the Redis command-count statement.

When SESSION_TRACK_USER_SESSIONS is enabled, a tracked Redis Cluster write can require a payload operation and user-index operations. Line 85 states the one-command behavior without this condition. State that this behavior applies when tracking is disabled, or document the tracked standalone and Cluster paths separately.

🤖 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/docs/session.md` around lines 85 - 87, Update the Redis command-count
statement near the session payload description to qualify the one-command
behavior as applying when SESSION_TRACK_USER_SESSIONS is disabled. Document the
additional user-index operations for tracked sessions, distinguishing standalone
Redis from Redis Cluster writes as needed.

Comment on lines +157 to 158
protected function getDefaultPayload(string $sessionId, string $data): array
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Find subclasses of DatabaseSessionHandler and any override of getDefaultPayload.
set -euo pipefail

# Test: locate overrides of getDefaultPayload. Expect: only DatabaseSessionHandler defines it.
rg -nP --type=php -C3 'function\s+getDefaultPayload\s*\('

# Test: locate subclasses of DatabaseSessionHandler.
rg -nP --type=php -C3 'extends\s+\w*DatabaseSessionHandler\b'

# Test: check whether the upgrade guide mentions the handler payload change.
fd -i 'upgrade.md' --exec rg -n -i -C3 'getDefaultPayload|DatabaseSessionHandler'

Repository: hypervel/components-backup

Length of output: 164


🏁 Script executed:

#!/bin/bash
set -u

printf '%s\n' '--- getDefaultPayload declarations ---'
rg -n --type=php -C4 'function[[:space:]]+getDefaultPayload[[:space:]]*\(' || true

printf '%s\n' '--- DatabaseSessionHandler declarations and subclasses ---'
rg -n --type=php -C5 'class[[:space:]]+[A-Za-z_][A-Za-z0-9_]*[[:space:]]+extends[[:space:]]+[^{]*DatabaseSessionHandler|class[[:space:]]+DatabaseSessionHandler|interface[[:space:]]+DatabaseSessionHandler|trait[[:space:]]+DatabaseSessionHandler' || true

printf '%s\n' '--- files containing upgrade or changelog documentation ---'
fd -i -t f 'upgrade|changelog|changes|release|readme' . || true

printf '%s\n' '--- documentation references ---'
rg -n -i -C3 --glob '*.md' --glob '*.rst' --glob '*.txt' 'getDefaultPayload|DatabaseSessionHandler' . || true

Repository: hypervel/components-backup

Length of output: 203


🏁 Script executed:

#!/bin/bash
set -u

printf '%s\n' '--- tracked session files ---'
git ls-files | rg '(^|/)DatabaseSessionHandler\.php$|(^|/)src/session/' || true

printf '%s\n' '--- all getDefaultPayload declarations ---'
rg -n -C4 'function[[:space:]]+getDefaultPayload[[:space:]]*\(' --glob '*.php' . || true

printf '%s\n' '--- all DatabaseSessionHandler references and subclasses ---'
rg -n -C5 'DatabaseSessionHandler|getDefaultPayload' --glob '*.php' . || true

printf '%s\n' '--- documentation files ---'
git ls-files | rg -i '(^|/)(upgrade|upgrading|changelog|changes|release|readme)([^/]*)$|\.md$|\.rst$|\.txt$' || true

printf '%s\n' '--- documentation references ---'
rg -n -i -C3 'getDefaultPayload|DatabaseSessionHandler' --glob '*.md' --glob '*.rst' --glob '*.txt' . || true

Repository: hypervel/components-backup

Length of output: 50382


🏁 Script executed:

#!/bin/bash
set -u

printf '%s\n' '--- declarations ---'
rg -n --glob '*.php' 'getDefaultPayload[[:space:]]*\(' . || true

printf '%s\n' '--- subclass declarations ---'
rg -n --glob '*.php' 'class[[:space:]]+[A-Za-z_][A-Za-z0-9_]*[[:space:]]+extends[[:space:]]+[A-Za-z_][A-Za-z0-9_]*DatabaseSessionHandler|class[[:space:]]+DatabaseSessionHandler' . || true

printf '%s\n' '--- test subclass bodies ---'
sed -n '800,875p' tests/Integration/Session/Database/DatabaseSessionHandlerTestCase.php

printf '%s\n' '--- upgrade-note references ---'
rg -n -i -C5 'getDefaultPayload|DatabaseSessionHandler|session handler|session persistence' src/docs/upgrade.md || true

Repository: hypervel/components-backup

Length of output: 3234


🏁 Script executed:

#!/bin/bash
set -u

if command -v php >/dev/null 2>&1; then
    php -v | head -n 1
    php -d display_errors=1 <<'PHP'
<?php
class ParentHandler {
    protected function getDefaultPayload(string $sessionId, string $data): array
    {
        return [];
    }
}

class ChildWithOldSignature extends ParentHandler {
    protected function getDefaultPayload(string $data): array
    {
        return [];
    }
}

echo "compatible\n";
PHP
else
    echo "php unavailable"
fi

Repository: hypervel/components-backup

Length of output: 680


Document the getDefaultPayload() signature change in the upgrade notes.

No in-repository subclass overrides getDefaultPayload(). DatabaseSessionHandler is non-final and the method is protected, so external subclasses using the old signature will cause a PHP signature-mismatch fatal error.

🤖 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/session/src/DatabaseSessionHandler.php` around lines 157 - 158, Document
the getDefaultPayload() signature change in the repository’s upgrade notes,
noting that external subclasses overriding this protected method must update
their signature to match the new string $sessionId and string $data parameters
and array return type.

Comment on lines +758 to +761
protected function ownerDigest(string $authProvider, string $userId): string
{
return hash('xxh128', strlen($authProvider) . ':' . $authProvider . ':' . $userId);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Use a cryptographic hash for the owner digest.

hash('xxh128', ...) is a non-cryptographic hash. It provides no collision resistance against chosen inputs. This digest is the only ownership token: it names the index key and is compared inside DESTROY_USER_SESSION_SCRIPT and CLUSTER_DESTROY_SCRIPT.

If user identifiers are attacker-influenced (for example usernames, emails, or externally supplied ids), an account whose identifier collides with a victim's digest gains read and delete access to the victim's sessions through userSessions(), destroyUserSession(), and destroyUserSessions().

A truncated SHA-256 keeps 32 hex characters, so OWNER_DIGEST_LENGTH, ENVELOPE_HEADER_LENGTH, and the Lua ^[0-9a-f]+$ check stay valid.

🔒 Proposed change
     protected function ownerDigest(string $authProvider, string $userId): string
     {
-        return hash('xxh128', strlen($authProvider) . ':' . $authProvider . ':' . $userId);
+        return substr(
+            hash('sha256', strlen($authProvider) . ':' . $authProvider . ':' . $userId),
+            0,
+            self::OWNER_DIGEST_LENGTH,
+        );
     }

Note the migration impact: existing tracked payload envelopes and index keys carry the old digest. After the change, those sessions still read correctly, but they no longer match a user index. Document this in the upgrade notes, or bump ENVELOPE_VERSION so stale envelopes are rejected.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected function ownerDigest(string $authProvider, string $userId): string
{
return hash('xxh128', strlen($authProvider) . ':' . $authProvider . ':' . $userId);
}
protected function ownerDigest(string $authProvider, string $userId): string
{
return substr(
hash('sha256', strlen($authProvider) . ':' . $authProvider . ':' . $userId),
0,
self::OWNER_DIGEST_LENGTH,
);
}
🤖 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/session/src/RedisSessionHandler.php` around lines 758 - 761, Update
RedisSessionHandler::ownerDigest to derive the 32-character owner token from a
cryptographic SHA-256 digest while preserving the existing length and lowercase
hexadecimal format required by OWNER_DIGEST_LENGTH, ENVELOPE_HEADER_LENGTH, and
the Lua validation. Handle the migration impact by documenting that existing
index keys and envelopes no longer match, or by bumping ENVELOPE_VERSION so
stale envelopes are rejected.

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