-
Notifications
You must be signed in to change notification settings - Fork 123
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
There was a problem hiding this 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 |
There was a problem hiding this comment.
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:
... 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(); |
There was a problem hiding this comment.
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
😊
api-library/lib/Auth/OAuth.php
Line 679 in 9087dde
$this->_expires = time() + $params['expires_in']; |
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.