🇮🇳India @bhanu951

Updated the code to fix the issue.

bhanu951 → created an issue.

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

Re-rolled patch 2884217-9.patch → against 11.x and updated test.

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

bhanu951 → changed the visibility of the branch 11.x to hidden.

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

@sabrina.liman , patches are no longer preferred, please use existing MR to contribute or create a different MR if you have an alternative solution than the existing one.

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

Rerolled patch from #10 against 11.x branch, added deprecation error message to constructor .

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

Re-rolled patch from #57 against 11.x branch.

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

I have fixed the phpcs and phpstan issues.

Tests still need to be added.

@heddn do you want me add full tests similar to the below MR or do you have any suggestions ?

https://git.drupalcode.org/project/drupal/-/commit/d4abe119f885d41104b98...

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

Re-Rolled patch from #149 against 11.x to bump the issue.

> What's the command to import recovery code ?

drush mim tfa_migration_recovery_code

Is the command to migrate recovery codes.

@jibus, I have made few more changes, now the migration of recovery codes would work properly.

Hi @jibus, I have pushed the changes, can you test it and let me know if it works ?

As I don't have your Drupal 7 private key I am unable to test decryption.

If the code doesn't work please share me your Drupal 7 private key and values of the backup codes from the site over slack.
So, I can test the decryption.

Can you share actual schema data with all the fields data.

Are they grouped by uid or something?

Hi @jibus can you share me sample data from Drupal 7 tfa_recovery_code table?

I don't have sample data or working Drupal 7 site to test the migration for recovery codes.

I wrote a draft version, need to test it.
You can share me 5-6 (or as many as you wish) sample data to process,

@jibus : I made code changes to fix the warning.

Can you apply the patch - 3499387-MR12 and check if the issue is resolved.

I didn't get what you mean. Can you share screenshot?

Recovery codes are migrated and are present in users_data table.

If data is migrated properly you will get page to provide TFA code instead of message to setup TFA.

Coming to accepted codes I think we can not utlize migrated data as in Drupal 7 Accepted codes are stored using this logic

 $hash = hash('sha1', drupal_get_hash_salt() . $code);

As the Accepted code is stored in the form of SHA1 + salt hash . It is highly impossible to reverse the hash and get the original OTP . So It is not possible to verify against the codes used in D7 in D10. I am not sure how to proceed here.

Related Discussions
https://www.drupal.org/project/tfa/issues/3328128#comment-15431448 💬 Migration from Drupal 7 to Drupal 9 Active

I believe we should add empty variable $decoded_data check at this line.

Can you add that check and confirm if it fixes the notice.

https://git.drupalcode.org/project/tfa_migration/-/blob/1.x/src/Plugin/m...

Hi @jibus, Thanks for reporting the issue.

I observed that notice only for one of my records that are being migrated which doesn't have sms plugin value and for other records it migrated with out notice.

Are you seeing it for multiple records ?

Can you ping me on Drupal slack @bhanu951, we can discuss it over there.

If I comment out code

       throw new NotFoundHttpException(sprintf('No batch with ID %s exists.', $request_id));

on line 47 in web/core/includes/batch.inc . I am getting below error.

Warning: Trying to access array offset on false in _batch_page() (line 71 of core/includes/batch.inc).
_batch_page() (Line: 664)
install_run_task() (Line: 572)
install_run_tasks() (Line: 121)
install_drupal() (Line: 53)

Warning: foreach() argument must be of type array|object, null given in _batch_page() (line 71 of core/includes/batch.inc).
_batch_page() (Line: 664)
install_run_task() (Line: 572)
install_run_tasks() (Line: 121)
install_drupal() (Line: 53)

Warning: Trying to access array offset on false in _batch_current_set() (line 407 of core/includes/batch.inc).
_batch_current_set() (Line: 86)
_batch_page() (Line: 664)
install_run_task() (Line: 572)
install_run_tasks() (Line: 121)
install_drupal() (Line: 53)

Warning: Trying to access array offset on null in _batch_page() (line 87 of core/includes/batch.inc).
_batch_page() (Line: 664)
install_run_task() (Line: 572)
install_run_tasks() (Line: 121)
install_drupal() (Line: 53)

Warning: Undefined array key "current_set" in _batch_current_set() (line 407 of core/includes/batch.inc).
_batch_current_set() (Line: 152)
_batch_progress_page() (Line: 88)
_batch_page() (Line: 664)
install_run_task() (Line: 572)
install_run_tasks() (Line: 121)
install_drupal() (Line: 53)

Warning: Trying to access array offset on null in _batch_progress_page() (line 159 of core/includes/batch.inc).
_batch_progress_page() (Line: 88)
_batch_page() (Line: 664)
install_run_task() (Line: 572)
install_run_tasks() (Line: 121)
install_drupal() (Line: 53)

Warning: Undefined array key "url" in _batch_progress_page() (line 199 of core/includes/batch.inc).
_batch_progress_page() (Line: 88)
_batch_page() (Line: 664)
install_run_task() (Line: 572)
install_run_tasks() (Line: 121)
install_drupal() (Line: 53)

I too can reproduce this issue on clean vanilla Drupal version 11.1.1 .

The issue happens only installing site via UI but not reproducible via Drush.

After the error occurred if the refresh the url the site works properly after re-login.

Possible regression from 🐛 When Batch ID doesn't exist, Drupal should emit a 404 Needs work

bhanu951 → changed the visibility of the branch 11.x to hidden.

bhanu951 → changed the visibility of the branch 3054641-local-tasks-should to hidden.

Made changes against head and fixed phpstan issues and tests.

bhanu951 → changed the visibility of the branch 2809177-introduce-entity-permission-11.x to hidden.

bhanu951 → changed the visibility of the branch 2607920-breadcrumb-render-cache-fail-test to hidden.

bhanu951 → changed the visibility of the branch 11.x to hidden.

Closing it as works as designed.

@anybody please reopen the issue if thats not the case.

bhanu951 → created an issue.

bhanu951 → created an issue.

@benjifisher : I made those changes, please re-review it. Thanks.

Thanks @benjifisher for the review, addressed your feedback.

I think it is ready for re-review.

Tested the fix on new dev instance of @fjgarlin , issue seems fixed.

Thanks @fjgarlin for the quick fix.

Setting issue to RTBC.

bhanu951 → created an issue.

@jibus : Thanks for reporting the issue.

I have pushed the fix can you please retest it and set it to RTBC.

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

Review comments addressed.

I tried to test the script on macOs and the script doesn't install the Drupal CMS as expected. Below are the steps I followed as per IS.

1. git clone git@git.drupal.org:project/drupal_cms.git drupal-cms
2. cd drupal-cms
3. cp -r project_template cms-test
4. cd cms-test
5. `vi launch-drupal-cms.sh` and comment out `COMPOSER_CREATE=${COMPOSER_CREATE:-drupal/cms --stability="RC"}` and uncommnet
`export COMPOSER_CREATE='drupal/cms --stability=dev --repository={"type":"vcs","url":"https://github.com/phenaproxima/test-ddev-cms.git"}'`
6. Run ./launch-drupal-cms.sh from cms-test directory.

Then I got below error.

cms-test $./launch-drupal-cms.sh
it usually does not make sense to `ddev config` in a subdirectory of an existing project. Is it possible you wanted to `ddev config` in parent directory /Users/bhanu951/Projects/Contrib/drupal-cms?

@nicxvan : Thanks for the review.

> My question is did we consider adding a helper ?

AFAIK, there wasn't any discussion in this regards.

> It seems like a lot to use the whole registry just to get the templates in the engine.

Agreed.

> I really think this piece should be moved to a themetemplatefinder.

Seems reasonable. Where do you suggest its location ?

How does core/lib/Drupal/Core/Theme/Themetemplatefinder.php sound ?

Ad do we need interface for this ?

@smustgrave

Phpcs failing without those nit changes hence keeping them.

@smustgrave issue is rescoped check #40, patch rerolls are not correct. MR 3248 is relevant.

bhanu951 → changed the visibility of the branch 11.x to hidden.

bhanu951 → created an issue.

Rebased against latest head and moved changes oops hook files

Rebased against head and moved changes to OOPS hooks.

Rebased to latest head, seems it ready for review.

Made changes to MR and address review comments.

Now sure how to handle adding new constructor argument.

Created new issue to add handle adding new constructor argument. 📌 Inject the file_system service in the theme registry service Active .

Should we handle adding new constructor argument in this issue or in other issue ?

bhanu951 → created an issue.

Adding MR changes as patch for backup before rebase.

Bot false positive.

bhanu951 → created an issue. See original summary → .

bhanu951 → created an issue.

Re-rolled latest patch against 11.x branch.

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

Seems issue summary is good. Update the code against latest head.

Changed Target Branch to 11.x and updated latest changes as patch for backup before target branch change.

Thanks @ longwave, @kreynen, @greggles for clarification will try to use 3.x and make the port.

I am planning to work on this port , currently The JS library https://github.com/Valve/fingerprintjs2 redirects to https://github.com/fingerprintjs/fingerprintjs which is licensed under Business Source License 1.1 https://mariadb.com/bsl11/ is it okay to use in this module or do we need to look for alternative library.

The BSL prohibits the licensed code from being used in production — without explicit approval from the licensor.

https://fossa.com/blog/business-source-license-requirements-provisions-h...

https://www.tldrlegal.com/license/business-source-license-bsl-1-1

Does anyone has any suggestions ?

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

Fixed Tests and updated depreciations.

Seems its ready for review.

Uploading MR changes as backup patch before changing target branch.

Uploaded MR Changes as patch before changing target.

Updated the MR to use message

"You have used a one-time login link. You can set your new password now."