Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vesper8 opened this issue Feb 9, 2018 · 4 comments
Open

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

vesper8 opened this issue Feb 9, 2018 · 4 comments

Comments

@vesper8
Copy link

vesper8 commented Feb 9, 2018

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
Copy link
Member

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
Copy link
Author

vesper8 commented Feb 10, 2018

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
Copy link
Member

antonkomarev commented Feb 10, 2018

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.

@antonkomarev antonkomarev added this to the v5.0 milestone Aug 19, 2019
@joearcher
Copy link
Contributor

I have submitted a pull request for this #63

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants