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

Acl inheritance wildcard #12004

Closed
KuLi opened this issue Jul 22, 2016 · 4 comments
Closed

Acl inheritance wildcard #12004

KuLi opened this issue Jul 22, 2016 · 4 comments
Milestone

Comments

@KuLi
Copy link

KuLi commented Jul 22, 2016

Hi, i inherit from role and check for a non existing resource i get deny on wildcard check.

In my case a guest can access payment with resource paypal. A user can access all from payment and also an admin and superadmin. The user can access the not existing resource but the admin can't.

Here my testcase

<?php

$oAcl = new \Phalcon\Acl\Adapter\Memory();

$oAcl->setDefaultAction(\Phalcon\Acl::DENY);

$oRoleGuest = new \Phalcon\Acl\Role("guest");
$oRoleUser = new \Phalcon\Acl\Role("user");
$oRoleAdmin = new \Phalcon\Acl\Role("admin");
$oRoleSuperAdmin = new \Phalcon\Acl\Role("superadmin");

$oAcl->addRole($oRoleGuest);
$oAcl->addRole($oRoleUser, $oRoleGuest);
$oAcl->addRole($oRoleAdmin, $oRoleUser);
$oAcl->addRole($oRoleSuperAdmin, $oRoleAdmin);

$oAcl->addResource("payment", array("paypal", "facebook",));

$oAcl->allow($oRoleGuest->getName(), "payment", "paypal");
$oAcl->allow($oRoleGuest->getName(), "payment", "facebook");

$oAcl->allow($oRoleUser->getName(), "payment", "*");

var_dump(
    $oAcl->isAllowed($oRoleUser->getName(), "payment", "notSet"),
    $oAcl->isAllowed($oRoleUser->getName(), "payment", "*"),
    $oAcl->isAllowed($oRoleAdmin->getName(), "payment", "notSet"),
    $oAcl->isAllowed($oRoleAdmin->getName(), "payment", "*")
);

My output

true

true

false

true

Version 2.1.0 RC 1

@Jurigag
Copy link
Contributor

Jurigag commented Jul 22, 2016

Checked on 2.0.x and happening as well.

This is happening because _access and _roleInherits looks like this:

  '_access' => 
  array (
    'guest!*!*' => true,
    'user!*!*' => true,
    'admin!*!*' => true,
    'superadmin!*!*' => true,
    'guest!payment!paypal' => 1,
    'guest!payment!*' => 0,
    'guest!payment!facebook' => 1,
    'user!payment!*' => 1,
  ),
   '_roleInherits' => 
  array (
    'user' => 
    array (
      0 => 'guest',
    ),
    'admin' => 
    array (
      0 => 'guest',
      1 => 'user',
    ),
    'superadmin' => 
    array (
      0 => 'guest',
      1 => 'guest',
      2 => 'user',
      3 => 'admin',
    ),

So the first hit role is guest, and it's checked for wildcard, because of default action setted to deny we can't allow admin to access this. My guess is that this break needs to be removed perhaps - https://github.com/phalcon/cphalcon/blob/2.0.x/phalcon/acl/adapter/memory.zep#L577

I think we should change defaultAction behaviour, like don't add record to _access(which is actually wildcard access with default action), just check in isAllowed method if haveAccess is still null then just return defaultAction and that's it.

Jurigag added a commit to Jurigag/cphalcon that referenced this issue Jul 22, 2016
Jurigag added a commit to Jurigag/cphalcon that referenced this issue Jul 22, 2016
Jurigag added a commit to Jurigag/cphalcon that referenced this issue Jul 22, 2016
@Jurigag
Copy link
Contributor

Jurigag commented Jul 22, 2016

Well created PR, but it's changing somehow internally how acl was working. Not sure if it will affect any app, but it shouldn't and i clean a code somehow.

Fixed in 2.1.x

@sergeyklay
Copy link
Contributor

Fixed in the 2.1.x branch.

@sergeyklay sergeyklay added this to the 3.0.0 milestone Jul 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants