-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Security] Lazy load guard authenticators and authentication providers #21450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
In case a logger is provided, the number of authenticators is provided in the context array (GuardAuthenticationListener.php#L69). Do you think it'll be worth it to make |
I think it's a good idea to make |
@chalasr , @nicolas-grekas : See #21455 |
062580c
to
6e4e1ba
Compare
@@ -15,6 +15,7 @@ | |||
use Symfony\Component\DependencyInjection\ChildDefinition; | |||
use Symfony\Component\DependencyInjection\ContainerBuilder; | |||
use Symfony\Component\DependencyInjection\Reference; | |||
use Symfony\Component\DependencyInjection\Argument\IteratorArgument; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alpha order, should be first I guess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 with minor comment
6e4e1ba
to
fef35f9
Compare
Uses order fixed (must be the last time, moved to a real IDE everywhere) |
fef35f9
to
1d6a3fe
Compare
Same done for authentication providers in last commit cd6422a. This service is injected into all listeners. WDYT? I can do it in another PR if you prefer. |
f3a5447
to
118f2cb
Compare
118f2cb
to
cd6422a
Compare
…anagi) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Allow to count on lazy collection arguments | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #21450 (comment) | License | MIT | Doc PR | todo (with symfony/symfony-docs#7336) When using the new iterator feature of the DI component to lazy load collection, we always know the number of arguments in the collection (only the invalidBehavior set to `IGNORE_ON_INVALID_REFERENCE` may change this number). So we are able to generate and use a `RewindableGenerator` implementing `\Countable` by computing this value ahead. So, in a service accepting `array|iterable`, like in the `GuardAuthenticationListener` (see #21450): ```php class GuardAuthenticationListener implements ListenerInterface { private $guardAuthenticators; /** * @param iterable|GuardAuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider * @param LoggerInterface $logger A LoggerInterface instance */ public function __construct($guardAuthenticators, LoggerInterface $logger = null) { // ... } public function handle(GetResponseEvent $event) { if (null !== $this->logger) { $context = array() if (is_array($this->guardAuthenticators) || $this->guardAuthenticators instanceof \Countable) { $context['authenticators'] = count($this->guardAuthenticators); } $this->logger->debug('Checking for guard authentication credentials.', $context); } // ... } } ``` we still keep the ability to call count without loosing the lazy load benefits. Commits ------- f23e460 [DI] Allow to count on lazy collection arguments
…anagi) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Allow to count on lazy collection arguments | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony/symfony#21450 (comment) | License | MIT | Doc PR | todo (with symfony/symfony-docs#7336) When using the new iterator feature of the DI component to lazy load collection, we always know the number of arguments in the collection (only the invalidBehavior set to `IGNORE_ON_INVALID_REFERENCE` may change this number). So we are able to generate and use a `RewindableGenerator` implementing `\Countable` by computing this value ahead. So, in a service accepting `array|iterable`, like in the `GuardAuthenticationListener` (see #21450): ```php class GuardAuthenticationListener implements ListenerInterface { private $guardAuthenticators; /** * @param iterable|GuardAuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider * @param LoggerInterface $logger A LoggerInterface instance */ public function __construct($guardAuthenticators, LoggerInterface $logger = null) { // ... } public function handle(GetResponseEvent $event) { if (null !== $this->logger) { $context = array() if (is_array($this->guardAuthenticators) || $this->guardAuthenticators instanceof \Countable) { $context['authenticators'] = count($this->guardAuthenticators); } $this->logger->debug('Checking for guard authentication credentials.', $context); } // ... } } ``` we still keep the ability to call count without loosing the lazy load benefits. Commits ------- f23e460 [DI] Allow to count on lazy collection arguments
@nicolas-grekas still ok for you? |
Still OK |
Thank you @chalasr. |
…cation providers (chalasr) This PR was squashed before being merged into the 3.3-dev branch (closes #21450). Discussion ---------- [Security] Lazy load guard authenticators and authentication providers | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Authentication stops on the first authenticator that fails or succeeds, let's instantiate them only if actually needed. Commits ------- cd6422a [SecurityBundle] Lazy load authentication providers b8a23de [Security][Guard] Lazy load authenticators
Authentication stops on the first authenticator that fails or succeeds, let's instantiate them only if actually needed.