diff --git a/classes/models/FrmAddon.php b/classes/models/FrmAddon.php index 9696bb3a25..2fddffcd78 100644 --- a/classes/models/FrmAddon.php +++ b/classes/models/FrmAddon.php @@ -585,7 +585,9 @@ public function activate_defined_license() { if ( $license && ! $this->is_active() && ! $this->checked_recently( '1 day' ) ) { $response = $this->activate_license( $license ); - if ( ! $response['success'] ) { + // Drop the license only when the API reported on it. An unreachable API says + // nothing, so the defined license is kept and checked again later. + if ( ! $response['success'] && empty( $response['inconclusive'] ) ) { $license = ''; } } @@ -655,12 +657,21 @@ public function clear_license() { * Don't save an invalid license. * * @since 6.8.3 + * @since x.x Added the $is_conclusive param. * - * @param bool|string $is_valid If license activation was successful. May be a string 'valid'. + * @param bool|string $is_valid If license activation was successful. May be a string 'valid'. + * @param bool $is_conclusive Whether the API reported on the license. False when the + * check could not reach a verdict, such as a connection + * error, in which case the saved license is left alone. * * @return void */ - protected function maybe_set_active( $is_valid ) { + protected function maybe_set_active( $is_valid, $is_conclusive = true ) { + if ( ! $is_valid && ! $is_conclusive ) { + // Nothing came back about this license, so don't discard what is saved. + return; + } + update_option( $this->option_name . 'active', $is_valid ); if ( $is_valid ) { @@ -1002,6 +1013,12 @@ private function is_license_revoked() { $response = $this->get_license_status(); + if ( ! empty( $response['inconclusive'] ) ) { + // The API did not report on this license, so there is nothing to act on. A + // license that activated before stays in place until the API says otherwise. + return; + } + if ( ! empty( $this->save_status['response_code'] ) && 429 === $this->save_status['response_code'] ) { // If we got a rate limit response, don't clear the license. return; @@ -1169,7 +1186,7 @@ private function activate_license( $license ) { $response['success'] = true; } - $this->maybe_set_active( $is_valid ); + $this->maybe_set_active( $is_valid, empty( $response['inconclusive'] ) ); } $this->update_last_checked( (bool) $is_valid ); @@ -1197,14 +1214,46 @@ private function die_if_not_allowed() { } /** + * The statuses the API sends to describe the license itself. + * + * Anything else that comes back, a connection error message or an unreadable + * body, says nothing about the license and must not be treated as an answer + * about it. See the inconclusive handling in get_license_status. + * + * @since x.x + * + * @return array + */ + private function get_reported_license_statuses() { + return array( + 'valid', + 'invalid', + 'expired', + 'revoked', + 'blocked', + 'disabled', + 'missing', + 'inactive', + 'site_inactive', + 'no_activations_left', + 'invalid_item_id', + 'item_name_mismatch', + ); + } + + /** + * @since x.x Added the inconclusive key, so a check that never reached the API + * can be told apart from one where the API reported on the license. + * * @return array */ private function get_license_status() { $this->set_running(); $response = array( - 'status' => 'missing', - 'error' => true, + 'status' => 'missing', + 'error' => true, + 'inconclusive' => false, ); if ( ! $this->license ) { @@ -1216,21 +1265,37 @@ private function get_license_status() { try { $response['error'] = false; - $license_data = $this->send_mothership_request( 'activate_license' ); - - // $license_data->license will be either "valid" or "invalid" + // Until the response is read, nothing is known about the license. The default + // status above is only a placeholder, never an answer from the API. + $response['inconclusive'] = true; + $license_data = $this->send_mothership_request( 'activate_license' ); + $status = ''; + + // $license_data['license'] will be a status such as "valid" or "invalid". A + // string response is an error message rather than a license status. if ( is_array( $license_data ) ) { - if ( ! empty( $license_data['license'] ) && in_array( $license_data['license'], array( 'valid', 'invalid' ), true ) ) { - $response['status'] = $license_data['license']; - $this->save_status['status'] = $license_data['license']; - $is_valid = 'valid' === $license_data['license']; + if ( ! empty( $license_data['license'] ) && is_string( $license_data['license'] ) ) { + $status = $license_data['license']; } - } else { - $response['status'] = $license_data; + } elseif ( is_string( $license_data ) ) { + $status = $license_data; + } + + if ( $status && in_array( $status, $this->get_reported_license_statuses(), true ) ) { + $response['status'] = $status; + $response['inconclusive'] = false; + $is_valid = 'valid' === $status; + + if ( in_array( $status, array( 'valid', 'invalid' ), true ) ) { + $this->save_status['status'] = $status; + } + } elseif ( $status ) { + // Keep the message for display, but it is not a verdict on the license. + $response['status'] = $status; } } catch ( Exception $e ) { $response['status'] = $e->getMessage(); - } + }//end try $this->update_last_checked( $is_valid ); $this->done_running(); @@ -1319,9 +1384,12 @@ private static function set_license_from_post() { } /** - * @param string $action + * @since x.x The return type covers the decoded body, which is an array when the + * API answers with JSON, and an error message string otherwise. * - * @return string + * @param string $action The request to send, such as activate_license. + * + * @return array|string */ public function send_mothership_request( $action ) { $api_params = array( diff --git a/tests/phpunit/misc/test_FrmAddon.php b/tests/phpunit/misc/test_FrmAddon.php index ed0155cfe1..a752db2ec6 100644 --- a/tests/phpunit/misc/test_FrmAddon.php +++ b/tests/phpunit/misc/test_FrmAddon.php @@ -91,6 +91,126 @@ public function test_checked_recently() { } } + /** + * Builds an add-on with a saved license whose API request comes back with the + * given payload, so the license checks can run without a request leaving the + * machine. + * + * @since x.x + * + * @param mixed $payload What the API request should come back with. + * + * @return PHPUnit\Framework\MockObject\MockObject + */ + private function get_licensed_addon( $payload ) { + $addon = $this->getMockBuilder( 'FrmTestAddon' ) + ->disableOriginalConstructor() + ->setMethods( array( 'send_mothership_request', 'clear_license' ) ) + ->getMock(); + + $addon->method( 'send_mothership_request' )->willReturn( $payload ); + + $addon->plugin_file = FrmAppHelper::plugin_path() . '/formidable.php'; + $addon->plugin_slug = 'test_license'; + $addon->option_name = 'edd_test_license_license_'; + $addon->license = 'TEST-LICENSE-KEY'; + + // Clear the weekly throttle so the check under test actually runs. + $key = $this->run_private_method( array( $addon, 'transient_key' ) ); + delete_option( $key ); + delete_site_option( $key ); + + return $addon; + } + + /** + * A check that never got an answer about the license must report itself as + * inconclusive, so nothing downstream treats it as a verdict. + * + * @since x.x + * + * @covers FrmAddon::get_license_status + * + * @dataProvider license_status_provider + * + * @param mixed $payload What the API request comes back with. + * @param bool $is_inconclusive Whether the check should report no verdict. + * @param string $status The status the check should report. + * + * @return void + */ + public function test_get_license_status_only_reports_a_verdict_from_the_api( $payload, $is_inconclusive, $status ) { + $addon = $this->get_licensed_addon( $payload ); + $response = $this->run_private_method( array( $addon, 'get_license_status' ) ); + + $this->assertSame( $is_inconclusive, ! empty( $response['inconclusive'] ) ); + $this->assertSame( $status, $response['status'] ); + } + + /** + * @since x.x + * + * @return void mixed>, mixed>> + */ + public function license_status_provider(): \Iterator { + // The API reported on the license, so the status is a verdict. + yield 'valid' => array( array( 'license' => 'valid' ), false, 'valid' ); + yield 'invalid' => array( array( 'license' => 'invalid' ), false, 'invalid' ); + yield 'revoked' => array( array( 'license' => 'revoked' ), false, 'revoked' ); + yield 'disabled' => array( array( 'license' => 'disabled' ), false, 'disabled' ); + yield 'expired' => array( array( 'license' => 'expired' ), false, 'expired' ); + yield 'no_activations_left' => array( array( 'license' => 'no_activations_left' ), false, 'no_activations_left' ); + // Nothing came back about the license, so there is no verdict to report. + yield 'error payload' => array( array( 'code' => 500 ), true, 'missing' ); + yield 'empty payload' => array( array(), true, 'missing' ); + yield 'connection error' => array( 'You had an error communicating with the Formidable API.', true, 'You had an error communicating with the Formidable API.' ); + yield 'no body' => array( null, true, 'missing' ); + } + + /** + * A license that activated before stays in place until the API says otherwise. + * Losing the connection is not a revocation. + * + * @since x.x + * + * @covers FrmAddon::is_license_revoked + * + * @dataProvider revoked_license_provider + * + * @param mixed $payload What the API request comes back with. + * @param bool $should_clear Whether the saved license should be dropped. + * + * @return void + */ + public function test_is_license_revoked_only_clears_on_a_reported_revocation( $payload, $should_clear ) { + $addon = $this->get_licensed_addon( $payload ); + + $addon->expects( $should_clear ? $this->once() : $this->never() )->method( 'clear_license' ); + + $this->run_private_method( array( $addon, 'is_license_revoked' ) ); + } + + /** + * @since x.x + * + * @return void mixed>, mixed>> + */ + public function revoked_license_provider(): \Iterator { + // The API reported the license is no longer usable. + yield 'revoked' => array( array( 'license' => 'revoked' ), true ); + yield 'blocked' => array( array( 'license' => 'blocked' ), true ); + yield 'disabled' => array( array( 'license' => 'disabled' ), true ); + yield 'missing' => array( array( 'license' => 'missing' ), true ); + // The license is still usable, or nothing came back about it. + yield 'valid' => array( array( 'license' => 'valid' ), false ); + yield 'invalid' => array( array( 'license' => 'invalid' ), false ); + yield 'expired' => array( array( 'license' => 'expired' ), false ); + yield 'error payload' => array( array( 'code' => 500 ), false ); + yield 'empty payload' => array( array(), false ); + yield 'connection error' => array( 'You had an HTTP error connecting to the Formidable API', false ); + yield 'no body' => array( null, false ); + } + /** * @covers FrmAddon::update_pro_capabilities */