Update GeoIP to support authentication by account id+license key #1302
Conversation
| $ch = curl_init($urlSchema . $downloadServer . $file['path']); | ||
| curl_setopt($ch, CURLOPT_HTTPAUTH, CURLAUTH_BASIC); | ||
| curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true); | ||
| curl_setopt($ch, CURLOPT_USERNAME, MAXMIND_ACCOUNT_ID); | ||
| curl_setopt($ch, CURLOPT_PASSWORD, MAXMIND_LICENSE_KEY); | ||
| curl_setopt($ch, CURLOPT_USERAGENT, 'MailWatch/' . mailwatch_version()); | ||
| if (defined('USE_PROXY') && USE_PROXY === true) { | ||
| curl_setopt($ch, CURLOPT_PROXY, PROXY_SERVER); | ||
| curl_setopt($ch, CURLOPT_PROXYPORT, PROXY_PORT); |
There was a problem hiding this comment.
Did you try Requests_Auth_Basic in $requestSession->options['auth']? I don't like very much removing Requests integration
There was a problem hiding this comment.
I
Did you try
Requests_Auth_Basicin $requestSession->options['auth']? I don't like very much removing Requests integration
To be honest I thought it might be good to remove the Request library as it is big piece of code for just downloading a file and it was orignally introduced for geoip download (if the commit history is correct) and isn't used anywhere else right now. But I'm fine if you want to keep it.
I did try the auth option of the requestsession but it didn't work that time. But I also didn't invest a lot of time trying. So it might just have been me 🤷.
There was a problem hiding this comment.
You are correct, Requests was introduced simplify the geoip file download in 2015, and it proved useful to manage automatically the redirection that MaxMind apply on the download link when they switched to CloudFlare R2 storage: with curl you have to check the first response for a 301/302 redirect location and then make a second request to download from the correct location. The actual library in mailscanner/lib/request has also been patched to support HTTP/2 calls.
There was a problem hiding this comment.
The CURLOPT_FOLLOWLOCATION options tells curl to follow such redirects and I'm pretty sure that HTTP/2 is supported out of the box. Corresponding feature flags are supported since PHP 7.0.7 (release 05/2016) and matching curl versions also since 2016.
There was a problem hiding this comment.
I know it seems anachronistic, but the 1.2.x branch needs to support PHP 5, so PHP 7+ solutions are a no go in 1.2 :(
We should start a 1.3 branch with php 7.4 support or even better align with officially supported only PHP version
| $file['description'] = __('geoip15'); | ||
| $file['path'] = '/app/geoip_download?edition_id=GeoLite2-Country&suffix=tar.gz&license_key=' . MAXMIND_LICENSE_KEY; |
There was a problem hiding this comment.
This is still working for my legacy Maxmind key, should we fall back and attempt this download when MAXMIND_ACCOUNT_ID is not populated?
fixes #1301