fuelphp-storage / fuelphp

FuelPHP framework
http://fuelphp.com
MIT License
274 stars 57 forks source link

$global_access in has_access should remain null, but is false. #78

Closed Romouz closed 7 years ago

Romouz commented 7 years ago

In the has_access function in the ormacl.php file, if the user does not have any filter (which is the case), $global_access ends up having the value false, when it should be null.

in our case, a user was assigned a role that has no filter (neither 'A', 'R', nor 'D'). this is the cache dump of his permissions:

Array
(
    [0] => Array
        (
            [logistics] => Array
                (
                    [main] => Array
                        (
                            [0] => view
                        )

                    [order] => Array
                        (
                            [0] => view
                        )

                )

        )

    [1] => Array
        (
        )

    [2] => 
)

When \Auth::has_access('logistics.main.view'); or \Auth::has_access('logistics.order.view') is called for this user, it returns false.

after some debugging in the has_access function, the false returned specifically is this one:

// was a global filter applied?
        if (is_bool($global_access))
        {
            // we're done here
            return $global_access;
        }

Is this normal?

WanWizard commented 7 years ago

There are only two places where $global_access gets a value, which is when a role filter exists. In all other cases, it should remain null.

Does this happen both with and without an ACL cache present? What happens if you delete the ACL cache for this user? Can you dump $this->_acl_cache[$cache_key] after line 105 and see what the value for $global_access is when you read it back from cache?

You do delete the ACL cache when you update the ACL for a user, do you?

Romouz commented 7 years ago

Thanks for the quick response! After further debugging (checking the value of $global_access line by line), this is the line after which $global_access becomes false.

// cache in memory to avoid multiple cache hits for the same cache key
            if ( ! isset($this->_acl_cache[$cache_key]))
            {
                $this->_acl_cache[$cache_key] = \Cache::get($cache_key);
            }
            list($current_rights, $revoked_rights, $global_access) = $this->_acl_cache[$cache_key];

I always use \Cache::delete_all('auth'); before assigning roles. When it does clear the cache successfully, we get the same result. has_access returns false for any user without a filter (since $global_access still has a value of false) for some reason.

Sometimes, \Cache::delete_all('auth') gives this error: "rmdir: Directory not Empty!", which is weird because we know it's not empty as we're calling delete_all on the entire tree.

WanWizard commented 7 years ago

Then it loads the ACL from cache, and it is written to the cache as a boolean.

So delete the cache again, and debug https://github.com/fuel/auth/blob/1.9/develop/classes/auth/acl/ormacl.php#L239. If you don't get there, the cache isn't deleted.

p.s. you are using the wrong issue tracker, this is Fuel v2, you need to be at https://github.com/fuel/auth/issues for Fuel v1 Auth support.

Romouz commented 7 years ago

Well this is quite embarrassing.

We ran a script to make all users of group "User" (Group of id 3). We're not sure exactly how we messed up something so simple, but all users ended up with group of id 1. And group of ID 1 is assigned the role "banned". We didn't bother checking.

So this isn't an issue. Sorry for wasting your time.

WanWizard commented 7 years ago

No problem, glad you got it sorted.

Happy holidays! (and don't forget to use the correct repo next time!)