From cf497ad38c6bc40333ebeda73aa26c1735de4c7e Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Tue, 24 Dec 2019 23:59:25 +0800 Subject: [PATCH] Change method of retrieving IP --- app/Http/Controllers/AdminController.php | 5 -- app/Http/Controllers/AuthController.php | 25 ++++++-- app/Http/Controllers/SetupController.php | 6 +- app/helpers.php | 22 ------- composer.json | 3 +- composer.lock | 58 ++++++++++++++++++- config/options.php | 1 - resources/lang/en/options.yml | 5 -- resources/misc/changelogs/en/5.0.0.md | 2 + resources/misc/changelogs/zh_CN/5.0.0.md | 2 + tests/BrowserKitTests/AdminFormsTest.php | 2 - .../ControllersTest/AuthControllerTest.php | 17 ++++-- 12 files changed, 100 insertions(+), 48 deletions(-) diff --git a/app/Http/Controllers/AdminController.php b/app/Http/Controllers/AdminController.php index c226775a..36b6b8eb 100644 --- a/app/Http/Controllers/AdminController.php +++ b/app/Http/Controllers/AdminController.php @@ -281,11 +281,6 @@ class AdminController extends Controller $form->text('regs_per_ip'); - $form->select('ip_get_method') - ->option('0', trans('options.general.ip_get_method.HTTP_X_FORWARDED_FOR')) - ->option('1', trans('options.general.ip_get_method.REMOTE_ADDR')) - ->hint(); - $form->group('max_upload_file_size') ->text('max_upload_file_size')->addon('KB') ->hint(trans('options.general.max_upload_file_size.hint', ['size' => ini_get('upload_max_filesize')])); diff --git a/app/Http/Controllers/AuthController.php b/app/Http/Controllers/AuthController.php index 668f9f09..a2f3bb88 100644 --- a/app/Http/Controllers/AuthController.php +++ b/app/Http/Controllers/AuthController.php @@ -17,15 +17,19 @@ use Laravel\Socialite\Facades\Socialite; use Mail; use Session; use URL; +use Vectorface\Whip\Whip; use View; class AuthController extends Controller { public function login() { + $whip = new Whip(); + $ip = $whip->getValidIpAddress(); + return view('auth.login', [ 'extra' => [ - 'tooManyFails' => cache(sha1('login_fails_'.get_client_ip())) > 3, + 'tooManyFails' => cache(sha1('login_fails_'.$ip)) > 3, 'recaptcha' => option('recaptcha_sitekey'), 'invisible' => (bool) option('recaptcha_invisible'), ], @@ -58,7 +62,9 @@ class AuthController extends Controller } // Require CAPTCHA if user fails to login more than 3 times - $loginFailsCacheKey = sha1('login_fails_'.get_client_ip()); + $whip = new Whip(); + $ip = $whip->getValidIpAddress(); + $loginFailsCacheKey = sha1('login_fails_'.$ip); $loginFails = (int) Cache::get($loginFailsCacheKey, 0); if ($loginFails > 3) { @@ -156,7 +162,9 @@ class AuthController extends Controller // If amount of registered accounts of IP is more than allowed amounts, // then reject the register. - if (User::where('ip', get_client_ip())->count() >= option('regs_per_ip')) { + $whip = new Whip(); + $ip = $whip->getValidIpAddress(); + if (User::where('ip', $ip)->count() >= option('regs_per_ip')) { return json(trans('auth.register.max', ['regs' => option('regs_per_ip')]), 7); } @@ -169,7 +177,7 @@ class AuthController extends Controller $user->avatar = 0; $user->password = $user->getEncryptedPwdFromEvent($data['password']) ?: app('cipher')->hash($data['password'], config('secure.salt')); - $user->ip = get_client_ip(); + $user->ip = $ip; $user->permission = User::NORMAL; $user->register_at = Carbon::now(); $user->last_sign_at = Carbon::now()->subDay(); @@ -228,7 +236,9 @@ class AuthController extends Controller $dispatcher->dispatch('auth.forgot.attempt', [$email]); $rateLimit = 180; - $lastMailCacheKey = sha1('last_mail_'.get_client_ip()); + $whip = new Whip(); + $ip = $whip->getValidIpAddress(); + $lastMailCacheKey = sha1('last_mail_'.$ip); $remain = $rateLimit + Cache::get($lastMailCacheKey, 0) - time(); if ($remain > 0) { return json(trans('auth.forgot.frequent-mail'), 2); @@ -354,13 +364,16 @@ class AuthController extends Controller $user = User::where('email', $email)->first(); if (!$user) { + $whip = new Whip(); + $ip = $whip->getValidIpAddress(); + $user = new User(); $user->email = $email; $user->nickname = $remoteUser->nickname ?? $remoteUser->name ?? $email; $user->score = option('user_initial_score'); $user->avatar = 0; $user->password = ''; - $user->ip = get_client_ip(); + $user->ip = $ip; $user->permission = User::NORMAL; $user->register_at = Carbon::now(); $user->last_sign_at = Carbon::now()->subDay(); diff --git a/app/Http/Controllers/SetupController.php b/app/Http/Controllers/SetupController.php index 0e3dddfe..b38b9535 100644 --- a/app/Http/Controllers/SetupController.php +++ b/app/Http/Controllers/SetupController.php @@ -12,6 +12,7 @@ use Illuminate\Filesystem\Filesystem; use Illuminate\Http\Request; use Illuminate\Support\Arr; use Illuminate\Support\Str; +use Vectorface\Whip\Whip; class SetupController extends Controller { @@ -145,6 +146,9 @@ class SetupController extends Controller 'site_url' => $siteUrl, ]); + $whip = new Whip(); + $ip = $whip->getValidIpAddress(); + // Register super admin $user = new User(); $user->email = $data['email']; @@ -152,7 +156,7 @@ class SetupController extends Controller $user->score = option('user_initial_score'); $user->avatar = 0; $user->password = app('cipher')->hash($data['password'], config('secure.salt')); - $user->ip = get_client_ip(); + $user->ip = $ip; $user->permission = User::SUPER_ADMIN; $user->register_at = Carbon::now(); $user->last_sign_at = Carbon::now()->subDay(); diff --git a/app/helpers.php b/app/helpers.php index 1af0d2d2..67c5d3e7 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -83,28 +83,6 @@ if (!function_exists('option_localized')) { } } -if (!function_exists('get_client_ip')) { - /** - * Return the client IP address. - * - * We define this function because Symfony's "Request::getClientIp()" method - * needs "setTrustedProxies()", which sucks when load balancer is enabled. - */ - function get_client_ip(): string - { - $request = request(); - if (option('ip_get_method') == '0') { - $ip = $request->server('HTTP_X_FORWARDED_FOR') - ?? $request->server('HTTP_CLIENT_IP') - ?? $request->server('REMOTE_ADDR'); - } else { - $ip = $request->server('REMOTE_ADDR'); - } - - return $ip; - } -} - if (!function_exists('get_string_replaced')) { /** * Replace content of string according to given rules. diff --git a/composer.json b/composer.json index 60e1cbcb..73c98cde 100644 --- a/composer.json +++ b/composer.json @@ -30,7 +30,8 @@ "symfony/process": "^4.4", "symfony/yaml": "^4.3", "twig/twig": "^2.11", - "tymon/jwt-auth": "dev-develop" + "tymon/jwt-auth": "dev-develop", + "vectorface/whip": "^0.3.2" }, "require-dev": { "barryvdh/laravel-debugbar": "^3.2", diff --git a/composer.lock b/composer.lock index f9c31e52..6e519b70 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "1d98f570b7b67b1ecd3b72dfcec58e7b", + "content-hash": "7941921d4db971b81f1b97420c1831a6", "packages": [ { "name": "composer/ca-bundle", @@ -4853,6 +4853,62 @@ ], "time": "2019-09-09T03:33:47+00:00" }, + { + "name": "vectorface/whip", + "version": "v0.3.2", + "source": { + "type": "git", + "url": "https://github.com/Vectorface/whip.git", + "reference": "c3cdf71f532c83c3ab512cfa57130dad36fdbc83" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/Vectorface/whip/zipball/c3cdf71f532c83c3ab512cfa57130dad36fdbc83", + "reference": "c3cdf71f532c83c3ab512cfa57130dad36fdbc83", + "shasum": "" + }, + "require": { + "php": ">=5.3.0" + }, + "require-dev": { + "phpunit/phpunit": "~4.0", + "psr/http-message": "~1.0", + "squizlabs/php_codesniffer": "~2.0", + "vectorface/dunit": "~2.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Vectorface\\Whip\\": "./src", + "VectorFace\\Whip\\": "./src", + "Vectorface\\WhipTests\\": "./tests" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Daniel Bruce", + "email": "dbruce@vectorface.com", + "role": "Developer" + }, + { + "name": "Cory Darby", + "email": "ckdarby@vectorface.com", + "role": "Developer" + } + ], + "description": "A PHP class for retrieving accurate IP address information for the client.", + "homepage": "https://github.com/Vectorface/whip", + "keywords": [ + "IP", + "cdn", + "cloudflare" + ], + "time": "2017-10-30T14:05:31+00:00" + }, { "name": "vlucas/phpdotenv", "version": "v3.6.0", diff --git a/config/options.php b/config/options.php index 8e15189e..a5d98c6f 100644 --- a/config/options.php +++ b/config/options.php @@ -8,7 +8,6 @@ return [ 'register_with_player_name' => 'true', 'require_verification' => 'false', 'regs_per_ip' => '3', - 'ip_get_method' => '0', 'api_type' => 'false', 'announcement' => 'Welcome to Blessing Skin {version}!', 'home_pic_url' => './app/bg.png', diff --git a/resources/lang/en/options.yml b/resources/lang/en/options.yml index 03ac70c6..f40716f3 100644 --- a/resources/lang/en/options.yml +++ b/resources/lang/en/options.yml @@ -111,11 +111,6 @@ general: title: Account Verification label: Users must verify their email address first. regs_per_ip: Max accounts of one IP - ip_get_method: - title: Get IP via - HTTP_X_FORWARDED_FOR: HTTP_X_FORWARDED_FOR (can be fabricated) - REMOTE_ADDR: REMOTE_ADDR (NOT suitable for sites under load balancer) - hint: Unfortunately, we have no method to get the accurate client IP address with pure PHP. max_upload_file_size: title: Max Upload Size hint: "Limit specified in php.ini: :size" diff --git a/resources/misc/changelogs/en/5.0.0.md b/resources/misc/changelogs/en/5.0.0.md index 9fed4985..0da23226 100644 --- a/resources/misc/changelogs/en/5.0.0.md +++ b/resources/misc/changelogs/en/5.0.0.md @@ -29,6 +29,7 @@ - Lengthened `ip` field in order to support IPv6. - Optimized performance of validating player name. - Optimized performance of invoking texture previewer (skinview3d). +- Changed method of retrieving IP. ## Fixed @@ -51,6 +52,7 @@ - Removed `commit` property from `blessing` global. - Removed Element UI. - Removed restriction of texture name and nickname. +- Removed settings of "Method of Retrieving IP". ## Internal Changes diff --git a/resources/misc/changelogs/zh_CN/5.0.0.md b/resources/misc/changelogs/zh_CN/5.0.0.md index cd352fec..f218dfc6 100644 --- a/resources/misc/changelogs/zh_CN/5.0.0.md +++ b/resources/misc/changelogs/zh_CN/5.0.0.md @@ -29,6 +29,7 @@ - 将 `ip` 字段的长度增加到 39 以支持 IPv6 - 优化角色名校验的性能 - 优化调用材质预览器（skinview3d）的性能 +- 修改获取 IP 地址的方法 ## 修复 @@ -51,6 +52,7 @@ - 从全局变量 `blessing` 中移除 `commit` 属性 - 移除 Element UI - 移除对材质名和用户昵称的要求 +- 移除「IP 获取方法」的设置 ## 内部更改 diff --git a/tests/BrowserKitTests/AdminFormsTest.php b/tests/BrowserKitTests/AdminFormsTest.php index dfa0d6d0..624d176b 100644 --- a/tests/BrowserKitTests/AdminFormsTest.php +++ b/tests/BrowserKitTests/AdminFormsTest.php @@ -110,7 +110,6 @@ class AdminFormsTest extends BrowserKitTestCase ->type('http://blessing.skin/', 'site_url') ->uncheck('user_can_register') ->type('8', 'regs_per_ip') - ->select('1', 'ip_get_method') ->type('2048', 'max_upload_file_size') ->see(trans( 'options.general.max_upload_file_size.hint', @@ -131,7 +130,6 @@ class AdminFormsTest extends BrowserKitTestCase $this->assertEquals('http://blessing.skin', option('site_url')); $this->assertFalse(option('user_can_register')); $this->assertEquals('8', option('regs_per_ip')); - $this->assertEquals('1', option('ip_get_method')); $this->assertEquals('2048', option('max_upload_file_size')); $this->assertEquals('cjk', option('player_name_rule')); $this->assertEquals('/^([0-9]+)$/', option('custom_player_name_regexp')); diff --git a/tests/HttpTest/ControllersTest/AuthControllerTest.php b/tests/HttpTest/ControllersTest/AuthControllerTest.php index 377988e9..4f5251db 100644 --- a/tests/HttpTest/ControllersTest/AuthControllerTest.php +++ b/tests/HttpTest/ControllersTest/AuthControllerTest.php @@ -15,6 +15,7 @@ use Illuminate\Support\Facades\URL; use Illuminate\Support\Str; use Laravel\Socialite\AbstractUser; use Laravel\Socialite\Facades\Socialite; +use Vectorface\Whip\Whip; class AuthControllerTest extends TestCase { @@ -97,7 +98,9 @@ class AuthControllerTest extends TestCase $this->flushSession(); Event::fake(); - $loginFailsCacheKey = sha1('login_fails_'.get_client_ip()); + $whip = new Whip(); + $ip = $whip->getValidIpAddress(); + $loginFailsCacheKey = sha1('login_fails_'.$ip); // Logging in should be failed if password is wrong $this->postJson( @@ -223,6 +226,8 @@ class AuthControllerTest extends TestCase public function testHandleRegister() { Event::fake(); + $whip = new Whip(); + $ip = $whip->getValidIpAddress(); // Should return a warning if `email` is empty $this->postJson('/auth/register')->assertJsonValidationErrors('email'); @@ -407,7 +412,7 @@ class AuthControllerTest extends TestCase 'email' => 'a@b.c', 'nickname' => 'nickname', 'score' => option('user_initial_score'), - 'ip' => '127.0.0.1', + 'ip' => $ip, 'permission' => User::NORMAL, ]); $this->assertAuthenticated(); @@ -484,7 +489,9 @@ class AuthControllerTest extends TestCase ]); config(['mail.driver' => 'smtp']); - $lastMailCacheKey = sha1('last_mail_'.get_client_ip()); + $whip = new Whip(); + $ip = $whip->getValidIpAddress(); + $lastMailCacheKey = sha1('last_mail_'.$ip); // Should be forbidden if sending email frequently Cache::put($lastMailCacheKey, time()); @@ -738,6 +745,8 @@ class AuthControllerTest extends TestCase public function testOAuthCallback() { Event::fake(); + $whip = new Whip(); + $ip = $whip->getValidIpAddress(); Socialite::shouldReceive('driver') ->with('github') @@ -782,7 +791,7 @@ class AuthControllerTest extends TestCase 'nickname' => 'abc', 'score' => option('user_initial_score'), 'avatar' => 0, - 'ip' => '127.0.0.1', + 'ip' => $ip, 'permission' => User::NORMAL, 'verified' => true, ]);