[ADF-2904] Search - redirect breaks application (#3268)

* set navigation commands over plain string

* fix test

* lint
This commit is contained in:
Cilibiu Bogdan
2018-05-08 16:46:35 +03:00
committed by Eugenio Romano
parent c90ad3f875
commit d456b3cba1
12 changed files with 205 additions and 105 deletions

View File

@@ -110,12 +110,12 @@ describe('LoginComponent', () => {
spyOn(authService, 'login').and.returnValue(Observable.of({ type: 'type', ticket: 'ticket'}));
const redirect = '/home';
component.successRoute = redirect;
authService.setRedirectUrl({ provider: 'ECM', url: 'redirect-url' } );
authService.setRedirect({ provider: 'ECM', navigation: ['some-route'] } );
spyOn(router, 'navigate');
loginWithCredentials('fake-username', 'fake-password');
expect(router.navigate).toHaveBeenCalledWith(['redirect-url']);
expect(router.navigate).toHaveBeenCalledWith(['some-route']);
});
it('should update user preferences upon login', async(() => {

View File

@@ -213,16 +213,16 @@ export class LoginComponent implements OnInit {
this.authService.login(values.username, values.password, this.rememberMe)
.subscribe(
(token: any) => {
const redirectUrl = this.authService.getRedirectUrl(this.providers);
const redirect = this.authService.getRedirect(this.providers);
this.actualLoginStep = LoginSteps.Welcome;
this.userPreferences.setStoragePrefix(values.username);
values.password = null;
this.success.emit(new LoginSuccessEvent(token, values.username, null));
if (redirectUrl) {
this.authService.setRedirectUrl(null);
this.router.navigate([redirectUrl]);
if (redirect) {
this.authService.setRedirect(null);
this.router.navigate(redirect);
} else if (this.successRoute) {
this.router.navigate([this.successRoute]);
}

View File

@@ -21,14 +21,14 @@ import { RedirectionModel } from '../models/redirection.model';
// TODO: should be extending AuthenticationService
export class AuthenticationMock /*extends AuthenticationService*/ {
private redirectUrl: RedirectionModel = null;
private redirect: RedirectionModel = null;
setRedirectUrl(url: RedirectionModel) {
this.redirectUrl = url;
this.redirect = url;
}
getRedirectUrl(): string {
return this.redirectUrl ? this.redirectUrl.url : null;
getRedirectUrl(): any[] {
return this.redirect ? this.redirect.navigation : null;
}
// TODO: real auth service returns Observable<string>

View File

@@ -21,12 +21,12 @@
export class RedirectionModel {
provider: string;
url?: string;
navigation?: any[];
constructor(obj?: any) {
if (obj) {
this.provider = obj.provider;
this.url = obj.url || null;
this.navigation = obj.navigation || null;
}
}

View File

@@ -43,7 +43,7 @@ describe('AuthGuardService BPM', () => {
it('if the alfresco js api is logged in should canActivate be true', async(() => {
spyOn(authService, 'isBpmLoggedIn').and.returnValue(true);
const router: RouterStateSnapshot = <RouterStateSnapshot> {url : ''};
const router: RouterStateSnapshot = <RouterStateSnapshot> {url : 'some-url'};
expect(authGuard.canActivate(null, router)).toBeTruthy();
}));
@@ -51,7 +51,7 @@ describe('AuthGuardService BPM', () => {
it('if the alfresco js api is NOT logged in should canActivate be false', async(() => {
spyOn(authService, 'isBpmLoggedIn').and.returnValue(false);
spyOn(routerService, 'navigate').and.stub();
const router: RouterStateSnapshot = <RouterStateSnapshot> { url: '' };
const router: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };
expect(authGuard.canActivate(null, router)).toBeFalsy();
}));
@@ -59,32 +59,49 @@ describe('AuthGuardService BPM', () => {
it('if the alfresco js api is NOT logged in should trigger a redirect event', async(() => {
spyOn(routerService, 'navigate');
spyOn(authService, 'isBpmLoggedIn').and.returnValue(false);
const router: RouterStateSnapshot = <RouterStateSnapshot> {url : ''};
const router: RouterStateSnapshot = <RouterStateSnapshot> {url : 'some-url'};
expect(authGuard.canActivate(null, router)).toBeFalsy();
expect(routerService.navigate).toHaveBeenCalledWith(['/login']);
}));
it('should set redirect url', async(() => {
spyOn(authService, 'setRedirectUrl').and.callThrough();
it('should set redirect navigation commands', async(() => {
spyOn(authService, 'setRedirect').and.callThrough();
spyOn(routerService, 'navigate').and.stub();
const router: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };
authGuard.canActivate(null, router);
expect(authService.setRedirectUrl).toHaveBeenCalledWith({provider: 'BPM', url: 'some-url' } );
expect(authService.getRedirectUrl('BPM')).toBe('some-url');
expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'BPM', navigation: ['some-url', {}]
});
expect(authService.getRedirect('BPM')).toEqual(['some-url', {}]);
}));
it('should set redirect navigation commands with query params', async(() => {
spyOn(authService, 'setRedirect').and.callThrough();
spyOn(routerService, 'navigate').and.stub();
const router: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url;q=123' };
authGuard.canActivate(null, router);
expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'BPM', navigation: ['some-url', {q: '123'}]
});
expect(authService.getRedirect('BPM')).toEqual(['some-url', { q: '123' }]);
}));
it('should get redirect url from config if there is one configured', async(() => {
appConfigService.config.loginRoute = 'fakeLoginRoute';
spyOn(authService, 'setRedirectUrl').and.callThrough();
spyOn(authService, 'setRedirect').and.callThrough();
spyOn(routerService, 'navigate').and.stub();
const router: RouterStateSnapshot = <RouterStateSnapshot> { url: 'some-url' };
authGuard.canActivate(null, router);
expect(authService.setRedirectUrl).toHaveBeenCalledWith({provider: 'BPM', url: 'some-url' } );
expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'BPM', navigation: ['some-url', {}]
});
expect(routerService.navigate).toHaveBeenCalledWith(['/fakeLoginRoute']);
}));

View File

@@ -17,8 +17,8 @@
import { Injectable } from '@angular/core';
import {
ActivatedRouteSnapshot, CanActivate, CanActivateChild,
RouterStateSnapshot, Router
ActivatedRouteSnapshot, CanActivate, CanActivateChild, RouterStateSnapshot,
Router, PRIMARY_OUTLET, UrlTree, UrlSegmentGroup, UrlSegment
} from '@angular/router';
import { AppConfigService } from '../app-config/app-config.service';
import { AuthenticationService } from './authentication.service';
@@ -42,7 +42,9 @@ export class AuthGuardBpm implements CanActivate, CanActivateChild {
return true;
}
this.authService.setRedirectUrl({ provider: 'BPM', url: redirectUrl });
const navigation = this.getNavigationCommands(redirectUrl);
this.authService.setRedirect({ provider: 'BPM', navigation });
const pathToLogin = this.getRouteDestinationForLogin();
this.router.navigate(['/' + pathToLogin]);
@@ -54,4 +56,15 @@ export class AuthGuardBpm implements CanActivate, CanActivateChild {
this.appConfig.get<string>('loginRoute') ?
this.appConfig.get<string>('loginRoute') : 'login';
}
private getNavigationCommands(redirectUrl: string): any[] {
const urlTree: UrlTree = this.router.parseUrl(redirectUrl);
const urlSegmentGroup: UrlSegmentGroup = urlTree.root.children[PRIMARY_OUTLET];
const urlSegments: UrlSegment[] = urlSegmentGroup.segments;
return urlSegments.reduce(function(acc, item) {
acc.push(item.path, item.parameters);
return acc;
}, []);
}
}

View File

@@ -15,7 +15,8 @@
* limitations under the License.
*/
import { async, inject, TestBed } from '@angular/core/testing';
import { async, TestBed } from '@angular/core/testing';
import { RouterTestingModule } from '@angular/router/testing';
import { Router } from '@angular/router';
import { AlfrescoApiService } from './alfresco-api.service';
import { AuthGuardEcm } from './auth-guard-ecm.service';
@@ -23,10 +24,6 @@ import { AuthenticationService } from './authentication.service';
import { AppConfigService } from '../app-config/app-config.service';
import { HttpClientModule } from '@angular/common/http';
class RouterProvider {
navigate: Function = jasmine.createSpy('RouterProviderNavigate');
}
class AlfrescoApiServiceProvider {
private settings: any = {
validateTicket: true,
@@ -64,7 +61,7 @@ class AlfrescoApiServiceProvider {
}
class AuthenticationServiceProvider {
setRedirectUrl: Function = jasmine.createSpy('setRedirectUrl');
setRedirect: Function = jasmine.createSpy('setRedirect');
}
class TestConfig {
@@ -82,29 +79,21 @@ class TestConfig {
TestBed.configureTestingModule({
imports: [
HttpClientModule
HttpClientModule,
RouterTestingModule
],
providers: [
AppConfigService,
this.routerProvider,
this.alfrescoApiServiceProvider,
this.authenticationProvider,
AuthGuardEcm
]
});
inject([ AuthGuardEcm, Router, AuthenticationService ], (guard: AuthGuardEcm, router: Router, auth: AuthenticationService) => {
this.guard = guard;
this.router = router;
this.auth = auth;
})();
}
this.guard = TestBed.get(AuthGuardEcm);
this.router = TestBed.get(Router);
this.auth = TestBed.get(AuthenticationService);
private get routerProvider() {
return {
provide: Router,
useValue: new RouterProvider()
};
}
private get authenticationProvider() {
@@ -125,10 +114,6 @@ class TestConfig {
})
};
}
get navigateSpy() {
return this.router.navigate;
}
}
describe('AuthGuardService ECM', () => {
@@ -140,10 +125,11 @@ describe('AuthGuardService ECM', () => {
const { guard, router } = this.test;
guard.canActivate(null, { url: '' }).then((activate) => {
guard.canActivate(null, { url: 'some-url' }).then((activate) => {
this.activate = activate;
this.navigateSpy = router.navigate;
});
this.navigateSpy = spyOn(router, 'navigate');
}));
it('does not allow route to activate', () => {
@@ -164,10 +150,11 @@ describe('AuthGuardService ECM', () => {
const { guard, router } = this.test;
guard.canActivate(null, { url: '' }).then((activate) => {
guard.canActivate(null, { url: 'some-url' }).then((activate) => {
this.activate = activate;
this.navigateSpy = router.navigate;
});
this.navigateSpy = spyOn(router, 'navigate');
}));
it('does not allow route to activate', () => {
@@ -188,10 +175,11 @@ describe('AuthGuardService ECM', () => {
const { guard, router } = this.test;
guard.canActivate(null, { url: '' }).then((activate) => {
guard.canActivate(null, { url: 'some-url' }).then((activate) => {
this.activate = activate;
this.navigateSpy = router.navigate;
});
this.navigateSpy = spyOn(router, 'navigate');
}));
it('allows route to activate', () => {
@@ -203,21 +191,49 @@ describe('AuthGuardService ECM', () => {
});
});
describe('redirect url', () => {
beforeEach(async(() => {
this.test = new TestConfig({
isLoggedIn: false
describe('redirect', () => {
describe('path', () => {
beforeEach(async(() => {
this.test = new TestConfig({
isLoggedIn: false
});
const { guard, auth, router } = this.test;
guard.canActivate(null, { url: 'some-url/123' }).then((activate) => {
this.auth = auth;
});
this.navigateSpy = spyOn(router, 'navigate');
}));
it('should set redirect navigation commands', () => {
expect(this.auth.setRedirect).toHaveBeenCalledWith({
provider: 'ECM', navigation: ['some-url', {}, '123', {}]
});
});
});
const { guard, auth } = this.test;
describe('with query params', () => {
beforeEach(async(() => {
this.test = new TestConfig({
isLoggedIn: false
});
guard.canActivate(null, { url: 'some-url' }).then((activate) => {
this.auth = auth;
const { guard, auth, router } = this.test;
guard.canActivate(null, { url: 'some-url;q=123' }).then((activate) => {
this.auth = auth;
});
this.navigateSpy = spyOn(router, 'navigate');
}));
it('should set redirect navigation commands with query params', () => {
expect(this.auth.setRedirect).toHaveBeenCalledWith({
provider: 'ECM', navigation: ['some-url', { q: '123' }]
});
});
}));
it('should set redirect url', () => {
expect(this.auth.setRedirectUrl).toHaveBeenCalledWith({ provider: 'ECM', url: 'some-url' });
});
});
@@ -228,11 +244,13 @@ describe('AuthGuardService ECM', () => {
isLoggedIn: false
});
const { guard } = this.test;
const { guard, router } = this.test;
guard.canActivateChild(null, { url: '' }).then((activate) => {
guard.canActivateChild(null, { url: 'some-url' }).then((activate) => {
this.activate = activate;
});
this.navigateSpy = spyOn(router, 'navigate');
}));
it('should not allow route to activate', () => {
@@ -247,11 +265,13 @@ describe('AuthGuardService ECM', () => {
validateTicket: false
});
const { guard } = this.test;
const { guard, router } = this.test;
guard.canActivateChild(null, { url: '' }).then((activate) => {
guard.canActivateChild(null, { url: 'some-url' }).then((activate) => {
this.activate = activate;
});
this.navigateSpy = spyOn(router, 'navigate');
}));
it('should not allow route to activate', () => {
@@ -266,11 +286,13 @@ describe('AuthGuardService ECM', () => {
validateTicket: true
});
const { guard } = this.test;
const { guard, router } = this.test;
guard.canActivateChild(null, { url: '' }).then((activate) => {
this.activate = activate;
});
this.navigateSpy = spyOn(router, 'navigate');
}));
it('should allow route to activate', () => {

View File

@@ -16,7 +16,10 @@
*/
import { Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot, Router } from '@angular/router';
import {
ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot, Router,
PRIMARY_OUTLET, UrlTree, UrlSegmentGroup, UrlSegment
} from '@angular/router';
import { AlfrescoApiService } from './alfresco-api.service';
import { AuthenticationService } from './authentication.service';
import { AppConfigService } from '../app-config/app-config.service';
@@ -50,10 +53,11 @@ export class AuthGuardEcm implements CanActivate {
}
canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Promise<boolean> {
return this.isLoggedIn().then(isLoggedIn => {
if (!isLoggedIn) {
this.authService.setRedirectUrl({ provider: 'ECM', url: state.url });
const navigation = this.getNavigationCommands(state.url);
this.authService.setRedirect({ provider: 'ECM', navigation });
const pathToLogin = this.getRouteDestinationForLogin();
this.router.navigate(['/' + pathToLogin]);
}
@@ -67,4 +71,15 @@ export class AuthGuardEcm implements CanActivate {
this.appConfig.get<string>('loginRoute') ?
this.appConfig.get<string>('loginRoute') : 'login';
}
private getNavigationCommands(redirectUrl: string): any[] {
const urlTree: UrlTree = this.router.parseUrl(redirectUrl);
const urlSegmentGroup: UrlSegmentGroup = urlTree.root.children[PRIMARY_OUTLET];
const urlSegments: UrlSegment[] = urlSegmentGroup.segments;
return urlSegments.reduce(function(acc, item) {
acc.push(item.path, item.parameters);
return acc;
}, []);
}
}

View File

@@ -51,6 +51,7 @@ describe('AuthGuardService', () => {
}));
it('if the alfresco js api is NOT logged in should canActivate be false', async(() => {
state.url = 'some-url';
spyOn(router, 'navigate');
spyOn(authService, 'isLoggedIn').and.returnValue(false);
@@ -62,11 +63,27 @@ describe('AuthGuardService', () => {
state.url = 'some-url';
spyOn(router, 'navigate');
spyOn(authService, 'setRedirectUrl');
spyOn(authService, 'setRedirect');
service.canActivate(null, state);
expect(authService.setRedirectUrl).toHaveBeenCalledWith({ provider: 'ALL', url: 'some-url' });
expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ALL', navigation: ['some-url', {}]
});
expect(router.navigate).toHaveBeenCalledWith(['/login']);
}));
it('should set redirect url with query params', async(() => {
state.url = 'some-url;q=query';
spyOn(router, 'navigate');
spyOn(authService, 'setRedirect');
service.canActivate(null, state);
expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ALL', navigation: ['some-url', { q: 'query' } ]
});
expect(router.navigate).toHaveBeenCalledWith(['/login']);
}));
@@ -75,11 +92,13 @@ describe('AuthGuardService', () => {
appConfigService.config.loginRoute = 'fakeLoginRoute';
spyOn(router, 'navigate');
spyOn(authService, 'setRedirectUrl');
spyOn(authService, 'setRedirect');
service.canActivate(null, state);
expect(authService.setRedirectUrl).toHaveBeenCalledWith({ provider: 'ALL', url: 'some-url' });
expect(authService.setRedirect).toHaveBeenCalledWith({
provider: 'ALL', navigation: ['some-url', {}]
});
expect(router.navigate).toHaveBeenCalledWith(['/fakeLoginRoute']);
}));
});

View File

@@ -18,7 +18,8 @@
import { Injectable } from '@angular/core';
import {
ActivatedRouteSnapshot, CanActivate,
CanActivateChild, RouterStateSnapshot, Router
CanActivateChild, RouterStateSnapshot, Router,
PRIMARY_OUTLET, UrlTree, UrlSegmentGroup, UrlSegment
} from '@angular/router';
import { AppConfigService } from '../app-config/app-config.service';
import { AuthenticationService } from './authentication.service';
@@ -44,7 +45,9 @@ export class AuthGuard implements CanActivate, CanActivateChild {
return true;
}
this.authService.setRedirectUrl({ provider: 'ALL', url: redirectUrl } );
const navigation = this.getNavigationCommands(redirectUrl);
this.authService.setRedirect({ provider: 'ALL', navigation } );
const pathToLogin = this.getRouteDestinationForLogin();
this.router.navigate(['/' + pathToLogin]);
@@ -56,4 +59,15 @@ export class AuthGuard implements CanActivate, CanActivateChild {
this.appConfig.get<string>('loginRoute') ?
this.appConfig.get<string>('loginRoute') : 'login';
}
private getNavigationCommands(redirectUrl: string): any[] {
const urlTree: UrlTree = this.router.parseUrl(redirectUrl);
const urlSegmentGroup: UrlSegmentGroup = urlTree.root.children[PRIMARY_OUTLET];
const urlSegments: UrlSegment[] = urlSegmentGroup.segments;
return urlSegments.reduce(function(acc, item) {
acc.push(item.path, item.parameters);
return acc;
}, []);
}
}

View File

@@ -260,21 +260,21 @@ describe('AuthenticationService', () => {
});
it('[ECM] should set/get redirectUrl when provider is ECM', () => {
authService.setRedirectUrl({provider: 'ECM', url: 'some-url' } );
authService.setRedirect({provider: 'ECM', navigation: ['some-url'] } );
expect(authService.getRedirectUrl(preferences.authType)).toBe('some-url');
expect(authService.getRedirect(preferences.authType)).toEqual(['some-url']);
});
it('[ECM] should set/get redirectUrl when provider is BPM', () => {
authService.setRedirectUrl({provider: 'BPM', url: 'some-url' } );
authService.setRedirect({provider: 'BPM', navigation: ['some-url'] } );
expect(authService.getRedirectUrl(preferences.authType)).toBeNull();
expect(authService.getRedirect(preferences.authType)).toBeNull();
});
it('[ECM] should return null as redirectUrl when redirectUrl field is not set', () => {
authService.setRedirectUrl( null );
authService.setRedirect( null );
expect(authService.getRedirectUrl(preferences.authType)).toBeNull();
expect(authService.getRedirect(preferences.authType)).toBeNull();
});
});
@@ -405,21 +405,21 @@ describe('AuthenticationService', () => {
});
it('[BPM] should set/get redirectUrl when provider is BPM', () => {
authService.setRedirectUrl({provider: 'BPM', url: 'some-url' } );
authService.setRedirect({provider: 'BPM', navigation: ['some-url'] } );
expect(authService.getRedirectUrl(preferences.authType)).toBe('some-url');
expect(authService.getRedirect(preferences.authType)).toEqual(['some-url']);
});
it('[BPM] should set/get redirectUrl when provider is ECM', () => {
authService.setRedirectUrl({provider: 'ECM', url: 'some-url' } );
authService.setRedirect({provider: 'ECM', navigation: ['some-url'] } );
expect(authService.getRedirectUrl(preferences.authType)).toBeNull();
expect(authService.getRedirect(preferences.authType)).toBeNull();
});
it('[BPM] should return null as redirectUrl when redirectUrl field is not set', () => {
authService.setRedirectUrl( null );
authService.setRedirect( null );
expect(authService.getRedirectUrl(preferences.authType)).toBeNull();
expect(authService.getRedirect(preferences.authType)).toBeNull();
});
});
@@ -517,27 +517,27 @@ describe('AuthenticationService', () => {
});
it('[ALL] should set/get redirectUrl when provider is ALL', () => {
authService.setRedirectUrl({provider: 'ALL', url: 'some-url' } );
authService.setRedirect({provider: 'ALL', navigation: ['some-url'] } );
expect(authService.getRedirectUrl(preferences.authType)).toBe('some-url');
expect(authService.getRedirect(preferences.authType)).toEqual(['some-url']);
});
it('[ALL] should set/get redirectUrl when provider is BPM', () => {
authService.setRedirectUrl({provider: 'BPM', url: 'some-url' } );
authService.setRedirect({provider: 'BPM', navigation: ['some-url'] } );
expect(authService.getRedirectUrl(preferences.authType)).toBe('some-url');
expect(authService.getRedirect(preferences.authType)).toEqual(['some-url']);
});
it('[ALL] should set/get redirectUrl when provider is ECM', () => {
authService.setRedirectUrl({provider: 'ECM', url: 'some-url' } );
authService.setRedirect({provider: 'ECM', navigation: ['some-url'] } );
expect(authService.getRedirectUrl(preferences.authType)).toBe('some-url');
expect(authService.getRedirect(preferences.authType)).toEqual(['some-url']);
});
it('[ALL] should return null as redirectUrl when redirectUrl field is not set', () => {
authService.setRedirectUrl( null );
authService.setRedirect( null );
expect(authService.getRedirectUrl(preferences.authType)).toBeNull();
expect(authService.getRedirect(preferences.authType)).toBeNull();
});
});

View File

@@ -33,7 +33,7 @@ const REMEMBER_ME_UNTIL = 1000 * 60 * 60 * 24 * 30 ;
@Injectable()
export class AuthenticationService {
private redirectUrl: RedirectionModel = null;
private redirect: RedirectionModel = null;
onLogin: Subject<any> = new Subject<any>();
onLogout: Subject<any> = new Subject<any>();
@@ -246,24 +246,24 @@ export class AuthenticationService {
/** Sets the URL to redirect to after login.
* @param url URL to redirect to
*/
setRedirectUrl(url: RedirectionModel) {
this.redirectUrl = url;
setRedirect(url: RedirectionModel) {
this.redirect = url;
}
/** Gets the URL to redirect to after login.
* @param provider Service provider. Can be "ECM", "BPM" or "ALL".
* @returns The redirect URL
*/
getRedirectUrl(provider: string): string {
return this.hasValidRedirection(provider) ? this.redirectUrl.url : null;
getRedirect(provider: string): any[] {
return this.hasValidRedirection(provider) ? this.redirect.navigation : null;
}
private hasValidRedirection(provider: string): boolean {
return this.redirectUrl && (this.redirectUrl.provider === provider || this.hasSelectedProviderAll(provider));
return this.redirect && (this.redirect.provider === provider || this.hasSelectedProviderAll(provider));
}
private hasSelectedProviderAll(provider: string): boolean {
return this.redirectUrl && (this.redirectUrl.provider === 'ALL' || provider === 'ALL');
return this.redirect && (this.redirect.provider === 'ALL' || provider === 'ALL');
}
/**