Skip to content

Commit 815359a

Browse files
thorstenthorsten
committed
fix: removed obsolete session id for captchas
1 parent 939828a commit 815359a

File tree

8 files changed

+46
-88
lines changed

8 files changed

+46
-88
lines changed

‎phpmyfaq/news.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
$faqSession->setCurrentUser($user);
4646

4747
$captcha = $container->get('phpmyfaq.captcha');
48-
$captcha->setSessionId($sids);
4948

5049
$comment = new Comments($faqConfig);
5150

‎phpmyfaq/src/phpMyFAQ/Captcha/GoogleRecaptcha.php‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,6 @@ public function checkCaptchaCode(string $code): bool
5050
return $response['success'] === true;
5151
}
5252

53-
/**
54-
* Setter for session id.
55-
*/
56-
public function setSessionId(string $sessionId): GoogleRecaptcha
57-
{
58-
return $this;
59-
}
60-
6153
public function isUserIsLoggedIn(): bool
6254
{
6355
return $this->userIsLoggedIn;

‎phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/NewsController.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,8 @@ public function update(Request $request): JsonResponse
152152
->setMessage(html_entity_decode((string) $content))
153153
->setAuthor($author)
154154
->setEmail($email)
155-
->setActive($active)
156-
->setComment($comment)
155+
->setActive((bool) $active)
156+
->setComment((bool) $comment)
157157
->setLink($link ?? '')
158158
->setLinkTitle($linkTitle ?? '')
159159
->setLinkTarget($target ?? '')

‎phpmyfaq/src/phpMyFAQ/Controller/Frontend/AutoCompleteController.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
<?php
22

3-
declare(strict_types=1);
4-
53
/**
64
* The Autocomplete Controller
75
*
@@ -17,6 +15,8 @@
1715
* @since 2023-07-29
1816
*/
1917

18+
declare(strict_types=1);
19+
2020
namespace phpMyFAQ\Controller\Frontend;
2121

2222
use phpMyFAQ\Category;
@@ -37,14 +37,14 @@ final class AutoCompleteController extends AbstractController
3737
#[Route(path: 'api/autocomplete')]
3838
public function search(Request $request): JsonResponse
3939
{
40-
$searchString = Filter::filterVar($request->query->get('search'), FILTER_SANITIZE_SPECIAL_CHARS);
40+
$searchString = Filter::filterVar($request->query->get(key: 'search'), FILTER_SANITIZE_SPECIAL_CHARS);
4141

4242
[$currentUser, $currentGroups] = CurrentUser::getCurrentUserGroupId($this->currentUser);
4343

4444
$category = new Category($this->configuration, $currentGroups);
4545
$category->setUser($currentUser);
4646
$category->setGroups($currentGroups);
47-
$category->transform(0);
47+
$category->transform(categoryId: 0);
4848
$category->buildCategoryTree();
4949

5050
$faqPermission = $this->container->get(id: 'phpmyfaq.faq.permission');

‎phpmyfaq/src/phpMyFAQ/Controller/Frontend/CommentController.php‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -60,65 +60,70 @@ public function create(Request $request): JsonResponse
6060
return $this->json(['error' => Translation::get(key: 'msgCaptcha')], Response::HTTP_BAD_REQUEST);
6161
}
6262

63-
$data = json_decode($request->getContent(), false, 512, JSON_THROW_ON_ERROR);
63+
$data = json_decode($request->getContent(), associative: false, depth: 512, flags: JSON_THROW_ON_ERROR);
6464

6565
if (!Token::getInstance($this->container->get(id: 'session'))->verifyToken(
66-
'add-comment',
67-
$data->{'pmf-csrf-token'},
66+
page: 'add-comment',
67+
requestToken: $data->{'pmf-csrf-token'},
6868
)) {
6969
return $this->json(['error' => Translation::get(key: 'ad_msg_noauth')], Response::HTTP_UNAUTHORIZED);
7070
}
7171

7272
$type = Filter::filterVar($data->type, FILTER_SANITIZE_SPECIAL_CHARS);
73-
$faqId = Filter::filterVar($data->id ?? null, FILTER_VALIDATE_INT, 0);
73+
$faqId = Filter::filterVar($data->id ?? null, FILTER_VALIDATE_INT, default: 0);
7474
$newsId = Filter::filterVar($data->newsId ?? null, FILTER_VALIDATE_INT);
7575
$username = Filter::filterVar($data->user, FILTER_SANITIZE_SPECIAL_CHARS);
7676
$email = Filter::filterVar($data->mail, FILTER_VALIDATE_EMAIL);
7777
$commentText = Filter::filterVar($data->comment_text, FILTER_SANITIZE_SPECIAL_CHARS);
7878

7979
switch ($type) {
8080
case 'news':
81-
$commentId = $newsId;
81+
$commentId = (int) $newsId;
8282
break;
8383
case 'faq':
84-
$commentId = $faqId;
84+
$commentId = (int) $faqId;
8585
break;
86+
default:
87+
$commentId = 0;
8688
}
8789

88-
if (empty($commentId)) {
90+
if ($commentId === 0) {
8991
return $this->json(['error' => Translation::get(key: 'errSaveComment')], Response::HTTP_BAD_REQUEST);
9092
}
9193

9294
// Check display name and e-mail address for not logged-in users
9395
if (!$this->currentUser->isLoggedIn()) {
9496
$user = $this->container->get(id: 'phpmyfaq.user');
9597
if ($user->checkDisplayName($username) && $user->checkMailAddress($email)) {
96-
$this->configuration->getLogger()->error('Name and email already used by registered user.');
98+
$this->configuration->getLogger()->error(message: 'Name and email already used by registered user.');
9799
return $this->json(['error' => Translation::get(key: 'errSaveComment')], Response::HTTP_CONFLICT);
98100
}
99101
}
100102

101103
if (
102-
!empty($username)
103-
&& !empty($email)
104-
&& !empty($commentText)
104+
$username !== ''
105+
&& $email !== ''
106+
&& $commentText !== ''
105107
&& $stopWords->checkBannedWord($commentText)
106108
&& $comment->isCommentAllowed($commentId, $languageCode, $type)
107109
&& $faq->isActive($commentId, $languageCode, $type)
108110
) {
109-
$session->userTracking('save_comment', $commentId);
111+
$session->userTracking(
112+
action: 'save_comment',
113+
data: $commentId,
114+
);
110115
$commentEntity = new Comment();
111116
$commentEntity
112117
->setRecordId((int) $commentId)
113118
->setType($type)
114119
->setUsername($username)
115120
->setEmail($email)
116121
->setComment(nl2br(strip_tags((string) $commentText)))
117-
->setDate((string) $request->server->get('REQUEST_TIME'));
122+
->setDate((string) $request->server->get(key: 'REQUEST_TIME'));
118123

119124
if ($comment->create($commentEntity)) {
120125
$notification = $this->container->get(id: 'phpmyfaq.notification');
121-
if ('faq' == $type) {
126+
if ('faq' === $type) {
122127
$faq->getFaq($commentId);
123128
$notification->sendFaqCommentNotification($faq, $commentEntity);
124129
} else {
@@ -142,7 +147,10 @@ public function create(Request $request): JsonResponse
142147
], Response::HTTP_OK);
143148
}
144149

145-
$session->userTracking('error_save_comment', $commentId);
150+
$session->userTracking(
151+
action: 'error_save_comment',
152+
data: $commentId,
153+
);
146154
return $this->json(['error' => Translation::get(key: 'errSaveComment')], Response::HTTP_BAD_REQUEST);
147155
}
148156

‎phpmyfaq/src/phpMyFAQ/Controller/Frontend/ContactController.php‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
<?php
22

3-
declare(strict_types=1);
4-
53
/**
64
* The Contact Controller
75
*
@@ -17,6 +15,8 @@
1715
* @since 2024-03-09
1816
*/
1917

18+
declare(strict_types=1);
19+
2020
namespace phpMyFAQ\Controller\Frontend;
2121

2222
use phpMyFAQ\Controller\AbstractController;
@@ -35,6 +35,8 @@ final class ContactController extends AbstractController
3535
/**
3636
* @throws Exception
3737
* @throws \JsonException
38+
* @throws \Exception
39+
*
3840
*/
3941
#[Route(path: 'api/contact', methods: ['POST'])]
4042
public function create(Request $request): JsonResponse
@@ -54,7 +56,7 @@ public function create(Request $request): JsonResponse
5456
if (
5557
$author !== ''
5658
&& $author !== '0'
57-
&& !empty($email)
59+
&& $email !== ''
5860
&& $question !== ''
5961
&& $question !== '0'
6062
&& $stopWords->checkBannedWord($question)
@@ -73,7 +75,10 @@ public function create(Request $request): JsonResponse
7375
$mailer->setReplyTo($email, $author);
7476
$mailer->addTo($this->configuration->getAdminEmail());
7577
$mailer->setReplyTo($this->configuration->getNoReplyEmail());
76-
$mailer->subject = Utils::resolveMarkers('Feedback: %sitename%', $this->configuration);
78+
$mailer->subject = Utils::resolveMarkers(
79+
text: 'Feedback: %sitename%',
80+
configuration: $this->configuration,
81+
);
7782
$mailer->message = $question;
7883
$mailer->send();
7984
unset($mailer);
@@ -82,8 +87,8 @@ public function create(Request $request): JsonResponse
8287
} catch (Exception|TransportExceptionInterface $e) {
8388
return $this->json(['error' => $e->getMessage()], Response::HTTP_BAD_REQUEST);
8489
}
85-
} else {
86-
return $this->json(['error' => Translation::get(key: 'err_sendMail')], Response::HTTP_BAD_REQUEST);
8790
}
91+
92+
return $this->json(['error' => Translation::get(key: 'err_sendMail')], Response::HTTP_BAD_REQUEST);
8893
}
8994
}

‎phpmyfaq/src/phpMyFAQ/Controller/Frontend/QuestionController.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
<?php
22

3-
declare(strict_types=1);
4-
53
/**
64
* The Question & Smart Answer Controller
75
*
@@ -17,6 +15,8 @@
1715
* @since 2024-03-03
1816
*/
1917

18+
declare(strict_types=1);
19+
2020
namespace phpMyFAQ\Controller\Frontend;
2121

2222
use phpMyFAQ\Category;
@@ -53,7 +53,7 @@ public function create(Request $request): JsonResponse
5353

5454
$categories = $category->getAllCategories();
5555

56-
$data = json_decode($request->getContent(), false, 512, JSON_THROW_ON_ERROR);
56+
$data = json_decode($request->getContent(), associative: false, depth: 512, flags: JSON_THROW_ON_ERROR);
5757

5858
$author = trim((string) Filter::filterVar($data->name, FILTER_SANITIZE_SPECIAL_CHARS));
5959
$email = trim((string) Filter::filterVar($data->email, FILTER_VALIDATE_EMAIL));
@@ -103,7 +103,7 @@ public function create(Request $request): JsonResponse
103103

104104
$searchResult = array_merge(...array_map(static fn($word) => $faqSearch->search(
105105
$word,
106-
false,
106+
allLanguages: false,
107107
), array_filter($cleanQuestion)));
108108

109109
$searchResultSet->reviewResultSet($searchResult);

‎tests/phpMyFAQ/Captcha/GoogleRecaptchaTest.php‎

Lines changed: 2 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -101,45 +101,13 @@ public function testCheckCaptchaCodeWhenNotLoggedIn(): void
101101
$this->expectNotToPerformAssertions(); // Will attempt HTTP call
102102
}
103103

104-
/**
105-
* Test setSessionId method
106-
*/
107-
public function testSetSessionId(): void
108-
{
109-
$sessionId = 'test-session-123';
110-
$result = $this->googleRecaptcha->setSessionId($sessionId);
111-
112-
$this->assertInstanceOf(GoogleRecaptcha::class, $result); // Fluent interface
113-
// Note: The method currently doesn't store the session ID, just returns $this
114-
}
115-
116-
/**
117-
* Test setSessionId with various session ID formats
118-
*/
119-
public function testSetSessionIdWithVariousFormats(): void
120-
{
121-
$sessionIds = [
122-
'simple-session',
123-
'session_with_underscores',
124-
'session123456',
125-
'very-long-session-id-with-many-characters-and-hyphens',
126-
''
127-
];
128-
129-
foreach ($sessionIds as $sessionId) {
130-
$result = $this->googleRecaptcha->setSessionId($sessionId);
131-
$this->assertInstanceOf(GoogleRecaptcha::class, $result);
132-
}
133-
}
134-
135104
/**
136105
* Test fluent interface chaining
137106
*/
138107
public function testFluentInterface(): void
139108
{
140109
$result = $this->googleRecaptcha
141110
->setUserIsLoggedIn(true)
142-
->setSessionId('test-session')
143111
->setUserIsLoggedIn(false);
144112

145113
$this->assertInstanceOf(GoogleRecaptcha::class, $result);
@@ -175,11 +143,11 @@ public function testConfigurationIntegration(): void
175143

176144
$this->googleRecaptcha->setUserIsLoggedIn(false);
177145

178-
// This will attempt to make HTTP request and fail, but we verify config is called
146+
// This will attempt to make an HTTP request and fail, but we verify config is called
179147
try {
180148
$this->googleRecaptcha->checkCaptchaCode('test-token');
181149
} catch (\Error $e) {
182-
// Expected - file_get_contents will fail in test environment
150+
// Expected - file_get_contents will fail in the test environment
183151
$this->assertStringContainsString('file_get_contents', $e->getMessage());
184152
}
185153
}
@@ -216,20 +184,6 @@ public function testObjectStateIndependence(): void
216184
$this->assertFalse($recaptcha2->isUserIsLoggedIn());
217185
}
218186

219-
/**
220-
* Test session ID method behavior
221-
*/
222-
public function testSessionIdMethodBehavior(): void
223-
{
224-
// Test that setSessionId always returns the same instance
225-
$instance1 = $this->googleRecaptcha->setSessionId('session1');
226-
$instance2 = $this->googleRecaptcha->setSessionId('session2');
227-
228-
$this->assertSame($this->googleRecaptcha, $instance1);
229-
$this->assertSame($this->googleRecaptcha, $instance2);
230-
$this->assertSame($instance1, $instance2);
231-
}
232-
233187
/**
234188
* Test authentication bypass security
235189
*/

0 commit comments

Comments
 (0)