diff --git a/src/app/modules/user/info/info.component.ts b/src/app/modules/user/info/info.component.ts index 61d05ef..e16fa29 100644 --- a/src/app/modules/user/info/info.component.ts +++ b/src/app/modules/user/info/info.component.ts @@ -53,9 +53,9 @@ export class InfoComponent implements OnInit { await this.userService.update$(uid, {chordMode: value}); } - public getUserRoles = (roles: string): roles[] => (roles?.split(';') ?? []) as roles[]; - public transdormUserRoles = (roles: roles): string => - this.getUserRoles(roles) + public getUserRoles = (role: string): roles[] => (role?.split(';') ?? []) as roles[]; + public transdormUserRoles = (role: string): string => + this.getUserRoles(role) .map(_ => new RolePipe().transform(_)) .join(', '); } diff --git a/src/app/modules/user/info/role.pipe.ts b/src/app/modules/user/info/role.pipe.ts index dee5031..e5b860e 100644 --- a/src/app/modules/user/info/role.pipe.ts +++ b/src/app/modules/user/info/role.pipe.ts @@ -3,7 +3,7 @@ import {roles} from '../../../services/user/roles'; @Pipe({name: 'role'}) export class RolePipe implements PipeTransform { - public transform(role: roles): string { + public transform(role: roles | string): string { switch (role) { case 'contributor': return 'Mitarbeiter'; diff --git a/src/app/modules/user/info/users/user/user.component.ts b/src/app/modules/user/info/users/user/user.component.ts index 723adda..d3cbd77 100644 --- a/src/app/modules/user/info/users/user/user.component.ts +++ b/src/app/modules/user/info/users/user/user.component.ts @@ -1,7 +1,7 @@ import {Component, Input, inject} from '@angular/core'; import {User} from '../../../../../services/user/user'; import {UserService} from '../../../../../services/user/user.service'; -import {ROLE_TYPES} from '../../../../../services/user/roles'; +import {ROLE_TYPES, roles} from '../../../../../services/user/roles'; import {faTimes} from '@fortawesome/free-solid-svg-icons'; import {MatFormField, MatLabel} from '@angular/material/form-field'; @@ -24,7 +24,7 @@ export class UserComponent { public id = ''; public name = ''; - public roles: string[] = []; + public roles: roles[] = []; public ROLE_TYPES = ROLE_TYPES; public edit = false; public faClose = faTimes; @@ -36,7 +36,7 @@ export class UserComponent { this.roles = this.getRoleArray(value.role); } - public async onRoleChanged(id: string, roles: string[]): Promise { + public async onRoleChanged(id: string, roles: roles[]): Promise { const role = roles.join(';'); await this.userService.update$(id, {role}); } @@ -46,7 +46,7 @@ export class UserComponent { await this.userService.update$(id, {name: target.value}); } - public getRoleArray(role: string): string[] { - return role ? role.split(';') : []; + public getRoleArray(role: string): roles[] { + return (role ? role.split(';') : []) as roles[]; } } diff --git a/src/app/services/user/user-session.service.spec.ts b/src/app/services/user/user-session.service.spec.ts index d702b58..169c1b3 100644 --- a/src/app/services/user/user-session.service.spec.ts +++ b/src/app/services/user/user-session.service.spec.ts @@ -85,6 +85,7 @@ describe('UserSessionService', () => { const updateSpy = jasmine.createSpy('update').and.resolveTo(); dbServiceSpy.doc.and.returnValue({update: updateSpy} as never); runInFirebaseContextSpy.and.resolveTo({user: {uid: 'user-1'}}); + authStateSubject.next({uid: 'user-1'}); await expectAsync(service.login('mail', 'secret')).toBeResolvedTo('user-1'); @@ -92,6 +93,23 @@ describe('UserSessionService', () => { expect(updateSpy).toHaveBeenCalledWith({songUsage: {}}); }); + it('should wait for auth state propagation before resolving login', async () => { + runInFirebaseContextSpy.and.resolveTo({user: {uid: 'user-1'}}); + + let resolved = false; + const loginPromise = service.login('mail', 'secret').then(result => { + resolved = true; + return result; + }); + + await Promise.resolve(); + expect(resolved).toBeFalse(); + + authStateSubject.next({uid: 'user-1'}); + + await expectAsync(loginPromise).toBeResolvedTo('user-1'); + }); + it('should delegate logout and password reset to AngularFire auth APIs', async () => { runInFirebaseContextSpy.and.resolveTo(); diff --git a/src/app/services/user/user-session.service.ts b/src/app/services/user/user-session.service.ts index fe2f408..f5a350b 100644 --- a/src/app/services/user/user-session.service.ts +++ b/src/app/services/user/user-session.service.ts @@ -2,7 +2,7 @@ import {EnvironmentInjector, Injectable, inject, runInInjectionContext} from '@a import {Auth, authState, createUserWithEmailAndPassword, sendPasswordResetEmail, signInWithEmailAndPassword, signOut} from '@angular/fire/auth'; import {User as AuthUser} from 'firebase/auth'; import {firstValueFrom, Observable, of} from 'rxjs'; -import {map, shareReplay, switchMap} from 'rxjs/operators'; +import {filter, map, shareReplay, switchMap, take} from 'rxjs/operators'; import {DbService} from '../db.service'; import {environment} from '../../../environments/environment'; import {Router} from '@angular/router'; @@ -59,6 +59,7 @@ export class UserSessionService { const dUser = await this.readUser(aUser.user.uid); if (!dUser) return null; await this.initSongUsage(dUser); + await this.awaitAuthenticatedUser(aUser.user.uid); return aUser.user.uid; } @@ -90,6 +91,15 @@ export class UserSessionService { await this.update$(user.id, {songUsage: {}}); } + private async awaitAuthenticatedUser(uid: string): Promise { + await firstValueFrom( + this.user$.pipe( + filter((user): user is User => !!user && user.id === uid), + take(1) + ) + ); + } + private runInFirebaseContext = (factory: () => T): T => runInInjectionContext(this.environmentInjector, factory); private readUser$ = (uid: string) => this.db.doc$('users/' + uid); private readUser = (uid: string): Promise => firstValueFrom(this.readUser$(uid)); diff --git a/src/app/widget-modules/guards/role.guard.spec.ts b/src/app/widget-modules/guards/role.guard.spec.ts index dfe3fca..4baa3a4 100644 --- a/src/app/widget-modules/guards/role.guard.spec.ts +++ b/src/app/widget-modules/guards/role.guard.spec.ts @@ -89,4 +89,42 @@ describe('RoleGuard', () => { done(); }); }); + + it('should redirect members to shows instead of new-user', done => { + TestBed.resetTestingModule(); + routerSpy = jasmine.createSpyObj('Router', ['createUrlTree']); + routerSpy.createUrlTree.and.returnValue({redirect: ['shows']} as never); + TestBed.configureTestingModule({ + providers: [ + {provide: Router, useValue: routerSpy}, + {provide: UserService, useValue: {user$: of({role: 'member'})}}, + ], + }); + guard = TestBed.inject(RoleGuard); + + guard.canActivate({data: {requiredRoles: ['user']}} as never).subscribe(result => { + expect(routerSpy.createUrlTree).toHaveBeenCalledWith(['shows']); + expect(result).toEqual({redirect: ['shows']} as never); + done(); + }); + }); + + it('should choose a matching default route from all assigned roles', done => { + TestBed.resetTestingModule(); + routerSpy = jasmine.createSpyObj('Router', ['createUrlTree']); + routerSpy.createUrlTree.and.callFake(commands => ({redirect: commands}) as never); + TestBed.configureTestingModule({ + providers: [ + {provide: Router, useValue: routerSpy}, + {provide: UserService, useValue: {user$: of({role: ' none ; leader '})}}, + ], + }); + guard = TestBed.inject(RoleGuard); + + guard.canActivate({data: {requiredRoles: ['presenter']}} as never).subscribe(result => { + expect(routerSpy.createUrlTree).toHaveBeenCalledWith(['shows']); + expect(result).toEqual({redirect: ['shows']} as never); + done(); + }); + }); }); diff --git a/src/app/widget-modules/guards/role.guard.ts b/src/app/widget-modules/guards/role.guard.ts index ab18f87..577dad0 100644 --- a/src/app/widget-modules/guards/role.guard.ts +++ b/src/app/widget-modules/guards/role.guard.ts @@ -3,6 +3,7 @@ import {ActivatedRouteSnapshot, Router, UrlTree} from '@angular/router'; import {Observable} from 'rxjs'; import {UserService} from '../../services/user/user.service'; import {map, take} from 'rxjs/operators'; +import {roles} from '../../services/user/roles'; @Injectable({ providedIn: 'root', @@ -23,30 +24,45 @@ export class RoleGuard { if (!user) { return this.router.createUrlTree(['brand', 'new-user']); } - const roles = user.role?.split(';') ?? []; - if (roles.indexOf('admin') !== -1) { + const userRoles = this.parseRoles(user.role); + if (userRoles.includes('admin')) { return true; } - const allowed = roles.some(s => requiredRoles.indexOf(s) !== -1); + const allowed = userRoles.some(role => requiredRoles.includes(role)); - return allowed || this.router.createUrlTree(this.defaultRoute(roles)); + return allowed || this.router.createUrlTree(this.defaultRoute(userRoles)); }) ); } - private defaultRoute(roles: string[]): string[] { - if (!roles || roles.length === 0) { + private defaultRoute(userRoles: roles[]): string[] { + if (userRoles.length === 0) { return ['brand', 'new-user']; } - switch (roles[0]) { - case 'user': - return ['songs']; - case 'presenter': - return ['presentation']; - case 'leader': - return ['shows']; + + if (userRoles.includes('user') || userRoles.includes('contributor')) { + return ['songs']; } - return ['brand', 'new-user']; + if (userRoles.includes('leader') || userRoles.includes('member')) { + return ['shows']; + } + + if (userRoles.includes('presenter')) { + return ['presentation']; + } + + return ['user', 'info']; + } + + private parseRoles(role: string | null | undefined): roles[] { + if (!role) { + return []; + } + + return role + .split(';') + .map(value => value.trim()) + .filter((value): value is roles => value.length > 0 && value !== 'none'); } } diff --git a/src/app/widget-modules/pipes/sort-by/sort-by.pipe.ts b/src/app/widget-modules/pipes/sort-by/sort-by.pipe.ts index e3325d0..cd9d4d0 100644 --- a/src/app/widget-modules/pipes/sort-by/sort-by.pipe.ts +++ b/src/app/widget-modules/pipes/sort-by/sort-by.pipe.ts @@ -4,15 +4,15 @@ import {Pipe, PipeTransform} from '@angular/core'; name: 'sortBy', }) export class SortByPipe implements PipeTransform { - public transform(value: unknown[] | null, order: 'asc' | 'desc' = 'asc', column = ''): unknown[] | null { + public transform(value: T[] | null, order: 'asc' | 'desc' = 'asc', column = ''): T[] | null { if (!value || !order) { return value; } // no array if (!column || column === '') { if (order === 'asc') { - return value.sort(); + return [...value].sort(); } else { - return value.sort().reverse(); + return [...value].sort().reverse(); } } // sort 1d array if (value.length <= 1) { @@ -26,7 +26,7 @@ export class SortByPipe implements PipeTransform { }); } - private getComparableValue(item: unknown, column: string): string | number { + private getComparableValue(item: T, column: string): string | number { const value = (item as Record)[column]; if (value instanceof Date) { return value.getTime();