-
Notifications
You must be signed in to change notification settings - Fork 23
Refactor of internal role hierarchy model + changes for 3.0.0 #34
Changes from 12 commits
c10ad55
1f8a414
ad2ca66
238f7e5
4768f64
725a782
a256e3b
4a5b6e6
0a507b4
a5d84cd
ae79491
8c5bb1c
487b847
b8bf7ea
c49aa62
2f528dc
3ac0f94
c2c47ca
0e3c8c0
5c3c711
49ec38e
6f44772
26fc89d
b9098a8
2e8430f
0f792b2
125f3aa
e0ced4c
def5290
b1c91bb
6d934ce
ee968a6
23275ec
7a0dbf8
60bf337
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
"description": "provides a role-based access control management", | ||
"license": "BSD-3-Clause", | ||
"keywords": [ | ||
"zf2", | ||
"zf3", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We stop using version with zf components, so I'd suggest to use only |
||
"Rbac" | ||
], | ||
"homepage": "https://github.com/zendframework/zend-permissions-rbac", | ||
|
@@ -13,14 +13,14 @@ | |
} | ||
}, | ||
"require": { | ||
"php": "^5.6 || ^7.0" | ||
"php": "^7.1" | ||
}, | ||
"minimum-stability": "dev", | ||
"prefer-stable": true, | ||
"extra": { | ||
"branch-alias": { | ||
"dev-master": "2.6-dev", | ||
"dev-develop": "2.7-dev" | ||
"dev-master": "3.0-dev", | ||
"dev-develop": "3.1-dev" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't change these until the develop branch is merged to master (i.e., right before the 3.0 release). When you do, use the format |
||
} | ||
}, | ||
"autoload-dev": { | ||
|
@@ -29,7 +29,7 @@ | |
} | ||
}, | ||
"require-dev": { | ||
"phpunit/phpunit": "^5.7.15|| ^6.2.1", | ||
"phpunit/phpunit": "^6.4.4", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can switch to PHPUnit 7.0.1 as it supports only PHP 7.1+ ! |
||
"zendframework/zend-coding-standard": "~1.0.0" | ||
}, | ||
"scripts": { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,7 +83,7 @@ Checking permission using `isGranted()` with a class implementing | |
use Zend\Permissions\Rbac\AssertionInterface; | ||
use Zend\Permissions\Rbac\Rbac; | ||
|
||
class AssertUserIdMatches implements AssertionInterface | ||
class AssertUserRoleMatches implements AssertionInterface | ||
{ | ||
protected $userId; | ||
protected $article; | ||
|
@@ -98,7 +98,7 @@ class AssertUserIdMatches implements AssertionInterface | |
$this->article = $article; | ||
} | ||
|
||
public function assert(Rbac $rbac) | ||
public function assert(Rbac $rbac, $permission = null, $role = null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add some typehints? If these types are mixed could we please add PHPDocs about allowed types? |
||
{ | ||
if (! $this->article) { | ||
return false; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the permission more likely than the role to be passed? I would have assumed this would be in the following order:
For that matter: are the permission and role arguments actually optional? Will there ever be a case where they are not provided?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weierophinney I added
$role
and$permission
as optional parameters because in this way we can limit BC breaks from the previous implementation. The actual interface is:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weierophinney I changed the order of params, using the follows: