API and Tracking configuration - check that the options exist

[kuzmany]

Please be sure you are submitting this against the staging branch.

Q	A
Bug fix?
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

If we want hide some configuration from API settings, It crash with error. This PR fixed it

500 Internal Server Error - PHP Notice - Undefined index: api_enable_basic_auth

Steps to test this PR:

1. Just code review from devs

Or

Steps to reproduce this issue:

1. Copy https://gist.github.com/kuzmany/278196d75477bd8c41a5a5773ec31190 to app/config/security_local.php (this is security file which hide one of switch button from configurations > API)
2. Clear cache
3. Go to configuration - should return error

Then patch and test again
Try save configuration if everything works properly.

[codecov]

Codecov Report

Merging #8937 into 3.0 will increase coverage by 0.00%.
The diff coverage is 33.33%.

@@            Coverage Diff            @@
##                3.0    #8937   +/-   ##
=========================================
  Coverage     35.24%   35.24%           
- Complexity    27908    27916    +8     
=========================================
  Files          1731     1731           
  Lines         96457    96459    +2     
=========================================
+ Hits          33995    34001    +6     
+ Misses        62462    62458    -4     

Impacted Files	Coverage Δ	Complexity Δ
app/bundles/ApiBundle/Form/Type/ConfigType.php	0.00% <0.00%> (ø)	4.00 <0.00> (+2.00)
...es/PageBundle/Form/Type/ConfigTrackingPageType.php	0.00% <0.00%> (ø)	10.00 <0.00> (+5.00)
...ndles/ApiBundle/EventListener/ConfigSubscriber.php	57.14% <100.00%> (+40.47%)	4.00 <0.00> (+1.00)

[RCheesley]

Testing this locally on 3.0 and am able to enable the API and basic auth error free - maybe I am misunderstanding the test instructions?

Here's a quick screencast: https://youtu.be/a0TLv8L_rLQ

[kuzmany]

@RCheesley I've update steps to reproduce and step to tests.
After that just code review is enought to merge - is really small improvements

[RCheesley]

Thanks for the information - applied the gist as directed, field is hidden exactly as you describe, but no error (have not applied this PR): https://youtu.be/gKsyq0tywLA

[kuzmany]

@RCheesley the error is just on dev mode, because it's just PHP notice. Try look to logs, there should be notice about that error

[RCheesley]

Thanks so much for the extra detail.

I followed those steps but the following error is thrown if I try to enable the API (while the basic auth setting is disabled) so maybe something else is needing attention?

[2020-06-26 18:20:44] mautic.CRITICAL: Uncaught PHP Exception Symfony\Component\Debug\Exception\ContextErrorException: "PHP Notice - Undefined index: api_enable_basic_auth" at /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/bundles/ApiBundle/EventListener/ConfigSubscriber.php line 48 {"exception":"[object] (Symfony\\Component\\Debug\\Exception\\ContextErrorException(code: 0): PHP Notice - Undefined index: api_enable_basic_auth at /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/bundles/ApiBundle/EventListener/ConfigSubscriber.php:48)
[stacktrace]
#0 /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/bundles/ApiBundle/EventListener/ConfigSubscriber.php(48): Mautic\\CoreBundle\\ErrorHandler\\ErrorHandler->handleError(8, 'PHP Notice - Un...', '/Users/ruth.che...', 48, Array)
#1 /Users/ruth.cheesley/Sites/local.mautic3/mautic/vendor/symfony/event-dispatcher/Debug/WrappedListener.php(115): Mautic\\ApiBundle\\EventListener\\ConfigSubscriber->onConfigSave(Object(Mautic\\ConfigBundle\\Event\\ConfigEvent), 'mautic.config_p...', Object(Symfony\\Component\\HttpKernel\\Debug\\TraceableEventDispatcher))
#2 /Users/ruth.cheesley/Sites/local.mautic3/mautic/vendor/symfony/event-dispatcher/EventDispatcher.php(214): Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener->__invoke(Object(Mautic\\ConfigBundle\\Event\\ConfigEvent), 'mautic.config_p...', Object(Symfony\\Component\\HttpKernel\\Debug\\TraceableEventDispatcher))
#3 /Users/ruth.cheesley/Sites/local.mautic3/mautic/vendor/symfony/event-dispatcher/EventDispatcher.php(44): Symfony\\Component\\EventDispatcher\\EventDispatcher->doDispatch(Array, 'mautic.config_p...', Object(Mautic\\ConfigBundle\\Event\\ConfigEvent))
#4 /Users/ruth.cheesley/Sites/local.mautic3/mautic/vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php(143): Symfony\\Component\\EventDispatcher\\EventDispatcher->dispatch('mautic.config_p...', Object(Mautic\\ConfigBundle\\Event\\ConfigEvent))
#5 /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/bundles/ConfigBundle/Controller/ConfigController.php(107): Symfony\\Component\\EventDispatcher\\Debug\\TraceableEventDispatcher->dispatch('mautic.config_p...', Object(Mautic\\ConfigBundle\\Event\\ConfigEvent))
#6 /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/bundles/CoreBundle/Controller/CommonController.php(449): Mautic\\ConfigBundle\\Controller\\ConfigController->editAction(0, '')
#7 /Users/ruth.cheesley/Sites/local.mautic3/mautic/vendor/symfony/http-kernel/HttpKernel.php(151): Mautic\\CoreBundle\\Controller\\CommonController->executeAction('edit', 0, 0, '')
#8 /Users/ruth.cheesley/Sites/local.mautic3/mautic/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\\Component\\HttpKernel\\HttpKernel->handleRaw(Object(Symfony\\Component\\HttpFoundation\\Request), 1)
#9 /Users/ruth.cheesley/Sites/local.mautic3/mautic/vendor/symfony/http-kernel/Kernel.php(200): Symfony\\Component\\HttpKernel\\HttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#10 /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/AppKernel.php(104): Symfony\\Component\\HttpKernel\\Kernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#11 /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/middlewares/CORSMiddleware.php(91): AppKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#12 /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/middlewares/CatchExceptionMiddleware.php(43): Mautic\\Middleware\\CORSMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#13 /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/middlewares/Dev/IpRestrictMiddleware.php(61): Mautic\\Middleware\\CatchExceptionMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#14 /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/middlewares/VersionCheckMiddleware.php(67): Mautic\\Middleware\\Dev\\IpRestrictMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#15 /Users/ruth.cheesley/Sites/local.mautic3/mautic/app/middlewares/TrustMiddleware.php(51): Mautic\\Middleware\\VersionCheckMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#16 /Users/ruth.cheesley/Sites/local.mautic3/mautic/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Mautic\\Middleware\\TrustMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#17 /Users/ruth.cheesley/Sites/local.mautic3/mautic/vendor/stack/run/src/Stack/run.php(13): Stack\\StackedHttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request))
#18 /Users/ruth.cheesley/Sites/local.mautic3/mautic/index_dev.php(25): Stack\
un(Object(Stack\\StackedHttpKernel))
#19 {main}
"} 

Here's a screencast: https://youtu.be/6Rgz0MdRcSY

[kuzmany]

@RCheesley It's the same error. Just patch and see should working now

[RCheesley]

@RCheesley It's the same error. Just patch and see should working now

As demonstrated in the recording, this is happening with the PR applied.

[dennisameling]

Can confirm the bug exists:

• Copy your Gist file to app/config/security_local.php
• Cleared cache
• Opened MAUTIC_URL/index_dev.php
• Clicked Configuration in the menu bar; the following alert showed up:

Then applied this PR and repeated the steps above:

• Applied this PR
• Cleared cache for both prod and dev
• Opened MAUTIC_URL/index_dev.php
• Clicked Configuration in the menu bar
• Basic auth setting is hidden as expected:

@RCheesley could it be that you didn't clear cache in between? Especially after changing config files, clearing cache is crucial

[RCheesley]

The issue I had occurs when changing the state of the API switch and then saving, after the patch is applied. Whizz forward to 4.16 in this recording: https://youtu.be/17NojSpSq6Y

[kuzmany]

@RCheesley I think you didn't patch correctly. Can you check If changes were applied in app/bundles/ApiBundle/Form/Type/ConfigType.php?

[kuzmany]

It's the same improvement like John did here #6701
Code review should suffice.

[RCheesley]

@dennisameling could you test this one including saving the setting change? I am still seeing the error and I have definitely pulled using the GH CLI, cleared the cache and still see the error after saving the configuration change.

[dennisameling]

@RCheesley just tested and I'm also getting the error, but only when using the dev environment. Looks like ConfigSubscriber.php needs to be updated as well with an isset():

[2020-06-29 17:21:07] mautic.CRITICAL: Uncaught PHP Exception Symfony\Component\Debug\Exception\ContextErrorException: "PHP Notice - Undefined index: api_enable_basic_auth" at /var/www/html/app/bundles/ApiBundle/EventListener/ConfigSubscriber.php line 48 {"exception":"[object] (Symfony\\Component\\Debug\\Exception\\ContextErrorException(code: 0): PHP Notice - Undefined index: api_enable_basic_auth at /var/www/html/app/bundles/ApiBundle/EventListener/ConfigSubscriber.php:48)
[stacktrace]
#0 /var/www/html/app/bundles/ApiBundle/EventListener/ConfigSubscriber.php(48): Mautic\\CoreBundle\\ErrorHandler\\ErrorHandler->handleError(8, 'PHP Notice - Un...', '/var/www/html/a...', 48, Array)
#1 /var/www/html/vendor/symfony/event-dispatcher/Debug/WrappedListener.php(115): Mautic\\ApiBundle\\EventListener\\ConfigSubscriber->onConfigSave(Object(Mautic\\ConfigBundle\\Event\\ConfigEvent), 'mautic.config_p...', Object(Symfony\\Component\\HttpKernel\\Debug\\TraceableEventDispatcher))
#2 /var/www/html/vendor/symfony/event-dispatcher/EventDispatcher.php(214): Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener->__invoke(Object(Mautic\\ConfigBundle\\Event\\ConfigEvent), 'mautic.config_p...', Object(Symfony\\Component\\HttpKernel\\Debug\\TraceableEventDispatcher))
#3 /var/www/html/vendor/symfony/event-dispatcher/EventDispatcher.php(44): Symfony\\Component\\EventDispatcher\\EventDispatcher->doDispatch(Array, 'mautic.config_p...', Object(Mautic\\ConfigBundle\\Event\\ConfigEvent))
#4 /var/www/html/vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php(143): Symfony\\Component\\EventDispatcher\\EventDispatcher->dispatch('mautic.config_p...', Object(Mautic\\ConfigBundle\\Event\\ConfigEvent))
#5 /var/www/html/app/bundles/ConfigBundle/Controller/ConfigController.php(107): Symfony\\Component\\EventDispatcher\\Debug\\TraceableEventDispatcher->dispatch('mautic.config_p...', Object(Mautic\\ConfigBundle\\Event\\ConfigEvent))
#6 /var/www/html/app/bundles/CoreBundle/Controller/CommonController.php(449): Mautic\\ConfigBundle\\Controller\\ConfigController->editAction(0, '')
#7 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(151): Mautic\\CoreBundle\\Controller\\CommonController->executeAction('edit', 0, 0, '')
#8 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\\Component\\HttpKernel\\HttpKernel->handleRaw(Object(Symfony\\Component\\HttpFoundation\\Request), 1)
#9 /var/www/html/vendor/symfony/http-kernel/Kernel.php(200): Symfony\\Component\\HttpKernel\\HttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#10 /var/www/html/app/AppKernel.php(104): Symfony\\Component\\HttpKernel\\Kernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#11 /var/www/html/app/middlewares/CORSMiddleware.php(91): AppKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#12 /var/www/html/app/middlewares/CatchExceptionMiddleware.php(43): Mautic\\Middleware\\CORSMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#13 /var/www/html/app/middlewares/Dev/IpRestrictMiddleware.php(61): Mautic\\Middleware\\CatchExceptionMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#14 /var/www/html/app/middlewares/VersionCheckMiddleware.php(67): Mautic\\Middleware\\Dev\\IpRestrictMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#15 /var/www/html/app/middlewares/TrustMiddleware.php(51): Mautic\\Middleware\\VersionCheckMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#16 /var/www/html/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Mautic\\Middleware\\TrustMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#17 /var/www/html/vendor/stack/run/src/Stack/run.php(13): Stack\\StackedHttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request))
#18 /var/www/html/index_dev.php(25): Stack\
un(Object(Stack\\StackedHttpKernel))
#19 {main}
"} 

@kuzmany would you mind making that change? Thanks!!

[TravisBuddy]

Travis tests have failed

Hey @kuzmany,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: cdf34830-bdfd-11ea-a595-8f8781864445

[TravisBuddy]

Travis tests have failed

Hey @kuzmany,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: eff4b290-be00-11ea-a595-8f8781864445

[RCheesley]

Close & open for Travis

[RCheesley]

Thanks for making those changes @kuzmany - confirm that the behaviour is as expected without the errors! 🎉

[TravisBuddy]

Travis tests have failed

Hey @kuzmany,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: 8ef99cd0-bf9f-11ea-8212-0f9c14daf924

[RCheesley]

@kuzmany looks like Travis is reporting some issues - can you take a look at the failed build report?

[dennisameling]

@kuzmany I wrote a simple test for this case so that it should keep working as expected in the future. This will also ensure that the test coverage doesn't decrease because of this PR 😊

Without the PR applied, the test will fail:

denni@mautic3-web:/var/www/html$ bin/phpunit --bootstrap vendor/autoload.php --configuration app/phpunit.xml.dist --filter test app/bundles/ApiBundle/Tests/EventListener/ConfigSubscriberTest.php
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

E.                                                                  2 / 2 (100%)

Time: 104 ms, Memory: 4.00 MB

There was 1 error:

1) Mautic\ApiBundle\Tests\EventListener\ConfigSubscriberTest::testWithUnsetApiBasicAuthSetting
Undefined index: api_enable_basic_auth

/var/www/html/vendor/symfony/phpunit-bridge/DeprecationErrorHandler.php:125
/var/www/html/app/bundles/ApiBundle/EventListener/ConfigSubscriber.php:48
/var/www/html/app/bundles/ApiBundle/Tests/EventListener/ConfigSubscriberTest.php:34

ERRORS!
Tests: 2, Assertions: 1, Errors: 1.

After applying this PR, the test will pass:

denni@mautic3-web:/var/www/html$ bin/phpunit --bootstrap vendor/autoload.php --configuration app/phpunit.xml.dist --filter test app/bundles/ApiBundle/Tests/EventListener/ConfigSubscriberTest.php
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

..                                                                  2 / 2 (100%)

Time: 103 ms, Memory: 4.00 MB

OK (2 tests, 2 assertions)