Fix AuthGuard race conditions and false redirects

Fixes an error where the auth guard had race conditions
with the OpenSlides bootup routine (kinda hacky tbh)
Fixes false redirects in the Auth.Service

Also adjustes the file uploader to be a little more
usable.
This commit is contained in:
Sean Engelhardt 2019-04-23 13:23:18 +02:00
parent bf525cf852
commit 6a04d3ed0b
6 changed files with 78 additions and 7 deletions

View File

@ -2,6 +2,7 @@ import { Injectable } from '@angular/core';
import { CanActivate, ActivatedRouteSnapshot, CanActivateChild, Router } from '@angular/router'; import { CanActivate, ActivatedRouteSnapshot, CanActivateChild, Router } from '@angular/router';
import { OperatorService } from './operator.service'; import { OperatorService } from './operator.service';
import { OpenSlidesService } from './openslides.service';
/** /**
* Classical Auth-Guard. Checks if the user has to correct permissions to enter a page, and forwards to login if not. * Classical Auth-Guard. Checks if the user has to correct permissions to enter a page, and forwards to login if not.
@ -15,8 +16,13 @@ export class AuthGuard implements CanActivate, CanActivateChild {
* *
* @param router To navigate to a target URL * @param router To navigate to a target URL
* @param operator Asking for the required permission * @param operator Asking for the required permission
* @param openSlidesService Handle OpenSlides functions
*/ */
public constructor(private router: Router, private operator: OperatorService) {} public constructor(
private router: Router,
private operator: OperatorService,
private openSlidesService: OpenSlidesService
) {}
/** /**
* Checks of the operator has the required permission to see the state. * Checks of the operator has the required permission to see the state.
@ -44,10 +50,13 @@ export class AuthGuard implements CanActivate, CanActivateChild {
* *
* @param route the route the user wants to navigate to * @param route the route the user wants to navigate to
*/ */
public canActivateChild(route: ActivatedRouteSnapshot): boolean { public async canActivateChild(route: ActivatedRouteSnapshot): Promise<boolean> {
await this.operator.loaded;
if (this.canActivate(route)) { if (this.canActivate(route)) {
return true; return true;
} else { } else {
this.openSlidesService.redirectUrl = location.pathname;
this.router.navigate(['/error'], { this.router.navigate(['/error'], {
queryParams: { queryParams: {
error: 'Authentication Error', error: 'Authentication Error',

View File

@ -62,7 +62,9 @@ export class AuthService {
} }
let redirect = this.OpenSlides.redirectUrl ? this.OpenSlides.redirectUrl : '/'; let redirect = this.OpenSlides.redirectUrl ? this.OpenSlides.redirectUrl : '/';
if (redirect.includes('login')) {
const excludedUrls = ['login'];
if (excludedUrls.some(url => redirect.includes(url))) {
redirect = '/'; redirect = '/';
} }
this.router.navigate([redirect]); this.router.navigate([redirect]);

View File

@ -65,7 +65,9 @@ export class OpenSlidesService {
// start autoupdate if the user is logged in: // start autoupdate if the user is logged in:
const response = await this.operator.whoAmIFromStorage(); const response = await this.operator.whoAmIFromStorage();
if (!response.user && !response.guest_enabled) { if (!response.user && !response.guest_enabled) {
this.redirectUrl = location.pathname; if (!location.pathname.includes('error')) {
this.redirectUrl = location.pathname;
}
this.redirectToLoginIfNotSubpage(); this.redirectToLoginIfNotSubpage();
this.checkOperator(false); this.checkOperator(false);
} else { } else {

View File

@ -6,6 +6,7 @@ import { Group } from 'app/shared/models/users/group';
import { User } from '../../shared/models/users/user'; import { User } from '../../shared/models/users/user';
import { environment } from 'environments/environment'; import { environment } from 'environments/environment';
import { DataStoreService } from './data-store.service'; import { DataStoreService } from './data-store.service';
import { Deferred } from '../deferred';
import { OfflineService } from './offline.service'; import { OfflineService } from './offline.service';
import { OpenSlidesStatusService } from './openslides-status.service'; import { OpenSlidesStatusService } from './openslides-status.service';
import { ViewUser } from 'app/site/users/models/view-user'; import { ViewUser } from 'app/site/users/models/view-user';
@ -116,6 +117,12 @@ export class OperatorService implements OnAfterAppsLoaded {
*/ */
private currentWhoAmI: WhoAmI | null; private currentWhoAmI: WhoAmI | null;
private readonly _loaded: Deferred<void> = new Deferred();
public get loaded(): Promise<void> {
return this._loaded.promise;
}
/** /**
* Sets up an observer for watching changes in the DS. If the operator user or groups are changed, * Sets up an observer for watching changes in the DS. If the operator user or groups are changed,
* the operator's permissions are updated. * the operator's permissions are updated.
@ -177,6 +184,7 @@ export class OperatorService implements OnAfterAppsLoaded {
response = this.getDefaultWhoAmIResponse(); response = this.getDefaultWhoAmIResponse();
} }
await this.updateCurrentWhoAmI(response); await this.updateCurrentWhoAmI(response);
this._loaded.resolve();
return this.currentWhoAmI; return this.currentWhoAmI;
} }

View File

@ -0,0 +1,42 @@
/**
* Helper class to asynchronously wait until certain promises are resolved
*
* @example
* ```ts
* // myService
* private loaded: Deferred<void> = new Deferred();
* // after something was done
* this.loaded.resolve();
*
* // myOtherService or myComponent
* await this.myService.loaded;
* //
* ```
*/
export class Deferred<T> {
/**
* The promise to wait for
*/
public readonly promise: Promise<T>;
/**
* custom resolve function
*/
private _resolve: () => void;
/**
* Creates the promise and overloads the resolve function
*/
public constructor() {
this.promise = new Promise<T>(resolve => {
this.resolve = resolve;
});
}
/**
* Entry point for the resolve function
*/
public resolve(): void {
this._resolve();
}
}

View File

@ -3,7 +3,7 @@
<div class="selection-area"> <div class="selection-area">
<file-drop (onFileDrop)="onDropFile($event)" (click)="fileInput.click()" customstyle="file-drop-style"> <file-drop (onFileDrop)="onDropFile($event)" (click)="fileInput.click()" customstyle="file-drop-style">
<span translate>Drop files into this area OR select files</span> <span translate>Drop files into this area OR click here to select files</span>
</file-drop> </file-drop>
</div> </div>
@ -68,10 +68,18 @@
<!-- Upload and clear buttons --> <!-- Upload and clear buttons -->
<div class="action-buttons"> <div class="action-buttons">
<button type="button" mat-raised-button (click)="onUploadButton()" color="primary"> <button
type="button"
mat-raised-button
(click)="onUploadButton()"
color="primary"
[disabled]="uploadList.data.length === 0"
>
<span translate> Upload </span> <span translate> Upload </span>
</button> </button>
<button type="button" mat-raised-button (click)="onClearButton()"><span translate> Clear list </span></button> <button type="button" mat-raised-button (click)="onClearButton()" [disabled]="uploadList.data.length === 0">
<span translate> Clear list </span>
</button>
</div> </div>
<mat-card class="os-card" *ngIf="showProgress"> <mat-card class="os-card" *ngIf="showProgress">