cybercog / laravel-ban

Laravel Ban simplify blocking and banning Eloquent models.
https://komarev.com/sources/laravel-ban
MIT License
1.06k stars 63 forks source link

best way to define a default route for banned users? #19

Open vesper8 opened 6 years ago

vesper8 commented 6 years ago

I got your package implemented and working now but I see that when a banned user tries to access a page that requires auth they are just bounced back to the landing page as if they were not authorized

I am not using your forbid-banned-user route middleware because I actually need to allow users to log in so I can show them a customized page telling them they are suspended where I can also retrieve the "comment" added when the ban was done

When users log in I catch that they are banned and send them to this page. But if they then try to navigate to an authorized page then it creates a problem.

I guess I can create a "isBanned" middleware and handle all this. But I thought this should be part of this package.. with a configuration file where one can set the default route to send all users that are both authenticated and banned

Or maybe I'm missing something and this is already baked in ?

antonkomarev commented 6 years ago

Hello @vesper8!

If user is banned forbid-banned-user middleware just redirects him back with error message:

$user = $this->auth->user();
if ($user && $user->isBanned()) {
    return redirect()->back()->withInput()->withErrors([
        'login' => 'This account is blocked.',
    ]);
}

It's just a boilerplate for the most common case. I've added it for quick security solution out of the box.

I like the idea to have separate endpoint where user will be redirected as soon as he will be banned and since it's application specific it should be configurable as you said.

It could be solved in 2 ways:

  1. Create route middleware which will wrap all the routes except one, let's say you-was-banned route, and will redirect banned user from any route to you-was-banned.
  2. Create global middleware or route middleware which will wrap all the routes and have internal logic which will check current route and redirect user to you-was-banned route only if he isn't on it now. Otherwise it will lead to cyclic redirection.

Each solution has benefits and drawbacks. But we need to be aware of APIs, because they don't need any redirections and should return 403 Forbidden.

Feel free to continue discussion or make PR if you want to see this feature out of the box!

vesper8 commented 6 years ago

Well I ended up create a simple route middleware

use Closure;

use Illuminate\Contracts\Auth\Guard;

class IsBanned
{
    /**
     * The Guard implementation.
     *
     * @var \Illuminate\Contracts\Auth\Guard
     */
    protected $auth;
    /**
     * @param \Illuminate\Contracts\Auth\Guard $auth
     */
    public function __construct(Guard $auth)
    {
        $this->auth = $auth;
    }

    /**
     * Handle an incoming request.
     *
     * @param \Illuminate\Http\Request $request
     * @param \Closure $next
     * @return mixed
     * @throws \Exception
     */
    public function handle($request, Closure $next)
    {
        $user = $this->auth->user();
        if ($user && $user->isBanned()) {
            return redirect()->route('account.suspended');
        }
        return $next($request);
    }
'jailBanned' => \MyProject\Http\Middleware\IsBanned::class,

And the inside my controller that handles all my authenticated routes, and since I'm still using (and downright still in love with LaravelCollective/annotations

All I had to do was add this one line at the top of my controller and problem solved

* @Middleware("jailBanned", except={"accountSuspended"})

I suppose it could be useful to have this built-in and documented

antonkomarev commented 6 years ago

Thanks for sharing it, @vesper8!

Right now I'm busy with upgrading 50+ proprietary packages to Laravel 5.6 and then launching brand new project. And only after all this stuff I'm ready to continue to contribute in open source projects.

I wouldn't close this issue to have a reminder to add this functionality out of the box.

joearcher commented 3 years ago

I have submitted a pull request for this https://github.com/cybercog/laravel-ban/pull/63