diff --git a/assets/router/index.js b/assets/router/index.js index a9cbdf0..0ca5d30 100644 --- a/assets/router/index.js +++ b/assets/router/index.js @@ -10,6 +10,7 @@ import TemplateEditView from '../vue/views/TemplateEditView.vue' import BouncesView from '../vue/views/BouncesView.vue' import PublicPagesView from '../vue/views/PublicPagesView.vue' import PublicPageEditView from '../vue/views/PublicPageEditView.vue' +import SettingsView from '../vue/views/SettingsView.vue' export const router = createRouter({ history: createWebHistory(), @@ -28,6 +29,7 @@ export const router = createRouter({ { path: '/public', name: 'public-pages', component: PublicPagesView, meta: { title: 'Public Pages' } }, { path: '/public/create', name: 'public-page-create', component: PublicPageEditView, meta: { title: 'Create Public Page' } }, { path: '/public/:pageId/edit', name: 'public-page-edit', component: PublicPageEditView, meta: { title: 'Edit Public Page' } }, + { path: '/settings', name: 'settings', component: SettingsView, meta: { title: 'Settings' } }, { path: '/:pathMatch(.*)*', redirect: '/' }, ], }); diff --git a/assets/vue/api.js b/assets/vue/api.js index d2de813..31d22f8 100644 --- a/assets/vue/api.js +++ b/assets/vue/api.js @@ -11,6 +11,7 @@ import { SubscriberAttributesClient, TemplatesClient, BouncesClient, + ConfigClient, AdminAttributeClient, } from '@tatevikgr/rest-api-client'; const AUTHENTICATION_REDIRECT_PATH = '/login'; @@ -44,7 +45,6 @@ if (!apiBaseUrl) { const client = new Client(apiBaseUrl || '', { onAuthenticationError: redirectToLogin, - onAuthorizationError: redirectToLogin, }); if (apiToken) { @@ -64,6 +64,7 @@ client.axiosInstance?.interceptors?.response?.use( export const subscribersClient = new SubscribersClient(client); export const adminClient = new AdminClient(client); +export const adminAttributeClient = new AdminAttributeClient(client); export const listClient = new ListClient(client); export const campaignClient = new CampaignClient(client); export const listMessagesClient = new ListMessagesClient(client); @@ -73,6 +74,8 @@ export const subscribePagesClient = new SubscribePagesClient(client); export const subscriberAttributesClient = new SubscriberAttributesClient(client); export const templateClient = new TemplatesClient(client); export const bouncesClient = new BouncesClient(client); +export const configClient = new ConfigClient(client); + export const backendFetch = async (input, init = undefined) => { const response = await fetch(input, init); diff --git a/assets/vue/components/bounces/BouncesActionsPanel.vue b/assets/vue/components/bounces/BouncesActionsPanel.vue index c79604b..ad05614 100644 --- a/assets/vue/components/bounces/BouncesActionsPanel.vue +++ b/assets/vue/components/bounces/BouncesActionsPanel.vue @@ -18,7 +18,7 @@ v-for="tab in tabs" :key="tab.id" type="button" - class="px-4 py-2.5 rounded-lg text-sm font-medium transition-all whitespace-nowrap flex-shrink-0" + class="px-4 py-2.5 rounded-lg text-sm font-medium transition-all whitespace-nowrap shrink-0" :class="activeTab === tab.id ? 'bg-white text-slate-900 shadow-sm border border-slate-300' : 'text-slate-500 hover:text-slate-700 hover:bg-white/60'" diff --git a/assets/vue/components/campaigns/CampaignDirectory.vue b/assets/vue/components/campaigns/CampaignDirectory.vue index de4b5c2..af6c92c 100644 --- a/assets/vue/components/campaigns/CampaignDirectory.vue +++ b/assets/vue/components/campaigns/CampaignDirectory.vue @@ -25,22 +25,22 @@ Status Lists Processed - Statistics + Statistics Actions - Loading campaigns... + Loading campaigns... - {{ errorMessage }} + {{ errorMessage }} - No campaigns for this filter. + No campaigns for this filter. Text: {{ campaign.processedText }}

HTML: {{ campaign.processedHtml }}

- +

Total views: {{ campaign.totalViews }}

Unique views: {{ campaign.uniqueViews }}

Bounced: {{ campaign.bounced }}

@@ -223,7 +223,10 @@

Started: {{ campaign.startedAt }}

Time to send: {{ campaign.timeToSend }}

Processed: {{ campaign.processedTotal }} (Text: {{ campaign.processedText }}, HTML: {{ campaign.processedHtml }})

-

Statistics: Total views {{ campaign.totalViews }}, Unique views {{ campaign.uniqueViews }}, Bounced {{ campaign.bounced }}

+

+ Statistics: + Total views {{ campaign.totalViews }}, Unique views {{ campaign.uniqueViews }}, Bounced {{ campaign.bounced }} +

@@ -400,6 +403,7 @@ const selectedCampaign = ref(null) const viewErrorMessage = ref('') const isResending = ref(false) const resendErrorMessage = ref('') +const showStatistics = ref(true) const filterOptions = [ { id: 'all', label: 'All' }, @@ -732,62 +736,77 @@ const fetchAllCampaigns = async () => { const fetchCampaignStatistics = async () => { const statisticsMap = {} - let cursor = null - let guard = 0 + try { + let cursor = null + let guard = 0 - // Base campaign statistics - while (guard < 200) { - const response = await statisticsClient.getCampaignStatistics(cursor, 100) - const items = Array.isArray(response?.items) ? response.items : [] + while (guard < 200) { + const response = await statisticsClient.getCampaignStatistics(cursor, 100) + const items = Array.isArray(response?.items) ? response.items : [] - items.forEach((item) => { - statisticsMap[item.campaignId] = { - bounces: Number(item.bounces ?? 0), - sent: Number(item.sent ?? 0), - uniqueViews: Number(item.uniqueViews ?? 0), - } - }) + items.forEach((item) => { + statisticsMap[item.campaignId] = { + bounces: Number(item.bounces ?? 0), + sent: Number(item.sent ?? 0), + uniqueViews: Number(item.uniqueViews ?? 0), + } + }) - const hasMore = Boolean(response?.pagination?.hasMore) - const nextCursor = response?.pagination?.nextCursor ?? null - if (!hasMore || nextCursor === null) break + const hasMore = Boolean(response?.pagination?.hasMore) + const nextCursor = response?.pagination?.nextCursor ?? null - cursor = nextCursor - guard += 1 - } + if (!hasMore || nextCursor === null) break - cursor = null - guard = 0 + cursor = nextCursor + guard++ + } - // View-open statistics - while (guard < 200) { - const response = await statisticsClient.getStatisticsOfViewOpens(cursor, 100) - const items = Array.isArray(response?.items) ? response.items : [] + cursor = null + guard = 0 - items.forEach((item) => { - const existing = statisticsMap[item.campaignId] || { - bounces: 0, - sent: 0, - uniqueViews: 0, - } + while (guard < 200) { + const response = await statisticsClient.getStatisticsOfViewOpens(cursor, 100) + const items = Array.isArray(response?.items) ? response.items : [] - statisticsMap[item.campaignId] = { - ...existing, - // only merge fields that really belong here - sent: Number(item.sent ?? existing.sent), - } - }) + items.forEach((item) => { + const existing = statisticsMap[item.campaignId] ?? { + bounces: 0, + sent: 0, + uniqueViews: 0, + } - const hasMore = Boolean(response?.pagination?.hasMore) - const nextCursor = response?.pagination?.nextCursor ?? null - if (!hasMore || nextCursor === null) break + statisticsMap[item.campaignId] = { + ...existing, + sent: Number(item.sent ?? existing.sent), + } + }) - cursor = nextCursor - guard += 1 - } + const hasMore = Boolean(response?.pagination?.hasMore) + const nextCursor = response?.pagination?.nextCursor ?? null - statisticsByCampaignId.value = statisticsMap + if (!hasMore || nextCursor === null) break + + cursor = nextCursor + guard++ + } + + statisticsByCampaignId.value = statisticsMap + showStatistics.value = true + } catch (error) { + if ( + error?.name === 'AuthorizationException' || + error?.code === 'AuthorizationException' || + error?.status === 403 + ) { + showStatistics.value = false + statisticsByCampaignId.value = {} + return + } + + throw error + } } + const fetchListsForVisibleCampaigns = async () => { const pending = paginatedCampaigns.value .map((campaign) => campaign.id) diff --git a/assets/vue/components/settings/CreateAdminAttributeModal.vue b/assets/vue/components/settings/CreateAdminAttributeModal.vue new file mode 100644 index 0000000..74cfb5f --- /dev/null +++ b/assets/vue/components/settings/CreateAdminAttributeModal.vue @@ -0,0 +1,144 @@ + + + diff --git a/assets/vue/components/settings/CreateAdminModal.vue b/assets/vue/components/settings/CreateAdminModal.vue new file mode 100644 index 0000000..a50f4d2 --- /dev/null +++ b/assets/vue/components/settings/CreateAdminModal.vue @@ -0,0 +1,269 @@ + + + + diff --git a/assets/vue/components/settings/CreateSubscriberAttributeModal.vue b/assets/vue/components/settings/CreateSubscriberAttributeModal.vue new file mode 100644 index 0000000..4fd744f --- /dev/null +++ b/assets/vue/components/settings/CreateSubscriberAttributeModal.vue @@ -0,0 +1,243 @@ + + + diff --git a/assets/vue/components/settings/EditAdminAttributeModal.vue b/assets/vue/components/settings/EditAdminAttributeModal.vue new file mode 100644 index 0000000..15f7191 --- /dev/null +++ b/assets/vue/components/settings/EditAdminAttributeModal.vue @@ -0,0 +1,148 @@ + + + diff --git a/assets/vue/components/settings/EditAdminModal.vue b/assets/vue/components/settings/EditAdminModal.vue new file mode 100644 index 0000000..ccce1a7 --- /dev/null +++ b/assets/vue/components/settings/EditAdminModal.vue @@ -0,0 +1,274 @@ + + + + diff --git a/assets/vue/components/settings/EditSubscriberAttributeModal.vue b/assets/vue/components/settings/EditSubscriberAttributeModal.vue new file mode 100644 index 0000000..9ff79c1 --- /dev/null +++ b/assets/vue/components/settings/EditSubscriberAttributeModal.vue @@ -0,0 +1,225 @@ + + + diff --git a/assets/vue/components/settings/SettingsActionsPanel.vue b/assets/vue/components/settings/SettingsActionsPanel.vue new file mode 100644 index 0000000..9fb8eef --- /dev/null +++ b/assets/vue/components/settings/SettingsActionsPanel.vue @@ -0,0 +1,108 @@ + + + + diff --git a/assets/vue/components/settings/SettingsAdminAttributes.vue b/assets/vue/components/settings/SettingsAdminAttributes.vue new file mode 100644 index 0000000..ae39f0f --- /dev/null +++ b/assets/vue/components/settings/SettingsAdminAttributes.vue @@ -0,0 +1,304 @@ + + + diff --git a/assets/vue/components/settings/SettingsAdmins.vue b/assets/vue/components/settings/SettingsAdmins.vue new file mode 100644 index 0000000..bc02460 --- /dev/null +++ b/assets/vue/components/settings/SettingsAdmins.vue @@ -0,0 +1,263 @@ + + + diff --git a/assets/vue/components/settings/SettingsConfigs.vue b/assets/vue/components/settings/SettingsConfigs.vue new file mode 100644 index 0000000..5b6be46 --- /dev/null +++ b/assets/vue/components/settings/SettingsConfigs.vue @@ -0,0 +1,158 @@ + + + + diff --git a/assets/vue/components/settings/SettingsSubscriberAttributes.vue b/assets/vue/components/settings/SettingsSubscriberAttributes.vue new file mode 100644 index 0000000..30a8cfe --- /dev/null +++ b/assets/vue/components/settings/SettingsSubscriberAttributes.vue @@ -0,0 +1,304 @@ + + + diff --git a/assets/vue/views/DashboardView.spec.js b/assets/vue/views/DashboardView.spec.js new file mode 100644 index 0000000..2b5b350 --- /dev/null +++ b/assets/vue/views/DashboardView.spec.js @@ -0,0 +1,47 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { mount } from '@vue/test-utils' + +const layoutStub = { + name: 'AdminLayout', + template: '
', +} + +const blockStub = { + name: 'DashboardBlock', + template: '
', +} + +describe('DashboardView', () => { + beforeEach(() => { + document.body.innerHTML = '' + }) + + it('renders the dashboard error banner when provided by the server', async () => { + document.body.innerHTML = ` +
+ ` + + vi.resetModules() + + const { default: DashboardView } = await import('./DashboardView.vue') + + const wrapper = mount(DashboardView, { + global: { + stubs: { + AdminLayout: layoutStub, + KpiGrid: blockStub, + PerformanceChartCard: blockStub, + QuickActionsCard: blockStub, + RecentCampaignsCard: blockStub, + }, + }, + }) + + expect(wrapper.text()).toContain('Session expired') + expect(wrapper.find('[role="alert"]').exists()).toBe(true) + }) +}) diff --git a/assets/vue/views/DashboardView.vue b/assets/vue/views/DashboardView.vue index 16dff3d..2ff404c 100644 --- a/assets/vue/views/DashboardView.vue +++ b/assets/vue/views/DashboardView.vue @@ -4,6 +4,14 @@
+ + @@ -50,6 +58,7 @@ const parseDashboardStats = () => { } const dashboardStats = parseDashboardStats() +const dashboardError = appElement?.dataset.dashboardError || '' const chart = dashboardStats.chart || { labels: [], diff --git a/assets/vue/views/SettingsView.vue b/assets/vue/views/SettingsView.vue new file mode 100644 index 0000000..7c13671 --- /dev/null +++ b/assets/vue/views/SettingsView.vue @@ -0,0 +1,12 @@ + + + diff --git a/package.json b/package.json index d50dee4..dd09ffb 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ }, "dependencies": { "@ckeditor/ckeditor5-vue": "^7.4.0", - "@tatevikgr/rest-api-client": "^2.1.8", + "@tatevikgr/rest-api-client": "^2.1.15", "apexcharts": "^5.10.4", "ckeditor5": "^48.0.0", "vue": "^3.5.16", diff --git a/src/Controller/AuthController.php b/src/Controller/AuthController.php index ef3c7b3..7a06a17 100755 --- a/src/Controller/AuthController.php +++ b/src/Controller/AuthController.php @@ -7,6 +7,7 @@ use Exception; use GuzzleHttp\Exception\GuzzleException; use PhpList\RestApiClient\Endpoint\AuthClient; +use PhpList\WebFrontend\Trait\RedirectValidationTrait; use Psr\Log\LoggerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\JsonResponse; @@ -16,8 +17,10 @@ class AuthController extends AbstractController { + use RedirectValidationTrait; + public function __construct( - private readonly AuthClient $authClient, + private readonly AuthClient $authClient, private readonly LoggerInterface $logger ) { } @@ -120,20 +123,4 @@ private function resolveRedirectTarget(Request $request): ?string return $this->isSafeRedirectTarget($redirectTarget) ? $redirectTarget : null; } - - private function isSafeRedirectTarget(string $target): bool - { - if (!str_starts_with($target, '/') || str_starts_with($target, '//')) { - return false; - } - - $path = parse_url($target, PHP_URL_PATH); - if (!is_string($path)) { - return false; - } - - $normalizedPath = (string) preg_replace('#^/(?:app|app_test)\.php#', '', $path, 1); - - return $normalizedPath !== '/login' && !str_starts_with($normalizedPath, '/login'); - } } diff --git a/src/Controller/BaseController.php b/src/Controller/BaseController.php index 2cd7b78..b796bff 100644 --- a/src/Controller/BaseController.php +++ b/src/Controller/BaseController.php @@ -8,6 +8,7 @@ use PhpList\RestApiClient\Entity\Administrator; use PhpList\RestApiClient\Exception\ApiException; use PhpList\RestApiClient\Exception\AuthenticationException; +use PhpList\RestApiClient\Exception\AuthorizationException; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; class BaseController extends AbstractController @@ -21,7 +22,7 @@ protected function getAdmin(): ?Administrator { try { $admin = $this->authClient->getSessionUser(); - } catch (ApiException | AuthenticationException) { + } catch (ApiException | AuthenticationException | AuthorizationException) { $admin = null; } diff --git a/src/Controller/DashboardController.php b/src/Controller/DashboardController.php index ac9ee6e..6e865e7 100755 --- a/src/Controller/DashboardController.php +++ b/src/Controller/DashboardController.php @@ -4,6 +4,8 @@ namespace PhpList\WebFrontend\Controller; +use PhpList\RestApiClient\Exception\AuthenticationException; +use PhpList\RestApiClient\Exception\AuthorizationException; use PhpList\RestApiClient\Endpoint\StatisticsClient; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\Request; @@ -19,7 +21,27 @@ public function __construct(private readonly StatisticsClient $statisticsClient) #[Route('/', name: 'home', methods: ['GET'])] public function index(Request $request): Response { - $stats = $this->statisticsClient->getDashboardStats(); + $dashboardStats = []; + $dashboardError = null; + + try { + $stats = $this->statisticsClient->getDashboardStats(); + $dashboardStats = $this->buildDashboardStats($stats); + } catch (AuthenticationException | AuthorizationException $e) { + $dashboardError = $e->getMessage() ?: 'Unable to load dashboard statistics.'; + } + + return $this->render('@PhpListFrontend/spa.html.twig', [ + 'page' => 'Dashboard', + 'api_token' => $request->getSession()->get('auth_token'), + 'api_base_url' => $this->getParameter('api_base_url'), + 'dashboard_stats' => $dashboardStats, + 'dashboard_error' => $dashboardError, + ]); + } + + private function buildDashboardStats(object $stats): array + { $recentCampaigns = []; foreach ($stats->recentCampaigns as $campaign) { $recentCampaigns[] = [ @@ -40,36 +62,31 @@ public function index(Request $request): Response $chartClicks[] = $point->clicks; } - return $this->render('@PhpListFrontend/spa.html.twig', [ - 'page' => 'Dashboard', - 'api_token' => $request->getSession()->get('auth_token'), - 'api_base_url' => $this->getParameter('api_base_url'), - 'dashboard_stats' => [ - 'total_subscribers' => [ - 'value' => $stats->totalSubscribers->value, - 'change_vs_last_month' => $stats->totalSubscribers->changeVsLastMonth, - ], - 'active_campaigns' => [ - 'value' => $stats->activeCampaigns->value, - 'change_vs_last_month' => $stats->activeCampaigns->changeVsLastMonth, - ], - 'open_rate' => [ - 'value' => $stats->openRate->value, - 'change_vs_last_month' => $stats->openRate->changeVsLastMonth, - ], - 'bounce_rate' => [ - 'value' => $stats->bounceRate->value, - 'change_vs_last_month' => $stats->bounceRate->changeVsLastMonth, - ], - 'recent_campaigns' => $recentCampaigns, - 'chart' => [ - 'labels' => $chartLabels, - 'series' => [ - ['name' => 'Opens', 'data' => $chartOpens], - ['name' => 'Clicks', 'data' => $chartClicks], - ], + return [ + 'total_subscribers' => [ + 'value' => $stats->totalSubscribers->value, + 'change_vs_last_month' => $stats->totalSubscribers->changeVsLastMonth, + ], + 'active_campaigns' => [ + 'value' => $stats->activeCampaigns->value, + 'change_vs_last_month' => $stats->activeCampaigns->changeVsLastMonth, + ], + 'open_rate' => [ + 'value' => $stats->openRate->value, + 'change_vs_last_month' => $stats->openRate->changeVsLastMonth, + ], + 'bounce_rate' => [ + 'value' => $stats->bounceRate->value, + 'change_vs_last_month' => $stats->bounceRate->changeVsLastMonth, + ], + 'recent_campaigns' => $recentCampaigns, + 'chart' => [ + 'labels' => $chartLabels, + 'series' => [ + ['name' => 'Opens', 'data' => $chartOpens], + ['name' => 'Clicks', 'data' => $chartClicks], ], ], - ]); + ]; } } diff --git a/src/Controller/SettingsController.php b/src/Controller/SettingsController.php new file mode 100644 index 0000000..02d8684 --- /dev/null +++ b/src/Controller/SettingsController.php @@ -0,0 +1,24 @@ +render('@PhpListFrontend/spa.html.twig', [ + 'page' => 'Settings', + 'api_token' => $request->getSession()->get('auth_token'), + 'api_base_url' => $this->getParameter('api_base_url'), + ]); + } +} diff --git a/src/EventSubscriber/AuthGateSubscriber.php b/src/EventSubscriber/AuthGateSubscriber.php index 6c51ce4..eb77fef 100644 --- a/src/EventSubscriber/AuthGateSubscriber.php +++ b/src/EventSubscriber/AuthGateSubscriber.php @@ -4,6 +4,7 @@ namespace PhpList\WebFrontend\EventSubscriber; +use PhpList\WebFrontend\Trait\RedirectValidationTrait; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; @@ -18,6 +19,8 @@ */ class AuthGateSubscriber implements EventSubscriberInterface { + use RedirectValidationTrait; + private const ALLOW_LIST = [ '/api/v2', '/subscribe/', @@ -86,11 +89,6 @@ private function isPublicPath(Request $request): bool return false; } - private function normalizePath(string $path): string - { - return (string) preg_replace('#^/(?:app|app_test)\.php#', '', $path, 1); - } - private function buildLoginUrl(Request $request): string { $loginUrl = $this->urlGenerator->generate('login'); @@ -103,22 +101,6 @@ private function buildLoginUrl(Request $request): string return $loginUrl . '?' . http_build_query(['redirect' => $redirectTarget]); } - private function isSafeRedirectTarget(string $target): bool - { - if (!str_starts_with($target, '/') || str_starts_with($target, '//')) { - return false; - } - - $path = parse_url($target, PHP_URL_PATH); - if (!is_string($path)) { - return false; - } - - $normalizedPath = $this->normalizePath($path); - - return $normalizedPath !== '/login' && !str_starts_with($normalizedPath, '/login'); - } - private function matchesPrefix(string $path, string $prefix): bool { if (str_ends_with($prefix, '/')) { diff --git a/src/EventSubscriber/UnauthorizedSubscriber.php b/src/EventSubscriber/UnauthorizedSubscriber.php index dc73e03..d80c8cc 100755 --- a/src/EventSubscriber/UnauthorizedSubscriber.php +++ b/src/EventSubscriber/UnauthorizedSubscriber.php @@ -4,16 +4,21 @@ namespace PhpList\WebFrontend\EventSubscriber; +use PhpList\WebFrontend\Trait\RedirectValidationTrait; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\RedirectResponse; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\ExceptionEvent; use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use PhpList\RestApiClient\Exception\AuthenticationException; +use PhpList\RestApiClient\Exception\AuthorizationException; class UnauthorizedSubscriber implements EventSubscriberInterface { + use RedirectValidationTrait; + public function __construct( private readonly UrlGeneratorInterface $urlGenerator, ) { @@ -30,6 +35,25 @@ public function onKernelException(ExceptionEvent $event): void { $exception = $event->getThrowable(); + if ($exception instanceof AuthorizationException) { + $message = 'Access denied.'; + + if ($event->getRequest()->isXmlHttpRequest()) { + $event->setResponse(new JsonResponse([ + 'error' => 'access_denied', + 'message' => $message, + ], 403)); + + return; + } + + $event->setResponse(new Response($message, 403, [ + 'Content-Type' => 'text/plain; charset=UTF-8', + ])); + + return; + } + if ($exception instanceof AuthenticationException) { $request = $event->getRequest(); @@ -72,20 +96,4 @@ private function buildLoginUrl(string $redirectTarget): string return $loginUrl . '?' . http_build_query(['redirect' => $redirectTarget]); } - - private function isSafeRedirectTarget(string $target): bool - { - if (!str_starts_with($target, '/') || str_starts_with($target, '//')) { - return false; - } - - $path = parse_url($target, PHP_URL_PATH); - if (!is_string($path)) { - return false; - } - - $normalizedPath = (string) preg_replace('#^/(?:app|app_test)\.php#', '', $path, 1); - - return $normalizedPath !== '/login' && !str_starts_with($normalizedPath, '/login'); - } } diff --git a/src/Security/SessionAuthenticator.php b/src/Security/SessionAuthenticator.php index 00498d2..49f6d4c 100644 --- a/src/Security/SessionAuthenticator.php +++ b/src/Security/SessionAuthenticator.php @@ -4,6 +4,7 @@ namespace PhpList\WebFrontend\Security; +use PhpList\WebFrontend\Trait\RedirectValidationTrait; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -19,6 +20,8 @@ class SessionAuthenticator extends AbstractAuthenticator implements AuthenticationEntryPointInterface { + use RedirectValidationTrait; + private const NOT_SUPPORTED_PATHS = [ '/login', '/subscribe/', @@ -102,22 +105,6 @@ private function buildLoginUrl(string $redirectTarget): string return $loginUrl . '?' . http_build_query(['redirect' => $redirectTarget]); } - private function isSafeRedirectTarget(string $target): bool - { - if (!str_starts_with($target, '/') || str_starts_with($target, '//')) { - return false; - } - - $path = parse_url($target, PHP_URL_PATH); - if (!is_string($path)) { - return false; - } - - $normalizedPath = $this->normalizePath($path); - - return $normalizedPath !== '/login' && !str_starts_with($normalizedPath, '/login'); - } - private function matchesPrefix(string $path, string $prefix): bool { if (str_ends_with($prefix, '/')) { diff --git a/src/Trait/RedirectValidationTrait.php b/src/Trait/RedirectValidationTrait.php new file mode 100644 index 0000000..a8e653e --- /dev/null +++ b/src/Trait/RedirectValidationTrait.php @@ -0,0 +1,29 @@ +normalizePath($path); + + return $normalizedPath !== '/login' && !str_starts_with($normalizedPath, '/login'); + } + + private function normalizePath(string $path): string + { + return (string) preg_replace('#^/(?:app|app_test)\.php#', '', $path, 1); + } +} diff --git a/templates/spa.html.twig b/templates/spa.html.twig index d478a74..a0fda5a 100644 --- a/templates/spa.html.twig +++ b/templates/spa.html.twig @@ -7,6 +7,7 @@ data-api-token="{{ api_token|default('') }}" data-api-base-url="{{ api_base_url|default('') }}" data-dashboard-stats="{{ dashboard_stats|default({})|json_encode|e('html_attr') }}" + data-dashboard-error="{{ dashboard_error|default('')|e('html_attr') }}" data-bounce-actions="{{ bounce_actions|default({})|json_encode|e('html_attr') }}">
{% endblock %} diff --git a/tests/Integration/Controller/DashboardControllerTest.php b/tests/Integration/Controller/DashboardControllerTest.php index b5c8061..309bd9c 100644 --- a/tests/Integration/Controller/DashboardControllerTest.php +++ b/tests/Integration/Controller/DashboardControllerTest.php @@ -5,6 +5,8 @@ namespace PhpList\WebFrontend\Tests\Integration\Controller; use PhpList\RestApiClient\Endpoint\StatisticsClient; +use PhpList\RestApiClient\Exception\AuthenticationException; +use PhpList\RestApiClient\Exception\AuthorizationException; use PhpList\RestApiClient\Response\Statistics\DashboardStatisticsResponse; use PhpList\WebFrontend\Controller\DashboardController; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; @@ -52,6 +54,62 @@ public function testDashboardRendersSpaPayloadWithStats(): void self::assertStringContainsString('data-dashboard-stats=', $content); self::assertStringContainsString('"recent_campaigns"', $content); self::assertStringContainsString('Weekly Digest', $content); + self::assertStringContainsString('data-dashboard-error=""', $content); + } + + public function testDashboardRendersDashboardErrorWhenAuthFails(): void + { + self::bootKernel(); + + $statsClient = $this->createMock(StatisticsClient::class); + $statsClient->expects(self::once()) + ->method('getDashboardStats') + ->willThrowException(new AuthenticationException('Session expired')); + + $controller = new DashboardController($statsClient); + $controller->setContainer(static::getContainer()); + + $request = Request::create('/'); + $session = new Session(new MockArraySessionStorage()); + $session->set('auth_token', 'integration-token'); + $request->setSession($session); + + $response = $controller->index($request); + $content = (string) $response->getContent(); + + self::assertSame(200, $response->getStatusCode()); + self::assertStringContainsString('data-dashboard-error="Session expired"', $content); + self::assertStringContainsString('data-dashboard-stats="[]"', $content); + } + + public function testDashboardRendersDashboardErrorWhenAuthorizationFails(): void + { + self::bootKernel(); + + $statsClient = $this->createMock(StatisticsClient::class); + $statsClient->expects(self::once()) + ->method('getDashboardStats') + ->willThrowException(new AuthorizationException( + 'No valid session key was provided as basic auth password.', + 403 + )); + + $controller = new DashboardController($statsClient); + $controller->setContainer(static::getContainer()); + + $request = Request::create('/'); + $session = new Session(new MockArraySessionStorage()); + $session->set('auth_token', 'integration-token'); + $request->setSession($session); + + $response = $controller->index($request); + $content = (string) $response->getContent(); + + $error = 'data-dashboard-error="No valid session key was provided ' + . 'as basic auth password."'; + self::assertSame(200, $response->getStatusCode()); + self::assertStringContainsString($error, $content); + self::assertStringContainsString('data-dashboard-stats="[]"', $content); } private function createDashboardStatsResponse(): DashboardStatisticsResponse diff --git a/tests/Unit/EventSubscriber/UnauthorizedSubscriberTest.php b/tests/Unit/EventSubscriber/UnauthorizedSubscriberTest.php index 719cc1e..1183935 100644 --- a/tests/Unit/EventSubscriber/UnauthorizedSubscriberTest.php +++ b/tests/Unit/EventSubscriber/UnauthorizedSubscriberTest.php @@ -6,6 +6,7 @@ use Exception; use PhpList\RestApiClient\Exception\AuthenticationException; +use PhpList\RestApiClient\Exception\AuthorizationException; use PhpList\WebFrontend\EventSubscriber\UnauthorizedSubscriber; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -78,6 +79,38 @@ public function testOnKernelExceptionWithUnauthorizedException(): void $this->assertEquals('/login?redirect=%2Fdashboard%3Frange%3D30d', $response->getTargetUrl()); } + public function testOnKernelExceptionWithAuthorizationException(): void + { + $authException = new AuthorizationException('No valid session key was provided as basic auth password.', 403); + + $session = $this->createMock(SessionInterface::class); + $session->expects($this->never())->method('invalidate'); + + $request = $this->createMock(Request::class); + $request->method('hasSession')->willReturn(true); + $request->method('getSession')->willReturn($session); + $request->method('isXmlHttpRequest')->willReturn(true); + $request->method('getRequestUri')->willReturn('/dashboard'); + + $kernel = $this->createMock(HttpKernelInterface::class); + $event = new ExceptionEvent( + $kernel, + $request, + HttpKernelInterface::MAIN_REQUEST, + $authException + ); + + $this->subscriber->onKernelException($event); + + $response = $event->getResponse(); + $this->assertInstanceOf(JsonResponse::class, $response); + $this->assertEquals(403, $response->getStatusCode()); + + $data = json_decode($response->getContent(), true); + $this->assertEquals('access_denied', $data['error']); + $this->assertEquals('Access denied.', $data['message']); + } + public function testOnKernelExceptionWithOtherException(): void { $exception = new Exception('Some other error'); diff --git a/yarn.lock b/yarn.lock index af7b1cf..2b374b3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2119,10 +2119,10 @@ postcss "^8.5.6" tailwindcss "4.2.1" -"@tatevikgr/rest-api-client@^2.1.8": - version "2.1.9" - resolved "https://registry.yarnpkg.com/@tatevikgr/rest-api-client/-/rest-api-client-2.1.9.tgz#72dea37787fc31543a5f7a448b1e387e64c36466" - integrity sha512-4/9h3Bg2yRYiICMhr5uX6ZoRnnD/gNE/cFb7w1IycekEBjWaTyfsz4C4MB724gvWRPwUTRxxoqox3XoEW3xgHQ== +"@tatevikgr/rest-api-client@^2.1.15": + version "2.1.15" + resolved "https://registry.yarnpkg.com/@tatevikgr/rest-api-client/-/rest-api-client-2.1.15.tgz#5bf87fbaac2b4a8c0d34d478045c113da14f4756" + integrity sha512-HSzpfG5bSsZpEu/gSeiyDzSCpk3+BsIxp9F8zJOTgwoWGPiuJoyQqUs1oE+Md4okqvEt+ZwJKpsgnOYzcDiXhw== dependencies: axios "^1.6.0"