Skip to content

Use provider methods instead of model for 2FA #318

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

Closed
wants to merge 1 commit into from

Conversation

makstech
Copy link
Contributor

Description:

Current 2FA validation ignores custom authentication providers. When there's a provider defined that extends Illuminate\Auth\EloquentUserProvider with a custom query in newModelQuery method, it is ignored and instead, user search is done by calling on models, so it bypasses newModelQuery method.

Steps To Reproduce:

  1. Create a provider that extends Illuminate\Auth\EloquentUserProvider.
  2. Overwrite newModelQuery method and add to query something that would certainly not return a user, like
    protected function newModelQuery($model = null)
    {
        return parent::newModelQuery($model)
            ->where('id', '=', -1);
    }
  1. Try to log in with a gate that has the previously created provider.
  2. \Laravel\Fortify\Actions\RedirectIfTwoFactorAuthenticatable::validateCredentials will still find the user and will try to validate credentials.

@driesvints
Copy link
Member

This was attempted before by me here #189 and then reverted again. I'm not sure why we reverted that in the end.

Taylor has also stated that he thinks it's no problem that Fortify requires Eloquent: #180 (comment)

@mpyw
Copy link

mpyw commented Jun 25, 2022

@driesvints I guess the real reverted reason here: #189 (comment)
Actually this PR #318 does not have any problems, LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants