Fix loadIdentifier called after loadAuthenticator losing resolver config #755
Conversation
|
The fix is quite heavy, as it would need to move the defaulting out of the constructor.
|
| public function __construct(IdentifierInterface $identifier, array $config = []) | ||
| public function authenticate(ServerRequestInterface $request): ResultInterface |
There was a problem hiding this comment.
Is there a reason we can't use setIdentifier()/getIdentifier()? It seems it could be a simpler path forward.
If when we need the identifier, and the collection is empty, authenticators could create their default. We could even have a hook method on AbstractAuthenticator if you wanted to get fancy.
There was a problem hiding this comment.
If when we need the identifier, and the collection is empty, authenticators could create their default.
That's what's being done below isn't it? Just that the _identifier property is used directly instead of using setIdentifer()/getIdentifier().
There was a problem hiding this comment.
After the updates done by dereuromark I see now what you meant :)
There was a problem hiding this comment.
Do you want to make an alternative PR or adjust this one?
Or all good now?
|
We need to squash merge this, and then for merging to 4.x we probably want to go back to cleaner constructor way. |
When loadAuthenticator() was called before loadIdentifier(), the authenticator would receive an empty IdentifierCollection and create its own default Password identifier immediately in the constructor. Later calls to loadIdentifier() would add to the service's identifier collection, but the authenticator already had its own separate collection with the default identifier. This fix changes the default identifier loading from eager (in constructor) to lazy (in authenticate()). This ensures that if loadIdentifier() is called after loadAuthenticator(), the identifier will be loaded into the shared collection before the authenticator tries to use it. Fixes #754 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
dereuromark
force-pushed
the
fix/754-resolver-config-lost
branch
from
788944a to
acfdcf8
Compare
Summary
Fixes #754
When
loadAuthenticator()was called beforeloadIdentifier(), the resolver config (includinguserModelandfinder) would be lost because:loadAuthenticator()triggersauthenticators()→identifiers()which creates an emptyIdentifierCollectionFormAuthenticator) would see the empty collection and create a new defaultPasswordIdentifierwith default resolver configloadIdentifier()would add to the service's collection, but the authenticator already had its own separate collection