← All changes
|
includes/api/class-settings-management-endpoint.php
+171
-16
2.0.0
→
2.10.0
View file →
| @@ -159,8 +159,39 @@ | ||
| 159 | 159 | return $this->seo_managers[$category]; |
| 160 | 160 | } |
| 161 | 161 | |
| 162 | 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 | + /** | |
| 163 | 194 | * Register API routes |
| 164 | 195 | * |
| 165 | 196 | * @since 1.0.0 |
| 166 | 197 | */ |
| @@ -331,8 +362,9 @@ | ||
| 331 | 362 | 'openai_api_key', |
| 332 | 363 | 'claude_api_key', |
| 333 | 364 | 'gemini_api_key', |
| 334 | 365 | 'openrouter_api_key', |
| 366 | + 'openai_compatible_api_key', | |
| 335 | 367 | 'google_analytics_api_key', |
| 336 | 368 | 'google_search_console_api_key', |
| 337 | 369 | 'google_pagespeed_api_key', |
| 338 | 370 | 'google_access_token', |
| @@ -415,8 +447,56 @@ | ||
| 415 | 447 | * |
| 416 | 448 | * @param array $settings Flat key => value map from the request. |
| 417 | 449 | * @return array Map with masked secret values removed. |
| 418 | 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 | + | |
| 419 | 499 | private function strip_masked_secrets(array $settings): array { |
| 420 | 500 | foreach ($settings as $key => $value) { |
| 421 | 501 | if (!in_array($key, self::SENSITIVE_SETTING_KEYS, true)) { |
| 422 | 502 | continue; |
| @@ -448,10 +528,10 @@ | ||
| 448 | 528 | if (!isset($this->setting_categories[$category])) { |
| 449 | 529 | continue; |
| 450 | 530 | } |
| 451 | 531 | |
| 452 | - // Get settings for each category using Settings Manager | |
| 453 | - $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); | |
| 454 | 534 | $global_settings[$category] = $category_settings; |
| 455 | 535 | |
| 456 | 536 | // Get schema if requested |
| 457 | 537 | if ($include_schema && $this->has_seo_manager($category)) { |
| @@ -587,9 +667,9 @@ | ||
| 587 | 667 | // Get updated settings |
| 588 | 668 | $updated_settings = []; |
| 589 | 669 | foreach (array_keys($settings) as $category) { |
| 590 | 670 | if (isset($this->setting_categories[$category])) { |
| 591 | - $updated_settings[$category] = $this->settings_manager->get_settings($category); | |
| 671 | + $updated_settings[$category] = $this->read_category($category); | |
| 592 | 672 | } |
| 593 | 673 | } |
| 594 | 674 | |
| 595 | 675 | return new WP_REST_Response([ |
| @@ -634,9 +714,9 @@ | ||
| 634 | 714 | ); |
| 635 | 715 | } |
| 636 | 716 | |
| 637 | 717 | // Get category settings |
| 638 | - $category_settings = $this->settings_manager->get_settings($category); | |
| 718 | + $category_settings = $this->read_category($category); | |
| 639 | 719 | |
| 640 | 720 | // Get schema if requested |
| 641 | 721 | $schema = []; |
| 642 | 722 | if ($include_schema && $this->has_seo_manager($category)) { |
| @@ -728,8 +808,24 @@ | ||
| 728 | 808 | |
| 729 | 809 | // Reads mask secrets; never persist a mask back over the real one. |
| 730 | 810 | $settings = $this->strip_masked_secrets($settings); |
| 731 | 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 | + | |
| 732 | 828 | $validation_result = ['valid' => true]; |
| 733 | 829 | |
| 734 | 830 | // Validate settings if requested |
| 735 | 831 | if ($validate_before_update && $this->has_seo_manager($category)) { |
| @@ -852,11 +948,16 @@ | ||
| 852 | 948 | // Get updated settings. Read them back from whichever store actually |
| 853 | 949 | // owns the category: the generic store returns [] for the categories it |
| 854 | 950 | // does not know, which would report a successful save as zero settings |
| 855 | 951 | // and hand the UI an empty form to render (#371). |
| 856 | - $updated_settings = null === $generic_update && $this->has_seo_manager($category) | |
| 857 | - ? $this->get_seo_manager($category)->get_settings($context_type, $context_id) | |
| 858 | - : $this->settings_manager->get_settings($category, $context_type, $context_id); | |
| 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); | |
| 859 | 960 | |
| 860 | 961 | return new WP_REST_Response([ |
| 861 | 962 | 'success' => true, |
| 862 | 963 | 'data' => [ |
| @@ -1033,9 +1134,9 @@ | ||
| 1033 | 1134 | if (!isset($this->setting_categories[$category])) { |
| 1034 | 1135 | continue; |
| 1035 | 1136 | } |
| 1036 | 1137 | |
| 1037 | - $export_data[$category] = $this->settings_manager->get_settings($category); | |
| 1138 | + $export_data[$category] = $this->read_category($category); | |
| 1038 | 1139 | } |
| 1039 | 1140 | |
| 1040 | 1141 | // Never let secrets (API keys, OAuth tokens) leave the site in an |
| 1041 | 1142 | // export file — strip them entirely. |
| @@ -1171,9 +1272,9 @@ | ||
| 1171 | 1272 | } |
| 1172 | 1273 | |
| 1173 | 1274 | try { |
| 1174 | 1275 | // Check if settings exist and handle overwrite |
| 1175 | - $existing_settings = $this->settings_manager->get_settings($category); | |
| 1276 | + $existing_settings = $this->read_category($category); | |
| 1176 | 1277 | |
| 1177 | 1278 | if (!empty($existing_settings) && !$overwrite_existing) { |
| 1178 | 1279 | $import_results[$category] = [ |
| 1179 | 1280 | 'success' => false, |
| @@ -1246,9 +1347,9 @@ | ||
| 1246 | 1347 | // Create backup data |
| 1247 | 1348 | $backup_data = []; |
| 1248 | 1349 | foreach ($categories as $category) { |
| 1249 | 1350 | if (isset($this->setting_categories[$category])) { |
| 1250 | - $backup_data[$category] = $this->settings_manager->get_settings($category); | |
| 1351 | + $backup_data[$category] = $this->read_category($category); | |
| 1251 | 1352 | } |
| 1252 | 1353 | } |
| 1253 | 1354 | |
| 1254 | 1355 | // Create backup metadata |
| @@ -1549,15 +1650,49 @@ | ||
| 1549 | 1650 | * @since 1.0.0 |
| 1550 | 1651 | * |
| 1551 | 1652 | * @return bool Permission status |
| 1552 | 1653 | */ |
| 1553 | - public function check_read_permissions(): bool { | |
| 1654 | + public function check_read_permissions(WP_REST_Request $request): bool { | |
| 1554 | 1655 | // Plugin SEO/AI config is not subscriber-visible — require the same |
| 1555 | - // management capability as the write routes. | |
| 1556 | - 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 | + ); | |
| 1557 | 1661 | } |
| 1558 | 1662 | |
| 1559 | 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 | + /** | |
| 1560 | 1695 | * Check permissions for managing settings |
| 1561 | 1696 | * |
| 1562 | 1697 | * @since 1.0.0 |
| 1563 | 1698 | * |
| @@ -1562,10 +1697,12 @@ | ||
| 1562 | 1697 | * @since 1.0.0 |
| 1563 | 1698 | * |
| 1564 | 1699 | * @return bool Permission status |
| 1565 | 1700 | */ |
| 1566 | - public function check_manage_permissions(): bool { | |
| 1567 | - 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 | + ); | |
| 1568 | 1705 | } |
| 1569 | 1706 | |
| 1570 | 1707 | /** |
| 1571 | 1708 | * Check permissions for administrator-only settings operations. |
| @@ -2136,9 +2273,27 @@ | ||
| 2136 | 2273 | private function create_settings_snapshot(array $categories, string $label): string { |
| 2137 | 2274 | $backup_data = []; |
| 2138 | 2275 | foreach ($categories as $category) { |
| 2139 | 2276 | if (isset($this->setting_categories[$category])) { |
| 2140 | - $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 ''; | |
| 2141 | 2296 | } |
| 2142 | 2297 | } |
| 2143 | 2298 | |
| 2144 | 2299 | $backup_metadata = [ |