The "tfa_trusted_browser" plugin does not exist.

Created on 21 March 2024, 9 months ago
Updated 16 September 2024, 3 months ago

Problem/Motivation

When I edit a user (as root), and go to "TFA", I get the following message:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "tfa_trusted_browser" plugin does not exist. Valid plugin IDs for Drupal\tfa\TfaValidationPluginManager are: tfa_recovery_code, tfa_hotp, tfa_totp in Drupal\tfa\TfaValidationPluginManager->doGetDefinition() (line 53 of core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php).

The root cause is that tfa_trusted_browser is a login plugin, and the others are validation plugins.

Steps to reproduce

  • Go to a user with tfa enabled
  • Click on the tfa-tab
๐Ÿ› Bug report
Status

Needs work

Version

1.5

Component

Code

Created by

๐Ÿ‡ณ๐Ÿ‡ฑNetherlands joshahubbers

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @joshahubbers
  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands joshahubbers
  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands joshahubbers
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States cmlara

    A couple quick notes, not near a lab at the moment to check this:
    We probably want to validate that itโ€™s a validation plug-in class rather than excluding by plug-in name.

    Iโ€™m curious why the method is being set to a login plugin in the first place. Do we have a lower root fault that this would be masking?

    As an aside:
    The TFA project has moved fully to GitlabCi for the D8+ release branches. Due to this change we can only work with MRโ€™s as patch files can no longer be tested. This will become more common among contrib projects as DrupalCi is fast approaching a full shutdown. MRโ€™s are now recommended to be used whenever possible across the entire D.O. ecosystem.

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands joshahubbers

    I totally agree with you that this was not the way to go. I attach a new patch and will open a MR for it too (we only use patches in our CI because the diff-contents of a mr can change without notice).

    Also we should use dependency injection for the plugin manager, but that is beyond the scope of this bug.

  • Status changed to Needs review 9 months ago
  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands joshahubbers
  • Status changed to Needs work 9 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States cmlara

    I have only been able to reproduce this by Navigating to a user with TFA(at least the trusted browser plugin), clicking on the TFA tab, and than clicking on "Configure trusted browsers"

    Is that true for your scenario? If so the question regarding method being set from #4 would make sense, otherwise the method should not be present on the TFA page itself.

    It appears this stems from implementing #3075304: Users' recovery codes exposed to admin users โ†’ which implies we will need to ensure we evaluate the security implications of how we solve this.

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands joshahubbers

    Hm, I get the error as soon as I click on the "TFA"-tab of a user with TFA enabled, and setup (TOTP). We do have patch #34 of this issue โœจ Force user to setup TFA when required and there are no remaining skips Needs work applied.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States cmlara

    I get the error as soon as I click on the "TFA"-tab of a user with TFA enabled, and setup (TOTP). We do have patch #34 of this issue #3223327: Force user to setup TFA when required and there are no remaining skips applied.

    That should be the 'tfa.overview' page, which does not have the method named parameter (it will return NULL when called) see
    https://git.drupalcode.org/project/tfa/-/blob/b1ab6d112fff674db2d1b660c3....

    Can you try without the patch to confirm if the issue still exists or not, and validate if your routing file has any changes?

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands joshahubbers

    Hmm, I replayed the scenario in a clean d10 install, and I cannot reproduce it there. So we have to dig deeper in our setup and other authentication-related modules to find out what is causing this error.

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands joshahubbers

    When debugging the accessSelfOrAdmin function when accessing /user/x/security/tfa (tfa.overview), i notice that the error occurs on the route tfa.validation.setup. This is function also is the access check for this route, so it will be called when rendering the link to /user/x/security/tfa/tfa_trusted_browser.

    Now I come to your scenario mentioned above:

    I have only been able to reproduce this by Navigating to a user with TFA(at least the trusted browser plugin), clicking on the TFA tab, and than clicking on "Configure trusted browsers".

    Apperantly on a clean install the access-check for tfa.validation.setup is not called when rendering the link. But in our setup the access check is also called when rendering the link.

    So after this I think we can narrow the root problem down to the access check for the route tfa.validation.setup for login-plugins when $is_self is NOT true.

    So I think this patch is valid because this breaks:

    • access the tfa.validation.setup path
    • for another user, not yourself
    • for a login-plugin (currently only trusted_browser)

    The question I cannot answer is: why is there no access check on rendering the links in the clean d10 setup? Because this is the cause of also breaking the tfa.overview route in our setup.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States cmlara

    Apperantly on a clean install the access-check for tfa.validation.setup is not called when rendering the link. But in our setup the access check is also called when rendering the link, not only at accessing the link.

    OK, at least we have a matching proof at this point.

    So I think this patch is valid because this breaks:

    This is going to get into the architectural side now. While I do believe this was a bug from #3075304: Users' recovery codes exposed to admin users โ†’ I'm not sure if we should allow Login plugins to be maintained by admins in 8.x-1.x or not. This has acted as an unofficial prohibition against admins viewing/editing login plugins since at least the 8.x-1.0 stable release. We may need to make 1.x always deny for login plugins.

    Opened ๐Ÿ“Œ Evaluate if allowUserSetupAccess should be on API or removed Active to deal with the 2.x issue of how that method should be treated going forward.

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands joshahubbers
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia bhanu951

    I too am also able to reproduce this by Navigating to a user with TFA (at least the trusted browser plugin), clicking on the TFA tab, and than clicking on "Configure trusted browsers" as mentioned in #7 ๐Ÿ› The "tfa_trusted_browser" plugin does not exist. Needs work

Production build 0.71.5 2024