Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add token exchange update procedure. #845

Merged

Conversation

budzanowski
Copy link
Collaborator

Changes proposed in this Pull Request:

When the plugin gets updated the token exchange procedure should be triggered.

Closes #844 .

Relies on #840.

Detailed test instructions:

  1. Install the current production version and connect.
  2. Complete the setup and connection.
  3. Switch to this branch. Update manually PINTEREST_FOR_WOOCOMMERCE_VERSION to 1.4.0
  4. Add a breakpoint at the update method. Reload the plugin.
  5. Observer the flow.

@budzanowski budzanowski added the changelog: none Skip changelog entry for this PR label Nov 7, 2023
@budzanowski budzanowski self-assigned this Nov 7, 2023
@github-actions github-actions bot added changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement. labels Nov 7, 2023
Base automatically changed from add/token-exchange-endpoint to pinterest-v5-integration-branch November 8, 2023 09:45
src/PluginUpdate.php Outdated Show resolved Hide resolved
@message-dimke
Copy link
Contributor

Hey, @budzanowski !
There is one more issue with the API call headers.

Here:

if ( $request['auth_header'] ) {
$request['headers']['Authorization'] = 'Bearer ' . self::get_token()['access_token'];
}

self::get_token() uses Pinterest_For_Woocommerce()::get_access_token() which returns no token since is looking for v5 token keys not v3.

As a result of Authorization: Bearer I am getting 401 Reconnect to your Pinterest account., access token is not added to the header.

We can redo Base::get_token() to be something like you did recently

public static function get_token() {
	if ( is_null( self::$token ) ) {
		self::$token = Pinterest_For_Woocommerce()::get_data( 'token', true );
	}

	return self::$token;
}

@budzanowski
Copy link
Collaborator Author

Sorry for the hassle. I was too lazy to constantly change branches so I was injecting the v3 token manually for testing.

This latest change ae22258 should fix the issue.

* @return string $token The V3 token.
*/
public static function get_token() {
return Pinterest_For_Woocommerce()::get_data( 'token', true );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, @budzanowski

The last change.

We need to Pinterest/Crypto::decrypt() our [access_token].

Suggested change
return Pinterest_For_Woocommerce()::get_data( 'token', true );
$token = Pinterest_For_Woocommerce()::get_data( 'token', true );
try {
$token['access_token'] = empty( $token['access_token'] ) ? '' : Pinterest\Crypto::decrypt( $token['access_token'] );
} catch ( \Exception $th ) {
/* Translators: The error description */
Pinterest\Logger::log( sprintf( esc_html__( 'Could not decrypt the Pinterest API access token. Try reconnecting to Pinterest. [%s]', 'pinterest-for-woocommerce' ), $th->getMessage() ), 'error' );
}
return $token;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I had my access token in the DB in a plane text.

Co-authored-by: message-dimke <[email protected]>
Copy link
Contributor

@message-dimke message-dimke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected. Token exchange endpoint succeeds and we get v5 token from it. Also it gets saved into a database.

Thank you, @budzanowski !

@budzanowski budzanowski merged commit 817bf04 into pinterest-v5-integration-branch Nov 13, 2023
7 checks passed
@budzanowski budzanowski deleted the add/token-exchange-update-procedure branch November 13, 2023 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: add A new feature, function, or functionality was added. changelog: none Skip changelog entry for this PR type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants