ZCU-PUB/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; 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-PUB/fix: shibboleth special groups lost in short-lived and refreshed tokens - 403 on download (backport #1347) DSpace#1374. 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-data: ZCU-DATA/fix: reload redirect loop after Shibboleth login (absolute reload URL, no SSR redirect) #1385

[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: 5dfeb80b-42a8-444f-9cdb-4998cbb0e449

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

This PR addresses a post-authentication redirect loop (notably after Shibboleth login) by ensuring reload redirects are emitted as absolute, ui.nameSpace-aware URLs and by preventing SSR from repeatedly issuing the post-login redirect when it can’t clear the redirect cookie.

Changes:

• Build absolute, nameSpace-aware /reload/<ts> URLs in both AuthService.navigateToRedirectUrl() and LocaleService.refreshAfterChangeLanguage(), with a guard to avoid re-redirecting while already on a reload page.
• Prevent SSR from dispatching the redirect-after-login action (leaving it to the browser after hydration), avoiding redirect loops caused by non-functional server cookie mutation.
• Add tests for SSR vs browser behavior and add server-side hard-redirect defense to avoid emitting relative Location headers.

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	Updates test config setup to avoid mutating shared environment (but currently introduces a config key mismatch).
src/app/core/services/server-hard-redirect.service.ts	Ensures server redirects don’t emit relative Location headers.
src/app/core/services/server-hard-redirect.service.spec.ts	Expands redirect tests for relative and external URL handling.
src/app/core/locale/locale.service.ts	Makes language-refresh hard redirect use absolute, nameSpace-aware reload URL.
src/app/core/locale/locale.service.spec.ts	Adds a spec ensuring refresh redirect uses an absolute, nameSpace-aware reload URL.
src/app/core/auth/auth.service.ts	Makes post-login reload redirect absolute and avoids redirecting again when already on reload.
src/app/core/auth/auth.service.spec.ts	Updates redirect URL expectations to match absolute, nameSpace-aware reload URLs; adds “no re-redirect on reload page” coverage.
src/app/core/auth/auth.effects.ts	Browser-only dispatch of redirect-after-login; SSR retrieves EPerson instead.
src/app/core/auth/auth.effects.spec.ts	Adds coverage for browser vs SSR behavior of authenticatedSuccess$ when redirect URL is set.

Comments suppressed due to low confidence (1)

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

• APP_CONFIG.ui uses the nameSpace property (capital S) throughout the codebase (see src/environments/environment.test.ts:17-23 and usages like app.module.ts). This spec overrides ui with { namespace: ... }, which drops the real ui fields (host/port/baseUrl/…) and sets the wrong property name, so any code reading appConfig.ui.nameSpace will see undefined and the test becomes less representative.

    // 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.