Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

New Guard Authentication System (e.g. putting the joy back into security) #14673

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.

Sign up for GitHub

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

Merged
merged 24 commits into from
Sep 24, 2015
Merged

New Guard Authentication System (e.g. putting the joy back into security) #14673

Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
330aa7f
Improving phpdoc on AuthenticationEntryPointInterface so people that …
weaverryan May 17, 2015
05af97c
Initial commit (but after some polished work) of the new Guard authen…
weaverryan May 17, 2015
a0bceb4
adding Guard tests
weaverryan May 17, 2015
873ed28
Renaming the tokens to be clear they are "post" and "pre" auth - also…
weaverryan May 17, 2015
180e2c7
Properly handles "post auth" tokens that have become not authenticated
weaverryan May 17, 2015
6c180c7
Adding an edge case - this should not happen anyways
weaverryan May 17, 2015
c73c32e
Thanks fabbot!
weaverryan May 17, 2015
eb158cb
Updating interface method per suggestion - makes sense to me, Request…
weaverryan May 18, 2015
d693721
Adding periods at the end of exceptions, and changing one class name …
weaverryan May 18, 2015
6edb9e1
Tweaking docblock on interface thanks to @iltar
weaverryan May 18, 2015
ffdbc66
Splitting the getting of the user and checking credentials into two s…
weaverryan May 18, 2015
7de05be
A few more changes thanks to @iltar
weaverryan May 18, 2015
7a94994
Thanks again fabbot!
weaverryan May 18, 2015
81432f9
Adding missing factory registration
weaverryan May 25, 2015
293c8a1
meaningless author and license changes
weaverryan Sep 20, 2015
0501761
Allowing for other authenticators to be checked
weaverryan Sep 20, 2015
31f9cae
Adding a base class to assist with form login authentication
weaverryan Sep 20, 2015
c9d9430
Adding logging on this step and switching the order - not for any hu…
weaverryan Sep 20, 2015
396a162
Tweaks thanks to Wouter
weaverryan Sep 20, 2015
302235e
Fixing a bug where having an authentication failure would log you out.
weaverryan Sep 21, 2015
dd485f4
Adding a new exception and throwing it when the User changes
weaverryan Sep 21, 2015
e353833
fabbot
weaverryan Sep 21, 2015
d763134
Removing unnecessary override
weaverryan Sep 22, 2015
a01ed35
Adding the necessary files so that Guard can be its own installable c…
weaverryan Sep 24, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
A few more changes thanks to @iltar
  • Loading branch information
@weaverryan
weaverryan committed Sep 20, 2015
commit 7de05be3f6396e8893c7204f1dfdbec905063dfa
8 changes: 4 additions & 4 deletions 8 src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class GuardAuthenticationListener implements ListenerInterface
* @param GuardAuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider
* @param LoggerInterface $logger A LoggerInterface instance
*/
public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null)
public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, array $guardAuthenticators, LoggerInterface $logger = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anything preventing you from using a NullLogger instance if not provided ? In order to avoid such things:

if (null !== $this->logger) {
    ...
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing technical preventing that, but NullLogger isn't used anywhere in core currently - having an optional argument like this is used :)

Copy link
Contributor

Choose a reason for hiding this comment

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

What I think he means is the following:

$this->logger = $logger ?: new NullLogger();

IMO this is a cleaner solution as you don't have to worry about if($this->logger) statements. You can just log away safely.

Copy link
Contributor

Choose a reason for hiding this comment

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

See #14682

Copy link
Contributor

Choose a reason for hiding this comment

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

agree with @iltar it does simplify a little bit IMO

Copy link
Member

Choose a reason for hiding this comment

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

👎 for a NullLogger, Symfony should take options for most performance.

{
if (empty($providerKey)) {
throw new \InvalidArgumentException('$providerKey must not be empty.');
Expand All @@ -57,7 +57,7 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat
public function handle(GetResponseEvent $event)
{
if (null !== $this->logger) {
$this->logger->info('Checking for guard authentication credentials', array('firewall_key' => $this->providerKey, 'authenticators' => count($this->guardAuthenticators)));
$this->logger->info('Checking for guard authentication credentials.', array('firewall_key' => $this->providerKey, 'authenticators' => count($this->guardAuthenticators)));
}

foreach ($this->guardAuthenticators as $key => $guardAuthenticator) {
Expand Down Expand Up @@ -121,13 +121,13 @@ private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorIn
$response = $this->guardHandler->handleAuthenticationSuccess($token, $request, $guardAuthenticator, $this->providerKey);
if ($response instanceof Response) {
if (null !== $this->logger) {
$this->logger->info('Guard authenticator set success response', array('response' => $response, 'authenticator' => get_class($guardAuthenticator)));
$this->logger->info('Guard authenticator set success response.', array('response' => $response, 'authenticator' => get_class($guardAuthenticator)));
}

$event->setResponse($response);
} else {
if (null !== $this->logger) {
$this->logger->info('Guard authenticator set no success response: request continues', array('authenticator' => get_class($guardAuthenticator)));
$this->logger->info('Guard authenticator set no success response: request continues.', array('authenticator' => get_class($guardAuthenticator)));
}
}

Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.