Fixed error getting two different instance using same token

[sfelix-martins]

Fix #63
Probably fix #51
It extends the Auth to driver passport after Laravel Passport authenticate the users and check whats the correct user to be logged, based on guards passed on middleware.
The user resolver on request too was changed to check the correct logged user.

[tmcnicholls]

Are there any plans to merge this?

[sfelix-martins]

@tmcnicholls Did you test it? It worked for you?

[tmcnicholls]

@sfelix-martins I have the same issue in my custom solution based on suggestions in laravel/passport#161

I'm going to try this branch of your package now and will report back.

[sfelix-martins]

@tmcnicholls I'm waiting your feedback. Thanks.

[tmcnicholls]

@sfelix-martins I tested this branch and it doesn't work as expected for me. If I am authenticated with a bearer token for a user, I am able to access endpoints which should be restricted to agents only, and vice versa.

app/Http/Kernel.php

protected $routeMiddleware = [
  ...
    'multiauth' => \SMartins\PassportMultiauth\Http\Middleware\MultiAuthenticate::class,
    'oauth.providers' => \SMartins\PassportMultiauth\Http\Middleware\AddCustomProvider::class,
  ...
];

config/auth.php

'guards' => [
    'api' => [
        'driver' => 'passport',
        'provider' => 'users',
    ],
    'agent' => [
        'driver' => 'passport',
        'provider' => 'agents',
    ],
],
'providers' => [
    'users' => [
        'driver' => 'eloquent',
        'model' => App\Models\User::class,
    ],
    'agents' => [
        'driver' => 'eloquent',
        'model' => App\Models\Agent::class,
    ],
],

routes/api.php

Route::middleware('multiauth:api')->get('/user', function (Request $request) {
    return $request->user();
});

Route::middleware('multiauth:agent')->get('/agent', function (Request $request) {
    return $request->user();
});

Route::middleware('multiauth:agent')->get('/test', function (Request $request) {
    return new JsonResponse(['data' => 'testing']);
});

models/Agent.php

<?php
declare(strict_types=1);

namespace App\Models;

use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;
use SMartins\PassportMultiauth\HasMultiAuthApiTokens;
use Spatie\Permission\Traits\HasRoles;

class Agent extends Authenticatable
{
    use HasMultiAuthApiTokens, HasRoles, Notifiable;
}

models/User.php

<?php
declare(strict_types=1);

namespace App\Models;

use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;
use SMartins\PassportMultiauth\HasMultiAuthApiTokens;
use Spatie\Permission\Traits\HasRoles;

class User extends Authenticatable implements MustVerifyEmail
{
    use HasMultiAuthApiTokens, HasRoles, Notifiable;
}

I can see in my database that access tokens are being stored with the correct provider.

I started to xdebug the changes made to MultiAuthenticate if it helps. This case is a request to a multiauth:agent endpoint with a bearer token which is for the users provider, not agents.

I should not be authenticated to access GET /agent but instead I see the response including my user details.

[sfelix-martins]

@tmcnicholls I wrote a tests to try simulate the error:

<?php

namespace SMartins\PassportMultiauth\Tests\Feature;

use Illuminate\Http\Request;
use Laravel\Passport\Client;
use Illuminate\Support\Facades\Route;
use Laravel\Passport\Passport;
use SMartins\PassportMultiauth\Http\Middleware\AddCustomProvider;
use SMartins\PassportMultiauth\Tests\Fixtures\Http\Kernel;
use SMartins\PassportMultiauth\Tests\TestCase;
use SMartins\PassportMultiauth\Tests\Fixtures\Models\User;
use SMartins\PassportMultiauth\Tests\Fixtures\Models\Company;

class MultiauthTest extends TestCase
{
    public function setUp()
    {
        parent::setUp();

        $this->loadLaravelMigrations(['--database' => 'passport']);

        $this->artisan('migrate');

        $this->artisan('key:generate');

        $this->artisan('passport:install');

        $this->withFactories(__DIR__.'/../Fixtures/factories');

        $this->setAuthConfigs();

        $this->setUpRoutes();
    }

    /**
     * Resolve application HTTP Kernel implementation.
     *
     * @param  \Illuminate\Foundation\Application  $app
     * @return void
     */
    protected function resolveApplicationHttpKernel($app)
    {
        $app->singleton('Illuminate\Contracts\Http\Kernel', Kernel::class);
    }

    /**
     * Setup auth configs.
     *
     * @return void
     */
    protected function setAuthConfigs()
    {
        // Set up default entity
        config(['auth.guards.api.driver' => 'passport']);
        config(['auth.guards.api.provider' => 'users']);
        config(['auth.providers.users.driver' => 'eloquent']);
        config(['auth.providers.users.model' => User::class]);

        // Set up company entity
        config(['auth.guards.company.driver' => 'passport']);
        config(['auth.guards.company.provider' => 'companies']);
        config(['auth.providers.companies.driver' => 'eloquent']);
        config(['auth.providers.companies.model' => Company::class]);
    }

    /**
     * Create routes to tests authentication with guards and auth middleware.
     *
     * @return void
     */
    public function setUpRoutes()
    {
        Route::group(['middleware' => AddCustomProvider::class], function () {
            Passport::routes(function ($router) {
                return $router->forAccessTokens();
            });
        });

        Route::middleware('multiauth:api')->get('/user', function (Request $request) {
            return $request->user();
        });

        Route::middleware('multiauth:company')->get('/company', function (Request $request) {
            return $request->user();
        });
    }

    /**
     * @test
     */
    public function it_will_return_401_when_try_access_route_with_company_guard_as_user()
    {
        // Two different models with same id.
        $user = factory(User::class)->create();
        $company = factory(Company::class)->create();

        $this->assertEquals($user->getKey(), $company->getKey());

        $client = Client::query()
            ->where(['password_client' => 1, 'revoked' => 0])
            ->first();

        $params = [
            'grant_type' => 'password',
            'username' => $user->email,
            'password' => 'secret',
            'client_id' => $client->id,
            'client_secret' => $client->secret,
            'provider' => 'users',
        ];

        $response = $this->json('POST', '/oauth/token', $params);

        $accessToken = json_decode($response->getContent(), true)['access_token'];

        $this->json('GET', '/company', [], ['Authorization' => 'Bearer '.$accessToken])
            ->assertStatus(401);
    }

    /**
     * @test
     */
    public function it_will_return_401_when_try_access_route_with_user_guard_as_company()
    {
        // Two different models with same id.
        $user    = factory(User::class)->create();
        $company = factory(Company::class)->create();

        $this->assertEquals($user->getKey(), $company->getKey());

        $client = Client::query()
            ->where(['password_client' => 1, 'revoked' => 0])
            ->first();

        $params = [
            'grant_type' => 'password',
            'username' => $company->email,
            'password' => 'secret',
            'client_id' => $client->id,
            'client_secret' => $client->secret,
            'provider' => 'companies',
        ];

        $response = $this->json('POST', '/oauth/token', $params);

        $accessToken = json_decode($response->getContent(), true)['access_token'];

        $this->json('GET', '/user', [], ['Authorization' => 'Bearer '.$accessToken])
            ->assertStatus(401);
    }
}

tests/Feature/Fixtures/Kernel.php

<?php

namespace SMartins\PassportMultiauth\Tests\Fixtures\Http;

use Orchestra\Testbench\Http\Kernel as HttpKernel;
use Orchestra\Testbench\Http\Middleware\RedirectIfAuthenticated;

class Kernel extends HttpKernel
{
    /**
     * The application's route middleware.
     *
     * These middleware may be assigned to groups or used individually.
     *
     * @var array
     */
    protected $routeMiddleware = [
        'multiauth' => \SMartins\PassportMultiauth\Http\Middleware\MultiAuthenticate::class,
        'oauth.providers' => \SMartins\PassportMultiauth\Http\Middleware\AddCustomProvider::class,
        'auth.basic' => \Illuminate\Auth\Middleware\AuthenticateWithBasicAuth::class,
        'bindings' => \Illuminate\Routing\Middleware\SubstituteBindings::class,
        'can' => \Illuminate\Auth\Middleware\Authorize::class,
        'guest' => RedirectIfAuthenticated::class,
        'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class,
    ];
}

tests/Feature/Fixtures/User.php

<?php

namespace SMartins\PassportMultiauth\Tests\Fixtures\Models;

use SMartins\PassportMultiauth\HasMultiAuthApiTokens;
use Illuminate\Foundation\Auth\User as Authenticatable;

class User extends Authenticatable
{
    protected $table = 'users';

    use HasMultiAuthApiTokens;

    public function getAuthIdentifierName()
    {
        return 'id';
    }
}

test/Feature/Fixtures/Company.php

<?php

namespace SMartins\PassportMultiauth\Tests\Fixtures\Models;

use SMartins\PassportMultiauth\HasMultiAuthApiTokens;
use Illuminate\Foundation\Auth\User as Authenticatable;

class Company extends Authenticatable
{
    protected $table = 'companies';

    use HasMultiAuthApiTokens;

    public function getAuthIdentifierName()
    {
        return 'id';
    }
}

But a can't simulate the error. All tests passed correctly.

I'm using orchestra/testbench to help me on tests.

Can you check if there is something wrong?

I'll install in a fresh package and try simulate your same example.

[tmcnicholls]

@sfelix-martins I'll pull your tests into my project and xdebug to try and see what is happening differently. Thanks

[tmcnicholls]

@sfelix-martins I found the cause of the issue. In a default Laravel installation, the default guard is set to web. With this in place, your tests pass.

However, if you set your default guard to a valid api or company guard, the tests fail with the behaviour I saw.

Try adding this to MultiauthTest::setAuthConfigs and run the tests again:

config(['auth.defaults.guard' => 'api']);

or

config(['auth.defaults.guard' => 'company']);

This is happening because Illuminate\Auth\AuthManager::guard is passed null by the MultiAuthenticate::handle method, but then internally gets the default guard.

Test results:

Expected status code 401 but received 200.
Failed asserting that false is true.

[sfelix-martins]

@tmcnicholls Thanks! I'll check it ASAP.

[tmcnicholls]

Hi @sfelix-martins, did you make any progress on this?

Thanks

[sfelix-martins]

@tmcnicholls I’m sorry. Nothing yet. A pull request would be welcome! Thanks.

[sfelix-martins]

@tmcnicholls I analysed ways to finally fix this problem, and I think that I found the solution with your help. But it will broke the compatibility with older versions of the package.

I will close this pull request and create a pre-release to version 5.0. If you want help me test it you can:

Just follow this upgrade guide.

Thanks!

[tmcnicholls]

@sfelix-martins thanks for your work on this. I'll test the pre-release changes and let you know if it works for me.