antonioribeiro / google2fa-qrcode

QRCode for Google2FA
MIT License
106 stars 25 forks source link

Chillerlan fixes #11

Open X-Coder264 opened 3 years ago

X-Coder264 commented 3 years ago

This PR contains a couple of fixes:

  1. The Chillerlan adapter implementation currently produces a broken SVG QR code as the underlying Chillerlan implementation already does the base64 encode by default and the Chillerlan adapter in this package base64 encodes an already base64 encoded output. There is a test which asserts the expected output but it's failing (and now it passes again with this fix). It looks like the Travis CI pipeline is not running at all, otherwise this would've been caught a lot sooner as it never worker properly.
  2. Without this fix I thought I could override the imageBase64 option of the underlying Chillerlan implementation so that it doesn't do the base64 encode as this package already does it, but the \PragmaRX\Google2FAQRCode\QRCode\Chillerlan::setOptions method is for some reason protected which makes it unusable. I've chaged the visibility to public.

In the meantime I've worked around this in my Laravel project by using an anonymous class which overrides the $options array.

$this->app->bind(Google2FA::class, function (): Google2FA {
    $chillerlan = new class() extends Chillerlan {
        protected $options = ['imageBase64' => false];
    };

    return new Google2FA($chillerlan);
});
  1. I've fixed some docblock issues, the Chillerlan class_exists check which also checks for the BaconQrCode\Renderer\ImageRenderer class existence which doesn't make sense and I've updated the README.md (the PHP requirement is fixed).
codemasher commented 3 years ago

Hi, just a heads-up: base64 output for SVG exists only from v3.4 onwards (v4.x backport), so the best bet in this case is to always disable base64 and let Google2FA handle it - like the workaround above does.