diff --git a/src/app/core/auth/auth.actions.ts b/src/app/core/auth/auth.actions.ts index 96d043eef4..0e082f414e 100644 --- a/src/app/core/auth/auth.actions.ts +++ b/src/app/core/auth/auth.actions.ts @@ -8,18 +8,17 @@ import { type } from '../../shared/ngrx/type'; import { EPerson } from '../eperson/models/eperson.model'; import { AuthTokenInfo } from './models/auth-token-info.model'; import { AuthMethodModel } from './models/auth-method.model'; +import { AuthStatus } from './models/auth-status.model'; export const AuthActionTypes = { AUTHENTICATE: type('dspace/auth/AUTHENTICATE'), - START_SHIBBOLETH_AUTHENTICATION: type('dspace/auth/START_SHIBBOLETH_AUTHENTICATION'), - GET_JWT_AFTER_SHIBB_LOGIN: type('dspace/auth/GET_JWT_AFTER_SHIBB_LOGIN'), AUTHENTICATE_ERROR: type('dspace/auth/AUTHENTICATE_ERROR'), AUTHENTICATE_SUCCESS: type('dspace/auth/AUTHENTICATE_SUCCESS'), AUTHENTICATED: type('dspace/auth/AUTHENTICATED'), AUTHENTICATED_ERROR: type('dspace/auth/AUTHENTICATED_ERROR'), AUTHENTICATED_SUCCESS: type('dspace/auth/AUTHENTICATED_SUCCESS'), CHECK_AUTHENTICATION_TOKEN: type('dspace/auth/CHECK_AUTHENTICATION_TOKEN'), - CHECK_AUTHENTICATION_TOKEN_ERROR: type('dspace/auth/CHECK_AUTHENTICATION_TOKEN_ERROR'), + CHECK_AUTHENTICATION_TOKEN_COOKIE: type('dspace/auth/CHECK_AUTHENTICATION_TOKEN_COOKIE'), RETRIEVE_AUTH_METHODS: type('dspace/auth/RETRIEVE_AUTH_METHODS'), RETRIEVE_AUTH_METHODS_SUCCESS: type('dspace/auth/RETRIEVE_AUTH_METHODS_SUCCESS'), RETRIEVE_AUTH_METHODS_ERROR: type('dspace/auth/RETRIEVE_AUTH_METHODS_ERROR'), @@ -58,29 +57,6 @@ export class AuthenticateAction implements Action { } } -/** - * Authenticate. - * @class StartShibbolethAuthenticationAction - * @implements {Action} - */ -export class StartShibbolethAuthenticationAction implements Action { - public type: string = AuthActionTypes.START_SHIBBOLETH_AUTHENTICATION; - payload: AuthMethodModel; - - constructor(authMethodModel: AuthMethodModel) { - this.payload = authMethodModel; - } -} - -/** - * GetJWTafterShibbLoginAction. - * @class GetJWTafterShibbLoginAction - * @implements {Action} - */ -export class GetJWTafterShibbLoginAction implements Action { - public type: string = AuthActionTypes.GET_JWT_AFTER_SHIBB_LOGIN; -} - /** * Checks if user is authenticated. * @class AuthenticatedAction @@ -166,11 +142,11 @@ export class CheckAuthenticationTokenAction implements Action { /** * Check Authentication Token Error. - * @class CheckAuthenticationTokenErrorAction + * @class CheckAuthenticationTokenCookieAction * @implements {Action} */ -export class CheckAuthenticationTokenErrorAction implements Action { - public type: string = AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_ERROR; +export class CheckAuthenticationTokenCookieAction implements Action { + public type: string = AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_COOKIE; } /** @@ -349,6 +325,12 @@ export class ResetAuthenticationMessagesAction implements Action { */ export class RetrieveAuthMethodsAction implements Action { public type: string = AuthActionTypes.RETRIEVE_AUTH_METHODS; + + payload: AuthStatus; + + constructor(authStatus: AuthStatus) { + this.payload = authStatus; + } } /** @@ -391,14 +373,13 @@ export class SetRedirectUrlAction implements Action { */ export type AuthActions = AuthenticateAction - | GetJWTafterShibbLoginAction | AuthenticatedAction | AuthenticatedErrorAction | AuthenticatedSuccessAction | AuthenticationErrorAction | AuthenticationSuccessAction | CheckAuthenticationTokenAction - | CheckAuthenticationTokenErrorAction + | CheckAuthenticationTokenCookieAction | RedirectWhenAuthenticationIsRequiredAction | RedirectWhenTokenExpiredAction | RegistrationAction diff --git a/src/app/core/auth/auth.effects.spec.ts b/src/app/core/auth/auth.effects.spec.ts index 8c2b4026e0..a29be0b342 100644 --- a/src/app/core/auth/auth.effects.spec.ts +++ b/src/app/core/auth/auth.effects.spec.ts @@ -14,7 +14,7 @@ import { AuthenticatedSuccessAction, AuthenticationErrorAction, AuthenticationSuccessAction, - CheckAuthenticationTokenErrorAction, + CheckAuthenticationTokenCookieAction, LogOutErrorAction, LogOutSuccessAction, RefreshTokenErrorAction, @@ -146,7 +146,7 @@ describe('AuthEffects', () => { actions = hot('--a-', {a: {type: AuthActionTypes.CHECK_AUTHENTICATION_TOKEN, payload: token}}); - const expected = cold('--b-', {b: new CheckAuthenticationTokenErrorAction()}); + const expected = cold('--b-', {b: new CheckAuthenticationTokenCookieAction()}); expect(authEffects.checkToken$).toBeObservable(expected); }); diff --git a/src/app/core/auth/auth.effects.ts b/src/app/core/auth/auth.effects.ts index d6ecc23619..0643896320 100644 --- a/src/app/core/auth/auth.effects.ts +++ b/src/app/core/auth/auth.effects.ts @@ -1,12 +1,10 @@ -import { of as observableOf, Observable } from 'rxjs'; +import { Observable, of as observableOf } from 'rxjs'; -import { filter, debounceTime, switchMap, take, tap, catchError, map } from 'rxjs/operators'; +import { catchError, debounceTime, filter, map, switchMap, take, tap } from 'rxjs/operators'; import { Injectable } from '@angular/core'; - // import @ngrx import { Actions, Effect, ofType } from '@ngrx/effects'; import { Action, select, Store } from '@ngrx/store'; - // import services import { AuthService } from './auth.service'; // import actions @@ -18,7 +16,7 @@ import { AuthenticatedSuccessAction, AuthenticationErrorAction, AuthenticationSuccessAction, - CheckAuthenticationTokenErrorAction, + CheckAuthenticationTokenCookieAction, LogOutErrorAction, LogOutSuccessAction, RefreshTokenAction, @@ -28,8 +26,8 @@ import { RegistrationErrorAction, RegistrationSuccessAction, RetrieveAuthMethodsAction, - RetrieveAuthMethodsSuccessAction, - GetJWTafterShibbLoginAction, StartShibbolethAuthenticationAction, RetrieveAuthMethodsErrorAction + RetrieveAuthMethodsErrorAction, + RetrieveAuthMethodsSuccessAction } from './auth.actions'; import { EPerson } from '../eperson/models/eperson.model'; import { AuthStatus } from './models/auth-status.model'; @@ -37,6 +35,7 @@ import { AuthTokenInfo } from './models/auth-token-info.model'; import { AppState } from '../../app.reducer'; import { isAuthenticated } from './selectors'; import { StoreActionTypes } from '../../store.actions'; +import { AuthMethodModel } from './models/auth-method.model'; @Injectable() export class AuthEffects { @@ -57,22 +56,6 @@ export class AuthEffects { }) ); - /** - * Shib Login. - * @method shibLogin - */ - @Effect() - public shibbLogin$: Observable = this.actions$.pipe( - ofType(AuthActionTypes.GET_JWT_AFTER_SHIBB_LOGIN), - switchMap((action: GetJWTafterShibbLoginAction) => { - return this.authService.startShibbAuth().pipe( - take(1), - map((response: AuthStatus) => new AuthenticationSuccessAction(response.token)), - catchError((error) => observableOf(new AuthenticationErrorAction(error))) - ); - }) - ); - @Effect() public authenticateSuccess$: Observable = this.actions$.pipe( ofType(AuthActionTypes.AUTHENTICATE_SUCCESS), @@ -102,17 +85,27 @@ export class AuthEffects { switchMap(() => { return this.authService.hasValidAuthenticationToken().pipe( map((token: AuthTokenInfo) => new AuthenticatedAction(token)), - catchError((error) => observableOf(new CheckAuthenticationTokenErrorAction())) + catchError((error) => observableOf(new CheckAuthenticationTokenCookieAction())) ); }) ); @Effect() - public checkTokenError$: Observable = this.actions$ - .pipe( - ofType(AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_ERROR), - map(() => new RetrieveAuthMethodsAction()) - ) + public checkTokenError$: Observable = this.actions$.pipe( + ofType(AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_COOKIE), + switchMap(() => { + return this.authService.checkAuthenticationCookie().pipe( + map((response: AuthStatus) => { + if (response.authenticated) { + return new AuthenticatedAction(response.token); + } else { + return new RetrieveAuthMethodsAction(response); + } + }), + catchError((error) => observableOf(new AuthenticatedErrorAction(error))) + ); + }) + ); @Effect() public createUser$: Observable = this.actions$.pipe( @@ -199,14 +192,14 @@ export class AuthEffects { public retrieveMethods$: Observable = this.actions$ .pipe( ofType(AuthActionTypes.RETRIEVE_AUTH_METHODS), - switchMap(() => { - return this.authService.retrieveAuthMethods() + switchMap((action: RetrieveAuthMethodsAction) => { + return this.authService.retrieveAuthMethods(action.payload) .pipe( - map((location: any) => new RetrieveAuthMethodsSuccessAction(location)), + map((authMethodModels: AuthMethodModel[]) => new RetrieveAuthMethodsSuccessAction(authMethodModels)), catchError((error) => observableOf(new RetrieveAuthMethodsErrorAction())) ) }) - ) + ); /** * @constructor diff --git a/src/app/core/auth/auth.interceptor.ts b/src/app/core/auth/auth.interceptor.ts index c8faad4e0c..41b367eaa6 100644 --- a/src/app/core/auth/auth.interceptor.ts +++ b/src/app/core/auth/auth.interceptor.ts @@ -123,6 +123,7 @@ export class AuthInterceptor implements HttpInterceptor { } else { authMethodModels.push(new AuthMethodModel(AuthMethodType.Password)); } + authMethodModels.push(new AuthMethodModel(AuthMethodType.Shibboleth, 'location')); return authMethodModels; } @@ -176,7 +177,7 @@ export class AuthInterceptor implements HttpInterceptor { // Get the auth header from the service. const Authorization = authService.buildAuthHeader(token); // Clone the request to add the new header. - newReq = req.clone({headers: req.headers.set('authorization', Authorization), withCredentials: true}); + newReq = req.clone({headers: req.headers.set('authorization', Authorization)}); } else { newReq = req.clone({withCredentials: true}); } diff --git a/src/app/core/auth/auth.reducer.spec.ts b/src/app/core/auth/auth.reducer.spec.ts index ca2ba00036..5c67174f69 100644 --- a/src/app/core/auth/auth.reducer.spec.ts +++ b/src/app/core/auth/auth.reducer.spec.ts @@ -8,7 +8,7 @@ import { AuthenticationErrorAction, AuthenticationSuccessAction, CheckAuthenticationTokenAction, - CheckAuthenticationTokenErrorAction, + CheckAuthenticationTokenCookieAction, LogOutAction, LogOutErrorAction, LogOutSuccessAction, @@ -18,10 +18,16 @@ import { RefreshTokenErrorAction, RefreshTokenSuccessAction, ResetAuthenticationMessagesAction, + RetrieveAuthMethodsAction, + RetrieveAuthMethodsErrorAction, + RetrieveAuthMethodsSuccessAction, SetRedirectUrlAction } from './auth.actions'; import { AuthTokenInfo } from './models/auth-token-info.model'; import { EPersonMock } from '../../shared/testing/eperson-mock'; +import { AuthStatus } from './models/auth-status.model'; +import { AuthMethodModel } from './models/auth-method.model'; +import { AuthMethodType } from '../../shared/log-in/methods/authMethods-type'; describe('authReducer', () => { @@ -158,18 +164,18 @@ describe('authReducer', () => { expect(newState).toEqual(state); }); - it('should properly set the state, in response to a CHECK_AUTHENTICATION_TOKEN_ERROR action', () => { + it('should properly set the state, in response to a CHECK_AUTHENTICATION_TOKEN_COOKIE action', () => { initialState = { authenticated: false, loaded: false, loading: true, }; - const action = new CheckAuthenticationTokenErrorAction(); + const action = new CheckAuthenticationTokenCookieAction(); const newState = authReducer(initialState, action); state = { authenticated: false, loaded: false, - loading: false, + loading: true, }; expect(newState).toEqual(state); }); @@ -408,4 +414,63 @@ describe('authReducer', () => { }; expect(newState).toEqual(state); }); + + it('should properly set the state, in response to a RETRIEVE_AUTH_METHODS action', () => { + initialState = { + authenticated: false, + loaded: false, + loading: false, + authMethods: [] + }; + const action = new RetrieveAuthMethodsAction(new AuthStatus()); + const newState = authReducer(initialState, action); + state = { + authenticated: false, + loaded: false, + loading: true, + authMethods: [] + }; + expect(newState).toEqual(state); + }); + + it('should properly set the state, in response to a RETRIEVE_AUTH_METHODS_SUCCESS action', () => { + initialState = { + authenticated: false, + loaded: false, + loading: true, + authMethods: [] + }; + const authMethods = [ + new AuthMethodModel(AuthMethodType.Password), + new AuthMethodModel(AuthMethodType.Shibboleth, 'location') + ]; + const action = new RetrieveAuthMethodsSuccessAction(authMethods); + const newState = authReducer(initialState, action); + state = { + authenticated: false, + loaded: false, + 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, + loading: true, + authMethods: [] + }; + + const action = new RetrieveAuthMethodsErrorAction(); + const newState = authReducer(initialState, action); + state = { + authenticated: false, + loaded: false, + loading: false, + authMethods: [] + }; + expect(newState).toEqual(state); + }); }); diff --git a/src/app/core/auth/auth.reducer.ts b/src/app/core/auth/auth.reducer.ts index 8df059cb71..2019920670 100644 --- a/src/app/core/auth/auth.reducer.ts +++ b/src/app/core/auth/auth.reducer.ts @@ -62,7 +62,7 @@ const initialState: AuthState = { authenticated: false, loaded: false, loading: false, - authMethods: new Array() + authMethods: [] }; /** @@ -81,21 +81,9 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut info: undefined }); - case AuthActionTypes.START_SHIBBOLETH_AUTHENTICATION: - return Object.assign({}, state, { - error: undefined, - loading: true, - info: undefined - }); - - case AuthActionTypes.GET_JWT_AFTER_SHIBB_LOGIN: - return Object.assign({}, state, { - error: undefined, - loading: true, - info: undefined - }); - case AuthActionTypes.AUTHENTICATED: + case AuthActionTypes.CHECK_AUTHENTICATION_TOKEN: + case AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_COOKIE: return Object.assign({}, state, { loading: true }); @@ -129,21 +117,10 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut loading: false }); - case AuthActionTypes.AUTHENTICATED: case AuthActionTypes.AUTHENTICATE_SUCCESS: case AuthActionTypes.LOG_OUT: return state; - case AuthActionTypes.CHECK_AUTHENTICATION_TOKEN: - return Object.assign({}, state, { - loading: true - }); - - case AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_ERROR: - return Object.assign({}, state, { - loading: false - }); - case AuthActionTypes.LOG_OUT_ERROR: return Object.assign({}, state, { authenticated: true, diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index e49d4f128c..e57269e18f 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -18,7 +18,11 @@ import { isEmpty, isNotEmpty, isNotNull, isNotUndefined } from '../../shared/emp import { CookieService } from '../services/cookie.service'; import { getAuthenticationToken, getRedirectUrl, isAuthenticated, isTokenRefreshing } from './selectors'; import { AppState, routerStateSelector } from '../../app.reducer'; -import { ResetAuthenticationMessagesAction, SetRedirectUrlAction } from './auth.actions'; +import { + CheckAuthenticationTokenAction, + ResetAuthenticationMessagesAction, + SetRedirectUrlAction +} from './auth.actions'; import { NativeWindowRef, NativeWindowService } from '../services/window.service'; import { Base64EncodeUrl } from '../../shared/utils/encode-decode.util'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; @@ -115,17 +119,11 @@ export class AuthService { } - public startShibbAuth(): Observable { - - return this.authRequestService.postToEndpoint('login').pipe( - map((status: AuthStatus) => { - if (status.authenticated) { - return status; - } else { - throw(new Error('Shibboleth login failed')); - } - })) - + /** + * Checks if token is present into the request cookie + */ + public checkAuthenticationCookie(): Observable { + return this.authRequestService.postToEndpoint('login'); } /** @@ -162,7 +160,7 @@ export class AuthService { * Checks if token is present into browser storage and is valid. (NB Check is done only on SSR) */ public checkAuthenticationToken() { - return + this.store.dispatch(new CheckAuthenticationTokenAction()); } /** @@ -215,16 +213,12 @@ export class AuthService { * Retrieve authentication methods available * @returns {User} */ - public retrieveAuthMethods(): Observable { - return this.authRequestService.postToEndpoint('login', {}).pipe( - map((status: AuthStatus) => { - let authMethods: AuthMethodModel[]; - if (isNotEmpty(status.authMethods)) { - authMethods = status.authMethods; - } - return authMethods; - }) - ) + public retrieveAuthMethods(status: AuthStatus): Observable { + let authMethods: AuthMethodModel[] = []; + if (isNotEmpty(status.authMethods)) { + authMethods = status.authMethods; + } + return observableOf(authMethods); } /** diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index 271164e1a6..00c114bbfb 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -45,13 +45,6 @@ export class ServerAuthService extends AuthService { })); } - /** - * Checks if token is present into browser storage and is valid. (NB Check is done only on SSR) - */ - public checkAuthenticationToken() { - this.store.dispatch(new CheckAuthenticationTokenAction()); - } - /** * Redirect to the route navigated before the login */