From 750e45ba9981a418955a5debf6c6a6f3c3e974c4 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Mon, 8 Sep 2025 15:36:20 +0700 Subject: [PATCH] MDL-86495 core: Better errors handling for IP Lookup - Since `Geoplugin` is not free anymore, a new admin setting called `GeoPlugin API key` has been added for user to input their API key if they still want to use `Geoplugin` service. - The `iplookup_find_location()` method has been improved to handle error responses more effectively. --- public/admin/settings/location.php | 8 ++++++++ public/iplookup/lib.php | 15 ++++++++++----- public/iplookup/tests/geoip_test.php | 11 ++++++++++- public/iplookup/tests/geoplugin_test.php | 20 +++++++++++++++++++- public/lang/en/admin.php | 2 ++ 5 files changed, 49 insertions(+), 7 deletions(-) diff --git a/public/admin/settings/location.php b/public/admin/settings/location.php index ace2d8712ee..761b1cfb971 100644 --- a/public/admin/settings/location.php +++ b/public/admin/settings/location.php @@ -70,6 +70,14 @@ if ($hassiteconfig) { $temp->add(new admin_setting_configtext('googlemapkey3', new lang_string('googlemapkey3', 'core_admin'), new lang_string('googlemapkey3_help', 'core_admin'), '', PARAM_RAW, 60)); + + $temp->add(new admin_setting_configtext( + 'geopluginapikey', + new lang_string('geopluginapikey', 'core_admin'), + new lang_string('geopluginapikey_desc', 'core_admin'), + '', + PARAM_TEXT, + )); } $ADMIN->add('location', $temp); diff --git a/public/iplookup/lib.php b/public/iplookup/lib.php index 2570d223cb5..2071ce2f7fc 100644 --- a/public/iplookup/lib.php +++ b/public/iplookup/lib.php @@ -64,7 +64,7 @@ function iplookup_find_location($ip) { return $info; - } else { + } else if (!empty($CFG->geopluginapikey)) { require_once($CFG->libdir.'/filelib.php'); if (strpos($ip, ':') !== false) { @@ -73,11 +73,13 @@ function iplookup_find_location($ip) { return $info; } - $ipdata = download_file_content('http://www.geoplugin.net/json.gp?ip='.$ip); - if ($ipdata) { - $ipdata = preg_replace('/^geoPlugin\((.*)\)\s*$/s', '$1', $ipdata); - $ipdata = json_decode($ipdata, true); + $requesturl = new moodle_url('https://api.geoplugin.com', ['ip' => $ip, 'auth' => $CFG->geopluginapikey]); + $response = download_file_content($requesturl->out(false), null, null, true); + if ($response->response_code != 200) { + $info['error'] = get_string('cannotgeoplugin', 'error'); + return $info; } + $ipdata = json_decode($response->results, true); if (!is_array($ipdata)) { $info['error'] = get_string('cannotgeoplugin', 'error'); return $info; @@ -104,4 +106,7 @@ function iplookup_find_location($ip) { return $info; } + $info['error'] = get_string('iplookupfailed', 'error', $ip); + return $info; + } diff --git a/public/iplookup/tests/geoip_test.php b/public/iplookup/tests/geoip_test.php index f623f26dc0c..c683c98108c 100644 --- a/public/iplookup/tests/geoip_test.php +++ b/public/iplookup/tests/geoip_test.php @@ -50,8 +50,14 @@ final class geoip_test extends \advanced_testcase { * @param string $ip The IP to test */ public function test_ip($ip): void { + global $CFG; + if (!defined('TEST_GEOIP_APIKEY') || empty(TEST_GEOIP_APIKEY)) { + $this->markTestSkipped('External geo tests are disabled.'); + } $this->resetAfterTest(); - + // Store the old value to restore later. + $oldvalue = $CFG->geopluginapikey; + $CFG->geopluginapikey = TEST_GEOIP_APIKEY; $this->setup_geoip2file(); // Note: The results we get from the iplookup tests are beyond our control. @@ -69,6 +75,9 @@ final class geoip_test extends \advanced_testcase { $this->assertIsString($result['title'][0]); $this->assertIsString($result['title'][1]); $this->assertNull($result['error']); + + // Restore the old value. + $CFG->geopluginapikey = $oldvalue; } /** diff --git a/public/iplookup/tests/geoplugin_test.php b/public/iplookup/tests/geoplugin_test.php index ac0bf1989ef..efb4619c040 100644 --- a/public/iplookup/tests/geoplugin_test.php +++ b/public/iplookup/tests/geoplugin_test.php @@ -25,6 +25,8 @@ namespace core; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ final class geoplugin_test extends \advanced_testcase { + /** @var string Current Geoplugin API key. */ + private string $currentgeopluginapikey = ''; /** * Load required test libraries @@ -36,13 +38,29 @@ final class geoplugin_test extends \advanced_testcase { } /** - * In order to execute this test PHPUNIT_LONGTEST should be defined as true in phpunit.xml or directly in config.php + * In order to execute this test: + * - PHPUNIT_LONGTEST should be defined as true in phpunit.xml or directly in config.php + * - GeoPlugin API key should be defined in config.php as TEST_GEOIP_APIKEY */ public function setUp(): void { + global $CFG; parent::setUp(); if (!PHPUNIT_LONGTEST) { $this->markTestSkipped('PHPUNIT_LONGTEST is not defined'); } + + if (!defined('TEST_GEOIP_APIKEY') || empty(TEST_GEOIP_APIKEY)) { + $this->markTestSkipped('External geo tests are disabled.'); + } + // Store the old value to restore later. + $this->currentgeopluginapikey = $CFG->geopluginapikey; + $CFG->geopluginapikey = TEST_GEOIP_APIKEY; + } + + protected function tearDown(): void { + global $CFG; + $CFG->geopluginapikey = $this->currentgeopluginapikey; + parent::tearDown(); } /** diff --git a/public/lang/en/admin.php b/public/lang/en/admin.php index ae7be298914..3547f245206 100644 --- a/public/lang/en/admin.php +++ b/public/lang/en/admin.php @@ -710,6 +710,8 @@ $string['geoipmaxmindaccid'] = 'MaxMind account ID'; $string['geoipmaxmindaccid_desc'] = 'The account ID of the account created on the MaxMind API service.'; $string['geoipmaxmindlicensekey'] = 'MaxMind license key'; $string['geoipmaxmindlicensekey_desc'] = 'The license key of the account created on the MaxMind API service.'; +$string['geopluginapikey'] = 'GeoPlugin API key'; +$string['geopluginapikey_desc'] = 'The API key used to access the GeoPlugin service. Get your own key at GeoPlugin page.'; $string['getremoteaddrconf'] = 'Logged IP address source'; $string['globalsearch'] = 'Global search'; $string['globalsearchmanage'] = 'Manage global search';