-
Notifications
You must be signed in to change notification settings - Fork 788
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
Geozak redirection handling #749
base: master
Are you sure you want to change the base?
Conversation
A view template for a page to be displayed when a user is redirected between pages.
Added lines to make calls to Redirect display the redirect template view and exit. this should be satisfactory to fix this comment from Auth.php // to prevent fetching views via cURL (which "ignores" the header-redirect above) we leave the application // the hard way, via exit(). @see panique#453 // this is not optimal and will be fixed in future releases this should make redirecting users friendly in that they won't see empty/broken pages.
Replaced header location calls with Redirect::to to use the friendly redirection.
Replaced header location calls with Redirect::to to use the friendly redirection.
Removed unreachable statement.
The comment about preventing empty/broken pages is response to issue #453. The project already contains a Redirect class. These commits make shift the exit to into the redirect methods that way when a developer wants to redirect the user they only have to worry about adding one line and security is still intact. The redirect template view for that if it takes the users browsers while to actually redirect or in the case of the curl example from #453 then a meaningful page will be displayed/returned. A slightly better implementation to the template would be to make the address displayed a hyperlink. |
Thanks for the commits, but I think this has a problem: It takes out extremely important logic (the exit()) out of the Auth class! This is super-important and might break the application, as the application is not stopped from running when authentication fails and the user is not using redirects! As lots of people might use the application without being aware that they absolutly have to use redirects to keep the app save... hmm, sorry but in this case I would prefer to keep it like it is instead of merging this pull request. Is this okay for you ? |
The exit isn't removed. It's actually more reinforced because its inside the redirect methods. The having a view for the momentary delay in redirecting might not be necessary for most uses. But I think it would be good to include the exit call in the redirect methods even if we leave the exit calls in auth and such. |
Created a template view to use for friendly redirection.
Made Redirect class methods render the redirect view then exit to prevent other views from rendering.
Replaced uses of header('location: ...') with Redirect::to().