ZCU-PUB/test: reload redirect must be absolute and must not fire during SSR

Replicates dataquest-dev/dspace-customers#236: after a Shibboleth login the
relative 'reload/<timestamp>' URL is resolved against the current URL
(e.g. /bitstreams/<uuid>/download), producing growing
/bitstreams/<uuid>/reload/reload/... URLs and a redirect loop, because
during SSR the redirect repeats on every request (the dsRedirectUrl cookie
cannot be cleared server-side - ServerCookieService.remove is a no-op).

Also stops static-page.component.spec from mutating the shared environment
object (it replaced environment.ui, breaking later specs reading
environment.ui.nameSpace).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: milanmajchrak

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

@@ -1,5 +1,7 @@
 import { fakeAsync, TestBed, tick } from '@angular/core/testing';
+import { NgZone } from '@angular/core';
 
+import { Actions } from '@ngrx/effects';
 import { provideMockActions } from '@ngrx/effects/testing';
 import { Store, StoreModule } from '@ngrx/store';
 import { MockStore, provideMockStore } from '@ngrx/store/testing';
@@ -17,6 +19,7 @@ import {
   CheckAuthenticationTokenCookieAction,
   LogOutErrorAction,
   LogOutSuccessAction,
+  RedirectAfterLoginSuccessAction,
   RefreshTokenErrorAction,
   RefreshTokenSuccessAction,
   RetrieveAuthenticatedEpersonAction,
@@ -176,6 +179,52 @@ describe('AuthEffects', () => {
       done();
     });
 
+    describe('when a redirect url is set', () => {
+      const redirectUrl = '/bitstreams/eb0ca4e9-3e00-4c1f-8b19-a2c9f4f01234/download';
+
+      beforeEach(() => {
+        spyOn((authEffects as any).authService, 'storeToken');
+        authServiceStub.setRedirectUrl(redirectUrl);
+        actions = hot('--a-', {
+          a: {
+            type: AuthActionTypes.AUTHENTICATED_SUCCESS, payload: {
+              authenticated: true,
+              authToken: token,
+              userHref: EPersonMock._links.self.href
+            }
+          }
+        });
+      });
+
+      afterEach(() => {
+        authServiceStub.setRedirectUrl(undefined);
+      });
+
+      it('should return a REDIRECT_AFTER_LOGIN_SUCCESS action in the browser', () => {
+        const expected = cold('--b-', { b: new RedirectAfterLoginSuccessAction(redirectUrl) });
+
+        expect(authEffects.authenticatedSuccess$).toBeObservable(expected);
+      });
+
+      it('should return a RETRIEVE_AUTHENTICATED_EPERSON action during server-side rendering', () => {
+        // The server cannot clear the redirect cookie (ServerCookieService.set/remove are no-ops),
+        // so a hard redirect during SSR would repeat on every request and create a redirect loop.
+        // The redirect must be left to the browser.
+        const serverAuthEffects: AuthEffects = new (AuthEffects as any)(
+          TestBed.inject(Actions),
+          TestBed.inject(NgZone),
+          authorizationService,
+          authServiceStub as any,
+          TestBed.inject(Store) as any,
+          'server'
+        );
+
+        const expected = cold('--b-', { b: new RetrieveAuthenticatedEpersonAction(EPersonMock._links.self.href) });
+
+        expect(serverAuthEffects.authenticatedSuccess$).toBeObservable(expected);
+      });
+    });
+
   });
 
   describe('checkToken$', () => {

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

@@ -34,6 +34,7 @@ import { NotificationsServiceStub } from '../../shared/testing/notifications-ser
 import { SetUserAsIdleAction, UnsetUserAsIdleAction } from './auth.actions';
 import { SpecialGroupDataMock, SpecialGroupDataMock$ } from '../../shared/testing/special-group.mock';
 import { cold } from 'jasmine-marbles';
+import { environment } from '../../../environments/environment';
 
 describe('AuthService test', () => {
 
@@ -103,7 +104,7 @@ describe('AuthService test', () => {
     linkService = {
       resolveLinks: {}
     };
-    hardRedirectService = jasmine.createSpyObj('hardRedirectService', ['redirect']);
+    hardRedirectService = jasmine.createSpyObj('hardRedirectService', ['redirect', 'getCurrentRoute']);
     spyOn(linkService, 'resolveLinks').and.returnValue({ authenticated: true, eperson: observableOf({ payload: {} }) });
 
   }
@@ -374,28 +375,39 @@ describe('AuthService test', () => {
       expect(storage.remove).toHaveBeenCalled();
     });
 
-    it('should redirect to reload with redirect url', () => {
+    // The reload URL must be absolute (nameSpace-aware): a relative 'reload/...' URL is resolved
+    // against the current URL - e.g. against /bitstreams/<uuid>/download after an external
+    // (Shibboleth) login - producing invalid nested URLs like /bitstreams/<uuid>/reload/reload/...
+    const reloadPrefix = environment.ui.nameSpace.replace(/\/$/, '') + '/reload/';
+
+    it('should redirect to the absolute reload URL with redirect url', () => {
       authService.navigateToRedirectUrl('/collection/123');
       // Reload with redirect URL set to /collection/123
-      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('reload/[0-9]*\\?redirect=' + encodeURIComponent('/collection/123'))));
+      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('^' + reloadPrefix + '[0-9]+\\?redirect=' + encodeURIComponent('/collection/123') + '$')));
     });
 
-    it('should redirect to reload with /home', () => {
+    it('should redirect to the absolute reload URL with /home', () => {
       authService.navigateToRedirectUrl('/home');
       // Reload with redirect URL set to /home
-      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('reload/[0-9]*\\?redirect=' + encodeURIComponent('/home'))));
+      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('^' + reloadPrefix + '[0-9]+\\?redirect=' + encodeURIComponent('/home') + '$')));
     });
 
-    it('should redirect to regular reload and not to /login', () => {
+    it('should redirect to the absolute reload URL and not to /login', () => {
       authService.navigateToRedirectUrl('/login');
       // Reload without a redirect URL
-      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('reload/[0-9]*(?!\\?)$')));
+      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('^' + reloadPrefix + '[0-9]+$')));
     });
 
-    it('should redirect to regular reload when no redirect url is found', () => {
+    it('should redirect to the absolute reload URL when no redirect url is found', () => {
       authService.navigateToRedirectUrl(undefined);
       // Reload without a redirect URL
-      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('reload/[0-9]*(?!\\?)$')));
+      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('^' + reloadPrefix + '[0-9]+$')));
+    });
+
+    it('should not redirect again when the current route is already the reload page', () => {
+      hardRedirectService.getCurrentRoute.and.returnValue(reloadPrefix + '123456789?redirect=' + encodeURIComponent('/home'));
+      authService.navigateToRedirectUrl('/collection/123');
+      expect(hardRedirectService.redirect).not.toHaveBeenCalled();
     });
 
     describe('impersonate', () => {

src/app/core/locale/locale.service.spec.ts

@@ -1,6 +1,9 @@
-import { TestBed, waitForAsync } from '@angular/core/testing';
+import { fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing';
 
 import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core';
+import { of } from 'rxjs';
+
+import { environment } from '../../../environments/environment';
 
 import { CookieService } from '../services/cookie.service';
 import { CookieServiceMock } from '../../shared/mocks/cookie.service.mock';
@@ -147,4 +150,37 @@ describe('LocaleService test suite', () => {
       expect(service.getLanguageCodeList).toHaveBeenCalled();
     });
   });
+
+  describe('refreshAfterChangeLanguage', () => {
+    let originalUi;
+    let originalGetCurrentUrl;
+
+    beforeEach(() => {
+      // Pin the tested nameSpace value: environment is a shared mutable object and some specs
+      // replace environment.ui entirely, so this test must not rely on the suite order.
+      originalUi = (environment as any).ui;
+      (environment as any).ui = Object.assign({}, originalUi, { nameSpace: '/angular-dspace' });
+      originalGetCurrentUrl = (routeService as any).getCurrentUrl;
+    });
+
+    afterEach(() => {
+      (environment as any).ui = originalUi;
+      // routeServiceStub is a shared module-level singleton - restore it
+      (routeService as any).getCurrentUrl = originalGetCurrentUrl;
+    });
+
+    it('should hard redirect to the absolute (nameSpace-aware) reload URL', fakeAsync(() => {
+      // A relative 'reload/...' URL would be resolved against the current URL
+      // (e.g. /items/<uuid>) and produce an invalid nested URL like /items/<uuid>/reload/...
+      const currentUrl = '/items/1234';
+      const fakeLocation = { href: '' };
+      serviceAsAny._window = { nativeWindow: { location: fakeLocation } };
+      (routeService as any).getCurrentUrl = jasmine.createSpy('getCurrentUrl').and.returnValue(of(currentUrl));
+
+      service.refreshAfterChangeLanguage();
+      tick();
+
+      expect(fakeLocation.href).toMatch(new RegExp('^/angular-dspace/reload/[0-9]+\\?redirect=' + encodeURIComponent(currentUrl) + '$'));
+    }));
+  });
 });

src/app/core/services/server-hard-redirect.service.spec.ts

@@ -23,7 +23,7 @@ describe('ServerHardRedirectService', () => {
   });
 
   describe('when performing a default redirect', () => {
-    const redirect = 'test redirect';
+    const redirect = '/test/redirect';
 
     beforeEach(() => {
       service.redirect(redirect);
@@ -36,7 +36,7 @@ describe('ServerHardRedirectService', () => {
   });
 
   describe('when performing a 301 redirect', () => {
-    const redirect = 'test 301 redirect';
+    const redirect = '/test/301/redirect';
     const redirectStatusCode = 301;
 
     beforeEach(() => {
@@ -49,6 +49,35 @@ describe('ServerHardRedirectService', () => {
     });
   });
 
+  describe('when performing a redirect to a relative url', () => {
+    // A relative URL in the Location header is resolved by the browser against the request URL,
+    // e.g. 'reload/123' requested at /bitstreams/<uuid>/download resolves
+    // to /bitstreams/<uuid>/reload/123 - such a redirect must never be emitted
+    const redirect = 'reload/123456789';
+
+    beforeEach(() => {
+      service.redirect(redirect);
+    });
+
+    it('should redirect to the URL prefixed with a slash', () => {
+      expect(mockResponse.redirect).toHaveBeenCalledWith(302, '/' + redirect);
+      expect(mockResponse.end).toHaveBeenCalled();
+    });
+  });
+
+  describe('when performing a redirect to an external url', () => {
+    const redirect = 'https://external-host.com/path';
+
+    beforeEach(() => {
+      service.redirect(redirect);
+    });
+
+    it('should redirect to the unchanged external URL', () => {
+      expect(mockResponse.redirect).toHaveBeenCalledWith(302, redirect);
+      expect(mockResponse.end).toHaveBeenCalled();
+    });
+  });
+
   describe('when requesting the current route', () => {
 
     beforeEach(() => {

src/app/static-page/static-page.component.spec.ts

@@ -27,7 +27,9 @@ describe('StaticPageComponent', () => {
       getCurrentLanguageCode: jasmine.createSpy('getCurrentLanguageCode'),
     });
 
-    appConfig = Object.assign(environment, {
+    // Do not mutate the shared `environment` object - replacing `environment.ui` would
+    // break any later spec that reads e.g. environment.ui.nameSpace
+    appConfig = Object.assign({}, environment, {
       ui: {
         namespace: 'testNamespace'
       }