Fix Redirect URL (#6229)

* Fix Redirect URL

* fix unit test

* browser false

* workaround
This commit is contained in:
Eugenio Romano
2020-10-08 16:59:19 +01:00
committed by GitHub
parent d236ed40a2
commit a7ebfa9fba
6 changed files with 57 additions and 50 deletions

View File

@@ -131,7 +131,7 @@ exports.config = {
} }
} }
}, },
args: ['--incognito', args: [
`--window-size=${width},${height}`, `--window-size=${width},${height}`,
'--disable-gpu', '--disable-gpu',
'--no-sandbox', '--no-sandbox',

View File

@@ -20,9 +20,9 @@ import {
CanActivate, CanActivate,
ActivatedRouteSnapshot, ActivatedRouteSnapshot,
RouterStateSnapshot, RouterStateSnapshot,
CanActivateChild CanActivateChild,
UrlTree
} from '@angular/router'; } from '@angular/router';
import { Observable } from 'rxjs';
import { AuthenticationService } from './authentication.service'; import { AuthenticationService } from './authentication.service';
import { import {
AppConfigService, AppConfigService,
@@ -30,12 +30,15 @@ import {
} from '../app-config/app-config.service'; } from '../app-config/app-config.service';
import { OauthConfigModel } from '../models/oauth-config.model'; import { OauthConfigModel } from '../models/oauth-config.model';
import { MatDialog } from '@angular/material/dialog'; import { MatDialog } from '@angular/material/dialog';
import { StorageService } from './storage.service';
import { Observable } from 'rxjs';
export abstract class AuthGuardBase implements CanActivate, CanActivateChild { export abstract class AuthGuardBase implements CanActivate, CanActivateChild {
abstract checkLogin( abstract checkLogin(
activeRoute: ActivatedRouteSnapshot, activeRoute: ActivatedRouteSnapshot,
redirectUrl: string redirectUrl: string
): Observable<boolean> | Promise<boolean> | boolean; ): Observable<boolean | UrlTree> | Promise<boolean | UrlTree> | boolean | UrlTree;
protected get withCredentials(): boolean { protected get withCredentials(): boolean {
return this.appConfigService.get<boolean>( return this.appConfigService.get<boolean>(
@@ -48,13 +51,25 @@ export abstract class AuthGuardBase implements CanActivate, CanActivateChild {
protected authenticationService: AuthenticationService, protected authenticationService: AuthenticationService,
protected router: Router, protected router: Router,
protected appConfigService: AppConfigService, protected appConfigService: AppConfigService,
protected dialog: MatDialog protected dialog: MatDialog,
) {} private storageService: StorageService
) {
}
canActivate( canActivate(
route: ActivatedRouteSnapshot, route: ActivatedRouteSnapshot,
state: RouterStateSnapshot state: RouterStateSnapshot
): Observable<boolean> | Promise<boolean> | boolean { ): Observable<boolean | UrlTree> | Promise<boolean | UrlTree> | boolean | UrlTree {
const redirectFragment = this.storageService.getItem('loginFragment');
if (this.authenticationService.isEcmLoggedIn() || this.withCredentials) {
if (redirectFragment) {
this.storageService.removeItem('loginFragment');
return this.router.createUrlTree([redirectFragment]);
}
return true;
}
const checkLogin = this.checkLogin(route, state.url); const checkLogin = this.checkLogin(route, state.url);
if (!checkLogin) { if (!checkLogin) {
@@ -67,7 +82,7 @@ export abstract class AuthGuardBase implements CanActivate, CanActivateChild {
canActivateChild( canActivateChild(
route: ActivatedRouteSnapshot, route: ActivatedRouteSnapshot,
state: RouterStateSnapshot state: RouterStateSnapshot
): Observable<boolean> | Promise<boolean> | boolean { ): Observable<boolean | UrlTree> | Promise<boolean | UrlTree> | boolean | UrlTree {
return this.canActivate(route, state); return this.canActivate(route, state);
} }

View File

@@ -20,8 +20,8 @@ import { ActivatedRouteSnapshot, Router } from '@angular/router';
import { AppConfigService } from '../app-config/app-config.service'; import { AppConfigService } from '../app-config/app-config.service';
import { AuthenticationService } from './authentication.service'; import { AuthenticationService } from './authentication.service';
import { AuthGuardBase } from './auth-guard-base'; import { AuthGuardBase } from './auth-guard-base';
import { Observable } from 'rxjs';
import { MatDialog } from '@angular/material/dialog'; import { MatDialog } from '@angular/material/dialog';
import { StorageService } from './storage.service';
@Injectable({ @Injectable({
providedIn: 'root' providedIn: 'root'
@@ -31,12 +31,12 @@ export class AuthGuardBpm extends AuthGuardBase {
constructor(authenticationService: AuthenticationService, constructor(authenticationService: AuthenticationService,
router: Router, router: Router,
appConfigService: AppConfigService, appConfigService: AppConfigService,
dialog: MatDialog dialog: MatDialog,
) { storageService: StorageService) {
super(authenticationService, router, appConfigService, dialog); super(authenticationService, router, appConfigService, dialog, storageService);
} }
checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): Observable<boolean> | Promise<boolean> | boolean { checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): boolean {
if (this.authenticationService.isBpmLoggedIn() || this.withCredentials) { if (this.authenticationService.isBpmLoggedIn() || this.withCredentials) {
return true; return true;
} }

View File

@@ -22,7 +22,6 @@ import {
import { AuthenticationService } from './authentication.service'; import { AuthenticationService } from './authentication.service';
import { AppConfigService } from '../app-config/app-config.service'; import { AppConfigService } from '../app-config/app-config.service';
import { AuthGuardBase } from './auth-guard-base'; import { AuthGuardBase } from './auth-guard-base';
import { Observable } from 'rxjs';
import { MatDialog } from '@angular/material/dialog'; import { MatDialog } from '@angular/material/dialog';
import { StorageService } from './storage.service'; import { StorageService } from './storage.service';
@@ -35,19 +34,11 @@ export class AuthGuardEcm extends AuthGuardBase {
router: Router, router: Router,
appConfigService: AppConfigService, appConfigService: AppConfigService,
dialog: MatDialog, dialog: MatDialog,
private storageService: StorageService) { storageService: StorageService) {
super(authenticationService, router, appConfigService, dialog); super(authenticationService, router, appConfigService, dialog, storageService);
} }
checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): Observable<boolean> | Promise<boolean> | boolean { checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): boolean {
const redirectFragment = this.storageService.getItem('loginFragment');
if (this.authenticationService.isEcmLoggedIn() || this.withCredentials) {
if (redirectFragment) {
this.router.navigateByUrl(redirectFragment);
this.storageService.removeItem('loginFragment');
}
return true;
}
this.redirectToUrl('ECM', redirectUrl); this.redirectToUrl('ECM', redirectUrl);
if (!this.authenticationService.isEcmLoggedIn() && this.isSilentLogin() && !this.authenticationService.isPublicUrl()) { if (!this.authenticationService.isEcmLoggedIn() && this.isSilentLogin() && !this.authenticationService.isPublicUrl()) {
this.authenticationService.ssoImplicitLogin(); this.authenticationService.ssoImplicitLogin();

View File

@@ -50,70 +50,70 @@ describe('AuthGuardService', () => {
appConfigService.config.oauth2 = {}; appConfigService.config.oauth2 = {};
}); });
it('if the alfresco js api is logged in should canActivate be true', async(() => { it('if the alfresco js api is logged in should canActivate be true', async(async () => {
spyOn(router, 'navigateByUrl'); spyOn(router, 'navigateByUrl');
spyOn(authService, 'isLoggedIn').and.returnValue(true); spyOn(authService, 'isLoggedIn').and.returnValue(true);
expect(authGuard.canActivate(null, state)).toBeTruthy(); expect(await authGuard.canActivate(null, state)).toBeTruthy();
expect(router.navigateByUrl).not.toHaveBeenCalled(); expect(router.navigateByUrl).not.toHaveBeenCalled();
})); }));
it('if the alfresco js api is NOT logged in should canActivate be false', async(() => { it('if the alfresco js api is NOT logged in should canActivate be false', async(async () => {
state.url = 'some-url'; state.url = 'some-url';
spyOn(router, 'navigateByUrl'); spyOn(router, 'navigateByUrl');
spyOn(authService, 'isLoggedIn').and.returnValue(false); spyOn(authService, 'isLoggedIn').and.returnValue(false);
expect(authGuard.canActivate(null, state)).toBeFalsy(); expect(await authGuard.canActivate(null, state)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalled(); expect(router.navigateByUrl).toHaveBeenCalled();
})); }));
it('if the alfresco js api is configured with withCredentials true should canActivate be true', async(() => { it('if the alfresco js api is configured with withCredentials true should canActivate be true', async(async () => {
spyOn(authService, 'isBpmLoggedIn').and.returnValue(true); spyOn(authService, 'isBpmLoggedIn').and.returnValue(true);
appConfigService.config.auth.withCredentials = true; appConfigService.config.auth.withCredentials = true;
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' }; const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };
expect(authGuard.canActivate(null, route)).toBeTruthy(); expect(await authGuard.canActivate(null, route)).toBeTruthy();
})); }));
it('should redirect url if the User is NOT logged in and isOAuthWithoutSilentLogin', async(() => { it('should redirect url if the User is NOT logged in and isOAuthWithoutSilentLogin', async(async () => {
spyOn(router, 'navigateByUrl').and.stub(); spyOn(router, 'navigateByUrl').and.stub();
spyOn(authService, 'isLoggedIn').and.returnValue(false); spyOn(authService, 'isLoggedIn').and.returnValue(false);
spyOn(authService, 'isOauth').and.returnValue(true); spyOn(authService, 'isOauth').and.returnValue(true);
appConfigService.config.oauth2.silentLogin = false; appConfigService.config.oauth2.silentLogin = false;
expect(authGuard.canActivate(null, state)).toBeFalsy(); expect(await authGuard.canActivate(null, state)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalled(); expect(router.navigateByUrl).toHaveBeenCalled();
})); }));
it('should redirect url if the User is NOT logged in and isOAuth but no silentLogin configured', async(() => { it('should redirect url if the User is NOT logged in and isOAuth but no silentLogin configured', async(async () => {
spyOn(router, 'navigateByUrl').and.stub(); spyOn(router, 'navigateByUrl').and.stub();
spyOn(authService, 'isLoggedIn').and.returnValue(false); spyOn(authService, 'isLoggedIn').and.returnValue(false);
spyOn(authService, 'isOauth').and.returnValue(true); spyOn(authService, 'isOauth').and.returnValue(true);
appConfigService.config.oauth2.silentLogin = undefined; appConfigService.config.oauth2.silentLogin = undefined;
expect(authGuard.canActivate(null, state)).toBeFalsy(); expect(await authGuard.canActivate(null, state)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalled(); expect(router.navigateByUrl).toHaveBeenCalled();
})); }));
it('should NOT redirect url if the User is NOT logged in and isOAuth but with silentLogin configured', async(() => { it('should NOT redirect url if the User is NOT logged in and isOAuth but with silentLogin configured', async(async () => {
spyOn(router, 'navigateByUrl').and.stub(); spyOn(router, 'navigateByUrl').and.stub();
spyOn(authService, 'isLoggedIn').and.returnValue(false); spyOn(authService, 'isLoggedIn').and.returnValue(false);
spyOn(authService, 'isOauth').and.returnValue(true); spyOn(authService, 'isOauth').and.returnValue(true);
appConfigService.config.oauth2.silentLogin = true; appConfigService.config.oauth2.silentLogin = true;
expect(authGuard.canActivate(null, state)).toBeFalsy(); expect(await authGuard.canActivate(null, state)).toBeFalsy();
expect(router.navigateByUrl).not.toHaveBeenCalled(); expect(router.navigateByUrl).not.toHaveBeenCalled();
})); }));
it('should set redirect url', async(() => { it('should set redirect url', async(async () => {
state.url = 'some-url'; state.url = 'some-url';
appConfigService.config.loginRoute = 'login'; appConfigService.config.loginRoute = 'login';
spyOn(router, 'navigateByUrl'); spyOn(router, 'navigateByUrl');
spyOn(authService, 'setRedirect'); spyOn(authService, 'setRedirect');
authGuard.canActivate(null, state); await authGuard.canActivate(null, state);
expect(authService.setRedirect).toHaveBeenCalledWith({ expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ALL', url: 'some-url' provider: 'ALL', url: 'some-url'
@@ -121,14 +121,14 @@ describe('AuthGuardService', () => {
expect(router.navigateByUrl).toHaveBeenCalledWith('/login?redirectUrl=some-url'); expect(router.navigateByUrl).toHaveBeenCalledWith('/login?redirectUrl=some-url');
})); }));
it('should set redirect url with query params', async(() => { it('should set redirect url with query params', async(async () => {
state.url = 'some-url;q=query'; state.url = 'some-url;q=query';
appConfigService.config.loginRoute = 'login'; appConfigService.config.loginRoute = 'login';
spyOn(router, 'navigateByUrl'); spyOn(router, 'navigateByUrl');
spyOn(authService, 'setRedirect'); spyOn(authService, 'setRedirect');
authGuard.canActivate(null, state); await authGuard.canActivate(null, state);
expect(authService.setRedirect).toHaveBeenCalledWith({ expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ALL', url: 'some-url;q=query' provider: 'ALL', url: 'some-url;q=query'
@@ -136,14 +136,14 @@ describe('AuthGuardService', () => {
expect(router.navigateByUrl).toHaveBeenCalledWith('/login?redirectUrl=some-url;q=query'); expect(router.navigateByUrl).toHaveBeenCalledWith('/login?redirectUrl=some-url;q=query');
})); }));
it('should get redirect url from config if there is one configured', async(() => { it('should get redirect url from config if there is one configured', async(async () => {
state.url = 'some-url'; state.url = 'some-url';
appConfigService.config.loginRoute = 'fakeLoginRoute'; appConfigService.config.loginRoute = 'fakeLoginRoute';
spyOn(router, 'navigateByUrl'); spyOn(router, 'navigateByUrl');
spyOn(authService, 'setRedirect'); spyOn(authService, 'setRedirect');
authGuard.canActivate(null, state); await authGuard.canActivate(null, state);
expect(authService.setRedirect).toHaveBeenCalledWith({ expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ALL', url: 'some-url' provider: 'ALL', url: 'some-url'
@@ -151,13 +151,13 @@ describe('AuthGuardService', () => {
expect(router.navigateByUrl).toHaveBeenCalledWith('/fakeLoginRoute?redirectUrl=some-url'); expect(router.navigateByUrl).toHaveBeenCalledWith('/fakeLoginRoute?redirectUrl=some-url');
})); }));
it('should pass actual redirect when no state segments exists', async(() => { it('should pass actual redirect when no state segments exists', async(async () => {
state.url = '/'; state.url = '/';
spyOn(router, 'navigateByUrl'); spyOn(router, 'navigateByUrl');
spyOn(authService, 'setRedirect'); spyOn(authService, 'setRedirect');
authGuard.canActivate(null, state); await authGuard.canActivate(null, state);
expect(authService.setRedirect).toHaveBeenCalledWith({ expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ALL', url: '/' provider: 'ALL', url: '/'

View File

@@ -18,11 +18,11 @@
import { Injectable } from '@angular/core'; import { Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, Router } from '@angular/router'; import { ActivatedRouteSnapshot, Router } from '@angular/router';
import { AuthenticationService } from './authentication.service'; import { AuthenticationService } from './authentication.service';
import { Observable } from 'rxjs';
import { AppConfigService } from '../app-config/app-config.service'; import { AppConfigService } from '../app-config/app-config.service';
import { AuthGuardBase } from './auth-guard-base'; import { AuthGuardBase } from './auth-guard-base';
import { JwtHelperService } from './jwt-helper.service'; import { JwtHelperService } from './jwt-helper.service';
import { MatDialog } from '@angular/material/dialog'; import { MatDialog } from '@angular/material/dialog';
import { StorageService } from './storage.service';
@Injectable({ @Injectable({
providedIn: 'root' providedIn: 'root'
@@ -35,8 +35,9 @@ export class AuthGuard extends AuthGuardBase {
authenticationService: AuthenticationService, authenticationService: AuthenticationService,
router: Router, router: Router,
appConfigService: AppConfigService, appConfigService: AppConfigService,
dialog: MatDialog) { dialog: MatDialog,
super(authenticationService, router, appConfigService, dialog); storageService: StorageService) {
super(authenticationService, router, appConfigService, dialog, storageService);
this.ticketChangeBind = this.ticketChange.bind(this); this.ticketChangeBind = this.ticketChange.bind(this);
window.addEventListener('storage', this.ticketChangeBind); window.addEventListener('storage', this.ticketChangeBind);
@@ -68,7 +69,7 @@ export class AuthGuard extends AuthGuardBase {
window.removeEventListener('storage', this.ticketChangeBind); window.removeEventListener('storage', this.ticketChangeBind);
} }
checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): Observable<boolean> | Promise<boolean> | boolean { async checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): Promise<boolean> {
if (this.authenticationService.isLoggedIn() || this.withCredentials) { if (this.authenticationService.isLoggedIn() || this.withCredentials) {
return true; return true;
} }