Skip to content

Commit

Permalink
enforce configuration of Safe Browsing API key
Browse files Browse the repository at this point in the history 
The built-in fallback API key must not be used anymore. Enforce
configuration of a custom API key and deactivate the feature, if none
is provided.
  • Loading branch information
@stklcode
stklcode committed Sep 10, 2022
1 parent e55943c commit 4d7e3be
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 78 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Changelog

### unreleased ###
* Enforce use of custom Safe Browsing API key (#108)

### 1.4.4 ###
* Fix warning on SafeBrowsing API errors with PHP 8.1+ (#123)
* Tested up to WordPress 6.2
Expand Down
2 changes: 1 addition & 1 deletion antivirus.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* Text Domain: antivirus
* License: GPLv2 or later
* License URI: http://www.gnu.org/licenses/gpl-2.0.html
* Version: 1.4.4
* Version: 1.5.0
*
* @package AntiVirus
*/
Expand Down
25 changes: 8 additions & 17 deletions inc/class-antivirus-safebrowsing.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@ class AntiVirus_SafeBrowsing extends AntiVirus {
public static function check_safe_browsing() {
// Check if API key is provided in config.
$key = parent::_get_option( 'safe_browsing_key' );
$custom_key = true;
// Fallback to default key if not.
// Opt-out, if no API key was specified.
if ( empty( $key ) ) {
$key = 'AIzaSyCGHXUd7vQAySRLNiC5y1M_wzR2W0kCVKI';
$custom_key = false;
return;
}

// Request the API.
Expand All @@ -44,7 +42,7 @@ public static function check_safe_browsing() {
array(
'client' => array(
'clientId' => 'wpantivirus',
'clientVersion' => '1.4.3',
'clientVersion' => '1.5.0',
),
'threatInfo' => array(
'threatTypes' => array(
Expand Down Expand Up @@ -105,18 +103,11 @@ public static function check_safe_browsing() {
);
}

// Add advice to solve the problem, depending on the key (custom or default).
if ( $custom_key ) {
$mail_body .= sprintf(
"\r\n%s",
esc_html__( 'Please check if your API key is correct and its limit not exceeded. If everything is correct and the error persists for the next requests, please contact the plugin support.', 'antivirus' )
);
} else {
$mail_body .= sprintf(
"\r\n%s",
esc_html__( 'This might be due to an exceeded rate limit on the shared API key. To ensure this does not happen please provide your own key using the settings page.', 'antivirus' )
);
}
// Add advice to solve the problem, depending on the key.
$mail_body .= sprintf(
"\r\n%s",
esc_html__( 'Please check if your API key is correct and its limit not exceeded. If everything is correct and the error persists for the next requests, please contact the plugin support.', 'antivirus' )
);

self::_send_warning_notification(
esc_html__( 'Safe Browsing check failed', 'antivirus' ),
Expand Down
109 changes: 57 additions & 52 deletions inc/class-antivirus.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@ public static function activation() {
if ( self::_cron_enabled( self::_get_options() ) ) {
self::_add_scheduled_hook();
}

// Add admin notice and disable the feature, if Safe Browsing is enabled without custom API key.
$safe_browsing_key = self::_get_option( 'safe_browsing_key' );
if ( self::_get_option( 'safe_browsing' ) && empty( $safe_browsing_key ) ) {
self::_update_option( 'safe_browsing', 0 );
set_transient( 'antivirus-activation-notice', true, 2592000 );
}
}

/**
Expand Down Expand Up @@ -586,14 +593,40 @@ public static function get_ajax_response() {
* Show notice on the dashboard.
*/
public static function show_dashboard_notice() {
// Add admin notice to users who can manage options, if Safe Browsing is enabled without custom API key.
// Add admin notice to users who can manage options, Safe Browsing has been disabled without custom API key.
if ( current_user_can( 'manage_options' ) ) {
$screen = get_current_screen();
if ( ! is_object( $screen ) || 'settings_page_antivirus' !== $screen->base ) {
$safe_browsing_key = self::_get_option( 'safe_browsing_key' );
if ( self::_get_option( 'safe_browsing' ) && empty( $safe_browsing_key ) ) {
self::show_safebrowsing_notice();
}
if ( get_transient( 'antivirus-activation-notice' ) ) {
printf(
'<div class="notice notice-warning is-dismissible"><p><strong>%1$s</strong></p><p>%2$s</p><p>%3$s %4$s</p></div>',
esc_html( 'No Safe Browsing API key provided for AntiVirus', 'antivirus' ),
esc_html( 'Google Safe Browsing check was disabled, because no API key has been provided.', 'antivirus' ),
wp_kses(
sprintf(
/* translators: First placeholder (%1$s) starting link tag to the plugin settings page, second placeholder (%2$s) closing link tag */
__( 'If you want to continue using this feature, please provide an API key using the %1$sAntiVirus settings page%2$s.', 'antivirus' ),
'<a href="' . esc_attr( add_query_arg( array( 'page' => 'antivirus' ), admin_url( '/options-general.php' ) ) ) . '">',
'</a>'
),
array( 'a' => array( 'href' => array() ) )
),
wp_kses(
sprintf(
/* translators: First placeholder (%1$s) starting link tag to the documentation page, second placeholder (%2$s) closing link tag */
__( 'See official %1$sdocumentation%2$s from Google.', 'antivirus' ),
'<a href="https://cloud.google.com/docs/authentication/api-keys" target="_blank" rel="noopener noreferrer">',
'</a>'
),
array(
'a' => array(
'href' => array(),
'target' => array(),
'rel' => array(),
),
)
)
);
delete_transient( 'antivirus-activation-notice' );
}
}

Expand Down Expand Up @@ -656,7 +689,6 @@ public static function show_admin_menu() {

// Save options.
self::_update_options( $options ); ?>

<div id="message" class="notice notice-success">
<p>
<strong>
Expand All @@ -666,12 +698,6 @@ public static function show_admin_menu() {
</div>
<?php
}

// Show admin notice for Safe Browsing without API key immediately after saving settings.
$safe_browsing_key = self::_get_option( 'safe_browsing_key' );
if ( self::_get_option( 'safe_browsing' ) && empty( $safe_browsing_key ) ) {
self::show_safebrowsing_notice();
}
?>

<div class="wrap" id="av_main">
Expand Down Expand Up @@ -771,12 +797,28 @@ public static function show_admin_menu() {
<?php esc_html_e( 'Safe Browsing API key', 'antivirus' ); ?>
</label>
<br/>
<input type="text" name="av_safe_browsing_key" id="av_safe_browsing_key" size="45"
<input type="text" name="av_safe_browsing_key" id="av_safe_browsing_key" size="45" required
value="<?php echo esc_attr( self::_get_option( 'safe_browsing_key' ) ); ?>" />

<p class="description">
<?php
esc_html_e( 'Provide a custom key for the Google Safe Browsing API (v4). If this value is left empty, a fallback will be used. However, to ensure valid results due to rate limitations, it is recommended to use your own key.', 'antivirus' );
printf(
'%1$s %2$s<br>%3$s',
esc_html( 'Provide a custom key for the Google Safe Browsing API (v4).', 'antivirus' ),
wp_kses(
__( 'A key is <em>required</em> in order to use this check.', 'antivirus' ),
array( 'em' => array() )
),
wp_kses(
sprintf(
/* translators: First placeholder (%1$s) starting link tag to the documentation page, second placeholder (%2$s) closing link tag */
__( 'See official %1$sdocumentation%2$s from Google.', 'antivirus' ),
'<a href="https://cloud.google.com/docs/authentication/api-keys">',
'</a>'
),
array( 'a' => array( 'href' => array() ) )
)
);
?>
</p>
</fieldset>
Expand Down Expand Up @@ -846,41 +888,4 @@ class="regular-text"
</div>
<?php
}

/**
* Show admin notice for Safe Browsing use without API key.
*
* @since 1.4.3
*/
private static function show_safebrowsing_notice() {
printf(
'<div class="notice notice-warning is-dismissible"><p><strong>%1$s</strong></p><p>%2$s</p><p>%3$s %4$s</p></div>',
esc_html( 'No Safe Browsing API key provided for AntiVirus', 'antivirus' ),
esc_html( 'Google Safe Browsing check is enabled without a custom API key. The built-in key is no longer supported and will be be removed with the next release of AntiVirus.', 'antivirus' ),
wp_kses(
sprintf(
/* translators: First placeholder (%1$s) starting link tag to the plugin settings page, second placeholder (%2$s) closing link tag */
__( 'If you want to continue using this feature, please provide an API key using the %1$sAntiVirus settings page%2$s.', 'antivirus' ),
'<a href="' . esc_attr( add_query_arg( array( 'page' => 'antivirus' ), admin_url( '/options-general.php' ) ) ) . '">',
'</a>'
),
array( 'a' => array( 'href' => array() ) )
),
wp_kses(
sprintf(
/* translators: First placeholder (%1$s) starting link tag to the documentation page, second placeholder (%2$s) closing link tag */
__( 'See official %1$sdocumentation%2$s from Google.', 'antivirus' ),
'<a href="https://cloud.google.com/docs/authentication/api-keys" target="_blank" rel="noopener noreferrer">',
'</a>'
),
array(
'a' => array(
'href' => array(),
'target' => array(),
'rel' => array(),
),
)
)
);
}
}
27 changes: 19 additions & 8 deletions tests/test-safebrowsing.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,16 @@ public function test(): void {
'{"error":{"message":"Invalid API key"}}'
);

// Specify API key.
$this->update_options( array( 'safe_browsing_key' => 'custom-api-key' ) );

AntiVirus_SafeBrowsing::check_safe_browsing();

// Validate the request.
self::assertEquals(
'https://safebrowsing.googleapis.com/v4/threatMatches:find?key=AIzaSyCGHXUd7vQAySRLNiC5y1M_wzR2W0kCVKI',
'https://safebrowsing.googleapis.com/v4/threatMatches:find?key=custom-api-key',
$request_url,
'expected call to Safe Browsing API with default key'
'expected call to Safe Browsing API with custom key'
);
self::assertIsArray( $request_data, 'unexpected request' );
$request_body = json_decode( $request_data['body'] );
Expand Down Expand Up @@ -107,12 +110,7 @@ public function test(): void {
/*
* Case 3: With custom API key and notification address.
*/
$this->update_options(
array(
'notify_email' => '[email protected]',
'safe_browsing_key' => 'custom-api-key',
)
);
$this->update_options( array( 'notify_email' => '[email protected]' ) );

AntiVirus_SafeBrowsing::check_safe_browsing();

Expand Down Expand Up @@ -149,5 +147,18 @@ public function test(): void {
'expected different subject for Safe Browsing check failing with 400'
);
self::assertStringContainsString( "\r\n Invalid API key\r\n", $mail_body, 'Message from response not transported to mail' );

/*
* Case 6: Without API key.
*/
$this->update_options( array( 'safe_browsing_key' => '' ) );
$request_url = null;

AntiVirus_SafeBrowsing::check_safe_browsing();

self::assertNull(
$request_url,
'API should not be called without an API key'
);
}
}

0 comments on commit 4d7e3be

Please sign in to comment.