Skip to content

Conversation

blanchonvincent
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

When we are using two privoders with the same supported class but with different users, there is a faill because the first UsernameNotFoundException will stop the refresh processus.

}
}

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

Choose a reason for hiding this comment

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

This breaks CI. Previously, if you hadn't got any userProvider registered there, the code shoud throw an error. Now it retirns null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where do you see this line breaks CI? I don't see any break there https://travis-ci.org/symfony/symfony/jobs/41113847 :/
Previously, if you hadn't got any userProvider registered there, , the code shoud not throw an error : https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Security/Http/Firewall/ContextListener.php#L173.
Previously the code return null.

@fabpot fabpot added the Security label Dec 7, 2014
foreach ($exceptions as $exception) {
list($notFound, $provider) = $exception;
$this->logger->warning(sprintf('Username "%s" could not be found from "%s" provider.', $notFound->getUsername(), get_class($provider)));
}
}
Copy link
Member

Choose a reason for hiding this comment

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

There is a missing return here, like the current code.

@xabbuh
Copy link
Member

xabbuh commented Feb 27, 2017

closing in favour of #21791

@xabbuh xabbuh closed this Feb 27, 2017
fabpot added a commit that referenced this pull request Feb 28, 2017
This PR was merged into the 2.7 branch.

Discussion
----------

[SecurityBundle] only pass relevant user provider

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #4498, #12465, #20401, #21737
| License       | MIT
| Doc PR        |

There is no need for the context listener to be aware of all the configured user providers. It must only use the provider for the current firewall (the one identified by the context key passed to the constructor) to refresh the user from the session.

Commits
-------

d97e07f [SecurityBundle] only pass relevant user provider
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants