Fixed authentication module

src/app/+login-page/login-page.component.ts

@@ -1,10 +1,21 @@
-import { Component } from '@angular/core';
+import { Component, OnDestroy } from '@angular/core';
+
+import { Store } from '@ngrx/store';
+
+import { AppState } from '../app.reducer';
+import { ResetAuthenticationMessagesAction } from '../core/auth/auth.actions';
 
 @Component({
   selector: 'ds-login-page',
   styleUrls: ['./login-page.component.scss'],
   templateUrl: './login-page.component.html'
 })
-export class LoginPageComponent {
+export class LoginPageComponent implements OnDestroy {
 
+  constructor(private store: Store<AppState>) {}
+
+  ngOnDestroy() {
+    // Clear all authentication messages when leaving login page
+    this.store.dispatch(new ResetAuthenticationMessagesAction());
+  }
 }

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

@@ -17,8 +17,9 @@ export const AuthActionTypes = {
   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'),
-  REDIRECT: type('dspace/auth/REDIRECT'),
-  RESET_ERROR: type('dspace/auth/RESET_ERROR'),
+  REDIRECT_TOKEN_EXPIRED: type('dspace/auth/REDIRECT_TOKEN_EXPIRED'),
+  REDIRECT_AUTHENTICATION_REQUIRED: type('dspace/auth/REDIRECT_AUTHENTICATION_REQUIRED'),
+  RESET_MESSAGES: type('dspace/auth/RESET_MESSAGES'),
   LOG_OUT: type('dspace/auth/LOG_OUT'),
   LOG_OUT_ERROR: type('dspace/auth/LOG_OUT_ERROR'),
   LOG_OUT_SUCCESS: type('dspace/auth/LOG_OUT_SUCCESS'),
@@ -171,13 +172,27 @@ export class LogOutSuccessAction implements Action {
   constructor(public payload?: any) {}
 }
 
+/**
+ * Redirect to login page when authentication is required.
+ * @class RedirectWhenAuthenticationIsRequiredAction
+ * @implements {Action}
+ */
+export class RedirectWhenAuthenticationIsRequiredAction implements Action {
+  public type: string = AuthActionTypes.REDIRECT_AUTHENTICATION_REQUIRED;
+  payload: string;
+
+  constructor(message: string) {
+    this.payload = message ;
+  }
+}
+
 /**
  * Redirect to login page when token is expired.
  * @class RedirectWhenTokenExpiredAction
  * @implements {Action}
  */
 export class RedirectWhenTokenExpiredAction implements Action {
-  public type: string = AuthActionTypes.REDIRECT;
+  public type: string = AuthActionTypes.REDIRECT_TOKEN_EXPIRED;
   payload: string;
 
   constructor(message: string) {
@@ -229,11 +244,11 @@ export class RegistrationSuccessAction implements Action {
 
 /**
  * Reset error.
- * @class ResetAuthenticationErrorAction
+ * @class ResetAuthenticationMessagesAction
  * @implements {Action}
  */
-export class ResetAuthenticationErrorAction implements Action {
-  public type: string = AuthActionTypes.RESET_ERROR;
+export class ResetAuthenticationMessagesAction implements Action {
+  public type: string = AuthActionTypes.RESET_MESSAGES;
 }
 
 /* tslint:enable:max-classes-per-file */
@@ -252,6 +267,7 @@ export type AuthActions
   | AuthenticationSuccessAction
   | CheckAuthenticationTokenAction
   | CheckAuthenticationTokenErrorAction
+  | RedirectWhenAuthenticationIsRequiredAction
   | RedirectWhenTokenExpiredAction
   | RegistrationAction
   | RegistrationErrorAction

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

@@ -112,11 +112,12 @@ export class AuthEffects {
   @Effect({dispatch: false})
   public logOutSuccess: Observable<Action> = this.actions$
     .ofType(AuthActionTypes.LOG_OUT_SUCCESS)
-    .do((action: LogOutSuccessAction) => this.authService.removeToken());
+    .do(() => this.authService.removeToken())
+    .do(() => this.authService.refreshPage());
 
   @Effect({dispatch: false})
   public redirectToLogin: Observable<Action> = this.actions$
-    .ofType(AuthActionTypes.REDIRECT)
+    .ofType(AuthActionTypes.REDIRECT_TOKEN_EXPIRED, AuthActionTypes.REDIRECT_AUTHENTICATION_REQUIRED)
     .do(() => this.authService.redirectToLogin());
 
   /**

src/app/core/auth/auth.interceptor.ts

@@ -1,20 +1,19 @@
 import { Injectable, Injector } from '@angular/core';
-import { Router } from '@angular/router';
 import {
-  HttpClient, HttpEvent, HttpInterceptor, HttpHandler, HttpRequest, HttpResponse,
+  HttpEvent, HttpInterceptor, HttpHandler, HttpRequest, HttpResponse,
   HttpErrorResponse
 } from '@angular/common/http';
+
 import { Observable } from 'rxjs/Rx';
 import 'rxjs/add/observable/throw'
 import 'rxjs/add/operator/catch';
 
+import { AppState } from '../../app.reducer';
+import { AuthError } from './models/auth-error.model';
 import { AuthService } from './auth.service';
 import { AuthStatus } from './models/auth-status.model';
-import { AuthType } from './auth-type';
-import { ResourceType } from '../shared/resource-type';
 import { AuthTokenInfo } from './models/auth-token-info.model';
 import { isNotEmpty } from '../../shared/empty.util';
-import { AppState } from '../../app.reducer';
 import { RedirectWhenTokenExpiredAction } from './auth.actions';
 import { Store } from '@ngrx/store';
 
@@ -47,7 +46,7 @@ export class AuthInterceptor implements HttpInterceptor {
       authStatus.token = new AuthTokenInfo(accessToken);
     } else {
       authStatus.authenticated = false;
-      authStatus.error = isNotEmpty(error) ? JSON.parse(error) : null;
+      authStatus.error = isNotEmpty(error) ? ((typeof error === 'string') ? JSON.parse(error) : error) : null;
     }
     return authStatus;
   }

src/app/core/auth/auth.reducers.ts

@@ -1,7 +1,8 @@
 // import actions
 import {
   AuthActions, AuthActionTypes, AuthenticatedSuccessAction, AuthenticationErrorAction,
-  AuthenticationSuccessAction, LogOutErrorAction, RedirectWhenTokenExpiredAction
+  AuthenticationSuccessAction, LogOutErrorAction, RedirectWhenAuthenticationIsRequiredAction,
+  RedirectWhenTokenExpiredAction
 } from './auth.actions';
 
 // import models
@@ -26,7 +27,7 @@ export interface AuthState {
   loading: boolean;
 
   // info message
-  message?: string;
+  info?: string;
 
   // the authenticated user
   user?: Eperson;
@@ -54,7 +55,7 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut
       return Object.assign({}, state, {
         error: undefined,
         loading: true,
-        message: undefined
+        info: undefined
       });
 
     case AuthActionTypes.AUTHENTICATED_ERROR:
@@ -71,7 +72,7 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut
         loaded: true,
         error: undefined,
         loading: false,
-        message: undefined,
+        info: undefined,
         user: (action as AuthenticatedSuccessAction).payload.user
       });
 
@@ -96,18 +97,6 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut
         loading: false
       });
 
-    case AuthActionTypes.REGISTRATION_SUCCESS:
-      return state;
-
-    case AuthActionTypes.RESET_ERROR:
-      return Object.assign({}, state, {
-        authenticated: false,
-        error: undefined,
-        loaded: false,
-        loading: false,
-        message: undefined,
-      });
-
     case AuthActionTypes.LOG_OUT_ERROR:
       return Object.assign({}, state, {
         authenticated: true,
@@ -121,16 +110,17 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut
         error: undefined,
         loaded: false,
         loading: false,
-        message: undefined,
+        info: undefined,
         user: undefined
       });
 
-    case AuthActionTypes.REDIRECT:
+    case AuthActionTypes.REDIRECT_AUTHENTICATION_REQUIRED:
+    case AuthActionTypes.REDIRECT_TOKEN_EXPIRED:
       return Object.assign({}, state, {
         authenticated: false,
         loaded: false,
         loading: false,
-        message: (action as RedirectWhenTokenExpiredAction).payload,
+        info: (action as RedirectWhenTokenExpiredAction as RedirectWhenAuthenticationIsRequiredAction).payload,
         user: undefined
       });
 
@@ -139,7 +129,19 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut
         authenticated: false,
         error: undefined,
         loading: true,
-        message: undefined
+        info: undefined
+      });
+
+    case AuthActionTypes.REGISTRATION_SUCCESS:
+      return state;
+
+    case AuthActionTypes.RESET_MESSAGES:
+      return Object.assign({}, state, {
+        authenticated: false,
+        error: undefined,
+        loaded: false,
+        loading: false,
+        info: undefined,
       });
 
     default:
@@ -181,11 +183,11 @@ export const getAuthenticationError = (state: AuthState) => state.error;
 
 /**
  * Returns the authentication info message.
- * @function getAuthenticationError
+ * @function getAuthenticationInfo
  * @param {State} state
  * @returns {String}
  */
-export const getAuthenticationMessage = (state: AuthState) => state.message;
+export const getAuthenticationInfo = (state: AuthState) => state.info;
 
 /**
  * Returns true if request is in progress.

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

@@ -10,12 +10,12 @@ import { AuthStatus } from './models/auth-status.model';
 import { AuthTokenInfo, TOKENITEM } from './models/auth-token-info.model';
 import { isNotEmpty, isNotNull, isNotUndefined } from '../../shared/empty.util';
 import { CookieService } from '../../shared/services/cookie.service';
-import { ActivatedRoute, Router } from '@angular/router';
 import { isAuthenticated } from './selectors';
 import { AppState, routerStateSelector } from '../../app.reducer';
 import { Store } from '@ngrx/store';
-import { ResetAuthenticationErrorAction } from './auth.actions';
+import { ResetAuthenticationMessagesAction } from './auth.actions';
 import { RouterReducerState } from '@ngrx/router-store';
+import { Router } from '@angular/router';
 
 export const LOGIN_ROUTE = '/login';
 /**
@@ -36,8 +36,7 @@ export class AuthService {
    */
   private _redirectUrl: string;
 
-  constructor(private route: ActivatedRoute,
-              private authRequestService: AuthRequestService,
+  constructor(private authRequestService: AuthRequestService,
               private router: Router,
               private storage: CookieService,
               private store: Store<AppState>) {
@@ -46,7 +45,7 @@ export class AuthService {
       .subscribe((authenticated: boolean) => this._authenticated = authenticated);
 
     // If current route is different from the one setted in authentication guard
-    // and is not the login route, clear it
+    // and is not the login route, clear redirect url and messages
     this.store.select(routerStateSelector)
       .filter((routerState: RouterReducerState) => isNotUndefined(routerState))
       .filter((routerState: RouterReducerState) =>
@@ -54,7 +53,7 @@ export class AuthService {
         && isNotEmpty(this._redirectUrl)
         && (routerState.state.url !== this._redirectUrl))
       .distinctUntilChanged()
-      .subscribe((routerState: RouterReducerState) => {
+      .subscribe(() => {
         this._redirectUrl = '';
       })
   }
@@ -125,7 +124,7 @@ export class AuthService {
    * Clear authentication errors
    */
   public resetAuthenticationError(): void {
-    this.store.dispatch(new ResetAuthenticationErrorAction());
+    this.store.dispatch(new ResetAuthenticationMessagesAction());
   }
 
   /**
@@ -221,6 +220,19 @@ export class AuthService {
     }
   }
 
+  /**
+   * Refresh route navigated
+   */
+  public refreshPage() {
+    this.store.select(routerStateSelector)
+      .take(1)
+      .subscribe((router) => {
+        // TODO Chack a way to hard refresh the same route
+        // this.router.navigate([router.state.url],  { replaceUrl: true });
+        this.router.navigate(['/']);
+      })
+  }
+
   /**
    * Get redirect url
    */

src/app/core/auth/authenticated.guard.ts

@@ -8,6 +8,8 @@ import { Store } from '@ngrx/store';
 import { CoreState } from '../core.reducers';
 import { isAuthenticated, isAuthenticationLoading } from './selectors';
 import { AuthService } from './auth.service';
+import { RedirectWhenAuthenticationIsRequiredAction } from './auth.actions';
+import { isEmpty } from '../../shared/empty.util';
 
 /**
  * Prevent unauthorized activating and loading of routes
@@ -27,7 +29,6 @@ export class AuthenticatedGuard implements CanActivate, CanLoad {
    */
   canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean> {
     const url = state.url;
-
     return this.handleAuth(url);
   }
 
@@ -50,15 +51,18 @@ export class AuthenticatedGuard implements CanActivate, CanLoad {
   }
 
   private handleAuth(url: string): Observable<boolean> {
-    console.log('handleAuth', url)
     // get observable
     const observable = this.store.select(isAuthenticated);
 
     // redirect to sign in page if user is not authenticated
-    observable.subscribe((authenticated) => {
+    observable
+      // .filter(() => isEmpty(this.router.routerState.snapshot.url) || this.router.routerState.snapshot.url === url)
+      .take(1)
+      .subscribe((authenticated) => {
+        console.log('handleAuth', url, this.router.routerState.snapshot.url);
         if (!authenticated) {
           this.authService.redirectUrl = url;
-        this.authService.redirectToLogin();
+          this.store.dispatch(new RedirectWhenAuthenticationIsRequiredAction('Login required'));
         }
     });
 

src/app/core/auth/selectors.ts

@@ -40,12 +40,12 @@ export const getAuthenticationError = createSelector(getAuthState, auth.getAuthe
 
 /**
  * Returns the authentication info message.
- * @function getAuthenticationError
+ * @function getAuthenticationInfo
  * @param {AuthState} state
  * @param {any} props
- * @return {Error}
+ * @return {string}
  */
-export const getAuthenticationMessage = createSelector(getAuthState, auth.getAuthenticationMessage);
+export const getAuthenticationInfo = createSelector(getAuthState, auth.getAuthenticationInfo);
 
 /**
  * Returns true if the user is authenticated

src/app/shared/log-in/log-in.component.ts

@@ -10,11 +10,11 @@ import 'rxjs/add/operator/filter';
 import 'rxjs/add/operator/takeWhile';
 
 // actions
-import { AuthenticateAction, ResetAuthenticationErrorAction } from '../../core/auth/auth.actions';
+import { AuthenticateAction, ResetAuthenticationMessagesAction } from '../../core/auth/auth.actions';
 
 // reducers
 import {
-  getAuthenticationError, getAuthenticationMessage,
+  getAuthenticationError, getAuthenticationInfo,
   isAuthenticated,
   isAuthenticationLoading,
 } from '../../core/auth/selectors';
@@ -109,7 +109,7 @@ export class LogInComponent implements OnDestroy, OnInit {
       });
 
     // set error
-    this.message = this.store.select(getAuthenticationMessage)
+    this.message = this.store.select(getAuthenticationInfo)
       .map((message) => {
         this.hasMessage = (isNotEmpty(message));
         return message;
@@ -140,7 +140,7 @@ export class LogInComponent implements OnDestroy, OnInit {
    */
   public resetErrorOrMessage() {
     if (this.hasError || this.hasMessage) {
-      this.store.dispatch(new ResetAuthenticationErrorAction());
+      this.store.dispatch(new ResetAuthenticationMessagesAction());
       this.hasError = false;
       this.hasMessage = false;
     }