Harden JWT validation, revocation, and key generation - #492
Merged
Conversation
JWT registered date claims are parsed before signature validation. Malformed external values could therefore escape the provider as native PHP errors, while an epoch-zero expiration was incorrectly treated as an absent claim.\n\nTranslate parser failures at the untrusted decode boundary into TokenInvalidException without hiding application-owned encode failures. Treat only null expiration as absent, and document why not-before validation remains active during refresh.\n\nAdd focused regressions for malformed registered dates, string-zero parsing, epoch-zero expiration, and the existing leeway behavior.
JWT previously maintained separate refresh and blacklist lifetimes, ignored cache write failures, and cleared guard state before revocation had settled. Those gaps could allow a refreshable token to outlive its blacklist entry or report a successful logout without durable invalidation.\n\nUse refresh_ttl as the single acceptance and retention lifetime, including expiration leeway and the final minute boundary. Reject missing issued-at claims before infinite refresh, use one clock snapshot for finite retention, honor grace periods without allowing repeated refreshes to extend them, and keep explicit permanent invalidation immediate.\n\nReturn and enforce real storage results through the manager and guard. Preserve guard state and suppress Logout when persistence fails. Remove the destructive unused PSR adapter, retain the tagged-cache and custom-storage extension points, and keep configuration defaults at their owning merged boundary.\n\nThe regressions cover finite and infinite refresh, missing claims, delayed cache reads, leeway, grace, false writes and flushes, zero identifiers, provider replacement, custom storage, and transactional logout.
The certificate command wrote private and public keys directly, accepted RSA sizes rejected by the installed signer, and treated the valid passphrase string zero as empty. A failed or interrupted write could leave incomplete key material with permissive modes.\n\nUse the framework Filesystem owner to create the directory and atomically replace both key files with explicit private and public permissions. Reject RSA keys below 2048 bits before OpenSSL work begins, validate exported public-key contents, and preserve every non-empty passphrase.\n\nDeclare the direct Filesystem dependency and cover supported publication, file modes, invalid RSA sizes, passphrase encryption, overwrite behavior, and the existing EC validation paths.
Bring the canonical JWT guide in line with the corrected runtime behavior. Explain custom signing drivers and storage implementations, the taggable-cache requirement, unified refresh and blacklist retention, grace-aware invalidation, transactional logout failures, and the RSA key-size floor.\n\nKeep the prose application-focused and Laravel-shaped. Narrow the refresh example to token validity failures, describe key configuration accurately, and avoid presenting infrastructure or configuration failures as authentication errors.
Make the package README a thin entry point instead of a second documentation surface. Link to the canonical JWT guide, retain only the public differences developers must account for, and keep the tracked upstream reference last.\n\nRemove implementation detail and duplicated package guidance that would otherwise drift from the framework documentation.
Bring the remaining JWT fixtures in line with the repository's test conventions. Add explicit void returns, remove unused untyped state, and leave each test responsible only for behavior it actually exercises.\n\nThese changes keep the suite strict and readable without adding production code or test-only framework machinery.
Several tests passed multiple method names to shouldNotReceive even though Mockery registers that call as one method expectation. The assertions looked strict but did not prohibit the intended calls.\n\nRegister each prohibited method through a never expectation and remove dead arguments from the single Database prohibition. This makes the existing tests enforce their stated contracts without changing framework behavior or adding recurrence machinery.
Add the reviewed JWT correctness, security, and lifecycle plan with its evidence, rejected designs, regression coverage, performance assessment, and completion criteria.\n\nRecord the final jwt-01 through jwt-15 decisions in the companion ledger, close the JWT checklist, and mark the shared enum-identifier and Macroable dependency revalidations complete. Clear the active routing entry now that implementation, verification, self-review, and independent review have all finished.
…security-lifecycle # Conflicts: # docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md
The existing blacklist read prevents repeated revocations from restarting a configured grace period. It does not provide cross-store atomic insertion for first writes that were already overlapping. Document the bounded scheduling tradeoff and the safety properties that make additional synchronization unnecessary. Clarify that certificate publication is atomic per generated file rather than across the pair. Add the missing method documentation to the public storage extension contract so its finite, permanent, read, removal, and flush operations are explicit.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
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
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 change hardens JWT parsing, refresh, revocation, certificate generation, and worker-lifetime behavior.
Malformed registered date claims now fail through the JWT exception boundary instead of escaping as native type errors. Epoch-zero expiration is validated correctly, and refresh requires an issued-at claim before applying either finite or unlimited refresh lifetimes.
Revocation now uses the same lifetime as token refresh, includes expiration leeway, and calculates retention from one clock snapshot. Cache write failures remain visible through invalidate, refresh, and logout. Logout only clears guard state and emits its event after revocation succeeds.
Storage
The blacklist storage contract now returns the result of finite writes, permanent writes, and flushes. The tagged-cache implementation passes those results through without additional I/O.
The unused PSR cache adapter has been removed. It could not clear only JWT-owned entries and its flush operation could clear the application cache. Tagged storage remains the built-in implementation, while custom storage continues through
StorageContract.Blacklist identifiers accept non-empty strings and integers, including zero, and reject values that cannot safely form storage keys.
Certificate generation
The certificate command now:
0.The generated key paths and algorithm continue to be written to the environment file after both key files are ready.
Configuration and documentation
The duplicate blacklist refresh lifetime has been removed.
refresh_ttlis now the single refresh and blacklist-retention setting, including unlimited refresh lifetimes.Nested provider configuration keeps explicit child fallbacks because application configuration may replace the full provider array. Redundant top-level defaults were removed after tracing the package configuration merge.
The JWT guide now documents custom providers, custom blacklist storage, taggable-cache requirements, revocation failure behavior, force-forever invalidation, refresh retention, and certificate requirements. The package README is reduced to public differences and links to the main documentation.
Supported auth guard, manager, facade, parser, custom provider, and custom storage APIs remain available. The removed configuration key and PSR adapter were Hypervel-specific surfaces that duplicated or weakened the supported behavior.
Tests
Coverage was added for malformed date claims, epoch-zero expiration, missing issued-at refreshes, finite and unlimited revocation, leeway boundaries, delayed cache operations, failed persistence, transactional logout, identifier validation, provider fallback behavior, certificate modes and limits, passphrase handling, and tagged-storage results.
The focused JWT suite, split package validation, facade lint, repository formatting, static analysis, parallel test suite, Testbench suite, and dogfood checks pass.