diff --git a/src/wp-includes/connectors.php b/src/wp-includes/connectors.php index 9aed5f1f4d7aa..a5ba672bd402b 100644 --- a/src/wp-includes/connectors.php +++ b/src/wp-includes/connectors.php @@ -581,6 +581,10 @@ function wp_connectors_get_application_password_credentials( array $auth ): arra /** * Checks whether an API key is valid for a given provider. * + * A key is only reported as invalid when the provider rejects it. Failures that + * do not reflect on the key, such as network errors, server errors, or rate + * limiting, return null. + * * @since 7.0.0 * @access private * @@ -610,7 +614,7 @@ function _wp_connectors_is_ai_api_key_valid( string $key, string $provider_id ): new ApiKeyRequestAuthentication( $key ) ); - return $registry->isProviderConfigured( $provider_id ); + return $registry->verifyProviderCredentials( $provider_id ); } catch ( Exception $e ) { wp_trigger_error( __FUNCTION__, $e->getMessage() ); return null; @@ -683,8 +687,9 @@ function wp_connectors_sanitize_application_password_credentials( $value, string * password field of default application-password credential objects. * * On POST or PUT requests, validates each updated AI provider API key before - * masking. If validation fails, the key is reverted to an empty string. - * Application password values are masked but not validated. + * masking. If the provider rejects the key, it is reverted to an empty string. + * A key that cannot be verified, for example because the provider is + * unreachable, is kept. Application password values are masked but not validated. * * @since 7.0.0 * @access private @@ -738,7 +743,8 @@ function _wp_connectors_rest_settings_dispatch( WP_REST_Response $response, WP_R && is_string( $value ) && '' !== $value && 'ai_provider' === $connector_data['type'] ) { - if ( true !== _wp_connectors_is_ai_api_key_valid( $value, $connector_id ) ) { + // Only discard a key the provider rejected, not one that could not be verified. + if ( false === _wp_connectors_is_ai_api_key_valid( $value, $connector_id ) ) { update_option( $setting_name, '' ); $data[ $setting_name ] = ''; continue; diff --git a/src/wp-includes/php-ai-client/src/Providers/ApiBasedImplementation/ListModelsApiBasedProviderAvailability.php b/src/wp-includes/php-ai-client/src/Providers/ApiBasedImplementation/ListModelsApiBasedProviderAvailability.php index 128184e737df8..c5e18b792670b 100644 --- a/src/wp-includes/php-ai-client/src/Providers/ApiBasedImplementation/ListModelsApiBasedProviderAvailability.php +++ b/src/wp-includes/php-ai-client/src/Providers/ApiBasedImplementation/ListModelsApiBasedProviderAvailability.php @@ -4,8 +4,11 @@ namespace WordPress\AiClient\Providers\ApiBasedImplementation; use Exception; +use WordPress\AiClient\Common\Contracts\CachesDataInterface; use WordPress\AiClient\Providers\Contracts\ModelMetadataDirectoryInterface; use WordPress\AiClient\Providers\Contracts\ProviderAvailabilityInterface; +use WordPress\AiClient\Providers\Contracts\VerifiesCredentialsInterface; +use WordPress\AiClient\Providers\Http\Exception\ClientException; /** * Class to check availability for an API-based provider via a test request to the endpoint to list models. * @@ -15,7 +18,7 @@ * * @since 0.1.0 */ -class ListModelsApiBasedProviderAvailability implements ProviderAvailabilityInterface +class ListModelsApiBasedProviderAvailability implements ProviderAvailabilityInterface, VerifiesCredentialsInterface { /** * @var ModelMetadataDirectoryInterface The model metadata directory to use for checking availability. @@ -49,4 +52,27 @@ public function isConfigured(): bool return \false; } } + /** + * {@inheritDoc} + * + * The cached model list is invalidated first, as it may have been fetched with other credentials. + * + * @since n.e.x.t + */ + public function verifyCredentials(): bool + { + if ($this->modelMetadataDirectory instanceof CachesDataInterface) { + $this->modelMetadataDirectory->invalidateCaches(); + } + try { + $this->modelMetadataDirectory->listModelMetadata(); + } catch (ClientException $e) { + // A request timeout or rate limit says nothing about the credentials. + if (in_array($e->getCode(), [408, 429], \true)) { + throw $e; + } + return \false; + } + return \true; + } } diff --git a/src/wp-includes/php-ai-client/src/Providers/Contracts/VerifiesCredentialsInterface.php b/src/wp-includes/php-ai-client/src/Providers/Contracts/VerifiesCredentialsInterface.php new file mode 100644 index 0000000000000..08d647a83a38e --- /dev/null +++ b/src/wp-includes/php-ai-client/src/Providers/Contracts/VerifiesCredentialsInterface.php @@ -0,0 +1,27 @@ + $idOrClassName The provider ID or class name. + * @return bool True if the provider accepted the credentials, false if it rejected them. + * @throws InvalidArgumentException If the provider is not registered. + * @throws \Exception If the credentials could not be verified. + */ + public function verifyProviderCredentials(string $idOrClassName): bool + { + $className = $this->resolveProviderClassName($idOrClassName); + // Use static method from ProviderInterface + /** @var class-string $className */ + $availability = $className::availability(); + if ($availability instanceof VerifiesCredentialsInterface) { + return $availability->verifyCredentials(); + } + return $availability->isConfigured(); + } /** * Finds models across all available providers that support the given requirements. * diff --git a/tests/phpunit/includes/wp-ai-client-mock-provider-trait.php b/tests/phpunit/includes/wp-ai-client-mock-provider-trait.php index 42f85c212d092..aba733f46b246 100644 --- a/tests/phpunit/includes/wp-ai-client-mock-provider-trait.php +++ b/tests/phpunit/includes/wp-ai-client-mock-provider-trait.php @@ -8,13 +8,20 @@ use WordPress\AiClient\AiClient; use WordPress\AiClient\Providers\AbstractProvider; +use WordPress\AiClient\Providers\ApiBasedImplementation\AbstractApiProvider; +use WordPress\AiClient\Providers\ApiBasedImplementation\ListModelsApiBasedProviderAvailability; use WordPress\AiClient\Providers\Contracts\ModelMetadataDirectoryInterface; use WordPress\AiClient\Providers\Contracts\ProviderAvailabilityInterface; use WordPress\AiClient\Providers\DTO\ProviderMetadata; use WordPress\AiClient\Providers\Enums\ProviderTypeEnum; +use WordPress\AiClient\Providers\Http\DTO\Request; +use WordPress\AiClient\Providers\Http\DTO\Response; +use WordPress\AiClient\Providers\Http\Enums\HttpMethodEnum; use WordPress\AiClient\Providers\Http\Enums\RequestAuthenticationMethod; use WordPress\AiClient\Providers\Models\Contracts\ModelInterface; use WordPress\AiClient\Providers\Models\DTO\ModelMetadata; +use WordPress\AiClient\Providers\Models\Enums\CapabilityEnum; +use WordPress\AiClient\Providers\OpenAiCompatibleImplementation\AbstractOpenAiCompatibleModelMetadataDirectory; /** * Mock provider availability with a controllable flag. @@ -137,6 +144,111 @@ protected static function createModel( } } +/** + * Mock model metadata directory that lists models over HTTP. + * + * Built on the same base class as the official provider plugins, so requests go + * through the WP AI Client HTTP transporter and can be mocked with the + * `pre_http_request` filter. + * + * @since 7.2.0 + */ +class Mock_Connectors_Test_Http_Model_Metadata_Directory extends AbstractOpenAiCompatibleModelMetadataDirectory { + + /** + * Creates a request to the mock provider API. + * + * @param HttpMethodEnum $method The HTTP method. + * @param string $path The API path. + * @param array $headers The request headers. + * @param mixed $data The request data. + * @return Request The request. + */ + protected function createRequest( HttpMethodEnum $method, string $path, array $headers = array(), $data = null ): Request { + return new Request( $method, Mock_Connectors_Test_Http_Provider::url( $path ), $headers, $data ); + } + + /** + * Parses the list models response. + * + * @param Response $response The response. + * @return ModelMetadata[] The listed models. + */ + protected function parseResponseToModelMetadataList( Response $response ): array { + $data = $response->getData(); + $models = array(); + foreach ( $data['data'] ?? array() as $model ) { + $models[] = new ModelMetadata( $model['id'], $model['id'], array( CapabilityEnum::textGeneration() ), array() ); + } + return $models; + } +} + +/** + * Mock provider that checks its availability by listing models over HTTP, + * like the official provider plugins. + * + * @since 7.2.0 + */ +class Mock_Connectors_Test_Http_Provider extends AbstractApiProvider { + + /** + * Returns the base URL of the mock provider API. + * + * @return string + */ + protected static function baseUrl(): string { + return 'https://api.example.com/v1'; + } + + /** + * Creates the provider metadata. + * + * @return ProviderMetadata + */ + protected static function createProviderMetadata(): ProviderMetadata { + return new ProviderMetadata( + 'mock-connectors-http-test', + 'Mock Connectors HTTP Test', + ProviderTypeEnum::cloud(), + null, + RequestAuthenticationMethod::apiKey() + ); + } + + /** + * Creates the provider availability checker. + * + * @return ProviderAvailabilityInterface + */ + protected static function createProviderAvailability(): ProviderAvailabilityInterface { + return new ListModelsApiBasedProviderAvailability( static::modelMetadataDirectory() ); + } + + /** + * Creates the model metadata directory. + * + * @return ModelMetadataDirectoryInterface + */ + protected static function createModelMetadataDirectory(): ModelMetadataDirectoryInterface { + return new Mock_Connectors_Test_Http_Model_Metadata_Directory(); + } + + /** + * Creates a model instance. + * + * @param ModelMetadata $model_metadata The model metadata. + * @param ProviderMetadata $provider_metadata The provider metadata. + * @throws \RuntimeException Always, as model creation is not needed for these tests. + */ + protected static function createModel( + ModelMetadata $model_metadata, + ProviderMetadata $provider_metadata + ): ModelInterface { + throw new \RuntimeException( 'Not implemented.' ); + } +} + /** * Trait providing a mock AI provider for testing connector functions. * @@ -200,4 +312,74 @@ private static function unregister_mock_connector_setting(): void { unregister_setting( 'connectors', $setting_name ); remove_filter( "option_{$setting_name}", '_wp_connectors_mask_api_key' ); } + + /** + * How the HTTP mock provider's models endpoint responds. + * + * @var int|WP_Error HTTP status code, or a WP_Error to simulate a network failure. + */ + private $mock_models_endpoint_response = 200; + + /** + * API keys sent to the HTTP mock provider's models endpoint, in request order. + * + * @var string[] + */ + private array $mock_models_endpoint_api_keys = array(); + + /** + * Registers the HTTP mock provider in the AI Client registry. + * + * Safe to call multiple times; skips registration if already done. + * Must be called from set_up_before_class() after parent::set_up_before_class(). + */ + private static function register_mock_connectors_http_provider(): void { + $ai_registry = AiClient::defaultRegistry(); + if ( ! $ai_registry->hasProvider( 'mock-connectors-http-test' ) ) { + $ai_registry->registerProvider( Mock_Connectors_Test_Http_Provider::class ); + } + } + + /** + * Sets how the HTTP mock provider's models endpoint responds. + * + * @param int|WP_Error $response HTTP status code, or a WP_Error to simulate a network failure. + */ + private function mock_models_endpoint_response( $response ): void { + $this->mock_models_endpoint_response = $response; + add_filter( 'pre_http_request', array( $this, 'filter_mock_models_endpoint_request' ), 10, 3 ); + } + + /** + * Responds to requests to the HTTP mock provider's models endpoint. + * + * @param false|array|WP_Error $response A preemptive return value of an HTTP request. + * @param array $parsed_args HTTP request arguments. + * @param string $url The request URL. + * @return false|array|WP_Error The mocked models endpoint response, otherwise the unchanged value. + */ + public function filter_mock_models_endpoint_request( $response, $parsed_args, $url ) { + if ( Mock_Connectors_Test_Http_Provider::url( 'models' ) !== $url ) { + return $response; + } + + $this->mock_models_endpoint_api_keys[] = str_replace( 'Bearer ', '', $parsed_args['headers']['Authorization'] ?? '' ); + + if ( is_wp_error( $this->mock_models_endpoint_response ) ) { + return $this->mock_models_endpoint_response; + } + + $status = $this->mock_models_endpoint_response; + + return array( + 'headers' => array( 'content-type' => 'application/json' ), + 'body' => 200 === $status ? '{"data":[{"id":"mock-model"}]}' : '{"error":{"message":"Mock error."}}', + 'response' => array( + 'code' => $status, + 'message' => get_status_header_desc( $status ), + ), + 'cookies' => array(), + 'filename' => null, + ); + } } diff --git a/tests/phpunit/tests/connectors/wpConnectorsIsApiKeyValid.php b/tests/phpunit/tests/connectors/wpConnectorsIsApiKeyValid.php index 4ec59670d2c38..e6792b279821e 100644 --- a/tests/phpunit/tests/connectors/wpConnectorsIsApiKeyValid.php +++ b/tests/phpunit/tests/connectors/wpConnectorsIsApiKeyValid.php @@ -18,6 +18,7 @@ class Tests_Connectors_WpConnectorsIsApiKeyValid extends WP_UnitTestCase { public static function set_up_before_class() { parent::set_up_before_class(); self::register_mock_connectors_provider(); + self::register_mock_connectors_http_provider(); } /** @@ -66,4 +67,105 @@ public function test_unconfigured_provider_returns_false() { $this->assertFalse( $result ); } + + /** + * Tests that a key the provider accepts returns true. + * + * @ticket 65551 + */ + public function test_key_accepted_by_provider_returns_true() { + $this->mock_models_endpoint_response( 200 ); + + $result = _wp_connectors_is_ai_api_key_valid( 'test-key', 'mock-connectors-http-test' ); + + $this->assertTrue( $result ); + $this->assertSame( array( 'test-key' ), $this->mock_models_endpoint_api_keys, 'The key should be sent to the provider.' ); + } + + /** + * Tests that a key the provider rejects returns false. + * + * @ticket 65551 + * + * @dataProvider data_rejecting_status_codes + * + * @param int $status_code HTTP status code the provider responds with. + */ + public function test_key_rejected_by_provider_returns_false( $status_code ) { + $this->mock_models_endpoint_response( $status_code ); + + $result = _wp_connectors_is_ai_api_key_valid( 'test-key', 'mock-connectors-http-test' ); + + $this->assertFalse( $result ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_rejecting_status_codes() { + return array( + '400 Bad Request' => array( 400 ), + '401 Unauthorized' => array( 401 ), + '403 Forbidden' => array( 403 ), + ); + } + + /** + * Tests that a key that cannot be verified returns null. + * + * @ticket 65551 + * + * @dataProvider data_unverifiable_responses + * + * @param int|WP_Error $response HTTP status code, or a WP_Error for a network failure. + */ + public function test_key_that_cannot_be_verified_returns_null( $response ) { + $this->mock_models_endpoint_response( $response ); + + $errors = array(); + add_filter( 'wp_trigger_error_trigger_error', '__return_false' ); + add_action( + 'wp_trigger_error_always_run', + static function ( $function_name ) use ( &$errors ) { + $errors[] = $function_name; + } + ); + + $result = _wp_connectors_is_ai_api_key_valid( 'test-key', 'mock-connectors-http-test' ); + + $this->assertNull( $result ); + $this->assertSame( array( '_wp_connectors_is_ai_api_key_valid' ), $errors, 'The failure should be reported.' ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_unverifiable_responses() { + return array( + 'network error' => array( new WP_Error( 'http_request_failed', 'cURL error 28: Operation timed out' ) ), + '408 Request Timeout' => array( 408 ), + '429 Too Many Requests' => array( 429 ), + '500 Internal Server Error' => array( 500 ), + '503 Service Unavailable' => array( 503 ), + ); + } + + /** + * Tests that a model list cached for one key does not validate another. + * + * @ticket 65551 + */ + public function test_cached_model_list_does_not_validate_a_different_key() { + $this->mock_models_endpoint_response( 200 ); + $this->assertTrue( _wp_connectors_is_ai_api_key_valid( 'valid-key', 'mock-connectors-http-test' ) ); + + $this->mock_models_endpoint_response( 401 ); + $this->assertFalse( _wp_connectors_is_ai_api_key_valid( 'invalid-key', 'mock-connectors-http-test' ) ); + + $this->assertSame( array( 'valid-key', 'invalid-key' ), $this->mock_models_endpoint_api_keys, 'Each key should be sent to the provider.' ); + } } diff --git a/tests/phpunit/tests/connectors/wpConnectorsRestSettingsDispatch.php b/tests/phpunit/tests/connectors/wpConnectorsRestSettingsDispatch.php index eb723701731f1..2eeb6a61e5ebc 100644 --- a/tests/phpunit/tests/connectors/wpConnectorsRestSettingsDispatch.php +++ b/tests/phpunit/tests/connectors/wpConnectorsRestSettingsDispatch.php @@ -15,13 +15,16 @@ class Tests_Connectors_WpConnectorsRestSettingsDispatch extends WP_UnitTestCase const CONNECTOR_ID = 'wp_test_application_password_connector'; const CREDENTIALS_SETTING_NAME = 'connectors_test_remote_credentials'; const AI_KEY_SETTING_NAME = 'connectors_ai_mock_connectors_test_api_key'; + const HTTP_CONNECTOR_ID = 'mock-connectors-http-test'; + const HTTP_AI_KEY_SETTING_NAME = 'connectors_ai_mock_connectors_http_test_api_key'; /** - * Registers the mock AI provider connector once before any tests in this class run. + * Registers the mock AI providers once before any tests in this class run. */ public static function set_up_before_class(): void { parent::set_up_before_class(); self::register_mock_connectors_provider(); + self::register_mock_connectors_http_provider(); } /** @@ -33,7 +36,7 @@ public static function tear_down_after_class(): void { } /** - * Registers an application password connector before each test. + * Registers the test connectors before each test. */ public function set_up(): void { parent::set_up(); @@ -51,15 +54,29 @@ public function set_up(): void { ), ) ); + + WP_Connector_Registry::get_instance()->register( + self::HTTP_CONNECTOR_ID, + array( + 'name' => 'Mock Connectors HTTP Test', + 'type' => 'ai_provider', + 'authentication' => array( + 'method' => 'api_key', + 'setting_name' => self::HTTP_AI_KEY_SETTING_NAME, + ), + ) + ); } /** - * Removes the test connector after each test. + * Removes the test connectors after each test. */ public function tear_down(): void { $registry = WP_Connector_Registry::get_instance(); - if ( null !== $registry && $registry->is_registered( self::CONNECTOR_ID ) ) { - $registry->unregister( self::CONNECTOR_ID ); + foreach ( array( self::CONNECTOR_ID, self::HTTP_CONNECTOR_ID ) as $connector_id ) { + if ( null !== $registry && $registry->is_registered( $connector_id ) ) { + $registry->unregister( $connector_id ); + } } parent::tear_down(); @@ -183,4 +200,66 @@ public function test_keeps_and_masks_submitted_valid_ai_key(): void { 'The submitted AI provider key should be masked in the response.' ); } + + /** + * Ensures a submitted AI provider key is kept when the provider cannot be reached to validate it. + * + * @ticket 65551 + */ + public function test_keeps_submitted_ai_key_when_provider_is_unreachable(): void { + $submitted_key = 'sk-submitted-valid-key'; + update_option( self::HTTP_AI_KEY_SETTING_NAME, $submitted_key ); + + $this->mock_models_endpoint_response( new WP_Error( 'http_request_failed', 'cURL error 28: Operation timed out' ) ); + add_filter( 'wp_trigger_error_trigger_error', '__return_false' ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/settings' ); + $request->set_param( self::HTTP_AI_KEY_SETTING_NAME, $submitted_key ); + $response = new WP_REST_Response( array( self::HTTP_AI_KEY_SETTING_NAME => $submitted_key ) ); + + $result = _wp_connectors_rest_settings_dispatch( $response, rest_get_server(), $request ); + $data = $result->get_data(); + + $this->assertSame( array( $submitted_key ), $this->mock_models_endpoint_api_keys, 'The key should be validated with the provider.' ); + $this->assertSame( + $submitted_key, + get_option( self::HTTP_AI_KEY_SETTING_NAME ), + 'A submitted AI provider key that cannot be validated should be kept.' + ); + $this->assertSame( + _wp_connectors_mask_api_key( $submitted_key ), + $data[ self::HTTP_AI_KEY_SETTING_NAME ], + 'The submitted AI provider key should be masked in the response.' + ); + } + + /** + * Ensures a submitted AI provider key is discarded when the provider rejects it. + * + * @ticket 65551 + */ + public function test_discards_submitted_ai_key_rejected_by_provider(): void { + $submitted_key = 'sk-submitted-invalid-key'; + update_option( self::HTTP_AI_KEY_SETTING_NAME, $submitted_key ); + + $this->mock_models_endpoint_response( 401 ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/settings' ); + $request->set_param( self::HTTP_AI_KEY_SETTING_NAME, $submitted_key ); + $response = new WP_REST_Response( array( self::HTTP_AI_KEY_SETTING_NAME => $submitted_key ) ); + + $result = _wp_connectors_rest_settings_dispatch( $response, rest_get_server(), $request ); + $data = $result->get_data(); + + $this->assertSame( + '', + get_option( self::HTTP_AI_KEY_SETTING_NAME ), + 'A submitted AI provider key that the provider rejects should be discarded.' + ); + $this->assertSame( + '', + $data[ self::HTTP_AI_KEY_SETTING_NAME ], + 'The discarded key should be returned as an empty string.' + ); + } }