Merged in task/dspace-cris-2023_02_x/DSC-2400 (pull request DSpace#4614)

[DSC-2400] fix redirect for item page resolver

Approved-by: Andrea Barbasso

Author: Francesco Mauto

bitbucket-pipelines.yml

@@ -6,7 +6,7 @@ definitions:
     - step: &unittest-code-checks
         name: test-code-checks
         image:
-          name: cypress/browsers:node-18.20.3-chrome-125.0.6422.141-1-ff-126.0.1-edge-125.0.2535.85-1
+          name: cypress/browsers:node-22.21.0-chrome-141.0.7390.107-1-ff-144.0-edge-141.0.3537.92-1
           run-as-user: 1000
         size: 4x
         caches:

src/app/core/auth/auth.actions.ts

@@ -34,6 +34,7 @@ export const AuthActionTypes = {
   LOG_OUT_ERROR: type('dspace/auth/LOG_OUT_ERROR'),
   LOG_OUT_SUCCESS: type('dspace/auth/LOG_OUT_SUCCESS'),
   SET_REDIRECT_URL: type('dspace/auth/SET_REDIRECT_URL'),
+  SET_REDIRECT_URL_AND_NAVIGATE: type('dspace/auth/SET_REDIRECT_URL_AND_NAVIGATE'),
   RETRIEVE_AUTHENTICATED_EPERSON: type('dspace/auth/RETRIEVE_AUTHENTICATED_EPERSON'),
   RETRIEVE_AUTHENTICATED_EPERSON_SUCCESS: type('dspace/auth/RETRIEVE_AUTHENTICATED_EPERSON_SUCCESS'),
   RETRIEVE_AUTHENTICATED_EPERSON_ERROR: type('dspace/auth/RETRIEVE_AUTHENTICATED_EPERSON_ERROR'),
@@ -359,6 +360,23 @@ export class SetRedirectUrlAction implements Action {
   }
 }
 
+/**
+ * Change the redirect url.
+ * @class SetRedirectUrlAction
+ * @implements {Action}
+ */
+export class SetRedirectUrlAndNavigateAction implements Action {
+  public type: string = AuthActionTypes.SET_REDIRECT_URL_AND_NAVIGATE;
+  payload: {
+    redirectUrl: string;
+    navigateUrl: string;
+  };
+
+  constructor(redirectUrl: string, navigateUrl: string) {
+    this.payload = { redirectUrl, navigateUrl };
+  }
+}
+
 /**
  * Start loading for a hard redirect
  * @class StartHardRedirectLoadingAction

src/app/core/auth/auth.effects.ts

@@ -51,7 +51,8 @@ import {
   RetrieveAuthMethodsErrorAction,
   RetrieveAuthMethodsSuccessAction,
   RetrieveTokenAction,
-  SetUserAsIdleAction
+  SetRedirectUrlAndNavigateAction,
+  SetUserAsIdleAction,
 } from './auth.actions';
 import { hasValue, isNotNull } from '../../shared/empty.util';
 import { Router } from '@angular/router';
@@ -123,6 +124,13 @@ export class AuthEffects {
     })
   ), { dispatch: false });
 
+  public redirectAndNavigate$: Observable<Action> = createEffect(() => this.actions$
+    .pipe(ofType(AuthActionTypes.SET_REDIRECT_URL_AND_NAVIGATE),
+      tap((action: SetRedirectUrlAndNavigateAction) => this.router.navigate([decodeURIComponent(action.payload.navigateUrl)])),
+    ),
+  { dispatch: false },
+  );
+
   // It means "reacts to this action but don't send another"
   public authenticatedError$: Observable<Action> = createEffect(() => this.actions$.pipe(
     ofType(AuthActionTypes.AUTHENTICATED_ERROR),

src/app/core/auth/auth.reducer.ts

@@ -12,8 +12,10 @@ import {
   RefreshTokenAndRedirectSuccessAction,
   RefreshTokenSuccessAction,
   RetrieveAuthenticatedEpersonSuccessAction,
-  RetrieveAuthMethodsSuccessAction, SetAuthCookieStatus,
-  SetRedirectUrlAction
+  RetrieveAuthMethodsSuccessAction,
+  SetAuthCookieStatus,
+  SetRedirectUrlAction,
+  SetRedirectUrlAndNavigateAction,
 } from './auth.actions';
 // import models
 import { AuthTokenInfo } from './models/auth-token-info.model';
@@ -266,6 +268,11 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut
         redirectUrl: (action as SetRedirectUrlAction).payload,
       });
 
+    case AuthActionTypes.SET_REDIRECT_URL_AND_NAVIGATE:
+      return Object.assign({}, state, {
+        redirectUrl: (action as SetRedirectUrlAndNavigateAction).payload.redirectUrl,
+      });
+
     case AuthActionTypes.REDIRECT_AFTER_LOGIN_SUCCESS:
       return Object.assign({}, state, {
         loading: true,

src/app/core/auth/auth.service.ts

@@ -31,6 +31,7 @@ import {
   RefreshTokenAction,
   ResetAuthenticationMessagesAction, SetAuthCookieStatus,
   SetRedirectUrlAction,
+  SetRedirectUrlAndNavigateAction,
   SetUserAsIdleAction,
   UnsetUserAsIdleAction
 } from './auth.actions';
@@ -518,15 +519,19 @@ export class AuthService {
   /**
    * Set redirect url
    */
-  setRedirectUrl(url: string) {
+   setRedirectUrl(redirectUrl: string, navigateUrl?: string) {
     // Add 1 hour to the current date
     const expireDate = Date.now() + (1000 * 60 * 60);
 
     // Set the cookie expire date
     const expires = new Date(expireDate);
-    const options: CookieAttributes = {expires: expires};
-    this.storage.set(REDIRECT_COOKIE, url, options);
-    this.store.dispatch(new SetRedirectUrlAction(isNotUndefined(url) ? url : ''));
+    const options: Cookies.CookieAttributes = { expires: expires };
+    this.storage.set(REDIRECT_COOKIE, redirectUrl, options);
+    if (hasValue(navigateUrl)) {
+      this.store.dispatch(new SetRedirectUrlAndNavigateAction(isNotUndefined(redirectUrl) ? redirectUrl : '', navigateUrl));
+    } else {
+      this.store.dispatch(new SetRedirectUrlAction(isNotUndefined(redirectUrl) ? redirectUrl : ''));
+    }
   }
 
   /**

src/app/core/shared/authorized.operators.ts

@@ -33,8 +33,11 @@ export const redirectOn4xx = <T>(router: Router, authService: AuthService) =>
               router.navigateByUrl(getForbiddenRoute(), { skipLocationChange: true });
               return false;
             } else {
-              authService.setRedirectUrl(router.url);
-              router.navigateByUrl('login');
+              // During a resolver the navigation hasn't committed yet, so router.url still
+              // points to the previous URL (e.g. '/'). Use the in-flight navigation's URL
+              // when available, falling back to router.url for component-level calls.
+              const redirectUrl = router.getCurrentNavigation()?.extractedUrl?.toString() ?? router.url;
+              authService.setRedirectUrl(redirectUrl, 'login');
               return false;
             }
           }

src/app/core/shared/operators.spec.ts

@@ -180,7 +180,7 @@ describe('Core Module - RxJS Operators', () => {
       testScheduler = new TestScheduler((actual, expected) => {
         expect(actual).toEqual(expected);
       });
-      router = jasmine.createSpyObj('router', ['navigateByUrl']);
+      router = jasmine.createSpyObj('router', ['navigateByUrl', 'getCurrentNavigation']);
       authService = jasmine.createSpyObj('authService', {
         isAuthenticated: observableOf(true),
         setRedirectUrl: {}
@@ -267,7 +267,6 @@ describe('Core Module - RxJS Operators', () => {
           expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values);
           flush();
           expect(authService.setRedirectUrl).toHaveBeenCalled();
-          expect(router.navigateByUrl).toHaveBeenCalledWith('login');
         });
       });
 
@@ -281,7 +280,6 @@ describe('Core Module - RxJS Operators', () => {
           expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values);
           flush();
           expect(authService.setRedirectUrl).toHaveBeenCalled();
-          expect(router.navigateByUrl).toHaveBeenCalledWith('login');
         });
       });
     });