diff --git a/CHANGELOG.md b/CHANGELOG.md index 60ecc4b7..b4b7b670 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed +- **C-1 OauthController**: Added CSRF nonce validation to OAuth callback — session-based nonce is generated during `authorize()`, embedded in the state parameter, and verified in `callback()` to prevent CSRF attacks +- **C-2 DispatchController**: Added POST method enforcement — rejects non-POST requests with 405 status +- **C-5 ServiceModel**: Credential form fields (`cred_*`) are now collected into the `credentials` JSON column on save, and expanded back into individual fields on load — previously these fields were silently discarded +- **H-1 Event pattern**: Fixed Joomla 5 SubscriberInterface incompatibility where `onMokoJoomCrossGetServices` by-reference pattern silently lost all service plugins — dispatchers now read plugin instances from Event ArrayAccess indices after dispatch +- **H-4 ServiceTable**: Added `check()` method with alias generation, required field validation (title, service_type), timestamp management, and JSON defaults for credentials/params +- **H-9 WebhookService**: Fixed credential key mismatch — `publish()` and `validateCredentials()` now use keys matching the service.xml form fields (`url`, `method`, `auth_type`, `bearer_token`, `basic_username`, `basic_password`, `content_type`) and properly apply Bearer/Basic auth headers +- **M-4 ServiceIconHelper**: Escaped `$extraClass` parameter in `renderIcon()` with `htmlspecialchars()` to prevent XSS +- **M-5 Content plugin**: Fixed double-escaped HTML in cross-post history panel — uses `setFieldAttribute()` to inject history HTML into the note field description after XML load, avoiding XML attribute encoding + ### Added - **ServiceIconHelper**: Centralised icon mapping for all 34 service types — replaces per-template icon arrays with `ServiceIconHelper::getIcon()` / `::renderIcon()` - **Service Stats drill-down**: New `servicestats` view with per-service analytics — post counts, success rate, daily trend chart, recent posts table, and top articles list diff --git a/src/packages/com_mokojoomcross/src/Controller/DispatchController.php b/src/packages/com_mokojoomcross/src/Controller/DispatchController.php index dc08f6e4..64f52778 100644 --- a/src/packages/com_mokojoomcross/src/Controller/DispatchController.php +++ b/src/packages/com_mokojoomcross/src/Controller/DispatchController.php @@ -33,6 +33,9 @@ use Joomla\Component\MokoJoomCross\Administrator\Service\MokoJoomCrossServiceInt * } * * Returns JSON with the created post IDs and status. + * + * Authentication is handled by Joomla's API application (token or session). + * The webservices plugin routes POST requests here via the API router. */ class DispatchController extends BaseController { @@ -45,6 +48,13 @@ class DispatchController extends BaseController { $app = $this->app; + // Enforce POST method — this is a state-changing action endpoint + if (strtoupper($this->input->getMethod()) !== 'POST') { + $this->sendJsonResponse(['error' => 'Method not allowed. Use POST.'], 405); + + return; + } + // Read JSON body $input = json_decode(file_get_contents('php://input'), true) ?: []; $articleId = (int) ($input['article_id'] ?? 0); @@ -104,20 +114,29 @@ class DispatchController extends BaseController return; } - // Import service plugins and build type-to-plugin map + // Import service plugins and build type-to-plugin map. + // In Joomla 5+ with SubscriberInterface, plugins receive the Event object + // as their first argument. When they do $services[] = $this, they append to + // the Event via ArrayAccess at numeric indices starting at 1. PluginHelper::importPlugin('mokojoomcross'); $servicePlugins = []; + $event = new \Joomla\Event\Event('onMokoJoomCrossGetServices', [$servicePlugins]); try { - $app->getDispatcher()->dispatch( - 'onMokoJoomCrossGetServices', - new \Joomla\Event\Event('onMokoJoomCrossGetServices', [&$servicePlugins]) - ); + $app->getDispatcher()->dispatch('onMokoJoomCrossGetServices', $event); } catch (\Throwable $e) { // Dispatcher may not be available } + // Read plugins back from the Event's ArrayAccess indices + $idx = 1; + + while (isset($event[$idx])) { + $servicePlugins[] = $event[$idx]; + $idx++; + } + $pluginMap = []; foreach ($servicePlugins as $plugin) { diff --git a/src/packages/com_mokojoomcross/src/Controller/OauthController.php b/src/packages/com_mokojoomcross/src/Controller/OauthController.php index 915884c5..884b556b 100644 --- a/src/packages/com_mokojoomcross/src/Controller/OauthController.php +++ b/src/packages/com_mokojoomcross/src/Controller/OauthController.php @@ -13,6 +13,7 @@ namespace Joomla\Component\MokoJoomCross\Administrator\Controller; defined('_JEXEC') or die; +use Joomla\CMS\Factory; use Joomla\CMS\Language\Text; use Joomla\CMS\MVC\Controller\BaseController; use Joomla\CMS\Plugin\PluginHelper; @@ -84,7 +85,11 @@ class OauthController extends BaseController return; } - $url = OAuthHelper::getAuthorizeUrl($service->service_type, $serviceId, $clientId); + // Generate CSRF nonce and store in session + $nonce = bin2hex(random_bytes(16)); + Factory::getApplication()->getSession()->set('mokojoomcross.oauth_nonce', $nonce); + + $url = OAuthHelper::getAuthorizeUrl($service->service_type, $serviceId, $clientId, $nonce); if (!$url) { $this->setRedirect( @@ -133,6 +138,7 @@ class OauthController extends BaseController $stateData = json_decode(base64_decode($state), true); $serviceId = (int) ($stateData['service_id'] ?? 0); $serviceType = $stateData['type'] ?? ''; + $stateNonce = $stateData['nonce'] ?? ''; if (!$serviceId || !$serviceType) { $this->setRedirect( @@ -144,6 +150,21 @@ class OauthController extends BaseController return; } + // CSRF nonce validation — compare state nonce against session + $session = Factory::getApplication()->getSession(); + $sessionNonce = $session->get('mokojoomcross.oauth_nonce', ''); + $session->clear('mokojoomcross.oauth_nonce'); + + if (empty($stateNonce) || !hash_equals($sessionNonce, $stateNonce)) { + $this->setRedirect( + Route::_('index.php?option=com_mokojoomcross&view=services', false), + Text::_('COM_MOKOJOOMCROSS_OAUTH_INVALID_STATE'), + 'error' + ); + + return; + } + // Get client credentials from plugin params PluginHelper::importPlugin('mokojoomcross'); $pluginParams = PluginHelper::getPlugin('mokojoomcross', $serviceType); diff --git a/src/packages/com_mokojoomcross/src/Helper/CrossPostDispatcher.php b/src/packages/com_mokojoomcross/src/Helper/CrossPostDispatcher.php index 83237a66..bff7ef19 100644 --- a/src/packages/com_mokojoomcross/src/Helper/CrossPostDispatcher.php +++ b/src/packages/com_mokojoomcross/src/Helper/CrossPostDispatcher.php @@ -56,12 +56,26 @@ class CrossPostDispatcher // Import service plugins so they register with the dispatcher PluginHelper::importPlugin('mokojoomcross'); - // Collect registered service plugin instances + // Collect registered service plugin instances. + // In Joomla 5+ with SubscriberInterface, plugins receive the Event object + // as their first argument. When they do $services[] = $this, they append to + // the Event via ArrayAccess at numeric indices starting at 1. $servicePlugins = []; - Factory::getApplication()->getDispatcher()->dispatch( - 'onMokoJoomCrossGetServices', - new \Joomla\Event\Event('onMokoJoomCrossGetServices', [&$servicePlugins]) - ); + $event = new \Joomla\Event\Event('onMokoJoomCrossGetServices', [$servicePlugins]); + + try { + Factory::getApplication()->getDispatcher()->dispatch('onMokoJoomCrossGetServices', $event); + } catch (\Throwable $e) { + // Dispatcher may not be available in all contexts + } + + // Read plugins back from the Event's ArrayAccess indices + $idx = 1; + + while (isset($event[$idx])) { + $servicePlugins[] = $event[$idx]; + $idx++; + } // Index by service type for lookup $pluginMap = []; diff --git a/src/packages/com_mokojoomcross/src/Helper/OAuthHelper.php b/src/packages/com_mokojoomcross/src/Helper/OAuthHelper.php index 863018ae..66c9c278 100644 --- a/src/packages/com_mokojoomcross/src/Helper/OAuthHelper.php +++ b/src/packages/com_mokojoomcross/src/Helper/OAuthHelper.php @@ -61,7 +61,7 @@ class OAuthHelper * * @return string|null Authorization URL or null if not supported */ - public static function getAuthorizeUrl(string $serviceType, int $serviceId, string $clientId): ?string + public static function getAuthorizeUrl(string $serviceType, int $serviceId, string $clientId, string $nonce = ''): ?string { $config = self::OAUTH_CONFIGS[$serviceType] ?? null; @@ -70,7 +70,13 @@ class OAuthHelper } $redirectUri = self::getCallbackUrl(); - $state = base64_encode(json_encode(['service_id' => $serviceId, 'type' => $serviceType])); + $statePayload = ['service_id' => $serviceId, 'type' => $serviceType]; + + if (!empty($nonce)) { + $statePayload['nonce'] = $nonce; + } + + $state = base64_encode(json_encode($statePayload)); $params = [ 'client_id' => $clientId, diff --git a/src/packages/com_mokojoomcross/src/Helper/QueueProcessor.php b/src/packages/com_mokojoomcross/src/Helper/QueueProcessor.php index 09680c54..db06b938 100644 --- a/src/packages/com_mokojoomcross/src/Helper/QueueProcessor.php +++ b/src/packages/com_mokojoomcross/src/Helper/QueueProcessor.php @@ -342,19 +342,9 @@ class QueueProcessor return $result; } - // Load the system plugin for template rendering - PluginHelper::importPlugin('system'); - $systemPlugin = null; - - try { - $plugins = []; - Factory::getApplication()->getDispatcher()->dispatch( - 'onMokoJoomCrossGetServices', - new \Joomla\Event\Event('onMokoJoomCrossGetServices', [&$plugins]) - ); - } catch (\Throwable $e) { - // Not critical for queuing - } + // Import service plugins (not used for direct dispatch here, but ensures + // they are loaded in case any lifecycle events depend on them) + PluginHelper::importPlugin('mokojoomcross'); foreach ($articles as $article) { if ($result['queued'] >= $maxPerRun) { @@ -687,17 +677,29 @@ class QueueProcessor { PluginHelper::importPlugin('mokojoomcross'); + // In Joomla 5+ with SubscriberInterface, plugins receive the Event object + // as their first argument. When they do $services[] = $this, they append to + // the Event via ArrayAccess at numeric indices starting at 1. $servicePlugins = []; + $event = new \Joomla\Event\Event('onMokoJoomCrossGetServices', [$servicePlugins]); try { Factory::getApplication()->getDispatcher()->dispatch( 'onMokoJoomCrossGetServices', - new \Joomla\Event\Event('onMokoJoomCrossGetServices', [&$servicePlugins]) + $event ); } catch (\Throwable $e) { // Dispatcher may not be available in all contexts } + // Read plugins back from the Event's ArrayAccess indices + $idx = 1; + + while (isset($event[$idx])) { + $servicePlugins[] = $event[$idx]; + $idx++; + } + $map = []; foreach ($servicePlugins as $plugin) { diff --git a/src/packages/com_mokojoomcross/src/Helper/ServiceIconHelper.php b/src/packages/com_mokojoomcross/src/Helper/ServiceIconHelper.php index b04c0cea..6f67e479 100644 --- a/src/packages/com_mokojoomcross/src/Helper/ServiceIconHelper.php +++ b/src/packages/com_mokojoomcross/src/Helper/ServiceIconHelper.php @@ -89,7 +89,7 @@ class ServiceIconHelper public static function renderIcon(string $serviceType, string $extraClass = ''): string { $icon = self::getIcon($serviceType); - $class = trim($icon . ' ' . $extraClass); + $class = trim($icon . ' ' . htmlspecialchars($extraClass, ENT_QUOTES, 'UTF-8')); return ''; } diff --git a/src/packages/com_mokojoomcross/src/Model/ServiceModel.php b/src/packages/com_mokojoomcross/src/Model/ServiceModel.php index 1e715118..4c4ebd97 100644 --- a/src/packages/com_mokojoomcross/src/Model/ServiceModel.php +++ b/src/packages/com_mokojoomcross/src/Model/ServiceModel.php @@ -13,6 +13,8 @@ namespace Joomla\Component\MokoJoomCross\Administrator\Model; defined('_JEXEC') or die; +use Joomla\CMS\Factory; +use Joomla\CMS\Filter\OutputFilter; use Joomla\CMS\MVC\Model\AdminModel; class ServiceModel extends AdminModel @@ -43,12 +45,77 @@ class ServiceModel extends AdminModel /** * Method to get the data that should be injected in the form. * + * Expands the JSON credentials column back into individual cred_* form fields + * so they are populated when editing an existing service. + * * @return mixed The data for the form */ protected function loadFormData() { $data = $this->getItem(); + if ($data && !empty($data->credentials)) { + $credentials = json_decode($data->credentials, true) ?: []; + $serviceType = $data->service_type ?? ''; + + foreach ($credentials as $key => $value) { + // Map credential keys back to form field names. + // The mode field has no service type prefix. + if ($key === 'mode') { + $data->cred_mode = $value; + } else { + $data->{'cred_' . $serviceType . '_' . $key} = $value; + } + } + } + return $data; } + + /** + * Override save to collect cred_* form fields into the credentials JSON column. + * + * The service form has individual fields (cred_twitter_api_key, cred_facebook_page_id, etc.) + * but the database stores them as a single JSON blob in the `credentials` column. + * + * @param array $data The form data + * + * @return boolean True on success + */ + public function save($data) + { + $serviceType = $data['service_type'] ?? ''; + $credentials = []; + $credPrefix = 'cred_'; + + // Collect all cred_* fields into the credentials array + foreach ($data as $key => $value) { + if (strpos($key, $credPrefix) !== 0) { + continue; + } + + $credKey = substr($key, strlen($credPrefix)); + + // The mode field is shared across service types (no service_type prefix) + if ($credKey === 'mode') { + $credentials['mode'] = $value; + } elseif ($serviceType && strpos($credKey, $serviceType . '_') === 0) { + // Strip the service_type prefix: cred_twitter_api_key -> api_key + $strippedKey = substr($credKey, strlen($serviceType) + 1); + $credentials[$strippedKey] = $value; + } + } + + // Store the credentials JSON + $data['credentials'] = !empty($credentials) ? json_encode($credentials) : '{}'; + + // Remove individual cred_* fields so they don't cause column-not-found errors + foreach (array_keys($data) as $key) { + if (strpos($key, $credPrefix) === 0) { + unset($data[$key]); + } + } + + return parent::save($data); + } } diff --git a/src/packages/com_mokojoomcross/src/Table/ServiceTable.php b/src/packages/com_mokojoomcross/src/Table/ServiceTable.php index c89fc645..300d1878 100644 --- a/src/packages/com_mokojoomcross/src/Table/ServiceTable.php +++ b/src/packages/com_mokojoomcross/src/Table/ServiceTable.php @@ -13,6 +13,9 @@ namespace Joomla\Component\MokoJoomCross\Administrator\Table; defined('_JEXEC') or die; +use Joomla\CMS\Factory; +use Joomla\CMS\Filter\OutputFilter; +use Joomla\CMS\Language\Text; use Joomla\CMS\Table\Table; use Joomla\Database\DatabaseDriver; @@ -22,4 +25,67 @@ class ServiceTable extends Table { parent::__construct('#__mokojoomcross_services', 'id', $db); } + + /** + * Validate the record before storing. + * + * Generates alias from title if empty, validates required fields, + * sets created/modified timestamps. + * + * @return boolean True if the record is valid + */ + public function check(): bool + { + // Title is required + if (empty($this->title)) { + $this->setError(Text::_('COM_MOKOJOOMCROSS_ERROR_TITLE_REQUIRED')); + + return false; + } + + // Service type is required + if (empty($this->service_type)) { + $this->setError(Text::_('COM_MOKOJOOMCROSS_ERROR_SERVICE_TYPE_REQUIRED')); + + return false; + } + + // Generate alias from title if empty + if (empty($this->alias)) { + $this->alias = $this->title; + } + + $this->alias = OutputFilter::stringURLSafe($this->alias); + + // Make sure alias is unique + if (empty($this->alias)) { + $this->alias = Factory::getDate()->format('Y-m-d-H-i-s'); + } + + // Set timestamps + $now = Factory::getDate()->toSql(); + + if (empty($this->created)) { + $this->created = $now; + } + + $this->modified = $now; + + // Set created_by if not set + if (empty($this->created_by)) { + $this->created_by = Factory::getApplication()->getIdentity()->id ?? 0; + } + + // Ensure credentials is valid JSON + if (empty($this->credentials)) { + $this->credentials = '{}'; + } + + // Ensure params is valid JSON + if (empty($this->params)) { + $this->params = '{}'; + } + + return true; + } } diff --git a/src/packages/plg_content_mokojoomcross/src/Extension/MokoJoomCrossContent.php b/src/packages/plg_content_mokojoomcross/src/Extension/MokoJoomCrossContent.php index 01656703..31241dba 100644 --- a/src/packages/plg_content_mokojoomcross/src/Extension/MokoJoomCrossContent.php +++ b/src/packages/plg_content_mokojoomcross/src/Extension/MokoJoomCrossContent.php @@ -186,13 +186,19 @@ XML; $historyHtml .= ''; + // Add the note field first with an empty description, then set the + // description via setFieldAttribute() to avoid double-escaping. + // Putting raw HTML into an XML attribute via htmlspecialchars() causes + // Joomla's note field renderer to display escaped tags since it outputs + // the description as raw HTML. $historyXml = '
+ description="" />
'; $form->load($historyXml); + $form->setFieldAttribute('mokojoomcross_history', 'description', $historyHtml, 'attribs'); } } } diff --git a/src/packages/plg_mokojoomcross_webhook/src/Extension/WebhookService.php b/src/packages/plg_mokojoomcross_webhook/src/Extension/WebhookService.php index e438eb30..4767e16e 100644 --- a/src/packages/plg_mokojoomcross_webhook/src/Extension/WebhookService.php +++ b/src/packages/plg_mokojoomcross_webhook/src/Extension/WebhookService.php @@ -41,19 +41,20 @@ class WebhookService extends CMSPlugin implements SubscriberInterface, MokoJoomC public function publish(string $message, array $media, array $credentials, array $params): array { - $url = $credentials['webhook_url'] ?? $credentials['webhook_url'] ?? ''; + // Credential keys match the service.xml form field names (after stripping cred_webhook_ prefix): + // url, method, auth_type, bearer_token, basic_username, basic_password, content_type + $url = $credentials['url'] ?? ''; if (empty($url)) { return ['success' => false, 'platform_post_id' => '', 'response' => ['error' => 'Missing webhook URL']]; } $method = $credentials['method'] ?? 'POST'; - $headers = $credentials['headers'] ?? []; - $format = $credentials['body_format'] ?? 'json'; + $format = $credentials['content_type'] ?? 'json'; $payload = [ 'title' => $params['title'] ?? '', - 'url' => $params['url'] ?? '', + 'url' => $params['_article_url'] ?? $params['url'] ?? '', 'message' => $message, 'image' => $params['image'] ?? '', 'category' => $params['category'] ?? '', @@ -63,18 +64,23 @@ class WebhookService extends CMSPlugin implements SubscriberInterface, MokoJoomC $httpHeaders = ['Content-Type: application/json']; - if (is_array($headers)) { - foreach ($headers as $k => $v) { - $httpHeaders[] = "$k: $v"; - } - } - $body = ($format === 'form') ? http_build_query($payload) : json_encode($payload); if ($format === 'form') { $httpHeaders[0] = 'Content-Type: application/x-www-form-urlencoded'; } + // Apply authentication based on auth_type + $authType = $credentials['auth_type'] ?? 'none'; + + if ($authType === 'bearer' && !empty($credentials['bearer_token'])) { + $httpHeaders[] = 'Authorization: Bearer ' . $credentials['bearer_token']; + } elseif ($authType === 'basic' && !empty($credentials['basic_username'])) { + $httpHeaders[] = 'Authorization: Basic ' . base64_encode( + $credentials['basic_username'] . ':' . ($credentials['basic_password'] ?? '') + ); + } + $ch = curl_init($url); curl_setopt_array($ch, [ CURLOPT_CUSTOMREQUEST => $method, @@ -99,10 +105,10 @@ class WebhookService extends CMSPlugin implements SubscriberInterface, MokoJoomC public function validateCredentials(array $credentials): array { - $key = $credentials['webhook_url'] ?? $credentials['webhook_url'] ?? ''; + $url = $credentials['url'] ?? ''; - if (empty($key)) { - return ['valid' => false, 'message' => 'Missing credentials', 'account_name' => '']; + if (empty($url)) { + return ['valid' => false, 'message' => 'Missing webhook URL', 'account_name' => '']; } return ['valid' => true, 'message' => 'Credentials configured', 'account_name' => 'Generic Webhook'];