Skip to content

Commit

Permalink
【システム】アクセスルールで設定された権限のチェックが行われていない fix #2603
Browse files Browse the repository at this point in the history
  • Loading branch information
@ryuring
ryuring committed Jul 24, 2023
1 parent d018b45 commit f138ef2
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 15 deletions.
18 changes: 15 additions & 3 deletions plugins/baser-core/src/Controller/AppController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,20 @@ public function beforeFilter(EventInterface $event)
throw new ForbiddenException(__d('baser_core', '指定されたAPIエンドポイントへのアクセスは許可されていません。'));
} else {
if (BcUtil::loginUser()) {
$this->BcMessage->setError(__d('baser_core', '指定されたページへのアクセスは許可されていません。'));
if($this->getRequest()->getMethod() === 'GET') {
$this->BcMessage->setError(__d('baser_core', '指定されたページへのアクセスは許可されていません。'));
} else {
$this->BcMessage->setError(__d('baser_core', '実行した操作は許可されていません。'));
}
}
return $this->redirect(Configure::read("BcPrefixAuth.{$prefix}.loginRedirect"));
// リファラが存在する場合はリファラにリダイレクトする
// $this->referer() で判定した場合、リファラがなくてもトップのURLが返却されるため ServerRequest で判定
if($this->getRequest()->getEnv('HTTP_REFERER')) {
$url = $this->referer();
} else {
$url = Configure::read("BcPrefixAuth.{$prefix}.loginRedirect");
}
return $this->redirect($url);
}
}

Expand All @@ -193,7 +204,8 @@ private function checkPermission()
}
/* @var PermissionsServiceInterface $permission */
$permission = $this->getService(PermissionsServiceInterface::class);
return $permission->check($this->getRequest()->getPath(), $userGroupsIds);
$request = $this->getRequest();
return $permission->check($request->getPath(), $userGroupsIds, $request->getMethod());
}

/**
Expand Down
47 changes: 37 additions & 10 deletions plugins/baser-core/src/Service/PermissionsService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,12 +328,13 @@ protected function autoFillRecord($data = []): array
*
* @param string $url
* @param array $userGroupId
* @param string $method
* @return bool
* @checked
* @unitTest
* @noTodo
*/
public function check(string $url, array $userGroupId): bool
public function check(string $url, array $userGroupId, string $method = 'GET'): bool
{
if (in_array(Configure::read('BcApp.adminGroupId'), $userGroupId)) return true;
if ($this->checkDefaultDeny($url)) return false;
Expand All @@ -348,12 +349,12 @@ public function check(string $url, array $userGroupId): bool
if($userGroupId) {
$userGroup = $userGroupsService->get($userGroupId);
}
if ($this->checkGroup($url, $permissionGroup, $userGroup)) {
if ($this->checkGroup($url, $permissionGroup, $userGroup, $method)) {
return true;
}
}
} else {
if ($this->checkGroup($url, [], null)) {
if ($this->checkGroup($url, [], null, $method)) {
return true;
}
}
Expand Down Expand Up @@ -421,12 +422,17 @@ private function checkDefaultDeny(string $url): bool
* @param string $url
* @param array $groupPermission
* @param UserGroup|EntityInterface|null $userGroup
* @param string $method
* @return boolean
* @checked
* @unitTest
* @noTodo
*/
private function checkGroup(string $url, array $groupPermission, $userGroup): bool
private function checkGroup(
string $url,
array $groupPermission,
EntityInterface $userGroup,
string $method = 'GET'): bool
{
// ドメイン部分を除外
if(preg_match('/^(http(s|):\/\/[^\/]+?\/)(.*?)$/', $url, $matches)) {
Expand Down Expand Up @@ -484,17 +490,38 @@ private function checkGroup(string $url, array $groupPermission, $userGroup): bo
$url = preg_replace($regex, '/baser/' . Inflector::underscore($key) . '/', $url);
}

// permissionType
// 1: ホワイトリスト（全部拒否して一部許可を設定）
// 2: ブラックリスト（全部許可して一部拒否を設定）
$ret = ((int) $prefixAuthSetting['permissionType'] === 2);
return $this->isAuthorized($prefixAuthSetting['permissionType'], $url, $method, $groupPermission);
}

/**
* URLとメソッドについて許可されているか確認
*
* @param int $permissionType
* - 1: ホワイトリスト（全部拒否して一部許可を設定）
* - 2: ブラックリスト（全部許可して一部拒否を設定）
* @param string $url
* @param string $method
* @param array $groupPermission
* @return bool
*/
public function isAuthorized(int $permissionType, string $url, string $method, array $groupPermission)
{
$ret = ($permissionType === 2);
foreach($groupPermission as $permission) {
$pattern = $this->convertRegexUrl($permission->url);
if (preg_match($pattern, $url)) {
$ret = $permission->auth;
if(!$permission->auth) {
$ret = false;
} else {
if($permission->method === 'GET' && $method !== 'GET') {
$ret = false;
} else {
$ret = true;
}
}
}
}
return (boolean)$ret;
return $ret;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion plugins/baser-core/src/Service/PermissionsServiceInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,13 @@ public function getAuthList(): array;
*
* @param string $url
* @param array $userGroupId
* @param string $method
* @return bool
* @checked
* @noTodo
* @unitTest
*/
public function check(string $url, array $userGroupId): bool;
public function check(string $url, array $userGroupId, string $method = 'GET'): bool;

/**
* 権限チェック対象を追加する
Expand Down
49 changes: 49 additions & 0 deletions plugins/baser-core/tests/TestCase/Service/PermissionsServiceTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class PermissionsServiceTest extends BcTestCase
*/
public function setUp(): void
{
$this->setFixtureTruncate();
parent::setUp();
$this->PermissionsService = new PermissionsService();
}
Expand Down Expand Up @@ -323,6 +324,54 @@ public function checkDataProvider()
];
}

/**
* test check on post
* @return void
*/
public function testIsAuthorized()
{
$this->truncateTable('permissions');

// ホワイトリスト
$this->assertTrue($this->PermissionsService->isAuthorized(2, '/', 'GET', []));
$this->assertTrue($this->PermissionsService->isAuthorized(2, '/', 'POST', []));
$this->assertTrue($this->PermissionsService->isAuthorized(2, '/', 'PUT', []));
$this->assertTrue($this->PermissionsService->isAuthorized(2, '/', 'PATCH', []));
$this->assertTrue($this->PermissionsService->isAuthorized(2, '/', 'DELETE', []));

// ブラックリスト（データなし）
$this->assertFalse($this->PermissionsService->isAuthorized(1, '/', 'GET', []));

// ブラックリスト（/ に対し 表示 のみ許可）
PermissionFactory::make(['url' => '/', 'method' => 'GET', 'auth' => true])->persist();
$permissions = PermissionFactory::find()->all()->toArray();
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'GET', $permissions));
$this->assertFalse($this->PermissionsService->isAuthorized(1, '/', 'POST', $permissions));
$this->assertFalse($this->PermissionsService->isAuthorized(1, '/', 'PUT', $permissions));
$this->assertFalse($this->PermissionsService->isAuthorized(1, '/', 'PATCH', $permissions));
$this->assertFalse($this->PermissionsService->isAuthorized(1, '/', 'DELETE', $permissions));

// ブラックリスト（/ に対し表示と編集を許可）
// ※ 現在、* と挙動が同じになっている。DELETE を * の場合だけ許可するか検討が必要
$this->truncateTable('permissions');
PermissionFactory::make(['url' => '/', 'method' => 'POST', 'auth' => true])->persist();
$permissions = PermissionFactory::find()->all()->toArray();
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'GET', $permissions));
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'POST', $permissions));
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'PUT', $permissions));
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'PATCH', $permissions));
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'DELETE', $permissions));

// ブラックリスト（/ に対し全て許可）
$this->truncateTable('permissions');
PermissionFactory::make(['url' => '/', 'method' => '*', 'auth' => true])->persist();
$permissions = PermissionFactory::find()->all()->toArray();
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'GET', $permissions));
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'POST', $permissions));
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'PUT', $permissions));
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'PATCH', $permissions));
$this->assertTrue($this->PermissionsService->isAuthorized(1, '/', 'DELETE', $permissions));
}

/**
* 権限チェック対象を追加する
Expand Down
4 changes: 3 additions & 1 deletion plugins/bc-blog/src/Controller/Admin/BlogPostsController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ public function initialize(): void
*/
public function beforeFilter(EventInterface $event)
{
parent::beforeFilter($event);
$response = parent::beforeFilter($event);
if($response) return $response;

$blogContentId = $this->request->getParam('pass.0');
if (!$blogContentId) throw new BcException(__d('baser_core', '不正なURLです。'));

Expand Down

0 comments on commit f138ef2

Please sign in to comment.