yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.23k stars 6.91k forks source link

FR,ENH: Separate defaultRole for authenticate and guest user #12806

Closed mdmunir closed 7 years ago

mdmunir commented 7 years ago

Introduce new properties


public $defaultAuthRoles = [];
public $defaultGuestRoles = [];

// DbManager::checkAccessFromCache()
if (isset($assignments[$itemName]) || in_array($itemName, $this->defaultRoles) || 
        ($user !== null && in_array($itemName, $this->defaultAuthRoles) || ($user === null && in_array($itemName, $this->defaultGuestRoles)) {
            return true;
        }
samdark commented 7 years ago

Please elaborate.

mdmunir commented 7 years ago

currently yii\rbac\Manager::$defaultRoles affected to both guest and authenticate user. But, there some page that only allowed for guest user like login, signup, etc. Another page only allowed for authenticate user. Without defaultRoles, mean i must attach role to each user. But using defaultRoles mean, guest also has authority to access the page.

samdark commented 7 years ago

So what's preventing doing a role check currently?

mdmunir commented 7 years ago

The goal is. I want all authenticate user has the default roles but not for guest user. So, when i do this in action

if(!$user->can('authenticate')){
    throw ForbiddenHttpException();
}
// ...

then guest user will be rejected to access page. And do this

if(!$user->can('guest')){
    throw ForbiddenHttpException();
}
// ...

Then all loged user (authenticate user) will be rejected.

dynasource commented 7 years ago

you want to split defaultRoles per role. Isnt that adding too much complexity?

For example. How should we have to use $defaultGuestRoles. This would imply that the Application knows which role a guest has, which he can't right, because he is a guest? Unless you mean that authenticated users should be able to fake to be a guest, which is still a bit complex and confusing to be adopted as core functionality?

mdmunir commented 7 years ago

@dynasource What your suggestion?

which role a guest has, which he can't right, because he is a guest?

Think more simple. Lock at login action. She is only can be accessed by guest.

But, the true problem is, current defaultRoles also affected to guest user.

dynasource commented 7 years ago

@dynasource What your suggestion?

I dont have a suggestion for you yet, sorry. Within a few weeks I will, when I get to the RBAC parts of my development. Ill come back to you.

Apart from the solution. Why do we actually want to prevent users from looking at the login page? Is it not up to the user to make that decision? Ie. why can't he login into another account he is managing? Ive noticed this restriction in the Advanced App also.

mdmunir commented 7 years ago

Why do we actually want to prevent users from looking at the login page?

Ok, forget about defaultGuestRoles. How about defaultAuthRoles?

dynasource commented 7 years ago

do you really want to add another dimension to the defaultRoles concept?

mdmunir commented 7 years ago

My issue is more simple. I want all authenticate user has default role (only authenticate, not guest). So, if you have better(more simple) solution, i will take it.

dynasource commented 7 years ago

wouldnt a solution with the special role @ (http://www.yiiframework.com/doc-2.0/guide-security-authorization.html) be more appropiate?

samdark commented 7 years ago

I didn't get it. Why don't you use @ for authenticated users and ? for guests as shown in the guide @dynasource linked to?

mdmunir commented 7 years ago

@ only work for AccessControl. I am using $user->can() to calculate menu list.

samdark commented 7 years ago

So why it's a problem? Guest user doesn't have any roles so it will give false.

mdmunir commented 7 years ago

So why it's a problem? Guest user doesn't have any roles so it will give false.

I dont think so. This code seem not doing like that https://github.com/yiisoft/yii2/blob/master/framework/rbac/DbManager.php#L157 If role is mentioned in defaultRoles, checkAccess() will return true event the user are guest.

klimov-paul commented 7 years ago

If role is mentioned in defaultRoles, checkAccess() will return true event the user are guest.

This is expected behavior. You can use business rule to exclude non authenticated users from default role.