Hard redirect after log in

[Atmire-Kristof]

References

• Fixes Cache should be authorization aware #635
• Fixes First few requests never get the authorization header #756
• Fixes Logging in should also trigger a refresh #712

Description

This PR ensures a hard redirect is triggered after the user logs in. The purpose of doing so is to ensure everything in the ngrx store was retrieved using the correct authorizations.

After logging in and the hard redirect is triggered, the user is redirected back to the page they were originally on (before being sent to /login, or the page they opened the dropdown on).

This fixes several cache and authentication related issues.

Instructions for Reviewers

Changes made:

• Modified AuthenticatedGuard to return a UrlTree to /login when the user is not authenticated. This used to contain a dispatch of a RedirectWhenAuthenticationIsRequiredAction, which caused the guard to finish async to other guards. Important to note is that this guard only emits a value when loading in the AuthState is false.
• Added new ReloadGuard to the /reload route, which now accepts a redirect url parameter and will redirect the user to the redirect url when present, or /home when absent. (the use of this is explained below)
• Added new service HardRedirectService and two implementations for both the browser and server app module. These services contain a redirect() method, which performs a hard redirect to the provided url
• Modified AuthService's navigateToRedirectUrl to call HardRedirectService.redirect() to /reload with the url as the redirect query parameter
• The redirect URL in the store is set in two possible ways:
  -- The user visits a page and is redirected to /login by the authenticated-guard. The guard will set the redirect URL to the route the user originally tried to go to.
  -- The user visits a page and submits a login using the dropdown at the top of the page. The submit methods (in password and shiboleth components) will set the redirect URL to the current page, before triggering the login.
• Another important change to ensure the app is only loaded when the authenticated and hard redirect is finished, is the addition of the blocking property in the AuthState
  -- This property is set to "true" by default and is only set to "false" when the redirect action happens
  -- The entire app (app-component) is wrapped in an *ngIf that is true when blocking is false, as suggested on #756
  -- On top of the *ngIf, all routes are locked behind the new AuthBlockingGuard, which returns true once blocking turns from true to false. This is because we noticed angular was still loading resolvers for parts of the template inside the false ngIf. Presumably this is a performance optimisation from the framework.

How to test:
(Whenever I mention a "hard redirect", the page should perform a hard redirect, navigate to /reload/..., display a loading indicator in the middle of your screen and eventually redirect you back to your original page)

• Visit a route locked behind by AuthenticatedGuard as anonymous. This should redirect you to /login. After logging in, a hard redirect should take place, ending up on the page you originally tried to access.
• Navigate to a random public page as anonymous and login using the dropdown on top of the page. A hard redirect should take place, ending up on the page you were originally on.
• Manually visit /login and login. A hard redirect should take place, ending up on the homepage.
• Content locked behind authentication should display correctly right after the hard redirect takes place. See Cache should be authorization aware #635 for more details.
• Every rest request after the redirect should contain the authorization header. See First few requests never get the authorization header #756 for more details.

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!

• 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 TSLint validation using yarn run lint
• 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 for any bug fixes, improvements or new features. A few reminders about what constitutes good tests:
  • Include tests for different user types (if behavior differs), including: (1) Anonymous user, (2) Logged in user (non-admin), and (3) Administrator.
  • Include tests for error scenarios, e.g. when errors/warnings should appear (or buttons should be disabled).
  • For bug fixes, include a test that reproduces the bug and proves it is fixed. For clarity, it may be useful to provide the test in a separate commit from the bug fix.
• 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]

This pull request introduces 8 alerts when merging 3d08eb0 into cd6c5b7 - view on LGTM.com

new alerts:

• 8 for Unused variable, import, function or class

[tdonohue]

@Atmire-Kristof and @artlowel : Overall, this looks good. I have some minor inline comments.

However, in testing (using yarn start, i.e. AoT compilation), I'm finding the app occasionally "crashes" (for lack of a better word). It only happens in one scenario though, and it's also possible my local environment is acting up, so let me explain what I've tried.

1. I've tried selecting login from the homepage. This redirects back to the homepage & all works well
2. I've tried selecting login from a different page (Community/Collection/Item/Search). This redirects back to that page & all works well
3. I've tried pasting a restricted URL into my browser...I've been using the MyDSpace page: http://localhost:4000/mydspace?configuration=workspace . This redirects back to the MyDSpace page. If I think click around, eventually the app crashes and I stop seeing activity in Chrome DevTools "Network" tab. By "click around", here's what I'm doing.
   • First paste http://localhost:4000/mydspace?configuration=workspace into browser tab
   • Login, and I'm redirected to the MyDSpace page. All seems to work fine.
   • Click on header, to go back to Homepage. Sometimes this works, sometimes not
   • From the "All of DSpace" menu, select "Browse by Title". Again, sometimes this works
   • From the "All of DSpace" menu, select "Browse by Author". Again, sometimes this works
   • From the "All of DSpace" menu, select "Communities & Collections". Again, sometimes this works
   • EVENTUALLY (usually in 1-3 clicks) one of these pages comes back either entirely empty or with a "Loading" message. It's odd as I'm not able to reliably get it to happen in the same number of steps every time. However, it never seems to happen when I trigger the login from the "Log In" menu in the header. It only happens when I begin from pasting in an access restricted URL.

Because I cannot reliably get this to occur, I'm actually not yet sure if this is a bug in the PR or maybe something odd in my local environment. But, I wanted to log this so that I can test more next week to see if it's still happening to me then.

Overall, this PR is working well. I just need to test it further next week.

[artlowel]

@tdonohue It doesn't seem the mydspace issue is related to this PR. I can replicate it on the demo site.

[tdonohue]

👍 I've re-tested/re-reviewed today, and I'm not able to reproduce the strange "crashing" behavior I was seeing on Friday in this PR (or on the demo site). So, I'm not sure what it was.

In any case, this works well for me. I'm not noticing any changes in behavior after applying this PR. I did not test with external authentication systems (like Shibboleth), but internal auth works fine. Thanks @Atmire-Kristof and @artlowel !

[atarix83]

@Atmire-Kristof
thanks for this PR. I've looked at the code and it seems good.
I have tested also with shibboleth authentication and it works properly.
I found only an issue that it occurs only during shibboleth authentication.
While I'm not logged in and going to a protected page (i.e. /mydspace) the system redirects to the login page. After logging in authentication is successful but I'm not redirect to the previuopus protected page (/mydpsace) instead the application show the login page with an hanging loading icon :

Since this PR is blocking others, I agree to resolve in a follow-up PR if It can't be fixed quickly

[artlowel]

@atarix83 the shibboleth issue should be fixed

[atarix83]

thanks @artlowel I've checked and issue is resolved