[CORD-2827] Fixed unauthorized error handling
Change-Id: I6ddef7f869c17db4d8479f23f6e8734f6002d8fc
diff --git a/src/app/core/header/header.scss b/src/app/core/header/header.scss
index f6006d2..2db767c 100644
--- a/src/app/core/header/header.scss
+++ b/src/app/core/header/header.scss
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
+@import './../../style/vars.scss';
#toast-container [toast]:not(:first-child) {
margin-top: 10px !important;
@@ -41,6 +41,7 @@
background: #2a2d35 !important;
min-width: 300px;
+ /* service status */
.table {
margin-bottom: 0;
}
@@ -52,6 +53,20 @@
.table tr th:last-child {
padding-right: 20px;
}
+
+ /* search bar */
+
+ li.uib-typeahead-match > a {
+ color: #ccc;
+ }
+
+ li.uib-typeahead-match.active > a,
+ li.uib-typeahead-match.active > a:hover,
+ li.uib-typeahead-match.active > a:focus {
+ font-weight: bold;
+ background-color: rgba(68, 70, 79, 0.5);
+ color: #fff;
+ }
}
.navbar-nav a {
@@ -61,7 +76,8 @@
.navbar-nav > .open > a,
.navbar-nav > .open > a:hover {
color:#ffffff !important;
- background-color:rgba(246,168,33,0.1) !important;;
+ background-color:rgba(246,168,33,0.1) !important;
border-color:#f6a821;
}
+
}
\ No newline at end of file
diff --git a/src/app/core/loader/loader.scss b/src/app/core/loader/loader.scss
new file mode 100644
index 0000000..0f7021a
--- /dev/null
+++ b/src/app/core/loader/loader.scss
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2017-present Open Networking Foundation
+
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+
+ * http://www.apache.org/licenses/LICENSE-2.0
+
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+xos-loader {
+ .alert.alert-danger {
+ margin-top: 100px;
+
+ .fa.fa-exclamation-triangle {
+ font-size: 40px;
+ }
+ }
+}
\ No newline at end of file
diff --git a/src/app/core/loader/loader.spec.ts b/src/app/core/loader/loader.spec.ts
index 44fc60f..0744c11 100644
--- a/src/app/core/loader/loader.spec.ts
+++ b/src/app/core/loader/loader.spec.ts
@@ -77,86 +77,116 @@
timeout = $timeout;
location = $location;
spyOn(location, 'path');
-
- MockDiscover.discover = jasmine.createSpy('discover')
- .and.callFake(() => {
- const d = $q.defer();
- d.resolve(true);
- return d.promise;
- });
-
- MockOnboarder.onboard = jasmine.createSpy('onboard')
- .and.callFake(() => {
- const d = $q.defer();
- d.resolve();
- return d.promise;
- });
}));
- describe('when models are already loaded', () => {
-
- beforeEach(() => {
- compileElement();
- spyOn(isolatedScope, 'moveOnTo');
- isolatedScope.run();
- timeout.flush();
- });
-
- it('should redirect to the last visited page', (done) => {
- window.setTimeout(() => {
- expect(isolatedScope.moveOnTo).toHaveBeenCalledWith('/test');
- expect(location.path).toHaveBeenCalledWith('/test');
- done();
- }, 600);
- });
- });
-
- describe('when the last visited page is "loader"', () => {
-
- beforeEach(() => {
- MockConfig.lastVisitedUrl = '/loader';
- compileElement();
- spyOn(isolatedScope, 'moveOnTo');
- isolatedScope.run();
- });
-
- it('should redirect to the "dashboard" page', (done) => {
- window.setTimeout(() => {
- expect(isolatedScope.moveOnTo).toHaveBeenCalledWith('/loader');
- expect(location.path).toHaveBeenCalledWith('/dashboard');
- done();
- }, 600);
- });
- });
-
- describe('when user is not authenticated', () => {
-
- beforeEach(() => {
+ describe('when chameleon is not responding', () => {
+ beforeEach(inject(($q: ng.IQService) => {
loaded = false;
- authenticated = false;
- compileElement();
- isolatedScope.run();
- });
-
- it('should redirect to the login page', () => {
- expect(MockState.go).toHaveBeenCalledWith('xos.login');
- });
-
- afterEach(() => {
authenticated = true;
+ MockDiscover.discover = jasmine.createSpy('discover')
+ .and.callFake(() => {
+ const d = $q.defer();
+ d.resolve('chameleon');
+ return d.promise;
+ });
+ compileElement();
+ spyOn(isolatedScope, 'moveOnTo');
+ isolatedScope.run();
+ }));
+
+ it('should print an error', () => {
+ expect(isolatedScope.moveOnTo).not.toHaveBeenCalled();
+ expect(isolatedScope.error).toBe('chameleon');
+ expect(isolatedScope.loader).toBeFalsy();
});
});
- describe('when models are not loaded', () => {
+ describe('when chameleon is available', () => {
- beforeEach(() => {
- loaded = false;
- compileElement();
- spyOn(isolatedScope, 'moveOnTo');
+ beforeEach(inject(($q: ng.IQService) => {
+ loaded = true;
+ authenticated = true;
+ MockDiscover.discover = jasmine.createSpy('discover')
+ .and.callFake(() => {
+ const d = $q.defer();
+ d.resolve(true);
+ return d.promise;
+ });
+
+ MockOnboarder.onboard = jasmine.createSpy('onboard')
+ .and.callFake(() => {
+ const d = $q.defer();
+ d.resolve();
+ return d.promise;
+ });
+ }));
+
+ describe('when models are already loaded', () => {
+
+ beforeEach(() => {
+ compileElement();
+ spyOn(isolatedScope, 'moveOnTo');
+ isolatedScope.run();
+ timeout.flush();
+ });
+
+ it('should redirect to the last visited page', (done) => {
+ window.setTimeout(() => {
+ expect(isolatedScope.moveOnTo).toHaveBeenCalledWith('/test');
+ expect(location.path).toHaveBeenCalledWith('/test');
+ done();
+ }, 600);
+ });
});
- it('should call XosModelDiscoverer.discover', () => {
- expect(MockDiscover.discover).toHaveBeenCalled();
+ describe('when the last visited page is "loader"', () => {
+
+ beforeEach(() => {
+ MockConfig.lastVisitedUrl = '/loader';
+ compileElement();
+ spyOn(isolatedScope, 'moveOnTo');
+ isolatedScope.run();
+ });
+
+ it('should redirect to the "dashboard" page', (done) => {
+ window.setTimeout(() => {
+ expect(isolatedScope.moveOnTo).toHaveBeenCalledWith('/loader');
+ expect(location.path).toHaveBeenCalledWith('/dashboard');
+ done();
+ }, 600);
+ });
+ });
+
+ describe('when user is not authenticated', () => {
+
+ beforeEach(() => {
+ loaded = false;
+ authenticated = false;
+ compileElement();
+ isolatedScope.run();
+ });
+
+ it('should redirect to the login page', () => {
+ expect(MockState.go).toHaveBeenCalledWith('xos.login');
+ });
+
+ afterEach(() => {
+ authenticated = true;
+ });
+ });
+
+ describe('when models are not loaded', () => {
+
+ beforeEach(() => {
+ loaded = false;
+ authenticated = true;
+ compileElement();
+ spyOn(isolatedScope, 'moveOnTo');
+ });
+
+ it('should call XosModelDiscoverer.discover', () => {
+ expect(MockDiscover.discover).toHaveBeenCalled();
+ });
});
});
diff --git a/src/app/core/loader/loader.ts b/src/app/core/loader/loader.ts
index b93aebf..3606ca8 100644
--- a/src/app/core/loader/loader.ts
+++ b/src/app/core/loader/loader.ts
@@ -19,6 +19,8 @@
import {IXosModelDiscovererService} from '../../datasources/helpers/model-discoverer.service';
import {IXosOnboarder} from '../../extender/services/onboard.service';
import {IXosAuthService} from '../../datasources/rest/auth.rest';
+import './loader.scss';
+
class LoaderCtrl {
static $inject = [
'$log',
@@ -32,6 +34,9 @@
`XosOnboarder`
];
+ public loader: boolean = true;
+ public error: string;
+
constructor (
private $log: ng.ILogService,
private $rootScope: ng.IScope,
@@ -49,26 +54,45 @@
public run() {
if (this.XosModelDiscoverer.areModelsLoaded()) {
+ this.$log.debug(`[XosLoader] Models are already loaded, moving to: ${this.XosConfig.lastVisitedUrl}`);
this.moveOnTo(this.XosConfig.lastVisitedUrl);
}
else if (!this.XosAuthService.isAuthenticated()) {
+ this.$log.debug(`[XosLoader] Not authenticated, send to login`);
this.$state.go('xos.login');
}
else {
// NOTE loading XOS Models
this.XosModelDiscoverer.discover()
.then((res) => {
- if (res) {
+ this.$log.info('[XosLoader] res: ' + res, res, typeof res);
+ if (res === 'chameleon') {
+ this.loader = false;
+ this.error = 'chameleon';
+ return 'chameleon';
+ }
+ else if (res) {
this.$log.info('[XosLoader] All models loaded');
+ // NOTE loading GUI Extensions
+ this.XosOnboarder.onboard();
+ return true;
}
else {
this.$log.info('[XosLoader] Failed to load some models, moving on.');
+ return true;
}
- // NOTE loading GUI Extensions
- return this.XosOnboarder.onboard();
})
- .then(() => {
- this.moveOnTo(this.XosConfig.lastVisitedUrl);
+ .then((res) => {
+ if (res === true) {
+ this.moveOnTo(this.XosConfig.lastVisitedUrl);
+ }
+ // NOTE otherwise stay here since we're printing some error messages
+ })
+ .catch(() => {
+ // XosModelDiscoverer.discover reject only in case of authentication error
+ this.XosAuthService.clearUser();
+ this.moveOnTo('/login');
+
})
.finally(() => {
// NOTE it is in a timeout as the searchService is loaded after that
@@ -99,7 +123,22 @@
export const xosLoader: angular.IComponentOptions = {
template: `
- <div class="loader"></div>
+ <div ng-show="vm.loader" class="loader"></div>
+ <div class="row" ng-show="vm.error == 'chameleon'">
+ <div class="col-sm-6 col-sm-offset-3">
+ <div class="alert alert-danger">
+ <div class="row">
+ <div class="col-xs-2">
+ <i class="fa fa-exclamation-triangle"></i>
+ </div>
+ <div class="col-xs-10">
+ <strong>Cannot load models definition.</strong><br>
+ Please check that the Chameleon container is running
+ </div>
+ </div>
+ </div>
+ </div>
+ </div>
`,
controllerAs: 'vm',
controller: LoaderCtrl
diff --git a/src/app/datasources/helpers/model-discoverer.service.ts b/src/app/datasources/helpers/model-discoverer.service.ts
index fca593c..3ee5a15 100644
--- a/src/app/datasources/helpers/model-discoverer.service.ts
+++ b/src/app/datasources/helpers/model-discoverer.service.ts
@@ -44,7 +44,7 @@
// Service
export interface IXosModelDiscovererService {
- discover(): ng.IPromise<boolean>;
+ discover(): ng.IPromise<string>;
getApiUrlFromModel(model: IXosModel): string;
areModelsLoaded(): boolean;
}
@@ -127,28 +127,42 @@
return this.$q.resolve('true');
})
.catch(err => {
- this.$log.error(`[XosModelDiscovererService] Model ${model.name} NOT stored`, err);
+ this.$log.warn(`[XosModelDiscovererService] Model ${model.name} NOT stored`, err);
+ const isAuthError = this.AuthService.isAuthError(err);
+ if (isAuthError) {
+ this.$log.warn(`[XosModelDiscovererService] User is not authentincated`);
+ return this.$q.reject(err);
+ }
return this.$q.resolve('false');
});
pArray.push(p);
});
+
+
this.$q.all(pArray)
.then((res) => {
- // the Model Loader promise won't ever be reject, in case it will be resolve with value false,
+ // the ModelLoader promise won't ever be reject, in case it will be resolve with value false,
// that's because we want to wait anyway for all the models to be loaded
if (res.indexOf('false') > -1) {
return d.resolve(false);
}
d.resolve(true);
+ this.modelsLoaded = true;
})
.catch((e) => {
- this.$log.error(`[XosModelDiscovererService]`, e);
- d.resolve(false);
+ this.XosModelStore.clean(); // reset all the observable otherwise they'll store login errors
+ this.$log.warn(`[XosModelDiscovererService]`, e);
+ // the ModelLoader promise will be rejected in case of authentication error
+ d.reject(e);
})
.finally(() => {
this.progressBar.complete();
- this.modelsLoaded = true;
});
+ })
+ .catch(err => {
+ this.progressBar.complete();
+ this.$log.error(`[XosModelDiscovererService] Cannot load model defs`, err);
+ return d.resolve('chameleon');
});
return d.promise;
}
@@ -249,16 +263,17 @@
}
private cacheModelEntries(model: IXosModel): ng.IPromise<IXosModel> {
+
const d = this.$q.defer();
const apiUrl = this.getApiUrlFromModel(model);
this.XosModelStore.query(model.name, apiUrl)
+ .skip(1) // NOTE observables returns as first an empty array, so skip it
.subscribe(
() => {
return d.resolve(model);
},
err => {
- this.AuthService.handleUnauthenticatedRequest(err);
return d.reject(err);
}
);
diff --git a/src/app/datasources/helpers/model.discoverer.service.spec.ts b/src/app/datasources/helpers/model.discoverer.service.spec.ts
index a6e6340..5deb444 100644
--- a/src/app/datasources/helpers/model.discoverer.service.spec.ts
+++ b/src/app/datasources/helpers/model.discoverer.service.spec.ts
@@ -73,9 +73,13 @@
};
const MockXosModelStore = {
query: jasmine.createSpy('modelStore.query')
- .and.callFake(() => {
+ .and.callFake((model) => {
const list = new BehaviorSubject([]);
list.next([]);
+ window.setTimeout(() => {
+ // NOTE force the Observable to call next two times, as cacheModelEntries is not caching the first result (since it's generated)
+ list.next([]);
+ }, 100);
return list.asObservable();
})
};
@@ -165,6 +169,22 @@
expect(service['getParentStateFromModel']({name: 'Tenant', app: 'services.vsg'})).toBe('xos.vsg');
});
+ it('should add an item to navigation', () => {
+ service['addNavItem']({name: 'Tenant', app: 'services.vsg'});
+ expect(MockXosNavigationService.add).toHaveBeenCalledWith({
+ label: 'Tenants',
+ state: 'xos.vsg.tenant',
+ parent: 'xos.vsg'
+ });
+
+ service['addNavItem']({name: 'Tenant', verbose_name: 'Verbose', app: 'services.vsg'});
+ expect(MockXosNavigationService.add).toHaveBeenCalledWith({
+ label: 'Verboses',
+ state: 'xos.vsg.tenant',
+ parent: 'xos.vsg'
+ });
+ });
+
it('should add a new service entry in the system', () => {
service['addService']({name: 'Tenant', app: 'services.vsg'});
expect(MockXosRuntimeStates.addState).toHaveBeenCalledWith('xos.vsg', {
@@ -226,22 +246,6 @@
scope.$apply();
});
- it('should add an item to navigation', () => {
- service['addNavItem']({name: 'Tenant', app: 'services.vsg'});
- expect(MockXosNavigationService.add).toHaveBeenCalledWith({
- label: 'Tenants',
- state: 'xos.vsg.tenant',
- parent: 'xos.vsg'
- });
-
- service['addNavItem']({name: 'Tenant', verbose_name: 'Verbose', app: 'services.vsg'});
- expect(MockXosNavigationService.add).toHaveBeenCalledWith({
- label: 'Verboses',
- state: 'xos.vsg.tenant',
- parent: 'xos.vsg'
- });
- });
-
it('should cache a model', () => {
service['cacheModelEntries']({name: 'Tenant', app: 'services.vsg'});
expect(MockXosModelStore.query).toHaveBeenCalledWith('Tenant', '/vsg/tenants');
@@ -276,7 +280,7 @@
service.discover()
.then((res) => {
expect(MockProgressBar.start).toHaveBeenCalled();
- expect(MockXosModelDefs.get).toHaveBeenCalled(); // FIXME replace correct spy
+ expect(MockXosModelDefs.get).toHaveBeenCalled();
expect(service['cacheModelEntries'].calls.count()).toBe(2);
expect(service['addState'].calls.count()).toBe(2);
expect(service['addNavItem'].calls.count()).toBe(2);
@@ -287,6 +291,10 @@
done();
});
scope.$apply();
+ window.setTimeout(() => {
+ // resolve promises after the observable.next as been called twice (cacheModelEntries requires it)
+ scope.$apply();
+ }, 101);
});
});
});
diff --git a/src/app/datasources/rest/auth.rest.spec.ts b/src/app/datasources/rest/auth.rest.spec.ts
index 8ba77d3..044cf88 100644
--- a/src/app/datasources/rest/auth.rest.spec.ts
+++ b/src/app/datasources/rest/auth.rest.spec.ts
@@ -106,36 +106,36 @@
});
});
- describe('the handleUnauthenticatedRequest method', () => {
+ describe('the isAuthError method', () => {
beforeEach(() => {
spyOn(service, 'clearUser');
});
it('should logout the user and redirect to login', () => {
- service.handleUnauthenticatedRequest({
+ let res = service.isAuthError({
error: 'XOSPermissionDenied',
fields: {},
specific_error: 'test'
});
- expect(service.clearUser).toHaveBeenCalled();
+ expect(res).toBeTruthy();
});
it('should catch errors from strings', () => {
- service.handleUnauthenticatedRequest('{"fields": {}, "specific_error": "failed to authenticate token g09et150o2s25kdzg8t2n9wotvds9jyl", "error": "XOSPermissionDenied"}');
- expect(service.clearUser).toHaveBeenCalled();
+ let res = service.isAuthError('{"fields": {}, "specific_error": "failed to authenticate token g09et150o2s25kdzg8t2n9wotvds9jyl", "error": "XOSPermissionDenied"}');
+ expect(res).toBeTruthy();
});
it('should not catch other errors', () => {
- service.handleUnauthenticatedRequest({
+ let res = service.isAuthError({
error: 'XOSProgrammingError',
fields: {},
specific_error: 'test'
});
- expect(service.clearUser).not.toHaveBeenCalled();
+ expect(res).toBeFalsy();
- service.handleUnauthenticatedRequest('some error');
- expect(service.clearUser).not.toHaveBeenCalled();
+ res = service.isAuthError('some error');
+ expect(res).toBeFalsy();
});
});
});
diff --git a/src/app/datasources/rest/auth.rest.ts b/src/app/datasources/rest/auth.rest.ts
index 2bc2bbc..0c202b0 100644
--- a/src/app/datasources/rest/auth.rest.ts
+++ b/src/app/datasources/rest/auth.rest.ts
@@ -46,7 +46,7 @@
getUser(): any; // NOTE how to define return user || false ???
isAuthenticated(): boolean;
clearUser(): void;
- handleUnauthenticatedRequest(error: IXosRestError | string): void;
+ isAuthError(error: IXosRestError | string): boolean;
}
export class AuthService {
@@ -111,7 +111,7 @@
return angular.isDefined(session);
}
- public handleUnauthenticatedRequest(res: IXosRestError | string): void {
+ public isAuthError(res: IXosRestError | string): boolean {
let err;
if (angular.isString(res)) {
try {
@@ -129,9 +129,7 @@
if (err && err.error) {
switch (err.error) {
case 'XOSPermissionDenied':
- this.clearUser();
- this.$state.go('login');
- break;
+ return true;
}
}
}
diff --git a/src/app/datasources/rest/modeldefs.rest.ts b/src/app/datasources/rest/modeldefs.rest.ts
index 60fc917..38ffb16 100644
--- a/src/app/datasources/rest/modeldefs.rest.ts
+++ b/src/app/datasources/rest/modeldefs.rest.ts
@@ -18,6 +18,7 @@
import {IXosModelDefsField} from '../../core/services/helpers/config.helpers';
import {IXosAppConfig} from '../../../index';
+import IPromise = angular.IPromise;
export interface IXosModelDefsRelation {
model: string; // model name
@@ -35,7 +36,7 @@
}
export interface IXosModeldefsService {
- get(): Promise<IXosModeldef[]>;
+ get(): IPromise<IXosModeldef[]>;
}
export class XosModeldefsService implements IXosModeldefsService {
@@ -49,9 +50,9 @@
) {
}
- public get(): Promise<any> {
+ public get(): IPromise<IXosModeldef[]> {
const d = this.$q.defer();
- this.$http.get(`${this.AppConfig.apiEndpoint}/modeldefs`)
+ this.$http.get(`${this.AppConfig.apiEndpoint}/modeldefs`, {timeout: 5 * 1000})
.then((res: any) => {
d.resolve(res.data.items);
})
diff --git a/src/app/datasources/stores/model.store.ts b/src/app/datasources/stores/model.store.ts
index 4f430e6..b26c262 100644
--- a/src/app/datasources/stores/model.store.ts
+++ b/src/app/datasources/stores/model.store.ts
@@ -29,6 +29,7 @@
query(model: string, apiUrl?: string): Observable<any>;
get(model: string, id: string | number): Observable<any>;
search(modelName: string): any[];
+ clean(): void;
}
export class XosModelStore implements IXosModelStoreService {
@@ -54,6 +55,10 @@
this.efficientNext = this.XosDebouncer.debounce(this.next, 500, this, false);
}
+ public clean() {
+ this._collections = {};
+ }
+
public query(modelName: string, apiUrl?: string): Observable<any> {
this.$log.debug(`[XosModelStore] QUERY: ${modelName}`);
// if there isn't already an observable for that item
diff --git a/src/interceptors.ts b/src/interceptors.ts
index 7473226..616063b 100644
--- a/src/interceptors.ts
+++ b/src/interceptors.ts
@@ -26,13 +26,17 @@
export function userStatusInterceptor($state: angular.ui.IStateService, $cookies: ng.cookies.ICookiesService, $q: ng.IQService) {
const checkLogin = (res) => {
+
+ // NOTE canceled request (as the modeldefs one) have no res
+ if (angular.isUndefined(res) || res === null) {
+ return $q.reject(res);
+ }
+
// NOTE this interceptor may never be called as the request is not rejected byt the "model-discoverer" service
switch (res.status) {
- case -1:
case 401:
case 403:
$cookies.remove('sessionid', {path: '/'});
- $state.go('login');
return $q.reject(res);
default:
return $q.reject(res);