fix OIDC login redirect flow

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);
@@ -447,6 +448,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

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}`;
   }
 
   /**