Merge branch 'simplify-auth'

This merge switches from JWT-based authN to a simpler and more flexible session-based approach. Closes #25, closes #19
vendure-ecommerce · Sep 25, 2018 · 72c8c9f · 72c8c9f
2 parents f0f39ca + 7e93d4d
commit 72c8c9f
Show file tree

Hide file tree

Showing 44 changed files with 469 additions and 453 deletions.
diff --git a/admin-ui/src/app/core/components/app-shell/app-shell.component.ts b/admin-ui/src/app/core/components/app-shell/app-shell.component.ts
@@ -23,7 +23,8 @@ export class AppShellComponent implements OnInit {
     }
 
     logOut() {
-        this.authService.logOut();
-        this.router.navigate(['/login']);
+        this.authService.logOut().subscribe(() => {
+            this.router.navigate(['/login']);
+        });
     }
 }
diff --git a/admin-ui/src/app/core/providers/auth/auth.service.ts b/admin-ui/src/app/core/providers/auth/auth.service.ts
@@ -1,9 +1,8 @@
 import { Injectable } from '@angular/core';
 import { Observable, of } from 'rxjs';
-import { catchError, map, mergeMap, switchMap } from 'rxjs/operators';
-import { AttemptLogin, SetAsLoggedIn } from 'shared/generated-types';
+import { catchError, mapTo, mergeMap, switchMap } from 'rxjs/operators';
+import { SetAsLoggedIn } from 'shared/generated-types';
 
-import { _ } from '../../../core/providers/i18n/mark-for-extraction';
 import { DataService } from '../../../data/providers/data.service';
 import { LocalStorageService } from '../local-storage/local-storage.service';
 
@@ -18,13 +17,10 @@ export class AuthService {
      * Attempts to log in via the REST login endpoint and updates the app
      * state on success.
      */
-    logIn(username: string, password: string): Observable<SetAsLoggedIn> {
-        return this.dataService.auth.attemptLogin(username, password).pipe(
+    logIn(username: string, password: string, rememberMe: boolean): Observable<SetAsLoggedIn> {
+        return this.dataService.auth.attemptLogin(username, password, rememberMe).pipe(
             switchMap(response => {
-                this.localStorageService.setForSession(
-                    'activeChannelToken',
-                    response.login.user.channelTokens[0],
-                );
+                this.setChannelToken(response.login.user.channelTokens[0]);
                 return this.dataService.client.loginSuccess(username);
             }),
         );
@@ -33,9 +29,19 @@ export class AuthService {
     /**
      * Update the user status to being logged out.
      */
-    logOut(): void {
-        this.dataService.client.logOut();
-        this.localStorageService.remove('authToken');
+    logOut(): Observable<boolean> {
+        return this.dataService.client.userStatus().single$.pipe(
+            switchMap(status => {
+                if (status.userStatus.isLoggedIn) {
+                    return this.dataService.client
+                        .logOut()
+                        .pipe(mergeMap(() => this.dataService.auth.logOut()));
+                } else {
+                    return [];
+                }
+            }),
+            mapTo(true),
+        );
     }
 
     /**
@@ -59,19 +65,20 @@ export class AuthService {
      * that token against the API.
      */
     validateAuthToken(): Observable<boolean> {
-        if (!this.localStorageService.get('authToken')) {
-            return of(false);
-        }
-
         return this.dataService.auth.checkLoggedIn().single$.pipe(
-            map(result => {
+            mergeMap(result => {
                 if (!result.me) {
-                    return false;
+                    return of(false);
                 }
-                this.dataService.client.loginSuccess(result.me.identifier);
-                return true;
+                this.setChannelToken(result.me.channelTokens[0]);
+                return this.dataService.client.loginSuccess(result.me.identifier);
             }),
+            mapTo(true),
             catchError(err => of(false)),
         );
     }
+
+    private setChannelToken(channelToken: string) {
+        this.localStorageService.set('activeChannelToken', channelToken);
+    }
 }
diff --git a/admin-ui/src/app/core/providers/guard/auth.guard.ts b/admin-ui/src/app/core/providers/guard/auth.guard.ts
@@ -16,11 +16,8 @@ export class AuthGuard implements CanActivate {
     canActivate(route: ActivatedRouteSnapshot): Observable<boolean> {
         return this.authService.checkAuthenticatedStatus().pipe(
             tap(authenticated => {
-                if (authenticated) {
-                    return true;
-                } else {
+                if (!authenticated) {
                     this.router.navigate(['/login']);
-                    return false;
                 }
             }),
         );

diff --git a/admin-ui/src/app/data/client-state/client-types.graphql b/admin-ui/src/app/data/client-state/client-types.graphql
@@ -7,8 +7,8 @@ extend type Query {
 extend type Mutation {
     requestStarted: Int!
     requestCompleted: Int!
-    setAsLoggedIn(username: String!, loginTime: String!): UserStatus
-    setAsLoggedOut: UserStatus
+    setAsLoggedIn(username: String!, loginTime: String!): UserStatus!
+    setAsLoggedOut: UserStatus!
     setUiLanguage(languageCode: LanguageCode): LanguageCode
 }
 

diff --git a/admin-ui/src/app/data/data.module.ts b/admin-ui/src/app/data/data.module.ts
@@ -7,7 +7,7 @@ import { ApolloLink } from 'apollo-link';
 import { setContext } from 'apollo-link-context';
 import { withClientState } from 'apollo-link-state';
 import { createUploadLink } from 'apollo-upload-client';
-import { API_PATH, REFRESH_TOKEN_KEY } from 'shared/shared-constants';
+import { API_PATH } from 'shared/shared-constants';
 
 import { environment } from '../../environments/environment';
 import { API_URL } from '../app.config';
@@ -44,20 +44,12 @@ export function createApollo(
             stateLink,
             new OmitTypenameLink(),
             setContext(() => {
-                // Add JWT auth token & channel token to all requests.
                 const channelToken = localStorageService.get('activeChannelToken');
-                const authToken = localStorageService.get('authToken') || '';
-                const refreshToken = localStorageService.get('refreshToken') || '';
-                if (!authToken) {
-                    return {};
-                } else {
+                if (channelToken) {
                     return {
-                        // prettier-ignore
                         headers: {
-                            'Authorization': `Bearer ${authToken}`,
                             'vendure-token': channelToken,
-                            [REFRESH_TOKEN_KEY]: refreshToken,
-                        }
+                        },
                     };
                 }
             }),

diff --git a/admin-ui/src/app/data/definitions/auth-definitions.ts b/admin-ui/src/app/data/definitions/auth-definitions.ts
@@ -5,13 +5,12 @@ export const CURRENT_USER_FRAGMENT = gql`
         id
         identifier
         channelTokens
-        roles
     }
 `;
 
 export const ATTEMPT_LOGIN = gql`
-    mutation AttemptLogin($username: String!, $password: String!) {
-        login(username: $username, password: $password) {
+    mutation AttemptLogin($username: String!, $password: String!, $rememberMe: Boolean!) {
+        login(username: $username, password: $password, rememberMe: $rememberMe) {
             user {
                 ...CurrentUser
             }
@@ -20,6 +19,12 @@ export const ATTEMPT_LOGIN = gql`
     ${CURRENT_USER_FRAGMENT}
 `;
 
+export const LOG_OUT = gql`
+    mutation LogOut {
+        logout
+    }
+`;
+
 export const GET_CURRENT_USER = gql`
     query GetCurrentUser {
         me {

diff --git a/admin-ui/src/app/data/providers/auth-data.service.ts b/admin-ui/src/app/data/providers/auth-data.service.ts
@@ -1,7 +1,7 @@
 import { Observable } from 'rxjs';
-import { AttemptLogin, AttemptLoginVariables, GetCurrentUser } from 'shared/generated-types';
+import { AttemptLogin, AttemptLoginVariables, GetCurrentUser, LogOut } from 'shared/generated-types';
 
-import { ATTEMPT_LOGIN, GET_CURRENT_USER } from '../definitions/auth-definitions';
+import { ATTEMPT_LOGIN, GET_CURRENT_USER, LOG_OUT } from '../definitions/auth-definitions';
 import { QueryResult } from '../query-result';
 
 import { BaseDataService } from './base-data.service';
@@ -13,10 +13,15 @@ export class AuthDataService {
         return this.baseDataService.query<GetCurrentUser>(GET_CURRENT_USER);
     }
 
-    attemptLogin(username: string, password: string): Observable<AttemptLogin> {
+    attemptLogin(username: string, password: string, rememberMe: boolean): Observable<AttemptLogin> {
         return this.baseDataService.mutate<AttemptLogin, AttemptLoginVariables>(ATTEMPT_LOGIN, {
             username,
             password,
+            rememberMe,
         });
     }
+
+    logOut(): Observable<LogOut> {
+        return this.baseDataService.mutate<LogOut>(LOG_OUT);
+    }
 }
diff --git a/admin-ui/src/app/data/providers/client-data.service.ts b/admin-ui/src/app/data/providers/client-data.service.ts
@@ -27,6 +27,10 @@ import { QueryResult } from '../query-result';
 
 import { BaseDataService } from './base-data.service';
 
+/**
+ * Note: local queries all have a fetch-policy of "cache-first" explicitly specified due to:
+ * https://github.com/apollographql/apollo-link-state/issues/236
+ */
 export class ClientDataService {
     constructor(private baseDataService: BaseDataService) {}
 
@@ -39,7 +43,7 @@ export class ClientDataService {
     }
 
     getNetworkStatus(): QueryResult<GetNetworkStatus> {
-        return this.baseDataService.query<GetNetworkStatus>(GET_NEWTORK_STATUS);
+        return this.baseDataService.query<GetNetworkStatus>(GET_NEWTORK_STATUS, {}, 'cache-first');
     }
 
     loginSuccess(username: string): Observable<SetAsLoggedIn> {
@@ -54,11 +58,11 @@ export class ClientDataService {
     }
 
     userStatus(): QueryResult<GetUserStatus> {
-        return this.baseDataService.query<GetUserStatus>(GET_USER_STATUS);
+        return this.baseDataService.query<GetUserStatus>(GET_USER_STATUS, {}, 'cache-first');
     }
 
     uiState(): QueryResult<GetUiState> {
-        return this.baseDataService.query<GetUiState>(GET_UI_STATE);
+        return this.baseDataService.query<GetUiState>(GET_UI_STATE, {}, 'cache-first');
     }
 
     setUiLanguage(languageCode: LanguageCode): Observable<SetUiLanguage> {

diff --git a/admin-ui/src/app/data/providers/data.service.mock.ts b/admin-ui/src/app/data/providers/data.service.mock.ts
@@ -68,6 +68,7 @@ export class MockDataService implements DataServiceMock {
     auth = {
         checkLoggedIn: spyObservable('checkLoggedIn'),
         attemptLogin: spyObservable('attemptLogin'),
+        logOut: spyObservable('logOut'),
     };
     facet = {
         getFacets: spyQueryResult('getFacets'),

diff --git a/admin-ui/src/app/data/providers/fetch-adapter.ts b/admin-ui/src/app/data/providers/fetch-adapter.ts
@@ -20,6 +20,7 @@ export class FetchAdapter {
                 headers: init.headers as any,
                 observe: 'response',
                 responseType: 'json',
+                withCredentials: true,
             })
             .toPromise()
             .then(result => {

diff --git a/admin-ui/src/app/data/providers/interceptor.ts b/admin-ui/src/app/data/providers/interceptor.ts
@@ -10,7 +10,6 @@ import { Injectable, Injector } from '@angular/core';
 import { Router } from '@angular/router';
 import { Observable } from 'rxjs';
 import { tap } from 'rxjs/operators';
-import { AUTH_TOKEN_KEY, REFRESH_TOKEN_KEY } from 'shared/shared-constants';
 
 import { API_URL } from '../../app.config';
 import { AuthService } from '../../core/providers/auth/auth.service';
@@ -42,18 +41,6 @@ export class DefaultInterceptor implements HttpInterceptor {
             tap(
                 event => {
                     if (event instanceof HttpResponse) {
-                        if (event.headers.get(AUTH_TOKEN_KEY)) {
-                            this.localStorageService.setForSession(
-                                'authToken',
-                                event.headers.get(AUTH_TOKEN_KEY),
-                            );
-                        }
-                        if (event.headers.get(REFRESH_TOKEN_KEY)) {
-                            this.localStorageService.set(
-                                'refreshToken',
-                                event.headers.get(REFRESH_TOKEN_KEY),
-                            );
-                        }
                         this.notifyOnError(event);
                         this.dataService.client.completeRequest().subscribe();
                     }
@@ -62,6 +49,8 @@ export class DefaultInterceptor implements HttpInterceptor {
                     if (err instanceof HttpErrorResponse) {
                         this.notifyOnError(err);
                         this.dataService.client.completeRequest().subscribe();
+                    } else {
+                        this.displayErrorNotification(err.message);
                     }
                 },
             ),
@@ -86,12 +75,15 @@ export class DefaultInterceptor implements HttpInterceptor {
                         this.displayErrorNotification(_(`error.401-unauthorized`));
                         break;
                     case 403:
-                        this.displayErrorNotification(_(`error.403-forbidden`));
-                        this.authService.logOut();
-                        this.router.navigate(['/login'], {
-                            queryParams: {
-                                [AUTH_REDIRECT_PARAM]: btoa(this.router.url),
-                            },
+                        this.authService.logOut().subscribe(() => {
+                            if (!window.location.pathname.includes('login')) {
+                                this.displayErrorNotification(_(`error.403-forbidden`));
+                            }
+                            this.router.navigate(['/login'], {
+                                queryParams: {
+                                    [AUTH_REDIRECT_PARAM]: btoa(this.router.url),
+                                },
+                            });
                         });
                         break;
                     default:

diff --git a/admin-ui/src/app/login/components/login/login.component.html b/admin-ui/src/app/login/components/login/login.component.html
@@ -17,7 +17,10 @@
                    [(ngModel)]="password"
                    [placeholder]="'common.password' | translate">
             <div class="checkbox">
-                <input type="checkbox" id="rememberme">
+                <input type="checkbox"
+                       id="rememberme"
+                       name="rememberme"
+                       [(ngModel)]="rememberMe">
                 <label for="rememberme">
                     {{ 'common.remember-me' | translate }}
                 </label>

diff --git a/admin-ui/src/app/login/components/login/login.component.ts b/admin-ui/src/app/login/components/login/login.component.ts
@@ -12,14 +12,15 @@ import { AUTH_REDIRECT_PARAM } from '../../../data/providers/interceptor';
 export class LoginComponent {
     username = '';
     password = '';
+    rememberMe = false;
 
     constructor(private authService: AuthService, private router: Router) {}
 
     logIn(): void {
-        this.authService.logIn(this.username, this.password).subscribe(
+        this.authService.logIn(this.username, this.password, this.rememberMe).subscribe(
             () => {
                 const redirect = this.getRedirectRoute();
-                this.router.navigate([redirect ? redirect : '/']);
+                this.router.navigateByUrl(redirect ? redirect : '/');
             },
             err => {
                 /* error handled by http interceptor */

diff --git a/admin-ui/src/app/login/login.module.ts b/admin-ui/src/app/login/login.module.ts
@@ -5,11 +5,12 @@ import { SharedModule } from '../shared/shared.module';
 
 import { LoginComponent } from './components/login/login.component';
 import { loginRoutes } from './login.routes';
+import { LoginGuard } from './providers/login.guard';
 
 @NgModule({
     imports: [SharedModule, RouterModule.forChild(loginRoutes)],
     exports: [],
     declarations: [LoginComponent],
-    providers: [],
+    providers: [LoginGuard],
 })
 export class LoginModule {}
diff --git a/admin-ui/src/app/login/login.routes.ts b/admin-ui/src/app/login/login.routes.ts
@@ -1,11 +1,13 @@
 import { Routes } from '@angular/router';
 
 import { LoginComponent } from './components/login/login.component';
+import { LoginGuard } from './providers/login.guard';
 
 export const loginRoutes: Routes = [
     {
         path: '',
         component: LoginComponent,
         pathMatch: 'full',
+        canActivate: [LoginGuard],
     },
 ];
diff --git a/admin-ui/src/app/login/providers/login.guard.ts b/admin-ui/src/app/login/providers/login.guard.ts
@@ -0,0 +1,25 @@
+import { Injectable } from '@angular/core';
+import { ActivatedRouteSnapshot, CanActivate, Router } from '@angular/router';
+import { Observable } from 'rxjs';
+import { map } from 'rxjs/operators';
+
+import { AuthService } from '../../core/providers/auth/auth.service';
+
+/**
+ * This guard prevents loggen-in users from navigating to the login screen.
+ */
+@Injectable()
+export class LoginGuard implements CanActivate {
+    constructor(private router: Router, private authService: AuthService) {}
+
+    canActivate(route: ActivatedRouteSnapshot): Observable<boolean> {
+        return this.authService.checkAuthenticatedStatus().pipe(
+            map(authenticated => {
+                if (authenticated) {
+                    this.router.navigate(['/']);
+                }
+                return !authenticated;
+            }),
+        );
+    }
+}