? ? ? Pending

User tests: Successful: Unsuccessful:

avatar PhilETaylor
PhilETaylor
21 Jun 2020

Pull Request for Issue #29696 .

Summary of Changes

A very light touch change to reuse existing code to display a warning and remove the ability to add authenticators if the PHP extension GMP is not loaded.

Testing Instructions

Install Joomla 4 on a server (or docker container in my case) without GMP PHP Extension enabled (On a cPanel server you can toggle this, and its off by default in Easy Apache 4)

Ensure you access Joomla over a secure & valid SSL connection (learn ngrok if you need this, quick and easy)

Before PR

When adding an authenticator you get an internal server error

After PR

You get a error and no button allowing you to add a new authenticator (but you can still see the list of existing authenticators - if any - rename them, and delete them.)

Screenshot 2020-06-21 at 12 24 50

Who else to ping for visibility ;-)

// @nikosdion

avatar PhilETaylor PhilETaylor - open - 21 Jun 2020
avatar PhilETaylor PhilETaylor - change - 21 Jun 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 21 Jun 2020
Category Administration Language & Strings Layout
avatar PhilETaylor PhilETaylor - change - 21 Jun 2020
The description was changed
avatar PhilETaylor PhilETaylor - edited - 21 Jun 2020
avatar PhilETaylor PhilETaylor - change - 21 Jun 2020
The description was changed
avatar PhilETaylor PhilETaylor - edited - 21 Jun 2020
avatar richard67
richard67 - comment - 21 Jun 2020

@PhilETaylor Maybe some general explanation about or PR descriptions: "Expected result" always shall describe what one expects, i.e. how it is with the PR applied, "Actual result" shall describe what one currently observes, i.e. how it is without the PR applied. Your description above currently mixes that up.

avatar PhilETaylor
PhilETaylor - comment - 21 Jun 2020

ah well... "Expected" and "Actual" are very unhelpful terms anyway. "Before" and "After" are more suited to PRs IMHO. Edited :)

avatar PhilETaylor PhilETaylor - change - 21 Jun 2020
The description was changed
avatar PhilETaylor PhilETaylor - edited - 21 Jun 2020
avatar PhilETaylor PhilETaylor - change - 21 Jun 2020
The description was changed
avatar PhilETaylor PhilETaylor - edited - 21 Jun 2020
avatar richard67
richard67 - comment - 21 Jun 2020

ah well... "Expected" and "Actual" are very unhelpful terms anyway. "Before" and "After" are more suited to PRs IMHO.

Agree .. it's often subject of confusion.

avatar nikosdion
nikosdion - comment - 21 Jun 2020

I'd actually change the detection code to use function_exists() with some GMP function names for two reasons.

First, I've seen many hosts where get_loaded_extensions itself is disabled, making that kind of code fail immediately with a fatal error. I know, these hosts are crap but they are used by people using Joomla...

Second, it's possible that the extension is enabled but the host disabled the functions. I have not seen that with GMP but I can't rule out outright stupidity when it comes to cheap hosting.

avatar PhilETaylor
PhilETaylor - comment - 21 Jun 2020

Thanks - I'll get that done.

avatar PhilETaylor PhilETaylor - change - 21 Jun 2020
Labels Added: ? ?
avatar PhilETaylor
PhilETaylor - comment - 21 Jun 2020

Ive gone for

if (false === function_exists('gmp_intval'))

because gmp_intval was the first function called by the code which caused the Internal Server Error.

(Subject to passing code style haha)

avatar richard67
richard67 - comment - 21 Jun 2020

@PhilETaylor Normally we do it the other way round, i.e. if (function_exists('gmp_intval') === false) and not if (false === function_exists('gmp_intval')). But I'm ok with it if that passes PHPCS.

avatar PhilETaylor
PhilETaylor - comment - 21 Jun 2020

lol I knew someone was either going to say that or ask why I did not just use a

if (!function_exists('gmp_intval'))
avatar PhilETaylor
PhilETaylor - comment - 21 Jun 2020

There are 333 examples of doing it my way in Joomla 4 (admittedly only 5 are in Joomla code outside of vendor ;))

There is a good reason to do it - although I cannot find some article to link to, here is the gist:

// using this:
if (false == $var)

// its impossible to this by mistake
if (false = $var)
// because a PHP error would happen

// whereas if you flipped and did it the other way
if ($var == false)

//then its VERY easy to make a typo and get:
if ($var = false)
// which would ALWAYS be true and PHP would not error to alert you.... 

found a link (12 years ago haha) https://www.php.net/manual/en/control-structures.if.php#81698

avatar richard67
richard67 - comment - 21 Jun 2020

@PhilETaylor PHPCS is ok with it, so I am, too. Unfortunately I can't help with testing, because my local testing environment uses a self signed SSL certificate, and I haven't set up any webauthentication for me anywhere. And testing space on my domain which has a valid SSL cert is protected with password, so I am not sure if it would work there.

avatar PhilETaylor
PhilETaylor - comment - 21 Jun 2020

@richard67 You need to learn https://ngrok.com then :) Install that, and then at your command line type

ngrok http 80

Where 80 is the port number of your local Joomla install

Then you will be given a SSL url like: https://RANDOM.ngrok.io

When you go to that url it proxies to your local install and gives you a valid SSL connection :)

avatar richard67
richard67 - comment - 21 Jun 2020

@PhilETaylor You can't teach an old dog new tricks ;-)

(have to go back to my vi editor)

avatar PhilETaylor
PhilETaylor - comment - 21 Jun 2020

Not much to learn :)

brew cask install ngrok
ngrok http 80 

Done. :-)

avatar PhilETaylor
PhilETaylor - comment - 21 Jun 2020

All checks have passed ??

avatar richard67
richard67 - comment - 21 Jun 2020

All checks have passed ??

There must be something wrong ... ;-)

avatar Quy Quy - test_item - 30 Jun 2020 - Tested successfully
avatar Quy
Quy - comment - 30 Jun 2020

I have tested this item successfully on dba7e75

Thank you for the ngrok info. So simple!!!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29731.

avatar nikosdion
nikosdion - comment - 1 Jul 2020

Self hosted alternative to Ngrok written in PHP: https://pociot.dev/28-introducing-expose-an-easy-to-use-tunneling-service-implemented-in-pure-php#introducing-expose-an-easy-to-use-tunneling-service-implemented-in-pure-php

Apparently it took the PHP world by storm over the last couple of weeks.

avatar PhilETaylor
PhilETaylor - comment - 8 Jul 2020

Just needs one more human test (if you have time please) to get to RTC.

avatar bonzani bonzani - test_item - 19 Jul 2020 - Tested successfully
avatar bonzani
bonzani - comment - 19 Jul 2020

I have tested this item successfully on dba7e75


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29731.

avatar bonzani
bonzani - comment - 19 Jul 2020

I have tested this item successfully on dba7e75


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29731.

avatar richard67 richard67 - change - 19 Jul 2020
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 19 Jul 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29731.

avatar richard67 richard67 - change - 20 Jul 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-07-20 19:35:19
Closed_By richard67
Labels Added: ?
avatar richard67 richard67 - close - 20 Jul 2020
avatar richard67 richard67 - merge - 20 Jul 2020
avatar richard67
richard67 - comment - 20 Jul 2020

Thanks!

Add a Comment

Login with GitHub to post a comment