Skip to content

Commit

Permalink
[MNT-21636] Use URLTree for redirect (#6691)
Browse files Browse the repository at this point in the history 
* use URLTree for redirect

* use always urltree

* fix e2e

* fix
  • Loading branch information
@eromano
eromano authored Feb 18, 2021
1 parent 81f9f48 commit 4a381b4
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 83 deletions.
8 changes: 4 additions & 4 deletions e2e/content-services/permissions/site-permissions.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,20 @@ import CONSTANTS = require('../../util/constants');
describe('Permissions Component', () => {

const apiService = new ApiService();
const uploadActions = new UploadActions(apiService);

const loginPage = new LoginPage();
const contentServicesPage = new ContentServicesPage();
const permissionsPage = new PermissionsPage();
const uploadActions = new UploadActions(apiService);

const contentList = contentServicesPage.getDocumentList();

const viewerPage = new ViewerPage();
const navigationBarPage = new NavigationBarPage();
const metadataViewPage = new MetadataViewPage();
const notificationHistoryPage = new NotificationHistoryPage();
const uploadDialog = new UploadDialogPage();
const versionManagePage = new VersionManagePage();

const contentList = contentServicesPage.getDocumentList();

let publicSite, privateSite, folderName;

const fileModel = new FileModel({
Expand Down
10 changes: 5 additions & 5 deletions lib/core/services/auth-guard-base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ export abstract class AuthGuardBase implements CanActivate, CanActivateChild {
return this.canActivate(route, state);
}

protected async redirectSSOSuccessURL(): Promise<boolean> {
protected async redirectSSOSuccessURL(): Promise<boolean | UrlTree> {
const redirectFragment = this.storageService.getItem('loginFragment');

if (redirectFragment && this.getLoginRoute() !== redirectFragment) {
this.storageService.removeItem('loginFragment');
return this.navigate(redirectFragment);
return this.router.parseUrl(redirectFragment);
}

return true;
Expand All @@ -90,7 +90,7 @@ export abstract class AuthGuardBase implements CanActivate, CanActivateChild {
return !!this.storageService.getItem('loginFragment');
}

protected async redirectToUrl(url: string): Promise<boolean> {
protected async redirectToUrl(url: string): Promise<boolean | UrlTree> {
let urlToRedirect = `/${this.getLoginRoute()}`;

if (!this.authenticationService.isOauth()) {
Expand All @@ -110,9 +110,9 @@ export abstract class AuthGuardBase implements CanActivate, CanActivateChild {
return false;
}

protected navigate(url: string): Promise<boolean> {
protected navigate(url: string): UrlTree {
this.dialog.closeAll();
return this.router.navigateByUrl(url);
return this.router.parseUrl(url);
}

protected getOauthConfig(): OauthConfigModel {
Expand Down
24 changes: 8 additions & 16 deletions lib/core/services/auth-guard-bpm.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('AuthGuardService BPM', () => {
spyOn(router, 'navigateByUrl').and.stub();
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

expect(await authGuard.canActivate(null, route)).toBeFalsy();
expect(await authGuard.canActivate(null, route)).toEqual(router.parseUrl('/login?redirectUrl=some-url'));
}));

it('if the alfresco js api is NOT logged in should trigger a redirect event', async(async () => {
Expand All @@ -105,8 +105,7 @@ describe('AuthGuardService BPM', () => {
spyOn(authService, 'isBpmLoggedIn').and.returnValue(false);
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

expect(await authGuard.canActivate(null, route)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalledWith('/login?redirectUrl=some-url');
expect(await authGuard.canActivate(null, route)).toEqual(router.parseUrl('/login?redirectUrl=some-url'));
}));

it('should redirect url if the alfresco js api is NOT logged in and isOAuthWithoutSilentLogin', async(async () => {
Expand All @@ -116,22 +115,20 @@ describe('AuthGuardService BPM', () => {
appConfigService.config.oauth2.silentLogin = false;
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

expect(await authGuard.canActivate(null, route)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalled();
expect(await authGuard.canActivate(null, route)).toEqual(router.parseUrl('/login'));
}));

it('should redirect url if NOT logged in and isOAuth but no silentLogin configured', async(async () => {
it('should redirect to login url if NOT you are not logged in and silentLogin is false', async(async () => {
spyOn(router, 'navigateByUrl').and.stub();
spyOn(authService, 'isBpmLoggedIn').and.returnValue(false);
spyOn(authService, 'isOauth').and.returnValue(true);
appConfigService.config.oauth2.silentLogin = undefined;
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

expect(await authGuard.canActivate(null, route)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalled();
expect(await authGuard.canActivate(null, route)).toEqual(router.parseUrl('/login'));
}));

it('should set redirect url', async(() => {
it('should set redirect url', async(async () => {
spyOn(authService, 'setRedirect').and.callThrough();
spyOn(router, 'navigateByUrl').and.stub();
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };
Expand Down Expand Up @@ -170,18 +167,13 @@ describe('AuthGuardService BPM', () => {
expect(authService.getRedirect()).toEqual('/');
}));

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 () => {
appConfigService.config.loginRoute = 'fakeLoginRoute';
spyOn(authService, 'setRedirect').and.callThrough();
spyOn(router, 'navigateByUrl').and.stub();
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

authGuard.canActivate(null, route);

expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'BPM', url: 'some-url'
});
expect(router.navigateByUrl).toHaveBeenCalledWith('/fakeLoginRoute?redirectUrl=some-url');
expect(await authGuard.canActivate(null, route)).toEqual(router.parseUrl('/fakeLoginRoute?redirectUrl=some-url'));
}));

it('should to close the material dialog if is redirect to the login', () => {
Expand Down
4 changes: 2 additions & 2 deletions lib/core/services/auth-guard-bpm.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/

import { Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, Router } from '@angular/router';
import { ActivatedRouteSnapshot, Router, UrlTree } from '@angular/router';
import { AppConfigService } from '../app-config/app-config.service';
import { AuthenticationService } from './authentication.service';
import { AuthGuardBase } from './auth-guard-base';
Expand All @@ -36,7 +36,7 @@ export class AuthGuardBpm extends AuthGuardBase {
super(authenticationService, router, appConfigService, dialog, storageService);
}

async checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): Promise<boolean> {
async checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): Promise<boolean | UrlTree> {
if (this.authenticationService.isBpmLoggedIn() || this.withCredentials) {
return true;
}
Expand Down
46 changes: 19 additions & 27 deletions lib/core/services/auth-guard-ecm.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,53 +51,51 @@ describe('AuthGuardService ECM', () => {
appConfigService.config.oauth2 = {};
});

it('if the alfresco js api is logged in should canActivate be true', async(async() => {
it('if the alfresco js api is logged in should canActivate be true', async(async () => {
spyOn(authService, 'isEcmLoggedIn').and.returnValue(true);
const route: RouterStateSnapshot = <RouterStateSnapshot> {url : 'some-url'};
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

expect(await authGuard.canActivate(null, route)).toBeTruthy();
}));

it('if the alfresco js api is configured with withCredentials true should canActivate be true', async(async() => {
it('if the alfresco js api is configured with withCredentials true should canActivate be true', async(async () => {
spyOn(authService, 'isBpmLoggedIn').and.returnValue(true);
appConfigService.config.auth.withCredentials = true;

const route: RouterStateSnapshot = <RouterStateSnapshot> {url : 'some-url'};
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

expect(await authGuard.canActivate(null, route)).toBeTruthy();
}));

it('if the alfresco js api is NOT logged in should canActivate be false', async(async() => {
it('if the alfresco js api is NOT logged in should canActivate be false', async(async () => {
spyOn(authService, 'isEcmLoggedIn').and.returnValue(false);
spyOn(router, 'navigateByUrl').and.stub();
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

expect(await authGuard.canActivate(null, route)).toBeFalsy();
expect(await authGuard.canActivate(null, route)).toEqual(router.parseUrl('/login?redirectUrl=some-url'));
}));

it('if the alfresco js api is NOT logged in should trigger a redirect event', async(async() => {
it('if the alfresco js api is NOT logged in should trigger a redirect event', async(async () => {
appConfigService.config.loginRoute = 'login';

spyOn(router, 'navigateByUrl');
spyOn(authService, 'isEcmLoggedIn').and.returnValue(false);
const route: RouterStateSnapshot = <RouterStateSnapshot> {url : 'some-url'};
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

expect(await authGuard.canActivate(null, route)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalledWith('/login?redirectUrl=some-url');
expect(await authGuard.canActivate(null, route)).toEqual(router.parseUrl('/login?redirectUrl=some-url'));
}));

it('should redirect url if the alfresco js api is NOT logged in and isOAuthWithoutSilentLogin', async(async() => {
it('should redirect url if the alfresco js api is NOT logged in and isOAuthWithoutSilentLogin', async(async () => {
spyOn(router, 'navigateByUrl').and.stub();
spyOn(authService, 'isEcmLoggedIn').and.returnValue(false);
spyOn(authService, 'isOauth').and.returnValue(true);
appConfigService.config.oauth2.silentLogin = false;
const route: RouterStateSnapshot = <RouterStateSnapshot> {url : 'some-url'};
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

expect(await authGuard.canActivate(null, route)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalled();
expect(await authGuard.canActivate(null, route)).toEqual(router.parseUrl('/login'));
}));

it('should redirect url if the alfresco js api is NOT logged in and isOAuth with silentLogin', async(async() => {
it('should redirect url if the alfresco js api is NOT logged in and isOAuth with silentLogin', async(async () => {
spyOn(authService, 'isEcmLoggedIn').and.returnValue(false);
spyOn(authService, 'isOauth').and.returnValue(true);
spyOn(authService, 'isPublicUrl').and.returnValue(false);
Expand All @@ -112,21 +110,20 @@ describe('AuthGuardService ECM', () => {
scope: 'openid'
};

const route: RouterStateSnapshot = <RouterStateSnapshot> {url : 'abc'};
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'abc' };

expect(await authGuard.canActivate(null, route)).toBeFalsy();
expect(authService.ssoImplicitLogin).toHaveBeenCalledTimes(1);
}));

it('should not redirect url if NOT logged in and isOAuth but no silentLogin configured', async(async() => {
it('should not redirect url if NOT logged in and isOAuth but no silentLogin configured', async(async () => {
spyOn(router, 'navigateByUrl').and.stub();
spyOn(authService, 'isEcmLoggedIn').and.returnValue(false);
spyOn(authService, 'isOauth').and.returnValue(true);
appConfigService.config.oauth2.silentLogin = undefined;
const route: RouterStateSnapshot = <RouterStateSnapshot> {url : 'some-url'};
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

expect(await authGuard.canActivate(null, route)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalled();
expect(await authGuard.canActivate(null, route)).toEqual(router.parseUrl('/login'));
}));

it('should set redirect navigation commands', async(() => {
Expand Down Expand Up @@ -168,18 +165,13 @@ describe('AuthGuardService ECM', () => {
expect(authService.getRedirect()).toEqual('/');
}));

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 () => {
appConfigService.config.loginRoute = 'fakeLoginRoute';
spyOn(authService, 'setRedirect').and.callThrough();
spyOn(router, 'navigateByUrl').and.stub();
const route: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };

authGuard.canActivate(null, route);

expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ECM', url: 'some-url'
});
expect(router.navigateByUrl).toHaveBeenCalledWith('/fakeLoginRoute?redirectUrl=some-url');
expect(await authGuard.canActivate(null, route)).toEqual(router.parseUrl('/fakeLoginRoute?redirectUrl=some-url'));
}));

it('should to close the material dialog if is redirect to the login', () => {
Expand Down
4 changes: 2 additions & 2 deletions lib/core/services/auth-guard-ecm.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import { Injectable } from '@angular/core';
import {
ActivatedRouteSnapshot, Router
ActivatedRouteSnapshot, Router, UrlTree
} from '@angular/router';
import { AuthenticationService } from './authentication.service';
import { AppConfigService } from '../app-config/app-config.service';
Expand All @@ -38,7 +38,7 @@ export class AuthGuardEcm extends AuthGuardBase {
super(authenticationService, router, appConfigService, dialog, storageService);
}

async checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): Promise<boolean> {
async checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): Promise<boolean | UrlTree> {
if (this.authenticationService.isEcmLoggedIn() || this.withCredentials) {
return true;
}
Expand Down
30 changes: 6 additions & 24 deletions lib/core/services/auth-guard.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ describe('AuthGuardService', () => {
spyOn(router, 'navigateByUrl');
spyOn(authService, 'isLoggedIn').and.returnValue(false);

expect(await authGuard.canActivate(null, state)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalled();
expect(await authGuard.canActivate(null, state)).toEqual(router.parseUrl('/login?redirectUrl=some-url'));
}));

it('if the alfresco js api is configured with withCredentials true should canActivate be true', async(async () => {
Expand Down Expand Up @@ -97,8 +96,7 @@ describe('AuthGuardService', () => {
spyOn(authService, 'isOauth').and.returnValue(true);
appConfigService.config.oauth2.silentLogin = false;

expect(await authGuard.canActivate(null, state)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalled();
expect(await authGuard.canActivate(null, state)).toEqual(router.parseUrl('/login'));
}));

it('should redirect url if the User is NOT logged in and isOAuth but no silentLogin configured', async(async () => {
Expand All @@ -107,8 +105,7 @@ describe('AuthGuardService', () => {
spyOn(authService, 'isOauth').and.returnValue(true);
appConfigService.config.oauth2.silentLogin = undefined;

expect(await authGuard.canActivate(null, state)).toBeFalsy();
expect(router.navigateByUrl).toHaveBeenCalled();
expect(await authGuard.canActivate(null, state)).toEqual(router.parseUrl('/login'));
}));

it('should NOT redirect url if the User is NOT logged in and isOAuth but with silentLogin configured', async(async () => {
Expand All @@ -128,12 +125,7 @@ describe('AuthGuardService', () => {
spyOn(router, 'navigateByUrl');
spyOn(authService, 'setRedirect');

await authGuard.canActivate(null, state);

expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ALL', url: 'some-url'
});
expect(router.navigateByUrl).toHaveBeenCalledWith('/login?redirectUrl=some-url');
expect(await authGuard.canActivate(null, state)).toEqual(router.parseUrl('/login?redirectUrl=some-url'));
}));

it('should set redirect url with query params', async(async () => {
Expand All @@ -144,12 +136,7 @@ describe('AuthGuardService', () => {
spyOn(router, 'navigateByUrl');
spyOn(authService, 'setRedirect');

await authGuard.canActivate(null, state);

expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ALL', url: 'some-url;q=query'
});
expect(router.navigateByUrl).toHaveBeenCalledWith('/login?redirectUrl=some-url;q=query');
expect(await authGuard.canActivate(null, state)).toEqual(router.parseUrl('/login?redirectUrl=some-url;q=query'));
}));

it('should get redirect url from config if there is one configured', async(async () => {
Expand All @@ -159,12 +146,7 @@ describe('AuthGuardService', () => {
spyOn(router, 'navigateByUrl');
spyOn(authService, 'setRedirect');

await authGuard.canActivate(null, state);

expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ALL', url: 'some-url'
});
expect(router.navigateByUrl).toHaveBeenCalledWith('/fakeLoginRoute?redirectUrl=some-url');
expect(await authGuard.canActivate(null, state)).toEqual(router.parseUrl('/fakeLoginRoute?redirectUrl=some-url'));
}));

it('should pass actual redirect when no state segments exists', async(async () => {
Expand Down
6 changes: 3 additions & 3 deletions lib/core/services/auth-guard.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/

import { Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, Router } from '@angular/router';
import { ActivatedRouteSnapshot, Router, UrlTree } from '@angular/router';
import { AuthenticationService } from './authentication.service';
import { AppConfigService } from '../app-config/app-config.service';
import { AuthGuardBase } from './auth-guard-base';
Expand Down Expand Up @@ -67,11 +67,11 @@ export class AuthGuard extends AuthGuardBase {
}
}

async checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): Promise<boolean> {
async checkLogin(_: ActivatedRouteSnapshot, redirectUrl: string): Promise<boolean | UrlTree> {
if (this.authenticationService.isLoggedIn() || this.withCredentials) {

return true;
}
return this.redirectToUrl( redirectUrl);
return this.redirectToUrl(redirectUrl);
}
}

0 comments on commit 4a381b4

Please sign in to comment.