minagerges / MODX-GoogleAuthenticatorX

Add 2-factor authentication to MODX manager login.
GNU General Public License v3.0
13 stars 11 forks source link

MODX 3 Compatibility #24

Open deJaya opened 1 year ago

deJaya commented 1 year ago

I've been testing this extra on MODX 3.0.3

It seems to install OK and works to a point.

However, as far as I can see the QR code is either not generated or not displayed for the user and there is therefore no way to set up the authenticator app.

I'd love to see this extra working on MODX3.

minagerges commented 1 year ago

The QR code is generated via google API, please check your browser console logs. This sounds like an environmental issue.

deJaya commented 1 year ago

Hey @minagerges ..

Here's the failed request on the Edit User Profile page:

image

And the corresponding 500 error from the error_log in assets/components/GoogleAuthenticatorX/connectors:

[09-Mar-2023 16:59:27 UTC] PHP Fatal error:  Uncaught Error: Class "modConnectorRequest" not found in /home/xxxxxx/modx3/core/src/Revolution/modX.php:1518
Stack trace:
#0 /home/xxxxxx/modx3/assets/components/GoogleAuthenticatorX/connectors/connector.php(29): MODX\Revolution\modX->getRequest()
#1 {main}
  thrown in /home/xxxxxx/modx3/core/src/Revolution/modX.php on line 1518

Out of my depth here so apologies if not helpful!

minagerges commented 1 year ago

Yea, expect many extras to get broken because of modx 3 lack of backward compatibility!

What happens if you comment out the below line in "assets/components/GoogleAuthenticatorX/connectors/connector.php"?

//$modx->getRequest();

deJaya commented 1 year ago

Hi again ..

Another 500 - this time:

[09-Mar-2023 17:47:30 UTC] PHP Fatal error:  Uncaught Error: Call to a member function handlerequest() on null in /home/xxxxxx/modx3/assets/components/GoogleAuthenticatorX/connectors/connector.php:31
Stack trace:
#0 {main}
  thrown in /home/xxxxxx/modx3/assets/components/GoogleAuthenticatorX/connectors/connector.php on line 31
minagerges commented 1 year ago

Thanks for your feedback. Not sure if the current ModX documentation is up to date. Couldn't locate a straight forward guide on how this should be. Thus, it requires from me some more reading, and installing version 3 to test. also ensuring that any changes won't break version 2 compatibility.

Leaving this issue opened till I get time.

Pull requests with minimal changes (ONLY the compatibility changes) are welcome. Proof of backward compatibility and full testing is required as well, because currently this GoogleAuthenticatorX is very stable, and I don't want to break it.

minagerges commented 1 year ago

ToDo: Apply connector.php style from Modx Dev docs, then test backward compatibility.

JoshuaLuckers commented 1 year ago

@minagerges you could also release a MODX 3.x specific release, but I'm happy to help if needed!

johnnydeep27 commented 1 year ago

Hi guys, I managed to make it work (just a band aid solution for modx 3 users like me). As of now in my local machine will try later on live

I just commented out these lines on connector.php

Line 24: $modx = new modx(); Line 29: $modx->getRequest();

I hope this helps

UPDATE: Working perfectly on live

Esger commented 10 months ago

@johnnydeep27 I commented out those lines and the QR-code is shown now! Thx, that works but aren't there any downsides of commenting out those lines?

johnnydeep27 commented 10 months ago

@Esger Honestly, I don't know if it has downsides but so far I encountered some issues on lexicons on emailtpl but I don't think it's connected here.

eydolan commented 7 months ago

Hi guys, I managed to make it work (just a band aid solution for modx 3 users like me). As of now in my local machine will try later on live

I just commented out these lines on connector.php

Line 24: $modx = new modx(); Line 29: $modx->getRequest();

I hope this helps

UPDATE: Working perfectly on live

@johnnydeep27 Tried this but did not seem to want to work on modx 3.0.4, any advice?

Jako commented 7 months ago

The connector code can be a lot simpler. It should work with MODX 3 then: #28

deJaya commented 5 months ago

The connector code can be a lot simpler. It should work with MODX 3 then: #28

@Jako your PR works for me - thanks!

MODX 3.0.4-pl