| @@ -52,8 +52,23 @@ | ||
| 52 | 52 | */ |
| 53 | 53 | protected $rest_base = 'integrations'; |
| 54 | 54 | |
| 55 | 55 | /** |
| 56 | + * Transient holding the verified Search Console property list. | |
| 57 | + * | |
| 58 | + * Deliberately a fixed key rather than one namespaced per account: a purge | |
| 59 | + * has to be possible from paths that have already cleared the credentials | |
| 60 | + * (disconnect, revoke), and those can no longer derive an account-specific | |
| 61 | + * key. The account is instead fingerprinted inside the payload and checked | |
| 62 | + * on read, so a cache written by one Google account can never be served to | |
| 63 | + * another even if a purge is missed. | |
| 64 | + * | |
| 65 | + * @since 1.28.0 | |
| 66 | + * @var string | |
| 67 | + */ | |
| 68 | + public const SITES_CACHE_KEY = 'thinkrank_gsc_sites_list'; | |
| 69 | + | |
| 70 | + /** | |
| 56 | 71 | * Settings instance |
| 57 | 72 | * |
| 58 | 73 | * @since 1.0.0 |
| 59 | 74 | * @var Settings |
| @@ -87,9 +102,9 @@ | ||
| 87 | 102 | ], |
| 88 | 103 | [ |
| 89 | 104 | 'methods' => 'POST', |
| 90 | 105 | 'callback' => [$this, 'update_settings'], |
| 91 | - 'permission_callback' => [$this, 'check_manage_permissions'], | |
| 106 | + 'permission_callback' => [$this, 'check_credential_permissions'], | |
| 92 | 107 | 'args' => $this->get_settings_args() |
| 93 | 108 | ] |
| 94 | 109 | ] |
| 95 | 110 | ); |
| @@ -101,51 +116,13 @@ | ||
| 101 | 116 | [ |
| 102 | 117 | [ |
| 103 | 118 | 'methods' => 'POST', |
| 104 | 119 | 'callback' => [$this, 'test_connections'], |
| 105 | - 'permission_callback' => [$this, 'check_manage_permissions'] | |
| 120 | + 'permission_callback' => [$this, 'check_credential_permissions'] | |
| 106 | 121 | ] |
| 107 | 122 | ] |
| 108 | 123 | ); |
| 109 | 124 | |
| 110 | - // Verify GA4 tracking | |
| 111 | - register_rest_route( | |
| 112 | - $this->namespace, | |
| 113 | - '/' . $this->rest_base . '/verify-ga4-tracking', | |
| 114 | - [ | |
| 115 | - [ | |
| 116 | - 'methods' => 'POST', | |
| 117 | - 'callback' => [$this, 'verify_ga4_tracking'], | |
| 118 | - 'permission_callback' => [$this, 'check_manage_permissions'], | |
| 119 | - 'args' => [ | |
| 120 | - 'measurement_id' => [ | |
| 121 | - 'required' => true, | |
| 122 | - 'type' => 'string', | |
| 123 | - // No regex delimiters — WP's REST validator wraps the | |
| 124 | - // pattern in its own (#...#u), so a leading/trailing | |
| 125 | - // slash would require literal slashes in the value. | |
| 126 | - 'pattern' => '^G-[A-Z0-9]{10}$', | |
| 127 | - 'sanitize_callback' => 'sanitize_text_field', | |
| 128 | - 'description' => 'GA4 Measurement ID in format G-XXXXXXXXXX' | |
| 129 | - ] | |
| 130 | - ] | |
| 131 | - ] | |
| 132 | - ] | |
| 133 | - ); | |
| 134 | - | |
| 135 | - // Detect GA4 conflicts | |
| 136 | - register_rest_route( | |
| 137 | - $this->namespace, | |
| 138 | - '/' . $this->rest_base . '/detect-ga4-conflicts', | |
| 139 | - [ | |
| 140 | - [ | |
| 141 | - 'methods' => 'GET', | |
| 142 | - 'callback' => [$this, 'detect_ga4_conflicts'], | |
| 143 | - 'permission_callback' => [$this, 'check_read_permissions'] | |
| 144 | - ] | |
| 145 | - ] | |
| 146 | - ); | |
| 147 | - | |
| 148 | 125 | // Get Search Console Sites |
| 149 | 126 | register_rest_route( |
| 150 | 127 | $this->namespace, |
| 151 | 128 | '/' . $this->rest_base . '/search-console/sites', |
| @@ -152,9 +129,16 @@ | ||
| 152 | 129 | [ |
| 153 | 130 | [ |
| 154 | 131 | 'methods' => 'GET', |
| 155 | 132 | 'callback' => [$this, 'get_search_console_sites'], |
| 156 | - 'permission_callback' => [$this, 'check_manage_permissions'] | |
| 133 | + 'permission_callback' => [$this, 'check_manage_permissions'], | |
| 134 | + 'args' => [ | |
| 135 | + 'refresh' => [ | |
| 136 | + 'description' => 'Bypass the cached property list and re-query Google.', | |
| 137 | + 'type' => 'boolean', | |
| 138 | + 'default' => false, | |
| 139 | + ], | |
| 140 | + ], | |
| 157 | 141 | ] |
| 158 | 142 | ] |
| 159 | 143 | ); |
| 160 | 144 | |
| @@ -161,9 +145,9 @@ | ||
| 161 | 145 | // Disconnect Google Account |
| 162 | 146 | register_rest_route($this->namespace, '/integrations/google/disconnect', [ |
| 163 | 147 | 'methods' => WP_REST_Server::CREATABLE, |
| 164 | 148 | 'callback' => [$this, 'disconnect_google_account'], |
| 165 | - 'permission_callback' => [$this, 'check_manage_permissions'] // Changed to check_manage_permissions for consistency | |
| 149 | + 'permission_callback' => [$this, 'check_credential_permissions'] | |
| 166 | 150 | ]); |
| 167 | 151 | |
| 168 | 152 | // Note: there is no save-google-token route. Tokens are swapped |
| 169 | 153 | // server-to-server in Google_OAuth_Proxy and never pass through the |
| @@ -307,16 +291,8 @@ | ||
| 307 | 291 | $settings['google_analytics_api_key'] = $this->settings->get('google_analytics_api_key'); |
| 308 | 292 | $settings['google_search_console_api_key'] = $this->settings->get('google_search_console_api_key'); |
| 309 | 293 | $settings['google_pagespeed_api_key'] = $this->settings->get('google_pagespeed_api_key'); |
| 310 | 294 | |
| 311 | - // Get GA4 tracking settings (let Settings class handle defaults) | |
| 312 | - $settings['ga4_measurement_id'] = $this->settings->get('ga4_measurement_id'); | |
| 313 | - $settings['ga4_auto_inject'] = $this->settings->get('ga4_auto_inject'); | |
| 314 | - $settings['ga4_anonymize_ip'] = $this->settings->get('ga4_anonymize_ip'); | |
| 315 | - $settings['ga4_exclude_admin'] = $this->settings->get('ga4_exclude_admin'); | |
| 316 | - $settings['ga4_tracking_verified'] = $this->settings->get('ga4_tracking_verified'); | |
| 317 | - $settings['ga4_last_verification'] = $this->settings->get('ga4_last_verification'); | |
| 318 | - | |
| 319 | 295 | // Get other integration settings (let Settings class handle defaults) |
| 320 | 296 | $settings['api_timeout'] = $this->settings->get('api_timeout'); |
| 321 | 297 | $settings['enable_rate_limiting'] = $this->settings->get('enable_rate_limiting'); |
| 322 | 298 | $settings['cache_duration'] = $this->settings->get('cache_duration'); |
| @@ -372,25 +348,33 @@ | ||
| 372 | 348 | $sanitized[$key_field] = sanitize_text_field($settings[$key_field]); |
| 373 | 349 | } |
| 374 | 350 | } |
| 375 | 351 | |
| 376 | - // Sanitize numeric settings | |
| 377 | - $sanitized['api_timeout'] = absint($settings['api_timeout'] ?? 30); | |
| 378 | - $sanitized['cache_duration'] = absint($settings['cache_duration'] ?? 3600); | |
| 352 | + // A key the payload never mentioned is left alone rather than being | |
| 353 | + // reset to a hard-coded default. These fallbacks used to fire on every | |
| 354 | + // save, so a partial payload — or a setting the admin has no control | |
| 355 | + // for, like retry_failed_requests — silently reverted to the default a | |
| 356 | + // site owner had deliberately changed in code (#297). | |
| 357 | + $numeric = ['api_timeout', 'cache_duration']; | |
| 379 | 358 | |
| 380 | - // Sanitize GA4 tracking settings | |
| 381 | - $sanitized['ga4_measurement_id'] = sanitize_text_field($settings['ga4_measurement_id'] ?? ''); | |
| 382 | - $sanitized['ga4_auto_inject'] = isset($settings['ga4_auto_inject']) ? (bool) $settings['ga4_auto_inject'] : false; | |
| 383 | - $sanitized['ga4_anonymize_ip'] = isset($settings['ga4_anonymize_ip']) ? (bool) $settings['ga4_anonymize_ip'] : false; | |
| 384 | - $sanitized['ga4_exclude_admin'] = isset($settings['ga4_exclude_admin']) ? (bool) $settings['ga4_exclude_admin'] : false; | |
| 385 | - $sanitized['ga4_tracking_verified'] = isset($settings['ga4_tracking_verified']) ? (bool) $settings['ga4_tracking_verified'] : false; | |
| 386 | - $sanitized['ga4_last_verification'] = sanitize_text_field($settings['ga4_last_verification'] ?? ''); | |
| 359 | + foreach ($numeric as $key) { | |
| 360 | + if (array_key_exists($key, $settings)) { | |
| 361 | + $sanitized[$key] = absint($settings[$key]); | |
| 362 | + } | |
| 363 | + } | |
| 387 | 364 | |
| 388 | - // Sanitize boolean settings | |
| 389 | - $sanitized['enable_rate_limiting'] = isset($settings['enable_rate_limiting']) ? (bool) $settings['enable_rate_limiting'] : true; | |
| 390 | - $sanitized['auto_test_connections'] = isset($settings['auto_test_connections']) ? (bool) $settings['auto_test_connections'] : true; | |
| 391 | - $sanitized['retry_failed_requests'] = isset($settings['retry_failed_requests']) ? (bool) $settings['retry_failed_requests'] : true; | |
| 365 | + $booleans = [ | |
| 366 | + 'enable_rate_limiting', | |
| 367 | + 'auto_test_connections', | |
| 368 | + 'retry_failed_requests', | |
| 369 | + ]; | |
| 392 | 370 | |
| 371 | + foreach ($booleans as $key) { | |
| 372 | + if (array_key_exists($key, $settings)) { | |
| 373 | + $sanitized[$key] = (bool) $settings[$key]; | |
| 374 | + } | |
| 375 | + } | |
| 376 | + | |
| 393 | 377 | return $sanitized; |
| 394 | 378 | } |
| 395 | 379 | |
| 396 | 380 | /** |
| @@ -771,9 +755,9 @@ | ||
| 771 | 755 | * @since 1.0.0 |
| 772 | 756 | * @param WP_REST_Request $request Request object |
| 773 | 757 | * @return WP_REST_Response|WP_Error Response object |
| 774 | 758 | */ |
| 775 | - public function disconnect_google_account(WP_REST_Request $request): WP_REST_Response|WP_Error { | |
| 759 | + public function disconnect_google_account(WP_REST_Request $request) { | |
| 776 | 760 | try { |
| 777 | 761 | // Best-effort revoke at Google so the refresh token (which never |
| 778 | 762 | // auto-expires) can't keep querying on the admin's behalf after |
| 779 | 763 | // disconnect. Failure here must not block local cleanup. |
| @@ -792,11 +776,22 @@ | ||
| 792 | 776 | $this->settings->set('google_refresh_token', ''); |
| 793 | 777 | $this->settings->set('google_token_expires_in', ''); |
| 794 | 778 | $this->settings->set('google_token_created', ''); |
| 795 | 779 | $this->settings->set('google_account_connected', false); |
| 796 | - // Also clear site selection | |
| 797 | - $this->settings->set('google_search_console_site', ''); | |
| 780 | + // Also clear site selection. This targeted `google_search_console_site`, | |
| 781 | + // which is not a declared setting — Settings::set() rejects unknown | |
| 782 | + // keys, so the line never cleared anything and the selection | |
| 783 | + // survived every disconnect. The property picker writes | |
| 784 | + // `search_console_property`; clear that and the GA4 property beside | |
| 785 | + // it, so a reconnect under a different Google account doesn't | |
| 786 | + // inherit the previous account's selections. | |
| 787 | + $this->settings->set('search_console_property', ''); | |
| 788 | + $this->settings->set('seo_analytics_google_analytics_property_id', ''); | |
| 798 | 789 | |
| 790 | + // The cached property list belongs to the account we just dropped — | |
| 791 | + // leaving it would serve those properties to whoever connects next. | |
| 792 | + self::purge_search_console_sites_cache(); | |
| 793 | + | |
| 799 | 794 | // A deliberate disconnect is not a forced re-authorization. |
| 800 | 795 | delete_option('thinkrank_google_reconnect_required'); |
| 801 | 796 | |
| 802 | 797 | return new WP_REST_Response([ |
| @@ -849,83 +844,56 @@ | ||
| 849 | 844 | return \ThinkRank\Core\Capability_Manager::current_user_can('thinkrank_settings'); |
| 850 | 845 | } |
| 851 | 846 | |
| 852 | 847 | /** |
| 853 | - * Verify GA4 tracking | |
| 854 | - * Following ThinkRank API response patterns | |
| 848 | + * Check permissions for credential-managing operations. | |
| 855 | 849 | * |
| 856 | - * @since 1.0.0 | |
| 857 | - * @param WP_REST_Request $request Request object | |
| 858 | - * @return WP_REST_Response|WP_Error Response object | |
| 850 | + * Writing provider API keys, disconnecting Google (a server-side token | |
| 851 | + * revoke) and running live connection tests manage the site's third-party | |
| 852 | + * credentials, so they require an administrator — `thinkrank_settings` is | |
| 853 | + * delegatable to non-admin roles through the Role Manager. Mirrors the | |
| 854 | + * pattern used by the AI-insights settings writes. | |
| 855 | + * | |
| 856 | + * @since 1.29.0 | |
| 857 | + * @return bool Permission status | |
| 859 | 858 | */ |
| 860 | - public function verify_ga4_tracking(WP_REST_Request $request): WP_REST_Response|WP_Error { | |
| 861 | - try { | |
| 862 | - $measurement_id = $request->get_param('measurement_id'); | |
| 859 | + public function check_credential_permissions(): bool { | |
| 860 | + return current_user_can('manage_options'); | |
| 861 | + } | |
| 863 | 862 | |
| 864 | - if (empty($measurement_id)) { | |
| 865 | - return new WP_Error( | |
| 866 | - 'missing_measurement_id', | |
| 867 | - 'Measurement ID is required', | |
| 868 | - ['status' => 400] | |
| 869 | - ); | |
| 870 | - } | |
| 863 | + /** | |
| 864 | + * Fingerprint the currently connected Google account. | |
| 865 | + * | |
| 866 | + * Prefers the refresh token: it is issued once per authorization grant and | |
| 867 | + * survives every access-token rotation, so the cache stays warm for a whole | |
| 868 | + * connection but changes the moment a different account authorizes. Falls | |
| 869 | + * back to the access token when no refresh token was granted, which merely | |
| 870 | + * shortens the effective cache life to one token lifetime. | |
| 871 | + * | |
| 872 | + * @since 1.28.0 | |
| 873 | + * @return string Non-reversible fingerprint, empty string when disconnected. | |
| 874 | + */ | |
| 875 | + private function get_google_account_fingerprint(): string { | |
| 876 | + $token = $this->settings->get('google_refresh_token', '') | |
| 877 | + ?: $this->settings->get('google_access_token', ''); | |
| 871 | 878 | |
| 872 | - // Load tracking manager | |
| 873 | - if (!class_exists('ThinkRank\\Frontend\\Google_Analytics_Tracking_Manager')) { | |
| 874 | - require_once THINKRANK_PLUGIN_DIR . 'includes/frontend/class-google-analytics-tracking-manager.php'; | |
| 875 | - } | |
| 876 | - | |
| 877 | - $tracking_manager = new \ThinkRank\Frontend\Google_Analytics_Tracking_Manager(); | |
| 878 | - $verification_result = $tracking_manager->verify_tracking($measurement_id); | |
| 879 | - | |
| 880 | - return new WP_REST_Response([ | |
| 881 | - 'success' => true, | |
| 882 | - 'data' => $verification_result, | |
| 883 | - 'message' => 'Tracking verification completed' | |
| 884 | - ], 200); | |
| 885 | - } catch (\Exception $e) { | |
| 886 | - return new WP_Error( | |
| 887 | - 'verification_failed', | |
| 888 | - 'Tracking verification failed: ' . $e->getMessage(), | |
| 889 | - ['status' => 500] | |
| 890 | - ); | |
| 891 | - } | |
| 879 | + return empty($token) ? '' : md5((string) $token); | |
| 892 | 880 | } |
| 893 | 881 | |
| 894 | 882 | /** |
| 895 | - * Detect GA4 conflicts | |
| 896 | - * Following ThinkRank API response patterns | |
| 883 | + * Drop the cached Search Console property list. | |
| 897 | 884 | * |
| 898 | - * @since 1.0.0 | |
| 899 | - * @param WP_REST_Request $request Request object | |
| 900 | - * @return WP_REST_Response|WP_Error Response object | |
| 885 | + * Public and static so the OAuth paths — which run outside this controller | |
| 886 | + * and after the credentials are gone — can invalidate the list on connect, | |
| 887 | + * disconnect and revoke. | |
| 888 | + * | |
| 889 | + * @since 1.28.0 | |
| 890 | + * @return void | |
| 901 | 891 | */ |
| 902 | - public function detect_ga4_conflicts(WP_REST_Request $request): WP_REST_Response|WP_Error { | |
| 903 | - try { | |
| 904 | - // Load tracking manager | |
| 905 | - if (!class_exists('ThinkRank\\Frontend\\Google_Analytics_Tracking_Manager')) { | |
| 906 | - require_once THINKRANK_PLUGIN_DIR . 'includes/frontend/class-google-analytics-tracking-manager.php'; | |
| 907 | - } | |
| 892 | + public static function purge_search_console_sites_cache(): void { | |
| 893 | + delete_transient(self::SITES_CACHE_KEY); | |
| 894 | + } | |
| 908 | 895 | |
| 909 | - $tracking_manager = new \ThinkRank\Frontend\Google_Analytics_Tracking_Manager(); | |
| 910 | - $conflicts = $tracking_manager->detect_existing_tracking(); | |
| 911 | - | |
| 912 | - return new WP_REST_Response([ | |
| 913 | - 'success' => true, | |
| 914 | - 'data' => [ | |
| 915 | - 'conflicts' => $conflicts, | |
| 916 | - 'has_conflicts' => !empty($conflicts) | |
| 917 | - ], | |
| 918 | - 'message' => 'Conflict detection completed' | |
| 919 | - ], 200); | |
| 920 | - } catch (\Exception $e) { | |
| 921 | - return new WP_Error( | |
| 922 | - 'conflict_detection_failed', | |
| 923 | - 'Conflict detection failed: ' . $e->getMessage(), | |
| 924 | - ['status' => 500] | |
| 925 | - ); | |
| 926 | - } | |
| 927 | - } | |
| 928 | 896 | /** |
| 929 | 897 | * Get Search Console Sites |
| 930 | 898 | * |
| 931 | 899 | * @since 1.0.0 |
| @@ -930,20 +898,13 @@ | ||
| 930 | 898 | * |
| 931 | 899 | * @since 1.0.0 |
| 932 | 900 | * @param WP_REST_Request $request Request object |
| 933 | 901 | * @return WP_REST_Response Response object |
| 902 | + * | |
| 903 | + * @throws \Exception On failure. | |
| 934 | 904 | */ |
| 935 | 905 | public function get_search_console_sites(WP_REST_Request $request): WP_REST_Response { |
| 936 | 906 | try { |
| 937 | - // The verified-sites list changes rarely but costs a live Google | |
| 938 | - // round-trip — serve from a 30-minute transient so the Google | |
| 939 | - // Services screen doesn't hit Google on every render. | |
| 940 | - $sites_cache_key = 'thinkrank_gsc_sites_list'; | |
| 941 | - $cached_sites = get_transient($sites_cache_key); | |
| 942 | - if (is_array($cached_sites)) { | |
| 943 | - return new WP_REST_Response($cached_sites, 200); | |
| 944 | - } | |
| 945 | - | |
| 946 | 907 | // Ensure Analytics_Manager is loaded for proactive token refresh |
| 947 | 908 | if (!class_exists('ThinkRank\\SEO\\Analytics_Manager')) { |
| 948 | 909 | require_once THINKRANK_PLUGIN_DIR . 'includes/seo/class-analytics-manager.php'; |
| 949 | 910 | } |
| @@ -961,8 +922,26 @@ | ||
| 961 | 922 | 'message' => 'Google account not connected' |
| 962 | 923 | ], 401); |
| 963 | 924 | } |
| 964 | 925 | |
| 926 | + // The verified-sites list changes rarely but costs a live Google | |
| 927 | + // round-trip — serve from a 30-minute transient so the Google | |
| 928 | + // Services screen doesn't hit Google on every render. The cache is | |
| 929 | + // only honoured for the account that wrote it: reconnecting as a | |
| 930 | + // different Google account must never hand back the previous | |
| 931 | + // account's properties, which reads as "my site is missing". | |
| 932 | + $account = $this->get_google_account_fingerprint(); | |
| 933 | + $cached_sites = get_transient(self::SITES_CACHE_KEY); | |
| 934 | + | |
| 935 | + if ( | |
| 936 | + !$request->get_param('refresh') | |
| 937 | + && is_array($cached_sites) | |
| 938 | + && isset($cached_sites['account'], $cached_sites['payload']) | |
| 939 | + && hash_equals($account, (string) $cached_sites['account']) | |
| 940 | + ) { | |
| 941 | + return new WP_REST_Response($cached_sites['payload'], 200); | |
| 942 | + } | |
| 943 | + | |
| 965 | 944 | // Initialize Search Console Client |
| 966 | 945 | if (!class_exists('ThinkRank\\Integrations\\Google_Search_Console_Client')) { |
| 967 | 946 | require_once THINKRANK_PLUGIN_DIR . 'includes/integrations/class-google-search-console-client.php'; |
| 968 | 947 | } |
| @@ -1034,11 +1013,21 @@ | ||
| 1034 | 1013 | ], |
| 1035 | 1014 | 'message' => 'Search Console sites retrieved successfully' |
| 1036 | 1015 | ]; |
| 1037 | 1016 | |
| 1038 | - // Cache successes only — errors must stay retryable. | |
| 1017 | + // Cache successes only — errors must stay retryable. Recompute the | |
| 1018 | + // fingerprint: the 401 retry above may have rotated the access | |
| 1019 | + // token, and the cache must be stamped with the account it came | |
| 1020 | + // from, not the one we started the request with. | |
| 1039 | 1021 | if (!empty($sites)) { |
| 1040 | - set_transient($sites_cache_key, $payload, 30 * MINUTE_IN_SECONDS); | |
| 1022 | + set_transient( | |
| 1023 | + self::SITES_CACHE_KEY, | |
| 1024 | + [ | |
| 1025 | + 'account' => $this->get_google_account_fingerprint(), | |
| 1026 | + 'payload' => $payload, | |
| 1027 | + ], | |
| 1028 | + 30 * MINUTE_IN_SECONDS | |
| 1029 | + ); | |
| 1041 | 1030 | } |
| 1042 | 1031 | |
| 1043 | 1032 | return new WP_REST_Response($payload, 200); |
| 1044 | 1033 | } catch (\Exception $e) { |
