Better way record IP so auto renewals from Authorize.net will not fail

[carl-alberto]

Bug Report

User Story

As a GiveWP user with Authorize.net auto-renewal setup, I want the plugin to accurately record the IP addresses so that auto-renewals from Authorize.net will not fail.

Current Behavior

Symptomps will show in the Donation Meta on individual donations page:

Tracing back to the code, one way the plugin records IP is from this code:

https://github.com/impress-org/give/blob/894a98b59843c4566a6a010e23f450c132e6912d/includes/misc-functions.php#L156-L158

Which uses the server variable $_SERVER['HTTP_X_FORWARDED_FOR'], for hosts that use 301 redirects in forcing the HTTPS to load (like in Pantheon) will always give at least 2 comma separate values of IPs that would be normal as per specifications https://en.wikipedia.org/wiki/X-Forwarded-For#Format:

The general format of the field is: X-Forwarded-For: client, proxy1, proxy2 where the value is a comma+space separated list of IP addresses, the left-most being the original client, and each successive proxy that passed the request adding the IP address where it received the request from. In this example, the request passed through proxy1, proxy2, and then proxy3 (not shown in the header). proxy3 appears as remote address of the request.

Expected Behavior

All IPs should be recorded in the DB as the correct format

Bug Type

• [ ] This bug describes functionality that once worked as expected in version X.X.X.
• [x] This bug describes functionality that never 100% worked as expected.
• [ ] I am not sure whether this functionality ever worked as expected.

Related & Steps to Reproduce

I believe @Benunc has the details and have a test setup ready in Pantheon because he is the one handling this issue with one of the clients.

Possible Solution

Put additional checks when values from $_SERVER['HTTP_X_FORWARDED_FOR'] for these possible scenarios: 1) Single IP - proceed in saving 2) 2 IPs but the same IP, truncate the duplicate before saving 3) 2 or more different IP means the site is in a proxy, might return an error or get the originating IP

Happy to help out with a PR if that would hasten the resolution.

Acceptance Criteria

• [ ] IPs should correctly stored
• [ ] Authorize.net should auto renew without any issues

Environment

Operating System

• Platform: Mac OS X
• Version: 10.14.x

Browser

• Name: Chrome
• Version: 72.x

WordPress System Info

WP v5.1.1 Give - Authorize.net Gateway - v 1.4.6 Give - Donation Plugin - v2.4.7 Give - Recurring Donations - v1.8.12

[carl-alberto]

By checking the code further, it seems this function can be overridden by a filter:

function overrid_ip( $ip ) {
    $ip = $_SERVER['REMOTE_ADDR'];
    return $ip;
}
add_filter( 'give_get_ip', 'overrid_ip' );

[DevinWalker]

Issue moved to impress-org/give-authorize-gateway #102 via ZenHub