Skip to content

Commit a58d566

Browse files
dkotterdkotter
authored
Merge pull request #1048 from 10up/fix/allow-empty-credentials
Allow empty credentials when saving settings
2 parents 85af4af + e6e5695 commit a58d566

File tree

7 files changed

+203
-188
lines changed

7 files changed

+203
-188
lines changed

‎includes/Classifai/Providers/AWS/AmazonPolly.php‎

Lines changed: 80 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -198,68 +198,98 @@ public function get_default_provider_settings(): array {
198198
* @return array
199199
*/
200200
public function sanitize_settings( array $new_settings ): array {
201-
$settings = $this->feature_instance->get_settings();
202-
$is_credentials_changed = false;
201+
$settings = $this->feature_instance->get_settings();
202+
$authenticated = $this->authenticate_credentials( $new_settings );
203203

204-
$new_settings[ static::ID ]['authenticated'] = $settings[ static::ID ]['authenticated'];
205-
$new_settings[ static::ID ]['voices'] = $settings[ static::ID ]['voices'];
206-
207-
if (
208-
! empty( $new_settings[ static::ID ]['access_key_id'] ) &&
209-
! empty( $new_settings[ static::ID ]['secret_access_key'] ) &&
210-
! empty( $new_settings[ static::ID ]['aws_region'] )
211-
) {
212-
$new_access_key_id = sanitize_text_field( $new_settings[ static::ID ]['access_key_id'] );
213-
$new_secret_access_key = sanitize_text_field( $new_settings[ static::ID ]['secret_access_key'] );
214-
$new_aws_region = sanitize_text_field( $new_settings[ static::ID ]['aws_region'] );
215-
216-
if (
217-
$new_access_key_id !== $settings[ static::ID ]['access_key_id'] ||
218-
$new_secret_access_key !== $settings[ static::ID ]['secret_access_key'] ||
219-
$new_aws_region !== $settings[ static::ID ]['aws_region']
220-
) {
221-
$is_credentials_changed = true;
222-
}
223-
224-
if ( $is_credentials_changed ) {
225-
$new_settings[ static::ID ]['access_key_id'] = $new_access_key_id;
226-
$new_settings[ static::ID ]['secret_access_key'] = $new_secret_access_key;
227-
$new_settings[ static::ID ]['aws_region'] = $new_aws_region;
228-
229-
// Connect to the service and get voices.
230-
$new_settings[ static::ID ]['voices'] = $this->connect_to_service(
231-
array(
232-
'access_key_id' => $new_access_key_id,
233-
'secret_access_key' => $new_secret_access_key,
234-
'aws_region' => $new_aws_region,
235-
)
236-
);
237-
238-
if ( ! empty( $new_settings[ static::ID ]['voices'] ) ) {
239-
$new_settings[ static::ID ]['authenticated'] = true;
240-
} else {
241-
$new_settings[ static::ID ]['voices'] = [];
242-
$new_settings[ static::ID ]['authenticated'] = false;
243-
}
244-
}
245-
} else {
246-
$new_settings[ static::ID ]['access_key_id'] = $settings[ static::ID ]['access_key_id'];
247-
$new_settings[ static::ID ]['secret_access_key'] = $settings[ static::ID ]['secret_access_key'];
248-
$new_settings[ static::ID ]['aws_region'] = $settings[ static::ID ]['aws_region'];
204+
if ( is_wp_error( $authenticated ) ) {
205+
$new_settings[ static::ID ]['authenticated'] = false;
249206

250207
add_settings_error(
251-
$this->feature_instance->get_option_name(),
252-
'classifai-ams-polly-auth-empty',
253-
esc_html__( 'One or more credentials required to connect to the Amazon Polly service is empty.', 'classifai' ),
208+
'api_key',
209+
'classifai-auth',
210+
$authenticated->get_error_message(),
254211
'error'
255212
);
213+
} else {
214+
$new_settings[ static::ID ]['authenticated'] = true;
215+
}
216+
217+
$new_access_key_id = sanitize_text_field( $new_settings[ static::ID ]['access_key_id'] ?? $settings[ static::ID ]['access_key_id'] );
218+
$new_secret_access_key = sanitize_text_field( $new_settings[ static::ID ]['secret_access_key'] ?? $settings[ static::ID ]['secret_access_key'] );
219+
$new_aws_region = sanitize_text_field( $new_settings[ static::ID ]['aws_region'] ?? $settings[ static::ID ]['aws_region'] );
220+
221+
$new_settings[ static::ID ]['access_key_id'] = $new_access_key_id;
222+
$new_settings[ static::ID ]['secret_access_key'] = $new_secret_access_key;
223+
$new_settings[ static::ID ]['aws_region'] = $new_aws_region;
224+
225+
// Connect to the service and get voices.
226+
$new_settings[ static::ID ]['voices'] = $this->connect_to_service(
227+
array(
228+
'access_key_id' => $new_access_key_id,
229+
'secret_access_key' => $new_secret_access_key,
230+
'aws_region' => $new_aws_region,
231+
)
232+
);
233+
234+
if ( ! empty( $new_settings[ static::ID ]['voices'] ) ) {
235+
$new_settings[ static::ID ]['authenticated'] = true;
236+
} else {
237+
$new_settings[ static::ID ]['voices'] = [];
238+
$new_settings[ static::ID ]['authenticated'] = false;
256239
}
257240

258241
$new_settings[ static::ID ]['voice'] = sanitize_text_field( $new_settings[ static::ID ]['voice'] ?? $settings[ static::ID ]['voice'] );
259242

260243
return $new_settings;
261244
}
262245

246+
/**
247+
* Authenticate our credentials.
248+
*
249+
* @param array $settings Settings being saved.
250+
* @return bool|WP_Error
251+
*/
252+
protected function authenticate_credentials( array $settings = [] ) {
253+
$response = false;
254+
255+
try {
256+
/**
257+
* Filters the return value of the connect to services function.
258+
*
259+
* Returning a non-false value from the filter will short-circuit
260+
* the describe voices request and return early with that value.
261+
* This filter is useful for E2E tests.
262+
*
263+
* @since 3.1.0
264+
* @hook classifai_aws_polly_pre_connect_to_service
265+
*
266+
* @param bool $pre The value of pre connect to service. Default false. A non-false value will short-circuit the describe voices request.
267+
*
268+
* @return bool|mixed The filtered value of connect to service.
269+
*/
270+
$pre = apply_filters( 'classifai_' . self::ID . '_pre_connect_to_service', false );
271+
272+
if ( false !== $pre ) {
273+
return $pre;
274+
}
275+
276+
$polly_client = $this->get_polly_client( $settings[ static::ID ] );
277+
278+
if ( $polly_client ) {
279+
$polly_voices = $polly_client->describeVoices();
280+
$polly_voices = $polly_voices->get( 'Voices' );
281+
} else {
282+
$polly_voices = [];
283+
}
284+
285+
$response = ! empty( $polly_voices ) ? true : new WP_Error( 'auth', esc_html__( 'Connection to Amazon Polly failed.', 'classifai' ) );
286+
} catch ( \Exception $e ) {
287+
$response = new WP_Error( 'auth', esc_html__( 'Connection to Amazon Polly failed.', 'classifai' ) );
288+
}
289+
290+
return ! is_wp_error( $response ) ? true : $response;
291+
}
292+
263293
/**
264294
* Connects to the Amazon Polly service.
265295
*

‎includes/Classifai/Providers/Azure/ComputerVision.php‎

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -204,47 +204,31 @@ public function get_default_provider_settings(): array {
204204
}
205205

206206
/**
207-
* Sanitization
207+
* Sanitize the settings for this Provider.
208208
*
209209
* @param array $new_settings The settings being saved.
210210
* @return array|mixed
211211
*/
212212
public function sanitize_settings( array $new_settings ) {
213-
$settings = $this->feature_instance->get_settings();
213+
$settings = $this->feature_instance->get_settings();
214+
$authenticated = $this->authenticate_credentials( $new_settings );
214215

215-
if ( ! empty( $new_settings[ static::ID ]['endpoint_url'] ) && ! empty( $new_settings[ static::ID ]['api_key'] ) ) {
216-
$new_settings[ static::ID ]['authenticated'] = $settings[ static::ID ]['authenticated'];
217-
$new_settings[ static::ID ]['endpoint_url'] = esc_url_raw( $new_settings[ static::ID ]['endpoint_url'] ?? $settings[ static::ID ]['endpoint_url'] );
218-
$new_settings[ static::ID ]['api_key'] = sanitize_text_field( $new_settings[ static::ID ]['api_key'] ?? $settings[ static::ID ]['api_key'] );
216+
if ( is_wp_error( $authenticated ) ) {
217+
$new_settings[ static::ID ]['authenticated'] = false;
219218

220-
$is_authenticated = $new_settings[ static::ID ]['authenticated'];
221-
$is_endpoint_same = $new_settings[ static::ID ]['endpoint_url'] === $settings[ static::ID ]['endpoint_url'];
222-
$is_api_key_same = $new_settings[ static::ID ]['api_key'] === $settings[ static::ID ]['api_key'];
223-
224-
if ( ! ( $is_authenticated && $is_endpoint_same && $is_api_key_same ) ) {
225-
$auth_check = $this->authenticate_credentials( $new_settings );
226-
227-
if ( is_wp_error( $auth_check ) ) {
228-
$new_settings[ static::ID ]['authenticated'] = false;
229-
230-
$error_message = $auth_check->get_error_message();
231-
232-
// Add an error message.
233-
add_settings_error(
234-
'api_key',
235-
'classifai-auth',
236-
$error_message,
237-
'error'
238-
);
239-
} else {
240-
$new_settings[ static::ID ]['authenticated'] = true;
241-
}
242-
}
219+
add_settings_error(
220+
'api_key',
221+
'classifai-auth',
222+
$authenticated->get_error_message(),
223+
'error'
224+
);
243225
} else {
244-
$new_settings[ static::ID ]['endpoint_url'] = $settings[ static::ID ]['endpoint_url'];
245-
$new_settings[ static::ID ]['api_key'] = $settings[ static::ID ]['api_key'];
226+
$new_settings[ static::ID ]['authenticated'] = true;
246227
}
247228

229+
$new_settings[ static::ID ]['endpoint_url'] = esc_url_raw( $new_settings[ static::ID ]['endpoint_url'] ?? $settings[ static::ID ]['endpoint_url'] );
230+
$new_settings[ static::ID ]['api_key'] = sanitize_text_field( $new_settings[ static::ID ]['api_key'] ?? $settings[ static::ID ]['api_key'] );
231+
248232
if ( $this->feature_instance instanceof DescriptiveTextGenerator ) {
249233
$new_settings[ static::ID ]['descriptive_confidence_threshold'] = floatval( $new_settings[ static::ID ]['descriptive_confidence_threshold'] ?? $settings[ static::ID ]['descriptive_confidence_threshold'] );
250234
}

‎includes/Classifai/Providers/Azure/OpenAI.php‎

Lines changed: 17 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -200,47 +200,26 @@ public function get_default_provider_settings(): array {
200200
* @return array
201201
*/
202202
public function sanitize_settings( array $new_settings ): array {
203-
$settings = $this->feature_instance->get_settings();
204-
205-
if (
206-
! empty( $new_settings[ static::ID ]['endpoint_url'] ) &&
207-
! empty( $new_settings[ static::ID ]['api_key'] ) &&
208-
! empty( $new_settings[ static::ID ]['deployment'] )
209-
) {
210-
$new_settings[ static::ID ]['authenticated'] = $settings[ static::ID ]['authenticated'];
211-
$new_settings[ static::ID ]['endpoint_url'] = esc_url_raw( $new_settings[ static::ID ]['endpoint_url'] ?? $settings[ static::ID ]['endpoint_url'] );
212-
$new_settings[ static::ID ]['api_key'] = sanitize_text_field( $new_settings[ static::ID ]['api_key'] ?? $settings[ static::ID ]['api_key'] );
213-
$new_settings[ static::ID ]['deployment'] = sanitize_text_field( $new_settings[ static::ID ]['deployment'] ?? $settings[ static::ID ]['deployment'] );
214-
215-
$is_authenticated = $new_settings[ static::ID ]['authenticated'];
216-
$is_endpoint_same = $new_settings[ static::ID ]['endpoint_url'] === $settings[ static::ID ]['endpoint_url'];
217-
$is_api_key_same = $new_settings[ static::ID ]['api_key'] === $settings[ static::ID ]['api_key'];
218-
$is_deployment_same = $new_settings[ static::ID ]['deployment'] === $settings[ static::ID ]['deployment'];
219-
220-
if ( ! ( $is_authenticated && $is_endpoint_same && $is_api_key_same && $is_deployment_same ) ) {
221-
$auth_check = $this->authenticate_credentials( $new_settings );
222-
223-
if ( is_wp_error( $auth_check ) ) {
224-
$new_settings[ static::ID ]['authenticated'] = false;
225-
$error_message = $auth_check->get_error_message();
226-
227-
// Add an error message.
228-
add_settings_error(
229-
'api_key',
230-
'classifai-auth',
231-
$error_message,
232-
'error'
233-
);
234-
} else {
235-
$new_settings[ static::ID ]['authenticated'] = true;
236-
}
237-
}
203+
$settings = $this->feature_instance->get_settings();
204+
$authenticated = $this->authenticate_credentials( $new_settings );
205+
206+
if ( is_wp_error( $authenticated ) ) {
207+
$new_settings[ static::ID ]['authenticated'] = false;
208+
209+
add_settings_error(
210+
'api_key',
211+
'classifai-auth',
212+
$authenticated->get_error_message(),
213+
'error'
214+
);
238215
} else {
239-
$new_settings[ static::ID ]['endpoint_url'] = $settings[ static::ID ]['endpoint_url'];
240-
$new_settings[ static::ID ]['api_key'] = $settings[ static::ID ]['api_key'];
241-
$new_settings[ static::ID ]['deployment'] = $settings[ static::ID ]['deployment'];
216+
$new_settings[ static::ID ]['authenticated'] = true;
242217
}
243218

219+
$new_settings[ static::ID ]['endpoint_url'] = esc_url_raw( $new_settings[ static::ID ]['endpoint_url'] ?? $settings[ static::ID ]['endpoint_url'] );
220+
$new_settings[ static::ID ]['api_key'] = sanitize_text_field( $new_settings[ static::ID ]['api_key'] ?? $settings[ static::ID ]['api_key'] );
221+
$new_settings[ static::ID ]['deployment'] = sanitize_text_field( $new_settings[ static::ID ]['deployment'] ?? $settings[ static::ID ]['deployment'] );
222+
244223
switch ( $this->feature_instance::ID ) {
245224
case ContentResizing::ID:
246225
case TitleGeneration::ID:

0 commit comments

Comments
 (0)