Skip to content

Commit 7d91529

Browse files
milanmajchrakclaude
andcommitted
ZCU-DATA/fix: absolute reload URL and no post-login hard redirect during SSR
Fixes the redirect loop after a Shibboleth login: - navigateToRedirectUrl() and refreshAfterChangeLanguage() build an absolute, ui.nameSpace-aware /reload/<ts> URL. A relative 'reload/<ts>' URL in the Location header is resolved against the request URL (e.g. /bitstreams/<uuid>/download after an external login), producing nested /bitstreams/<uuid>/reload/... URLs which never match the top-level 'reload/:rnd' route. navigateToRedirectUrl() also skips the redirect when the current path already is the reload page. - authenticatedSuccess$ dispatches RedirectAfterLoginSuccess only in the browser. The server cannot clear the dsRedirectUrl cookie (ServerCookieService.set/remove are no-ops), so a hard redirect during SSR repeats on every request - one 'reload/' segment per 302 hop - until the browser aborts at its redirect limit. - ServerHardRedirectService.redirect() never emits a relative Location header. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0b62b1b commit 7d91529

4 files changed

Lines changed: 36 additions & 6 deletions

File tree

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Injectable, NgZone } from '@angular/core';
1+
import { Inject, Injectable, NgZone, PLATFORM_ID } from '@angular/core';
2+
import { isPlatformBrowser } from '@angular/common';
23

34
import {
45
asyncScheduler,
@@ -100,7 +101,11 @@ export class AuthEffects {
100101
map((redirectUrl: string) => [action, redirectUrl])
101102
)),
102103
map(([action, redirectUrl]: [AuthenticatedSuccessAction, string]) => {
103-
if (hasValue(redirectUrl)) {
104+
// Perform the redirect only in the browser: the server cannot clear the redirect cookie
105+
// (ServerCookieService.set/remove are no-ops), so a hard redirect during SSR would repeat
106+
// on every request and create a redirect loop.
107+
// The browser re-runs this effect after hydration and performs the redirect itself.
108+
if (hasValue(redirectUrl) && isPlatformBrowser(this.platformId)) {
104109
return new RedirectAfterLoginSuccessAction(redirectUrl);
105110
} else {
106111
return new RetrieveAuthenticatedEpersonAction(action.payload.userHref);
@@ -287,6 +292,7 @@ export class AuthEffects {
287292
private zone: NgZone,
288293
private authorizationsService: AuthorizationDataService,
289294
private authService: AuthService,
290-
private store: Store<AppState>) {
295+
private store: Store<AppState>,
296+
@Inject(PLATFORM_ID) private platformId: any) {
291297
}
292298
}

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,9 +487,21 @@ export class AuthService {
487487
* @param redirectUrl
488488
*/
489489
public navigateToRedirectUrl(redirectUrl: string) {
490+
// Don't do redirect if the current page already is the reload page,
491+
// otherwise the reload could be repeated indefinitely
492+
// (only the path is checked - a query string could legitimately contain '/reload/')
493+
const currentRoute = this.hardRedirectService.getCurrentRoute();
494+
if (hasValue(currentRoute) && currentRoute.split('?')[0].includes('/reload/')) {
495+
return;
496+
}
490497
// Don't do redirect if already on reload url
491498
if (!hasValue(redirectUrl) || !redirectUrl.includes('reload/')) {
492-
let url = `reload/${new Date().getTime()}`;
499+
// The reload URL must be absolute (nameSpace-aware). A relative 'reload/...' URL is resolved
500+
// against the current URL - e.g. against /bitstreams/<uuid>/download after an external
501+
// (Shibboleth) login - and produces invalid nested URLs like /bitstreams/<uuid>/reload/...
502+
// which never match the 'reload/:rnd' route.
503+
const nameSpace = (environment.ui.nameSpace || '').replace(/\/$/, '');
504+
let url = `${nameSpace}/reload/${new Date().getTime()}`;
493505
if (isNotEmpty(redirectUrl) && !redirectUrl.startsWith(LOGIN_ROUTE)) {
494506
url += `?redirect=${encodeURIComponent(redirectUrl)}`;
495507
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,13 @@ export class LocaleService {
191191
public refreshAfterChangeLanguage() {
192192
this.routeService.getCurrentUrl().pipe(take(1)).subscribe((currentURL) => {
193193
// Hard redirect to the reload page with a unique number behind it
194-
// so that all state is definitely lost
195-
this._window.nativeWindow.location.href = `reload/${new Date().getTime()}?redirect=` + encodeURIComponent(currentURL);
194+
// so that all state is definitely lost.
195+
// The reload URL must be absolute (nameSpace-aware). A relative 'reload/...' URL is resolved
196+
// against the current URL and produces invalid nested URLs like /items/<uuid>/reload/...
197+
// which never match the 'reload/:rnd' route.
198+
const nameSpace = (environment.ui.nameSpace || '').replace(/\/$/, '');
199+
this._window.nativeWindow.location.href =
200+
`${nameSpace}/reload/${new Date().getTime()}?redirect=` + encodeURIComponent(currentURL);
196201
});
197202

198203
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ export class ServerHardRedirectService extends HardRedirectService {
2626
*/
2727
redirect(url: string, statusCode?: number) {
2828

29+
// A relative URL in the Location header is resolved by the browser against the request URL
30+
// (e.g. 'reload/123' requested at /bitstreams/<uuid>/download resolves to
31+
// /bitstreams/<uuid>/reload/123) - make sure only absolute URLs are emitted
32+
if (!url.startsWith('/') && !/^https?:\/\//i.test(url)) {
33+
url = '/' + url;
34+
}
35+
2936
if (url === this.req.url) {
3037
return;
3138
}

0 commit comments

Comments
 (0)