Inject auth config into adapters via AuthFactory#451
Merged
armanist merged 2 commits intosoftberg:masterfrom Apr 6, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #451 +/- ##
=========================================
Coverage 82.65% 82.66%
Complexity 2831 2831
=========================================
Files 243 243
Lines 7552 7556 +4
=========================================
+ Hits 6242 6246 +4
Misses 1310 1310 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughAuthFactory now injects the full Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,230,241,0.5)
participant Factory as AuthFactory
end
rect rgba(195,230,204,0.5)
participant Config as Config Store
end
rect rgba(255,224,178,0.5)
participant Adapter as Auth Adapter (JWT/Session)
end
rect rgba(248,202,202,0.5)
participant Trait as AuthTrait
end
Config->>Factory: get('auth') (array)
Factory->>Adapter: new Adapter(..., authConfig)
Adapter->>Trait: set $this->config = authConfig
Adapter->>Trait: read two_fa / otp_expires / session.remember_lifetime via $this->config
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/Unit/Auth/Adapters/JwtAuthAdapterTest.php (1)
73-75: Consider extracting config-mutate + adapter-recreate into a helper.The same pattern repeats in several tests. A tiny helper would reduce duplication and keep these scenarios easier to maintain.
Also applies to: 163-163, 178-178, 189-189, 198-198
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Auth/Adapters/JwtAuthAdapterTest.php` around lines 73 - 75, Create a small helper on the test class to DRY the repeated pattern of mutating the config and recreating the adapter: replace repeated config()->set('auth.two_fa', ...); $this->jwtAuth = $this->createJwtAuth(); with a single method (e.g., private function setTwoFaAndRecreateAdapter(bool $enabled)) that calls config()->set('auth.two_fa', $enabled) and then assigns $this->jwtAuth = $this->createJwtAuth(); update all test sites (around lines referencing createJwtAuth and config()->set('auth.two_fa')) to call that helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Auth/Adapters/JwtAuthAdapter.php`:
- Around line 43-54: The public constructor JwtAuthAdapter::__construct
currently requires a fifth parameter $config which breaks backward
compatibility; make the $config parameter optional by defaulting it to an empty
array in the constructor signature and ensure the body continues to assign
$this->config = $config so existing callers with four arguments continue to work
while in-repo callers can still pass config.
In `@tests/Unit/Auth/Adapters/SessionAuthAdapterTest.php`:
- Around line 233-241: The test testWebRememberTokenLifetimeIsConfigurable
currently only asserts cookie()->has('remember_token') and doesn't verify the
configured lifetime; update the test to retrieve the remember_token cookie
(after $this->sessionAuth->signin(...) ) and assert its expiry/Max-Age matches
the configured value (config('auth.session.remember_lifetime') or the literal
86400) so the test actually validates that SessionAuthAdapter/signin honors the
remember_lifetime setting.
---
Nitpick comments:
In `@tests/Unit/Auth/Adapters/JwtAuthAdapterTest.php`:
- Around line 73-75: Create a small helper on the test class to DRY the repeated
pattern of mutating the config and recreating the adapter: replace repeated
config()->set('auth.two_fa', ...); $this->jwtAuth = $this->createJwtAuth(); with
a single method (e.g., private function setTwoFaAndRecreateAdapter(bool
$enabled)) that calls config()->set('auth.two_fa', $enabled) and then assigns
$this->jwtAuth = $this->createJwtAuth(); update all test sites (around lines
referencing createJwtAuth and config()->set('auth.two_fa')) to call that helper.
🪄 Autofix (Beta)
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
Run ID: 1cd2d67b-962e-4a6c-94b6-9bfa4b6ccd49
📒 Files selected for processing (10)
src/Auth/Adapters/JwtAuthAdapter.phpsrc/Auth/Adapters/SessionAuthAdapter.phpsrc/Auth/Factories/AuthFactory.phpsrc/Auth/Traits/AuthTrait.phpsrc/Module/Templates/DemoApi/src/config/auth.php.tplsrc/Module/Templates/DemoWeb/src/config/auth.php.tpltests/Unit/Auth/Adapters/JwtAuthAdapterTest.phptests/Unit/Auth/Adapters/SessionAuthAdapterTest.phptests/Unit/Auth/AuthTest.phptests/_root/shared/config/auth.php
armanist
added
enhancement
…ward compatibility
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Auth/Adapters/SessionAuthAdapter.php (1)
208-213: Consider explicit type casting for the cookie lifetime parameter.The config value retrieved from
$this->config['session']['remember_lifetime']is of typemixed(per thearray<string, mixed>typing). WhileCookie::set()expectsint $time, PHP will implicitly coerce the value. If an invalid or non-numeric value is provided in config, it could silently become0, causing immediate cookie expiration.Additionally, if
$this->config['session']exists but is not an array, accessing['remember_lifetime']on it would cause an error.🛡️ Suggested defensive handling
- $rememberLifetime = $this->config['session']['remember_lifetime'] ?? self::DEFAULT_REMEMBER_LIFETIME; + $sessionConfig = $this->config['session'] ?? []; + $rememberLifetime = (int) ($sessionConfig['remember_lifetime'] ?? self::DEFAULT_REMEMBER_LIFETIME);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Auth/Adapters/SessionAuthAdapter.php` around lines 208 - 213, The retrieved remember lifetime may be non-array or non-numeric; update the logic around $this->config['session'] and the $rememberLifetime value used for cookie()->set so it defensively ensures $this->config['session'] is an array, extracts a numeric value for 'remember_lifetime', falls back to self::DEFAULT_REMEMBER_LIFETIME when missing/invalid, and explicitly casts to int before passing to cookie()->set (the cookie key is $this->keyFields[AuthKeys::REMEMBER_TOKEN]); ensure the final value passed to Cookie::set() is a validated int to avoid silent zero/expiry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Auth/Adapters/SessionAuthAdapter.php`:
- Around line 208-213: The retrieved remember lifetime may be non-array or
non-numeric; update the logic around $this->config['session'] and the
$rememberLifetime value used for cookie()->set so it defensively ensures
$this->config['session'] is an array, extracts a numeric value for
'remember_lifetime', falls back to self::DEFAULT_REMEMBER_LIFETIME when
missing/invalid, and explicitly casts to int before passing to cookie()->set
(the cookie key is $this->keyFields[AuthKeys::REMEMBER_TOKEN]); ensure the final
value passed to Cookie::set() is a validated int to avoid silent zero/expiry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc7a3dcf-1422-4e6a-89fd-9233b56c98d8
📒 Files selected for processing (2)
src/Auth/Adapters/JwtAuthAdapter.phpsrc/Auth/Adapters/SessionAuthAdapter.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Auth/Adapters/JwtAuthAdapter.php
andrey-smaelov
approved these changes
Apr 6, 2026
armanist
deleted the
449-Enhancement-Inject-auth-config-into-adapters-via-AuthFactory
branch
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
2 participants
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.
Closes #449
Summary by CodeRabbit
New Features
Refactor
Tests