crowdsecurity / cs-magento-bouncer

CrowdSec is an open-source cyber security tool. This module blocks detected attackers or display them a captcha to check they are not bots.
MIT License
6 stars 3 forks source link

[BUG] Cache storage issue #20

Open Nuranto opened 11 months ago

Nuranto commented 11 months ago

Describe the bug A clear and concise description of what the bug is.

Preconditions

To Reproduce Steps to reproduce the behavior:

  1. Go to module's configuration, cache config section

Expected result A clear and concise description of what you expected to happen.

Actual result A clear and concise description of the actual result.

julienloizelet commented 11 months ago

Hi @Nuranto , Thanks for your message.

Regarding Redis, I only found documentation that's say that the PHP Redis extension is required anyway (https://www.cloudways.com/blog/magento-redis/#Magento-Redis-Requirements for example); But maybe it's not ?

Regarding the filesystem mode, can you tell me more about what you mean by "a remote storage like import_export" ? I found the remote storage feature https://experienceleague.adobe.com/docs/commerce-operations/configuration-guide/storage/remote-storage/remote-storage.html?lang=en but I'm not sure this is related.

Thanks again

Nuranto commented 11 months ago

Hi @julienloizelet

Thanks for your quick answer !

For redis, Magento is using https://github.com/colinmollenhour/credis lib, which supports phpredis, but also propose a custom implementation. A lot of Magento users are using this custom implementation, because a lot of bugs occured with phpredis (Although I don't know the current state of this, may be it is stable now.)

For the filesystem, Magento propose a remote storage adapter (S3 or S3-like) which, as redis, prevents the need of shared volumes. Magento comes with two remote folders : media folder, and var/import_export. If you can access it through $filesystem->getDirectoryWrite(DirectoryList::VAR_IMPORT_EXPORT)

Actually, why don't use Magento's cache system directly, by adding a new cache type ? https://developer.adobe.com/commerce/php/development/cache/partial/cache-type/ With that approach, we would'nt have to bother with cache setup anymore when installing your extension. It would also simplify your extension's architecture.

julienloizelet commented 11 months ago

@Nuranto , thank you for this clarification.

Using a new type of Magento cache is certainly a good option. We didn't do it because the Magento module is built on top of a more generic CrowdSec PHP library that provides 3 Symfony "ready-to-use" cache systems (Redis, Filesystem and Memcached). This more generic lib is aimed to be used as a quick starter for implementing a bouncer in various PHP frameworks (WordPress, Drupal, etc). It should be extendable enough to handle a new cache type but it will require some work.

Regarding the filesystem issue, the /var/cache/crowdsec path value is hard-coded . Do you think it could resolve the problem to let a user choose a different path (maybe relative to the var folder) ? If yes, it could be a quick win to add a new setting for this.

What do you think ?

Nuranto commented 11 months ago

Ok, I see.

No changing the path won't solve the issue if you're not using Magento's filesystem classes (without it, it won't use the remote adapter, and still write files locally). I guess we'll have to either disable stream mode, or give another try to phpredis.