- Issue created by @sime
- 🇺🇸United States cmlara
I'm aware of ECA although to date I have not personally given it more than a cursory glance. Even though ECA's install count is small (around 2k reported at the moment) it does seem it might possibly become the replacement for Rules in the future (though I would be unable to opine on the strengths of either) especially because no D8+ stable release of Rules yet exists.
Cursory discussions indicate there there may indeed be at least one use for such a plugin (redirect on login) and I could see that some ohter uses might exist (such as only allow TFA enabled accounts access to certain nodes). As such I can see value in such a plugin existing.
The primary call isReady() is in a class marked @internal in 2.x. I would prefer to keep it so in order to allow TFA the flexibility to respond to changing needs in the future.
Given the above I would be supportive of an MR to add this into the main repo.
- Assigned to sime
- Status changed to Needs work
over 1 year ago 3:48am 1 December 2023 - 🇮🇳India bhanu951
I would suggest to go with submodule in TFA instead of standalone plugin in the TFA module, as this plugin will have external dependency of ECA Module which won't be present in all sites which are planning to use this module.
- 🇺🇸United States cmlara
as this plugin will have external dependency of ECA Module which won't be present in all sites which are planning to use this module.
It is my understanding that plugins are scoped to the ecosystem they work with eg tfa\Plugins\ECA, and should only be loaded when the plugin manager for ECA is installed.
There is 🐛 AttributeClassDiscovery fails while trying to include non valid plugin class Fixed in core, however that applies only to Attribute based plugins and to my understanding should only pose an issue if the ECA namespace is listed for discovery and the plugin has a dependency on yet another module that is not installed.
No other code should attempt discovery for tfa\Plugins\ECA except for ECA, or some other module that provides all of ECA's API. So long as our plugin doesn't use source beyond that of TFA or ECA it shouldn't pose an issue.
ECA will likely becomes a require-dev for testing however that is acceptable dependency as it doesn't impact end users.
- Status changed to Needs review
over 1 year ago 3:19am 4 December 2023 - 🇺🇸United States rtdean93
Bumping to ask about the status of this?
I am wanting to improve the UX for people by providing a Block which can be shown on pages when a TFA Status "is TFA setup" is False.
Any ideas?
Thank you.
- 🇺🇸United States cmlara
This one kinda fell through and was lost.
Wording is still the biggest one. With a fresh set of eyes it may need to be something along the lines of "Account is known to have configured TFA" or similar as the isReady() method really is checking for a known configured plugin yet doesn't assert that it is properly operational.
The entire 'check if a user needs to login with TFA' subsystem is seeing patch after patch as we find new bad assumptions (at the time my last comments were made https://www.drupal.org/sa-contrib-2024-003 → was likely running through my mind).
Eventually we can come back to firm it up to be more towards known operational.Needs work as it requires a rebase at a minimal. We also may want to look at the fact ECA 2.x is now out and see what impact this has, if both can be supported or not.