Ensure HALEndpointService doesn't use stale responses

[artlowel]

References

• Fixes Shibboleth login often fails on redirect back to DSpace #2669

Description

#2669 happens because when you get redirected back from the Shibboleth server, the requests to log you in with the dspace rest api using the shib cookie tend to fail because one of those tends to be stale: it's fired just after one of these invalidateRootCache calls. When that happens the login process stops

Instructions for Reviewers

This PR fixes that by filtering out stale responses.

That change also makes it possible to replace the workaround in ServerCheckGuard with a call to invalidateRootCache()

This issue can happen at any time, it's not related to Shibboleth specifically. But since it's a timing issue, the easiest way I found to reliably reproduce it, is by logging in with Shibboleth

So:

• Log in using Shibboleth
• Verify that when you get redirected back to DSpace, you're reliably logged in every time, and there are no undefined doesn't contain the link … errors in the console

Checklist

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
• My PR passes ESLint validation using yarn lint
• My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
• If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
• If my PR fixes an issue ticket, I've linked them together.

[misilot]

@artlowel thanks for this! This seems to have made logging in more consistent.

I still see a 3-4 second page on the log in screen when logging in via a url like https://server/submit?collection=6d684812-6d29-4564-8b51-f5670957936b

I don't know if that's related or can be fixed so the user doesn't see that page before being sent onto the final place?

[artlowel]

@misilot you're welcome!

The reason you're not immediately logged in when you get redirected back is because dspace still needs to call the login endpoint with the shibboleth cookie at that point. So you go to the Shibboleth server to get the credentials to log you in, but they still need to be used to do so by the time you get back to DSpace.

As far as I can tell, that's the way it has always worked in DSpace 7. It's conceivable that there's a way of doing this that would be less confusing for the user, but that isn't in the scope of this PR

[artlowel]

I found an issue with this PR. Putting it in draft until it's fixed

[misilot]

Hi @artlowel not sure if the same thing, I did notice that with this patch that when using http://localhost:4000 the shibboleth login option is not visible, but it is there when coming from our deployed site.

[artlowel]

@misilot maybe, if you were using dev mode. The PR worked fine in prod mode. The issue was that in dev mode the initial /api request and the initial invalidate happened right after one another, causing it to be set to stale while it was still waiting for the server to respond to the initial request. We didn't have a way to deal with that case.

The symptom was that in dev mode the login dropdown would be empty, and you'd see those same undefined doesn't contain the link … errors in the console

I've addressed this by also introducing a ResponsePendingStale state, which didn't exist before. If something was set to stale while the response was still pending it would end up in the state ErrorStale.

With this PR, If a request is set to stale while its response is still pending, the response will also be set to stale when it comes in. (The reason for that is that this might happen when you've pushed an update to the server, and even though you haven't received a response yet, since the request was made before you pushed the changes, we should treat the response as stale too)

This was also to root cause of #2577 btw. So this PR should now also address the comment I made on that issue. Reviewers can verify that this is the case by rolling back the changes to #2596 on this PR, following the steps on the issue and verifying that the issue is still fixed. I would however still keep those changes as they're useful anyway.

[pybrarian]

We (Colorado State University) tested this PR with Shibboleth login in production mode with 7.6.1 that is unmodified otherwise and it seems to resolve the issue for us. We've not gotten the undefined doesn't contain the link … error since applying the patch and Shibboleth logins happen at roughly the same speed as before.

[tdonohue]

Thanks @artlowel ! Tested this with our Shibboleth Docker image (using https://samltest.id/), and it appears to be working. I'm not seeing any errors and #2669 appears to be resolved (I attempted the login several times with no issues).

I also tried other areas of DSpace just to make sure there were no side effects. Attempted a regular password authentication, new submission, import from PubMed, bulk access mgt tool, collection-based theming, etc. I know none of those are related to this, but wanted to verify that there's nothing else obviously wrong.

Overall, the code looks reasonable to me. I'm not seeing any side effects & it appears to fix the bug. That all said, I would like @atarix83 's feedback before we merge this, in case he sees anything that I may have missed. Thanks!

[misilot]

@misilot maybe, if you were using dev mode. The PR worked fine in prod mode. The issue was that in dev mode the initial /api request and the initial invalidate happened right after one another, causing it to be set to stale while it was still waiting for the server to respond to the initial request. We didn't have a way to deal with that case.

Yup, in dev mode the Login option disappears :(

[artlowel]

@misilot Also with the latest version of this PR? 790e717 should have fixed that issue

[misilot]

@artlowel yup, I just reapplied the patch this morning to try the newer changes.

[artlowel]

@misilot Do you see any undefined doesn't contain the link … errors in the browser console when this happens?

[misilot]

@misilot Do you see any undefined doesn't contain the link … errors in the browser console when this happens?

Nope, but now I am really confused, as the button is clearly there now. 🤔 Sorry for the noise, as it seems to be there. Just wish it would redirect back to localhost:4000 and not the server so it would consistently login to the dev environment (probably need to put a ticket in for this)

Actually it seems to be there sometimes, and not there other times.

[artlowel]

@misilot I don't think that redirect can work on localhost. The shib server also has to be able to contact it, and since localhost is relative to the machine you run it on, it won't end up on your computer.

What you can do is use your production REST API as a backend for the local client. Then, when you login from localhost, and get redirected back to your production server and are logged in there, is to open the network tools of your browser, find the dsAuthInfo cookie, copy it, and then paste it on localhost. If you refresh afterwards, you should be logged in on localhost too. It's a bit cumbersome, but I don't know of another way, short of giving your development machine a public hostname

[tantz001]

I applied this patch at The University of Minnesota Libraries, and we are no longer encountering problems when logging in via Shibboleth.

[amgciadev]

The same here as per my comment in DSpace/DSpace#9109 - Shibboleth issues seem to be resolved.

[atarix83]

thanks @artlowel for this PR

I've looked at the code and it looks good to me. I've tested the application and didn't find any issue.

I've verified that this comment has been addressed by this PR.

I can't really test shibboleth authentication, but i see it's been already verified and confirmed by several institutions within this PR

[dspace-bot]

Successfully created backport PR for main:

• [Port main] Ensure HALEndpointService doesn't use stale responses #2731

[nbimbe]

Just applied this patch on 7.6.1 and it has solved the problem. Response time is okay.