Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ogizanagi commentedJan 29, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 |
nicolas-grekas commentedJan 29, 2017
I think it's a good idea to make |
ogizanagi commentedJan 29, 2017
@chalasr ,@nicolas-grekas : See#21455 |
062580c to6e4e1baCompare| useSymfony\Component\DependencyInjection\ChildDefinition; | ||
| useSymfony\Component\DependencyInjection\ContainerBuilder; | ||
| useSymfony\Component\DependencyInjection\Reference; | ||
| useSymfony\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
nicolas-grekas left a comment
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
chalasr commentedJan 31, 2017
Uses order fixed (must be the last time, moved to a real IDE everywhere) |
chalasr commentedJan 31, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Same done for authentication providers in last commitcd6422a. This service is injected into all listeners. WDYT? I can do it in another PR if you prefer. |
…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 (withsymfony/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):```phpclass 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 (withsymfony/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):```phpclass 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
chalasr commentedFeb 14, 2017
@nicolas-grekas still ok for you? |
nicolas-grekas commentedFeb 14, 2017
Still OK |
fabpot commentedFeb 16, 2017
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/aAuthentication stops on the first authenticator that fails or succeeds, let's instantiate them only if actually needed.Commits-------cd6422a [SecurityBundle] Lazy load authentication providersb8a23de [Security][Guard] Lazy load authenticators
Authentication stops on the first authenticator that fails or succeeds, let's instantiate them only if actually needed.