From 35a3e6d8edd97ae6ff237e3e39f9edefc4d48cbf Mon Sep 17 00:00:00 2001 From: Nizamudeen A Date: Sat, 16 Mar 2024 16:54:22 +0530 Subject: [PATCH] mgr/dashboard: securely store remote cluster token Instead of using the localStorage use cookies for storing the token more securely Fixes: https://tracker.ceph.com/issues/64958 Signed-off-by: Nizamudeen A --- .../mgr/dashboard/frontend/package-lock.json | 18 ++++++++++++++++ .../mgr/dashboard/frontend/package.json | 1 + .../multi-cluster-list.component.ts | 5 ++++- .../navigation/navigation.component.ts | 16 ++++++++++---- .../services/api-interceptor.service.ts | 7 ++++--- .../shared/services/cookie.service.spec.ts | 16 ++++++++++++++ .../src/app/shared/services/cookie.service.ts | 21 +++++++++++++++++++ 7 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/services/cookie.service.spec.ts create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/services/cookie.service.ts diff --git a/src/pybind/mgr/dashboard/frontend/package-lock.json b/src/pybind/mgr/dashboard/frontend/package-lock.json index 8e6a9c4018fdf..1af58a5101ce7 100644 --- a/src/pybind/mgr/dashboard/frontend/package-lock.json +++ b/src/pybind/mgr/dashboard/frontend/package-lock.json @@ -38,6 +38,7 @@ "ng-block-ui": "3.0.2", "ng-click-outside": "7.0.0", "ng2-charts": "4.1.1", + "ngx-cookie-service": "17.1.0", "ngx-pipe-function": "1.0.0", "ngx-toastr": "17.0.2", "rxjs": "6.6.3", @@ -25048,6 +25049,23 @@ "rxjs": "^6.5.3 || ^7.4.0" } }, + "node_modules/ngx-cookie-service": { + "version": "17.1.0", + "resolved": "https://registry.npmjs.org/ngx-cookie-service/-/ngx-cookie-service-17.1.0.tgz", + "integrity": "sha512-m4YI9IEgTaEBDMCz7oeVsO6UX14EmCzg29cTL6yxW8f7oye9wv56egi+3C4wAVSRPkI+cWlqnIOr+XyHwYQYmg==", + "dependencies": { + "tslib": "^2.6.2" + }, + "peerDependencies": { + "@angular/common": "^17.0.0", + "@angular/core": "^17.0.0" + } + }, + "node_modules/ngx-cookie-service/node_modules/tslib": { + "version": "2.6.2", + "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.6.2.tgz", + "integrity": "sha512-AEYxH93jGFPn/a2iVAwW87VuUIkR1FVUKB77NwMF7nBTDkDrrT/Hpt/IrCJ0QXhW27jTBDcf5ZY7w6RiqTMw2Q==" + }, "node_modules/ngx-pipe-function": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/ngx-pipe-function/-/ngx-pipe-function-1.0.0.tgz", diff --git a/src/pybind/mgr/dashboard/frontend/package.json b/src/pybind/mgr/dashboard/frontend/package.json index fd4f7a0bff6b7..f72dbdbe4125b 100644 --- a/src/pybind/mgr/dashboard/frontend/package.json +++ b/src/pybind/mgr/dashboard/frontend/package.json @@ -72,6 +72,7 @@ "ng-block-ui": "3.0.2", "ng-click-outside": "7.0.0", "ng2-charts": "4.1.1", + "ngx-cookie-service": "17.1.0", "ngx-pipe-function": "1.0.0", "ngx-toastr": "17.0.2", "rxjs": "6.6.3", diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/multi-cluster/multi-cluster-list/multi-cluster-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/multi-cluster/multi-cluster-list/multi-cluster-list.component.ts index 4496b3a34ceb1..07238019c1997 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/multi-cluster/multi-cluster-list/multi-cluster-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/multi-cluster/multi-cluster-list/multi-cluster-list.component.ts @@ -18,6 +18,7 @@ import { CellTemplate } from '~/app/shared/enum/cell-template.enum'; import { MultiCluster } from '~/app/shared/models/multi-cluster'; import { SummaryService } from '~/app/shared/services/summary.service'; import { Router } from '@angular/router'; +import { CookiesService } from '~/app/shared/services/cookie.service'; @Component({ selector: 'cd-multi-cluster-list', @@ -48,7 +49,8 @@ export class MultiClusterListComponent { public actionLabels: ActionLabelsI18n, private notificationService: NotificationService, private authStorageService: AuthStorageService, - private modalService: ModalService + private modalService: ModalService, + private cookieService: CookiesService ) { this.tableActions = [ { @@ -189,6 +191,7 @@ export class MultiClusterListComponent { itemNames: [cluster['cluster_alias'] + ' - ' + cluster['user']], submitAction: () => this.multiClusterService.deleteCluster(cluster['name'], cluster['user']).subscribe(() => { + this.cookieService.deleteToken(`${cluster['name']}-${cluster['user']}`); this.modalRef.close(); this.notificationService.show( NotificationType.success, diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.ts index 4ae8d1897e27e..8973843872bc9 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.ts @@ -9,6 +9,7 @@ import { Icons } from '~/app/shared/enum/icons.enum'; import { MultiCluster } from '~/app/shared/models/multi-cluster'; import { Permissions } from '~/app/shared/models/permissions'; import { AuthStorageService } from '~/app/shared/services/auth-storage.service'; +import { CookiesService } from '~/app/shared/services/cookie.service'; import { FeatureTogglesMap$, FeatureTogglesService @@ -56,7 +57,8 @@ export class NavigationComponent implements OnInit, OnDestroy { private featureToggles: FeatureTogglesService, private telemetryNotificationService: TelemetryNotificationService, public prometheusAlertService: PrometheusAlertService, - private motdNotificationService: MotdNotificationService + private motdNotificationService: MotdNotificationService, + private cookieService: CookiesService ) { this.permissions = this.authStorageService.getPermissions(); this.enabledFeature$ = this.featureToggles.get(); @@ -178,7 +180,12 @@ export class NavigationComponent implements OnInit, OnDestroy { onClusterSelection(value: object) { this.multiClusterService.setCluster(value).subscribe( (resp: any) => { - localStorage.setItem('cluster_api_url', value['url']); + if (value['cluster_alias'] === 'local-cluster') { + localStorage.setItem('cluster_api_url', ''); + } else { + localStorage.setItem('current_cluster_name', `${value['name']}-${value['user']}`); + localStorage.setItem('cluster_api_url', value['url']); + } this.selectedCluster = this.clustersMap.get(`${value['url']}-${value['user']}`) || {}; const clustersConfig = resp['config']; if (clustersConfig && typeof clustersConfig === 'object') { @@ -192,9 +199,10 @@ export class NavigationComponent implements OnInit, OnDestroy { if ( clusterName === this.selectedCluster['name'] && - clusterUser === this.selectedCluster['user'] + clusterUser === this.selectedCluster['user'] && + clusterDetails['cluster_alias'] !== 'local-cluster' ) { - localStorage.setItem('token_of_selected_cluster', clusterToken); + this.cookieService.setToken(`${clusterName}-${clusterUser}`, clusterToken); } }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts index 5cbd99911fb2d..d1e98487789be 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts @@ -20,6 +20,7 @@ import { NotificationService } from './notification.service'; import { MultiClusterService } from '../api/multi-cluster.service'; import { SummaryService } from './summary.service'; import { AuthStorageService } from './auth-storage.service'; +import { CookiesService } from './cookie.service'; export class CdHttpErrorResponse extends HttpErrorResponse { preventDefault: Function; @@ -37,7 +38,8 @@ export class ApiInterceptorService implements HttpInterceptor { public notificationService: NotificationService, private summaryService: SummaryService, private authStorageService: AuthStorageService, - private multiClusterService: MultiClusterService + private multiClusterService: MultiClusterService, + private cookieService: CookiesService ) { this.multiClusterService.subscribe((resp: any) => { const clustersConfig = resp['config']; @@ -91,14 +93,13 @@ export class ApiInterceptorService implements HttpInterceptor { 'api/multi-cluster/auth' ]; - const token = localStorage.getItem('token_of_selected_cluster'); - if ( !currentRoute.includes('login') && !ALWAYS_TO_HUB_APIs.includes(request.url) && apiUrl && !apiUrl.includes(origin) ) { + const token = this.cookieService.getToken(localStorage.getItem('current_cluster_name')); reqWithVersion = reqWithVersion.clone({ url: `${apiUrl}${reqWithVersion.url}`, setHeaders: { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/cookie.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/cookie.service.spec.ts new file mode 100644 index 0000000000000..49ab3d05ebcdb --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/cookie.service.spec.ts @@ -0,0 +1,16 @@ +import { TestBed } from '@angular/core/testing'; + +import { CookiesService } from './cookie.service'; + +describe('CookieService', () => { + let service: CookiesService; + + beforeEach(() => { + TestBed.configureTestingModule({}); + service = TestBed.inject(CookiesService); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); +}); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/cookie.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/cookie.service.ts new file mode 100644 index 0000000000000..cdbbadeb1cfe0 --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/cookie.service.ts @@ -0,0 +1,21 @@ +import { Injectable } from '@angular/core'; +import { CookieService } from 'ngx-cookie-service'; + +@Injectable({ + providedIn: 'root' +}) +export class CookiesService { + constructor(private cookieService: CookieService) {} + + setToken(name: string, token: string) { + this.cookieService.set(name, token, null, null, null, true, 'Strict'); + } + + getToken(name: string): string { + return this.cookieService.get(name); + } + + deleteToken(name: string) { + this.cookieService.delete(name); + } +}