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

[Improvement] Set scope pivot value by default to enable easy relationship updates #551

Closed
lucianholt97 opened this issue Jan 1, 2021 · 6 comments

Comments

@lucianholt97
Copy link

What does this improvement solve?

When managing roles and abilities in a SPA, you need to communicate updates with the API. If the api (like mine) uses traditional Laravel/Eloquent relation handling to update the assigned roles and permissions of a user, you cannot use the Bouncer facade (which does all the magic).
This results in relations being created but the pivot values for the scope not being filled.
I already have a middleware to set the global Bouncer scope before the update and a hook to refresh Bouncer after the update. Whats missing is setting the scope on the permissions and assigned_roles table automatically.

How can it be solved?

After digging deeply into Laravel I found out that you can actually set default pivot values (laravel/framework#22867).

So you could easily update the HasRoles and HasAbilities trait like follows:

public function roles()
    {
        $relation = $this->morphToMany(
            Models::classname(Role::class),
            'entity',
            Models::table('assigned_roles')
        )
            ->withPivot('scope')
            // applies where and sets default pivot value for new records
            ->withPivotValue(['scope' => Models::scope()->get()])
            ->orWherePivotNull('scope');

        return Models::scope()->applyToRelation($relation);
    }
public function abilities()
    {
        $relation = $this->morphToMany(
            Models::classname(Ability::class),
            'entity',
            Models::table('permissions')
        )
            ->withPivot('forbidden', 'scope')
            // applies where and sets default pivot value for new records
            ->withPivotValue(['scope' => Models::scope()->get()])
            ->orWherePivotNull('scope');

        return Models::scope()->applyToRelation($relation);
    }

I already created a new trait which overwrites those methods and it works perfectly. Still, I think others would be happy to have this, too.

Let me know what you think and if there are any side-effects I did not think of?

@lucianholt97
Copy link
Author

My proposed solution does not work yet because of the following (apparently unsolvable) issue: laravel/framework#29741

Another approach (without side-effects) is to introduce a pivot model:

use Illuminate\Database\Eloquent\Relations\MorphPivot;
use Silber\Bouncer\Database\Models;

/**
 * App\Models\BouncerPivot.
 *
 * @property int|string $scope
 */
class BouncerPivot extends MorphPivot
{
    protected static function boot()
    {
        parent::boot();
        self::creating(function (self $pivot) {
            $pivot->scope = Models::scope()->get();
        });
    }
}
public function roles()
{
    $relation = $this->morphToMany(
        Models::classname(Role::class),
        'entity',
        Models::table('assigned_roles')
    )->withPivot('scope')->using(BouncerPivot::class);

    return Models::scope()->applyToRelation($relation);
}
public function abilities()
{
    $relation = $this->morphToMany(
        Models::classname(Ability::class),
        'entity',
        Models::table('permissions')
    )->withPivot('forbidden', 'scope')->using(BouncerPivot::class);

    return Models::scope()->applyToRelation($relation);
}

@JosephSilber
Copy link
Owner

JosephSilber commented Jan 1, 2021

Roles and abilities in Bouncer are never meant to be done directly on the relation yourself. Always use the Bouncer class to toggle them.

In your case, where you're saving all of the user's settings at once, use the sync() methods:

Bouncer::sync($user)->abilities($abilities);
Bouncer::sync($user)->roles($roles);

@lucianholt97
Copy link
Author

@JosephSilber thanks for the quick response!
What exactly speaks against the direct approach? It works really fine for me when invoking Bouncer::refresh($user) after each update and using the Pivot Model to initially set the scope column.

@lucianholt97
Copy link
Author

As background info: I am using https://github.com/cloudcreativity/laravel-json-api for my api. It would be really dirty to overwrite the pre-build logic for updating relationships. In the background the default Laravel sync method is invoked which basically does the same thing as SyncsRolesAndAbilities::sync + my approach solves what AssociatesAbilities does. The only drawback from what I understand is not considering forbidden flags, but I don't use those at the moment.

@JosephSilber
Copy link
Owner

What exactly speaks against the direct approach?

As you've already discovered with forbidden (and there are others), Bouncer's relationships are more complex than standard Eloquent relationships.

Again, always use the Bouncer class to manage your roles and abilities.

@lucianholt97
Copy link
Author

Ok, I understand your point but will probably keep it like it is as it works and I don't need all the complexity of Bouncer.
Nevertheless, please consider using a Pivot model anyway, this would also simplify some other parts of the code and wouldn't break anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants