User::roles() returns empty collection when scope is null

[flemeur]

Hi @JosephSilber

We have observed a change in the way bouncer fetches roles from the DB when no scope is set after updating from 1.0.0 to 1.0.1.

Might be the same problem as #627

The change that causes the problem is here v1.0.0...v1.0.1#diff-e1cb4855519d09d688ca27b05e3a8d796aadef4546a1bcc383746ad3a68a2a76L132-L135 introduced in this PR #608

In our code we want to get all roles (scoped and unscoped) when no global scope has been set. Hope that makes sense?

Maybe we have been depending on broken behavior, but as far as I know this behavior has existed a long time before the 1.0 release. With that in mind it is kind of dangerous to introduce such a breaking change in a patch release.

Would it be possible to restore the old behavior? Possibly through configuration so people can opt-in/opt-out?

[JosephSilber]

@flemeur correct. I merged that PR with a heavy heart, knowing that it's a big change to introduce midcycle. But I do think that it's a bug, and being that Bouncer manages ACL, every such bug is a security bug, which can't wait for a major release.

---

Can you please explain how you were relying on this? What workflow are you using that would run without a scope, and allow permission only granted to users within a given scope?

[flemeur]

Hi again

I'll try to explain our use case.

We have a Laravel API project with three roles for users: student, teacher and admin.

Students and teachers can be on many schools at the same time. They can even be a student at one school and a teacher on the other. We use the scope column in the assigned_roles table to specify the ID of the school. For admins the scope column will always be NULL.

The problem we're facing is that we need to present the users with the option of choosing which school they want to interact with when they log in. Here we need to know which role they have on each school. That is currently not possible, since the request happens "outside" of a specific scope when we call/get the User::roles() relation. The same problem occurs in our admin app when we need to see all attached schools (including role) for a given user.

If a scope always needs to be set to get the roles of a user, we might need to do a custom query instead of relying on the roles relation. Or is there any other way I'm missing to get roles across scope boundaries? I'm just hoping we don't have to change too much code just because we're updating to Laravel 10 😅

[palokankare]

We are also experiencing issues with this change! We upgraded Bouncer from version 1.0.0 to 1.0.1 while updating Laravel to version 10. However, we did not notice that this change caused some parts of our application to break.

In our case, a user may have multiple projects (scopes), and each project may have its own tickets. If a user has the ability to view tickets in one or more projects, we display the link to tickets in the main navigation and in the sub-navigation of each project. When a user requests to view tickets, whether within a scoped project or without a scope in the main level, we check if the user has the necessary ability. Unfortunately, this no longer works without a scope.

[teemuikonen]

For anyone struggling with this issue I probably have found a temporary fix which seems to work:

First create a class which extends the Scope class but reverts the breaking changes:

<?php

namespace App;

use Silber\Bouncer\Database\Scope\Scope;

class BouncerHotfixScope extends Scope
{
    public function applyToModelQuery($query, $table = null)
    {
        if (is_null($this->scope) || $this->onlyScopeRelations) {
            if ($this->onlyScopeRelations) {
                return $query;
            }
        }

        if (is_null($table)) {
            $table = $query->getModel()->getTable();
        }

        return $this->applyToQuery($query, $table);
    }

    public function applyToRelationQuery($query, $table)
    {
        if (is_null($this->scope)) {
            return $query;
        }

        return $this->applyToQuery($query, $table);
    }

    protected function applyToQuery($query, $table)
    {
        return $query->where(function ($query) use ($table) {
            $query->where("{$table}.scope", $this->scope)
                ->orWhereNull("{$table}.scope");
        });
    }
}

Then in AppServiceProvider's register() method override the Scope class to be used:

use App\BouncerHotfixScope;
use Silber\Bouncer\Database\Models as BouncerModels;

...

BouncerModels::scope(new BouncerHotfixScope);

[swvjeff]

Same problem here, this was a big breaking change for us and we had to revert to 1.0.0 until we can work on a fix. Being able to query all of a user's abilities/roles outside of scope would be useful.

[JosephSilber]

Hi all. Been traveling while this blew up!

Seems like merging that PR in a minor release was a mistake. I see that y'all agree with the original functionality, and have actually been relying on it, so it can't be considered a bug.

That said, reverting that PR now doesn't really make sense either, since now some people already rely on this new behavior (see here for example).

It is pretty easy to add this as an option to the Scope. If any of you want to take a stab at it, please send a PR with tests. Otherwise I'll try to get to it in the next few days, as I'm getting back up to speed from all the work that's piled up while I was away.

[grantholle]

So, since I was the one who opened the original issue and submitted the change 😅...

I still stand by my original point that I don't think it should get roles in the current scope or without any scope. However, I do see the merit of it being a configuration option. The good thing is that you can do what @teemuikonen suggested and just use your own custom scope as a workaround. Great foresight by @JosephSilber!

I might take a crack at a PR to make it configurable.

[skalero01]

I had the same problem, just updated to laravel 10 and thats why i changed the version, just did a fork for v1.0.0-rc.13 to be able to accept Laravel 10, if someone wants to use it to continue using it.

"require": {
        "silber/bouncer": "v1.0.0-rc.14"
    },
    "repositories": {
        {
            "type": "git",
            "url": "https://github.com/skalero01/bouncer.git"
        }
    },

[JosephSilber]

@grantholle Do you still plan to submit a PR?

[grantholle]

I tinkered a little, but am swamped at the moment. I would plan on me not being able to.

[pecuchet]

Hi all, I indeed had the same surprise. Thanks @teemuikonen for the patch! If onlyScopeRelations is false, the nested conditional in BouncerHotfixScope::applyToModelQuery() would however reintroduce the constraint for model queries. So that method should be:

public function applyToModelQuery($query, $table = null)
{
    if (is_null($this->scope) || $this->onlyScopeRelations) {
        return $query;
    }

    if (is_null($table)) {
        $table = $query->getModel()->getTable();
    }

    return $this->applyToQuery($query, $table);
}

[gdarko]

@JosephSilber any update on this?