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

Using Middleware; redirect instead of throw exception #21

Closed
Naoray opened this issue Aug 26, 2016 · 9 comments
Closed

Using Middleware; redirect instead of throw exception #21

Naoray opened this issue Aug 26, 2016 · 9 comments

Comments

@Naoray
Copy link
Contributor

Naoray commented Aug 26, 2016

Wouldn't it be nicer if we would use a redirect to a specifiable route instead of just calling abort(403). Or maybe make it optional?!

We could ad a config entry like

'middleware_handling' => 'abort',
'middleware_params' => '403',

and change abort(403) in Middleware to

call_user_func(config('laratrust.midleware_handling'), config('middleware_params));

=> just an idea!

@santigarcor
Copy link
Owner

santigarcor commented Aug 26, 2016

Actually i think it's a good idea.

@Naoray
Copy link
Contributor Author

Naoray commented Aug 28, 2016

I tried it with:

// config
'middleware_handling' => 'abort',
'middleware_params' => '403',


// middleware
if ($this->auth->guest() || !$request->user()->ability($roles, $permissions, [ 'validate_all' => $validateAll ]))
{
    return call_user_func(Config::get('laratrust.middleware_handling'), Config::get('laratrust.middleware_params'));
}

... it works, but there are some errors in the tests:

1) LaratrustAbilityTest::testHandle_IsGuestWithNoAbility_ShouldAbort403
Mockery\Exception\NoMatchingExpectationException: No matching handler found for Mockery_9__config::get('laratrust.middleware_handling'). Either the method was unexpected or its arguments matched no expected argument list for this method

But I did not figure out how to resolve it yet. Any idea?

@santigarcor
Copy link
Owner

@Naoray Indeed i'm working on it. I think the problem is the test.

@santigarcor
Copy link
Owner

santigarcor commented Aug 28, 2016

Could you please try it out, i got it working in 556515a. remember to add the middleware_handling and middleware_params to your config/laratrus.php file

@Naoray
Copy link
Contributor Author

Naoray commented Aug 29, 2016

you've had a typo in the middlewares... fixed it in #24 . It works perfectly, but the tests are still failing!! :/

@santigarcor
Copy link
Owner

@Naoray yeah i had the typo, maybe i did the typo in the tests too, i'm going to check it out

@santigarcor
Copy link
Owner

santigarcor commented Aug 29, 2016

@Naoray Yeah the problem was that the typo was on the tests too. i'm solving it now.

Thank you! 👍

@Naoray
Copy link
Contributor Author

Naoray commented Aug 29, 2016

@santigarcor got them! fixed in #25

@santigarcor
Copy link
Owner

santigarcor commented Aug 29, 2016

@Naoray , Thanks but i already solved it with 934f260

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

2 participants