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

2-legged OAuth2 using the client_credentials #257

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented Aug 26, 2021

This is support for client_credentials grant type added in M4 mautic/mautic#9837

Also I've removed oauth1 mentions in readme. @RCheesley probably need more cleaning.

@cla-bot cla-bot bot added the cla-signed label Aug 26, 2021
@kuzmany kuzmany added the WIP label Aug 26, 2021
@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2021

Codecov Report

Merging #257 (b6e9c52) into main (9087dde) will decrease coverage by 1.12%.
The diff coverage is 0.00%.

❗ Current head b6e9c52 differs from pull request most recent head 6e03aec. Consider uploading reports for the commit 6e03aec to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##               main     #257      +/-   ##
============================================
- Coverage     51.45%   50.33%   -1.13%     
- Complexity      406      415       +9     
============================================
  Files            30       31       +1     
  Lines          1028     1051      +23     
============================================
  Hits            529      529              
- Misses          499      522      +23     
Impacted Files Coverage Δ
lib/Auth/TwoLeggedOAuth2.php 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9087dde...6e03aec. Read the comment docs.

@kuzmany kuzmany added Ready To Test and removed WIP labels Aug 26, 2021
kuzmany added a commit to Webmecanik/api-library that referenced this pull request Aug 30, 2021
Copy link
Member

@dennisameling dennisameling left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! This works correctly in my testing 👍🏼 Just left some comments.

There's also a bunch of old references to OAuth1 in https://github.com/mautic/api-library/blob/9087ddec6400eb0c8aa43859eb7710cc9948c1ce/lib/Auth/OAuth.php - do we want to remove those in this PR on in a follow-up PR maybe?

$auth = $initAuth->newAuth($settings, $settings['AuthMethod']);

if (!isset($settings['accessToken'])) {
// store it for one hour and use it in $settings above
Copy link
Member

@dennisameling dennisameling Sep 29, 2021

Choose a reason for hiding this comment

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

The token isn't always valid for one hour. Mautic users can set the access token lifetime under API settings:

image

... in which case /oauth/v2/token will return an expires_in of, for example, 7200 seconds (2 hrs) instead of 3600 seconds (1 hr):

{
    "access_token": "TOKEN_HERE",
    "expires_in": 7200,
    "token_type": "bearer",
    "scope": null
}


if (!isset($settings['accessToken'])) {
// store it for one hour and use it in $settings above
$accessToken = $auth->getAccessToken();
Copy link
Member

Choose a reason for hiding this comment

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

In the regular OAuth provider, there's a function called getAccessTokenData() which returns an array with access_token, expires, token_type, refresh_token.

Could we have the same method for TwoLeggedOAuth2 please, to keep things consistent? It can return access_token, expires, token_type. Especially expires is interesting here, because folks can use it to store when the token expires and they need to renew it 😊

You could store the expiration time just like it was done in lib/Auth/OAuth.php 😊

$this->_expires = time() + $params['expires_in'];

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

Successfully merging this pull request may close these issues.

None yet

4 participants