Laragear / TwoFactor

Two-Factor Authentication for all your users out-of-the-box.
https://github.com/sponsors/DarkGhostHunter
MIT License
259 stars 20 forks source link

Call Auth2FA::attempt gives warning 'Non static method 'attempt' should not be called statically.' #34

Closed michelterstege81 closed 1 year ago

michelterstege81 commented 1 year ago

PHP & Platform

8.1.6 Windows

Database

mysqlnd 8.1.6

Laravel version

9.43.0

Have you done this?

Expectation

No warnings should be issued

Description

When using this line:

$attempt = Auth2FA::attempt($request->only('email', 'password'), $request->filled('remember'));

like in the documentation VSCode intelephense marks it as: "Non static method 'attempt' should not be called statically."

Reproduction

/**
     * Handle an authentication attempt.
     *
     * @param  \Illuminate\Http\Request  $request
     * @return \Illuminate\Http\Response
     */
    public function authenticate(Request $request)
    {
        // If the user is trying for the first time, ensure both email and the password are
        // required to log in. If it's not, then he would issue its 2FA code. This ensures
        // the credentials are not required again when is just issuing his 2FA code alone.
        if ($request->isNotFilled('2fa_code')) {
            $request->validate([
                'email' => 'required|email',
                'password' => 'required|string'
            ]);
        }

        $attempt = Auth2FA::guard('admin')->attempt($request->only('email', 'password'), $request->filled('remember'));

        if ($attempt) {
            return redirect()->intended('/');
        }

        return back()->withErrors([
            'email' => 'The provided credentials do not match our records.',
        ])->onlyInput('email');
    }

Stack trace & logs

No response

DarkGhostHunter commented 1 year ago

That's clearly a bug on VS Code intelephense, because guard('admin') returns an intance of TwoFactorLoginHelper which has that dynamic method.

You can raise this issue on their repository of 500+ issues and pray it's fixed.

michelterstege81 commented 1 year ago

If I do:

Auth2FA::guard('admin')->attempt

I get the warning on ::guard

When doing Auth2FA::attempt I get the warning on attempt.

The functions indeed return an instance from where the second call can be done using ->

but it's the first call using the static :: that gives the warning.

michelterstege81 commented 1 year ago

I'm using:

use Laragear\TwoFactor\Facades\Auth2FA;

This is the actual warning:

image

michelterstege81 commented 1 year ago

image

This is how it's done in Laravel's original Auth facade. In that case it has no warnings in VSCode.

If you change to:

/**

the warning is gone

DarkGhostHunter commented 1 year ago

You're totally right. My mistake. Can you open a PR? I'm AFK right now.

michelterstege81 commented 1 year ago

I created the PR. Hope I have done it right. It's the first time I use github. I normally use Bitbucket.

Meanwhile I have a small other question. Totally unrelated:

In the database table I see that a users email is used in the 'label' column and that is used as a label in the authenticator app. If every application does this, then it would be hard to distinguish in the authenticator app from which application it is. Would it be possible to get the APP_NAME in that column? Or at least send that to the auth app when pairing?

DarkGhostHunter commented 1 year ago

Published. Thanks for the PR.

In the database table I see that a users email is used in the 'label' column and that is used as a label in the authenticator app. If every application does this, then it would be hard to distinguish in the authenticator app from which application it is. Would it be possible to get the APP_NAME in that column? Or at least send that to the auth app when pairing?

You mean these lines?

You can change it to your app name like this:

/**
 * Returns the label for TOTP URI.
 *
 * @return string
 */
protected function twoFactorLabel(): string
{
    return config('app.name') . ' - '. $this->getAttribute('email');
}

I don't remember why I set the label to be the email, but I think now the format is account:issuer, like john@mail.com:surrealdb.com. I may change it on the next major version, since it's a breaking change.

michelterstege81 commented 1 year ago

Thanks. I'll override the method.

Maybe it's an idea to make it a config option. Then it won't be a breaking change. Something like:

`/**

See PR #36