Skip to content

Commit 6212adf

Browse files
committed
Correctly log rbac permission check and renamed property reason to logReason to avoid confusion with Policy\Result::reason
1 parent 0a3a74e commit 6212adf

File tree

4 files changed

+30
-19
lines changed

4 files changed

+30
-19
lines changed

src/Policy/Result/SuperuserResult.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
namespace CakeDC\Auth\Policy\Result;
55

66
use Authorization\Policy\ResultInterface;
7-
use Psr\Http\Message\ServerRequestInterface;
87

98
class SuperuserResult implements ResultInterface
109
{

src/Rbac/PermissionMatchResult.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ class PermissionMatchResult
2525
* PermissionMatchResult constructor.
2626
*
2727
* @param bool $allowed rule was matched, allowed value
28-
* @param string $reason reason to either allow or deny
28+
* @param string $logReason reason to either allow or deny
2929
* @param array|null $resource The resource url to check if allowed
3030
* @param array|null $permission The matching permission
3131
*/
3232
public function __construct(
3333
protected bool $allowed = false,
34-
protected string $reason = '',
34+
protected string $logReason = '',
3535
protected ?array $resource = null,
3636
protected ?array $permission = null
3737
) {
@@ -59,21 +59,21 @@ public function isAllowed(): bool
5959
}
6060

6161
/**
62-
* @param string $reason reason
62+
* @param string $logReason reason
6363
*/
64-
public function setReason(string $reason): PermissionMatchResult
64+
public function setLogReason(string $logReason): PermissionMatchResult
6565
{
66-
$this->reason = $reason;
66+
$this->logReason = $logReason;
6767

6868
return $this;
6969
}
7070

7171
/**
7272
* @return string
7373
*/
74-
public function getReason(): string
74+
public function getLogReason(): string
7575
{
76-
return $this->reason;
76+
return $this->logReason;
7777
}
7878

7979
/**

src/Rbac/Rbac.php

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,19 +121,20 @@ public function checkPermissions(array|ArrayAccess $user, ServerRequestInterface
121121
foreach ($this->permissions as $permission) {
122122
$matchResult = $this->_matchPermission($permission, $user, $role, $request);
123123
if ($matchResult !== null) {
124-
if ($this->getConfig('log')) {
125-
$this->log($matchResult->getReason(), LogLevel::DEBUG);
126-
}
124+
$this->logResult($matchResult);
127125

128126
return $matchResult;
129127
}
130128
}
131-
132-
return new PermissionMatchResult(
129+
$resource = $this->parseSource($request, $role);
130+
$result = new PermissionMatchResult(
133131
false,
134-
'No permission matching',
135-
$this->parseSource($request, $role)
132+
sprintf('No permission matching resource: %s', json_encode($resource)),
133+
$resource
136134
);
135+
$this->logResult($result);
136+
137+
return $result;
137138
}
138139

139140
/**
@@ -266,4 +267,15 @@ protected function parseSource(ServerRequestInterface $request, mixed $role): ar
266267
'role' => $role,
267268
];
268269
}
270+
271+
/**
272+
* @param \CakeDC\Auth\Rbac\PermissionMatchResult $matchResult
273+
* @return void
274+
*/
275+
protected function logResult(PermissionMatchResult $matchResult): void
276+
{
277+
if ($this->getConfig('log')) {
278+
$this->log($matchResult->getLogReason(), LogLevel::DEBUG);
279+
}
280+
}
269281
}

tests/TestCase/Auth/Rbac/PermissionMatchResultTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,18 @@ public function testConstruct()
4848
{
4949
$permissionMatchResult = new PermissionMatchResult();
5050
$this->assertFalse($permissionMatchResult->isAllowed());
51-
$this->assertSame('', $permissionMatchResult->getReason());
51+
$this->assertSame('', $permissionMatchResult->getLogReason());
5252

5353
$permissionMatchResult = new PermissionMatchResult(true, 'bazinga');
5454
$this->assertTrue($permissionMatchResult->isAllowed());
55-
$this->assertSame('bazinga', $permissionMatchResult->getReason());
55+
$this->assertSame('bazinga', $permissionMatchResult->getLogReason());
5656
}
5757

5858
public function testSetGet()
5959
{
6060
$this->assertTrue($this->permissionMatchResult->setAllowed(true)->isAllowed());
6161
$this->assertFalse($this->permissionMatchResult->setAllowed(false)->isAllowed());
62-
$this->assertNotSame('bazinga', $this->permissionMatchResult->setReason('another-reason')->getReason());
63-
$this->assertSame('bazinga', $this->permissionMatchResult->setReason('bazinga')->getReason());
62+
$this->assertNotSame('bazinga', $this->permissionMatchResult->setLogReason('another-reason')->getLogReason());
63+
$this->assertSame('bazinga', $this->permissionMatchResult->setLogReason('bazinga')->getLogReason());
6464
}
6565
}

0 commit comments

Comments
 (0)