ZCU-DATA/fix: reload redirect loop after Shibboleth login (absolute reload URL, no SSR redirect)

[milanmajchrak]

Problem

After a Shibboleth login the browser can end up in a redirect loop with a growing URL, e.g.
/bitstreams/<uuid>/reload/reload/…(18x)…/<timestamp>?redirect=%2Fbitstreams%2F<uuid>%2Fdownload, and the user cannot navigate back (reported by a customer).

Two defects combine:

1. AuthService.navigateToRedirectUrl() and LocaleService.refreshAfterChangeLanguage() build a relative reload/<timestamp> URL. In the browser this stays hidden, because location.replace() resolves relative URLs against <base href>. But in an HTTP Location header there is no base - the browser resolves it against the request URL, so from /bitstreams/<uuid>/download it becomes /bitstreams/<uuid>/reload/<ts>, which never matches the top-level reload/:rnd route. Inherited from upstream (present on dspace-7_x and main).
2. During SSR the post-login redirect repeats on every request: the server cannot clear the dsRedirectUrl cookie (ServerCookieService.set/remove are no-ops), so whenever a valid dsAuthInfo cookie coexists with a dsRedirectUrl cookie on a full page load, every SSR pass emits another 302 with the relative Location header - one extra reload/ per hop, until the browser aborts at its redirect limit (the reported URL has 18).

Fix

• navigateToRedirectUrl() builds an absolute, ui.nameSpace-aware /reload/<ts> URL and skips the redirect when the current path already is the reload page.
• Same absolute URL in refreshAfterChangeLanguage().
• authenticatedSuccess$ dispatches the redirect action only in the browser; on the server it retrieves the EPerson instead. The browser re-runs the auth check after hydration, performs the redirect and can actually clear the cookie.
• Defense in depth: ServerHardRedirectService.redirect() never emits a relative Location header.

Reproduced live (dev instance, build without the fix)

The loop needs the state a not-finished login attempt leaves behind: a dsRedirectUrl cookie (lives 1 hour) alongside a valid dsAuthInfo cookie, plus one full page load of a nested URL. Verified on a dev instance - one page load produced 19 server 302s and ERR_TOO_MANY_REDIRECTS, with exactly the reported URL shape:

Steps:

1. Log in via Shibboleth. The browser now has a valid dsAuthInfo cookie.
2. Simulate the leftover of an interrupted login attempt (or actually interrupt one: open a restricted download URL anonymously in another tab, land on the login page and do not finish it). In DevTools console:
   document.cookie = 'dsRedirectUrl=' + encodeURIComponent('/bitstreams/<uuid>/download') + '; path=/; secure'
3. Paste any nested URL into the address bar (full page load, SSR), e.g. /bitstreams/<uuid>/download.
4. The URL grows by one reload/ segment per hop until the browser stops with ERR_TOO_MANY_REDIRECTS. The Back button re-enters the chain as long as the cookie lives.

The clean login round trip itself does not loop (the browser clears the cookie in time) - which is why the issue appears intermittently in production, whenever the stale cookie survives into a logged-in full page load.

TDD

Commit 1 adds/updates the specs. Run locally without the fix - 8 failures:

Commit 2 fixes the code - all pass:

(The images are rendered from the real local ng test logs of the zcu-pub twin branch - the changes are identical on both branches; the full local unit-test suite with the fix: 4843/4843 SUCCESS. Note: the first commit intentionally fails CI when checked out alone - TDD ordering. Evidence images live on the deletable branch assets/zcu-shibboleth-pr-evidence.)

Notes

• This does not fix the 403 on the download itself (UFAL/File preview spinner is still spinning when the submission is as workflow #900 in dataquest-dev/DSpace) - that is a backend bug fixed by ZCU-DATA/fix: shibboleth special groups lost in short-lived and refreshed tokens - 403 on download (backport #1347) DSpace#1375. With only this FE fix, an unauthorized user lands on the Forbidden page instead of looping. Both should be released together.
• Same fix for zcu-pub: ZCU-PUB/fix: reload redirect loop after Shibboleth login (absolute reload URL, no SSR redirect) #1384

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cbbd1431-9761-4520-a61b-15ba7a921e3b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Fixes a post-authentication redirect loop (notably after Shibboleth login and during SSR) by ensuring reload redirects are absolute, namespace-aware, and only performed client-side when needed.

Changes:

• Build absolute, ui.nameSpace-aware /reload/<ts> URLs in auth redirect and language refresh flows, with loop-avoidance when already on a reload route.
• Prevent SSR from repeatedly dispatching post-login redirects by gating redirect dispatch to browser-only execution.
• Add defense-in-depth on the server to avoid emitting relative Location headers; update/add corresponding unit tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/app/static-page/static-page.component.spec.ts	Avoids mutating the shared environment in tests (but introduces a config key mismatch issue noted in comments).
src/app/core/services/server-hard-redirect.service.ts	Ensures server redirects emit absolute URLs (leading / for relative inputs).
src/app/core/services/server-hard-redirect.service.spec.ts	Updates redirect expectations and adds coverage for relative vs external URLs.
src/app/core/locale/locale.service.ts	Makes reload redirect absolute and namespace-aware after language changes.
src/app/core/locale/locale.service.spec.ts	Adds test ensuring the reload URL is absolute and namespace-aware.
src/app/core/auth/auth.service.ts	Makes post-login reload redirect absolute/namespace-aware and avoids redirecting when already on reload.
src/app/core/auth/auth.service.spec.ts	Updates redirect URL assertions and adds coverage for “already on reload” case.
src/app/core/auth/auth.effects.ts	Dispatches redirect-after-login only in the browser; SSR retrieves EPerson instead.
src/app/core/auth/auth.effects.spec.ts	Adds coverage for browser vs SSR behavior when a redirect URL exists.

Comments suppressed due to low confidence (1)

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

• APP_CONFIG expects ui.nameSpace (used by StaticPageComponent.processLinks), but this spec sets ui.namespace. That leaves appConfig.ui.nameSpace undefined and can break link-handling tests (or future changes) in this suite.

    // 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'
      }
    });

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.