diff --git a/src/app/core/auth/auth.actions.ts b/src/app/core/auth/auth.actions.ts index f80be89034..e2cef3562f 100644 --- a/src/app/core/auth/auth.actions.ts +++ b/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; + } } /** diff --git a/src/app/core/auth/auth.effects.spec.ts b/src/app/core/auth/auth.effects.spec.ts index 5d530f39a9..cd4f456b44 100644 --- a/src/app/core/auth/auth.effects.spec.ts +++ b/src/app/core/auth/auth.effects.spec.ts @@ -43,10 +43,12 @@ describe('AuthEffects', () => { let initialState; let token; let store: MockStore; + 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$', () => { diff --git a/src/app/core/auth/auth.effects.ts b/src/app/core/auth/auth.effects.ts index 9452af1fb8..2ef90dd76c 100644 --- a/src/app/core/auth/auth.effects.ts +++ b/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))) ); }) ); diff --git a/src/app/core/auth/auth.reducer.spec.ts b/src/app/core/auth/auth.reducer.spec.ts index 4c6f1e2a25..914a1a152d 100644 --- a/src/app/core/auth/auth.reducer.spec.ts +++ b/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); + }); }); diff --git a/src/app/core/auth/auth.reducer.ts b/src/app/core/auth/auth.reducer.ts index 6d5635f263..dfe29a3ef2 100644 --- a/src/app/core/auth/auth.reducer.ts +++ b/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)] }); diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index fa29f1bc36..ed4fca615c 100644 --- a/src/app/core/auth/auth.service.ts +++ b/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); + } + } diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index 9840b22267..cccc1490f8 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/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) => 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); + } } diff --git a/src/app/shared/testing/auth-service.stub.ts b/src/app/shared/testing/auth-service.stub.ts index 3fa09b2b7e..9a7622b7b8 100644 --- a/src/app/shared/testing/auth-service.stub.ts +++ b/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; + } }