search

derpibooru / philomena

Next-generation imageboard
https://derpibooru.org/
GNU Affero General Public License v3.0
219 stars 56 forks source link

Failing captcha renders source page within page after failure #84

Closed SnowySailor closed 4 years ago

SnowySailor commented 4 years ago

Describe the bug When a user submits a form that requires a captcha (when they're not logged in, for example), failing the captcha will replace the form with the original source page (ends up with page inside a page). See images below.

To Reproduce Steps to reproduce the behavior:

  1. Sign out of any account
  2. Go to an image
  3. Attempt to change the Source but do not fill out captcha
  4. Submit

Expected behavior An error message should appear and the input + buttons should not go away.

Screenshots Before submitting image

After submitting image

Desktop (please complete the following information):

liamwhite commented 4 years ago

Intercept the error here and do something with it https://github.com/derpibooru/philomena/blob/5f9e9d98a69cb560e9e10f938ce112399b611294/assets/js/ujs.js#L51-L62

SnowySailor commented 4 years ago

This will probably also happen if the captcha is filled out but it's incorrect. We can probably catch some of the cases on the client side like if they didn't fill it out but there will need to be a server side component to this fix as well.

liamwhite commented 4 years ago

The server side component would need to be rendering text with the appropriate error code (so that the flash stays persisted) and the client side would be having JS reload the outer page to fetch the flash

SnowySailor commented 4 years ago

It looks like captcha_plug.ex is trying to redirect the user to the same page with the flash error but for some reason it's just replacing the input form with the redirection target page instead of redirecting the browser. Might want to see if this is unique to Firefox.

liamwhite commented 4 years ago

The UJS fetchcomplete event for the tags input replaces the entire tag input block with the received HTML page. https://github.com/derpibooru/philomena/blob/5f9e9d98a69cb560e9e10f938ce112399b611294/assets/js/tagsmisc.js#L38-L54

SnowySailor commented 4 years ago

So are you suggesting we check the status code for a 302 and refresh the entire document body with the response if it's a 302 and do what we normally do for a 200?

liamwhite commented 4 years ago

The server code should render a plain 400, then the client code should reload at top level navigation to fetch the flash.

SnowySailor commented 4 years ago

When a captcha fails, the captcha_plug.ex does a redirect which returns a 302. We could make it a 400 though.

liamwhite commented 4 years ago

Yes, that is what I am suggesting.

liamwhite commented 4 years ago

Fixed in #85