spatie / laravel-permission

Associate users with roles and permissions
https://spatie.be/docs/laravel-permission
MIT License
12.08k stars 1.76k forks source link

Question: Multi Tenant #280

Closed jonagoldman closed 7 years ago

jonagoldman commented 7 years ago

In a SaaS kind of app, I want the Roles to be tenant specific, meaning:

Roles for App X Roles for App Y
Owner Superintendent
Worker All mighty
Maintenance Superman

Its not only a matter of naming, each role has different permissions, and App X can't see App Y roles and vice versa. Also there can be an infinite number of apps...

Any suggestion for achieving this? Maybe using guards? Maybe adding a tenant column to the tables? Thanks.

drbyte commented 7 years ago

How are you implementing multi-tenancy? (bespoke code? using a package? which package?)

Are you using separate databases per tenant, to keep all their tenant-specific data separate? If no, why not? If yes, I imagine simply setting this package's tables in the config is all that's required.

jonagoldman commented 7 years ago

@drbyte multi-tenancy is implemented using a 'tenantId' column on relevant tables and a simple middleware that looks in the request header for a 'Tenant-Id' param . There is no need for extra security or data isolation so a single database is good enough in this situation, and much easier to maintain.

I just want to know in this case what will be the cleaner solution to have tenants in the roles table so each tenant can create its own set of roles.

freekmurze commented 7 years ago

An easy way to go about this is to suffix the role names and permission names with a tenant id: owner_1, 'persmission_1`, ... I'm thinking that should work without making any any changes to the package.

jonagoldman commented 7 years ago

@freekmurze the second option being adding a column to the roles table?

freekmurze commented 7 years ago

I don't know your app, but if some permissions or roles would have the same name but are used for different tenants, you might get in trouble.

antony2kx commented 7 years ago

What i did was extend the base models

Permission and Role

I override the findbyname and create functions to include the company_id which i had put into the migration.

The only problem is with the Registrar on boot

It is fetching all the permissions and innerjoining them with roles

select * from permissions 270μs /vendor/spatie/laravel-permission/src/PermissionRegistrar.php:63

select roles.*, role_has_permissions.permission_id as pivot_permission_id, role_has_permissions.role_id as pivot_role_id from roles inner join role_has_permissions on roles.id = role_has_permissions.role_id where role_has_permissions.permission_id in ('1', '2', '3', '4') 300μs /vendor/spatie/laravel-permission/src/PermissionRegistrar.php:63

at this point i would like to add where company_id={} but i cannot because its loaded directly from the package using app("Permission::class") and a trait wont solve it.

not sure

antony2kx commented 7 years ago

So i got it to work by just using the trait alone and extending the class.

namespace App;

use Spatie\Permission\Models\Permission AS BasePermission;
use App\Traits\AppendCompany;

class Permission extends BasePermission
{
    //
      use AppendCompany;

}

AND

<?php

namespace App;

use Spatie\Permission\Models\Role AS BaseRole;
use App\Traits\AppendCompany;

class Role extends BaseRole
{
    //
      use AppendCompany;

}

All i had to do after was change the classes inside config/permission.php to my extended classes like so

         */

        'permission' => App\Permission::class,

        /*
         * When using the "HasRoles" trait from this package, we need to know which
         * Eloquent model should be used to retrieve your roles. Of course, it
         * is often just the "Role" model but you may use whatever you like.
         *
         * The model you want to use as a Role model needs to implement the
         * `Spatie\Permission\Contracts\Role` contract.
         */

        'role' => App\Role::class,
SwainPrasad commented 7 years ago

@antony2kx We are also looking for the same thing which you have already did. Kindly create package with your changes so that it will be helpful for all.

antony2kx commented 7 years ago

@SwainPrasad I created my own roleModel

<?php

namespace App;

use Spatie\Permission\Models\Role AS BaseRole;
use App\Traits\AppendCompany;

use Illuminate\Database\Eloquent\SoftDeletes;
use App\Traits\Trackable;

class Role extends BaseRole
{
    //

      use AppendCompany;

      use SoftDeletes, Trackable;

      protected $dates = ['deleted_at'];

}

and permission model

<?php

namespace App;

use Spatie\Permission\Models\Permission AS BasePermission;
use App\Traits\AppendCompany;

use Illuminate\Database\Eloquent\SoftDeletes;
use App\Traits\Trackable;

class Permission extends BasePermission
{
    //
      use AppendCompany;
      use SoftDeletes, Trackable;

      protected $dates = ['deleted_at'];

}

I then modify app/config/permissions.php

and changed based models to

return [

    'models' => [

        /*
         * When using the "HasRoles" trait from this package, we need to know which
         * Eloquent model should be used to retrieve your permissions. Of course, it
         * is often just the "Permission" model but you may use whatever you like.
         *
         * The model you want to use as a Permission model needs to implement the
         * `Spatie\Permission\Contracts\Permission` contract.
         */

        'permission' => App\Permission::class,

        /*
         * When using the "HasRoles" trait from this package, we need to know which
         * Eloquent model should be used to retrieve your roles. Of course, it
         * is often just the "Role" model but you may use whatever you like.
         *
         * The model you want to use as a Role model needs to implement the
         * `Spatie\Permission\Contracts\Role` contract.
         */

        'role' => App\Role::class,

    ],

the models that i created

I also created a scope called

<?php
namespace App\Scopes;

// Scope
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Scope;
use Illuminate\Database\Eloquent\Model;

class CompanyScope implements Scope
{

    public function apply(Builder $builder, Model $model)
    {
      $builder->where('company_id', '=', app('App\Company')->id);
    }

}

append company and load it into a trait

This will append company_id to all your queries when fetching roles

You also need to make sure this "company_id" exists on your tables

And you are done

sebastiaanluca commented 7 years ago

Looking for a similar solution use multi-tenancy with one database per tenant.

Can't keep the package's tables in the tenant's database as we only have one set of roles and permissions (we define these, not the tenant). But keeping them in a shared database requires a role/permission to be set for a specific tenant (somewhat like Bouncer does but broader, per role). Yet this makes it difficult to use @can and other helpers as we'd need to check the current tenant and prevent access if the user has no such role or doesn't have it for this tenant.

@antony2kx Can you post the AppendCompany trait too? Where do you apply the company scope (just to roles and permissions?) and how does your roles and permissions tables look (with an extra company_id field?)?

Thanks in advance! Would really help in deciding the best approach.

antony2kx commented 7 years ago

So i only added company_id to the permissions table with an index and cascade on delete

<?php
namespace App\Traits;

trait AppendCompany
{

    public static function bootAppendCompany()
    {

        static::addGlobalScope(new \App\Scopes\CompanyScope);

    }

}

see above, I apply to company scope to all tables, even the users table


namespace App;

use Illuminate\Notifications\Notifiable;
use Illuminate\Foundation\Auth\User as Authenticatable;
use App\Traits\AppendTenant;

class User extends Authenticatable
{

    use AppendTenant;

that is it really, very simple.

you can hit me up via gmail at antony2kx [at] gmail [dot] com if you need help.

CodeAn commented 7 years ago

Make sure to alter the migration.

If you want to use global roles, but a multi tenant / company user application, you need a different solution.

Add this to modelhas* tables:

$table->integer('company_id')->unsigned(); $table->foreign('company_id') ->references('id') ->on(config('tablenames.companies'));

[note: I have my tables in a config file, change it to match your companies table]

Further, make sure to edit the primary keys of those tables: $table->primary(['permission_id', 'company_id', 'model_id', 'model_type'], 'model_has_permission_primary');

$table->primary(['role_id', 'company_id', 'model_id', 'model_type'], 'model_has_roles_primary');

Make sure you overwrite the function assignRole and assignPermission (and anything related/needed with assigning/deleting/syncing permissions and roles with users), as you need to store the extra company id on the relations table.

It depends on your app's requirements how you do this. E.g. it's always value x in session.

sebastiaanluca commented 7 years ago

Both really helpful!

@CodeAn Keeping the tenant/company ID in the roles/permissions tables and in the shared database is indeed the way to go. Allows me to define a role per tenant and check which role applies when in that tenant context. No need to seed roles per tenant, just in the shared database.

JGamboa commented 5 years ago

Can someone helps me a litte :D?

JGamboa commented 5 years ago

im having the problem of RoleAlreadyExists

AlanRezende commented 5 years ago

@CodeAn and @JGamboa any of you has any example in how to override the assignRole and others to work with a company_id in a modelhas[roles|permissions]?

stevegriffiths commented 5 years ago

I have come across an issue following the excellent instructions from @antony2kx above. Following these steps works in terms of getting the tables and models ready for multitenancy permissions, however I am finding that the permissions cache only ever populates with the current uer's active account roles and permissions whenever the cache updates.. This means when a user belonging to a different account tries to do anything, they have no effective permissions.

I see the cache being populated on line 131 of PermissionRegistrar.php, and considered the idea of overwriting this one method to somehow namespace the cache by account ID, but I don't know how clean this would end up being.

Has anyone been able to get around this?

JGamboa commented 5 years ago

i already solve the cache, but i need to find the link from who i take the idea

stevegriffiths commented 5 years ago

If anyone is interested, I solved the issue described in my above comment myself without needing to modify the caching behaviour in any way. The trick was to override the roles() method on the Permission model to remove the multitenancy scope (using withoutGlobalScopes()) from the relation query. This allows all permissions and roles to be retrieved and cached. The scope still works correctly when quering the roles and permissions directly and result in the correct permissions be inherited by account.

localhost777 commented 4 years ago

An easy way to go about this is to suffix the role names and permission names with a tenant id: owner_1, 'persmission_1`, ... I'm thinking that should work without making any any changes to the package.

Good idea! The accessors and mutators of Eloquent may help us to manage the tenant suffix.

micobarac commented 4 years ago

Has anyone managed to create multitenancy the way @CodeAn suggested? Any source code to share? Is it wise to change Spatie code or should you create migration and Role / Permission extended classes with overriden methods?

lovecoding-git commented 4 years ago

@micobarac did you solve issue?

micobarac commented 4 years ago

@lovecoding-git I started working on my on permission logics.

lovecoding-git commented 4 years ago

@micobarac you didn't use this package anymore?

micobarac commented 4 years ago

@lovecoding-git That's right. I created User, Conpany, Role, Permission entities with necessary relations.

malikandemir commented 3 years ago

https://divinglaravel.com/multi-tenancy-in-laravel Multi Tenant multi database multi domain We solved it adding by "app()->make(\Spatie\Permission\PermissionRegistrar::class)->forgetCachedPermissions();" into Tenant.php.

Kokil commented 2 years ago

app()->make(\Spatie\Permission\PermissionRegistrar::class)->forgetCachedPermissions();

can you explain more i stuck on same