[CSTPER-144] Fixed issue with authorization request encountered while logging-in with external idp

src/app/core/auth/auth.actions.ts

@@ -292,10 +292,13 @@ export class ResetAuthenticationMessagesAction implements Action {
 export class RetrieveAuthMethodsAction implements Action {
   public type: string = AuthActionTypes.RETRIEVE_AUTH_METHODS;
 
-  payload: AuthStatus;
+  payload: {
+    status: AuthStatus;
+    blocking: boolean;
+  };
 
-  constructor(authStatus: AuthStatus) {
-    this.payload = authStatus;
+  constructor(status: AuthStatus, blocking: boolean) {
+    this.payload = { status, blocking };
   }
 }
 
@@ -306,10 +309,14 @@ export class RetrieveAuthMethodsAction implements Action {
  */
 export class RetrieveAuthMethodsSuccessAction implements Action {
   public type: string = AuthActionTypes.RETRIEVE_AUTH_METHODS_SUCCESS;
-  payload: AuthMethod[];
 
-  constructor(authMethods: AuthMethod[] ) {
-    this.payload = authMethods;
+  payload: {
+    authMethods: AuthMethod[];
+    blocking: boolean;
+  };
+
+  constructor(authMethods: AuthMethod[], blocking: boolean ) {
+    this.payload = { authMethods, blocking };
   }
 }
 
@@ -320,6 +327,12 @@ export class RetrieveAuthMethodsSuccessAction implements Action {
  */
 export class RetrieveAuthMethodsErrorAction implements Action {
   public type: string = AuthActionTypes.RETRIEVE_AUTH_METHODS_ERROR;
+
+  payload: boolean;
+
+  constructor(blocking: boolean) {
+    this.payload = blocking;
+  }
 }
 
 /**

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

@@ -43,10 +43,12 @@ describe('AuthEffects', () => {
   let initialState;
   let token;
   let store: MockStore<AppState>;
+  let authStatus;
 
   function init() {
     authServiceStub = new AuthServiceStub();
     token = authServiceStub.getToken();
+    authStatus = Object.assign(new AuthStatus(), {});
     initialState = {
       core: {
         auth: {
@@ -217,16 +219,38 @@ describe('AuthEffects', () => {
         expect(authEffects.checkTokenCookie$).toBeObservable(expected);
       });
 
-      it('should return a RETRIEVE_AUTH_METHODS action in response to a CHECK_AUTHENTICATION_TOKEN_COOKIE action when authenticated is false', () => {
-        spyOn((authEffects as any).authService, 'checkAuthenticationCookie').and.returnValue(
-          observableOf(
-            { authenticated: false })
-        );
-        actions = hot('--a-', { a: { type: AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_COOKIE } });
+      describe('on CSR', () => {
+        it('should return a RETRIEVE_AUTH_METHODS action in response to a CHECK_AUTHENTICATION_TOKEN_COOKIE action when authenticated is false', () => {
+          spyOn((authEffects as any).authService, 'checkAuthenticationCookie').and.returnValue(
+            observableOf(
+              { authenticated: false })
+          );
+          spyOn((authEffects as any).authService, 'getRetrieveAuthMethodsAction').and.returnValue(
+            new RetrieveAuthMethodsAction({ authenticated: false } as AuthStatus, false)
+          );
+          actions = hot('--a-', { a: { type: AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_COOKIE } });
 
-        const expected = cold('--b-', { b: new RetrieveAuthMethodsAction({ authenticated: false } as AuthStatus) });
+          const expected = cold('--b-', { b: new RetrieveAuthMethodsAction({ authenticated: false } as AuthStatus, false) });
 
-        expect(authEffects.checkTokenCookie$).toBeObservable(expected);
+          expect(authEffects.checkTokenCookie$).toBeObservable(expected);
+        });
+      });
+
+      describe('on SSR', () => {
+        it('should return a RETRIEVE_AUTH_METHODS action in response to a CHECK_AUTHENTICATION_TOKEN_COOKIE action when authenticated is false', () => {
+          spyOn((authEffects as any).authService, 'checkAuthenticationCookie').and.returnValue(
+            observableOf(
+              { authenticated: false })
+          );
+          spyOn((authEffects as any).authService, 'getRetrieveAuthMethodsAction').and.returnValue(
+            new RetrieveAuthMethodsAction({ authenticated: false } as AuthStatus, true)
+          );
+          actions = hot('--a-', { a: { type: AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_COOKIE } });
+
+          const expected = cold('--b-', { b: new RetrieveAuthMethodsAction({ authenticated: false } as AuthStatus, true) });
+
+          expect(authEffects.checkTokenCookie$).toBeObservable(expected);
+        });
       });
     });
 
@@ -359,27 +383,74 @@ describe('AuthEffects', () => {
 
   describe('retrieveMethods$', () => {
 
-    describe('when retrieve authentication methods succeeded', () => {
-      it('should return a RETRIEVE_AUTH_METHODS_SUCCESS action in response to a RETRIEVE_AUTH_METHODS action', () => {
-        actions = hot('--a-', { a: { type: AuthActionTypes.RETRIEVE_AUTH_METHODS } });
+    describe('on CSR', () => {
+      describe('when retrieve authentication methods succeeded', () => {
+        it('should return a RETRIEVE_AUTH_METHODS_SUCCESS action in response to a RETRIEVE_AUTH_METHODS action', () => {
+          actions = hot('--a-', { a:
+            {
+              type: AuthActionTypes.RETRIEVE_AUTH_METHODS,
+              payload: { status: authStatus, blocking: false}
+            }
+          });
 
-        const expected = cold('--b-', { b: new RetrieveAuthMethodsSuccessAction(authMethodsMock) });
+          const expected = cold('--b-', { b: new RetrieveAuthMethodsSuccessAction(authMethodsMock, false) });
 
-        expect(authEffects.retrieveMethods$).toBeObservable(expected);
+          expect(authEffects.retrieveMethods$).toBeObservable(expected);
+        });
+      });
+
+      describe('when retrieve authentication methods failed', () => {
+        it('should return a RETRIEVE_AUTH_METHODS_ERROR action in response to a RETRIEVE_AUTH_METHODS action', () => {
+          spyOn((authEffects as any).authService, 'retrieveAuthMethodsFromAuthStatus').and.returnValue(observableThrow(''));
+
+          actions = hot('--a-', { a:
+            {
+              type: AuthActionTypes.RETRIEVE_AUTH_METHODS,
+              payload: { status: authStatus, blocking: false}
+            }
+          });
+
+          const expected = cold('--b-', { b: new RetrieveAuthMethodsErrorAction(false) });
+
+          expect(authEffects.retrieveMethods$).toBeObservable(expected);
+        });
       });
     });
 
-    describe('when retrieve authentication methods failed', () => {
-      it('should return a RETRIEVE_AUTH_METHODS_ERROR action in response to a RETRIEVE_AUTH_METHODS action', () => {
-        spyOn((authEffects as any).authService, 'retrieveAuthMethodsFromAuthStatus').and.returnValue(observableThrow(''));
+    describe('on SSR', () => {
+      describe('when retrieve authentication methods succeeded', () => {
+        it('should return a RETRIEVE_AUTH_METHODS_SUCCESS action in response to a RETRIEVE_AUTH_METHODS action', () => {
+          actions = hot('--a-', { a:
+            {
+              type: AuthActionTypes.RETRIEVE_AUTH_METHODS,
+              payload: { status: authStatus, blocking: true}
+            }
+          });
 
-        actions = hot('--a-', { a: { type: AuthActionTypes.RETRIEVE_AUTH_METHODS } });
+          const expected = cold('--b-', { b: new RetrieveAuthMethodsSuccessAction(authMethodsMock, true) });
 
-        const expected = cold('--b-', { b: new RetrieveAuthMethodsErrorAction() });
+          expect(authEffects.retrieveMethods$).toBeObservable(expected);
+        });
+      });
 
-        expect(authEffects.retrieveMethods$).toBeObservable(expected);
+      describe('when retrieve authentication methods failed', () => {
+        it('should return a RETRIEVE_AUTH_METHODS_ERROR action in response to a RETRIEVE_AUTH_METHODS action', () => {
+          spyOn((authEffects as any).authService, 'retrieveAuthMethodsFromAuthStatus').and.returnValue(observableThrow(''));
+
+          actions = hot('--a-', { a:
+            {
+              type: AuthActionTypes.RETRIEVE_AUTH_METHODS,
+              payload: { status: authStatus, blocking: true}
+            }
+          });
+
+          const expected = cold('--b-', { b: new RetrieveAuthMethodsErrorAction(true) });
+
+          expect(authEffects.retrieveMethods$).toBeObservable(expected);
+        });
       });
     });
+
   });
 
   describe('clearInvalidTokenOnRehydrate$', () => {

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

@@ -145,7 +145,7 @@ export class AuthEffects {
           if (response.authenticated) {
             return new RetrieveTokenAction();
           } else {
-            return new RetrieveAuthMethodsAction(response);
+            return this.authService.getRetrieveAuthMethodsAction(response);
           }
         }),
         catchError((error) => observableOf(new AuthenticatedErrorAction(error)))
@@ -234,10 +234,10 @@ export class AuthEffects {
     .pipe(
       ofType(AuthActionTypes.RETRIEVE_AUTH_METHODS),
       switchMap((action: RetrieveAuthMethodsAction) => {
-        return this.authService.retrieveAuthMethodsFromAuthStatus(action.payload)
+        return this.authService.retrieveAuthMethodsFromAuthStatus(action.payload.status)
           .pipe(
-            map((authMethodModels: AuthMethod[]) => new RetrieveAuthMethodsSuccessAction(authMethodModels)),
-            catchError((error) => observableOf(new RetrieveAuthMethodsErrorAction()))
+            map((authMethodModels: AuthMethod[]) => new RetrieveAuthMethodsSuccessAction(authMethodModels, action.payload.blocking)),
+            catchError((error) => observableOf(new RetrieveAuthMethodsErrorAction(action.payload.blocking)))
           );
       })
     );

src/app/core/auth/auth.reducer.spec.ts

@@ -512,7 +512,7 @@ describe('authReducer', () => {
       loading: false,
       authMethods: []
     };
-    const action = new RetrieveAuthMethodsAction(new AuthStatus());
+    const action = new RetrieveAuthMethodsAction(new AuthStatus(), true);
     const newState = authReducer(initialState, action);
     state = {
       authenticated: false,
@@ -536,7 +536,7 @@ describe('authReducer', () => {
       new AuthMethod(AuthMethodType.Password),
       new AuthMethod(AuthMethodType.Shibboleth, 'location')
     ];
-    const action = new RetrieveAuthMethodsSuccessAction(authMethods);
+    const action = new RetrieveAuthMethodsSuccessAction(authMethods, false);
     const newState = authReducer(initialState, action);
     state = {
       authenticated: false,
@@ -548,7 +548,31 @@ describe('authReducer', () => {
     expect(newState).toEqual(state);
   });
 
-  it('should properly set the state, in response to a RETRIEVE_AUTH_METHODS_ERROR action', () => {
+  it('should properly set the state, in response to a RETRIEVE_AUTH_METHODS_SUCCESS action with blocking as true', () => {
+    initialState = {
+      authenticated: false,
+      loaded: false,
+      blocking: true,
+      loading: true,
+      authMethods: []
+    };
+    const authMethods = [
+      new AuthMethod(AuthMethodType.Password),
+      new AuthMethod(AuthMethodType.Shibboleth, 'location')
+    ];
+    const action = new RetrieveAuthMethodsSuccessAction(authMethods, true);
+    const newState = authReducer(initialState, action);
+    state = {
+      authenticated: false,
+      loaded: false,
+      blocking: true,
+      loading: false,
+      authMethods: authMethods
+    };
+    expect(newState).toEqual(state);
+  });
+
+  it('should properly set the state, in response to a RETRIEVE_AUTH_METHODS_ERROR action ', () => {
     initialState = {
       authenticated: false,
       loaded: false,
@@ -557,7 +581,7 @@ describe('authReducer', () => {
       authMethods: []
     };
 
-    const action = new RetrieveAuthMethodsErrorAction();
+    const action = new RetrieveAuthMethodsErrorAction(false);
     const newState = authReducer(initialState, action);
     state = {
       authenticated: false,
@@ -568,4 +592,25 @@ describe('authReducer', () => {
     };
     expect(newState).toEqual(state);
   });
+
+  it('should properly set the state, in response to a RETRIEVE_AUTH_METHODS_ERROR action with blocking as true', () => {
+    initialState = {
+      authenticated: false,
+      loaded: false,
+      blocking: true,
+      loading: true,
+      authMethods: []
+    };
+
+    const action = new RetrieveAuthMethodsErrorAction(true);
+    const newState = authReducer(initialState, action);
+    state = {
+      authenticated: false,
+      loaded: false,
+      blocking: true,
+      loading: false,
+      authMethods: [new AuthMethod(AuthMethodType.Password)]
+    };
+    expect(newState).toEqual(state);
+  });
 });

src/app/core/auth/auth.reducer.ts

@@ -10,6 +10,7 @@ import {
   RedirectWhenTokenExpiredAction,
   RefreshTokenSuccessAction,
   RetrieveAuthenticatedEpersonSuccessAction,
+  RetrieveAuthMethodsErrorAction,
   RetrieveAuthMethodsSuccessAction,
   SetRedirectUrlAction
 } from './auth.actions';
@@ -211,14 +212,14 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut
     case AuthActionTypes.RETRIEVE_AUTH_METHODS_SUCCESS:
       return Object.assign({}, state, {
         loading: false,
-        blocking: false,
-        authMethods: (action as RetrieveAuthMethodsSuccessAction).payload
+        blocking: (action as RetrieveAuthMethodsSuccessAction).payload.blocking,
+        authMethods: (action as RetrieveAuthMethodsSuccessAction).payload.authMethods
       });
 
     case AuthActionTypes.RETRIEVE_AUTH_METHODS_ERROR:
       return Object.assign({}, state, {
         loading: false,
-        blocking: false,
+        blocking: (action as RetrieveAuthMethodsErrorAction).payload,
         authMethods: [new AuthMethod(AuthMethodType.Password)]
       });
 

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

@@ -35,6 +35,7 @@ import { AppState } from '../../app.reducer';
 import {
   CheckAuthenticationTokenAction,
   ResetAuthenticationMessagesAction,
+  RetrieveAuthMethodsAction,
   SetRedirectUrlAction
 } from './auth.actions';
 import { NativeWindowRef, NativeWindowService } from '../services/window.service';
@@ -518,4 +519,13 @@ export class AuthService {
     );
   }
 
+  /**
+   * Return a new instance of RetrieveAuthMethodsAction
+   *
+   * @param authStatus The auth status
+   */
+  getRetrieveAuthMethodsAction(authStatus: AuthStatus): RetrieveAuthMethodsAction {
+    return new RetrieveAuthMethodsAction(authStatus, false);
+  }
+
 }

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

@@ -10,6 +10,7 @@ import { AuthService } from './auth.service';
 import { AuthStatus } from './models/auth-status.model';
 import { AuthTokenInfo } from './models/auth-token-info.model';
 import { RemoteData } from '../data/remote-data';
+import { RetrieveAuthMethodsAction } from './auth.actions';
 
 /**
  * The auth service.
@@ -60,4 +61,13 @@ export class ServerAuthService extends AuthService {
       map((rd: RemoteData<AuthStatus>) => Object.assign(new AuthStatus(), rd.payload))
     );
   }
+
+  /**
+   * Return a new instance of RetrieveAuthMethodsAction
+   *
+   * @param authStatus The auth status
+   */
+  getRetrieveAuthMethodsAction(authStatus: AuthStatus): RetrieveAuthMethodsAction {
+    return new RetrieveAuthMethodsAction(authStatus, true);
+  }
 }

src/app/shared/testing/auth-service.stub.ts

@@ -6,6 +6,7 @@ import { EPerson } from '../../core/eperson/models/eperson.model';
 import { createSuccessfulRemoteDataObject$ } from '../remote-data.utils';
 import { AuthMethod } from '../../core/auth/models/auth.method';
 import { hasValue } from '../empty.util';
+import { RetrieveAuthMethodsAction } from '../../core/auth/auth.actions';
 
 export const authMethodsMock = [
   new AuthMethod('password'),
@@ -166,4 +167,12 @@ export class AuthServiceStub {
   clearRedirectUrl() {
     return;
   }
+
+  public replaceToken(token: AuthTokenInfo) {
+    return token;
+  }
+
+  getRetrieveAuthMethodsAction(authStatus: AuthStatus): RetrieveAuthMethodsAction {
+    return;
+  }
 }