Admin options don't appear after Shibboleth authentication

[ybnd]

PR

References

• Fixes #1515
• Builds on #1741

Description

Currently, most admin sidebar functions are unavailbable when logging in as an admin via Shibboleth; they only appear after reloading.

This appeared to be due to the fact that the final stages of Shibboleth authentication are performed client-side after the final redirect, but this wasn't taken into account properly by the initialization logic.

Solved by

• refactoring AuthRequestService to work more similarly to DataService: send requests within a subscribe instead a pipe to ensure that requests are sent even when nothing subscribes to the result.
• ensuring that *InitService waits until authentication is no longer blocking

Minor changes

• To test Shibboleth in dev mode, ng serve should be started with --disable-host-check. In order to facilitate this (and other similar cases), serve.ts has been altered to patch through CLI arguments to ng serve -- now we can just start it as yarn start:dev --disable-host-check

• The allowance for a pinned sidebar was removed from server-side rendered pages. This is no longer relevant as authentication is not possible with JS turned off, and was leading to a glitchy animation on first load.

  The sliding animation was also removed to make the transition smoother.

Future work

• We haven't tested this yet, but it seems likely that a similar issue could affect the ORCID login.

• A new problem came up regarding logging out of a Shibboleth session: if you log in with Shibboleth, the log out and log in as a regular user, you'll still be logged in as the Shibboleth EPerson. Upon investigating closer it could only be solved in the backend https://github.com/DSpace/DSpace/issues/8475

Instructions for Reviewers

The easiest way to test this, is to deploy this PR on publicly accessible instance that has Shibboleth configured.

If you need to test this on a local instance, but have a public instance with shibboleth configured, you could disable the UI on that public instance, and reverse tunnel to it to ensure it redirects to your local angular server (e.g. ssh -R 4000:localhost:4000 angular@...)

• All admin sidebar functions should now appear on the first redirect after logging in via
  • Password authentication
  • Shibboleth

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

• [x] 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.
  • Most of the changes here are duplicates from #1741; line count will go down after merging
• [x] My PR passes TSLint validation using yarn run lint
• [x] My PR doesn't introduce circular dependencies
• [x] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• [x] My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• [x] If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

[lgtm-com[bot]]

This pull request introduces 4 alerts when merging ed9570e7ccb838a2c1c9fc8ec21529f4f0b98033 into eee0d72345e7d9953d0dca21c27587574287ee29 - view on LGTM.com

new alerts:

• 4 for Unused variable, import, function or class

[artlowel]

Assigned @mspalti, @atarix83 and @tdonohue as discussed in yesterday's meeting

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 94f0ac30936156a8b9b90f24f893c25019752751 into 342a71251337f69b524a4012afaf075afa8e82d5 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[tdonohue]

@ybnd : After merging #1741, this has some merge conflicts. If you could get those fixed up, we may be able to merge this quickly as well. Thanks!

[tdonohue]

Merging this as it's now at +2. The code looks good to me too. It sounds like it may not fix all our Shibboleth issues (as @mspalti notes) but it at least fixes a major one.