archtechx / tenancy

Automatic multi-tenancy for Laravel. No code changes needed.
https://tenancyforlaravel.com
MIT License
3.66k stars 429 forks source link

Support Laravel Passport #200

Closed johenel closed 4 years ago

johenel commented 5 years ago

Description

Configure tenancy with Laravel Passport

Why this should be added

To support API Services developed with Laravel and using the Laravel Passport authorization

stancl commented 5 years ago

I haven't gotten to actually testing this, but this should be enough to integrate Passport:

(Optional: Use a middleware like #207 if you want the API to be on a single domain.)

Installation

Tenant & Client creation

lbausch commented 4 years ago

What is the best place to call Passport::loadKeysFrom()?

stancl commented 4 years ago

@lbausch Can you elaborate on that?

ravenberg commented 4 years ago

Hi @stancl thanks for sharing this amazing lib. I'll elaborate on @lbausch behalf.

So Passport::loadKeysFrom() is a method to configure where to locate the encryption keys Passport uses from the filesystem.

@lbausch i'd recommend using the option of loading the encryption keys from the env file instead of having to deal with the filesystem.

stancl commented 4 years ago

I see.

So if you go with the approach that stores them centrally for all tenants, calling Passport::loadKeysFrom() in for example the AppServiceProvider should work.

And if you store them in the tenants' custom storage folders, then I think this should work (again in a service provider):

tenancy()->hook('bootstrapped', function () {
    Passport::loadKeysFrom(...);
});

But I haven't tested that so I'm not sure.

@ravenberg That's a good tip. I think loading encryption keys from environment variables is ideal.

lbausch commented 4 years ago

Thanks @ravenberg for stepping in and @stancl for the additional hint. :+1:

In case of using environment variables and per tenant encryption keys, where would this .env file reside - in each tenants storage?

stancl commented 4 years ago

environment variables and per tenant encryption keys

That's not possible. .env is just a fancy source of environment variables, environment variables are set in the environment, before running the actual program.

I think you could do store the keys in the tenant storage and do this:

tenancy()->hook('boostrapped', function ($tm, Tenant $tenant) {
    config([
        'passport.public_key' => $tenant->passport_public_key,
        'passport.private_key' => $tenant->passport_private_key,
    ]);
});

Or you could take advantage of the Tenant Config feature to do this for you:

// config/tenancy.php
'storage_to_config_map' => [
    'passport_public_key' => 'passport.public_key',
    'passport_private_key' => 'passport.private_key',
],

And, of course, you need to store the keys in the tenant storage. See here how Laravel uses phpseclib\Crypt\RSA to generate the keys. The only difference is that you don't want to write them to a file, but instead to the tenant config.

The tenant storage approach might be the cleanest if you need tenant-specific keys. But before you introduce this complexity into your app, do consider the benefits of doing this. You're already using the same APP_KEY for all tenants, so using different Passport key pairs for each tenant might not bring much benefit in terms of security.

But you might have a legitimate use case for that. Just be mindful of the complexity of this approach compared to using the same keys for every tenant, and carefully consider the pros and cons.

lbausch commented 4 years ago

Thank you @stancl - I ended up wrapping the passport:keys command and saving the keys in the tenant storage. They are then loaded in the bootstrapped event using Passport::loadKeysFrom().

I was wondering the bootstrapped event is also a good place to call Passport::routes() - the downside is, that the routes don't show up when calling artisan route:list.

stancl commented 4 years ago

@lbausch I'm not that familiar with Passport, what routes does it provide / what are the routes used for?

Looking at the source of Passport::routes(), I think you should be able to do this:

Passport::routes(null, ['middleware' => 'tenancy']);

To mark the Passport routes as tenant routes.

stancl commented 4 years ago

saving the keys in the tenant storage. They are then loaded in the bootstrapped event

Note that using the Tenant Config feature is preferable, as it covers edge cases that hooking into bootstrapped doesn't. For example, setting the config back after tenancy is ended.

stancl commented 4 years ago

Passport integration is now documented.

lbausch commented 4 years ago

Awesome, thank you :+1: