From fc49ae926b34f0efd9b21c42ea8514551486a223 Mon Sep 17 00:00:00 2001 From: Jesiel Viana Date: Wed, 18 Mar 2026 15:39:47 -0300 Subject: [PATCH 1/2] fix OIDC login redirect flow --- src/app/core/auth/auth.service.spec.ts | 39 ++++++++++++++++++++++++++ src/app/core/auth/auth.service.ts | 26 +++++++++-------- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index 4c4efc2f62b..e9cc87c3bd3 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -447,6 +447,45 @@ describe('AuthService test', () => { expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('reload/[0-9]*\\?redirect=' + encodeURIComponent('/home')))); }); + it('should replace a top-level external redirectUrl', () => { + const externalServerUrl = authService.getExternalServerRedirectUrl( + 'https://dspace.test', + '/itemtemplate', + '/api/authn/shibboleth?redirectUrl=https://dspace.test/home', + ); + + expect(externalServerUrl).toBe('/api/authn/shibboleth?redirectUrl=https%3A%2F%2Fdspace.test%2Fitemtemplate'); + }); + + it('should inject redirectUrl into a nested redirect_uri for OIDC providers', () => { + const externalServerUrl = authService.getExternalServerRedirectUrl( + 'https://dspace.test', + '/itemtemplate', + 'https://provider.example/authorize?client_id=test-client&response_type=code&scope=openid%20email&redirect_uri=' + + encodeURIComponent('https://rest.test/api/authn/oidc'), + ); + + const providerUrl = new URL(externalServerUrl); + const redirectUri = new URL(providerUrl.searchParams.get('redirect_uri')); + + expect(redirectUri.toString()).toBe('https://rest.test/api/authn/oidc?redirectUrl=https%3A%2F%2Fdspace.test%2Fitemtemplate'); + }); + + it('should preserve existing redirect_uri query params when injecting redirectUrl', () => { + const externalServerUrl = authService.getExternalServerRedirectUrl( + 'https://dspace.test', + '/itemtemplate', + 'https://provider.example/authorize?client_id=test-client&response_type=code&scope=openid%20email&redirect_uri=' + + encodeURIComponent('https://rest.test/api/authn/orcid?foo=bar'), + ); + + const providerUrl = new URL(externalServerUrl); + const redirectUri = new URL(providerUrl.searchParams.get('redirect_uri')); + + expect(redirectUri.searchParams.get('foo')).toBe('bar'); + expect(redirectUri.searchParams.get('redirectUrl')).toBe('https://dspace.test/itemtemplate'); + }); + it('should redirect to regular reload and not to /login', () => { authService.navigateToRedirectUrl('/login'); // Reload without a redirect URL diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index a762c857626..d6e5d05df6e 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -23,7 +23,6 @@ import { Store, } from '@ngrx/store'; import { TranslateService } from '@ngx-translate/core'; -import Cookies from 'js-cookie'; import { Observable, of, @@ -613,20 +612,23 @@ export class AuthService { */ getExternalServerRedirectUrl(origin: string, redirectRoute: string, location: string): string { const correctRedirectUrl = new URLCombiner(origin, redirectRoute).toString(); + const externalServerUrl = new URL(location, origin); + + if (externalServerUrl.searchParams.has('redirectUrl')) { + externalServerUrl.searchParams.set('redirectUrl', correctRedirectUrl); + } else if (externalServerUrl.searchParams.has('redirect_uri')) { + const redirectUri = new URL(externalServerUrl.searchParams.get('redirect_uri'), origin); + redirectUri.searchParams.set('redirectUrl', correctRedirectUrl); + externalServerUrl.searchParams.set('redirect_uri', redirectUri.toString()); + } else { + externalServerUrl.searchParams.set('redirectUrl', correctRedirectUrl); + } - let externalServerUrl = location; - const myRegexp = /\?redirectUrl=(.*)/g; - const match = myRegexp.exec(location); - const redirectUrlFromServer = (match && match[1]) ? match[1] : null; - - // Check whether the current page is different from the redirect url received from rest - if (isNotNull(redirectUrlFromServer) && redirectUrlFromServer !== correctRedirectUrl) { - // change the redirect url with the current page url - const newRedirectUrl = `?redirectUrl=${correctRedirectUrl}`; - externalServerUrl = location.replace(/\?redirectUrl=(.*)/g, newRedirectUrl); + if (isNotNull(location.match(/^https?:\/\//))) { + return externalServerUrl.toString(); } - return externalServerUrl; + return `${externalServerUrl.pathname}${externalServerUrl.search}${externalServerUrl.hash}`; } /** From f3db1a440367d0443e0f7ad96ec1c18b99ccba47 Mon Sep 17 00:00:00 2001 From: Jesiel Viana Date: Wed, 18 Mar 2026 18:07:29 -0300 Subject: [PATCH 2/2] make AuthService token test deterministic in CI --- src/app/core/auth/auth.service.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index e9cc87c3bd3..7153ab9de20 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -316,6 +316,7 @@ describe('AuthService test', () => { mockStore = TestBed.inject(MockStore); authService = TestBed.inject(AuthService); mockStore.overrideSelector(isAuthenticated, true); + mockStore.overrideSelector(getAuthenticationToken, token); mockStore.refreshState(); storage = (authService as any).storage; storage.get = jasmine.createSpy().and.returnValue(null);