Add opt-in config to allow read-only users to access Discover and Visualize

DEVELOPER_GUIDE.md

@@ -65,6 +65,12 @@ opensearch.requestHeadersWhitelist: [ authorization,securitytenant ]
 opensearch_security.multitenancy.enabled: true
 opensearch_security.multitenancy.tenants.preferred: ["Private", "Global"]
 opensearch_security.readonly_mode.roles: ["kibana_read_only"]
+# Optional — opt in to letting read-only users access the Discover app.
+# Defaults to false to preserve existing behavior on upgrade.
+# opensearch_security.readonly_mode.allow_discover: true
+# Optional — opt in to letting read-only users access the Visualize app.
+# Defaults to false to preserve existing behavior on upgrade.
+# opensearch_security.readonly_mode.allow_visualize: true
 
 # Use this setting if you are running opensearch-dashboards without https
 opensearch_security.cookie.secure: false

public/plugin.ts

@@ -88,6 +88,20 @@ async function hasApiPermission(core: CoreSetup): Promise<boolean | undefined> {
 const DEFAULT_READONLY_ROLES = ['kibana_read_only'];
 const APP_ID_HOME = 'home';
 const APP_ID_DASHBOARDS = 'dashboards';
+// Modern OSD registers Discover twice, on purpose, and both ids must be
+// allowlisted together to avoid breaking the UI:
+//   - `data-explorer` hosts the real Discover UI at /app/data-explorer/discover.
+//   - `discover` is a thin shim that client-side-redirects to data-explorer
+//     (see src/plugins/discover/public/plugin.ts). Its mount function fires
+//     navigateToApp('data-explorer', ...) — but the AppUpdater status check
+//     runs BEFORE mount, so if `discover` is marked inaccessible the user
+//     sees "Application not found" and the redirect never executes.
+//   - The nav menu's "Discover" entry points at the `discover` id, not
+//     `data-explorer`, so blocking `discover` also breaks the nav link.
+// Dropping either id produces a visibly broken Discover for the read-only user.
+const APP_ID_DISCOVER = 'discover';
+const APP_ID_DATA_EXPLORER = 'data-explorer';
+const APP_ID_VISUALIZE = 'visualize';
 // OpenSearchDashboards app is for legacy url migration
 const APP_ID_OPENSEARCH_DASHBOARDS = 'kibana';
 const APP_LIST_FOR_READONLY_ROLE = [APP_ID_HOME, APP_ID_DASHBOARDS, APP_ID_OPENSEARCH_DASHBOARDS];
@@ -431,9 +445,15 @@ export class SecurityPlugin
       });
     }
 
+    const readonlyAppAllowlist = [
+      ...APP_LIST_FOR_READONLY_ROLE,
+      ...(config.readonly_mode?.allow_discover ? [APP_ID_DISCOVER, APP_ID_DATA_EXPLORER] : []),
+      ...(config.readonly_mode?.allow_visualize ? [APP_ID_VISUALIZE] : []),
+    ];
+
     core.application.registerAppUpdater(
       new BehaviorSubject<AppUpdater>((app) => {
-        if (!apiPermission && isReadonly && !APP_LIST_FOR_READONLY_ROLE.includes(app.id)) {
+        if (!apiPermission && isReadonly && !readonlyAppAllowlist.includes(app.id)) {
           return {
             status: AppStatus.inaccessible,
           };

public/test/plugin.test.ts

@@ -40,6 +40,17 @@ import {
   PLUGIN_USERS_APP_ID,
 } from '../../common/index.ts';
 
+jest.mock('../apps/account/utils', () => ({
+  fetchAccountInfoSafe: jest.fn(),
+}));
+jest.mock('../utils/dashboards-info-utils', () => ({
+  getDashboardsInfoSafe: jest.fn(),
+}));
+// eslint-disable-next-line @typescript-eslint/no-var-requires
+const { fetchAccountInfoSafe } = require('../apps/account/utils');
+// eslint-disable-next-line @typescript-eslint/no-var-requires
+const { getDashboardsInfoSafe } = require('../utils/dashboards-info-utils');
+
 // Mock the hasApiPermission function
 jest.mock('../plugin', () => {
   const originalModule = jest.requireActual('../plugin');
@@ -153,6 +164,142 @@ describe('SecurityPlugin', () => {
     });
   });
 
+  describe('readonly app allowlist — opt-in flags', () => {
+    const makeContext = (flags: { allow_discover?: boolean; allow_visualize?: boolean }) => ({
+      config: {
+        get: jest.fn().mockReturnValue({
+          readonly_mode: {
+            roles: ['kibana_read_only'],
+            ...flags,
+          },
+          multitenancy: { enabled: true, enable_aggregation_view: false },
+          clusterPermissions: { include: [] },
+          indexPermissions: { include: [] },
+          disabledTransportCategories: { exclude: [] },
+          disabledRestCategories: { exclude: [] },
+          ui: { autologout: false },
+        }),
+      },
+    });
+
+    const runAndGetUpdater = async (flags: {
+      allow_discover?: boolean;
+      allow_visualize?: boolean;
+    }) => {
+      // Force hasApiPermission() to resolve false so the readonly updater engages.
+      coreSetup.http.get = jest.fn().mockResolvedValue({ has_api_access: false });
+      (fetchAccountInfoSafe as jest.Mock).mockResolvedValue({
+        data: { roles: ['kibana_read_only'] },
+      });
+      (getDashboardsInfoSafe as jest.Mock).mockResolvedValue({
+        multitenancy_enabled: true,
+      });
+      coreSetup.chrome.navGroup = {
+        ...coreSetup.chrome.navGroup,
+        getNavGroupEnabled: () => false,
+      };
+
+      const registerAppUpdaterSpy = jest.spyOn(coreSetup.application, 'registerAppUpdater');
+      const callsBefore = registerAppUpdaterSpy.mock.calls.length;
+      plugin = new SecurityPlugin(makeContext(flags));
+      await plugin.setup(coreSetup, deps);
+
+      // Pick the subject registered during THIS setup — coreSetup is reused across
+      // runAndGetUpdater() calls within a single test, so spy.mock.calls accumulates.
+      const subject = registerAppUpdaterSpy.mock.calls[callsBefore][0];
+      return subject.getValue();
+    };
+
+    // The updater returns an object with a `status` field (AppStatus.inaccessible)
+    // when blocking an app, and `undefined` when allowing. Assert on shape so this
+    // test file does not need to import from `src/core/public` (which pulls in
+    // monaco-editor and fails under jsdom at module-load time).
+    const expectBlocked = (result: unknown) => {
+      expect(result).toEqual(expect.objectContaining({ status: expect.anything() }));
+    };
+
+    describe('allow_discover flag', () => {
+      it('blocks Discover (both ids) when the flag is unset (default off)', async () => {
+        const updater = await runAndGetUpdater({});
+        expectBlocked(updater({ id: 'discover' }));
+        expectBlocked(updater({ id: 'data-explorer' }));
+      });
+
+      it('blocks Discover (both ids) when the flag is explicitly false', async () => {
+        const updater = await runAndGetUpdater({ allow_discover: false });
+        expectBlocked(updater({ id: 'discover' }));
+        expectBlocked(updater({ id: 'data-explorer' }));
+      });
+
+      it('allows Discover (both legacy and data-explorer ids) when the flag is true', async () => {
+        const updater = await runAndGetUpdater({ allow_discover: true });
+        expect(updater({ id: 'discover' })).toBeUndefined();
+        expect(updater({ id: 'data-explorer' })).toBeUndefined();
+      });
+
+      it('still blocks other non-allowlisted apps when the flag is true', async () => {
+        const updater = await runAndGetUpdater({ allow_discover: true });
+        expectBlocked(updater({ id: 'visualize' }));
+        expectBlocked(updater({ id: 'management' }));
+      });
+    });
+
+    describe('allow_visualize flag', () => {
+      it('blocks Visualize when the flag is unset (default off)', async () => {
+        const updater = await runAndGetUpdater({});
+        expectBlocked(updater({ id: 'visualize' }));
+      });
+
+      it('blocks Visualize when the flag is explicitly false', async () => {
+        const updater = await runAndGetUpdater({ allow_visualize: false });
+        expectBlocked(updater({ id: 'visualize' }));
+      });
+
+      it('allows Visualize when the flag is true', async () => {
+        const updater = await runAndGetUpdater({ allow_visualize: true });
+        expect(updater({ id: 'visualize' })).toBeUndefined();
+      });
+
+      it('still blocks other non-allowlisted apps when the flag is true', async () => {
+        const updater = await runAndGetUpdater({ allow_visualize: true });
+        expectBlocked(updater({ id: 'discover' }));
+        expectBlocked(updater({ id: 'data-explorer' }));
+        expectBlocked(updater({ id: 'management' }));
+      });
+
+      it('does not affect Discover gating (flags are independent)', async () => {
+        const discoverOnly = await runAndGetUpdater({ allow_discover: true });
+        expectBlocked(discoverOnly({ id: 'visualize' }));
+
+        const visualizeOnly = await runAndGetUpdater({ allow_visualize: true });
+        expectBlocked(visualizeOnly({ id: 'discover' }));
+      });
+
+      it('allows both apps when both flags are true', async () => {
+        const updater = await runAndGetUpdater({ allow_discover: true, allow_visualize: true });
+        expect(updater({ id: 'discover' })).toBeUndefined();
+        expect(updater({ id: 'data-explorer' })).toBeUndefined();
+        expect(updater({ id: 'visualize' })).toBeUndefined();
+      });
+    });
+
+    it('keeps baseline allowlist members (home, dashboards) accessible regardless of flags', async () => {
+      const combos = [
+        {},
+        { allow_discover: false },
+        { allow_discover: true },
+        { allow_visualize: false },
+        { allow_visualize: true },
+        { allow_discover: true, allow_visualize: true },
+      ];
+      for (const flags of combos) {
+        const updater = await runAndGetUpdater(flags);
+        expect(updater({ id: 'home' })).toBeUndefined();
+        expect(updater({ id: 'dashboards' })).toBeUndefined();
+      }
+    });
+  });
+
   it('does not call register function for tenant app when multitenancy is off', async () => {
     // Mock hasApiPermission to return true
     pluginModule.hasApiPermission.mockResolvedValue(true);

public/types.ts

@@ -63,6 +63,8 @@ export interface DashboardsInfo {
 export interface ClientConfigType {
   readonly_mode: {
     roles: string[];
+    allow_discover?: boolean;
+    allow_visualize?: boolean;
   };
   ui: {
     basicauth: {

server/index.ts

@@ -45,6 +45,8 @@ export const configSchema = schema.object({
   allow_client_certificates: schema.boolean({ defaultValue: false }),
   readonly_mode: schema.object({
     roles: schema.arrayOf(schema.string(), { defaultValue: [] }),
+    allow_discover: schema.boolean({ defaultValue: false }),
+    allow_visualize: schema.boolean({ defaultValue: false }),
   }),
   clusterPermissions: schema.object({
     include: schema.arrayOf(schema.string(), { defaultValue: [] }),