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