Skip to content
This repository was archived by the owner on Aug 22, 2025. It is now read-only.

Added feature to create third party access tokens#968

Open
anubhavchaturvedi wants to merge 2 commits into
paypal:masterfrom
anubhavchaturvedi:thirdPartyChanges
Open

Added feature to create third party access tokens#968
anubhavchaturvedi wants to merge 2 commits into
paypal:masterfrom
anubhavchaturvedi:thirdPartyChanges

Conversation

@anubhavchaturvedi

Copy link
Copy Markdown

No description provided.

@jaypatel512 jaypatel512 left a comment

Copy link
Copy Markdown
Contributor

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.

*/
private $cipher;

/**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

encryted -> encrypted

$params['grant_type'] = 'refresh_token';
$params['refresh_token'] = $refreshToken;
}

Copy link
Copy Markdown
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
Copy Markdown
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