From ad769db4accd30c954a8f5429a917defd4d46912 Mon Sep 17 00:00:00 2001 From: tsvetkovv <11594423+Tsvetkovv@users.noreply.github.com> Date: Sat, 2 May 2020 16:13:24 +0400 Subject: [PATCH 1/3] refactor(select-union): extract to separate component --- src/_constants/unions.ts | 16 ++++ src/_helpers/login.guard.ts | 30 ++++++ src/_models/index.ts | 1 - src/_services/alert.service.ts | 2 +- src/_services/authentication.service.ts | 72 +++++++++----- src/_services/browser-storage.service.spec.ts | 12 +++ src/_services/browser-storage.service.ts | 29 ++++++ src/_services/index.ts | 4 - src/_services/token.service.ts | 31 +++++- src/_services/user.service.ts | 22 ++++- src/app/app-routing.module.ts | 12 ++- src/app/app.module.ts | 24 ++--- src/environments/environment.ts | 2 +- src/login/index.ts | 1 - src/login/login.component.html | 85 +++++++---------- src/login/login.component.ts | 94 +++++-------------- src/register/index.ts | 1 - src/register/register.component.html | 76 +++++++-------- src/register/register.component.ts | 40 ++++---- src/select-union/select-union.component.html | 13 +++ src/select-union/select-union.component.scss | 0 .../select-union.component.spec.ts | 25 +++++ src/select-union/select-union.component.ts | 26 +++++ 23 files changed, 385 insertions(+), 233 deletions(-) create mode 100644 src/_constants/unions.ts create mode 100644 src/_helpers/login.guard.ts delete mode 100644 src/_models/index.ts create mode 100644 src/_services/browser-storage.service.spec.ts create mode 100644 src/_services/browser-storage.service.ts delete mode 100644 src/_services/index.ts delete mode 100644 src/login/index.ts delete mode 100644 src/register/index.ts create mode 100644 src/select-union/select-union.component.html create mode 100644 src/select-union/select-union.component.scss create mode 100644 src/select-union/select-union.component.spec.ts create mode 100644 src/select-union/select-union.component.ts diff --git a/src/_constants/unions.ts b/src/_constants/unions.ts new file mode 100644 index 0000000..c242b6b --- /dev/null +++ b/src/_constants/unions.ts @@ -0,0 +1,16 @@ +import {InjectionToken} from '@angular/core'; + +export const unions: UnionMap = { + mydata: {key: 'mydata', name: 'My Data', url: 'https://mydata.webtree.org/applyToken'}, + imprint: {key: 'imprint', name: 'Imprint', url: 'https://imprint.webtree.org/applyToken'} +}; + +export interface Union { + key: string; + name: string; + url: string; +} + +export type UnionMap = Record; + +export const UNIONS_TOKEN = new InjectionToken('unions'); diff --git a/src/_helpers/login.guard.ts b/src/_helpers/login.guard.ts new file mode 100644 index 0000000..fdc9426 --- /dev/null +++ b/src/_helpers/login.guard.ts @@ -0,0 +1,30 @@ +import {Injectable} from '@angular/core'; +import {ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot} from '@angular/router'; +import {Observable} from 'rxjs'; +import {map} from 'rxjs/operators'; +import {AuthenticationService} from '../_services/authentication.service'; +import {TokenService} from '../_services/token.service'; + +@Injectable({providedIn: 'root'}) +export class LoginGuard implements CanActivate { + constructor( + private tokenService: TokenService, + private authenticationService: AuthenticationService, + ) { + } + + canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable | boolean { + if (!this.tokenService.tokenExists()) { + return true; + } + + return this.tokenService.isTokenValid().pipe( + map(isValid => { + if (isValid) { + return !this.authenticationService.redirectToUnionIfNeeded(route); + } + return true; + }) + ); + } +} diff --git a/src/_models/index.ts b/src/_models/index.ts deleted file mode 100644 index f6b9f36..0000000 --- a/src/_models/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './User'; diff --git a/src/_services/alert.service.ts b/src/_services/alert.service.ts index 34fbe57..20335b1 100644 --- a/src/_services/alert.service.ts +++ b/src/_services/alert.service.ts @@ -1,7 +1,7 @@ import {Injectable} from '@angular/core'; import {MatSnackBar} from '@angular/material'; -@Injectable() +@Injectable({ providedIn: 'root'}) export class AlertService { constructor(private snackBar: MatSnackBar) { diff --git a/src/_services/authentication.service.ts b/src/_services/authentication.service.ts index 5c84b0d..bfde780 100644 --- a/src/_services/authentication.service.ts +++ b/src/_services/authentication.service.ts @@ -1,39 +1,67 @@ -import {Injectable} from '@angular/core'; -import 'rxjs/add/operator/map'; +import {HttpClient, HttpErrorResponse} from '@angular/common/http'; +import {Inject, Injectable} from '@angular/core'; +import {Observable, of} from 'rxjs'; +import {catchError, map} from 'rxjs/operators'; import {TokenService} from './token.service'; -import {HttpClient} from '@angular/common/http'; import {environment} from '../environments/environment'; -import {User} from '../_models'; +import {User} from '../_models/User'; +import {AlertService} from './alert.service'; +import {ActivatedRouteSnapshot} from '@angular/router'; +import {Union, UnionMap, UNIONS_TOKEN} from '../_constants/unions'; +import {DOCUMENT} from '@angular/common'; -@Injectable() +@Injectable({providedIn: 'root'}) export class AuthenticationService { constructor(private http: HttpClient, - private tokenService: TokenService) { + private alertService: AlertService, + private tokenService: TokenService, + @Inject(UNIONS_TOKEN) private unions: UnionMap, + @Inject(DOCUMENT) private document: Document, + ) { } - login(user: User) { - return this.http.post(environment.backendUrl + 'token/new', user, {responseType: 'text'}); + login(user: User): Observable { + return this.http.post<{ token: string }>(environment.backendUrl + 'token/new', user) + .pipe( + map(res => { + if ('token' in res) { + return res.token; + } + return null; + }), + catchError((error: HttpErrorResponse) => { + console.log(error); + if (error.status === 401) { + this.alertService.error(error.error); + return of(null); + } + }) + ); } logout() { this.tokenService.removeToken(); } - async isAuthorized(): Promise { - if (!this.tokenService.tokenExists()) { - return Promise.resolve(this.tokenService.tokenExists()); - } + redirectToUnionIfNeeded(route: ActivatedRouteSnapshot): boolean { + const returnUnion = route.queryParamMap.get('returnUnion'); - return this.http.post(environment.backendUrl + 'checkToken', this.tokenService.getToken()) - .toPromise() - .then(() => { + if (typeof returnUnion === 'string') { + if (returnUnion in this.unions) { + this.redirectToUnion(this.unions[returnUnion]); return true; - }).catch((err) => { - if (err.status !== 401) { - console.error(err); - } - this.tokenService.removeToken(); - return false; - }); + } else { + this.alertService.error(`Unknown union: ${returnUnion}`); + } + } + + return false; + } + + redirectToUnion({url}: Union): void { + const token = this.tokenService.getToken(); + const tokenizedUrl = `${url}#token=${token}`; + + this.document.location.href = tokenizedUrl; } } diff --git a/src/_services/browser-storage.service.spec.ts b/src/_services/browser-storage.service.spec.ts new file mode 100644 index 0000000..0757236 --- /dev/null +++ b/src/_services/browser-storage.service.spec.ts @@ -0,0 +1,12 @@ +import {TestBed} from '@angular/core/testing'; + +import {BrowserStorageService} from './browser-storage.service'; + +describe('BrowserStorageServiceService', () => { + beforeEach(() => TestBed.configureTestingModule({})); + + it('should be created', () => { + const service: BrowserStorageService = TestBed.get(BrowserStorageService); + expect(service).toBeTruthy(); + }); +}); diff --git a/src/_services/browser-storage.service.ts b/src/_services/browser-storage.service.ts new file mode 100644 index 0000000..8549b4c --- /dev/null +++ b/src/_services/browser-storage.service.ts @@ -0,0 +1,29 @@ +import { Inject, Injectable, InjectionToken } from '@angular/core'; + +export const BROWSER_STORAGE = new InjectionToken('Browser Storage', { + providedIn: 'root', + factory: () => localStorage +}); + +@Injectable({ + providedIn: 'root' +}) +export class BrowserStorageService { + constructor(@Inject(BROWSER_STORAGE) public storage: Storage) {} + + get(key: string) { + return this.storage.getItem(key); + } + + set(key: string, value: string) { + return this.storage.setItem(key, value); + } + + remove(key: string) { + return this.storage.removeItem(key); + } + + clear() { + this.storage.clear(); + } +} diff --git a/src/_services/index.ts b/src/_services/index.ts deleted file mode 100644 index c0fd27c..0000000 --- a/src/_services/index.ts +++ /dev/null @@ -1,4 +0,0 @@ -export * from './authentication.service'; -export * from './alert.service'; -export * from './user.service'; -export * from './token.service'; diff --git a/src/_services/token.service.ts b/src/_services/token.service.ts index b4cdfc9..323de1b 100644 --- a/src/_services/token.service.ts +++ b/src/_services/token.service.ts @@ -1,22 +1,45 @@ import {Injectable} from '@angular/core'; +import {Observable, of} from 'rxjs'; +import {environment} from '../environments/environment'; +import {catchError, mapTo} from 'rxjs/operators'; +import {HttpClient, HttpErrorResponse} from '@angular/common/http'; +import {BrowserStorageService} from './browser-storage.service'; -@Injectable() +@Injectable({providedIn: 'root'}) export class TokenService { private tokenName = 'token'; + constructor(private http: HttpClient, + private storage: BrowserStorageService) { + } + tokenExists(): boolean { return !!this.getToken(); } getToken(): string { - return localStorage.getItem(this.tokenName); + return this.storage.get(this.tokenName); } saveToken(token: string) { - localStorage.setItem(this.tokenName, token); + this.storage.set(this.tokenName, token); } removeToken(): void { - localStorage.removeItem('token'); + this.storage.remove('token'); + } + + isTokenValid(): Observable { + return this.http.post(environment.backendUrl + 'checkToken', this.getToken()) + .pipe( + mapTo(true), + catchError((err: HttpErrorResponse) => { + if (err.status !== 401) { + console.error(err); + } + this.removeToken(); + return of(false); + }) + ); } } diff --git a/src/_services/user.service.ts b/src/_services/user.service.ts index 7a14038..e2c9b52 100644 --- a/src/_services/user.service.ts +++ b/src/_services/user.service.ts @@ -1,17 +1,29 @@ import {Injectable} from '@angular/core'; -import {User} from '../_models'; -import {HttpClient} from '@angular/common/http'; +import {User} from '../_models/User'; +import {HttpClient, HttpErrorResponse} from '@angular/common/http'; import {Observable} from 'rxjs/Observable'; import {environment} from '../environments/environment'; +import {catchError} from 'rxjs/operators'; +import {of, throwError} from 'rxjs'; -@Injectable() +@Injectable({providedIn: 'root'}) export class UserService { constructor(private http: HttpClient) { } - create(user: User): Observable { + create(user: User): Observable { const url = environment.backendUrl + 'user/register'; - return this.http.post(url, user); + + return this.http.post(url, user).pipe( + catchError((error: HttpErrorResponse) => { + console.error(error); + if (error.status === 400) { + return of(null); + } else { + return throwError(error); + } + }) + ); } } diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index 75afbbf..621e3c5 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -1,11 +1,14 @@ import {NgModule} from '@angular/core'; import {RouterModule, Routes} from '@angular/router'; -import {RegisterComponent} from '../register'; -import {LoginComponent} from '../login'; +import {RegisterComponent} from '../register/register.component'; +import {LoginComponent} from '../login/login.component'; +import {SelectUnionComponent} from '../select-union/select-union.component'; +import {LoginGuard} from '../_helpers/login.guard'; const routes: Routes = [ {path: 'register', component: RegisterComponent}, - {path: 'login', component: LoginComponent}, + {path: 'login', component: LoginComponent, canActivate: [LoginGuard]}, + {path: 'select-union', component: SelectUnionComponent}, {path: '**', redirectTo: 'login'} ]; @@ -13,4 +16,5 @@ const routes: Routes = [ imports: [RouterModule.forRoot(routes)], exports: [RouterModule] }) -export class AppRoutingModule { } +export class AppRoutingModule { +} diff --git a/src/app/app.module.ts b/src/app/app.module.ts index 12d4c0d..45608b9 100644 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -1,22 +1,23 @@ import {BrowserModule} from '@angular/platform-browser'; import {NgModule} from '@angular/core'; +import {HttpClientModule} from '@angular/common/http'; +import {FormsModule, ReactiveFormsModule} from '@angular/forms'; +import {BrowserAnimationsModule} from '@angular/platform-browser/animations'; import {AppRoutingModule} from './app-routing.module'; import {AppComponent} from './app.component'; -import {FormsModule, ReactiveFormsModule} from '@angular/forms'; -import {RegisterComponent} from '../register'; -import {AlertService, AuthenticationService, TokenService, UserService} from '../_services'; -import {HttpClientModule} from '@angular/common/http'; -import {Subject} from 'rxjs'; -import {LoginComponent} from '../login'; -import {BrowserAnimationsModule} from '@angular/platform-browser/animations'; +import {RegisterComponent} from '../register/register.component'; +import {LoginComponent} from '../login/login.component'; import {MaterialModule} from './material.module'; +import {unions, UNIONS_TOKEN} from '../_constants/unions'; +import {SelectUnionComponent} from '../select-union/select-union.component'; @NgModule({ declarations: [ AppComponent, LoginComponent, RegisterComponent, + SelectUnionComponent, ], imports: [ BrowserModule, @@ -28,11 +29,10 @@ import {MaterialModule} from './material.module'; MaterialModule ], providers: [ - UserService, - AlertService, - AuthenticationService, - TokenService, - Subject + { + provide: UNIONS_TOKEN, + useValue: unions + } ], bootstrap: [AppComponent] }) diff --git a/src/environments/environment.ts b/src/environments/environment.ts index 7e02ec5..d03135a 100644 --- a/src/environments/environment.ts +++ b/src/environments/environment.ts @@ -4,7 +4,7 @@ export const environment = { production: false, - backendUrl: 'http://localhost:9000/rest/', + backendUrl: 'https://auth-api.webtree.org/rest/', }; /* diff --git a/src/login/index.ts b/src/login/index.ts deleted file mode 100644 index 69c1644..0000000 --- a/src/login/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './login.component'; diff --git a/src/login/login.component.html b/src/login/login.component.html index 39ceb92..5c58924 100644 --- a/src/login/login.component.html +++ b/src/login/login.component.html @@ -1,52 +1,39 @@ -
-
-

Login

-
-
- - - -
Username is required
-
-
- - - -
Password is required
-
-
- - - Register -
-
-
-
+

Login

+
+

- Select Union to login - - - {{union.name}} - - + + + Username is required + - - -

-
+

+

+ + + + Password is required + + + +

+

+ + + Loading + + Register +

+ diff --git a/src/login/login.component.ts b/src/login/login.component.ts index 7a5d4a3..d0a363b 100644 --- a/src/login/login.component.ts +++ b/src/login/login.component.ts @@ -1,92 +1,50 @@ -import {Component, ElementRef, OnInit, ViewChild} from '@angular/core'; +import {Component} from '@angular/core'; import {ActivatedRoute, Router} from '@angular/router'; -import {AlertService, AuthenticationService, TokenService} from '../_services'; -import {User} from '../_models'; +import {User} from '../_models/User'; import {sha512} from 'js-sha512'; -import {FormBuilder} from '@angular/forms'; +import {FormGroup, FormControl, Validators} from '@angular/forms'; +import {AlertService} from '../_services/alert.service'; +import {AuthenticationService} from '../_services/authentication.service'; +import {TokenService} from '../_services/token.service'; @Component({ selector: 'app-login', templateUrl: './login.component.html', styleUrls: ['./login.component.css'] }) -export class LoginComponent implements OnInit { - - objectValues = Object.values; - model: User = {}; +export class LoginComponent { + form: FormGroup = new FormGroup({ + username: new FormControl( + null, [Validators.required]), + password: new FormControl( + null, [Validators.required]) + }); loading = false; - returnUnion: string; - unions = { - mydata: {key: 'mydata', name: 'My Data', url: 'https://mydata.webtree.org/applyToken'}, - imprint: {key: 'imprint', name: 'Imprint', url: 'https://imprint.webtree.org/applyToken'}, - unions: {key: 'unions', name: 'Unions', url: 'https://unions.webtree.org/!/applyToken'} - }; - loggedIn = false; - - @ViewChild('redirectForm', {read: ElementRef, static: true}) redirectForm: ElementRef; constructor(private route: ActivatedRoute, private router: Router, - private formBuilder: FormBuilder, private authenticationService: AuthenticationService, private tokenService: TokenService, private alertService: AlertService) { } - async ngOnInit() { - this.returnUnion = this.route.snapshot.queryParams.returnUnion; - await this.updateIsLoginField(); - this.redirectIfNeeded(); - } - - login() { + onSubmit({username, password}) { this.loading = true; - const user: User = {username: this.model.username, password: sha512(this.model.password)}; - this.authenticationService.login(user) - .subscribe( - async res => { - this.tokenService.saveToken(JSON.parse(res).token); - this.alertService.success('Logged in successfully'); - if (!this.redirectIfNeeded()) { - await this.updateIsLoginField(); - this.loading = false; - } - }, - error => { + const user: User = {username, password: sha512(password)}; + return this.authenticationService.login(user) + .subscribe(token => { this.loading = false; - console.log(error); - if (error.status === 401) { - this.alertService.error(error.error); - } else { - throw error; + if (token === null) { + this.alertService.error('Invalid username or password'); + return; + } + + this.tokenService.saveToken(token); + this.alertService.success('Logged in successfully'); + if (!this.authenticationService.redirectToUnionIfNeeded(this.route.snapshot)) { + this.router.navigate(['/select-union']); } } ); } - - private async updateIsLoginField() { - this.loggedIn = await this.authenticationService.isAuthorized(); - } - - getToken(): string { - return this.tokenService.getToken(); - } - - redirectIfNeeded() { - if (this.tokenService.tokenExists() && !!this.returnUnion) { - if (this.unions[this.returnUnion]) { - window.location.href = `${this.unions[this.returnUnion].url}#token=${this.tokenService.getToken()}`; - } else { - this.alertService.error('Unknown union ' + this.returnUnion); - } - return true; - } else { - return false; - } - } - - logout() { - this.authenticationService.logout(); - this.loggedIn = false; - } } diff --git a/src/register/index.ts b/src/register/index.ts deleted file mode 100644 index 55388b6..0000000 --- a/src/register/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './register.component'; diff --git a/src/register/register.component.html b/src/register/register.component.html index 4aae697..e4b781d 100644 --- a/src/register/register.component.html +++ b/src/register/register.component.html @@ -1,37 +1,39 @@ -
-

Register

-
-
- - - -
Username is required
-
-
- - - -
Password is required
-
-
- - - Login -
-
-
+

Login

+
+

+ + + + Username is required + + ` + +

+

+ + + + Password is required + + + +

+

+ + + Loading + + Login +

+
diff --git a/src/register/register.component.ts b/src/register/register.component.ts index 20acad2..2af5188 100644 --- a/src/register/register.component.ts +++ b/src/register/register.component.ts @@ -1,46 +1,40 @@ -import {Component, OnInit} from '@angular/core'; +import {Component} from '@angular/core'; import {AlertService} from '../_services/alert.service'; import {UserService} from '../_services/user.service'; -import {User} from '../_models'; +import {User} from '../_models/User'; import {sha512} from 'js-sha512'; +import {FormControl, FormGroup, Validators} from '@angular/forms'; @Component({ selector: 'app-register', templateUrl: './register.component.html', styleUrls: ['./register.component.css'] }) -export class RegisterComponent implements OnInit { - model: User = {}; - +export class RegisterComponent { + form: FormGroup = new FormGroup({ + username: new FormControl( + null, [Validators.required]), + password: new FormControl( + null, [Validators.required]) + }); loading = false; - constructor( - // private router: Router, - private userService: UserService, + constructor(private userService: UserService, private alertService: AlertService) { } - register() { + onSubmit({username, password}) { this.loading = true; - const user: User = {username: this.model.username, password: sha512(this.model.password)}; + const user: User = {username, password: sha512(password)}; this.userService.create(user) .subscribe( data => { - this.alertService.success('Registration successful'); - // this.router.navigate(['/login']); - }, - error => { - this.loading = false; - console.log(error); - if (error.status === 400) { - this.alertService.error(error.error); + if (data === null) { + this.alertService.error('Registration unsuccessful'); } else { - throw error; + this.alertService.success('Registration successful'); } + // this.router.navigate(['/login']); }); } - - ngOnInit(): void { - } - } diff --git a/src/select-union/select-union.component.html b/src/select-union/select-union.component.html new file mode 100644 index 0000000..300d815 --- /dev/null +++ b/src/select-union/select-union.component.html @@ -0,0 +1,13 @@ + + Select Union to login + + + {{union.name}} + + + + + diff --git a/src/select-union/select-union.component.scss b/src/select-union/select-union.component.scss new file mode 100644 index 0000000..e69de29 diff --git a/src/select-union/select-union.component.spec.ts b/src/select-union/select-union.component.spec.ts new file mode 100644 index 0000000..7dbb9de --- /dev/null +++ b/src/select-union/select-union.component.spec.ts @@ -0,0 +1,25 @@ +import { async, ComponentFixture, TestBed } from '@angular/core/testing'; + +import { SelectUnionComponent } from './select-union.component'; + +describe('SelectUnionComponent', () => { + let component: SelectUnionComponent; + let fixture: ComponentFixture; + + beforeEach(async(() => { + TestBed.configureTestingModule({ + declarations: [ SelectUnionComponent ] + }) + .compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(SelectUnionComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); +}); diff --git a/src/select-union/select-union.component.ts b/src/select-union/select-union.component.ts new file mode 100644 index 0000000..3b0ff95 --- /dev/null +++ b/src/select-union/select-union.component.ts @@ -0,0 +1,26 @@ +import {Component, Inject} from '@angular/core'; +import {AuthenticationService} from '../_services/authentication.service'; +import {Union, UnionMap, UNIONS_TOKEN} from '../_constants/unions'; + +@Component({ + selector: 'app-select-union', + templateUrl: './select-union.component.html', + styleUrls: ['./select-union.component.scss'] +}) +export class SelectUnionComponent { + public unions: Union[] = Object.values(this.unionsMap); + + constructor( + private authenticationService: AuthenticationService, + @Inject(UNIONS_TOKEN) private unionsMap: UnionMap, + ) { + } + + logout() { + this.authenticationService.logout(); + } + + redirectToUnion(unionKey: string) { + this.authenticationService.redirectToUnion(this.unionsMap[unionKey]); + } +} From 253126c16e74b6c82b8b54859962fbaea5d646a1 Mon Sep 17 00:00:00 2001 From: Viktor Tsvetkov <11594423+Tsvetkovv@users.noreply.github.com> Date: Sun, 3 Jan 2021 13:15:13 +0100 Subject: [PATCH 2/3] fix PR issues --- src/_constants/unions.ts | 3 +- src/_helpers/login.guard.ts | 9 ++--- src/_models/Login.ts | 3 ++ src/_services/authentication.service.ts | 45 ++++++++++++++----------- src/_services/user.service.ts | 13 +++---- src/login/login.component.ts | 23 +++++++------ src/register/register.component.html | 2 +- src/register/register.component.ts | 20 ++++++----- 8 files changed, 66 insertions(+), 52 deletions(-) create mode 100644 src/_models/Login.ts diff --git a/src/_constants/unions.ts b/src/_constants/unions.ts index c242b6b..c65ab65 100644 --- a/src/_constants/unions.ts +++ b/src/_constants/unions.ts @@ -2,7 +2,8 @@ import {InjectionToken} from '@angular/core'; export const unions: UnionMap = { mydata: {key: 'mydata', name: 'My Data', url: 'https://mydata.webtree.org/applyToken'}, - imprint: {key: 'imprint', name: 'Imprint', url: 'https://imprint.webtree.org/applyToken'} + imprint: {key: 'imprint', name: 'Imprint', url: 'https://imprint.webtree.org/applyToken'}, + unions: {key: 'unions', name: 'Unions', url: 'https://unions.webtree.org/!/applyToken'} }; export interface Union { diff --git a/src/_helpers/login.guard.ts b/src/_helpers/login.guard.ts index fdc9426..26c21e0 100644 --- a/src/_helpers/login.guard.ts +++ b/src/_helpers/login.guard.ts @@ -1,5 +1,5 @@ import {Injectable} from '@angular/core'; -import {ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot} from '@angular/router'; +import {ActivatedRouteSnapshot, CanActivate} from '@angular/router'; import {Observable} from 'rxjs'; import {map} from 'rxjs/operators'; import {AuthenticationService} from '../_services/authentication.service'; @@ -13,15 +13,16 @@ export class LoginGuard implements CanActivate { ) { } - canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable | boolean { + canActivate(route: ActivatedRouteSnapshot): Observable | boolean { if (!this.tokenService.tokenExists()) { return true; } return this.tokenService.isTokenValid().pipe( map(isValid => { - if (isValid) { - return !this.authenticationService.redirectToUnionIfNeeded(route); + if (isValid && this.authenticationService.shouldRedirect(route.queryParamMap)) { + this.authenticationService.redirect(route.queryParamMap); + return true; } return true; }) diff --git a/src/_models/Login.ts b/src/_models/Login.ts new file mode 100644 index 0000000..7e2852b --- /dev/null +++ b/src/_models/Login.ts @@ -0,0 +1,3 @@ +interface Login { + token: string; +} diff --git a/src/_services/authentication.service.ts b/src/_services/authentication.service.ts index bfde780..1e29f4b 100644 --- a/src/_services/authentication.service.ts +++ b/src/_services/authentication.service.ts @@ -1,40 +1,40 @@ import {HttpClient, HttpErrorResponse} from '@angular/common/http'; import {Inject, Injectable} from '@angular/core'; -import {Observable, of} from 'rxjs'; +import {Observable, throwError} from 'rxjs'; import {catchError, map} from 'rxjs/operators'; import {TokenService} from './token.service'; import {environment} from '../environments/environment'; import {User} from '../_models/User'; -import {AlertService} from './alert.service'; -import {ActivatedRouteSnapshot} from '@angular/router'; +import {ParamMap} from '@angular/router'; import {Union, UnionMap, UNIONS_TOKEN} from '../_constants/unions'; import {DOCUMENT} from '@angular/common'; @Injectable({providedIn: 'root'}) export class AuthenticationService { constructor(private http: HttpClient, - private alertService: AlertService, private tokenService: TokenService, @Inject(UNIONS_TOKEN) private unions: UnionMap, @Inject(DOCUMENT) private document: Document, ) { } - login(user: User): Observable { + login(user: User): Observable { return this.http.post<{ token: string }>(environment.backendUrl + 'token/new', user) .pipe( map(res => { if ('token' in res) { - return res.token; + return { token: res.token }; } - return null; + throw new Error(`Valid token is not returned. Got: ${res}`); }), - catchError((error: HttpErrorResponse) => { - console.log(error); - if (error.status === 401) { - this.alertService.error(error.error); - return of(null); + catchError((error: Error | HttpErrorResponse) => { + if ('status' in error) { + if (error.status === 401) { + return throwError('Invalid username or password'); + } + return throwError(error.message || error.error); } + return throwError(error); }) ); } @@ -43,25 +43,32 @@ export class AuthenticationService { this.tokenService.removeToken(); } - redirectToUnionIfNeeded(route: ActivatedRouteSnapshot): boolean { - const returnUnion = route.queryParamMap.get('returnUnion'); + shouldRedirect(queryParamMap: ParamMap): boolean { + const returnUnion = this.getReturnUnion(queryParamMap); if (typeof returnUnion === 'string') { if (returnUnion in this.unions) { - this.redirectToUnion(this.unions[returnUnion]); return true; } else { - this.alertService.error(`Unknown union: ${returnUnion}`); + throw new Error(`Unknown union: ${returnUnion}`); } } return false; } - redirectToUnion({url}: Union): void { + redirect(queryParamMap: ParamMap): void { + const returnUnion = this.getReturnUnion(queryParamMap); + this.redirectToUnion(this.unions[returnUnion]); + } + + redirectToUnion({ url }: Union): void { const token = this.tokenService.getToken(); - const tokenizedUrl = `${url}#token=${token}`; - this.document.location.href = tokenizedUrl; + this.document.location.href = `${url}#token=${token}`; + } + + getReturnUnion(queryParamMap: ParamMap): string | null { + return queryParamMap.get('returnUnion'); } } diff --git a/src/_services/user.service.ts b/src/_services/user.service.ts index e2c9b52..9f160de 100644 --- a/src/_services/user.service.ts +++ b/src/_services/user.service.ts @@ -5,24 +5,19 @@ import {HttpClient, HttpErrorResponse} from '@angular/common/http'; import {Observable} from 'rxjs/Observable'; import {environment} from '../environments/environment'; import {catchError} from 'rxjs/operators'; -import {of, throwError} from 'rxjs'; +import {throwError} from 'rxjs'; @Injectable({providedIn: 'root'}) export class UserService { constructor(private http: HttpClient) { } - create(user: User): Observable { + create(user: User): Observable { const url = environment.backendUrl + 'user/register'; return this.http.post(url, user).pipe( - catchError((error: HttpErrorResponse) => { - console.error(error); - if (error.status === 400) { - return of(null); - } else { - return throwError(error); - } + catchError(({message}: HttpErrorResponse) => { + return throwError(`${message}`); }) ); } diff --git a/src/login/login.component.ts b/src/login/login.component.ts index d0a363b..b693f03 100644 --- a/src/login/login.component.ts +++ b/src/login/login.component.ts @@ -6,6 +6,7 @@ import {FormGroup, FormControl, Validators} from '@angular/forms'; import {AlertService} from '../_services/alert.service'; import {AuthenticationService} from '../_services/authentication.service'; import {TokenService} from '../_services/token.service'; +import {finalize} from 'rxjs/operators'; @Component({ selector: 'app-login', @@ -32,19 +33,21 @@ export class LoginComponent { this.loading = true; const user: User = {username, password: sha512(password)}; return this.authenticationService.login(user) - .subscribe(token => { - this.loading = false; - if (token === null) { - this.alertService.error('Invalid username or password'); - return; - } - + .pipe(finalize(() => { + this.loading = false; + })) + .subscribe(({ token }) => { this.tokenService.saveToken(token); this.alertService.success('Logged in successfully'); - if (!this.authenticationService.redirectToUnionIfNeeded(this.route.snapshot)) { + + const queryParamMap = this.route.snapshot.queryParamMap; + if (this.authenticationService.shouldRedirect(queryParamMap)) { + this.authenticationService.redirect(queryParamMap); + } else { this.router.navigate(['/select-union']); } - } - ); + }, (error: string) => { + this.alertService.error(error); + }); } } diff --git a/src/register/register.component.html b/src/register/register.component.html index e4b781d..d49a853 100644 --- a/src/register/register.component.html +++ b/src/register/register.component.html @@ -1,4 +1,4 @@ -

Login

+

Register

diff --git a/src/register/register.component.ts b/src/register/register.component.ts index 2af5188..c42850e 100644 --- a/src/register/register.component.ts +++ b/src/register/register.component.ts @@ -4,6 +4,8 @@ import {UserService} from '../_services/user.service'; import {User} from '../_models/User'; import {sha512} from 'js-sha512'; import {FormControl, FormGroup, Validators} from '@angular/forms'; +import {finalize} from 'rxjs/operators'; +import {Router} from '@angular/router'; @Component({ selector: 'app-register', @@ -20,21 +22,23 @@ export class RegisterComponent { loading = false; constructor(private userService: UserService, - private alertService: AlertService) { + private alertService: AlertService, + private router: Router) { } onSubmit({username, password}) { this.loading = true; const user: User = {username, password: sha512(password)}; this.userService.create(user) + .pipe(finalize(() => { + this.loading = false; + })) .subscribe( - data => { - if (data === null) { - this.alertService.error('Registration unsuccessful'); - } else { - this.alertService.success('Registration successful'); - } - // this.router.navigate(['/login']); + () => { + this.alertService.success('Registration successful'); + this.router.navigate(['/login']); + }, (error: string) => { + this.alertService.error(error); }); } } From 74f89789fe432cf7df7db2a16d5666299f0052e3 Mon Sep 17 00:00:00 2001 From: tsvetkovv <11594423+Tsvetkovv@users.noreply.github.com> Date: Sun, 3 Jan 2021 16:18:26 +0100 Subject: [PATCH 3/3] refactor(select-union): add draft test --- src/_constants/__mock__/unions.ts | 6 ++ src/_helpers/login.guard.spec.ts | 73 ++++++++++++++++ src/_services/authentication.service.spec.ts | 88 +++++++++++++------- src/_services/user.service.spec.ts | 2 +- src/login/login.component.spec.ts | 2 +- src/register/register.component.spec.ts | 2 +- tslint.json | 5 +- 7 files changed, 146 insertions(+), 32 deletions(-) create mode 100644 src/_constants/__mock__/unions.ts create mode 100644 src/_helpers/login.guard.spec.ts diff --git a/src/_constants/__mock__/unions.ts b/src/_constants/__mock__/unions.ts new file mode 100644 index 0000000..9d44b17 --- /dev/null +++ b/src/_constants/__mock__/unions.ts @@ -0,0 +1,6 @@ +import {UnionMap} from '../unions'; + +export const unions: UnionMap = { + key1: {key: 'key1', name: 'name1', url: 'http://url1'}, + key2: {key: 'key2', name: 'name2', url: 'http://url2'}, +}; diff --git a/src/_helpers/login.guard.spec.ts b/src/_helpers/login.guard.spec.ts new file mode 100644 index 0000000..417634f --- /dev/null +++ b/src/_helpers/login.guard.spec.ts @@ -0,0 +1,73 @@ +import {TestBed} from '@angular/core/testing'; +import {LoginGuard} from './login.guard'; + +import {TokenService} from '../_services/token.service'; +import {AuthenticationService} from '../_services/authentication.service'; +import {ActivatedRouteSnapshot} from '@angular/router'; +import {isObservable, of} from 'rxjs'; + +jest.mock('../_services/token.service'); +jest.mock('../_services/authentication.service'); + +describe('Login Guard', () => { + let loginGuard: LoginGuard; + let tokenService: jest.Mocked; + let authService: jest.Mocked; + + const routeStub = {} as ActivatedRouteSnapshot; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [ + LoginGuard, + TokenService, + AuthenticationService + ] + }); + loginGuard = TestBed.get(LoginGuard); + tokenService = TestBed.get(TokenService); + authService = TestBed.get(AuthenticationService); + }); + + it('should allow if token is not exist', () => { + tokenService.tokenExists.mockImplementation(() => false); + + expect(loginGuard.canActivate(routeStub)).toBe(true); + }); + + it('should allow if token is not valid', (done) => { + tokenService.tokenExists.mockImplementation(() => true); + tokenService.isTokenValid.mockImplementation(() => of(false)); + + const res = loginGuard.canActivate(routeStub); + expect(isObservable(res)).toBeTruthy(); + // always true. Workaround for typescript instead of 'as Observable' + if (isObservable(res)) { + res.subscribe(canActivate => { + expect(canActivate).toBeTruthy(); + done(); + }); + } + }); + + it.each` + requiredRedirect | canActivate + ${true} | ${false} + ${false} | ${true} + `('canActivate = $s if token is valid and requiredRedirect = $requiredRedirect', + ({requiredRedirect, canActivate}) => { + tokenService.tokenExists.mockImplementation(() => true); + tokenService.isTokenValid.mockImplementation(() => of(true)); + authService.redirectToUnionIfNeeded.mockImplementation(() => requiredRedirect); + + const res = loginGuard.canActivate(routeStub); + expect(isObservable(res)).toBeTruthy(); + // always true. Workaround for typescript instead of 'as Observable' + if (isObservable(res)) { + res.subscribe(canActivateRes => { + expect(canActivateRes).toBe(canActivate); + }); + } + }); + +}); diff --git a/src/_services/authentication.service.spec.ts b/src/_services/authentication.service.spec.ts index 5d309df..6ee7779 100644 --- a/src/_services/authentication.service.spec.ts +++ b/src/_services/authentication.service.spec.ts @@ -1,46 +1,78 @@ import {TestBed} from '@angular/core/testing'; -import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing'; +import {HttpClientTestingModule, HttpTestingController, TestRequest} from '@angular/common/http/testing'; +import {DOCUMENT} from '@angular/common'; import {AuthenticationService} from './authentication.service'; -import {User} from '../_models'; +import {User} from '../_models/User'; + import {TokenService} from './token.service'; import {environment} from '../environments/environment'; +import {AlertService} from './alert.service'; +import {UNIONS_TOKEN} from '../_constants/unions'; +import {unions as unionsStub} from '../_constants/__mock__/unions'; jest.mock('./token.service'); jest.mock('../environments/environment'); +jest.mock('./alert.service'); -describe('AuthenticationService', () => { +const documentStub = { + location: { + set href(value) { + } + }, +}; +describe('AuthenticationService', () => { let authService: AuthenticationService; let httpMock: HttpTestingController; let tokenService: jest.Mocked; + let alertService: jest.Mocked; + const locationHrefSetterSpy = jest.spyOn(documentStub.location, 'href', 'set'); beforeEach(() => { + TestBed.configureTestingModule({ - providers: [TokenService, AuthenticationService], + providers: [ + AuthenticationService, + AlertService, + TokenService, + {provide: UNIONS_TOKEN, useValue: unionsStub}, + {provide: DOCUMENT, useValue: documentStub}, + ], imports: [HttpClientTestingModule] }); httpMock = TestBed.get(HttpTestingController); authService = TestBed.get(AuthenticationService); tokenService = TestBed.get(TokenService); - + alertService = TestBed.get(AlertService); }); - it('#login should sent request with correct data', (done) => { + describe('#login', () => { const user: User = {username: 'testUser', password: 'testPassword'}; + let req: TestRequest; + let responseBody; + beforeEach(() => { + responseBody = null; + }); - authService.login(user).subscribe((res) => { - expect(res).toEqual('someResponse'); - done(); + it('should return token received from server', (done) => { + responseBody = {token: 'testToken'}; + authService.login(user).subscribe((res) => { + expect(res).toEqual(responseBody.token); + done(); + }); + req.flush(responseBody); }); - const req = httpMock.expectOne('http://localhost:9000/rest/token/new'); - expect(req.request.method).toEqual('POST'); - expect(req.request.body).toEqual(user); - req.flush('someResponse'); - httpMock.verify(); + afterEach(() => { + // expect(req.request.method).toEqual('POST'); + // expect(req.request.body).toEqual(user); + + httpMock.verify(); + }); }); + it('#logout should call tokenService.removeToken()', () => { authService.logout(); expect(tokenService.removeToken).toHaveBeenCalled(); @@ -53,23 +85,23 @@ describe('AuthenticationService', () => { tokenService.tokenExists.mockImplementation(() => true); tokenService.getToken.mockImplementation(() => 'testToken'); - authService.isAuthorized().then(value => { - expect(value).toBeTruthy(); - done(); - }); + // authService.isAuthorized().then(value => { + // expect(value).toBeTruthy(); + // done(); + // }); verifyHttpCall(); }); it('should return false then token does not exist', async (done) => { - authService.isAuthorized().then(value => { - expect(value).toBeFalsy(); - done(); - }); + // authService.isAuthorized().then(value => { + // expect(value).toBeFalsy(); + // done(); + // }); }); it('should not call backend if token not exists', () => { - authService.isAuthorized().then(); + // authService.isAuthorized().then(); httpMock.expectNone(environment.backendUrl + 'checkToken'); }); @@ -77,10 +109,10 @@ describe('AuthenticationService', () => { tokenService.tokenExists.mockImplementation(() => true); tokenService.getToken.mockImplementation(() => 'wrongToken'); - authService.isAuthorized().then(value => { - expect(value).toBeFalsy(); - done(); - }); + // authService.isAuthorized().then(value => { + // expect(value).toBeFalsy(); + // done(); + // }); verifyHttpCall({status: 401, statusText: 'Unauthorized'}); }); @@ -89,7 +121,7 @@ describe('AuthenticationService', () => { tokenService.tokenExists.mockImplementation(() => true); tokenService.getToken.mockImplementation(() => token); - authService.isAuthorized().then(); + // authService.isAuthorized().then(); const req = verifyHttpCall(); expect(req.request.method).toEqual('POST'); diff --git a/src/_services/user.service.spec.ts b/src/_services/user.service.spec.ts index 378194a..463da2a 100644 --- a/src/_services/user.service.spec.ts +++ b/src/_services/user.service.spec.ts @@ -2,7 +2,7 @@ import {UserService} from './user.service'; import {TestBed} from '@angular/core/testing'; import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing'; -import {User} from '../_models'; +import {User} from '../_models/User'; describe('UserService', () => { diff --git a/src/login/login.component.spec.ts b/src/login/login.component.spec.ts index 3a10612..3a6afdd 100644 --- a/src/login/login.component.spec.ts +++ b/src/login/login.component.spec.ts @@ -5,7 +5,7 @@ import {DebugElement} from '@angular/core'; import {of} from 'rxjs'; import {LoginComponent} from './login.component'; -import {User} from '../_models'; +import {User} from '../_models/User'; import {AppModuleMock} from '../app/app.module.mock'; import {AlertService} from '../_services/alert.service'; diff --git a/src/register/register.component.spec.ts b/src/register/register.component.spec.ts index 4de0113..b2d0529 100644 --- a/src/register/register.component.spec.ts +++ b/src/register/register.component.spec.ts @@ -4,7 +4,7 @@ import {By} from '@angular/platform-browser'; import {DebugElement} from '@angular/core'; import {RegisterComponent} from './register.component'; -import {User} from '../_models'; +import {User} from '../_models/User'; import {AppModuleMock} from '../app/app.module.mock'; import {AlertService} from '../_services/alert.service'; diff --git a/tslint.json b/tslint.json index add926c..c1b2be7 100644 --- a/tslint.json +++ b/tslint.json @@ -17,7 +17,10 @@ "max-classes-per-file": false, "max-line-length": [ true, - 140 + { + "limit": 140, + "ignore-pattern": "^import |^export {(.*?)}" + } ], "member-access": false, "member-ordering": [