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

WIP: Multi-auth for Personal Access Tokens #1353

Closed

Conversation

Jnesselr
Copy link

@Jnesselr Jnesselr commented Sep 23, 2020

I should start out by saying that I'm a bit out of my depth here (mostly with how to test this well and with the oauth2 stuff), so this isn't a complete pull request yet. I'm opening this up now to see if I'm on the right track and get suggestions on how to handle some of the issues. I'm more than willing to do the work though. With that out of the way, let's get to the "what" and "why".

Previous attempts to support Multi-Auth in Passport focused on the password grant, but I have a use case where I have two independent models that both need their own sets of tokens (host and users in my case). I tried adding the "Has Api Tokens" to both of them, but quickly found that any shared ids would share tokens. I thought about working around this by making sure the IDs could never collide, but that felt like the wrong solution so here we are.

This enables the passport:client command to get a provider from the command line for the personal access token client (actually it forces you to pick one like the password grant). Things using the "HasApiTokens" trait will search the providers config for themselves and create a token for their specific client. When using the ->tokens call on that trait, it will be limited to the tokens related to your providers.

Here's some of the issues/known bugs that still need to be worked through:

  • Testing. I added like... one test and I'm not even sure that's correct. I'm not used to testing where everything is mocked. (Tests are failing locally too, it's not just the CI in case you were going to go look into that)
  • Current consumers are going to have a "null" in their provider and this code doesn't handle that at all.
  • Pulling ->tokens on the HasApiTokens trait pulls the right tokens, but the user_id is always null. It looks correct in the database.
  • In the Laravel\Passport\Token class, there's a ->user() method but it checks the api guard instead of the client's provider
  • The PersonalAccessTokenController has a "forUser" method that looks up the token from the TokenRepository using just the user id. That obviously won't work with multi-auth but I'm not sure how to address it.

There's probably more than just that but those are the ones I know about right now. Am I going in the right direction to implement this feature?

@Jnesselr Jnesselr marked this pull request as draft September 23, 2020 00:10
@taylorotwell
Copy link
Member

I don't have any plans to add or maintain this feature.

@Jnesselr
Copy link
Author

Why not? You let the multi-auth for password authentication grants go through. I need this for my project as I haven't found a secure workaround.

@eduarguz
Copy link

@billriess I see you have worked a lot on supporting multiple auth providers for passport and it worked really well. Thank you for that and everyone involved

Would like to know from your experience if there is any way this could also be done with personal access tokens.

when using $model->createToken() the client that will be used is the personal access token client and in this case, this token will not worry about the provider. Making the token "usable" by any provider.

Have you thought about this case by any chance?

@amcsi
Copy link

amcsi commented Mar 8, 2021

For people needing this: Although Laravel Passport does not support multi-auth for personal clients (in an easy way), with a little bit of plumbing, it's possible to make it work:

2021_03_08_122535_add_passport_client_tied_to_provider.php

    public function up()
    {
        Artisan::call('passport:client', ['--personal' => true, '--name' => 'My Personal Client 1']);
        Artisan::call('passport:client', ['--personal' => true, '--name' => 'My Personal Client 2']);
        // Cannot set the provider with passport:client. Do it below.
        // https://github.com/laravel/passport/issues/1353
        // https://github.com/laravel/passport/issues/1420
        Client::where('name', 'My Personal Client 1')->update(['provider' => 'myProvider1']);
        Client::where('name', 'My Personal Client 2')->update(['provider' => 'myProvider2']);
    }

PersonalTokenCreatorService.php

<?php

declare(strict_types=1);

namespace App\Auth;

use GuzzleHttp\Utils;
use Illuminate\Contracts\Http\Kernel;
use Illuminate\Http\Request;
use Laravel\Passport\Client;

class PersonalTokenCreatorService
{
    public function createForProvider1($userId)
    {
        $request = $this->createRequest('My Provider 1', $userId);
        $response = app()[Kernel::class]->handle($request);

        return Utils::jsonDecode($response->getContent(), true)['access_token'];
    }

    public function createForProvider2($userId)
    {
        $request = $this->createRequest('My Provider 2', $userId);
        $response = app()[Kernel::class]->handle($request);

        return Utils::jsonDecode($response->getContent(), true)['access_token'];
    }

    private function createRequest($clientName, $userId)
    {
        $client = Client::where('name', $clientName)->firstOrFail();

        return Request::create('/oauth/token', 'POST', [
            'grant_type' => 'personal_access',
            'client_id' => $client->id,
            'client_secret' => $client->secret,
            'user_id' => $userId,
        ]);
    }
}

This is roughly what I got working locally. These snippets have not been tested, so please let me know if I made a mistake.

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.

4 participants