[2.2] make refreshUser try all providers fixes #4498 #4778

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
8 participants
@Nyorai

Nyorai commented Jul 7, 2012

Make refreshUser try all user providers instead stopping at the first
UserNotFound exception.

Fix for issue 4498
Make refreshUser try all user providers instead stopping at the first
UserNotFound exception.
@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jul 13, 2012

Contributor

a provider should only be triggered if it supports the token and so imho if you have multiple providers that should work on the same token, it would be better to create a chained provider. changing this could have adverse affect on performance.

then again .. right now there is no way to control the order of the providers #2131 which means the order is accidental and not controllable anyway

Contributor

lsmith77 commented Jul 13, 2012

a provider should only be triggered if it supports the token and so imho if you have multiple providers that should work on the same token, it would be better to create a chained provider. changing this could have adverse affect on performance.

then again .. right now there is no way to control the order of the providers #2131 which means the order is accidental and not controllable anyway

@Nyorai

This comment has been minimized.

Show comment Hide comment
@Nyorai

Nyorai Jul 13, 2012

A chained provider would not be the right solution as the use case is for different firewalls with different, seperately managed user providers that just happen to provide the same class of user object. refreshUser currently has no way of knowing which firewall was hit and which providers are in it, so it checks them all and stops at the first success or failure.

The current implementation requires each user provider to provide a user of a different class to all other user providers. If this is by design, it isn't documented and non-obvious.

Nyorai commented Jul 13, 2012

A chained provider would not be the right solution as the use case is for different firewalls with different, seperately managed user providers that just happen to provide the same class of user object. refreshUser currently has no way of knowing which firewall was hit and which providers are in it, so it checks them all and stops at the first success or failure.

The current implementation requires each user provider to provide a user of a different class to all other user providers. If this is by design, it isn't documented and non-obvious.

@stof

This comment has been minimized.

Show comment Hide comment
@stof

stof Oct 13, 2012

Member
Member

stof commented Oct 13, 2012

@jrobeson

This comment has been minimized.

Show comment Hide comment
@jrobeson

jrobeson Jan 1, 2013

Contributor

bump

Contributor

jrobeson commented Jan 1, 2013

bump

@lyrixx

This comment has been minimized.

Show comment Hide comment
@lyrixx

lyrixx Jan 1, 2013

Member

This issue could be fixed with #4498 ?

Member

lyrixx commented Jan 1, 2013

This issue could be fixed with #4498 ?

@Renrhaf

This comment has been minimized.

Show comment Hide comment
@Renrhaf

Renrhaf May 3, 2014

Yes, it could

Renrhaf commented May 3, 2014

Yes, it could

@iltar

This comment has been minimized.

Show comment Hide comment
@iltar

iltar Jul 31, 2014

Contributor

Ran into this issue in 2.4 where multiple token/users on multiple firewalls with different authentication methods.

Contributor

iltar commented Jul 31, 2014

Ran into this issue in 2.4 where multiple token/users on multiple firewalls with different authentication methods.

@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Jun 16, 2016

Owner

Closing this old PR as this is not the right fix. The one explained in #4498 might be better.

Owner

fabpot commented Jun 16, 2016

Closing this old PR as this is not the right fix. The one explained in #4498 might be better.

@fabpot fabpot closed this Jun 16, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment