Adding global error handler for ajax calls which run into redirection…

[DeepDiver1975]

…s or unauthorized responses

refs #16726

@butonic @MorrisJobke

Let see how far we re getting with this one.

[scrutinizer-notifier]

A new inspection was created.

[PVince81]

This fix is fine for testing, but note that it might break in cases where 302 might be an expected response (not sure which ones though).

There is a small chance that this breaks WebDAV connectivity detection as it might rely on 401 error codes. Not tested yet, but something to look into if we decide to keep this fix after all.

Ideal would be to have the server always return 401 instead of anything else when the authentication expired.

[DeepDiver1975]

Ideal would be to have the server always return 401 instead of anything else when the authentication expired.

302 is used with shibboleth to redirect to the idp in case of session expiry - we might want to add some additional shib specific criteria ....

[PVince81]

Is that something that sits between Apache and the PHP code ? (aka hard to detect)

[DeepDiver1975]

Is that something that sits between Apache and the PHP code ? (aka hard to detect)

well the js code might extract some information from the response itself - maybe some header used in an shib env ....

[butonic]

I tested this and ran into:

XMLHttpRequest cannot load https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO?SAMLRequest=fZFdb4I…ss%3Amc%3A843092a07293cec8de8082ccda862a15578d7e87481b9ebe13dc61d0cbe6bd0d. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'https://lance' is therefore not allowed access.

As a result the status is 0 not 30x. Solved that by changing:

-       if (_.contains([302, 307, 401], request.status)) {
+       if (_.contains([0, 302, 307, 401], request.status)) {

After that the redirect worked bautifully for the list request, share action and heartbeat.

@DeepDiver1975 @LukasReschke does that make sense?

[butonic]

@LukasReschke @DeepDiver1975 should I just add that change as a commit? Then we test webdav again?

[DeepDiver1975]

Without shib enabled we should only react on 401. All the other cases are shibboleth only and should be treated within the shibboleth apps.

[PVince81]

How does shib behave with Webdav endpoints ? (asking for #16902)

[PVince81]

Defer to 9.0 ?

[felixboehm]

If one of several webservers is down, we would need to handle a 503. Is it missing? I am not sure ...

[DeepDiver1975]

If one of several webservers is down, we would need to handle a 503. Is it missing? I am not sure ...

Well if a webserver is physically down there should be no response at all and a timeout is raised.
Something to consider ... but let's first get this moving in.

[LukasReschke]

That should work now. Appframework does not need any adjustments from what I can tell and tested. 😄

[PVince81]

Nice, thanks a lot @LukasReschke!

[PVince81]

• TEST: OCS 401 (share dialog, notification)
• TEST: app framework REST (click favorite icon after session timeout)

Works!

@MorrisJobke if you like could you retry your test case ?

[MorrisJobke]

@MorrisJobke if you like could you retry your test case ?

Really nice - works now in all my tested scenarios like:

• folder/file creation
• switching navigation sidebar entries (right)
• open sidebar
• trigger actions

👍

Another topic is the upload handling. There it is already covered but only prompts a "Authentication error" and doesn't redirects. I'm open to do this in a new PR, because it's another code path or to directly put this in here. It's your decision.

[LukasReschke]

Another topic is the upload handling. There it is already covered but only prompts a "Authentication error" and doesn't redirects. I'm open to do this in a new PR, because it's another code path or to directly put this in here. It's your decision.

@MorrisJobke @PVince81 That should be done with b99c6f1 as well now. That's OC_JSON::checkLoggedIn() which is used for authentication checks in older code and now we're also sending a 401 there.

[PVince81]

@LukasReschke does that 401 also sends the DummyBasicAuth ?

[LukasReschke]

@LukasReschke does that 401 also sends the DummyBasicAuth ?

No. Because it won't reply with any auth request at all. It will simply return a 401 with a JSON blob stating that the auth failed.

[PVince81]

Ok, so let's test again with IE9 and IE10 to make sure it still works, then merge this.

This 👍 is for @LukasReschke's code. Will need another thumbs up for the JS code.

[PVince81]

• TEST: Chrome
• TEST: Firefox
• TEST: IE9
• TEST: IE10

Needs a second reviewer for the JS changes @LukasReschke @nickvergessen @rullzer

[PVince81]

Note, redirecting would cause whatever input the user had to be lost. This is fine short term, raised #22458 to discuss having a popup login dialog instead.

[LukasReschke]

👍 for the JS changes. Short-term this seems like the way to go. Better than before 😄

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.