Merge pull request #4611 from tsiegleauq/error-page-redirect

Fix AuthGuard race conditions and false redirects
This commit is contained in:
Finn Stutzenstein 2019-04-23 13:44:59 +02:00 committed by GitHub
commit d0c6fd1dd1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
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) {
if (!location.pathname.includes('error')) {
this.redirectUrl = location.pathname; 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">