Detecting a successful page save

Development
1

Hi everyone,

XWiki uses the actionButtons.js JavaScript module to save wiki pages asynchronously. This module shows progress, success and failure notification messages. For this it obviously needs to detect if the edited page was successfully saved or not. Currently it relies solely on the HTTP response code: 200 implies a successful page save (no matter the response body).

actionButtons.js is used most of the time on forms that post to SaveAction but it’s not limited to that. SaveAction supports merge on save and for this is returns a JSON to indicate the new page version and whether the save was done by merging the content or not. actionButtons.js understands this JSON response and takes it into account but it doesn’t enforce it. It works even if there’s no JSON response. Enforcing this JSON response is not possible without breaking existing code that uses actionButtons.js without returning the JSON. This is not only about XS, we have for sure extensions that rely on this, which we don’t wan to break.

This means we are currently forced to interpret a 200 response code as a successful page save. This is not a problem, until… you add a custom authenticator.

If the authentication session expires while editing, clicking on the Save button will go through the authenticator which will have to deny the action and:

  • redirect to login page (302)
  • or return 401 Unauthorized
  • or some else?

The problem is when the authenticator returns 302 which is currently followed automatically by the browser, and the new location responds with 200 making actionButtons.js think the save was successful, when it wasn’t.

Who’s fault is it:

  • XWiki’s fault because it checks only the response code (200) and not the response body
  • Authenticator’s fault because it returns 302 while it should return 401 for AJAX requests (based on x-requested-with HTTP header)
  • (custom) login page’s fault because it return 200 even if it wasn’t accessed directly but due to a redirect caused by an unauthorized request

WDYT?

Personally I think the problem is on the authenticator side, and that we shouldn’t change anything in actionButtons.js.

Thanks,
Marius

2

I’ve the feeling that we should fix all the three options you mentioned:

IMO it’s probably wrong to keep it like that, I know it would be breaking to force needing a specific body answer for getting a proper save successful. But I do think it would be better on the future to have a clear reliable semantic for it.

Feels like it’s really debatable if the code should be 401 or 302 if there’s always a redirection…

Feels like this should be prevented by Loading... no?

3

This means that by design we expect to have a 200 save response for a failed save. This feels wrong to me.

It’s not debatable at all if you view the save AJAX request as a REST call. A RESTful API should never redirect to a login page, simply because the login page is part of the presentation which the REST API is decoupled of (it’s not aware of). The REST API is supposed to return 401 when authentication is needed (due to missing on invalid credentials). The (decoupled) front-end is then responsible for displaying a login modal or redirecting to some login page.

The custom authenticator can redirect to a custom login page that may also be outside XWiki, which is not affected by XWIKI-17846. Our standard login page returns 200 if there’s no xredirect query string parameter and 401 if the xredirect is present. This is works for us, but it’s a bit hackish, because technically speaking the login page is found and returned successfully, so the response should be 200 in both cases.