Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bcmath_compat polyfill for servers without BCmath / GMP support #38488

Merged
merged 2 commits into from
Aug 17, 2022
Merged

Add bcmath_compat polyfill for servers without BCmath / GMP support #38488

merged 2 commits into from
Aug 17, 2022

Conversation

nikosdion
Copy link
Contributor

Pull Request for Issue #38485.

Summary of Changes

Added the BCmath_compat polyfill from the well–known and well–trusted phpSecLib. This library adds a userland (pure PHP) implementation of the BCmath functions used by the third party dependencies of the WebAuthn code for BOTH logging in AND Multi-factor Authentication. If you try to use WebAuthn on a server which has neither the BCmath nor the GMP PHP extensions enabled the polyfill will kick in and let WebAuthn work properly.

Testing Instructions

  • Apply this PR on a Joomla 4.2-dev working copy.
  • Run composer install to update the Composer dependencies.
  • Run the resulting Joomla 4.2 on a server which has neither BCmath nor GMP installed.
  • Set up MFA with WebAuthn for your user account.
  • Log out and log back in.
  • Proceed with the WebAuthn authentication.

Actual result BEFORE applying this Pull Request

You receive the error “Requires GMP or bcmath extension”.

Expected result AFTER applying this Pull Request

Everything works fine.

Documentation Changes Required

In the requirements section for Joomla 4.2 we should add something along the lines of:

  • We recommend enabling one of the PHP extensions BCmath or GMP (they are not required but do increase the performance of the WebAuthn features).

@nikosdion
Copy link
Contributor Author

There's one build failure in Drone but it seems it's not coming from this PR.

@richard67
Copy link
Member

There's one build failure in Drone but it seems it's not coming from this PR.

I've restarted it, and this time it worked. Sometimes we have that problem.

@HLeithner
Copy link
Member

Works for me without bcmath and gmp, funny note I also have to remove libsodium.

@richard67 richard67 added the Maintainers Checked Used if the PR is conceptional useful label Aug 17, 2022
@roland-d roland-d merged commit 337b389 into joomla:4.2-dev Aug 17, 2022
@roland-d
Copy link
Contributor

Thanks everybody

@roland-d roland-d added this to the Joomla 4.2.1 milestone Aug 17, 2022
@nikosdion nikosdion deleted the fix/38485-mfa-bcmath branch May 10, 2023 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Composer Dependency Changed Maintainers Checked Used if the PR is conceptional useful
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants