SuiteCRM
SuiteCRM copied to clipboard
Fix #7854 Adds authorization code grant type flow to Api V8 OAuth2
As discussed in #7854 this adds the OAuth2 authorization code grant type flow to the Api V8. The code leverages thephpleague's OAuth2 Server and follows the implementation guidelines.
The PR covers the following main changes:
- It adds a new module to store OAuth2 authorization codes. The module key is
OAuth2AuthCodes
- It implements the authorization code grant flow in the Api V8 middleware and stores the authorization codes in the newly created OAuth2AuthCodes module
The OAuth2 authorization code flow includes the following steps:
- A client redirects the user to the SuiteCRM instance (path-to-instance/Api/authorize) with a client_id and redirect_uri parameter.
- SuiteCRM validates that the client_id and the redirect_uri correspond to a valid OAuth2 Client and redirects the user to the login page (if not logged-in already). Once logged-in the user is requested to authorize the client. This takes place on a custom view (view.authorize.php) in the new module. The user can: a. decline the request, which redirects the user back to the client (the redirect contains an approapriate hint) b. authorize the client, which creates a new authorization code. The code is saved as a new record in the OAuth2AuthCode module. The user is redirected to the client. The redirect url contains the authorization code.
- The client than sends a direct request containing the authorization code to SuiteCRMs
Api/access_token
access point. The OAuth2 Server then again validates the client and additionally the client secret and the authorization code. If both are valid and match a new access_token on behalf of the authorizing user is returned. This access_token now grants the client access to the V8 Api. The client thereby acts as the authorizing user.
Motivation and Context
Access to the V8 Api is currently granted by either a client credentials grant or a password grant. The client credentials grant has the downside, that it allows to access the Api on behalf of a dedicated user only (the user is defined in the admin area). The password grant has the downside that the client system needs to handle the user's SuiteCRM password to obtain access to the Api, which is a potential security risk. The authoriazion code grant solves these issues.
Additional feature and implementation description
-
In line with similar implementations in other systems, we offer the user a third option (besides authorizing and declining the request): The user can choose to save the authorization decision for future requests. We thenflag the OAuth2AuthCode record. The flag first prevents that the record is deleted once the authorization code is expired (the code though does expire). Second, if a new authorization code is requested by that particular client AND if the corresponding user is logged-in into SuiteCRM a new authorization code is returned without showing the decision page. The authorize view looks as follows (my color scheme is blue...):
-
The OAuth2AuthCodes module appears in the module list, because users can access the list view. For normal users, the list view lists the authorization codes for that particular user. For admins, the list view lists all authorization codes. The list view is accessible for normal users, because they can revoke their authorization codes there. This is in particular relevant for the authoriation codes that have been saved for automatic approval of future requests (see point 4).
-
If a client redirects a user to the SuiteCRM instance (to obtain an auth code) and if that user is not logged-in already, the user will be redirected to the login page (the user logs-in in order to obtain the authorization code for the client). In this case, the user is logged-out automatically once the authorization request has been approved or declined. This is because we do not want to keep a logged-in SuiteCRM session, the user might not be aware of, because once the user is redirected to the client, there is no open SuiteCRM window any more.
How To Test This
- The OAuth2AuthCode module should be visible, if activated in the menu.
- Run the Api V8 activation robo task
- Create a new authoriation code client in the admin OAuth2 Clients section
- Ensure that the htaccess RewriteRule is in place.
- Access:
path-to-instance/Api/authorize?response_type=code&client_id=[client identifier]&redirect_uri=[redirect_url]
- Once an authoriation code is returned, you can use this auth code with the client id and the client secret to obtain an access token. This can be tested in Postman.
Please ask, if there are any questions! I'm happy to furhter explain and improve the code in case of any recommendations.
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Final checklist
- [x] My code follows the code style of this project found here.
- [x] My change requires a change to the documentation.
- [x] I have read the How to Contribute guidelines.
Also looks like Travis is failing the acceptance tests:
1) CompanyModuleCest: Select record from list view
Test tests/acceptance/Core/CompanyModuleCest.php:testScenarioViewRecordFromListView
[Facebook\WebDriver\Exception\NoSuchElementException] no such element: Unable to locate element: {"method":"css selector","selector":".listViewBody"}
(Session info: headless chrome=78.0.3904.108)
(Driver info: chromedriver=2.36.540471 (9c759b81a907e70363c6312294d30b6ccccc2752),platform=Linux 4.15.0-1028-gcp x86_64)
Scenario Steps:
15. $I->waitForElementVisible(".listViewBody") at tests/_support/Step/Acceptance/ListView.php:51
14. $I->click("CompanyTestModule","#toolbar.desktop-toolbar > ul.nav.navb...") at tests/_support/Step/Acceptance/NavigationBar.php:95
13. $I->waitForElementVisible("#toolbar.desktop-toolbar > ul.nav.navbar-n...") at tests/_support/Step/Acceptance/NavigationBar.php:94
12. $I->click("All","#toolbar.desktop-toolbar > ul.nav.navbar-nav > li.to...") at tests/_support/Step/Acceptance/NavigationBar.php:92
11. $I->waitForElementVisible("#toolbar.desktop-toolbar > ul.nav.navbar-n...") at tests/_support/Step/Acceptance/NavigationBar.php:91
10. $I->executeJS("return Math.max(document.documentElement.clientWidth, w...") at tests/_support/Page/Design.php:50
#1 /home/travis/build/salesagility/SuiteCRM/vendor/facebook/webdriver/lib/Exception/WebDriverException.php:102
#2 /home/travis/build/salesagility/SuiteCRM/vendor/facebook/webdriver/lib/Remote/HttpCommandExecutor.php:320
#3 /home/travis/build/salesagility/SuiteCRM/vendor/facebook/webdriver/lib/Remote/RemoteWebDriver.php:535
#4 /home/travis/build/salesagility/SuiteCRM/vendor/facebook/webdriver/lib/Remote/RemoteWebDriver.php:176
#5 /home/travis/build/salesagility/SuiteCRM/vendor/facebook/webdriver/lib/WebDriverExpectedCondition.php:186
#6 Facebook\WebDriver\WebDriverExpectedCondition::Facebook\WebDriver\{closure}
#7 /home/travis/build/salesagility/SuiteCRM/vendor/facebook/webdriver/lib/WebDriverWait.php:67
#8 Codeception\Module\WebDriver->waitForElementVisible
#9 /home/travis/build/salesagility/SuiteCRM/tests/_support/_generated/AcceptanceTesterActions.php:2386
#10 /home/travis/build/salesagility/SuiteCRM/tests/_support/Step/Acceptance/ListView.php:51
Codecov Report
Merging #8291 into hotfix-7.10.x will increase coverage by
0.01%
. The diff coverage is3.01%
.
@@ Coverage Diff @@
## hotfix-7.10.x #8291 +/- ##
=================================================
+ Coverage 10.57% 10.58% +0.01%
=================================================
Files 3226 3240 +14
Lines 239933 240356 +423
=================================================
+ Hits 25361 25444 +83
- Misses 214572 214912 +340
I just fould and solved the issue that caused Travis to fail.
Here is an overview of what got changed by this pull request:
Issues
======
- Added 6
See the complete overview on Codacy
Hi SalesAgility team, how do we proceed with this PR? Do I have to rebase to a different branch? I have this code running in a production system. Merging this PR in one of the next releases would simplify the update process on this production system significantly. Thanks!
Codecov Report
Merging #8291 into hotfix-7.10.x will decrease coverage by
0.00%
. The diff coverage is3.95%
.
@@ Coverage Diff @@
## hotfix-7.10.x #8291 +/- ##
=================================================
- Coverage 10.70% 10.69% -0.01%
=================================================
Files 3230 3243 +13
Lines 241058 241384 +326
=================================================
+ Hits 25798 25811 +13
- Misses 215260 215573 +313
I've resolved conflics that emerged due to recent updates on the hotfix branch. @Dillon-Brown @code-ph0y: Could you please take another look at this one, so that we can proceed? As mentioned previously, we're using this in a production system. Updates to that system would become much easier, if this PR get's merged... Thanks!
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.
:x: HamburgischerVereinSeefahrt
:x: code-ph0y
You have signed the CLA already but the status is still pending? Let us recheck it.