sfelix-martins/passport-multiauth

severe security issue - authenticated user is passing all types of guards

kudlohlavec opened this issue Β· 20 comments

Hi,

I am experiencing severe security issue with version 4.0 . It doesnt appear in version 3.0

Problem is that after retrieval of access token I am able to pass through all of the defined auth guards. I got 2 types of models, however type 1 can go through auth guard 1 and 2 as well and vice versa for model of type 2.

As I have already mentioned, I am not experiencing this bug on version 3.0.

Problematic version combination is:

Laravel - 5.7.15
Passport - 7.0.3
Multiauth - 4.0

EDIT:
I am returning user instance by Auth::user() as response and when I am accessing guard 1 with token from model 2, I am retrieving user from model 1 as response with id that is same as for model 2.

However when I am accessing guard 2 with model 2 I am getting right credentials for that models user. And when I am accessing guard 2 with model 1 I am getting credentials from user of model 1.

can you post your code? I don't have this issue

@kudlohlavec Can you post your code? Or link to a similar project simulating the error?

It still has this problem ,my code :

Route::group(['prefix' => 'data', 'middleware' => 'multiauth:api'], function ($route) {
    $route->get('readData', 'ChannelFormDataController@readData');
});

image
and when I cahnge the middleware to a not exist provider , It still works ,and this is the problem .

Route::group(['prefix' => 'data', 'middleware' => 'multiauth:somethingxxx'], function ($route) {
    $route->get('readData', 'ChannelFormDataController@readData');
});

the result is the same !

the action method as follow :

    public function readData(Request $request)
    {
        return response()->json([auth()->user()]);
}

@michaelnguyen547 , @sfelix-martins I'm posting related code:

This is login method for model 1 (user):

public function apiLogin(UserLoginRequest $request){
        
        $http = new Client();

        $validated = $request->validated();

        $response = $http->post(env('REVERSE_PROXY') . '/oauth/token', [
            'form_params' => [
                'grant_type' => 'password',
                'client_id' => env('PASSPORT_CLIENT_ID'),
                'client_secret' => env('PASSPORT_KEY'),
                'username' => $validated['email'],
                'password' => $validated['password'],
                'scope' => '',
                'provider' => 'users'
            ],
        ]);

        return json_decode((string) $response->getBody(), true);
    }

This is login method for model 2 (gateway):

public function apiLogin(GatewayLoginRequest $request){
        $http = new Client();

        $validated = $request->validated();

        $response = $http->post(env('REVERSE_PROXY') . '/oauth/token', [
            'form_params' => [
                'grant_type' => 'password',
                'client_id' => env('PASSPORT_CLIENT_ID'),
                'client_secret' => env('PASSPORT_KEY'),
                'username' => $validated['serial_code'],
                'password' => $validated['password'],
                'scope' => '',
                'provider' => 'gateways'
            ],
        ]);

        return json_decode((string) $response->getBody(), true);
    }

config/auth.php:

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

'guards' => [
        'api' => [
            'driver' => 'passport',
            'provider' => 'users',
        ],

        'gateway-api' => [
            'driver' => 'passport',
            'provider' => 'gateways',
        ],
    ],

'providers' => [
        'users' => [
            'driver' => 'eloquent',
            'model' => App\User::class,
        ],

        'gateways' => [
            'driver' => 'eloquent',
            'model' => App\Gateway::class,
        ],
    ],

'passwords' => [
        'users' => [
            'provider' => 'users',
            'table' => 'password_resets',
            'expire' => 60,
        ],
    ],

];

app/Http/Kernel.php:

protected $routeMiddleware = [
        // 'auth' => \App\Http\Middleware\Authenticate::class,
        'auth' => \SMartins\PassportMultiauth\Http\Middleware\MultiAuthenticate::class,
        'auth.basic' => \Illuminate\Auth\Middleware\AuthenticateWithBasicAuth::class,
        'bindings' => \Illuminate\Routing\Middleware\SubstituteBindings::class,
        'cache.headers' => \Illuminate\Http\Middleware\SetCacheHeaders::class,
        'can' => \Illuminate\Auth\Middleware\Authorize::class,
        'guest' => \App\Http\Middleware\RedirectIfAuthenticated::class,
        'signed' => \Illuminate\Routing\Middleware\ValidateSignature::class,
        'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class,
        'verified' => \Illuminate\Auth\Middleware\EnsureEmailIsVerified::class,
        'oauth.providers' => \SMartins\PassportMultiauth\Http\Middleware\AddCustomProvider::class,
    ];

app/Providers/AuthServiceProvider.php:

public function boot()
    {
        $this->registerPolicies();

        Passport::routes();

        // Middleware `oauth.providers` middleware defined on $routeMiddleware above
        Route::group(['middleware' => 'oauth.providers'], function () {
            Passport::routes(function ($router) {
                return $router->forAccessTokens();
            });
        });
    }

I am protecting routes by auth middleware like this:

User routes

Route::group([
    'prefix' => 'app-api/v1',
    'namespace' => 'Api\V1\Application',
    'middleware' => 'auth:api'
    ],
function()
{
//API endpoints for users
}

Gateway routes

Route::group([
    'prefix' => 'gateway-api/v1',
    'namespace' => 'Api\V1\Gateway',
    'middleware' => 'auth:gateway-api'
    ],
function()
{
//API endpoints for gateways
}

Hope that it will help, however i consider it to be very basic setup...

I got little bit more informations regarding this issue.

I tried clean laravel installation where I only added passport and multiauth extension and everything is working as it should be.

However I dont understand what can be the problem, for both projects, the broken one and the new one that is working properly, I used same setup steps for implementation of multiauth functionality. The only difference is that broken project runs in docker and the correct one runs directly on localhost...

@kudlohlavec @ShuiPingYang Thanks for you contribution. I will see it ASAP.

Thanks @sfelix-martins . I was also able to reproduce @ShuiPingYang problem with unexisting provider in my docker version. In my correct localhost version, unexisting provider throws error. So as @ShuiPingYang proposed it looks to be the same problem for me and him.

Anyone has a solution? I got the same problem
But I install this package for Lumen, It works but got our problem here.
Anyone try to specify scope for each guard?

@bienhoang @kudlohlavec @ShuiPingYang
I think that updates on branch fix-same-id-model-one-token-4.0 can solve the problem! Or fix-same-id-model-one-token if you are using version 3.0 of the package.

Can you test it, for the good of open source πŸ˜„ ?

Change your composer.json file:

"minimum-stability": "dev",
"prefer-stable": true,
"require": {
    "smartins/passport-multiauth": "dev-fix-same-id-model-one-token-4.0",
}

And run composer update smartins/passport-multiauth

Give me a feedback, please.

@sfelix-martins Sorry to announce, that it didnt work for me... Still same issue. But it would be for the best if others can confirm it too.

@pakcybershagufta did you tested the instructions from #63 (comment) ?

Please, delete your composer.lock and remove the vendor folder and try again

Can ii ask one more thing any facility availbe in this package i want secure my categories /countries api which does not require user login?

Do you want that just your client apps use your /countries route, but this route don’t have authentication?

Yes exactly..

@bienhoang @kudlohlavec @ShuiPingYang
I think that updates on branch fix-same-id-model-one-token-4.0 can solve the problem! Or fix-same-id-model-one-token if you are using version 3.0 of the package.

Can you test it, for the good of open source smile ?

Change your composer.json file:

"minimum-stability": "dev",
"prefer-stable": true,
"require": {
    "smartins/passport-multiauth": "dev-fix-same-id-model-one-token-4.0",
}

And run composer update smartins/passport-multiauth

Give me a feedback, please.

@sfelix-martins Sorry to announce, that it didnt work for me... Still same issue. But it would be for the best if others can confirm it too.

imp point is missing when ever you wnat use multauth you should deifne route something like

Route::middleware('multiauth:admin,user')->get('user', function (Request $request) {
return $request->user(); // Returns an instance of designer or preloved user
});
middleware 'multiauth:admin,user' instead of auth..Very important point.

@sfelix-martins @kudlohlavec
I can also verify this issue and I found a workaround (I think). In my case I'm using 3 models:

'guards' => [
        'web' => [
            'driver' => 'session',
            'provider' => 'users',
        ],
        'api' => [
            'driver' => 'passport',
            'provider' => 'users',
        ],
        'admin' => [
            'driver' => 'passport',
            'provider' => 'admins',
        ],
        'doctor' => [
            'driver' => 'passport',
            'provider' => 'doctors',
        ],
    ],`

When a user (from the User model) gets authenticated it will be able to pass the doctor and admin guard. The same applies to all models. The only difference was that with a user requested token trying to get the user from a doctor guard I would get the user model, and the same happened when requesting the admin data from passing an admin guard. This made me think that maybe my default guard had something to do with this issue since I changed my default to api (user model). I changed my default guard back to web and everything started working as it should.

I installed the dev-fix-same-id-model-one-token-4.0 package but I still have the same problem. two types of users access each other's routes. I get the problem when I change the default guard.

Is there a way to do this without the default I want to make a endpoint consisting of only api I do not want to use the default?

//Work
    'defaults' => [
        'guard' => 'web',
    ],

    'guards' => [
        'web' => [
            'driver' => 'session',
            'provider' => 'admins',
        ],
        'admin' => [
            'driver' => 'passport',
            'provider' => 'admins',
        ],
        'member' => [
            'driver' => 'passport',
            'provider' => 'members',
        ],
    ],

    //Bug
    'defaults' => [
        'guard' => 'admin',
    ],

    'guards' => [
        'admin' => [
            'driver' => 'passport',
            'provider' => 'admins',
        ],
        'member' => [
            'driver' => 'passport',
            'provider' => 'members',
        ],
    ],```

same problem here, the bug exists when I changed the default guard. It's okay when the default change back to 'web'.

I had the same problem with Lumen. Could solve this by automatically providing the "provider" param, whenever token is provided I get the token's provider and pass it to Passport. Don't know if this can cause some trouble in any way. For my ongoing project may works fine.

public function handle(Request $request, Closure $next)
    {
        $this->defaultApiProvider = config('auth.guards.api.provider');

        $provider = $request->get('provider', 'users');

        if ($this->invalidProvider($provider)) {
            throw OAuthServerException::invalidRequest('provider');
        }
        $token = $request->bearerToken();
        if($token) {
            $parsedToken = (new Parser)->parse($token)->getClaim('jti');
            $provider = Provider::find($parsedToken)->provider;
        }
        config(['auth.guards.api.provider' => $provider]);
        
        return $next($request);
    }