Add D10 support to 8.x-1.x

I've only given it about 60 seconds of lab testing so I certainly want to spend more time to make sure I haven't missed anything, however since it is passing all tests lets tag this as NR to get more eyes to be sure nothing has been missed, especially given the previous claims that "impossible to provide support for D8 and D10 with the same code."

Note: we don't have D8 to test on D.O. so this portion requires manual review..

Done some basic testing and all OK:

• log in - using code, try to skip entering code
• set up new code
• trusted browser - with & without, add & remove
• validation attempts - skip, use up

I think a key thing here is to make sure it still works on D8 - anyone have a D8 site to test it on?

@DamienMcKenna Drupal 8? In this economy? I kid but couldn't resist.

I'm going to check this out and rebase it on the latest 8.x-1.x so that it has yesterday's security update, and then I'll give it a try on our Drupal 10.1 site because this nearly made me spit out my coffee when I saw the security advisory → .

In my initial test of this on a local copy of a Drupal 10.1 site, two factor seems to be working. Will get more people to test it and report in.

Ensuring D8 is indeed a key part of this issues concern, as is ensuring we don't miss any D10 changes either.

D8 testing is ensuring the fixes provide appropriate BC to the older releases and are more narrow in scope (we only need to test the code that has changed) while D10 is about ensuring that there are any other deprecation that were not picked up by automated tooling.

I couldn't find any major changes in the 2.x branch however specifically tagged to D10 so I'm expecting that to work well.

Should we update the line 11 in composer.json file

https://git.drupalcode.org/issue/tfa-3370841/-/blob/3370841-add-d10-supp...

From:

"drupal/encrypt": "~3.0"

to:

"drupal/encrypt": "^3.1"

As what the 2.x branch did since Encrypt module 3.0 is not D10 supported.

If we don't actually need it in our code, I don't think we should. Other restrictions in a project will mean that Encrypt will in fact be at 3.1 if the project is on D10. All this is saying is that this project needs at least Encrypt 3.0 to run (which I'll have to assume is true, I don't actually know).

keshav.k → made their first commit to this issue’s fork.

I hand tested the EntryForm on the MR as it existed at 2074c6a0 using D8.9.20 (I should have use older but realized I fired up a newer lab only after I did testing) and as we expected it still works providing the correct flood service and the flood service still returns errors when the configured flood timeouts are exceed.

Should we update the line 11 in composer.json file

If its not 100% necessary in order to allow TFA to work on D10 (while retaining support for D8) it would be out of scope for this issue.

Its helpful to keep patches clean and on target as much as possible.

Upload a patch from the MR for testing purpose.

Sorry the patch #13 is misformatted.
Here is the new patch aiming for 8.x-1.x branch.

@Mingsong: It is not necessary to upload patches when a MR workflow is being used. If you need a patch for testing you may download it to your local environment following the instructions from https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... → without uploading to the issue.

I've updated the MR to revert 0ef241d0 per notes in #12.

Merged into Dev.

Hi @Conrad,

Thanks for merging it into the dev branch, that makes it easier for us to test it without requiring a patch.

As a MR could change anytime, we can't relying on a patch that directly comes from a merge URL, such as
https://git.drupalcode.org/project/tfa/-/merge_requests/26.patch

Thank you for all of your work on this. I am excited to see a new release with this change so our site does not have to rely on the patch anymore, and the security advisory policy covers the code.

Both operators will do almost similar things. There is very little difference https://getcomposer.org/doc/articles/versions.md#tilde-version-range-
I think we can go with ^ sign.
MR Updated, Thank you.

My comment was more about the change from requiring version 3.0 to 3.1 than what operator is in use.

@Mingsong:

Since the patch updates the version constraint, you will have to jump through some extra hoops if your Drupal version depends on that updated constraint. If you want to use a patch, then you can download a copy of the patch to your repository and use that. This is slightly more secure than downloading a patch from d.o.

You can also use the dev version of the module: something like composer require drupal/tfa:1.x-dev.

Thanks @Benji for the advice.

Yes, I think we need to reconsider how to patch a Drupal module via using Composer.

Automatically closed - issue fixed for 2 weeks with no activity.