laravel / passport

Laravel Passport provides OAuth2 server support to Laravel.
https://laravel.com/docs/passport
MIT License
3.28k stars 777 forks source link

Guard possible not being correctly loaded from config file #1616

Closed rickgoemans closed 1 year ago

rickgoemans commented 1 year ago

Description:

While using a different guard than the default version in config/auth.php, the request cannot resolve the user during authorization.

Steps To Reproduce:

We are using multiple guards and try to achieve a Authorization Code Grant with PKCE.

Here's out auth config (simplified):

<?php

return [
  'defaults' => [
    'guard'     => 'web',
    'passwords' => 'users',
  ],

  'guards' => [
    'web' => [
      'driver'   => 'session',
      'provider' => 'users',
    ],

    'web_new' => [
      'driver'   => 'session',
      'provider' => 'outsmart_users',
    ],

    'api' => [
      'driver'   => 'passport',
      'provider' => 'outsmart_users',
    ],
  ],
  'providers' => [
    'users' => [
      'driver' => 'eloquent',
      'model'  => Customer::class,
    ],
    'outsmart_users' => [
      'driver' => 'outsmart_users',
      'model'  => User::class,
    ],
  ],
];

We have web as default but we use web_new for the Laravel Passport. So we defined the guard in config/passport.php to be web_new, but we keep getting an error on the Laravel\Passport\Http\Controllers\AuthorizationController on line 102 which states:

return $this->approveRequest($authRequest, $user);

Which is caused by line 97:

$user = $request->user();

Because that ends up null.

Therefore I've tried to add some the following logging:

info('Current driver: ', [Auth::getDefaultDriver()]); // Current driver: ['web']
info('Current user provider: ', [Auth::getDefaultUserProvider()]); // Current user provider: [null]

info('(G) User: ', [$this->guard->user()?->toArray()]); // (G) User: [{id: 1, ...}] 
info('(R) User: ', [$request->user()?->toArray()]); // (R) user: [null]

As you can see the $this->guard->user() resolves correctly but the $request->user() does not. Also the current driver states web instead of web_new.

This is our (simplified) controller processing the login request (after being redirected to the view page with a form due to an unauthenticated exception) does this:

<?php

class AuthController extends Controller {
  public function login(Request $request): RedirectResponse
  {
    $guardName = 'web_new';

    if (!Auth::guard($guardName)->attempt($request->only('email', 'password'))) {
      return back()
        ->withInput()
        ->withErrors([
          '_general' => 'Invalid credentials',
        ]);
    }

    return redirect()
      ->intended();
  }
}

Am I misconfiguring something or is there something broken related to multiple guards?

driesvints commented 1 year ago

Heya, thanks for reporting.

We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as one separate commit on the main/master branch and share the repository here? Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

Please do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue.

Thanks!

rickgoemans commented 1 year ago

The repository can be found here:

https://github.com/rickgoemans/bug-report

P.S. This was set up and tested using: PHP: 8.2.0 Laravel: 9.47.0 Passport: 11.5.0 Database: MySQL 8.0.31

Steps to reproduce:

  1. Clone the project, and copy .env.example to .env (adjust to your needs).
  2. composer install
  3. Up the docker environment (using Laravel Sail) by running sail up -d
  4. Migrate and seed the database by running sail artisan migrate:fresh --seed
  5. (Optional) check if auth is working by going to http:///localhost/test, it should redirect to a basic login form (login with admin@admin.com and admin.
  6. Go to the Laravel Passport authorize url (http://localhost/oauth/authorize with a working configuration, example: http://localhost/oauth/authorize?client_id=1&redirect_uri=http://localhost:3000/auth/callback&response_type=code&scope=*&state=drq9a1mdykonu2uplhnjgit6wqelwocx0jfq3uq5&code_challenge=MsUnJvDKMIOox2WOvOjFCeFLvHb4DwpQTqgicT7szoE&code_challenge_method=S256)
  7. (Unauthenticated) If you are not authenticated you should be redirected to http://localhost/auth/login
  8. (Authenticated) You receive the error shown below: Screenshot 2023-01-12 at 16 03 42
driesvints commented 1 year ago

@rickgoemans there's many unrelated changes in that commit to the issue. Can you please create one separate commit for the passport install scaffold and one separate commit with the bare minimum of changes to reproduce the issue? Meaning no docblocks or styling changes etc

rickgoemans commented 1 year ago

@driesvints I've made a new repo: https://github.com/rickgoemans/laravel-passport-bug-report

The commit for the Laravel Passport scaffold: https://github.com/rickgoemans/laravel-passport-bug-report/commit/b8fe366b358702359cffe96d1cc3641b1c35ce59

The commit for the minimum changes to reproduce: https://github.com/rickgoemans/laravel-passport-bug-report/commit/af1cbdd7cca8b75a194229b8b3692531f35482e6

Setup:

  1. Clone project
  2. Copy .env.example to .env (and adjust to your needs)
  3. Install composer dependencies by running composer install
  4. Start docker (I've used Laravel Sail and ran sail up -d
  5. Setup Laravel Passport by running sail artisan migrate fresh --seed (might have to run sail artisan passport:install to setup encryption keys)
  6. (Optional) check if auth is working by going to http:///localhost/test, it should redirect to a basic login form (login with test@example.com and password). Once logged, it, it show a basic page with a logout button.
  7. Go to the Laravel Passport authorize url (http://localhost/oauth/authorize with a working configuration, example: http://localhost/oauth/authorize?client_id=1&redirect_uri=http://localhost:3000/auth/callback&response_type=code&scope=*&state=drq9a1mdykonu2uplhnjgit6wqelwocx0jfq3uq5&code_challenge=MsUnJvDKMIOox2WOvOjFCeFLvHb4DwpQTqgicT7szoE&code_challenge_method=S256)
  8. (Unauthenticated) If you are not authenticated you should be redirected to http://localhost/auth/login
  9. (Authenticated) You receive the error shown before.

Additional information

I've tried by defining the provider to outsmart_users in the PassportSeeder but the issue still persist.

driesvints commented 1 year ago

Thanks for the thorough info @rickgoemans.

I'm confused here. Your example nowhere shows the Authorization Code Grant with PKCE code and you're using views etc. Pasport is meant to be used with API's, not views (unless you're using PAT but we instead recommend Sanctum for that). This doesn't seem like a real-world example to me.

Can you please try a support channel instead?

rickgoemans commented 1 year ago

The reason I supplied a simple view for the login is that you can login and authorize the request.

We are using a separate Nuxt SPA application with the @nuxtjs/auth-next module for authorization. If you want me to setup a basic repo for that, I will, but I suppose that's not the focus here.

In the real project we will redirect from the frontend (http://localhost:3000 on local development) to Passport's authorize url (http://localhost/oauth/authorize?client_id=1&redirect_uri=http://localhost:3000/auth/callback&response_type=code&scope=*&state=drq9a1mdykonu2uplhnjgit6wqelwocx0jfq3uq5&code_challenge=MsUnJvDKMIOox2WOvOjFCeFLvHb4DwpQTqgicT7szoE&code_challenge_method=S256).

I really hope you could look into it because I don't think I can give any more information related to the (Laravel) backend. Plus the exception that is thrown is regardless of what frontend framework you throw at it.

hafezdivandari commented 1 year ago

As you can see the $this->guard->user() resolves correctly but the $request->user() does not. Also the current driver states web instead of web_new.

@rickgoemans what about $request->user(config('passport.guard')) also resolves correctly? check this PR #1613