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