Skip to content
This repository has been archived by the owner on Sep 29, 2023. It is now read-only.

Added feature to create third party access tokens #968

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

anubhavchaturvedi
Copy link

No description provided.

Copy link
Contributor

@jaypatel512 jaypatel512 left a comment

Choose a reason for hiding this comment

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

These are my comments based on my very limited understanding of subject based on this PR.

@@ -76,16 +76,25 @@ class OAuthTokenCredential extends PayPalResourceModel
private $cipher;

/**
* The encryted account number of the merchant on whose behalf the transaction is being done
Copy link
Contributor

Choose a reason for hiding this comment

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

encryted -> encrypted

@@ -267,6 +286,11 @@ private function generateAccessToken($config, $refreshToken = null)
$params['grant_type'] = 'refresh_token';
$params['refresh_token'] = $refreshToken;
}

if ($this->subject != null && $refreshToken != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If subject is associated with a refreshToken, should we just pass the subject as an argument here, instead of adding it to a constructor ?

$cred = new OAuthTokenCredential('clientId', 'clientSecret', 'subject');

//{"clientId":{"clientId":"clientId","accessToken":"accessToken","tokenCreateTime":1421204091,"tokenExpiresIn":288000000}}
AuthorizationCache::push($config, 'clientId', $cred->encrypt('accessTokenWithSubject'), 1421204091, 288000000);
Copy link
Contributor

Choose a reason for hiding this comment

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

If subject is relevant only for third party tokens (refresh tokens), we should not cache the response. This could lead to all consequent calls with just a clientId to use this subject based access token. This will be because, we only fetch this cache by clientId and nothing else.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants