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

TPROD-170: Enabling 2-legged authentication for Mautic #9837

Merged
merged 4 commits into from
Mar 29, 2021

Conversation

dongilbert
Copy link
Member

Q A
Branch? "features"
Bug fix? no
New feature? yes
Deprecations? no
BC breaks? yes
Automated tests included? yes
Related user documentation PR URL mautic/mautic-documentation#... PENDING
Related developer documentation PR URL mautic/developer-documentation#... PENDING
Issue(s) addressed Fixes TPROD-170

Description:

This PR adds support for a 2-legged OAuth2 authentication to the Mautic API using the client_credentials grant.

Steps to test this PR:

  1. Load up this PR
  2. Login as admin
  3. Go to Settings > API credentials
  4. Create new set of credentials
  5. Generate access_token using the endpoint oauth/v2/token (Postman would be a good way to do this)
  6. Use the following pattern in the body:
    { "grant_type": "client_credentials", "client_id": "YOUR_CLIENT_ID_FROM_STEP_4", "client_secret": "YOUR_CLIENT_SECRET_FROM_STEP_4" }
  7. You should receive access_token in the response. Copy that.
  8. Send an API request on any of the resources. For e.g: api/assets. In the header, send the Authorization header with the above token
  9. You should receive API response without any error.

BC Breaks

WIP

Add prefix to table name in foreign key constraint

Adding unit test

Fixing namespace issue in test class

Fixing migration issue and adding AuditLog username

Making user_id field nullable for client_credentials grant type

Cascade persist role in oauth_client

Fixing role_id issue in oauth2_clients table

Tweaked the audit log name and captured first/last name for "created by metadata"
@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Mar 26, 2021
@dongilbert dongilbert added this to the Mautic 4.0 milestone Mar 26, 2021
@dongilbert dongilbert changed the title [WIP] Enabling 2-legged authentication for Mautic [WIP] TPROD-170: Enabling 2-legged authentication for Mautic Mar 26, 2021
@codecov
Copy link

codecov bot commented Mar 26, 2021

Codecov Report

Merging #9837 (6707d57) into features (55f5802) will increase coverage by 0.02%.
The diff coverage is 79.31%.

Impacted file tree graph

@@              Coverage Diff               @@
##             features    #9837      +/-   ##
==============================================
+ Coverage       39.28%   39.31%   +0.02%     
- Complexity      34302    34310       +8     
==============================================
  Files            2035     2035              
  Lines          108409   108437      +28     
==============================================
+ Hits            42592    42630      +38     
+ Misses          65817    65807      -10     
Impacted Files Coverage Δ Complexity Δ
.../bundles/ApiBundle/Controller/ClientController.php 0.00% <0.00%> (ø) 36.00 <0.00> (+2.00)
app/bundles/ApiBundle/Model/ClientModel.php 10.20% <ø> (ø) 27.00 <0.00> (ø)
app/bundles/ApiBundle/Entity/oAuth2/Client.php 51.25% <72.72%> (+19.36%) 20.00 <3.00> (+3.00)
...pp/bundles/ApiBundle/Entity/oAuth2/AccessToken.php 80.55% <100.00%> (+13.88%) 6.00 <0.00> (ø)
...undle/Security/Firewall/AuthenticationListener.php 53.52% <100.00%> (+11.41%) 24.00 <2.00> (+3.00)

@RCheesley RCheesley added API Anything related to the API configuration Anything related to the Mautic configuration section feature A new feature for inclusion in minor or major releases T2 Medium difficulty to fix (issue) or test (PR) labels Mar 26, 2021
@RCheesley
Copy link
Sponsor Member

When I switch to OAuth2 in the dropdown with this PR applied before doing anything I get the following error:

screenshot-mautic4 ddev site-2021 03 28-16_23_12

[2021-03-28 15:23:05] mautic.ERROR: Doctrine\DBAL\Exception\InvalidFieldNameException: An exception occurred while executing 'SELECT o0_.id AS id_0, o0_.name AS name_1, o0_.random_id AS random_id_2, o0_.secret AS secret_3, o0_.redirect_uris AS redirect_uris_4, o0_.allowed_grant_types AS allowed_grant_types_5, o0_.role_id AS role_id_6 FROM oauth2_clients o0_':

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'o0_.role_id' in 'field list' - in file /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php - at line 79  

Perhaps the PR was not yet finished, or this may be a bug.

@RCheesley
Copy link
Sponsor Member

RCheesley commented Mar 28, 2021

Creating the credentials I get this error:

[2021-03-28 15:26:27] mautic.CRITICAL: Uncaught PHP Exception Doctrine\DBAL\Exception\InvalidFieldNameException: "An exception occurred while executing 'INSERT INTO oauth2_clients (name, random_id, secret, redirect_uris, allowed_grant_types, role_id) VALUES (?, ?, ?, ?, ?, ?)' with params ["Test Creds", "4mwdkdg5zw6co8owgwgok084s40wcggc4gcssg8w4gc8ck840c", "2ctwfpccoejo8888g8scowk4s8c84wswwcgssocksk0g4os804", "a:1:{i:0;s:39:\"https:\/\/mautic4.ddev.site\/index_dev.php\";}", "a:3:{i:0;s:18:\"authorization_code\";i:1;s:13:\"refresh_token\";i:2;s:18:\"client_credentials\";}", 8]:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'role_id' in 'field list'" at /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php line 79 {"exception":"[object] (Doctrine\\DBAL\\Exception\\InvalidFieldNameException(code: 0): An exception occurred while executing 'INSERT INTO oauth2_clients (name, random_id, secret, redirect_uris, allowed_grant_types, role_id) VALUES (?, ?, ?, ?, ?, ?)' with params [\"Test Creds\", \"4mwdkdg5zw6co8owgwgok084s40wcggc4gcssg8w4gc8ck840c\", \"2ctwfpccoejo8888g8scowk4s8c84wswwcgssocksk0g4os804\", \"a:1:{i:0;s:39:\\\"https:\\/\\/mautic4.ddev.site\\/index_dev.php\\\";}\", \"a:3:{i:0;s:18:\\\"authorization_code\\\";i:1;s:13:\\\"refresh_token\\\";i:2;s:18:\\\"client_credentials\\\";}\", 8]:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'role_id' in 'field list' at /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php:79, Doctrine\\DBAL\\Driver\\PDO\\Exception(code: 42S22): SQLSTATE[42S22]: Column not found: 1054 Unknown column 'role_id' in 'field list' at /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDO/Exception.php:18, PDOException(code: 42S22): SQLSTATE[42S22]: Column not found: 1054 Unknown column 'role_id' in 'field list' at /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOStatement.php:115)
[stacktrace]
#0 /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php(182): Doctrine\\DBAL\\Driver\\AbstractMySQLDriver->convertException('An exception oc...', Object(Doctrine\\DBAL\\Driver\\PDO\\Exception))
#1 /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php(159): Doctrine\\DBAL\\DBALException::wrapException(Object(Doctrine\\DBAL\\Driver\\PDO\\MySQL\\Driver), Object(Doctrine\\DBAL\\Driver\\PDO\\Exception), 'An exception oc...')
#2 /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php(2121): Doctrine\\DBAL\\DBALException::driverExceptionDuringQuery(Object(Doctrine\\DBAL\\Driver\\PDO\\MySQL\\Driver), Object(Doctrine\\DBAL\\Driver\\PDO\\Exception), 'INSERT INTO oau...', Array)
#3 /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Statement.php(173): Doctrine\\DBAL\\Connection->handleExceptionDuringQuery(Object(Doctrine\\DBAL\\Driver\\PDO\\Exception), 'INSERT INTO oau...', Array, Array)
#4 /var/www/html/vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php(287): Doctrine\\DBAL\\Statement->execute()
#5 /var/www/html/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php(1087): Doctrine\\ORM\\Persisters\\Entity\\BasicEntityPersister->executeInserts()
#6 /var/www/html/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php(400): Doctrine\\ORM\\UnitOfWork->executeInserts(Object(Doctrine\\ORM\\Mapping\\ClassMetadata))
#7 /var/www/html/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php(368): Doctrine\\ORM\\UnitOfWork->commit(Object(Mautic\\ApiBundle\\Entity\\oAuth2\\Client))
#8 /var/www/html/app/bundles/CoreBundle/Entity/CommonRepository.php(790): Doctrine\\ORM\\EntityManager->flush(Object(Mautic\\ApiBundle\\Entity\\oAuth2\\Client))
#9 /var/www/html/app/bundles/CoreBundle/Model/FormModel.php(111): Mautic\\CoreBundle\\Entity\\CommonRepository->saveEntity(Object(Mautic\\ApiBundle\\Entity\\oAuth2\\Client))
#10 /var/www/html/app/bundles/ApiBundle/Controller/ClientController.php(222): Mautic\\CoreBundle\\Model\\FormModel->saveEntity(Object(Mautic\\ApiBundle\\Entity\\oAuth2\\Client))
#11 /var/www/html/app/bundles/CoreBundle/Controller/CommonController.php(458): Mautic\\ApiBundle\\Controller\\ClientController->newAction(0, '')
#12 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(158): Mautic\\CoreBundle\\Controller\\CommonController->executeAction('new', 0, 0, '')
#13 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(80): Symfony\\Component\\HttpKernel\\HttpKernel->handleRaw(Object(Symfony\\Component\\HttpFoundation\\Request), 1)
#14 /var/www/html/vendor/symfony/http-kernel/Kernel.php(201): Symfony\\Component\\HttpKernel\\HttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#15 /var/www/html/app/AppKernel.php(104): Symfony\\Component\\HttpKernel\\Kernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#16 /var/www/html/app/middlewares/CORSMiddleware.php(91): AppKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#17 /var/www/html/app/middlewares/CatchExceptionMiddleware.php(43): Mautic\\Middleware\\CORSMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#18 /var/www/html/app/middlewares/Dev/IpRestrictMiddleware.php(61): Mautic\\Middleware\\CatchExceptionMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#19 /var/www/html/app/middlewares/VersionCheckMiddleware.php(67): Mautic\\Middleware\\Dev\\IpRestrictMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#20 /var/www/html/app/middlewares/TrustMiddleware.php(51): Mautic\\Middleware\\VersionCheckMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#21 /var/www/html/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Mautic\\Middleware\\TrustMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#22 /var/www/html/vendor/stack/run/src/Stack/run.php(13): Stack\\StackedHttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request))
#23 /var/www/html/index_dev.php(25): Stack\
un(Object(Stack\\StackedHttpKernel))
#24 {main}
"} 

I think that the PR is indeed still a WIP, maybe @dongilbert did not have time to get it completed on Friday? Or maybe there is a missing step in the test instructions - will double check for migrations etc.

@RCheesley
Copy link
Sponsor Member

Blocked from testing due to this issue #9848 which is currently preventing migrations being applied - should be fixed soon though!

@alanhartless
Copy link
Contributor

@RCheesley I pushed the commit that fixed the support for M2 schema.

Copy link
Member

@mabumusa1 mabumusa1 left a comment

Choose a reason for hiding this comment

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

I used the test case listed, everything worked as expected as shown in the screenshots below. I managed to get an access code and make an API call

image

image

@mabumusa1 mabumusa1 added the pending-test-confirmation PR's that require one test before they can be merged label Mar 28, 2021
Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

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

Works for me 👍

@RCheesley require doctrine migrations

image

@kuzmany kuzmany added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Mar 29, 2021
@RCheesley
Copy link
Sponsor Member

Thanks @mabumusa1 and @kuzmany for testing! I was blocked by the bug that prevented applying migrations - this has been fixed in #9850

@kuzmany
Copy link
Member

kuzmany commented Mar 29, 2021

We should remove required validation from Redirect URI in M4 beta. Now it's not necessary I guess

image

@RCheesley
Copy link
Sponsor Member

@kuzmany created a new issue for that!

@RCheesley RCheesley merged commit 5e2da4c into mautic:features Mar 29, 2021
@npracht
Copy link
Member

npracht commented Mar 30, 2021

@dongilbert thanks dude ! We absolutely need a PR on developer documentation for that !

@dongilbert dongilbert deleted the TPROD-170 branch March 31, 2021 18:56
@kuzmany
Copy link
Member

kuzmany commented Aug 19, 2021

Do Acquia have already support for API library of this? Or wee need create it?

@RCheesley
Copy link
Sponsor Member

@kuzmany I just asked and they do not use this so we'd have to implement support for this in the community if we need it.

@escopecz escopecz changed the title [WIP] TPROD-170: Enabling 2-legged authentication for Mautic TPROD-170: Enabling 2-legged authentication for Mautic Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Anything related to the API cla-signed The PR contributors have signed the contributors agreement configuration Anything related to the Mautic configuration section feature A new feature for inclusion in minor or major releases ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged T2 Medium difficulty to fix (issue) or test (PR)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants