8000 [Security] User refreshment from identical users provider type by blanchonvincent · Pull Request #12465 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Security] User refreshment from identical users provider type #12465

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

Closed
wants to merge 4 commits into from
Closed

[Security] User refreshment from identical users provider type #12465

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0476141
Add fail test case
blanchonvincent Nov 12, 2014
c0b27af
Fix fail test case
blanchonvincent Nov 12, 2014
37e2950
Fix coding style
blanchonvincent Nov 15, 2014
36c07a2
Improve error handling from piotrpasich's idea
blanchonvincent Nov 20, 2014
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
Fix fail test case
  • Loading branch information
@blanchonvincent
blanchonvincent committed Nov 12, 2014
commit c0b27af2881bc7c28a47dbf90a22c0cf49337e39
12 changes: 9 additions & 3 deletions src/Symfony/Component/Security/Http/Firewall/ContextListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,19 @@ protected function refreshUser(TokenInterface $token)
// let's try the next user provider
} catch (UsernameNotFoundException $notFound) {
if (null !== $this->logger) {
$this->logger->warning(sprintf('Username "%s" could not be found.', $notFound->getUsername()));
$this->logger->warning(sprintf('Username "%s" could not be found from "%s" provider.', $notFound->getUsername(), get_class($provider)));
Copy link

Choose a reason for hiding this comment

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

I assume that you want to achieve the situation where the framework doesn't throw an error when is able to find an user in any of user providers.
I have a question: is it ok to log information about not found user even if he exists in at least one user provider? Maybe this should be logged just before end of the method, when you're quite sure that there is no existing user

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"is it ok to log information about not found user even if he exists in at least one user provider?"
Yes, I did this. Each time there is a UsernameNotFoundException exception, it's because a user provider is able to load the user, but the user is not found inside this provider.

"Maybe this should be logged just before end of the method, when you're quite sure that there is no existing user"
Yes I can do that, good idea :)

85C6 Copy link
Contributor Author

Choose a reason for hiding this comment

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

@piotrpasich I did the change, thx!

}

return;
// let's try the next user provider
}
}

// if a UsernameNotFoundException has been thrown, there are providers
// associated with the token, but none able to refresh it
if (isset($notFound)) {

return;
}

throw new \RuntimeException(sprintf('There is no user provider for user "%s".', get_class($user)));
}
}
0