Skip to content

Commit 76548f7

Browse files
tatevikg1tatevikg1
committed
Fix: make unsubscribe path accessible
1 parent bb5a8a0 commit 76548f7

7 files changed

Lines changed: 77 additions & 16 deletions

File tree

‎src/Controller/PublicSubscribeController.php‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,28 +41,27 @@ public function unsubscribe(Request $request, int $pageId): Response
4141
{
4242
$page = $this->subscribePagesClient->getPublicSubscribePage($pageId);
4343
$pageData = $page->data;
44+
$languageFile = $pageData['language_file'] ?? 'english.inc';
45+
$languageTexts = $this->languageService->loadLanguageTexts(is_string($languageFile) ? $languageFile : null);
4446

45-
$successHtml = null;
47+
$success = false;
4648
if ($request->isMethod('POST')) {
4749
$email = trim((string) $request->request->get('email'));
4850

4951
if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
5052
throw $this->createNotFoundException('Invalid email address.');
5153
}
5254
$this->subscribePagesClient->deletePublicSubscription($pageId, $email);
53-
54-
$languageFile = $pageData['language_file'] ?? 'english.inc';
55-
$languageTexts = $this->languageService->loadLanguageTexts(is_string($languageFile) ? $languageFile : null);
56-
57-
$successHtml = $languageTexts['strUnsubscribeDone'] ?? 'You have been unsubscribed successfully.';
55+
$success = true;
5856
}
5957

6058
return $this->render('@PhpListFrontend/public/unsubscribe.html.twig', [
6159
'page' => 'Unsubscribe Page',
6260
'page_id' => $pageId,
6361
'data' => $pageData,
64-
'success_html' => $successHtml,
65-
'signature' => $this->config->getValue(ConfigOption::PoweredByImage)
62+
'success' => $success,
63+
'signature' => $this->config->getValue(ConfigOption::PoweredByImage),
64+
'language_texts' => $languageTexts,
6665
]);
6766
}
6867

‎src/DependencyInjection/PhpListFrontendExtension.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ public function prepend(ContainerBuilder $container): void
7777
'access_control' => [
7878
['path' => '^/login', 'roles' => 'PUBLIC_ACCESS'],
7979
['path' => '^/subscribe/', 'roles' => 'PUBLIC_ACCESS'],
80+
['path' => '^/unsubscribe/', 'roles' => 'PUBLIC_ACCESS'],
8081
['path' => '^/api/v2', 'roles' => 'PUBLIC_ACCESS'],
8182
['path' => '^/', 'roles' => 'ROLE_ADMIN'],
8283
],

‎src/EventSubscriber/AuthGateSubscriber.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ private function isPublicPath(Request $request): bool
7878

7979
// Allow static assets commonly served under these prefixes
8080
foreach (self::ALLOW_LIST as $prefix) {
81-
if (str_starts_with($path, $prefix)) {
81+
if ($this->matchesPrefix($path, $prefix)) {
8282
return true;
8383
}
8484
}
@@ -118,4 +118,14 @@ private function isSafeRedirectTarget(string $target): bool
118118

119119
return $normalizedPath !== '/login' && !str_starts_with($normalizedPath, '/login');
120120
}
121+
122+
private function matchesPrefix(string $path, string $prefix): bool
123+
{
124+
if (str_ends_with($prefix, '/')) {
125+
$exactPath = rtrim($prefix, '/');
126+
return $path === $exactPath || str_starts_with($path, $prefix);
127+
}
128+
129+
return str_starts_with($path, $prefix);
130+
}
121131
}

‎src/Security/SessionAuthenticator.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public function supports(Request $request): ?bool
4343
{
4444
$path = $this->normalizePath($request->getPathInfo());
4545
foreach (self::NOT_SUPPORTED_PATHS as $prefix) {
46-
if (str_starts_with($path, $prefix)) {
46+
if ($this->matchesPrefix($path, $prefix)) {
4747
return false;
4848
}
4949
}
@@ -117,4 +117,14 @@ private function isSafeRedirectTarget(string $target): bool
117117

118118
return $normalizedPath !== '/login' && !str_starts_with($normalizedPath, '/login');
119119
}
120+
121+
private function matchesPrefix(string $path, string $prefix): bool
122+
{
123+
if (str_ends_with($prefix, '/')) {
124+
$exactPath = rtrim($prefix, '/');
125+
return $path === $exactPath || str_starts_with($path, $prefix);
126+
}
127+
128+
return str_starts_with($path, $prefix);
129+
}
120130
}

‎templates/public/unsubscribe.html.twig‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{% extends '@PhpListFrontend/public/base.html.twig' %}
22

3-
{% block title %}{{ data['title']|default(language_texts.strSubscribeTitle|default('Subscribe')) }}{% endblock %}
3+
{% block title %}{{ language_texts.strUnsubscribeTitle }}{% endblock %}
44

55
{% block body %}
66
<div class="legacy-page-shell">
@@ -18,17 +18,17 @@
1818
</div>
1919
{% endif %}
2020

21-
{% if success_html %}
22-
<div class="legacy-html-text">{{ success_html|raw }}</div>
21+
{% if success %}
22+
<div class="legacy-html-text">{{ language_texts.strUnsubscribeDone|raw }}</div>
2323
{% else %}
2424
<form method="post" action="{{ path('public_unsubscribe', { pageId: page_id }) }}" class="legacy-form" novalidate>
25-
<div>{{ language_texts.strUnsubscribeTitle|default('Unsubscribe') }}</div>
25+
2626
<div class="legacy-field-group">
27-
<label class="legacy-label" for="email">{{ language_texts.strForwardEnterEmail|default('Email') }} *</label>
27+
<label class="legacy-label" for="email">{{ language_texts.strEnterEmail|default('Email') }} *</label>
2828
<input id="email" name="email" type="email" class="legacy-input" value="{{ form_data.email|default('') }}" required autocomplete="email">
2929
</div>
3030

31-
<button type="submit" class="legacy-button">{{ data['button']|default(language_texts.strContinue|default('Unsubscribe')) }}</button>
31+
<button type="submit" class="legacy-button">{{ language_texts.strContinue|default('Unsubscribe') }}</button>
3232
</form>
3333
{% endif %}
3434

‎tests/Unit/EventSubscriber/AuthGateSubscriberTest.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,33 @@ public function testSkipsRedirectForPublicSubscribePage(): void
8383

8484
$this->assertNull($event->getResponse());
8585
}
86+
87+
public function testSkipsRedirectForPublicUnsubscribePaths(): void
88+
{
89+
$session = $this->createMock(SessionInterface::class);
90+
91+
$request = Request::create('/unsubscribe');
92+
$request->setSession($session);
93+
94+
$event = new RequestEvent(
95+
$this->createMock(HttpKernelInterface::class),
96+
$request,
97+
HttpKernelInterface::MAIN_REQUEST
98+
);
99+
100+
$this->subscriber->onKernelRequest($event);
101+
$this->assertNull($event->getResponse());
102+
103+
$requestWithId = Request::create('/unsubscribe/7');
104+
$requestWithId->setSession($session);
105+
106+
$eventWithId = new RequestEvent(
107+
$this->createMock(HttpKernelInterface::class),
108+
$requestWithId,
109+
HttpKernelInterface::MAIN_REQUEST
110+
);
111+
112+
$this->subscriber->onKernelRequest($eventWithId);
113+
$this->assertNull($eventWithId->getResponse());
114+
}
86115
}

‎tests/Unit/Security/SessionAuthenticatorTest.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,18 @@ public function testSupportsReturnsFalseForLoginPath(): void
4242
$this->assertFalse($this->authenticator->supports($request));
4343
}
4444

45+
public function testSupportsReturnsFalseForUnsubscribePaths(): void
46+
{
47+
$request = Request::create('/unsubscribe');
48+
$this->assertFalse($this->authenticator->supports($request));
49+
50+
$request = Request::create('/unsubscribe/7');
51+
$this->assertFalse($this->authenticator->supports($request));
52+
53+
$request = Request::create('/subscribe');
54+
$this->assertFalse($this->authenticator->supports($request));
55+
}
56+
4557
public function testSupportsReturnsTrueForOtherPaths(): void
4658
{
4759
$request = Request::create('/dashboard');

0 commit comments

Comments
 (0)