Refactor middlewares

app/Http/Controllers/AuthController.php

@@ -227,6 +227,15 @@ class AuthController extends Controller
         return json(trans('auth.reset.success'), 0);
     }
 
+    public function fillEmail(Request $request)
+    {
+        $email = $this->validate($request, ['email' => 'required|email|unique:users'])['email'];
+        $user = $request->user();
+        $user->email = $email;
+        $user->save();
+        return redirect('/user');
+    }
+
     public function verify($uid)
     {
         if (! option('require_verification')) {

app/Http/Kernel.php

@@ -17,6 +17,7 @@ class Kernel extends HttpKernel
         \Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode::class,
         \Illuminate\Foundation\Http\Middleware\TrimStrings::class,
         \App\Http\Middleware\ConvertEmptyStringsToNull::class,
+        \App\Http\Middleware\DetectLanguagePrefer::class,
     ];
 
     /**
@@ -26,12 +27,12 @@ class Kernel extends HttpKernel
      */
     protected $middlewareGroups = [
         'web' => [
-            \App\Http\Middleware\EncryptCookies::class,
+            \Illuminate\Cookie\Middleware\EncryptCookies::class,
             \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
             \Illuminate\Session\Middleware\StartSession::class,
             \Illuminate\View\Middleware\ShareErrorsFromSession::class,
-            \App\Http\Middleware\DetectLanguagePrefer::class,
             \App\Http\Middleware\ForbiddenIE::class,
+            \Illuminate\Foundation\Http\Middleware\VerifyCsrfToken::class
         ],
 
         'api' => [
@@ -39,7 +40,12 @@ class Kernel extends HttpKernel
             'bindings',
         ],
 
-        'static' => [],
+        'authorize' => [
+            'auth:web',
+            \App\Http\Middleware\RejectBannedUser::class,
+            \App\Http\Middleware\EnsureEmailFilled::class,
+            \App\Http\Middleware\FireUserAuthenticated::class,
+        ],
     ];
 
     /**
@@ -50,9 +56,7 @@ class Kernel extends HttpKernel
      * @var array
      */
     protected $routeMiddleware = [
-        'csrf'        => \App\Http\Middleware\VerifyCsrfToken::class,
-        'auth'        => \App\Http\Middleware\CheckAuthenticated::class,
-        'auth.jwt'    => \Tymon\JWTAuth\Http\Middleware\Authenticate::class,
+        'auth'        => \App\Http\Middleware\Authenticate::class,
         'bindings'    => \Illuminate\Routing\Middleware\SubstituteBindings::class,
         'verified'    => \App\Http\Middleware\CheckUserVerified::class,
         'guest'       => \App\Http\Middleware\RedirectIfAuthenticated::class,

app/Http/Middleware/Authenticate.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace App\Http\Middleware;
+
+use Illuminate\Auth\Middleware\Authenticate as Middleware;
+
+class Authenticate extends Middleware
+{
+    protected function redirectTo($request)
+    {
+        if (! $request->expectsJson()) {
+            session([
+                'last_requested_path' => $request->path(),
+                'msg' => trans('auth.check.anonymous'),
+            ]);
+            return '/auth/login';
+        }
+    }
+}

app/Http/Middleware/CheckAuthenticated.php

@@ -1,68 +0,0 @@
-<?php
-
-namespace App\Http\Middleware;
-
-use App;
-use Closure;
-use Session;
-use App\Models\User;
-use App\Events\UserAuthenticated;
-use Illuminate\Support\Facades\Auth;
-
-class CheckAuthenticated
-{
-    public function handle($request, Closure $next)
-    {
-        if (Auth::check()) {
-            $user = Auth::user();
-
-            if ($user->permission == User::BANNED) {
-                Auth::logout();
-
-                abort(403, trans('auth.check.banned'));
-            }
-
-            // Ask for filling email
-            if ($user->email == '') {
-                return $this->askForFillingEmail($request, $next);
-            }
-
-            event(new UserAuthenticated($user));
-
-            return $next($request);
-        } else {
-            $this->flashLastRequestedPath();
-
-            return redirect('auth/login')->with('msg', trans('auth.check.anonymous'));
-        }
-    }
-
-    public function askForFillingEmail($request, Closure $next)
-    {
-        $user = Auth::user();
-
-        if (isset($request->email)) {
-            if (filter_var($request->email, FILTER_VALIDATE_EMAIL)) {
-                if (User::where('email', $request->email)->get()->isEmpty()) {
-                    $user->email = $request->input('email');
-                    $user->save();
-
-                    return $next($request);
-                } else {
-                    return response()->view('auth.bind', ['msg' => trans('auth.bind.registered')]);
-                }
-            } else {
-                return response()->view('auth.bind', ['msg' => trans('auth.validation.email')]);
-            }
-        }
-
-        return response()->view('auth.bind');
-    }
-
-    protected function flashLastRequestedPath($path = null)
-    {
-        $path = $path ?: app('request')->path();
-
-        return session(['last_requested_path' => $path]);
-    }
-}

app/Http/Middleware/DetectLanguagePrefer.php

@@ -10,25 +10,13 @@ class DetectLanguagePrefer
 {
     public function handle($request, \Closure $next)
     {
-        $response = $next($request);
-
-        if ($response instanceof Response) {
-            $response->cookie('locale', config('app.locale'));
-        }
-
-        return $response;
-    }
-
-    public function detect(Request $request)
-    {
-        $locale = $request->input('lang') ?: ($request->cookie('locale') ?: $request->getPreferredLanguage());
-
-        // If current locale is an alias of other locale
+        $locale = $request->input('lang') ?? session('locale') ?? $request->getPreferredLanguage();
         if (($info = Arr::get(config('locales'), $locale)) && ($alias = Arr::get($info, 'alias'))) {
             $locale = $alias;
         }
-
         app()->setLocale($locale);
-        session(['locale' => config('app.locale')]);
+        session(['locale' => $locale]);
+
+        return $next($request);
     }
 }

app/Http/Middleware/EncryptCookies.php

@@ -1,17 +0,0 @@
-<?php
-
-namespace App\Http\Middleware;
-
-use Illuminate\Cookie\Middleware\EncryptCookies as BaseEncrypter;
-
-class EncryptCookies extends BaseEncrypter
-{
-    /**
-     * The names of the cookies that should not be encrypted.
-     *
-     * @var array
-     */
-    protected $except = [
-        'locale',
-    ];
-}

app/Http/Middleware/EnsureEmailFilled.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace App\Http\Middleware;
+
+use Closure;
+
+class EnsureEmailFilled
+{
+    public function handle($request, Closure $next)
+    {
+        if ($request->user()->email != '' && $request->is('auth/bind')) {
+            return redirect('/user');
+        } elseif ($request->user()->email == '' && ! $request->is('auth/bind')) {
+            return redirect('/auth/bind');
+        }
+
+        return $next($request);
+    }
+}

app/Http/Middleware/FireUserAuthenticated.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace App\Http\Middleware;
+
+use Closure;
+
+class FireUserAuthenticated
+{
+    public function handle($request, Closure $next)
+    {
+        if (auth()->check()) {
+            event(new \App\Events\UserAuthenticated($request->user()));
+        }
+        return $next($request);
+    }
+}

app/Http/Middleware/RejectBannedUser.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace App\Http\Middleware;
+
+use Closure;
+use App\Models\User;
+
+class RejectBannedUser
+{
+    public function handle($request, Closure $next)
+    {
+        if ($request->user()->permission == User::BANNED) {
+            if ($request->expectsJson()) {
+                $response = json(trans('auth.check.banned'), -1);
+                $response->setStatusCode(403);
+                return $response;
+            } else {
+                abort(403, trans('auth.check.banned'));
+            }
+        }
+        return $next($request);
+    }
+}

app/Http/Middleware/VerifyCsrfToken.php

@@ -1,17 +0,0 @@
-<?php
-
-namespace App\Http\Middleware;
-
-use Illuminate\Foundation\Http\Middleware\VerifyCsrfToken as Middleware;
-
-class VerifyCsrfToken extends Middleware
-{
-    /**
-     * The URIs that should be excluded from CSRF verification.
-     *
-     * @var array
-     */
-    protected $except = [
-        //
-    ];
-}

app/Providers/RouteServiceProvider.php

@@ -58,7 +58,7 @@ class RouteServiceProvider extends ServiceProvider
     protected function mapWebRoutes(Router $router)
     {
         $router->group([
-            'middleware' => ['web', 'csrf'],
+            'middleware' => ['web'],
             'namespace' => $this->namespace,
         ], function ($router) {
             require base_path('routes/web.php');
@@ -93,10 +93,7 @@ class RouteServiceProvider extends ServiceProvider
      */
     protected function mapStaticRoutes(Router $router)
     {
-        $router->group([
-            'middleware' => 'static',
-            'namespace' => $this->namespace,
-        ], function ($router) {
+        $router->group(['namespace' => $this->namespace], function ($router) {
             require base_path('routes/static.php');
         });
     }

app/Providers/RuntimeCheckServiceProvider.php

@@ -16,9 +16,6 @@ class RuntimeCheckServiceProvider extends ServiceProvider
      */
     public function boot(Request $request)
     {
-        // Detect current locale
-        $this->app->call('App\Http\Middleware\DetectLanguagePrefer@detect');
-
         // Skip the installation check when in setup or under CLI
         if ($request->is('setup*') || $this->app->runningInConsole()) {
             return;

resources/views/auth/bind.blade.php

@@ -12,7 +12,7 @@
     <div class="login-box-body">
         <p class="login-box-msg">@lang('auth.bind.message')</p>
 
-        <form method="post" id="login-form">
+        <form method="post" id="login-form" action="{{ url('/auth/bind') }}">
             @csrf
             <div class="form-group has-feedback">
                 <input name="email" type="email" class="form-control" placeholder="@lang('auth.email')">
@@ -21,8 +21,8 @@
 
             <p>@lang('auth.bind.introduction')</p>
 
-            @if (isset($msg))
-            <div id="msg" class="callout callout-warning">{{ $msg }}</div>
+            @if ($errors->count() > 0)
+            <div id="msg" class="callout callout-warning">{{ $errors->first() }}</div>
             @endif
 
             <div class="row">

routes/api.php

@@ -6,7 +6,7 @@ Route::prefix('auth')->group(function ($route) {
     $route->post('refresh', 'AuthController@apiRefresh')->middleware('auth.jwt');
 });
 
-Route::prefix('user')->middleware('auth.jwt')->group(function ($route) {
+Route::prefix('user')->middleware('auth:api')->group(function ($route) {
     $route->put('sign', 'UserController@sign');
 
     $route->get('players', 'PlayerController@listAll');

routes/web.php

@@ -1,5 +1,7 @@
 <?php
 
+use App\Http\Middleware;
+
 /*
 |--------------------------------------------------------------------------
 | Application Routes
@@ -34,7 +36,8 @@ Route::group(['prefix' => 'auth'], function () {
     Route::post('/register', 'AuthController@handleRegister');
     Route::post('/forgot', 'AuthController@handleForgot');
     Route::post('/reset/{uid}', 'AuthController@handleReset')->middleware('signed');
-
+    Route::view('/bind', 'auth.bind')->middleware(['authorize', Middleware\EnsureEmailFilled::class]);
+    Route::post('/bind', 'AuthController@fillEmail')->middleware(['authorize', Middleware\EnsureEmailFilled::class]);
     Route::get('/verify/{uid}', 'AuthController@verify')->name('auth.verify')->middleware('signed');
 });
 
@@ -42,7 +45,7 @@ Route::group(['prefix' => 'auth'], function () {
  * User Center
  */
 Route::group([
-    'middleware' => ['web', 'auth', \App\Http\Middleware\RequireBindPlayer::class],
+    'middleware' => ['authorize', Middleware\RequireBindPlayer::class],
     'prefix' => 'user',
 ], function () {
     Route::any('', 'UserController@index');
@@ -90,7 +93,9 @@ Route::group(['prefix' => 'skinlib'], function () {
     Route::any('/show/{tid}', 'SkinlibController@show');
     Route::any('/data', 'SkinlibController@getSkinlibFiltered');
 
-    Route::group(['middleware' => ['auth', 'verified']], function () {
+    Route::group([
+        'middleware' => ['authorize', 'verified']
+    ], function () {
         Route::get('/upload', 'SkinlibController@upload');
         Route::post('/upload', 'SkinlibController@handleUpload');
         Route::post('/model', 'SkinlibController@model');
@@ -104,7 +109,7 @@ Route::group(['prefix' => 'skinlib'], function () {
 /*
  * Admin Panel
  */
-Route::group(['middleware' => ['auth', 'admin'], 'prefix' => 'admin'], function () {
+Route::group(['middleware' => ['authorize', 'admin'], 'prefix' => 'admin'], function () {
     Route::view('/', 'admin.index');
     Route::get('/chart', 'AdminController@chartData');
 

tests/AuthControllerTest.php

@@ -505,6 +505,19 @@ class AuthControllerTest extends TestCase
         $this->assertTrue($user->verifyPassword('12345678'));
     }
 
+    public function testFillEmail()
+    {
+        $user = factory(User::class)->create(['email' => '']);
+        $other = factory(User::class)->create();
+        $this->actingAs($user)->post('/auth/bind')->assertRedirect('/');
+        $this->actingAs($user)->post('/auth/bind', ['email' => 'a'])->assertRedirect('/');
+        $this->actingAs($user)->post('/auth/bind', ['email' => $other->email])->assertRedirect('/');
+
+        $this->actingAs($user)->post('/auth/bind', ['email' => 'a@b.c'])->assertRedirect('/user');
+        $user->refresh();
+        $this->assertEquals('a@b.c', $user->email);
+    }
+
     public function testVerify()
     {
         $url = URL::signedRoute('auth.verify', ['uid' => 1]);

tests/MiddlewareTest.php

@@ -2,6 +2,7 @@
 
 namespace Tests;
 
+use Event;
 use App\Models\User;
 use App\Models\Player;
 use App\Services\Facades\Option;
@@ -12,42 +13,10 @@ class MiddlewareTest extends TestCase
 {
     use DatabaseTransactions;
 
-    public function testCheckAuthenticated()
+    public function testAuthenticate()
     {
-        // Not logged in
         $this->get('/user')->assertRedirect('auth/login');
-        $this->assertGuest();
-
-        // Normal user
-        $this->actAs('normal')
-            ->assertAuthenticated();
-
-        // Banned User
-        $this->actAs('banned')
-            ->get('/user')
-            ->assertSee('banned')
-            ->assertStatus(403);
-
-        // Binding email
-        $noEmailUser = factory(\App\Models\User::class)->create(['email' => '']);
-        $this->actingAs($noEmailUser)
-            ->get('/user')
-            ->assertSee('Bind')
-            ->assertDontSee('User Center');
-
-        $this->actingAs($noEmailUser)
-            ->get('/user?email=email')
-            ->assertSee('Bind');
-
-        $other = factory(User::class)->create();
-        $this->actingAs($noEmailUser)
-            ->get('/user?email='.$other->email)
-            ->assertSee(trans('auth.bind.registered'));
-
-        $this->actingAs($noEmailUser)
-            ->get('/user?email=a@b.c')
-            ->assertSee('User Center');
-        $this->assertEquals('a@b.c', User::find($noEmailUser->uid)->email);
+        $this->actAs('normal')->assertAuthenticated();
     }
 
     public function testCheckUserVerified()
@@ -168,6 +137,26 @@ class MiddlewareTest extends TestCase
             ]);
     }
 
+    public function testEnsureEmailFilled()
+    {
+        $noEmailUser = factory(User::class)->create(['email' => '']);
+        $this->actingAs($noEmailUser)->get('/user')->assertRedirect('/auth/bind');
+
+        $normalUser = factory(User::class)->create();
+        $this->actingAs($normalUser)->get('/auth/bind')->assertRedirect('/user');
+    }
+
+    public function testFireUserAuthenticated()
+    {
+        Event::fake();
+        $user = factory(User::class)->create();
+        $this->actingAs($user)->get('/user');
+        Event::assertDispatched(\App\Events\UserAuthenticated::class, function ($event) use ($user) {
+            $this->assertEquals($user->uid, $event->user->uid);
+            return true;
+        });
+    }
+
     public function testRedirectIfAuthenticated()
     {
         $this->get('/auth/login')
@@ -179,6 +168,15 @@ class MiddlewareTest extends TestCase
             ->assertRedirect('/user');
     }
 
+    public function testRejectBannedUser()
+    {
+        $user = factory(User::class, 'banned')->create();
+        $this->actingAs($user)->get('/user')->assertForbidden();
+        $this->get('/user', ['accept' => 'application/json'])
+            ->assertForbidden()
+            ->assertJson(['code' => -1, 'message' => trans('auth.check.banned')]);
+    }
+
     public function testRequireBindPlayer()
     {
         $user = factory(User::class)->create();