Skip to content

feat(api): expose ProviderRegistry and ConditionRegistry with an interface#313

Open
dt-thomas-durand wants to merge 1 commit intoscheb:8.xfrom
dt-thomas-durand:feat/interface-expose-registries
Open

feat(api): expose ProviderRegistry and ConditionRegistry with an interface#313
dt-thomas-durand wants to merge 1 commit intoscheb:8.xfrom
dt-thomas-durand:feat/interface-expose-registries

Conversation

@dt-thomas-durand
Copy link
Contributor

Description

This Pull Request adds TwoFactorProviderRegistryInterface and TwoFactorConditionRegistryInterface, and expose the underlying classes to the rest of the code using those interface.

The purpose is to allow a rewrite ; or a decoration of the logic underneath, unlocking more customization possibilities.

One example: in my codebase, I need to know when 2fa is enabled on a User, but skipped because of a condition.
This is easy when decorating TwoFactorConditionRegistry ; but because of the lack of interface, I must extends a @final class. It's possible, but not recommended.

…rface

Motivation: allow override, and decoration of those pillars of the decision for when a 2fa triggers, and which one triggers.
@scheb
Copy link
Owner

scheb commented Mar 13, 2026

Thanks a lot for the suggestion and for taking the time to draft up a solution.

I'm personally not fully convinced by the idea, mainly because both of these classes are really just internal "infrastructure" that the bundle uses to do its work. They weren't designed as extension points where custom code is supposed to be injected. From the bundle's perspective, its functionality shouldn't depend on whether these classes exist in exactly this form or not. If I decide to refactor, I'd like to be able to do so freely within the internal implementation of the bundle.

Regarding the "final" aspect of those classes: the intention here is to make it clear that these classes are not part of the public API. Instead of using an actual final class, I intentionally chose the @final annotation, so that people still have the option to extend/override internals at their own risk if they really need to. I'm fully aware the bundle can't cover every possible use case out there, so this is the escape hatch for those cases. If you do decide to "hook in some code", I'd strongly recommend making sure everything stays compatible with integration tests or by other means.


That said, I'd really like to better understand your use case and what you’re trying to achieve. Maybe we can find a more generic approach that fits better with the bundle’s design, rather than expanding its public surface area in a way that makes maintenance harder on the long-term.

@dt-thomas-durand
Copy link
Contributor Author

@scheb We're in some kind of edge case.

We use conditions to decide whether 2fa should be skipped or not, even when the user have 2fa enabled.
Therefore, we have two features that are concurrent:

  • A condition uses the trusted device cookie to prevent re-authenticating too much ; associated with a "grace period" and could make the 2fa to be skipped
  • A requirement of authenticating with 2fa could be checked ; preventing accessing some resources if this requirement is not met.

A user that would skip 2fa because of the conditions, but have 2fa enabled could be prevented from accessing the 2fa protected resources, but we want to avoid that.

Or solution is to make the 2fa artificially complete when condition skip 2fa and the user have 2fa enabled.
We did it by "decorating" the TwoFactorConditionRegistry class, and extending it at the same time (even though it's marked as @final)

That way we could do:

$shouldPerformTwoFactorAuthentication = $this->decorated->shouldPerformTwoFactorAuthentication($context);
$account = $context->getUser();
if (!$shouldPerformTwoFactorAuthentication && $account->hasTfaEnabled()) {
  $context->getToken()->setAttribute(SchebTwoFactorAuthenticator::FLAG_2FA_COMPLETE, true);
}
return $shouldPerformTwoFactorAuthentication;

Having the interface would make the decoration more "natural" ; and skip the issue with extending a @final class.
But I understand that those classes being an implementation details you want to keep them out of the exposed surface of the API.

Another approach could be an event triggered when 2fa is skipped because of conditions.
Doing so, you expose an event, that can be listened.
Would you prefer that approach?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants