diff --git a/includes/admin/class-rop-rest-api.php b/includes/admin/class-rop-rest-api.php index 7e1097d7e..150bed585 100644 --- a/includes/admin/class-rop-rest-api.php +++ b/includes/admin/class-rop-rest-api.php @@ -1194,7 +1194,15 @@ private function add_account_li( $data ) { $model = new Rop_Services_Model(); $db = new Rop_Db_Upgrade(); - $linkedin_service->add_account_with_app( $data ); + $added = $linkedin_service->add_account_with_app( $data ); + + if ( ! $added ) { + $this->response->set_code( '400' ) + ->set_message( 'Could not add the LinkedIn account, the authorization data was invalid. Check the Revive Social log for details.' ) + ->set_data( array() ); + + return $this->response->to_array(); + } $services[ $linkedin_service->get_service_id() ] = $linkedin_service->get_service(); $active_accounts = array_merge( $active_accounts, $linkedin_service->get_service_active_accounts() ); diff --git a/includes/admin/services/class-rop-linkedin-service.php b/includes/admin/services/class-rop-linkedin-service.php index 15b4feacf..b741627cb 100644 --- a/includes/admin/services/class-rop-linkedin-service.php +++ b/includes/admin/services/class-rop-linkedin-service.php @@ -135,10 +135,11 @@ public function authorize() { } } catch ( Exception $e ) { - $message = 'Linkedin Error: Code[ ' . $e->getCode() . ' ] ' . $e->getDescription(); + $description = method_exists( $e, 'getDescription' ) ? $e->getDescription() : $e->getMessage(); + $message = 'Linkedin Error: Code[ ' . $e->getCode() . ' ] ' . $description; $this->logger->alert_error( $message ); $this->rop_get_error_docs( $message ); - $referrer = $_SERVER['HTTP_REFERER']; + $referrer = isset( $_SERVER['HTTP_REFERER'] ) ? $_SERVER['HTTP_REFERER'] : ''; // If the user is trying to authenticate. if ( ! empty( substr_count( $referrer, 'linkedin.com' ) ) ) { exit( wp_redirect( $this->get_legacy_url() ) ); @@ -838,15 +839,42 @@ function ( $matches ) { * @access public */ public function add_account_with_app( $accounts_data ) { - if ( ! $this->is_set_not_empty( $accounts_data, array( 'id' ) ) ) { + if ( ! $this->is_set_not_empty( $accounts_data, array( 'id', 'pages' ) ) ) { + return false; + } + + // is_set_not_empty() accepts array values, which base64_decode() would fatal on. + if ( ! is_string( $accounts_data['id'] ) || ! is_string( $accounts_data['pages'] ) ) { + $this->logger->alert_error( 'Linkedin Error: received malformed account data, the account was not added.' ); return false; } $the_id = unserialize( base64_decode( $accounts_data['id'] ) ); $accounts_array = unserialize( base64_decode( $accounts_data['pages'] ) ); + if ( empty( $the_id ) || ! is_string( $the_id ) || ! is_array( $accounts_array ) ) { + $this->logger->alert_error( 'Linkedin Error: received malformed account data, the account was not added.' ); + return false; + } + // last array item contains notify date $notify_user_at = array_pop( $accounts_array ); + + if ( empty( $accounts_array ) || ! is_array( $notify_user_at ) || ! isset( $notify_user_at['notify_user_at'] ) || ! is_numeric( $notify_user_at['notify_user_at'] ) ) { + $this->logger->alert_error( 'Linkedin Error: received malformed account data, the account was not added.' ); + return false; + } + + // every remaining item must be a complete account entry, otherwise reading its fields below can fatal + foreach ( $accounts_array as $account_data ) { + if ( ! is_array( $account_data ) || ! isset( $account_data['id'], $account_data['img'], $account_data['account'], $account_data['is_company'], $account_data['user'], $account_data['access_token'] ) || ! is_string( $account_data['id'] ) || '' === $account_data['id'] ) { + $this->logger->alert_error( 'Linkedin Error: received malformed account data, the account was not added.' ); + return false; + } + } + + $accounts_array = array_values( $accounts_array ); + // save timestamp for when to notify user to refresh their linkedin token // set notified count to 0 $notify_data = array( diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 22ff07d7a..7d4b63a1f 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -3924,12 +3924,6 @@ parameters: count: 4 path: includes/admin/services/class-rop-gmb-service.php - - - message: '#^Call to an undefined method Exception\:\:getDescription\(\)\.$#' - identifier: method.notFound - count: 1 - path: includes/admin/services/class-rop-linkedin-service.php - - message: '#^Call to function is_object\(\) with LinkedIn\\Client will always evaluate to true\.$#' identifier: function.alreadyNarrowedType @@ -4146,12 +4140,6 @@ parameters: count: 1 path: includes/admin/services/class-rop-linkedin-service.php - - - message: '#^Variable \$account might not be defined\.$#' - identifier: variable.undefined - count: 1 - path: includes/admin/services/class-rop-linkedin-service.php - - message: '#^Variable \$data in empty\(\) always exists and is not falsy\.$#' identifier: empty.variable diff --git a/tests/e2e/specs/dashboard/linkedin-error-handling.spec.js b/tests/e2e/specs/dashboard/linkedin-error-handling.spec.js new file mode 100644 index 000000000..405ab53d2 --- /dev/null +++ b/tests/e2e/specs/dashboard/linkedin-error-handling.spec.js @@ -0,0 +1,200 @@ +import { test, expect } from '../../fixtures'; + +// base64( serialize( 'urn:li:person:E2ETEST' ) ) +const VALID_ID = 'czoyMToidXJuOmxpOnBlcnNvbjpFMkVURVNUIjs='; +// base64( serialize( [ account array, [ 'notify_user_at' => 4102444800 ] ] ) ) +const VALID_PAGES = 'YToyOntpOjA7YTo2OntzOjI6ImlkIjtzOjIxOiJ1cm46bGk6cGVyc29uOkUyRVRFU1QiO3M6MzoiaW1nIjtzOjA6IiI7czo3OiJhY2NvdW50IjtzOjEzOiJFMkUgVGVzdCBVc2VyIjtzOjEwOiJpc19jb21wYW55IjtiOjA7czo0OiJ1c2VyIjtzOjEzOiJFMkUgVGVzdCBVc2VyIjtzOjEyOiJhY2Nlc3NfdG9rZW4iO3M6MTQ6ImUyZS10ZXN0LXRva2VuIjt9aToxO2E6MTp7czoxNDoibm90aWZ5X3VzZXJfYXQiO2k6NDEwMjQ0NDgwMDt9fQ=='; +// Same as VALID_PAGES but the last entry is not the notify date. +const PAGES_WITHOUT_NOTIFY = 'YToyOntpOjA7YTo2OntzOjI6ImlkIjtzOjIxOiJ1cm46bGk6cGVyc29uOkUyRVRFU1QiO3M6MzoiaW1nIjtzOjA6IiI7czo3OiJhY2NvdW50IjtzOjEzOiJFMkUgVGVzdCBVc2VyIjtzOjEwOiJpc19jb21wYW55IjtiOjA7czo0OiJ1c2VyIjtzOjEzOiJFMkUgVGVzdCBVc2VyIjtzOjEyOiJhY2Nlc3NfdG9rZW4iO3M6MTQ6ImUyZS10ZXN0LXRva2VuIjt9aToxO2E6MTp7czoxMDoidW5leHBlY3RlZCI7aToxO319'; +// base64( serialize( [ [ 'notify_user_at' => ... ] ] ) ) — notify entry only, no accounts. +const PAGES_ONLY_NOTIFY = 'YToxOntpOjA7YToxOntzOjE0OiJub3RpZnlfdXNlcl9hdCI7aTo0MTAyNDQ0ODAwO319'; +// base64( serialize( [ 'bad-account', [ 'notify_user_at' => 4102444800 ] ] ) ) — account entry is a string, not an array. +const PAGES_WITH_STRING_ACCOUNT = 'YToyOntpOjA7czoxMToiYmFkLWFjY291bnQiO2k6MTthOjE6e3M6MTQ6Im5vdGlmeV91c2VyX2F0IjtpOjQxMDI0NDQ4MDA7fX0='; +// base64( serialize( [ 'urn:li:person:E2ETEST' ] ) ) — id decodes to an array instead of a string. +const ARRAY_ID = 'YToxOntpOjA7czoyMToidXJuOmxpOnBlcnNvbjpFMkVURVNUIjt9'; + +/** + * Call a plugin API endpoint from the dashboard page. + * + * @param {import('@playwright/test').Page} page The page object. + * @param {string} req The API method to call. + * @param {Object} body The request payload. + */ +async function callRopApi( page, req, body ) { + return await page.evaluate( async ( { req, body } ) => { + // `root` carries a query string only on plain permalinks; let URL sort + // out `?` vs `&` the way the plugin's own `params` option does. + const url = new URL( window.ropApiSettings.root, window.location.href ); + url.searchParams.set( 'req', req ); + + const response = await fetch( url.toString(), { + method: 'POST', + body: JSON.stringify( body ), + headers: { + 'Content-Type': 'application/json', + 'X-WP-Nonce': window.ropApiSettings.nonce, + }, + } ); + + const text = await response.text(); + let json = null; + try { + json = JSON.parse( text ); + } catch ( e ) { + // Non-JSON response, e.g. the plugin's ROP_DEBUG output. + } + + return { status: response.status, body: json, text }; + }, { req, body } ); +} + +test.describe( 'LinkedIn error handling (issue #1098)', () => { + + test.beforeEach( async ( { page, admin, ropUtils } ) => { + // Start every scenario with no service registered. The happy-path test + // adds a LinkedIn account and its own cleanup does not run when it + // fails, so a retry would otherwise inherit that account. + await ropUtils.reset(); + await admin.visitAdminPage( '/admin.php?page=TweetOldPost' ); + await page.waitForSelector( '.tab-view[type="accounts"]' ); + } ); + + test( 'error payload without pages is rejected without a fatal error', async ( { page } ) => { + const response = await callRopApi( page, 'add_account_li', { id: VALID_ID } ); + + // Before the fix this fataled (array_pop on bool) and surfaced as a + // WordPress critical-error response. After the fix the payload is + // rejected by validation: with ROP_DEBUG on the plugin answers with + // its debug text, in production with a JSON code 400 response. + expect( response.text ).not.toContain( 'critical error' ); + if ( response.body ) { + expect( response.body.code ).toBe( '400' ); + } else { + expect( response.text ).toContain( 'Value not set' ); + } + + // The dashboard must survive the failed attempt. + await page.reload(); + await page.waitForSelector( '.tab-view[type="accounts"]' ); + await expect( page.getByRole( 'button', { name: 'LinkedIn' } ) ).toBeVisible(); + } ); + + test( 'array-valued id and pages are rejected without a fatal error', async ( { page } ) => { + // `is_set_not_empty()` accepts arrays, so these reach `base64_decode()` + // and used to raise a PHP 8 TypeError before the string guard. + const response = await callRopApi( page, 'add_account_li', { + id: [ VALID_ID ], + pages: [ VALID_PAGES ], + } ); + + expect( response.text ).not.toContain( 'critical error' ); + expect( response.status ).toBe( 200 ); + expect( response.body.code ).toBe( '400' ); + } ); + + test( 'garbled pages payload is rejected without a fatal error', async ( { page } ) => { + const response = await callRopApi( page, 'add_account_li', { + id: VALID_ID, + pages: btoa( 'linkedin-error-string-not-account-data' ), + } ); + + expect( response.status ).toBe( 200 ); + expect( response.body.code ).toBe( '400' ); + + // No LinkedIn service must have been registered from the bad payload. + const services = await callRopApi( page, 'get_authenticated_services', {} ); + const serviceNames = Object.values( services.body.data || {} ).map( ( s ) => s.service ); + expect( serviceNames ).not.toContain( 'linkedin' ); + } ); + + test( 'empty payload is rejected without a fatal error', async ( { page } ) => { + const response = await callRopApi( page, 'add_account_li', {} ); + + expect( response.text ).not.toContain( 'critical error' ); + if ( response.body ) { + expect( response.body.code ).toBe( '400' ); + } else { + expect( response.text ).toContain( 'Value not set' ); + } + } ); + + test( 'pages without a notify entry are rejected', async ( { page } ) => { + const response = await callRopApi( page, 'add_account_li', { + id: VALID_ID, + pages: PAGES_WITHOUT_NOTIFY, + } ); + + expect( response.status ).toBe( 200 ); + expect( response.body.code ).toBe( '400' ); + } ); + + test( 'pages with only a notify entry and no accounts are rejected', async ( { page } ) => { + const response = await callRopApi( page, 'add_account_li', { + id: VALID_ID, + pages: PAGES_ONLY_NOTIFY, + } ); + + expect( response.status ).toBe( 200 ); + expect( response.body.code ).toBe( '400' ); + + const services = await callRopApi( page, 'get_authenticated_services', {} ); + const serviceNames = Object.values( services.body.data || {} ).map( ( s ) => s.service ); + expect( serviceNames ).not.toContain( 'linkedin' ); + } ); + + test( 'account entry that is not an array is rejected', async ( { page } ) => { + const response = await callRopApi( page, 'add_account_li', { + id: VALID_ID, + pages: PAGES_WITH_STRING_ACCOUNT, + } ); + + expect( response.status ).toBe( 200 ); + expect( response.body.code ).toBe( '400' ); + } ); + + test( 'id that does not decode to a string is rejected', async ( { page } ) => { + const response = await callRopApi( page, 'add_account_li', { + id: ARRAY_ID, + pages: VALID_PAGES, + } ); + + expect( response.status ).toBe( 200 ); + expect( response.body.code ).toBe( '400' ); + } ); + + test( 'rejected payload leaves the LinkedIn error in the plugin log', async ( { page } ) => { + // Earlier tests in this spec log the same entry — clear the log first + // so the assertion can only be satisfied by this request. + await callRopApi( page, 'get_log', { force: true } ); + + await callRopApi( page, 'add_account_li', { + id: VALID_ID, + pages: btoa( 'linkedin-error-string-not-account-data' ), + } ); + + const log = await callRopApi( page, 'get_log', {} ); + expect( JSON.stringify( log.body.data ) ).toContain( 'Linkedin Error' ); + } ); + + test( 'valid payload still adds the account', async ( { page } ) => { + const response = await callRopApi( page, 'add_account_li', { + id: VALID_ID, + pages: VALID_PAGES, + } ); + + expect( response.status ).toBe( 200 ); + expect( response.body.code ).toBe( '200' ); + + // The service must actually be registered, with the account exposed. + const services = await callRopApi( page, 'get_authenticated_services', {} ); + const linkedin = Object.values( services.body.data || {} ).find( + ( s ) => s.service === 'linkedin' + ); + expect( linkedin ).toBeTruthy(); + expect( JSON.stringify( linkedin.available_accounts ) ).toContain( 'E2E Test User' ); + + // Clean up so other specs start from a pristine accounts state. + const reset = await callRopApi( page, 'reset_accounts', {} ); + expect( reset.body.code ).toBe( '200' ); + } ); +} ); diff --git a/vue/src/vue-elements/sign-in-btn.vue b/vue/src/vue-elements/sign-in-btn.vue index db9199f1f..bf854282f 100644 --- a/vue/src/vue-elements/sign-in-btn.vue +++ b/vue/src/vue-elements/sign-in-btn.vue @@ -867,7 +867,23 @@ export default { return; } - const accountData = JSON.parse(event.data); + let accountData; + try { + accountData = JSON.parse(event.data); + } catch (e) { + this.is_loading = false; + window.removeEventListener("message", this.getChildWindowMessage ); + Vue.$log.error('Received a malformed message from the authorization window', e); + return; + } + + if ( ! accountData || 'object' !== typeof accountData || accountData.error ) { + this.is_loading = false; + window.removeEventListener("message", this.getChildWindowMessage ); + Vue.$log.error('Authorization failed', accountData); + return; + } + const serviceName = this.modal.serviceName; let storing;