Skip to content

Commit 23b48ee

Browse files
committed
fix: corrected and improved permission checks
1 parent d6fbdd4 commit 23b48ee

28 files changed

+368
-35
lines changed

phpmyfaq/admin/assets/src/api/tags.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,16 @@ describe('Tags API', () => {
5555
);
5656

5757
const tagId = '1';
58-
await deleteTag(tagId);
58+
const csrfToken = 'test-csrf';
59+
await deleteTag(tagId, csrfToken);
5960

6061
expect(global.fetch).toHaveBeenCalledWith('./api/content/tags/1', {
6162
method: 'DELETE',
6263
cache: 'no-cache',
6364
headers: {
6465
'Content-Type': 'application/json',
6566
},
67+
body: JSON.stringify({ csrfToken: 'test-csrf' }),
6668
redirect: 'follow',
6769
referrerPolicy: 'no-referrer',
6870
});
@@ -73,8 +75,9 @@ describe('Tags API', () => {
7375
global.fetch = vi.fn(() => Promise.reject(mockError));
7476

7577
const tagId = '1';
78+
const csrfToken = 'test-csrf';
7679

77-
await expect(deleteTag(tagId)).rejects.toThrow(mockError);
80+
await expect(deleteTag(tagId, csrfToken)).rejects.toThrow(mockError);
7881
});
7982
});
8083
});

phpmyfaq/admin/assets/src/api/tags.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@ export const fetchTags = async (searchString: string): Promise<unknown> => {
2727
});
2828
};
2929

30-
export const deleteTag = async (tagId: string): Promise<unknown> => {
30+
export const deleteTag = async (tagId: string, csrfToken: string): Promise<unknown> => {
3131
return await fetchJson(`./api/content/tags/${tagId}`, {
3232
method: 'DELETE',
3333
cache: 'no-cache',
3434
headers: {
3535
'Content-Type': 'application/json',
3636
},
37+
body: JSON.stringify({ csrfToken: csrfToken }),
3738
redirect: 'follow',
3839
referrerPolicy: 'no-referrer',
3940
});

phpmyfaq/admin/assets/src/content/tags.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ describe('handleTags', () => {
8585
describe('delete button', () => {
8686
const setupDeleteDom = () => {
8787
document.body.innerHTML = `
88+
<input name="pmf-csrf-token" value="test-csrf" />
8889
<table>
8990
<tbody>
9091
<tr id="pmf-row-tag-id-1">
@@ -112,7 +113,7 @@ describe('handleTags', () => {
112113

113114
await new Promise((resolve) => setTimeout(resolve, 10));
114115

115-
expect(deleteTag).toHaveBeenCalledWith('1');
116+
expect(deleteTag).toHaveBeenCalledWith('1', 'test-csrf');
116117
expect(pushNotification).toHaveBeenCalledWith('Tag deleted successfully');
117118

118119
const row = document.getElementById('pmf-row-tag-id-1');
@@ -133,7 +134,7 @@ describe('handleTags', () => {
133134

134135
await new Promise((resolve) => setTimeout(resolve, 10));
135136

136-
expect(deleteTag).toHaveBeenCalledWith('1');
137+
expect(deleteTag).toHaveBeenCalledWith('1', 'test-csrf');
137138
expect(consoleErrorSpy).toHaveBeenCalledWith('Network response was not ok:', 'Tag deletion failed');
138139

139140
// Row should still be present

phpmyfaq/admin/assets/src/content/tags.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,9 @@ export const handleTags = (): void => {
7373
element.addEventListener('click', async (event: Event) => {
7474
const target = event.target as HTMLElement;
7575
const tagId = target.getAttribute('data-pmf-id') as string;
76+
const csrfToken = (document.querySelector('input[name=pmf-csrf-token]') as HTMLInputElement).value;
7677

77-
const response = (await deleteTag(tagId)) as DeleteTagResponse;
78+
const response = (await deleteTag(tagId, csrfToken)) as DeleteTagResponse;
7879
if (response.success) {
7980
pushNotification(response.success);
8081
const row = document.getElementById(`pmf-row-tag-id-${tagId}`) as HTMLElement;

phpmyfaq/assets/templates/admin/configuration/instances.edit.twig

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
</h1>
1010
</div>
1111
<form action="./instance/update" method="post" accept-charset="utf-8">
12+
<input type="hidden" name="pmf-csrf-token" value="{{ csrfTokenUpdateInstance }}"/>
1213
<input type="hidden" name="id" value="{{ instance.id }}"/>
1314
<div class="row mb-2">
1415
<label for="url" class="col-lg-2 col-form-label">{{ ad_instance_url }}:</label>

phpmyfaq/assets/templates/admin/user/group.twig

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
<i class="bi bi-info-circle" aria-hidden="true"></i> {{ 'ad_group_details' | translate }}
4747
</h5>
4848
<form action="./group/update" method="post">
49+
<input type="hidden" name="pmf-csrf-token" value="{{ csrfTokenUpdateGroup }}">
4950
<input id="update_group_id" type="hidden" name="group_id" value="0">
5051
<div class="card-body">
5152
<div class="row mb-2">
@@ -91,6 +92,7 @@
9192

9293
<div class="col-lg-4" id="groupMemberships">
9394
<form id="group_membership" name="group_membership" method="post" action="./group/update/members">
95+
<input type="hidden" name="pmf-csrf-token" value="{{ csrfTokenUpdateGroupMembers }}">
9496
<input id="update_member_group_id" type="hidden" name="group_id" value="0">
9597
<div class="card shadow mb-4">
9698
<h5 class="card-header py-3">
@@ -170,6 +172,7 @@
170172

171173
<div id="groupRights" class="card shadow mb-4">
172174
<form id="rightsForm" action="./group/update/permissions" method="post">
175+
<input type="hidden" name="pmf-csrf-token" value="{{ csrfTokenUpdateGroupPermissions }}">
173176
<input id="rights_group_id" type="hidden" name="group_id" value="0">
174177
<h5 class="card-header py-3" id="user_rights_legend">
175178
<i aria-hidden="true" class="bi bi-lock"></i> {{ 'ad_group_rights' | translate }}

phpmyfaq/src/phpMyFAQ/Controller/AbstractController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ private function isPublicAuthenticationPath(string $pathInfo): bool
255255
/**
256256
* @throws UnauthorizedHttpException
257257
*/
258-
protected function userIsAuthenticated(): void
258+
public function userIsAuthenticated(): void
259259
{
260260
if (!$this->currentUser->isLoggedIn()) {
261261
throw new UnauthorizedHttpException(challenge: 'User is not authenticated.');

phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/AdminLogController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ final class AdminLogController extends AbstractAdministrationApiController
4141
#[Route(path: 'statistics/admin-log', name: 'admin.api.statistics.admin-log.delete', methods: ['DELETE'])]
4242
public function delete(Request $request): JsonResponse
4343
{
44-
$this->userHasPermission(PermissionType::STATISTICS_VIEWLOGS);
44+
$this->userHasPermission(PermissionType::STATISTICS_ADMINLOG);
4545

4646
$data = json_decode($request->getContent(), associative: false, depth: 512, flags: JSON_THROW_ON_ERROR);
4747

phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/CategoryController.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public function delete(Request $request): JsonResponse
107107
#[Route(path: 'category/permissions', name: 'admin.api.category.permissions', methods: ['GET'])]
108108
public function permissions(Request $request): JsonResponse
109109
{
110-
$this->userIsAuthenticated();
110+
$this->userHasPermission(PermissionType::CATEGORY_EDIT);
111111

112112
$categoryData = $request->attributes->get('categories');
113113

@@ -132,7 +132,7 @@ public function permissions(Request $request): JsonResponse
132132
#[Route(path: 'category/translations', name: 'admin.api.category.translations', methods: ['GET'])]
133133
public function translations(Request $request): JsonResponse
134134
{
135-
$this->userIsAuthenticated();
135+
$this->userHasPermission(PermissionType::CATEGORY_EDIT);
136136

137137
$category = new Category($this->configuration, [], false);
138138

phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/ConfigurationTabController.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ public function seoMetaTags(Request $request): Response
479479
)]
480480
public function translationProvider(Request $request): Response
481481
{
482-
$this->userIsAuthenticated();
482+
$this->userHasPermission(PermissionType::CONFIGURATION_EDIT);
483483

484484
return new Response(AdminMenuBuilder::renderTranslationProviderOptions($request->attributes->get(
485485
key: 'current',
@@ -493,7 +493,7 @@ public function translationProvider(Request $request): Response
493493
)]
494494
public function mailProvider(Request $request): Response
495495
{
496-
$this->userIsAuthenticated();
496+
$this->userHasPermission(PermissionType::CONFIGURATION_EDIT);
497497

498498
return new Response(AdminMenuBuilder::renderMailProviderOptions($request->attributes->get(key: 'current')));
499499
}
@@ -505,7 +505,7 @@ public function mailProvider(Request $request): Response
505505
)]
506506
public function cacheAdapter(Request $request): Response
507507
{
508-
$this->userIsAuthenticated();
508+
$this->userHasPermission(PermissionType::CONFIGURATION_EDIT);
509509

510510
return new Response(AdminMenuBuilder::renderCacheAdapterOptions($request->attributes->get(key: 'current')));
511511
}

0 commit comments

Comments
 (0)