feat (AuthService): added a way to remove listeners

docs: improved

src/app/app.component.spec.ts

@@ -3,6 +3,16 @@ import {
     TestBed,
     async
 } from '@angular/core/testing';
+import {
+    HttpTestingController
+} from '@angular/common/http/testing';
+import {
+    of
+} from 'rxjs';
+import {
+    first,
+    tap
+} from 'rxjs/operators';
 
 import {
     CommonTestingModule
@@ -15,11 +25,15 @@ import {
 import {
     AppComponent
 } from './app.component';
+import {
+    VALID_TOKEN_1
+} from 'testing/tokens';
 
 describe('AppComponent', () => {
     let auth: AuthService;
     let fixture: ComponentFixture < AppComponent > ;
     let app: AppComponent;
+    let httpController: HttpTestingController;
 
     beforeEach(async (() => {
         TestBed.configureTestingModule({
@@ -36,22 +50,227 @@ describe('AppComponent', () => {
         app = fixture.componentInstance;
 
         auth = TestBed.get(AuthService);
+        httpController = TestBed.get(HttpTestingController);
     }));
 
-    it('should create the app', async (() => {
+    afterEach(() => {
+        auth.logOut();
+        auth.ngOnDestroy();
+    });
+
+    it('should create the app', () => {
         expect(app).toBeTruthy();
-    }));
+    });
+
+    it(`should call refresh`, () => {
+        const refresh = spyOn(auth, 'refresh').and.returnValue( of (true));
+        app.refresh();
+        expect(refresh).toHaveBeenCalledTimes(1);
+    });
+
+    it(`should create AAP account`, () => {
+        const createAccount = spyOn(auth, 'createAAPaccount').and.returnValue( of (true));
+
+        const initialState = {
+            ...app.createAAP.value
+        };
+        expect(app.createAAP.invalid).toBe(true);
+
+        const firstUser = {
+            ...initialState,
+            username: 'username',
+            password: '44444'
+        };
+        app.createAAP.reset(firstUser);
+        expect(app.createAAP.valid).toBe(true);
+        app.createAAPaccount();
+        expect(createAccount).toHaveBeenCalledWith(firstUser);
+        expect(app.createAAP.value).toEqual(initialState);
+        expect(app.createAAP.invalid).toBe(true);
+
+        const secondUser = {
+            name: '12345',
+            username: 'username',
+            password: 'password',
+            email: 'email@com',
+            organization: 'organization'
+        };
+        app.createAAP.reset(secondUser);
+        expect(app.createAAP.valid).toBe(true);
+        app.createAAPaccount();
+        expect(createAccount).toHaveBeenCalledWith(secondUser);
+        expect(app.createAAP.value).toEqual(initialState);
+        expect(app.createAAP.invalid).toBe(true);
+    });
+
+    it(`should login AAP account`, () => {
+        expect(app.loginAAP.invalid).toBe(true);
+        const initialState = {
+            ...app.loginAAP.value
+        };
+
+        app.loginAAP.reset({
+            username: 'test1',
+            password: 'bbbbb'
+        });
+        expect(app.loginAAP.valid).toBe(true);
+
+        app.loginAAPaccount();
+
+        httpController.expectOne((app as any)._authURL).flush(VALID_TOKEN_1);
+
+        app.isAuthenticated$.pipe(
+            first(),
+            tap(result => expect(result).toEqual('Yes!')),
+            tap(_ => expect(app.loginAAP.value).toEqual(initialState)),
+            tap(_ => expect(app.loginAAP.invalid).toBe(true))
+        ).subscribe();
+    });
+
+    it(`should change password AAP account`, () => {
+        const changePassword = spyOn(auth, 'changePasswordAAP').and.returnValue( of (true));
+
+        const initialState = {
+            ...app.changePasswordAAP.value
+        };
+        expect(app.changePasswordAAP.invalid).toBe(true);
+
+        const firstChange = {
+            username: 'username',
+            oldPassword: '44444',
+            newPassword: '12345'
+        };
+        app.changePasswordAAP.reset(firstChange);
+        expect(app.changePasswordAAP.valid).toBe(true);
+        app.changePasswordAAPaccount();
+        expect(changePassword).toHaveBeenCalledWith(firstChange);
+        expect(app.changePasswordAAP.value).toEqual(initialState);
+        expect(app.changePasswordAAP.invalid).toBe(true);
+    });
+
+    it(`should create domain`, () => {
+        const refresh = spyOn(app, 'refresh');
+
+        // Login so we can see the Authorization header
+        app.loginAAP.reset({
+            username: 'test1',
+            password: 'bbbbb'
+        });
+        expect(app.loginAAP.valid).toBe(true);
+
+        app.loginAAPaccount();
+
+        const uid = 'dummy-uid';
+        const newDomain = 'my-new-domain';
+
+        const loginRequests = httpController.expectOne((app as any)._authURL);
+        loginRequests.flush(VALID_TOKEN_1);
+        expect(loginRequests.request.headers.get('Authorization')).toMatch(/Basic .+/);
 
-    it(`should not be authenticated`, async (() => {
+        app.domains$.pipe(
+            first(),
+            tap(domains => expect(domains).toEqual(['aap-users-domain']))
+        ).subscribe();
+
+        const initialState = {
+            ...app.domain.value
+        };
+        expect(app.domain.invalid).toBe(true);
+
+        // Partial completion
+        app.domain.reset({
+            domainDesc: ''
+        });
+        expect(app.domain.invalid).toBe(true);
+
+        const firstDomain = {
+            domainName: 'hello'
+        };
+        app.domain.reset(firstDomain);
+        expect(app.domain.valid).toBe(true);
+        app.createDomain(uid);
+
+        const domainRequests0 = httpController.expectOne((app as any)._domainsURL);
+        domainRequests0.flush({
+            domainReference: newDomain
+        });
+        const domainRequests1 = httpController.expectOne(`${(app as any)._domainsURL}/${newDomain}/${uid}/user`);
+        domainRequests1.flush('ignored');
+
+        expect(domainRequests0.request.headers.get('Authorization')).toMatch(`Bearer ${VALID_TOKEN_1}`);
+        expect(domainRequests0.request.method).toBe('POST');
+
+        expect(domainRequests1.request.headers.get('Authorization')).toMatch(`Bearer ${VALID_TOKEN_1}`);
+        expect(domainRequests1.request.method).toBe('PUT');
+
+        expect(refresh).toHaveBeenCalledTimes(2);
         auth.logOut();
-        app.isAuthenticated$.subscribe(result => {
-            expect(result).toEqual('Nope');
     });
+
+    it(`should delete domain`, () => {
+        const refresh = spyOn(app, 'refresh');
+        const gid = 'my-gid';
+
+        app.deleteDomain(gid);
+
+        const req = httpController.expectOne(`${(app as any)._domainsURL}/${gid}`);
+        req.flush('dummy');
+
+        expect(req.request.method).toBe('DELETE');
+        expect(refresh).toHaveBeenCalledTimes(1);
+    });
+
+    it(`should list managed domains`, () => {
+        const managedDomains = spyOn(app, 'listManagedDomains').and.callThrough();
+
+        const myManagedDomains = [{
+            domainName: 'domainName',
+            domainDesc: 'domainDesc',
+            domainReference: 'domainReference'
+        }];
+
+        app.managedDomains$.pipe(
+            first(),
+            tap(domains => expect(domains).toEqual(myManagedDomains))
+        ).subscribe();
+
+        httpController.expectOne((app as any)._managementURL).flush(myManagedDomains);
+
+        expect(managedDomains).toHaveBeenCalledTimes(1);
+    });
+
+    it(`should fail to list managed domains`, () => {
+        const managedDomains = spyOn(app, 'listManagedDomains').and.callThrough();
+
+        const myManagedDomains = [{
+            domainName: 'domainName',
+            domainDesc: 'domainDesc',
+            domainReference: 'domainReference'
+        }];
+
+        app.managedDomains$.pipe(
+            first(),
+            tap(domains => expect(domains).toEqual([]))
+        ).subscribe();
+
+        httpController.expectOne((app as any)._managementURL).flush(myManagedDomains, {
+            status: 500,
+            statusText: 'Problem'
+        });
+
+        expect(managedDomains).toHaveBeenCalledTimes(1);
+    });
+
+    it(`should not be authenticated`, () => {
         auth.logOut();
-    }));
+        app.isAuthenticated$.pipe(
+            first(),
+            tap(result => expect(result).toEqual('Nope'))
+        ).subscribe();
+    });
 
-    it('should render title in a h1 tag', async (() => {
+    it('should render title in a h1 tag', () => {
         const compiled = fixture.debugElement.nativeElement;
         expect(compiled.querySelector('h1').textContent).toContain('Auth testing app');
-    }));
+    });
 });

src/app/app.component.ts

 import {
     Component,
+    Inject,
     OnInit
 } from '@angular/core';
 import {
@@ -25,8 +26,9 @@ import {
 } from 'rxjs/operators';
 
 import {
-    environment
-} from 'src/environments/environment';
+    AAP_CONFIG,
+    AuthConfig
+} from 'src/app/modules/auth/auth.config';
 
 import {
     AuthService,
@@ -63,51 +65,54 @@ export class AppComponent implements OnInit {
     // * test forms
     // * add custom sync validator for username
     createAAP = this._fb.group({
-        name: ['', {
+        name: [null, {
             validators: [Validators.minLength(5), Validators.maxLength(255)]
         }],
-        username: ['', {
+        username: [null, {
             validators: [Validators.required, Validators.minLength(5), Validators.maxLength(255)]
         }],
-        password: ['', {
+        password: [null, {
             validators: [Validators.required, Validators.minLength(5), Validators.maxLength(255)]
         }],
-        email: ['', {
+        email: [null, {
             validators: [Validators.email, Validators.minLength(5), Validators.maxLength(255)]
         }],
-        organization: ['', {
+        organization: [null, {
             validators: [Validators.maxLength(255)]
         }],
     });
 
     loginAAP = this._fb.group({
-        username: ['', {
+        username: [null, {
             validators: [Validators.required, Validators.minLength(5), Validators.maxLength(255)]
         }],
-        password: ['', {
+        password: [null, {
             validators: [Validators.required, Validators.minLength(5), Validators.maxLength(255)]
         }],
     });
 
     changePasswordAAP = this._fb.group({
-        username: ['', {
+        username: [null, {
             validators: [Validators.required, Validators.minLength(5), Validators.maxLength(255)]
         }],
-        oldPassword: ['', {
+        oldPassword: [null, {
             validators: [Validators.required, Validators.minLength(5), Validators.maxLength(255)]
         }],
-        newPassword: ['', {
+        newPassword: [null, {
             validators: [Validators.required, Validators.minLength(5), Validators.maxLength(255)]
         }],
     });
 
     domain = this._fb.group({
-        domainName: ['', Validators.required],
-        domainDesc: ['']
+        domainName: [null, {
+            validators: [Validators.required]
+        }],
+        domainDesc: [null]
     });
 
-    private readonly domainsURL = `${environment.aapURL}/domains`;
-    private readonly authURL = `${environment.aapURL}/auth`;
+    private readonly _domainsURL: string;
+    private readonly _authURL: string;
+    private readonly _managementURL: string;
 
     constructor(
         // Public for demonstration purposes
@@ -115,8 +120,13 @@ export class AppComponent implements OnInit {
         private _tokens: TokenService,
         // private _jwt: JwtHelperService,
         private _fb: FormBuilder,
-        private _http: HttpClient
+        private _http: HttpClient,
+        @Inject(AAP_CONFIG) private _config: AuthConfig
     ) {
+        this._domainsURL = `${_config.aapURL}/domains`;
+        this._authURL = `${_config.aapURL}/auth`;
+        this._managementURL = `${_config.aapURL}/my/management`;
+
         this.user$ = auth.user();
 
         this.isAuthenticated$ = this.user$.pipe(
@@ -124,13 +134,7 @@ export class AppComponent implements OnInit {
         );
 
         this.expiration$ = this.user$.pipe(
-            map(_ => {
-                try {
-                    return _tokens.getTokenExpirationDate();
-                } catch (e) {
-                    return null;
-                }
-            })
+            map(_ => _tokens.getTokenExpirationDate())
         );
 
         /* More complicate version
@@ -139,7 +143,7 @@ export class AppComponent implements OnInit {
                 const token = this._tokens.getToken();
                 try {
                     return _jwt.getTokenExpirationDate( < string > token);
-                } catch (e) {
+                } catch (error) {
                     return null;
                 }
             })
@@ -147,13 +151,7 @@ export class AppComponent implements OnInit {
          */
 
         this.domains$ = this.user$.pipe(
-            map(_ => {
-                try {
-                    return _tokens.getClaim < string[], string[] > ('domains', []);
-                } catch (e) {
-                    return [];
-                }
-            })
+            map(_ => _tokens.getClaim < string[], string[] > ('domains', []))
         );
 
         this.managedDomains$ = this.user$.pipe(
@@ -192,12 +190,7 @@ export class AppComponent implements OnInit {
         this.auth.createAAPaccount(this.createAAP.value).pipe(
             first(),
             filter(Boolean),
-            tap(_ => this.createAAP.reset({
-                name: '',
-                username: '',
-                password: '',
-                organization: ''
-            }))
+            tap(_ => this.createAAP.reset())
         ).subscribe();
     }
 
@@ -208,10 +201,7 @@ export class AppComponent implements OnInit {
         this.auth.loginAAP(this.loginAAP.value).pipe(
             first(),
             filter(Boolean),
-            tap(_ => this.createAAP.reset({
-                name: '',
-                username: '',
-            }))
+            tap(_ => this.loginAAP.reset())
         ).subscribe();
     }
 
@@ -222,11 +212,7 @@ export class AppComponent implements OnInit {
         this.auth.changePasswordAAP(this.changePasswordAAP.value).pipe(
             first(),
             filter(Boolean),
-            tap(_ => this.changePasswordAAP.reset({
-                name: '',
-                oldPassword: '',
-                newPassword: ''
-            }))
+            tap(_ => this.changePasswordAAP.reset())
         ).subscribe();
     }
 
@@ -237,30 +223,14 @@ export class AppComponent implements OnInit {
      * @param description Description of the new domain/group/team
      */
     createDomain(uid: string): void {
-        this._http.post < Domain > (this.domainsURL, this.domain.value, {
-            observe: 'response',
-        }).pipe(
+        this._http.post < Domain > (this._domainsURL, this.domain.value).pipe(
             first(),
-            map(response => {
-                if (response.status === 201 && response.body) {
-                    this.refresh();
-                    return response.body.domainReference;
-                }
-                return null;
-            }),
-            tap(_ => this.domain.reset({
-                domainName: '',
-                domainDesc: ''
-            })),
+            pluck('domainReference'),
             filter(Boolean),
-            concatMap(gid => this._http.put < Domain > (`${this.domainsURL}/${gid}/${uid}/user`, null, {
-                observe: 'response',
-            })),
-            first(),
-            map(response => {
-                this.refresh();
-                return response;
-            }),
+            tap(_ => this.refresh()),
+            tap(_ => this.domain.reset()),
+            concatMap(gid => this._http.put < Domain > (`${this._domainsURL}/${gid}/${uid}/user`, null)),
+            tap(_ => this.refresh())
         ).subscribe();
     }
 
@@ -271,7 +241,7 @@ export class AppComponent implements OnInit {
      */
     deleteDomain(gid: string): void {
         console.log(gid);
-        this._http.delete < Domain > (`${this.domainsURL}/${gid}`, ).pipe(
+        this._http.delete < Domain > (`${this._domainsURL}/${gid}`, ).pipe(
             first(),
             tap(_ => this.refresh())
         ).subscribe();
@@ -281,10 +251,9 @@ export class AppComponent implements OnInit {
      * List domains that the user manage
      */
     listManagedDomains() {
-        return this._http.get < Domain[] > (`${environment.aapURL}/my/management`).pipe(
+        return this._http.get < Domain[] > (`${this._config.aapURL}/my/management`).pipe(
             first(),
             catchError(error => of ([]))
         );
     }
 }
-

src/app/modules/auth/auth.service.ts

 import {
     Injectable,
     Inject,
+    OnDestroy,
     RendererFactory2,
     Renderer2
 } from '@angular/core';
@@ -42,13 +43,16 @@ export interface User {
 }
 
 @Injectable()
-export class AuthService {
+export class AuthService implements OnDestroy {
 
     private _user = new BehaviorSubject < User | null > (null);
 
     private _loginCallbacks: Function[] = [];
     private _logoutCallbacks: Function[] = [];
 
+    private _unlistenLoginMessage: Function;
+    private _unlistenChangesFromOtherWindows: Function;
+
     private _timeoutID: number | null = null;
 
     // Configuration
@@ -84,12 +88,17 @@ export class AuthService {
         }
 
         const renderer = this._rendererFactory.createRenderer(null, null);
-        this._listenLoginMessage(renderer);
-        this._listenChangesFromOtherWindows(renderer);
+        this._unlistenLoginMessage = this._listenLoginMessage(renderer);
+        this._unlistenChangesFromOtherWindows = this._listenChangesFromOtherWindows(renderer);
 
         this._updateUser(); // TODO: experiment with setTimeOut
     }
 
+    public ngOnDestroy() {
+        this._unlistenLoginMessage();
+        this._unlistenChangesFromOtherWindows();
+    }
+
     public user(): Observable < User | null > {
         return this._user.asObservable();
     }
@@ -427,8 +436,8 @@ export class AuthService {
      * These messages contain the tokens from the AAP.
      * If a token is received then the callbacks are triggered.
      */
-    private _listenLoginMessage(renderer: Renderer2) {
-        renderer.listen('window', 'message', (event: MessageEvent) => {
+    private _listenLoginMessage(renderer: Renderer2): Function {
+        return renderer.listen('window', 'message', (event: MessageEvent) => {
             if (!this._messageIsAcceptable(event)) {
                 return;
             }
@@ -456,8 +465,8 @@ export class AuthService {
      * Notice that changes in the '_commKeyName' produced by this class doesn't
      * trigger this event.
      */
-    private _listenChangesFromOtherWindows(renderer: Renderer2) {
-        renderer.listen('window', 'storage', (event: StorageEvent) => {
+    private _listenChangesFromOtherWindows(renderer: Renderer2): Function {
+        return renderer.listen('window', 'storage', (event: StorageEvent) => {
             if (event.key === this._commKeyName) {
                 this._updateUser();
             }

src/app/modules/auth/token.service.spec.ts

@@ -117,7 +117,7 @@ describe('TokenService', () => {
         });
     });
 
-    describe('with malformed token)', () => {
+    describe('with malformed token', () => {
         const malformedToken = 'asdfk.asdf.asdf';
         beforeEach(() => {
             TestBed.configureTestingModule({

src/app/modules/auth/token.service.ts

@@ -26,7 +26,7 @@ export class TokenService {
     public getTokenExpirationDate(): Date | null {
         try {
             return this._jwt.getTokenExpirationDate();
-        } catch (e) {
+        } catch (error) {
             return null;
         }
     }
@@ -54,7 +54,7 @@ export class TokenService {
                 return defaultValue;
             }
             return value;
-        } catch (e) {
+        } catch (error) {
             return defaultValue;
         }
     }