Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
2 of 3 tasks
michelterstege81 opened this issue Dec 15, 2022 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@michelterstege81
Copy link
Contributor

PHP & Platform

8.1.6 Windows

Database

mysqlnd 8.1.6

Laravel version

9.43.0

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

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

@michelterstege81 michelterstege81 added the bug Something isn't working label Dec 15, 2022
@DarkGhostHunter
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

I'm using:

use Laragear\TwoFactor\Facades\Auth2FA;

This is the actual warning:

image

@michelterstege81
Copy link
Contributor Author

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:

/**

  • @method static bool attempt(array $credentials = [], mixed $remember = false)
  • @method static \Laragear\TwoFactor\TwoFactorLoginHelper view(string $view)
  • @method static \Laragear\TwoFactor\TwoFactorLoginHelper message(string $message)
  • @method static \Laragear\TwoFactor\TwoFactorLoginHelper input(string $input)
  • @method static \Laragear\TwoFactor\TwoFactorLoginHelper sessionKey(string $sessionKey)
  • @method static \Laragear\TwoFactor\TwoFactorLoginHelper guard(string $guard)
  • @see \Laragear\TwoFactor\TwoFactorLoginHelper
    */
    class Auth2FA extends Facade

the warning is gone

@DarkGhostHunter
Copy link
Contributor

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

@michelterstege81
Copy link
Contributor Author

michelterstege81 commented Dec 15, 2022

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
Copy link
Contributor

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 [email protected]:surrealdb.com. I may change it on the next major version, since it's a breaking change.

@michelterstege81
Copy link
Contributor Author

michelterstege81 commented Dec 15, 2022

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:

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

See PR #36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants