Roadmap for 2.0.0 release

Created on 16 July 2023, over 1 year ago
Updated 7 February 2024, 10 months ago

Overview

Note this is still a work in progress list and should not be considered final at this time. The items in each category are not in any specific order.

Must Have

πŸ“Œ SA-CONTRIB-2023-030 and 2.x Active
πŸ› Creation of dynamic property Drupal\tfa\Plugin\Tfa\TfaTotp::$isValid is deprecated Fixed
πŸ“Œ Public Followup for 8.x-1.3 security release. Postponed
πŸ› Resolve SA-CONTRIB-2024-003 in 2.x branch Postponed
πŸ› Installing contrib modules can lead to TFA accidently being bypassed Fixed
πŸ› Import 8.x-1.4 security fix into 2.x Fixed

Should Have

πŸ“Œ Inform admin that role does not have permission to setup own tfa Fixed
πŸ› Gracefully handle EncryptionServiceInterface Exceptions Active
✨ Allow TFA authentication through REST routes Active
πŸ’¬ Cleanup accepted codes stored in user data Postponed
πŸ“Œ Switch to spomky-labs/otphp Fixed Note: I think we should reconsider πŸ“Œ Remove TOTP/HOTP plugins from TFA module (adding was a regression) Closed: works as designed and move TOTP/HOTP to their own module(s)
πŸ“Œ Mark removed code as deprecated Postponed
πŸ› TFA prevents Devel switch user Closed: works as designed
πŸ“Œ Mark classes as internal/final/api as applicable Needs work
πŸ“Œ Add return, property, and parameter type hints to code Fixed
πŸ“Œ Convert plugin configuration to Config Entities Active
πŸ“Œ Require plugins to be responsible for locking Active
πŸ“Œ Convert plugins to use PHP Attributes only. Active
πŸ“Œ Evaluate restoring using a form alter instead of extending UserLoginForm Active
πŸ“Œ Remove usage of alreadyAcceptedCode()/storeAccepedCode() in the TOTP,HOTP, Recovery Code plugins. Active
πŸ“Œ Convert TFA plugin forms to use subforms Active

Like to have

#3322260: Add client IP address as criteria for invalidating TFA β†’
✨ Disallow viewing recovery codes after first display Active
✨ Invalidate other sessions when a user enables TFA Active

🌱 Plan
Status

Active

Version

2.0

Component

Miscellaneous

Created by

πŸ‡ΊπŸ‡ΈUnited States cmlara

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

Comments & Activities

  • Issue created by @cmlara
  • πŸ‡¬πŸ‡§United Kingdom mustanggb Coventry, United Kingdom
  • πŸ‡ΊπŸ‡ΈUnited States cmlara

    Removing ✨ Increase security with single failure for login, password, and tfa Postponed: needs info from the should have list as it appears it would introduce security concerns.

  • πŸ‡ΉπŸ‡·Turkey orkut murat yΔ±lmaz Istanbul

    Hello and thanks for this beautiful module:)

    Which items in the "must have" and "should have" lists are considered as beta version blockers?

    Best,
    Orkut

  • πŸ‡ΊπŸ‡ΈUnited States cmlara

    With πŸ“Œ Mark classes as internal/final/api as applicable Needs work we formally moved a large amount of the code to not be API, which could allow us to make sweeping changes across the code base post beta release, limited primarily by our plugin related code which is mostly public API.

    That leaves with needing to close any public security concerns and anything that might modify the remaining portion of public API as blockers:

    As it stands at the moment I believe we have:

    Security Followups:
    πŸ“Œ SA-CONTRIB-2023-030 and 2.x Active
    πŸ“Œ Public Followup for 8.x-1.3 security release. Postponed

    Possible API changes:
    πŸ› Gracefully handle EncryptionServiceInterface Exceptions Active
    πŸ’¬ Cleanup accepted codes stored in user data Postponed
    ✨ Allow TFA authentication through REST routes Active
    πŸ“Œ Deprecate the services_tfa module Active

    πŸ“Œ Mark removed code as deprecated Postponed may also be necessary to aid developers in upgrading to 2.x.

  • πŸ‡΅πŸ‡ΉPortugal jcnventura

    @cmlara, just a note on the above mention of making the plugins be their own module again.. If I understand your objection well, the problem is that it is not really possible to completely remove them, and could interfere with a third-party plugin.

    I would rather move to have config entities, so that you could have multiple (or zero) instances of the plugins with user-defined identifiers. Splitting the plugins into their own modules only adds complexity to the release process as ga_login and tfa were installed together in 99% of the cases. And a few users that only installed tfa were confused why nothing seemed to work.

  • πŸ‡ΊπŸ‡ΈUnited States cmlara

    I would rather move to have config entities, so that you could have multiple (or zero) instances of the plugins with user-defined identifiers.

    That sounds like an amazing idea. I can also see that working with the fact that I could see us wanting/needing to add some lifecycle management in the future, and having configuration be on entities would likely help manage that.

    Created followup πŸ“Œ Convert plugin configuration to Config Entities Active and added as a should-have (we could release 2.x without it using our existing method, however this will likely require a major bump so there is an advantage to making sure it is in 2.x.)

    While this doesn't appear to solve my concerns regarding 'dead code' it does appear as a viable fix to some of my UI concerns for UX.

    the problem is that it is not really possible to completely remove them, and could interfere with a third-party plugin

    I'm not really sure I see how this is possible? The plugins themselves are relativity independent code (and are now marked final so not being extended)

    The only code that I think might be relevant to 3rd party plugins would be the TfaBasePlugin, which (and realizing I haven't written this down in the original linked issue) we could consider TFA API and keep in TFA without the impact of extra libraries and unnecessary plugins being loaded.

    I don't believe there is any way TFAHotp and TFATotp could break other modules, however if there is I will preemptively mention 2.x already requires a refactor, now would be the perfect time before we lock in the API with a beta release signaling to other maintainers they should start updating their modules.

    splitting the plugins into their own modules only adds complexity to the release process as ga_login and tfa were installed together in 99% of the cases. And a few users that only installed tfa were confused why nothing seemed to work.

    I can imagine a large majority of sites were using them, and probably will be for the long term. I will addtionaly admit it requires a few more dev cycles to maintain a second release package.

    I'm hoping to encourage us to use methods that allow flexibility and keep the security scope of the core TFA module as minimal as possible.

    Ideally we never have any vulnerabilities, however when we do it is nice if we can lock them into their own small areas where only those who are impacted need to update and each modules advisory can be more narrowly focused (an issue I faced writing the recent notes regarding 8.x-1.3).

    Pulling out the plugins inherently makes TFA 'less opinionated' in its design which is a bonus for all sites. I believe it would also encourage thinking in the abstract, rather than instinctively assuming the plugins included are always how the API will be used, increasing the overall flexibility and security of the API.

    Even with removing the plugin from TFA itself, there are a few ways we could still leverage composer to download the plugin for the average deployment while still providing a method for more advanced site owners to prevent the plugin from being downloaded.

  • πŸ‡¦πŸ‡ΊAustralia acbramley

    Is πŸ› Resolve SA-CONTRIB-2024-003 in 2.x branch Postponed also a must have?

  • πŸ‡ΊπŸ‡ΈUnited States cmlara

    Updated with some entries that should have been previously added.

    Yes πŸ› Resolve SA-CONTRIB-2024-003 in 2.x branch Postponed is a must-have.

  • πŸ‡΅πŸ‡ΉPortugal jcnventura

    I think ideally, the plan should be to release a 2.0.0-alpha3 that fixes all the postponed security issues, so that the 2.x branch is at least as secure as the 1.x branch.

    The only module that I know has a public release and is based on TFA is https://www.drupal.org/project/tfa_email_otp β†’ , and that one is still using 1.x, so I think we should not rush to beta as long as all the work on re-factoring the plugins to be configuration entities and using PHP attributes are still pending. Ideally, we should split the "Should have" list into "beta release-blocker" and "stable release-blocker". And move anything that changes the API into the "beta release-blocker" section.

Production build 0.71.5 2024