Issuer missing or double

[Weemple]

PHP & Platform

8.2.4

Database

No response

Laravel version

10.13.5

Have you done this?

• I have checked my logs and I'm sure is a bug in this package.
• I can reproduce this bug in isolation (vanilla Laravel install)
• I can suggest a workaround as a Pull Request

Expectation

Expected result
otpauth://totp/Laravel:[email protected]?secret=THISISMYSECRETPLEASEDONOTSHAREIT&issuer=Laravel&label=taylor%40laravel.com&algorithm=SHA1&digits=6&period=30

Description

Result without setting the issuer config variable
otpauth://totp/%3A:[email protected]?label=%3Ataylor%40laravel.com&secret=THISISMYSECRETPLEASEDONOTSHAREIT&algorithm=SHA1&digits=6

Result with setting the issuer config variable
otpauth://totp/Name%3AName:[email protected]?issuer=Name&label=Name%3Ataylor%40laravel.com&secret=THISISMYSECRETPLEASEDONOTSHAREIT&algorithm=SHA1&digits=6

Reproduction

$secret = $user->createTwoFactorAuth();

Stack trace & logs

No response

[DarkGhostHunter]

What's the problem?

By default, the OTP Auth URL uses the application name as issuer when not present (null), Laravel by default.

In any case, I'm pushing a patch for #51 to throw something if the issuer is empty.

[Weemple]

The issue is that currently don’t work. It use %3A instead of “Laravel”. And if I set a name, then the name is used two times

[DarkGhostHunter]

The issue is that currently don’t work. It use %3A instead of “Laravel”. And if I set a name, then the name is used two times

Can you dump the following?

• config('app.name')
• config('two-factor.issuer')

Again, the patch version is live and you should get an error if you set an empty issuer. When it's empty, it uses the application name. It's replicated at the issuer query because the first is to set the [app + account name], like Laravel:[email protected].

I don't know why you're not returning the issuer, but there must be a problem on your two-factor.php config file.

[Weemple]

Here two detailed example of the issue.

.env
APP_NAME=AppName

two-factor.php
'issuer' => env('OTP_TOTP_ISSUER'),

\Log::info(config('app.name'));
AppName

\Log::info(config('two-factor.issuer'));
AppName

Missing name issue

---

.env
APP_NAME=AppName

two-factor.php
'issuer' => env('APP_NAME'),

\Log::info(config('app.name'));
AppName

\Log::info(config('two-factor.issuer'));
AppName

Double name issue

[DarkGhostHunter]

Are you on v1.2.2?

[Weemple]

v1.2.1

[DarkGhostHunter]

v1.2.1

Try v1.2.2

[helios-sr]

Hi Italo,

Thanks for implementing this library. Really helped in getting 2FA sorted in my project.

I've looked at your library and detected the lines that cause this non or double issuer. I've managed to mitigate the bug on my project by modifying the output of the toUri() function. I've tried to do a PR but I'm not used to gitHub and run out of time, so I'll post a possible solution here in hopes you'll consider it.

The first suggestion I have is to add the app name fallback in the 'label' so it doesn't hit the DB as ':foo@bar' when no label nor issuer is found on the config:

TwoFactorAuthentication.php line 111

Original:
$issuer = config('two-factor.label') ?? config('two-factor.issuer');

New:
$issuer = config('two-factor.label') ?? config('two-factor.issuer') ?? config('app.name');

The second and most important is to fix the toUri function of the SerializesSharedSecret trait
This is the original code:

Because the label already contains the issuer when present '<the_issuer>:<account_name>', line 39 adds it twice:
'<the_issuer>%3A<the_issuer>:<account_name>?...'
Or if you don't have issuer then you get '%3A:<account_name>?...' which shows as double ::

This is what I propose provided we add that app name fallback in the label:

        [$issuer, $account_name] = explode(':', $this->attributes['label'] ?? '');
        if (!$issuer) throw new InvalidArgumentException('The TOTP issuer cannot be empty.');
        if (!$account_name) throw new InvalidArgumentException('The TOTP account name cannot be empty.');
        $query = http_build_query([
            'issuer'    => $issuer,
            'label'     => $this->attributes['label'],
            'secret'    => $this->shared_secret,
            'algorithm' => strtoupper($this->attributes['algorithm']),
            'digits'     => $this->attributes['digits'],
        ], '', '&', PHP_QUERY_RFC3986);

        return 'otpauth://totp/'.rawurlencode($issuer).'%3A'.$account_name."?$query";

Hope this helps in keeping your library awesome.

This is the mitigation I've done in my project, in case someone needs it:

$user = Auth::user();
$secret = $user->createTwoFactorAuth();
$uri_params = explode('?', $secret->toUri())[1];
$uri = 'otpauth://totp/' . rawurlencode(config('two-factor.issuer')) . ':' . $user->email . '?' . $uri_params;

Regards and saludos,

[juanparati]

I can also experience that issues is duplicate due that label already has the issuer, however I also think that we should take care modifying the behavior because different authentication apps can represent the label and issuer in different ways.