Merge pull request #69958 from rhcs-dashboard/fix-access-denied-issue

Reviewed-by: Afreen Misbah <afreen@ibm.com>

qa/tasks/mgr/dashboard/test_host.py

@@ -30,7 +30,7 @@ class HostControllerTest(DashboardTestCase):
     def test_data_daemons(self):
         return self.ORCHESTRATOR_TEST_DATA['daemons']
 
-    @DashboardTestCase.RunAs('test', 'test', ['block-manager'])
+    @DashboardTestCase.RunAs('test', 'test', ['rgw-manager'])
     def test_access_permissions(self):
         self._get(self.URL_HOST, version='1.1')
         self.assertStatus(403)

src/pybind/mgr/dashboard/controllers/health.py

@@ -395,7 +395,8 @@ class Health(BaseController):
                 'quorum': data.get('quorum', {})
             }
 
-        if self._has_permissions(Permission.READ, Scope.OSD):
+        if (self._has_permissions(Permission.READ, Scope.OSD)
+                or self._has_permissions(Permission.READ, Scope.POOL)):
             summary['osdmap'] = {
                 'in': data.get('osdmap', {}).get('num_in_osds'),
                 'up': data.get('osdmap', {}).get('num_up_osds'),

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/prometheus/prometheus-tabs/prometheus-tabs.component.html

@@ -24,17 +24,19 @@
       ></a
     >
   </ng-container>
-  <ng-container ngbNavItem>
-    <a
-      class="nav-link"
-      routerLink="/monitoring/silences"
-      routerLinkActive="active"
-      ariaCurrentWhenActive="page"
-      [routerLinkActiveOptions]="{ exact: true }"
-      i18n
-      >Silences</a
-    >
-  </ng-container>
+  @if (prometheusPermissions.read) {
+    <ng-container ngbNavItem>
+      <a
+        class="nav-link"
+        routerLink="/monitoring/silences"
+        routerLinkActive="active"
+        ariaCurrentWhenActive="page"
+        [routerLinkActiveOptions]="{ exact: true }"
+        i18n
+        >Silences</a
+      >
+    </ng-container>
+  }
   <ng-container ngbNavItem>
     <a
       class="nav-link"

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/prometheus/prometheus-tabs/prometheus-tabs.component.spec.ts

@@ -4,6 +4,8 @@ import { RouterTestingModule } from '@angular/router/testing';
 import { NgbNavModule } from '@ng-bootstrap/ng-bootstrap';
 
 import { PrometheusAlertService } from '~/app/shared/services/prometheus-alert.service';
+import { AuthStorageService } from '~/app/shared/services/auth-storage.service';
+import { Permission } from '~/app/shared/models/permissions';
 import { configureTestBed } from '~/testing/unit-test-helper';
 import { PrometheusTabsComponent } from './prometheus-tabs.component';
 
@@ -14,7 +16,17 @@ describe('PrometheusTabsComponent', () => {
   configureTestBed({
     imports: [RouterTestingModule, NgbNavModule],
     declarations: [PrometheusTabsComponent],
-    providers: [{ provide: PrometheusAlertService, useValue: { alerts: [] } }]
+    providers: [
+      { provide: PrometheusAlertService, useValue: { alerts: [] } },
+      {
+        provide: AuthStorageService,
+        useValue: {
+          getPermissions: () => ({
+            prometheus: new Permission(['read'])
+          })
+        }
+      }
+    ]
   });
 
   beforeEach(() => {
@@ -26,4 +38,8 @@ describe('PrometheusTabsComponent', () => {
   it('should create', () => {
     expect(component).toBeTruthy();
   });
+
+  it('should show silences to users with read access', () => {
+    expect(component.prometheusPermissions.read).toBe(true);
+  });
 });

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/prometheus/prometheus-tabs/prometheus-tabs.component.ts

@@ -1,6 +1,8 @@
 import { Component } from '@angular/core';
 
 import { PrometheusAlertService } from '~/app/shared/services/prometheus-alert.service';
+import { AuthStorageService } from '~/app/shared/services/auth-storage.service';
+import { Permission } from '~/app/shared/models/permissions';
 
 @Component({
   selector: 'cd-prometheus-tabs',
@@ -9,5 +11,12 @@ import { PrometheusAlertService } from '~/app/shared/services/prometheus-alert.s
   standalone: false
 })
 export class PrometheusTabsComponent {
-  constructor(public prometheusAlertService: PrometheusAlertService) {}
+  prometheusPermissions: Permission;
+
+  constructor(
+    public prometheusAlertService: PrometheusAlertService,
+    private authStorageService: AuthStorageService
+  ) {
+    this.prometheusPermissions = this.authStorageService.getPermissions().prometheus;
+  }
 }

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/prometheus/silence-list/silence-list.component.spec.ts

@@ -54,6 +54,13 @@ describe('SilenceListComponent', () => {
     expect(component).toBeTruthy();
   });
 
+  it('should create for read-only prometheus users', () => {
+    (authStorageService.getPermissions as jasmine.Spy).and.callFake(() => ({
+      prometheus: new Permission(['read'])
+    }));
+    expect(() => TestBed.createComponent(SilenceListComponent)).not.toThrow();
+  });
+
   it('should test all TableActions combinations', () => {
     const permissionHelper: PermissionHelper = new PermissionHelper(component.permission);
     const tableActions: TableActionsComponent = permissionHelper.setPermissionsAndGetActions(

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/prometheus/silence-list/silence-list.component.ts

@@ -4,7 +4,6 @@ import { NgbModalRef } from '@ng-bootstrap/ng-bootstrap';
 import { Observable, Subscriber } from 'rxjs';
 
 import { PrometheusListHelper } from '~/app/shared/helpers/prometheus-list-helper';
-import { SilenceFormComponent } from '~/app/ceph/cluster/prometheus/silence-form/silence-form.component';
 import { PrometheusService } from '~/app/shared/api/prometheus.service';
 import { DeleteConfirmationModalComponent } from '~/app/shared/components/delete-confirmation-modal/delete-confirmation-modal.component';
 import { ActionLabelsI18n, SucceededActionLabelsI18n } from '~/app/shared/constants/app.constants';
@@ -29,10 +28,7 @@ import { CdSortPropDir } from '~/app/shared/models/cd-sort-prop-dir';
 const BASE_URL = 'monitoring/silences';
 
 @Component({
-  providers: [
-    { provide: URLBuilderService, useValue: new URLBuilderService(BASE_URL) },
-    SilenceFormComponent
-  ],
+  providers: [{ provide: URLBuilderService, useValue: new URLBuilderService(BASE_URL) }],
   selector: 'cd-silences-list',
   templateUrl: './silence-list.component.html',
   styleUrls: ['./silence-list.component.scss'],
@@ -62,7 +58,6 @@ export class SilenceListComponent extends PrometheusListHelper {
     private urlBuilder: URLBuilderService,
     private actionLabels: ActionLabelsI18n,
     private succeededLabels: SucceededActionLabelsI18n,
-    private silenceFormComponent: SilenceFormComponent,
     private silenceMatcher: PrometheusSilenceMatcherService,
     @Inject(PrometheusService) prometheusService: PrometheusService
   ) {
@@ -172,7 +167,7 @@ export class SilenceListComponent extends PrometheusListHelper {
           const activeSilences = silences.filter(
             (silence: AlertmanagerSilence) => silence.status.state !== 'expired'
           );
-          this.getAlerts(activeSilences);
+          this.loadRulesAndMatchAlerts(activeSilences);
         },
         () => {
           this.prometheusService.disableAlertmanagerConfig();
@@ -185,13 +180,32 @@ export class SilenceListComponent extends PrometheusListHelper {
     this.selection = selection;
   }
 
-  getAlerts(silences: any) {
-    const rules = this.silenceFormComponent.getRules();
-    silences.forEach((silence: any) => {
-      silence.matchers.forEach((matcher: any) => {
-        this.rules = this.silenceMatcher.getMatchedRules(matcher, rules);
+  private loadRulesAndMatchAlerts(silences: AlertmanagerSilence[]) {
+    this.prometheusService.ifPrometheusConfigured(
+      () =>
+        this.prometheusService.getRules().subscribe(
+          (groups) => {
+            this.rules = groups.groups.flatMap((group) => group.rules);
+            this.getAlerts(silences);
+          },
+          () => {
+            this.rules = [];
+            this.getAlerts(silences);
+          }
+        ),
+      () => {
+        this.rules = [];
+        this.getAlerts(silences);
+      }
+    );
+  }
+
+  getAlerts(silences: AlertmanagerSilence[]) {
+    silences.forEach((silence) => {
+      silence.matchers.forEach((matcher) => {
+        const matchedRules = this.silenceMatcher.getMatchedRules(matcher, this.rules);
         const alertNames: string[] = [];
-        for (const rule of this.rules) {
+        for (const rule of matchedRules) {
           alertNames.push(rule.name);
         }
         silence.silencedAlerts = alertNames;

src/pybind/mgr/dashboard/frontend/src/app/ceph/overview/overview.component.ts

@@ -99,7 +99,12 @@ export class OverviewComponent {
   );
 
   readonly hasNoOSDs$ = this.healthData$.pipe(
-    map((data: HealthSnapshotMap) => (data?.osdmap?.num_osds ?? 0) === 0),
+    map((data: HealthSnapshotMap) => {
+      if (data?.osdmap == null) {
+        return false;
+      }
+      return data.osdmap.num_osds === 0;
+    }),
     shareReplay({ bufferSize: 1, refCount: true })
   );
 

src/pybind/mgr/dashboard/frontend/src/app/core/navigation/dashboard-help/dashboard-help.component.html

@@ -36,11 +36,13 @@
     i18n
     >About</cds-overflow-menu-option
   >
-  <cds-overflow-menu-option
-    (click)="openFeedbackModal()"
-    i18n
-    >Report an issue...</cds-overflow-menu-option
-  >
+  @if (configOptPermission.read) {
+    <cds-overflow-menu-option
+      (click)="openFeedbackModal()"
+      i18n
+      >Report an issue...</cds-overflow-menu-option
+    >
+  }
 </cds-overflow-menu>
 
 <ng-template #customTrigger>

src/pybind/mgr/dashboard/frontend/src/app/core/navigation/dashboard-help/dashboard-help.component.spec.ts

@@ -1,21 +1,29 @@
 import { HttpClientTestingModule } from '@angular/common/http/testing';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { By } from '@angular/platform-browser';
 import { RouterTestingModule } from '@angular/router/testing';
 
 import { SharedModule } from '~/app/shared/shared.module';
+import { AuthStorageService } from '~/app/shared/services/auth-storage.service';
+import { Permission, Permissions } from '~/app/shared/models/permissions';
 import { configureTestBed } from '~/testing/unit-test-helper';
 import { DashboardHelpComponent } from './dashboard-help.component';
 
 describe('DashboardHelpComponent', () => {
   let component: DashboardHelpComponent;
   let fixture: ComponentFixture<DashboardHelpComponent>;
+  let permissions: Permissions;
 
   configureTestBed({
     imports: [HttpClientTestingModule, SharedModule, RouterTestingModule],
-    declarations: [DashboardHelpComponent]
+    declarations: [DashboardHelpComponent],
+    providers: [AuthStorageService]
   });
 
   beforeEach(() => {
+    permissions = new Permissions({});
+    permissions.configOpt = new Permission(['read']);
+    spyOn(TestBed.inject(AuthStorageService), 'getPermissions').and.returnValue(permissions);
     fixture = TestBed.createComponent(DashboardHelpComponent);
     component = fixture.componentInstance;
     fixture.detectChanges();
@@ -24,4 +32,28 @@ describe('DashboardHelpComponent', () => {
   it('should create', () => {
     expect(component).toBeTruthy();
   });
+
+  it('should show report issue when config-opt is readable', () => {
+    const options = fixture.debugElement.queryAll(By.css('cds-overflow-menu-option'));
+    // About + Report an issue (Documentation and API are plain <li> links)
+    expect(options.length).toBe(2);
+    expect(options.map((o) => o.nativeElement.textContent.trim())).toEqual([
+      'About',
+      'Report an issue...'
+    ]);
+  });
+
+  it('should hide report issue when config-opt is not readable', () => {
+    permissions.configOpt = new Permission([]);
+    (TestBed.inject(AuthStorageService).getPermissions as jasmine.Spy).and.returnValue(permissions);
+
+    fixture = TestBed.createComponent(DashboardHelpComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+
+    const options = fixture.debugElement.queryAll(By.css('cds-overflow-menu-option'));
+    // Only About remains; Documentation and API are plain <li> links
+    expect(options.length).toBe(1);
+    expect(options[0].nativeElement.textContent.trim()).toBe('About');
+  });
 });

src/pybind/mgr/dashboard/frontend/src/app/core/navigation/dashboard-help/dashboard-help.component.ts

@@ -1,6 +1,8 @@
 import { Component, OnInit } from '@angular/core';
 
 import { Icons } from '~/app/shared/enum/icons.enum';
+import { Permission } from '~/app/shared/models/permissions';
+import { AuthStorageService } from '~/app/shared/services/auth-storage.service';
 import { DocService } from '~/app/shared/services/doc.service';
 
 import { AboutComponent } from '../about/about.component';
@@ -16,11 +18,15 @@ import { FeedbackComponent } from '~/app/ceph/shared/feedback/feedback.component
 export class DashboardHelpComponent implements OnInit {
   docsUrl: string;
   icons = Icons;
+  configOptPermission: Permission;
 
   constructor(
     private docService: DocService,
-    private modalCdsService: ModalCdsService
-  ) {}
+    private modalCdsService: ModalCdsService,
+    private authStorageService: AuthStorageService
+  ) {
+    this.configOptPermission = this.authStorageService.getPermissions().configOpt;
+  }
 
   ngOnInit() {
     this.docService.subscribeOnce('dashboard', (url: string) => {

src/pybind/mgr/dashboard/frontend/src/app/shared/models/alertmanager-silence.ts

@@ -1,5 +1,3 @@
-import { PrometheusRule } from './prometheus-alerts';
-
 export class AlertmanagerSilenceMatcher {
   name: string;
   value: any;
@@ -22,5 +20,5 @@ export class AlertmanagerSilence {
   status?: {
     state: 'expired' | 'active' | 'pending';
   };
-  silencedAlerts?: PrometheusRule[];
+  silencedAlerts?: string[];
 }

src/pybind/mgr/dashboard/services/access_control.py

@@ -218,9 +218,10 @@ ADMIN_ROLE = Role(
 # read-only role provides read-only permission for all scopes
 READ_ONLY_ROLE = Role(
     'read-only',
-    'allows read permission for all security scope except dashboard settings and config-opt', {
+    'allows read permission for all security scope except user, '
+    'dashboard settings and config-opt', {
         scope_name: [_P.READ] for scope_name in Scope.all_scopes()
-        if scope_name not in (Scope.DASHBOARD_SETTINGS, Scope.CONFIG_OPT)
+        if scope_name not in (Scope.USER, Scope.DASHBOARD_SETTINGS, Scope.CONFIG_OPT)
     })
 
 
@@ -231,6 +232,7 @@ BLOCK_MGR_ROLE = Role(
         Scope.POOL: [_P.READ],
         Scope.ISCSI: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
         Scope.RBD_MIRRORING: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
+        Scope.HOSTS: [_P.READ],
         Scope.GRAFANA: [_P.READ],
         Scope.NVME_OF: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
         Scope.PROMETHEUS: [_P.READ]
@@ -253,6 +255,7 @@ CLUSTER_MGR_ROLE = Role(
     and config-opt scopes""", {
         Scope.HOSTS: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
         Scope.OSD: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
+        Scope.POOL: [_P.READ],
         Scope.MONITOR: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
         Scope.MANAGER: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
         Scope.CONFIG_OPT: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
@@ -291,6 +294,8 @@ GANESHA_MGR_ROLE = Role(
 SMB_MGR_ROLE = Role(
     'smb-manager', 'allows full permissions for the smb scope', {
         Scope.SMB: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
+        Scope.HOSTS: [_P.READ],
+        Scope.POOL: [_P.READ],
         Scope.CEPHFS: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
         Scope.RGW: [_P.READ, _P.CREATE, _P.UPDATE, _P.DELETE],
         Scope.GRAFANA: [_P.READ],

src/pybind/mgr/dashboard/tests/test_access_control.py

@@ -156,9 +156,30 @@ class AccessControlTest(unittest.TestCase, CLICommandTestMixin):
         self.assertEqual(role['name'], 'read-only')
         self.assertEqual(
             role['description'],
-            'allows read permission for all security scope except dashboard settings and config-opt'
+            'allows read permission for all security scope except user, '
+            'dashboard settings and config-opt'
         )
 
+    def test_cluster_manager_role_has_pool_read(self):
+        role = self.exec_cmd('ac-role-show', rolename='cluster-manager')
+        self.assertEqual(role['scopes_permissions'][Scope.POOL], [Permission.READ])
+
+    def test_smb_manager_role_has_hosts_read(self):
+        role = self.exec_cmd('ac-role-show', rolename='smb-manager')
+        self.assertEqual(role['scopes_permissions'][Scope.HOSTS], [Permission.READ])
+
+    def test_smb_manager_role_has_pool_read(self):
+        role = self.exec_cmd('ac-role-show', rolename='smb-manager')
+        self.assertEqual(role['scopes_permissions'][Scope.POOL], [Permission.READ])
+
+    def test_block_manager_role_has_pool_read(self):
+        role = self.exec_cmd('ac-role-show', rolename='block-manager')
+        self.assertEqual(role['scopes_permissions'][Scope.POOL], [Permission.READ])
+
+    def test_block_manager_role_has_hosts_read(self):
+        role = self.exec_cmd('ac-role-show', rolename='block-manager')
+        self.assertEqual(role['scopes_permissions'][Scope.HOSTS], [Permission.READ])
+
     def test_delete_system_role(self):
         with self.assertRaises(CmdException) as ctx:
             self.exec_cmd('ac-role-delete', rolename='administrator')