On my site www.jeffgeerling.com, I upgraded from Drupal 7 to Drupal 8 recently. On both sites, I'm proxying web traffic through Nginx, so I can cache pages on the site.

In my settings.php file I have:

$settings['reverse_proxy'] = TRUE;
$settings['reverse_proxy_addresses'] = ['server.ip.address.here'];

(with my server IP address in the settings there).

It seems like with Drupal 7, CleanTalk was getting the proper IP addresses for form submissions, but in Drupal 8, all submissions are being logged as coming from 104.236.203.61, the IP address of my server.

It seems like the module is not accounting for the reverse_proxy setting, if configured.

Because of this, all my forms keep getting blocked because that IP is put on my spam blacklist/firewall every day or two, after I manually go in and remove it.

Can this module be made to work correctly with the reverse_proxy setting in Drupal 8?

Comments

geerlingguy created an issue. See original summary.

geerlingguy’s picture

It looks like some of the code surrounding proxy IP handling is commented out in the Cleantalk::ct_session_ip() function.

geerlingguy’s picture

My Nginx configuration has X-Forwarded-For and Host configured, and if I look at Drupal's watchdog logs, I see that all the log entries are tied to real IP addresses—before I set up the reverse_proxy settings those logs all reported the server's IP address.

It seems like the code that determines the client IP address in the CleanTalk module could just be updated to use something like \Drupal::request()->getClientIp()

geerlingguy’s picture

It looks like the code inside Drupal\cleantalk\lib\Cleantalk\Common\Helper::ip_get() currently assumes an Apache-based setup, but a lot of that logic should be easily able to be removed using the getClientIp() functionality provided by the framework. I'm going to test out a patch and see if it works for me.

It looks a bit complicated actually, as there's a number of data points sent back to CleanTalk in the $ct_request... since I'm not up to speed on how it's supposed to send the data back I'll hold off on trying to create a patch right now.

Can someone from the CleanTalk team take a look at this? It's causing all my site's comment forms to stop working every day or so, and I'd rather not have to disable CleanTalk as it's worked so well in the past :(

znaeff’s picture

Hello, geerlingguy

Thank you for your request.

Please, give us 1-2 days to investigate the issue.

Best regards.

znaeff’s picture

Hello,

Thank you for your help. We've made a patch. Could you check it?

geerlingguy’s picture

Status: Active » Needs review

I will test the patch for a couple days and see if it works better.

geerlingguy’s picture

The patch doesn't apply to the module, I get:

Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2020-06-03/Upd__Helper__ip_get%28%29_adapted_for_Drupal_8_.patch
geerlingguy’s picture

Status: Needs review » Needs work
geerlingguy’s picture

Status: Needs work » Needs review
StatusFileSize
new1.39 KB

Updated patch attached.

geerlingguy’s picture

Version: 8.x-3.8 » 8.x-3.x-dev
geerlingguy’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new325.42 KB

With the patch applied, the spam logs look correct again, and it also seems that spam is more correctly approved and rejected (I was getting dozens of spam posts per day that got through before the patch, now it seems they're blocked again).

See image:

Spam logs for jeffgeerling.com

znaeff’s picture

Thank you for your feedback. Contact us if you have more questions.

geerlingguy’s picture

Version: 8.x-3.x-dev » 9.1.1
Status: Reviewed & tested by the community » Needs work

This patch is not applying to the 9.1.1 version of the module; when I try updating with Composer I get:

Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2020-06-06/3143365-10-helper-ip-get.patch

I will check and see if I need to manually adjust it for the latest version...

geerlingguy’s picture

Status: Needs work » Fixed

It looks like this patch was merged at some point, so I think this issue can now be closed.

znaeff’s picture

Thank you for the feedback. Contact us anytime.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.