PluginProbe
ThinkRank AI SEO – AI SEO Plugin for WordPress: Schema, XML Sitemaps, Meta Tags, Search Console & Local SEO / 2.14.2
ThinkRank AI SEO – AI SEO Plugin for WordPress: Schema, XML Sitemaps, Meta Tags, Search Console & Local SEO v2.14.2
2.14.2 2.14.1 2.14.0 2.13.0 2.12.0 2.11.0 2.10.0 2.9.0 2.8.0 2.7.0 2.6.0 2.5.0 2.4.0 2.3.0 2.2.0 2.1.1 2.1.0 2.0.2 2.0.1 2.0.0 1.32.0 1.31.0 1.30.0 1.29.0 1.28.0 All 57 releases
← All changes | includes/api/class-settings-management-endpoint.php +353 -26 1.31.0 → 2.14.2 View file →
@@ -23,8 +23,9 @@
23 23 use ThinkRank\SEO\Content_Optimization_Manager;
24 24 use ThinkRank\SEO\Schema_Management_System;
25 25 use ThinkRank\SEO\Social_Meta_Manager;
26 26 use ThinkRank\SEO\Sitemap_Generator;
27 +use ThinkRank\SEO\Analytics_Manager;
27 28 use WP_REST_Controller;
28 29 use WP_REST_Request;
29 30 use WP_REST_Response;
30 31 use WP_Error;
@@ -92,10 +93,15 @@
92 93 'social_media' => 'Social Media & Open Graph',
93 94 'sitemap' => 'XML Sitemap Management',
94 95 'integrations' => 'External Integrations',
95 96 'analytics_integration' => 'Analytics Integration',
96 - 'seo_analytics' => 'SEO Analytics & Intelligence',
97 - 'global_defaults' => 'Global Default Settings'
97 + 'seo_analytics' => 'SEO Analytics & Intelligence'
98 + // 'global_defaults' was listed here but is registered in no settings
99 + // store and read by no client — the only mention in the codebase was
100 + // this label. Every save against it reached the compound write with
101 + // nothing to persist to and answered 500, so accepting the name only
102 + // promised a category that could never be stored. It now falls through
103 + // to the 400 invalid_category branch like any other unknown name (#371).
98 104 ];
99 105
100 106 /**
101 107 * Constructor
@@ -116,9 +122,14 @@
116 122 */
117 123 private array $seo_manager_classes = [
118 124 'site_identity' => Site_Identity_Manager::class,
119 125 'performance_monitoring' => Performance_Monitoring_Manager::class,
120 - 'ai_content_analyzer' => AI_Content_Analyzer::class,
126 + // Keyed by the endpoint's own category name. It was 'ai_content_analyzer',
127 + // which appears in no other registry, so the route rejected it with 400
128 + // invalid_category and this manager was never reachable — while the
129 + // endpoint's actual category, 'content_analysis', had no manager and
130 + // therefore nowhere to persist (#371).
131 + 'content_analysis' => AI_Content_Analyzer::class,
121 132 'content_optimization' => Content_Optimization_Manager::class,
122 133 'schema_management' => Schema_Management_System::class,
123 134 'social_media' => Social_Meta_Manager::class,
124 135 'sitemap' => Sitemap_Generator::class,
@@ -149,8 +160,39 @@
149 160 return $this->seo_managers[$category];
150 161 }
151 162
152 163 /**
164 + * Read a category from whichever store actually owns it.
165 + *
166 + * The generic store returns `[]` for the eight SEO categories: it looks for
167 + * rows whose key carries a `<category>_` prefix, and the rows carry no such
168 + * prefix — `social_media` is stored as `social_meta`, `schema_management` as
169 + * `schema_management_system`, and their keys are bare (`og_site_name`). So a
170 + * direct `Settings_Manager::get_settings()` reports a configured site as
171 + * having no settings at all.
172 + *
173 + * Writes never had the problem, because the write path already falls back to
174 + * the owning manager. That asymmetry is what made this invisible from the UI
175 + * and dangerous underneath it: the pre-reset rollback snapshotted `[]` and
176 + * then defaults were written over live settings, so Reset could not be undone
177 + * (#689). Every read goes through here now, so there is one place to be wrong.
178 + *
179 + * @since 2.7.0
180 + *
181 + * @param string $category Category key.
182 + * @param string $context_type Optional. Context type. Default 'site'.
183 + * @param int|null $context_id Optional. Context ID.
184 + * @return array The category's stored settings.
185 + */
186 + private function read_category(string $category, string $context_type = 'site', ?int $context_id = null): array {
187 + if ($this->has_seo_manager($category)) {
188 + return (array) $this->get_seo_manager($category)->get_settings($context_type, $context_id);
189 + }
190 +
191 + return (array) $this->settings_manager->get_settings($category, $context_type, $context_id);
192 + }
193 +
194 + /**
153 195 * Register API routes
154 196 *
155 197 * @since 1.0.0
156 198 */
@@ -321,8 +363,9 @@
321 363 'openai_api_key',
322 364 'claude_api_key',
323 365 'gemini_api_key',
324 366 'openrouter_api_key',
367 + 'openai_compatible_api_key',
325 368 'google_analytics_api_key',
326 369 'google_search_console_api_key',
327 370 'google_pagespeed_api_key',
328 371 'google_access_token',
@@ -405,8 +448,56 @@
405 448 *
406 449 * @param array $settings Flat key => value map from the request.
407 450 * @return array Map with masked secret values removed.
408 451 */
452 + /**
453 + * Drop setting keys the category does not define.
454 + *
455 + * The known set is whatever describes the category: the generic store's key
456 + * list, and the dedicated manager's default settings when one owns it.
457 + * Fails open — if neither store can describe the category there is nothing
458 + * to check against, and silently dropping everything would be worse than
459 + * storing an unknown key.
460 + *
461 + * @since 2.0.1
462 + *
463 + * @param array $settings Incoming settings.
464 + * @param string $category Settings category.
465 + * @param string $context_type Context the write is scoped to.
466 + * @return array Settings limited to recognised keys.
467 + */
468 + private function filter_known_setting_keys(array $settings, string $category, string $context_type): array {
469 + $known = [];
470 +
471 + // $this->setting_categories maps category => label; the key lists live
472 + // in the generic store.
473 + $known = array_merge($known, $this->settings_manager->get_category_keys($category));
474 +
475 + if ($this->has_seo_manager($category)) {
476 + $known = array_merge(
477 + $known,
478 + array_keys($this->get_seo_manager($category)->get_default_settings($context_type))
479 + );
480 + }
481 +
482 + /**
483 + * Filter the setting keys a category accepts.
484 + *
485 + * @since 2.0.1
486 + *
487 + * @param string[] $known Recognised setting keys.
488 + * @param string $category Settings category.
489 + * @param string $context_type Context the write is scoped to.
490 + */
491 + $known = apply_filters('thinkrank_known_setting_keys', $known, $category, $context_type);
492 +
493 + if (empty($known)) {
494 + return $settings;
495 + }
496 +
497 + return array_intersect_key($settings, array_flip($known));
498 + }
499 +
409 500 private function strip_masked_secrets(array $settings): array {
410 501 foreach ($settings as $key => $value) {
411 502 if (!in_array($key, self::SENSITIVE_SETTING_KEYS, true)) {
412 503 continue;
@@ -438,10 +529,10 @@
438 529 if (!isset($this->setting_categories[$category])) {
439 530 continue;
440 531 }
441 532
442 - // Get settings for each category using Settings Manager
443 - $category_settings = $this->settings_manager->get_settings($category);
533 + // Get settings for each category from the store that owns it.
534 + $category_settings = $this->read_category($category);
444 535 $global_settings[$category] = $category_settings;
445 536
446 537 // Get schema if requested
447 538 if ($include_schema && $this->has_seo_manager($category)) {
@@ -577,9 +668,9 @@
577 668 // Get updated settings
578 669 $updated_settings = [];
579 670 foreach (array_keys($settings) as $category) {
580 671 if (isset($this->setting_categories[$category])) {
581 - $updated_settings[$category] = $this->settings_manager->get_settings($category);
672 + $updated_settings[$category] = $this->read_category($category);
582 673 }
583 674 }
584 675
585 676 return new WP_REST_Response([
@@ -624,9 +715,9 @@
624 715 );
625 716 }
626 717
627 718 // Get category settings
628 - $category_settings = $this->settings_manager->get_settings($category);
719 + $category_settings = $this->read_category($category);
629 720
630 721 // Get schema if requested
631 722 $schema = [];
632 723 if ($include_schema && $this->has_seo_manager($category)) {
@@ -701,11 +792,41 @@
701 792 ['status' => 400]
702 793 );
703 794 }
704 795
796 + // SECURITY: this route also accepts an object context and forwards it
797 + // to the category's SEO manager, which upserts rows keyed by that ID.
798 + // The `thinkrank_settings` capability authorises entry to the Settings
799 + // section — it is not authorisation to edit every post on the site — so
800 + // resolve and authorise the object before ANY write happens below (#367).
801 + $context_type = $request->get_param('context_type') ?? 'site';
802 + $context_id = $request->get_param('context_id');
803 + $context_id = null === $context_id ? null : (int) $context_id;
804 +
805 + $context_error = $this->authorize_settings_context($context_type, $context_id);
806 + if (is_wp_error($context_error)) {
807 + return $context_error;
808 + }
809 +
705 810 // Reads mask secrets; never persist a mask back over the real one.
706 811 $settings = $this->strip_masked_secrets($settings);
707 812
813 + // Drop keys the category does not define. This route persisted any
814 + // key it was handed — a probe key written through it is still
815 + // readable in the settings table afterwards — which bloats the
816 + // store and lets a client invent settings the plugin will never
817 + // read (#395). Mirrors the same guard on the schema and
818 + // social-media routes.
819 + $settings = $this->filter_known_setting_keys($settings, $category, $context_type);
820 +
821 + if (empty($settings)) {
822 + return new WP_Error(
823 + 'invalid_settings',
824 + "No recognized settings were provided for category: {$category}",
825 + ['status' => 400]
826 + );
827 + }
828 +
708 829 $validation_result = ['valid' => true];
709 830
710 831 // Validate settings if requested
711 832 if ($validate_before_update && $this->has_seo_manager($category)) {
@@ -723,19 +844,34 @@
723 844 );
724 845 }
725 846 }
726 847
727 - // Update settings
728 - $update_success = $this->settings_manager->update_settings($settings, $category);
848 + // Update settings. The context must be forwarded: update_settings()
849 + // defaults to the 'site' context, so a post-scoped request was also
850 + // silently rewriting the site-wide defaults (#367).
851 + $generic_update = $this->settings_manager->update_settings($settings, $category, $context_type, $context_id);
852 + $manager_update = null;
729 853
730 - // Also update through specific SEO manager if available
854 + // Also update through specific SEO manager if available. The context was
855 + // resolved and authorised above.
731 856 if ($this->has_seo_manager($category)) {
732 - $context_type = $request->get_param('context_type') ?? 'site';
733 - $context_id = $request->get_param('context_id') ?? null;
734 857 $manager_update = $this->get_seo_manager($category)->save_settings($context_type, $context_id, $settings);
735 - $update_success = $update_success && $manager_update;
736 858 }
737 859
860 + // null from a store means "this category is not mine", not "the write
861 + // failed" — the two registries use different category vocabularies, so
862 + // most categories are owned by exactly one store (#371). Judge only the
863 + // stores that actually attempted a write: the save succeeded if at least
864 + // one store owned the category and none of the owners failed. ANDing the
865 + // raw values reported 500 for every category the generic store does not
866 + // know, while the dedicated manager's row had already committed.
867 + $attempted = array_filter(
868 + [$generic_update, $manager_update],
869 + static fn($result) => null !== $result
870 + );
871 +
872 + $update_success = [] !== $attempted && !in_array(false, $attempted, true);
873 +
738 874 if (!$update_success) {
739 875 // Name the settings that did not persist. The write is not
740 876 // transactional, so "failed" can mean some keys saved and others
741 877 // did not — without the list the UI can only show a generic
@@ -741,8 +877,41 @@
741 877 // did not — without the list the UI can only show a generic
742 878 // error and the user has no idea what to re-enter (#300).
743 879 $failed_keys = $this->settings_manager->get_last_failed_keys();
744 880
881 + // Report which store failed. Collapsing both writes into one boolean
882 + // meant a committed manager row could be reported as a total failure,
883 + // hiding a persisted change behind a 500 (#367). Only a literal false
884 + // is a failure — null means the store does not own this category and
885 + // never attempted a write, so it must not be named here (#371).
886 + $stores_failed = [];
887 + if (false === $generic_update) {
888 + $stores_failed[] = 'settings';
889 + }
890 + if (false === $manager_update) {
891 + $stores_failed[] = 'category_manager';
892 + }
893 +
894 + // No store owns the category. That is a routing defect rather than a
895 + // failed write, and it is worth distinguishing: the settings were
896 + // never persisted anywhere, so reporting it as a plain write failure
897 + // would send the user back to re-enter values that have nowhere to go.
898 + if ([] === $attempted) {
899 + return new WP_Error(
900 + 'category_not_persistable',
901 + sprintf(
902 + 'No settings store is registered for category %s, so nothing was saved.',
903 + $category
904 + ),
905 + [
906 + 'status' => 500,
907 + 'failed_keys' => $failed_keys,
908 + 'stores_failed' => $stores_failed,
909 + 'partial_write' => false,
910 + ]
911 + );
912 + }
913 +
745 914 return new WP_Error(
746 915 'update_failed',
747 916 empty($failed_keys)
748 917 ? "Failed to update settings for category: {$category}"
@@ -753,8 +922,14 @@
753 922 ),
754 923 [
755 924 'status' => 500,
756 925 'failed_keys' => $failed_keys,
926 + 'stores_failed' => $stores_failed,
927 + // True when more than one store attempted the write and they
928 + // disagreed, so the client knows the request was not a clean
929 + // no-op. Stores that did not own the category are excluded.
930 + 'partial_write' => in_array(true, $attempted, true)
931 + && in_array(false, $attempted, true),
757 932 ]
758 933 );
759 934 }
760 935
@@ -759,11 +934,12 @@
759 934 }
760 935
761 936 // Clear analytics cache when GSC/GA settings change so fresh data is fetched
762 937 if ($category === 'seo_analytics') {
938 + foreach (Analytics_Manager::dashboard_cache_keys() as $cache_key) {
939 + delete_transient($cache_key);
940 + }
763 941 foreach (['7d', '30d', '90d'] as $range) {
764 - delete_transient("analytics_dashboard_v5_{$range}");
765 - delete_transient("seo_opportunities_{$range}");
766 942 delete_transient("seo_insights_{$range}");
767 943 }
768 944 delete_transient('indexing_status');
769 945 }
@@ -770,10 +946,20 @@
770 946
771 947 // Update category metadata
772 948 $this->update_category_metadata($category);
773 949
774 - // Get updated settings
775 - $updated_settings = $this->settings_manager->get_settings($category);
950 + // Get updated settings. Read them back from whichever store actually
951 + // owns the category: the generic store returns [] for the categories it
952 + // does not know, which would report a successful save as zero settings
953 + // and hand the UI an empty form to render (#371).
954 + //
955 + // Which store *accepted the write* is the wrong question to ask here,
956 + // and `sitemap` is the case that proves it: the generic store claims
957 + // that write (update_settings() returns true, not null) and then reads
958 + // the category back as [], so keying off $generic_update sent the one
959 + // read path that had been fixed straight back into the empty store.
960 + // Ownership is a property of the category, not of the last write (#689).
961 + $updated_settings = $this->read_category($category, $context_type, $context_id);
776 962
777 963 return new WP_REST_Response([
778 964 'success' => true,
779 965 'data' => [
@@ -950,9 +1136,9 @@
950 1136 if (!isset($this->setting_categories[$category])) {
951 1137 continue;
952 1138 }
953 1139
954 - $export_data[$category] = $this->settings_manager->get_settings($category);
1140 + $export_data[$category] = $this->read_category($category);
955 1141 }
956 1142
957 1143 // Never let secrets (API keys, OAuth tokens) leave the site in an
958 1144 // export file — strip them entirely.
@@ -1088,9 +1274,9 @@
1088 1274 }
1089 1275
1090 1276 try {
1091 1277 // Check if settings exist and handle overwrite
1092 - $existing_settings = $this->settings_manager->get_settings($category);
1278 + $existing_settings = $this->read_category($category);
1093 1279
1094 1280 if (!empty($existing_settings) && !$overwrite_existing) {
1095 1281 $import_results[$category] = [
1096 1282 'success' => false,
@@ -1163,9 +1349,9 @@
1163 1349 // Create backup data
1164 1350 $backup_data = [];
1165 1351 foreach ($categories as $category) {
1166 1352 if (isset($this->setting_categories[$category])) {
1167 - $backup_data[$category] = $this->settings_manager->get_settings($category);
1353 + $backup_data[$category] = $this->read_category($category);
1168 1354 }
1169 1355 }
1170 1356
1171 1357 // Create backup metadata
@@ -1466,15 +1652,49 @@
1466 1652 * @since 1.0.0
1467 1653 *
1468 1654 * @return bool Permission status
1469 1655 */
1470 - public function check_read_permissions(): bool {
1656 + public function check_read_permissions(WP_REST_Request $request): bool {
1471 1657 // Plugin SEO/AI config is not subscriber-visible — require the same
1472 - // management capability as the write routes.
1473 - return \ThinkRank\Core\Capability_Manager::current_user_can('thinkrank_settings');
1658 + // management capability as the write routes, resolved per category so a
1659 + // role granted one section can reach that section and no other (#573).
1660 + return \ThinkRank\Core\Capability_Manager::current_user_can(
1661 + $this->capability_for_request($request)
1662 + );
1474 1663 }
1475 1664
1476 1665 /**
1666 + * The capability a settings-management request requires.
1667 + *
1668 + * Category routes belong to the section owning the category; every other
1669 + * route on this controller is plugin-wide configuration and stays on
1670 + * `thinkrank_settings`. The gate in Role_Manager::gate_rest() reaches the
1671 + * same answer through Capability_Manager::capability_for_route() — both are
1672 + * kept so neither layer alone is load-bearing.
1673 + *
1674 + * @since 2.1.3
1675 + *
1676 + * @param WP_REST_Request $request Request.
1677 + * @return string
1678 + */
1679 + private function capability_for_request(WP_REST_Request $request): string {
1680 + // URL params only. get_param() searches the JSON body, the POST body
1681 + // and the query string ahead of the route path, so on the routes that
1682 + // declare no {category} — /global, /validate, /schema, /export,
1683 + // /backup, /restore — it read pure caller input and let a request
1684 + // nominate the capability it would be checked against (#582). Reading
1685 + // the path is also what Role_Manager::gate_rest() does, so the two
1686 + // layers now agree and the claim above is true again.
1687 + $category = $request->get_url_params()['category'] ?? null;
1688 +
1689 + if (!is_string($category) || '' === $category) {
1690 + return 'thinkrank_settings';
1691 + }
1692 +
1693 + return \ThinkRank\Core\Capability_Manager::capability_for_settings_category($category);
1694 + }
1695 +
1696 + /**
1477 1697 * Check permissions for managing settings
1478 1698 *
1479 1699 * @since 1.0.0
1480 1700 *
@@ -1479,10 +1699,12 @@
1479 1699 * @since 1.0.0
1480 1700 *
1481 1701 * @return bool Permission status
1482 1702 */
1483 - public function check_manage_permissions(): bool {
1484 - return \ThinkRank\Core\Capability_Manager::current_user_can('thinkrank_settings');
1703 + public function check_manage_permissions(WP_REST_Request $request): bool {
1704 + return \ThinkRank\Core\Capability_Manager::current_user_can(
1705 + $this->capability_for_request($request)
1706 + );
1485 1707 }
1486 1708
1487 1709 /**
1488 1710 * Check permissions for administrator-only settings operations.
@@ -1761,13 +1983,100 @@
1761 1983 'required' => false,
1762 1984 'type' => 'boolean',
1763 1985 'default' => true,
1764 1986 'description' => 'Whether to validate settings before updating'
1987 + ],
1988 + // Declared so the REST schema validates/normalises them. They were read
1989 + // by the handler while undeclared, which skipped validation entirely (#367).
1990 + 'context_type' => [
1991 + 'required' => false,
1992 + 'type' => 'string',
1993 + 'enum' => ['site', 'post', 'page', 'product'],
1994 + 'default' => 'site',
1995 + 'description' => 'Object context these settings apply to'
1996 + ],
1997 + 'context_id' => [
1998 + 'required' => false,
1999 + 'type' => 'integer',
2000 + 'minimum' => 1,
2001 + 'description' => 'Object ID when context_type is not "site"'
1765 2002 ]
1766 2003 ];
1767 2004 }
1768 2005
1769 2006 /**
2007 + * Authorise the object context a category settings write targets.
2008 + *
2009 + * The Settings section capability is delegatable, so a non-administrator can
2010 + * reach this controller. Writing settings for a specific post is an edit of
2011 + * that post and must be authorised as one — mirroring the per-object check the
2012 + * social-media write route performs (#277, #367).
2013 + *
2014 + * @since 1.32.0
2015 + *
2016 + * @param string $context_type Requested context type.
2017 + * @param int|null $context_id Requested object ID.
2018 + * @return true|WP_Error True when the write is allowed, WP_Error otherwise.
2019 + */
2020 + private function authorize_settings_context(string $context_type, ?int $context_id) {
2021 + if ('site' === $context_type) {
2022 + return true;
2023 + }
2024 +
2025 + if (!in_array($context_type, ['post', 'page', 'product'], true)) {
2026 + return new WP_Error(
2027 + 'invalid_context',
2028 + 'Invalid context type provided',
2029 + ['status' => 400]
2030 + );
2031 + }
2032 +
2033 + if (!$context_id || $context_id <= 0) {
2034 + return new WP_Error(
2035 + 'invalid_context',
2036 + 'A valid context_id is required for non-site contexts',
2037 + ['status' => 400]
2038 + );
2039 + }
2040 +
2041 + $post = get_post($context_id);
2042 +
2043 + if (!$post || 'revision' === $post->post_type) {
2044 + return new WP_Error(
2045 + 'invalid_context',
2046 + 'The requested content could not be found',
2047 + ['status' => 404]
2048 + );
2049 + }
2050 +
2051 + // The declared context must match the one the front-end read path derives
2052 + // from the real post type, otherwise `page`/`product` can alias an arbitrary
2053 + // object and the row is written where nothing will ever read it. Mirrors
2054 + // Seo_Manager::get_context_type() — custom post types fall back to 'post'.
2055 + $expected_context = in_array($post->post_type, ['post', 'page', 'product'], true)
2056 + ? $post->post_type
2057 + : 'post';
2058 +
2059 + if ($context_type !== $expected_context) {
2060 + return new WP_Error(
2061 + 'invalid_context',
2062 + 'The context type does not match the requested content.',
2063 + ['status' => 400]
2064 + );
2065 + }
2066 +
2067 + if (!current_user_can('edit_post', $context_id)) {
2068 + return new WP_Error(
2069 + 'rest_forbidden',
2070 + 'You are not allowed to edit settings for this content.',
2071 + ['status' => 403]
2072 + );
2073 + }
2074 +
2075 + return true;
2076 + }
2077 +
2078 + /**
1770 2079 * Get arguments for validation endpoint
1771 2080 *
1772 2081 * @since 1.0.0
1773 2082 *
@@ -1966,9 +2275,27 @@
1966 2275 private function create_settings_snapshot(array $categories, string $label): string {
1967 2276 $backup_data = [];
1968 2277 foreach ($categories as $category) {
1969 2278 if (isset($this->setting_categories[$category])) {
1970 - $backup_data[$category] = $this->settings_manager->get_settings($category);
2279 + $backup_data[$category] = $this->read_category($category);
2280 + }
2281 + }
2282 +
2283 + // A snapshot that captured nothing for a category that does hold settings
2284 + // is worse than no snapshot: reset checks only that an id came back, so an
2285 + // empty one is accepted as a rollback point and the defaults go over live
2286 + // data that can no longer be recovered. That is exactly what #689 was.
2287 + //
2288 + // Ask the owning manager directly rather than trusting read_category(),
2289 + // so this stays a real check if a future edit sends a read back to the
2290 + // wrong store instead of quietly agreeing with it.
2291 + foreach ($backup_data as $category => $captured) {
2292 + if (!empty($captured) || !$this->has_seo_manager($category)) {
2293 + continue;
2294 + }
2295 +
2296 + if (!empty((array) $this->get_seo_manager($category)->get_settings('site', null))) {
2297 + return '';
1971 2298 }
1972 2299 }
1973 2300
1974 2301 $backup_metadata = [