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

[5.3] Add authentication check to the Authorize middleware #13113

Closed

Conversation

JosephSilber
Copy link
Member

If you're using the default auth, there's now no need to use the auth middleware too.

Before this change, if you forgo the auth middleware, the Authorize middleware would incorrectly return a 403 instead of a 401.

@JosephSilber JosephSilber changed the title Add authentication check to the Authorize middleware [5.3] Add authentication check to the Authorize middleware Apr 11, 2016
@taylorotwell
Copy link
Member

This is giving us multiple places to modify the redirect path on failures though?

@JosephSilber
Copy link
Member Author

Unfortunately yes, but I believe it's worth it.

@mark86092
Copy link
Contributor

mark86092 commented Apr 20, 2016

Could it possible to throw a unauthenticated exception, and let the error handler to handle it?
For example, we have ValidationException, and AuthorizationException.

@GrahamCampbell
Copy link
Member

Could it possible to throw a unauthenticated exception, and let the error handler to handle it?

That's always how I do things, but laravel's sample implementations in the core don't really do that.

@taylorotwell
Copy link
Member

I just can't really get behind having to modify the redirect in two places.

@JosephSilber
Copy link
Member Author

JosephSilber commented Apr 21, 2016 via email

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

Successfully merging this pull request may close these issues.

4 participants