PISHPS-328: added ApplePayDirectDomainAllowList #790

m-muxfeld-diw · 2024-07-09T11:39:15Z

added ApplePayDirectDomainAllowList, integrated allow list into applepay direct validate route, updated documentation

…ault saleschannel urls, integrated allow list into applepay validate route

boxblinkracer · 2024-07-09T12:19:29Z

src/Resources/config/config.xml

@@ -152,6 +152,15 @@
                </option>
            </options>
        </input-field>
+        <input-field type="text">
+            <name>ApplePayValidationAllowList</name>


just for consistency, i think other settings start with lower case?

and please use applePayDirect as prefix, because non-direct would be another payment method

boxblinkracer · 2024-07-09T12:21:26Z

src/Resources/config/config.xml

+        <input-field type="text">
+            <name>ApplePayValidationAllowList</name>
+            <label>Allowed Domains for Apple Pay Validation</label>
+            <label lang="de-DE">Zulässige Domains für die Apple Pay Validierung</label>


could you add something like "leave empty if no separate headless ...stuff with additional domain is used"
or "only required if a custom domain is used for a headless storefront"
just to avoid people getting confused and thinking this needs to be added
one of the mollie usps is a plugn'play approach without lots of required configs

currently it is always required but the ApplePayConfigSubscriber adds all associated domains of the shop automatically to the configuration on install/update

boxblinkracer · 2024-07-09T12:23:05Z

src/Components/ApplePayDirect/ApplePayDirect.php

+     * @throws ApplePayValidationUrlNotInAllowListException
+     * @return string
+     */
+    public function validateValidationUrl(string $validationUrl): string


validate DOMAIN? or?
validation URL is something else, we need to validate the domain
which also means we should rename the whole member variables and probably classes?
(we can talk in zoom if you want)

also why do we require a string return value? its no "getter" it just validates with exception approach, meaning void is totally fine?
so sanitizing might be better as a second function? function naming and content is a bit different i think?

boxblinkracer · 2024-07-09T12:25:27Z

src/Components/ApplePayDirect/Gateways/ApplePayValidationUrlAllowListGateway.php

+     */
+    public function getAllowList(): ApplePayValidationUrlAllowList
+    {
+        $allowList = $this->systemConfigService->get('MolliePayments.config.ApplePayValidationAllowList');


please use our own SettingsService to avoid duplicate hardcoded config strings

boxblinkracer · 2024-07-09T12:29:23Z

src/Components/ApplePayDirect/Subscriber/ApplePayConfigSubscriber.php

+     * @param SystemConfigService $systemConfigService
+     * @param EntityRepository $salesChannelDomainRepository
+     */
+    public function __construct(SystemConfigService $systemConfigService, EntityRepository $salesChannelDomainRepository)


i dont see any benefit in this?
we get the string anyway if we use the settings service when we need this?!

boxblinkracer · 2024-07-09T12:30:41Z

src/Controller/StoreApi/ApplePayDirect/ApplePayDirectControllerBase.php

@@ -117,6 +117,7 @@ public function createPaymentSession(RequestDataBag $data, SalesChannelContext $
            throw new \Exception('Please provide a validation url!');
        }

+        $validationURL = $this->applePay->validateValidationUrl($validationURL);


i think thats wrong
ApplePayDirect.php::createPaymentSession
in this fucntion we should verify the domain,
the validationUrl comes as response from apple and must not be touched

boxblinkracer · 2024-07-09T12:31:35Z

tests/PHPUnit/Components/ApplePayDirect/Gateways/ApplePayValidationUrlAllowListGatewayTest.php

+
+    public function testProvidesAllowList(): void
+    {
+        $allowListString = 'https://example.com,https://example-url.org';


great test
could you also add a simple domain or a separate unit test where it loads just domains without https scheme?

boxblinkracer · 2024-07-09T12:32:03Z

.../PHPUnit/Components/ApplePayDirect/Models/Collections/ApplePayValidationUrlAllowListTest.php

+
+class ApplePayValidationUrlAllowListTest extends TestCase
+{
+    public function canDetermineIfValueIsContainedInList(): void


attention
tests without "test" prefix will not be executed

boxblinkracer · 2024-07-09T12:33:12Z

tests/PHPUnit/Components/ApplePayDirect/Services/ApplePayValidationUrlSanitizerTest.php

+    public function sanitationTestDataProvider(): array
+    {
+        return [
+            'keeps http value if provided' => ['http://example.com', 'http://example.com/'],


not quite sure if trailing slash is good?

boxblinkracer · 2024-07-09T12:33:47Z

tests/PHPUnit/Service/ApplePayDirect/ApplePayDirectTest.php

+            ->willReturn(ApplePayValidationUrlAllowList::create());
+
+        $this->expectException(ApplePayValidationUrlAllowListCanNotBeEmptyException::class);
+        $this->expectExceptionMessage('The Apple Pay validation URL allow list can not be empty. Please check the configuration.');


thats wrong, it can be empty

ok sorry misleading title, only throws exception if custom domain is provided but allow list is empty :)

boxblinkracer

comments

boxblinkracer · 2024-07-11T06:41:10Z

src/Components/ApplePayDirect/Gateways/ApplePayDirectDomainAllowListGateway.php

+    public function getAllowList(SalesChannelContext $context): ApplePayDirectDomainAllowList
+    {
+        $settings = $this->settingsService->getSettings($context->getSalesChannel()->getId());
+        $allowList = $settings->applePayDirectDomainAllowList ?? '';


achtung, hier sollte ein getter das schon machen, direkt member-access sollte nicht erlaubt sein

boxblinkracer · 2024-07-11T06:44:15Z

src/Components/ApplePayDirect/Models/ApplePayDirectDomainAllowListItem.php

+
+        # we need to have a protocol before the parse url command
+        # in order to have it work correctly
+        if (strpos($value, 'http') !== 0) {


i think you could use the sanitizer in here?

boxblinkracer · 2024-07-11T06:45:43Z

src/Service/SettingsService.php

@@ -18,6 +18,7 @@ class SettingsService implements PluginSettingsServiceInterface
    const TEST_API_KEY = 'testApiKey';
    const LIVE_PROFILE_ID = 'liveProfileId';
    const TEST_PROFILE_ID = 'testProfileId';
+    const APPLE_PAY_DIRECT_DOMAIN_ALLOW_LIST = 'applePayDirectDomainAllowList';


glaub das ist nicht notwendig

aaaa, MollieSettingsStruct is the correct one

boxblinkracer

comments

boxblinkracer · 2024-07-11T11:18:22Z

tests/PHPUnit/Components/ApplePayDirect/Gateways/ApplePayDirectDomainAllowListGatewayTest.php

+
+    public function testProvidesEmptyAllowList(): void
+    {
+        $struct = $this->createConfiguredMock(MollieSettingStruct::class, ['getApplePayDirectDomainAllowList' => '']);


just a styling thing, but maybe something for the future
you dont always need to mock things
you could just create a new setting struct, set your value and use it :)

boxblinkracer · 2024-07-11T11:19:28Z

tests/PHPUnit/Components/ApplePayDirect/Services/ApplePayDirectDomainSanitizerTest.php

+    public function sanitationTestDataProvider(): array
+    {
+        return [
+            'removes http value if provided' => ['http://example.com', 'example.com'],


can you add a subdomain too? that would be good

…sub domains

commit d61758c Author: m-muxfeld-diw <[email protected]> Date: Thu Aug 8 10:06:03 2024 +0200 PISHPS-329: added UrlParsingService to separate tracking code from tracking url when not entered correctly (mollie#805) commit 1549c12 Author: Marcus <[email protected]> Date: Thu Aug 8 10:01:00 2024 +0200 Check for database connection during logger creation instead of dsn (mollie#804) Resolves mollie#803 commit 9f8b001 Author: m-muxfeld-diw <[email protected]> Date: Thu Aug 8 10:00:14 2024 +0200 PISHPS-259: MollieLineItemBuilder now checks if the price definition is a AbsolutePriceDefinition and will set quantity to 1 if it is (mollie#800) commit 4faed11 Author: Niklas Wolf <[email protected]> Date: Tue Aug 6 12:33:24 2024 +0200 fix: load order delivery state machine state (mollie#799) commit 9558e0d Author: Vitalij Mik <[email protected]> Date: Tue Jul 30 14:33:56 2024 +0200 NTR: fix buid zip (mollie#797) Co-authored-by: Vitalij Mik <[email protected]> commit 9fbf487 Author: Vitalij Mik <[email protected]> Date: Tue Jul 30 07:03:44 2024 +0200 NTR: fix cypress for 6.4 (mollie#796) Co-authored-by: Vitalij Mik <[email protected]> commit 5668b08 Author: Vitalij Mik <[email protected]> Date: Mon Jul 29 12:49:21 2024 +0200 NTR: fix cypress for payconiq (mollie#794) Co-authored-by: Vitalij Mik <[email protected]> commit 8b3253b Author: Vitalij Mik <[email protected]> Date: Mon Jul 29 12:38:41 2024 +0200 NTR: fix polyfill autoloading (mollie#795) fix mollie#791 Co-authored-by: Vitalij Mik <[email protected]> commit f2436dc Author: m-muxfeld-diw <[email protected]> Date: Fri Jul 26 16:01:44 2024 +0200 PISHPS-303: extended LineItemDataExtractor. It now also sanitizes query parameters (mollie#789) * PISHPS-303: extended LineItemDataExtractor. It now also sanitizes query parameters * PISHPS-303: removed redundant comments * PISHPS-303: removed debug line * PISHPS-303: fixed phpstan error * PISHPS-303: added missing space commit 39ebd2e Author: m-muxfeld-diw <[email protected]> Date: Fri Jul 26 08:22:42 2024 +0200 PISHPS-328: added ApplePayDirectDomainAllowList (mollie#790) * PISHPS-328: added ApplePayValidationAllowList, subscriber to list default saleschannel urls, integrated allow list into applepay validate route * PISHPS-303: applied requested todos * PISHPS-328: applied requested todos * PISHPS-328: added test case to ApplePayDirectDomainSanitizerTest for sub domains commit 99f0e81 Author: Marcus <[email protected]> Date: Wed Jul 24 13:09:17 2024 +0200 RefundAdminSearchIndexer: Fix MySQL syntax (mollie#792) commit 79e8e7f Author: Christian <[email protected]> Date: Tue Jul 16 10:21:15 2024 +0200 NTR: Add Shopware 6.6.4.1 to pipeline commit b2b7361 Author: Vitalij Mik <[email protected]> Date: Fri Jul 5 09:31:38 2024 +0200 PISHPS-301: add elastic indexer for entities (mollie#786) * NTR: PISHPS-301: add elastic indexer for entities * NTR: update --------- Co-authored-by: Vitalij Mik <[email protected]> commit c843557 Author: Vitalij Mik <[email protected]> Date: Thu Jul 4 11:03:07 2024 +0200 NTR: PISHPS-291: do not create new guest accounts (mollie#784) Co-authored-by: Vitalij Mik <[email protected]> commit eab675b Author: Vitalij Mik <[email protected]> Date: Thu Jul 4 09:47:10 2024 +0200 NTR: fix settings commit e8a5832 Author: Vitalij Mik <[email protected]> Date: Thu Jul 4 09:34:37 2024 +0200 NTR: fix settings commit 72be783 Author: Vitalij Mik <[email protected]> Date: Thu Jul 4 08:12:43 2024 +0200 NTR: prepare hotfix commit 7feec11 Author: Vitalij Mik <[email protected]> Date: Wed Jul 3 12:27:42 2024 +0200 NTR: fix apple pay without phone (mollie#783) Co-authored-by: Vitalij Mik <[email protected]> commit ac6407d Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 14:14:28 2024 +0200 NTR: prepare release commit a04bacb Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 13:23:46 2024 +0200 NTR: cs fix commit 9241ea6 Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 13:15:12 2024 +0200 NTR: fix save/delete settings commit d51a4c3 Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 12:14:25 2024 +0200 NTR: use get to reduce db requests (mollie#782) Co-authored-by: Vitalij Mik <[email protected]> commit 4a72cb8 Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 11:35:37 2024 +0200 NTR: add payconiq payment (mollie#781) * NTR: added payconiq payment * NTR: csfix * NTR: fix stylelint * NTR: stylelint fix --------- Co-authored-by: Vitalij Mik <[email protected]> commit 3dd7db6 Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 10:27:26 2024 +0200 NTR: PISHPS-300: add trustly payment (mollie#780) Co-authored-by: Vitalij Mik <[email protected]>

* NTR: PISHPS-288: add riverty * NTR: Squashed commit of the following: commit d61758c Author: m-muxfeld-diw <[email protected]> Date: Thu Aug 8 10:06:03 2024 +0200 PISHPS-329: added UrlParsingService to separate tracking code from tracking url when not entered correctly (#805) commit 1549c12 Author: Marcus <[email protected]> Date: Thu Aug 8 10:01:00 2024 +0200 Check for database connection during logger creation instead of dsn (#804) Resolves #803 commit 9f8b001 Author: m-muxfeld-diw <[email protected]> Date: Thu Aug 8 10:00:14 2024 +0200 PISHPS-259: MollieLineItemBuilder now checks if the price definition is a AbsolutePriceDefinition and will set quantity to 1 if it is (#800) commit 4faed11 Author: Niklas Wolf <[email protected]> Date: Tue Aug 6 12:33:24 2024 +0200 fix: load order delivery state machine state (#799) commit 9558e0d Author: Vitalij Mik <[email protected]> Date: Tue Jul 30 14:33:56 2024 +0200 NTR: fix buid zip (#797) Co-authored-by: Vitalij Mik <[email protected]> commit 9fbf487 Author: Vitalij Mik <[email protected]> Date: Tue Jul 30 07:03:44 2024 +0200 NTR: fix cypress for 6.4 (#796) Co-authored-by: Vitalij Mik <[email protected]> commit 5668b08 Author: Vitalij Mik <[email protected]> Date: Mon Jul 29 12:49:21 2024 +0200 NTR: fix cypress for payconiq (#794) Co-authored-by: Vitalij Mik <[email protected]> commit 8b3253b Author: Vitalij Mik <[email protected]> Date: Mon Jul 29 12:38:41 2024 +0200 NTR: fix polyfill autoloading (#795) fix #791 Co-authored-by: Vitalij Mik <[email protected]> commit f2436dc Author: m-muxfeld-diw <[email protected]> Date: Fri Jul 26 16:01:44 2024 +0200 PISHPS-303: extended LineItemDataExtractor. It now also sanitizes query parameters (#789) * PISHPS-303: extended LineItemDataExtractor. It now also sanitizes query parameters * PISHPS-303: removed redundant comments * PISHPS-303: removed debug line * PISHPS-303: fixed phpstan error * PISHPS-303: added missing space commit 39ebd2e Author: m-muxfeld-diw <[email protected]> Date: Fri Jul 26 08:22:42 2024 +0200 PISHPS-328: added ApplePayDirectDomainAllowList (#790) * PISHPS-328: added ApplePayValidationAllowList, subscriber to list default saleschannel urls, integrated allow list into applepay validate route * PISHPS-303: applied requested todos * PISHPS-328: applied requested todos * PISHPS-328: added test case to ApplePayDirectDomainSanitizerTest for sub domains commit 99f0e81 Author: Marcus <[email protected]> Date: Wed Jul 24 13:09:17 2024 +0200 RefundAdminSearchIndexer: Fix MySQL syntax (#792) commit 79e8e7f Author: Christian <[email protected]> Date: Tue Jul 16 10:21:15 2024 +0200 NTR: Add Shopware 6.6.4.1 to pipeline commit b2b7361 Author: Vitalij Mik <[email protected]> Date: Fri Jul 5 09:31:38 2024 +0200 PISHPS-301: add elastic indexer for entities (#786) * NTR: PISHPS-301: add elastic indexer for entities * NTR: update --------- Co-authored-by: Vitalij Mik <[email protected]> commit c843557 Author: Vitalij Mik <[email protected]> Date: Thu Jul 4 11:03:07 2024 +0200 NTR: PISHPS-291: do not create new guest accounts (#784) Co-authored-by: Vitalij Mik <[email protected]> commit eab675b Author: Vitalij Mik <[email protected]> Date: Thu Jul 4 09:47:10 2024 +0200 NTR: fix settings commit e8a5832 Author: Vitalij Mik <[email protected]> Date: Thu Jul 4 09:34:37 2024 +0200 NTR: fix settings commit 72be783 Author: Vitalij Mik <[email protected]> Date: Thu Jul 4 08:12:43 2024 +0200 NTR: prepare hotfix commit 7feec11 Author: Vitalij Mik <[email protected]> Date: Wed Jul 3 12:27:42 2024 +0200 NTR: fix apple pay without phone (#783) Co-authored-by: Vitalij Mik <[email protected]> commit ac6407d Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 14:14:28 2024 +0200 NTR: prepare release commit a04bacb Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 13:23:46 2024 +0200 NTR: cs fix commit 9241ea6 Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 13:15:12 2024 +0200 NTR: fix save/delete settings commit d51a4c3 Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 12:14:25 2024 +0200 NTR: use get to reduce db requests (#782) Co-authored-by: Vitalij Mik <[email protected]> commit 4a72cb8 Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 11:35:37 2024 +0200 NTR: add payconiq payment (#781) * NTR: added payconiq payment * NTR: csfix * NTR: fix stylelint * NTR: stylelint fix --------- Co-authored-by: Vitalij Mik <[email protected]> commit 3dd7db6 Author: Vitalij Mik <[email protected]> Date: Tue Jul 2 10:27:26 2024 +0200 NTR: PISHPS-300: add trustly payment (#780) Co-authored-by: Vitalij Mik <[email protected]> * NTR: csfix --------- Co-authored-by: Vitalij Mik <[email protected]>

PISHPS-328: added ApplePayValidationAllowList, subscriber to list def…

36a96c4

…ault saleschannel urls, integrated allow list into applepay validate route

m-muxfeld-diw requested a review from boxblinkracer July 9, 2024 11:39

boxblinkracer reviewed Jul 9, 2024

View reviewed changes

boxblinkracer requested changes Jul 9, 2024

View reviewed changes

m-muxfeld-diw changed the title ~~PISHPS-328: added ApplePayValidationAllowList~~ PISHPS-328: added ApplePayDirectDomainAllowList Jul 10, 2024

PISHPS-303: applied requested todos

fc8fcc2

m-muxfeld-diw requested a review from boxblinkracer July 10, 2024 07:56

boxblinkracer reviewed Jul 11, 2024

View reviewed changes

boxblinkracer requested changes Jul 11, 2024

View reviewed changes

PISHPS-328: applied requested todos

f792781

m-muxfeld-diw force-pushed the users/mm/PISHPS-328 branch from d7c73ee to f792781 Compare July 11, 2024 08:20

boxblinkracer reviewed Jul 11, 2024

View reviewed changes

boxblinkracer approved these changes Jul 11, 2024

View reviewed changes

PISHPS-328: added test case to ApplePayDirectDomainSanitizerTest for …

ef4a49c

…sub domains

BlackScorp linked an issue Jul 26, 2024 that may be closed by this pull request

Apple Pay Headless - Missing Control over Headless Domain #788

Closed

BlackScorp merged commit 39ebd2e into mollie:master Jul 26, 2024
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PISHPS-328: added ApplePayDirectDomainAllowList #790

PISHPS-328: added ApplePayDirectDomainAllowList #790

m-muxfeld-diw commented Jul 9, 2024 •

edited

Loading

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

m-muxfeld-diw Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer Jul 9, 2024

boxblinkracer left a comment

boxblinkracer Jul 11, 2024

boxblinkracer Jul 11, 2024

boxblinkracer Jul 11, 2024

boxblinkracer Jul 11, 2024

boxblinkracer left a comment

boxblinkracer Jul 11, 2024

boxblinkracer Jul 11, 2024

PISHPS-328: added ApplePayDirectDomainAllowList #790

PISHPS-328: added ApplePayDirectDomainAllowList #790

Conversation

m-muxfeld-diw commented Jul 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boxblinkracer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boxblinkracer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-muxfeld-diw commented Jul 9, 2024 •

edited

Loading