Subsequent protected routes test fails, but pass independently

[bkuhl]

I'm upgrading to Laravel 5.6 and v1.0.0-rc.1 from Laravel 5.5. and beta4. I'm noticing all my tests are failing that check permissions except for the first one. If a test file has 5 tests on routes that are protected by can: middleware, the first will pass and every other will fail at the $this->visit('my-url') step. If I run all the tests separately they will pass.

My version: "silber/bouncer": "v1.0.0-rc.1",

My routes:

        Route::group([
            'middleware' => ['can:'.Ability::MANAGE_GROUPS],
        ], function () {
            Route::get('groups', 'GroupController@index');
            Route::get('groups/{groupId}', 'GroupController@show');
        });

I've tried adding Bouncer::dontCache(); in my setUp() with no luck. Any ideas on things that might be an issue causing this or something I can look at?

[bkuhl]

For anyone else who might be having the same trouble, https://github.com/JosephSilber/bouncer/blob/v1.0.0-rc.1/src/Database/Queries/Abilities.php#L21-L24 is returning different results on subsequent calls regardless of my tests running in transactions and disabling Bouncer's cache.

[bkuhl]

I attempted to revert with no luck as only v1.0.0-rc.1 is compatible with Laravel 5.6

[bkuhl]

It's not making this easier that I'm having to piecemeal my production migrations together to get what the database state should look like since this project is still modifying a single migration...

[bkuhl]

Here's an example of the test:

    public function setUp()
    {
        parent::setUp();

        // handles a "actingAs()" and seeds some session data
        $this->setupAsDirector();

        $this->group = Group::where('name', DatabaseSeeder::GROUP_NAME)->first();
    }

    /** @test */
    public function searchGroupList()
    {
        $this
            ->visit('/admin/groups')
            ->see('Southeast')
            ->visit('/admin/groups?q=Mount')
            ->click($this->group->name)
            ->seePageIs('/admin/groups/'.$this->group->id);
    }

    /** @test */
    public function viewGroup()
    {
        // fails if run after the previous test, works if run standalone
        $this
            ->visit('/admin/groups/'.$this->group->id)
            ->see($this->group->owner->full_name);
    }

I'm not sure what else I can do without some feedback from Joseph.

[bkuhl]

Joseph are you able to speak to this issue?

[JosephSilber]

I'm upgrading to Laravel 5.6 and v1.0.0-rc.1 from Laravel 5.5

Which version of Bouncer are you upgrading from?

[bkuhl]

1.0.0-beta.4

[JosephSilber]

The only meaningful schema changes since beta 4 is the addition of the scope column, as outlined in the release notes for beta 5:

alter table `abilities` add `scope` int null after `only_owned`
alter table `roles` add `scope` int null after `level`
alter table `assigned_roles` add `scope` int null after `entity_type`
alter table `permissions` add `scope` int null after `forbidden`

[bkuhl]

I don't think this is a database problem. I have added those columns via the below database migration and have looked over them several times.

        Schema::table('abilities', function (Blueprint $table) {
            $table->dropUnique('abilities_unique_index');

            $table->string('title')->nullable()->after('name');
            $table->integer('scope')->after('only_owned')->nullable();
            $table->string('name', 150)->change();
            $table->string('entity_type', 150)->nullable()->change();
        });
        Schema::table('roles', function (Blueprint $table) {
            $table->dropUnique('roles_name_unique');
            $table->integer('scope')->after('level')->nullable();
            $table->unique(['name', 'scope'], 'roles_name_unique');
        });
        Schema::table('assigned_roles', function (Blueprint $table) {
            $table->integer('scope')->after('entity_type')->nullable();

            $table->index(['entity_id', 'entity_type', 'scope'], 'assigned_roles_entity_index');
        });
        Schema::table('permissions', function (Blueprint $table) {
            $table->integer('scope')->after('forbidden')->nullable();
            $table->index(
                ['entity_id', 'entity_type', 'scope'],
                'permissions_entity_index'
            );
        });

[JosephSilber]

Could you maybe create a simple repo that demonstrates your issue with the least amount of code possible?

1. Create a new Laravel project and install Bouncer. Commit.
2. Add the least amount of code necessary to reproduce your issue. Commit.
3. Push it up to Github or wherever and share a link, so that I can take a look at it.

👍

[bkuhl]

I understand the need to go hands on with it. I'll see what I can do. It's a sizable project and these failures aren't with unit tests.

[pr4xx]

I have a similar issue with a fresh 5.6 project. At first I had one single feature test which logs in some different users with different roles and does some tests against GET routes. This works flawless. As soon as I add another test file or class which comes before the said test, it stops working. Some debugging shows that the user has roles, but the roles do not have any abilities and therefore failing the "can" checks. I tried turning off cache and refreshing but this does not help either.

[pr4xx]

Ah I found something: if you edit the phpunit.xml and set CACHE_DRIVER from array to file it works again!

EDIT: It seems it does not work in every constellation. Multiple tests in one class work, but two classes still fail.

EDIT2: Doing multiple (more than once) $this->actingAs seems to trigger this effect somehow. If I duplicate my very first working test and running that test class (= 2 test methods), I get the error indicating no abilities. Even if I set the cache driver to file. Otherwise in another test class: if I duplicate a method which does some GET requests, it only works with file cache driver.

EDIT3: New finding: if I remove Bouncer::cache(); from my AppServiceProvider, then multiple tests with cache driver file do not work anymore. Even without using multiple actingAs. So the following things have an impact to bouncers behavior: using actingAs, using Bouncer::cache(), running multiple feature tests, setting cache driver from array to file.

EDIT4 (I am sorry ;)): Just found a "fix": I use Bouncer::useRoleModel(MyRole::class) which seem to be the issue. Removing this line in my AppServiceProvider eliminates every error. My tests run fine now. Nethertheless, I need this custom role model.

[pr4xx]

Finally I found the base problem: As soon as you use custom models, you have to use useRoleModel in the register function of the app service provider. Otherwise the morph map will not be updated and the database tables will contain the actual role class name instead of the string "roles".
I think my PR is obsolete now.

[JosephSilber]

What's the problem with the actual class name being stored in the DB? I personally don't like it, so I always call Relation::morphMap for all my model classes. But that's a personal choice. Why would it introduce this bug?

[pr4xx]

The problem is that when running phpunit for my test with a custom role model (defined in the boot method of my app service provider), each test except the first one will fail because the morphmap returns roles instead of the class name. Since my permission table contains the class name and not roles it fails to find the abilities.
Models::role()->getMorphClass() is the piece of code which acts differently when running more than one phpunit test. Only when using useRoleModel in the boot method.
Wouldn't it be useful to always query for the string roles instead of the morph map?

[JosephSilber]

Models::role()->getMorphClass() is the piece of code which acts differently when running more than one phpunit test

Why? This doesn't make any sense to me.

• Models::role() should be returning an instance of your Role if you've told Bouncer to useRoleModel with your role model's class name.

• Calling getMorphClass on an instance of your model should always return either the model's full class name, or (if you're called Relation::morphMap yourself) the table name.

• Why would this be different on subsequent tests?

[pr4xx]

I made an example laravel project containing this issue, please take a look:
https://github.com/pr4xx/example-bouncer-project

[JosephSilber]

@pr4xx Thanks for the sample project ❤️

---

Now that I got to look at it, I actually figured out what's going on:

1. Bouncer registers the morph map for the role & ability models in its boot method. If a custom role/ability model has been set with Bouncer, it registers the morph map for those custom models. In hindsight this may have been a mistake, since the custom models haven't been configured yet (so it doesn't actually register it) as we'll see shortly.

2. Since the app's service providers always run after any package service providers, your boot method runs after Bouncer's. Since you're setting your custom role model in your boot method, it ends up running after Bouncer has registered the morph map. At this point, your custom model does not have a morph map set up, so Bouncer::role()->getMorphClass() returns the full class name - in this case App\Role.

3. When running the initial seeding, the DB is set up using App\Role as the morph class.

4. When running the first test, it's using App\Role as the morph class, so everything is working as it should.

5. Since these Laravel feature tests all use the CreatesApplication trait, the application is rebooted before each test. This means that before the second test runs, all service providers' register and boot methods are called again.

6. At this point, Bouncer already has your custom role model set, so it now registers the morph map with your custom model class.

7. Now when the second test runs, Bouncer::role()->getMorphClass() return roles instead of App\Role. Since the initial seeds all ran with App\Role as the morph class, Bouncer can't find the relationship between the role and the abilities in the DB.

---

The solution to this is for you to register the morph map for your custom model on your own in your service provider:

use Bouncer;
use App\Role;
use Illuminate\Database\Eloquent\Relations\Relation;

public function boot()
{
    Bouncer::useRoleModel(Role::class);

    Relation::morphMap([Role::class]);
}

---

I'll now have to figure out where to add it to the docs.

Thanks for all the help!

[JosephSilber]

BTW, if any of you are not clear about what morphMap does, take a look at Laravel's documentation (scroll down to the section titled Custom Polymorphic Types):

What's missing from the documentation there is that instead of passing an associative array to morphMap, you can actually just pass it an array of model names. It'll automatically set up the map using the model's database table name 👍

[pr4xx]

Thanks for this super detailed answer! This fixes this issue for me. Maybe @bkuhl can try to resolve his issue with this information?

[bkuhl]

I'm not sure when this will be a priority for me again. The project dependent on this is on the back burner for a few weeks. I'll close it for now until I can confirm none of what has been discussed resolves it for me.

Thanks!

[JosephSilber]

@pr4xx setting out to document this, I realized there isn't really anywhere I could point people that fully explains what the morph map is, or why you'd want to use that.

So I created a detailed article about it:

How to rid your database of PHP class names in Eloquent's Polymorphic tables

Enjoy!

[bkuhl]

I picked up this project again. Resuming where I left off (I think), adding the morphMap does not resolve the issue for me.

Running php artisan migrate:fresh --seed here's what my polymorphic relations look like:

• assigned_roles.entity_type = App\User (for all values)
• permissions.entity_type = roles (for all values)
• abilities.entity_type = NULL (for all values)

I understand the role morphMap() plays in being able to resolve both App\User and roles from the container so I'm unsure what's going on with my app, I'll continue investigating.

[bkuhl]

The good news is that this isn't the issue I'm having, the issue I'm having is because I was using Bouncer::runAfterPolicies();, but that's a separate issue I need to look into. Thanks for posting these details!