Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dashboard admin(groups/users) implementation and integrating with dynamic application config #303

Merged
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
72fc938
dashboard admin(groups/users) implementation and add unit/integration…
yubonluo Mar 20, 2024
29236e3
Add test cases of user Id matching dashboard admin
yubonluo Mar 20, 2024
22323c1
delete useless code
yubonluo Mar 20, 2024
f9666a0
change isDashboard function name to isRequestByDashboardAdmin
yubonluo Mar 21, 2024
123c87e
Merge branch 'workspace-pr-integr' of github.com:ruanyl/OpenSearch-Da…
yubonluo Apr 1, 2024
166451a
dashboard admin config integrating with dynamic application config
yubonluo Apr 2, 2024
363aeaa
Optimize the code
yubonluo Apr 2, 2024
c8bb0b5
fix test error
yubonluo Apr 2, 2024
74d9a26
delete useless code
yubonluo Apr 3, 2024
a07dde0
optimize code
yubonluo Apr 3, 2024
518cd75
optimize code and add unit test
yubonluo Apr 3, 2024
dc77999
optimize code according to comments
yubonluo Apr 3, 2024
71c20d9
change dashboardAdmin yml config to openseachDashboard
yubonluo Apr 3, 2024
c61dac9
add missed code
yubonluo Apr 3, 2024
0bf41b3
solve conflict
yubonluo Apr 8, 2024
f0c1b5d
optimize code
yubonluo Apr 8, 2024
2018bdf
Merge branch 'workspace-pr-integr' into pr-integr-dashboard-admin
yubonluo Apr 9, 2024
979851a
solve conflict
yubonluo Apr 9, 2024
3a70922
solve conflict
yubonluo Apr 9, 2024
a69cc58
delete useless code
yubonluo Apr 9, 2024
7d83f48
delete useless code
yubonluo Apr 9, 2024
f50093f
optimize code
yubonluo Apr 10, 2024
e9fd21a
delete useless code
yubonluo Apr 11, 2024
909af1c
add utils unit tests
yubonluo Apr 11, 2024
b342950
Merge branch 'workspace-pr-integr' of github.com:ruanyl/OpenSearch-Da…
yubonluo Apr 11, 2024
27d95fe
optimize code
yubonluo Apr 11, 2024
acfb166
Fix the wrong reference
yubonluo Apr 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions config/opensearch_dashboards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -290,3 +290,8 @@

# Set the value to true to enable workspace feature
# workspace.enabled: false

# Set the backend roles in groups or users, whoever has the backend roles or exactly match the user ids defined in this config will be regard as dashboard admin.
# Dashboard admin will have the access to all the workspaces(workspace.enabled: true) and objects inside OpenSearch Dashboards.
# opensearchDashboards.dashboardAdmin.groups: ["dashboard_admin"]
# opensearchDashboards.dashboardAdmin.users: ["dashboard_admin"]
1 change: 1 addition & 0 deletions src/core/server/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export function pluginInitializerContextConfigMock<T>(config: T) {
configIndex: '.opensearch_dashboards_config_tests',
autocompleteTerminateAfter: duration(100000),
autocompleteTimeout: duration(1000),
dashboardAdmin: { groups: [], users: [] },
},
opensearch: {
shardTimeout: duration('30s'),
Expand Down
8 changes: 8 additions & 0 deletions src/core/server/opensearch_dashboards_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ export const config = {
defaultValue: 'https://survey.opensearch.org',
}),
}),
dashboardAdmin: schema.object({
groups: schema.arrayOf(schema.string(), {
defaultValue: [],
}),
users: schema.arrayOf(schema.string(), {
defaultValue: [],
}),
}),
}),
deprecations,
};
1 change: 1 addition & 0 deletions src/core/server/plugins/plugin_context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ describe('createPluginInitializerContext', () => {
configIndex: '.opensearch_dashboards_config',
autocompleteTerminateAfter: duration(100000),
autocompleteTimeout: duration(1000),
dashboardAdmin: { groups: [], users: [] },
},
opensearch: {
shardTimeout: duration(30, 's'),
Expand Down
1 change: 1 addition & 0 deletions src/core/server/plugins/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ export const SharedGlobalConfigKeys = {
'configIndex',
'autocompleteTerminateAfter',
'autocompleteTimeout',
'dashboardAdmin',
] as const,
opensearch: ['shardTimeout', 'requestTimeout', 'pingTimeout'] as const,
path: ['data'] as const,
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/utils/workspace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ describe('updateWorkspaceState', () => {
const requestMock = httpServerMock.createOpenSearchDashboardsRequest();
updateWorkspaceState(requestMock, {
requestWorkspaceId: 'foo',
isDashboardAdmin: true,
});
expect(getWorkspaceState(requestMock)).toEqual({
requestWorkspaceId: 'foo',
isDashboardAdmin: true,
});
});
});
4 changes: 3 additions & 1 deletion src/core/server/utils/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router';

export interface WorkspaceState {
requestWorkspaceId?: string;
isDashboardAdmin?: boolean;
}

/**
Expand All @@ -29,8 +30,9 @@ export const updateWorkspaceState = (
};

export const getWorkspaceState = (request: OpenSearchDashboardsRequest): WorkspaceState => {
const { requestWorkspaceId } = ensureRawRequest(request).app as WorkspaceState;
const { requestWorkspaceId, isDashboardAdmin } = ensureRawRequest(request).app as WorkspaceState;
return {
requestWorkspaceId,
isDashboardAdmin,
};
};
4 changes: 4 additions & 0 deletions src/legacy/server/config/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,10 @@ export default () =>
survey: Joi.object({
url: Joi.any().default('/'),
}),
dashboardAdmin: Joi.object({
groups: Joi.array().items(Joi.string()).default([]),
users: Joi.array().items(Joi.string()).default([]),
}),
}).default(),

savedObjects: HANDLED_IN_NEW_PLATFORM,
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/workspace/opensearch_dashboards.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
"requiredPlugins": [
"savedObjects"
],
"optionalPlugins": ["savedObjectsManagement"],
"optionalPlugins": ["savedObjectsManagement", "applicationConfig"],
"requiredBundles": ["opensearchDashboardsReact"]
}
14 changes: 11 additions & 3 deletions src/plugins/workspace/server/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,17 @@
import { OnPreRoutingHandler } from 'src/core/server';
import { coreMock, httpServerMock } from '../../../core/server/mocks';
import { WorkspacePlugin } from './plugin';
import { AppPluginSetupDependencies } from './types';
import { getWorkspaceState } from '../../../core/server/utils';

describe('Workspace server plugin', () => {
const mockApplicationConfig = {
getConfigurationClient: jest.fn().mockResolvedValue({}),
registerConfigurationClient: jest.fn().mockResolvedValue({}),
};
const mockDependencies: AppPluginSetupDependencies = {
applicationConfig: mockApplicationConfig,
};
it('#setup', async () => {
let value;
const setupMock = coreMock.createSetup();
Expand All @@ -17,7 +25,7 @@ describe('Workspace server plugin', () => {
});
setupMock.capabilities.registerProvider.mockImplementationOnce((fn) => (value = fn()));
const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock);
await workspacePlugin.setup(setupMock);
await workspacePlugin.setup(setupMock, mockDependencies);
expect(value).toMatchInlineSnapshot(`
Object {
"workspaces": Object {
Expand All @@ -43,7 +51,7 @@ describe('Workspace server plugin', () => {
return fn;
});
const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock);
await workspacePlugin.setup(setupMock);
await workspacePlugin.setup(setupMock, mockDependencies);
const toolKitMock = httpServerMock.createToolkit();

const requestWithWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({
Expand Down Expand Up @@ -78,7 +86,7 @@ describe('Workspace server plugin', () => {
});

const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock);
await workspacePlugin.setup(setupMock);
await workspacePlugin.setup(setupMock, mockDependencies);
await workspacePlugin.start(startMock);
expect(startMock.savedObjects.createSerializer).toBeCalledTimes(1);
});
Expand Down
92 changes: 77 additions & 15 deletions src/plugins/workspace/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ import {
WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
WORKSPACE_ID_CONSUMER_WRAPPER_ID,
} from '../common/constants';
import { IWorkspaceClientImpl, WorkspacePluginSetup, WorkspacePluginStart } from './types';
import {
IWorkspaceClientImpl,
WorkspacePluginSetup,
WorkspacePluginStart,
AppPluginSetupDependencies,
} from './types';
import { WorkspaceClient } from './workspace_client';
import { registerRoutes } from './routes';
import { WorkspaceSavedObjectsClientWrapper } from './saved_objects';
Expand All @@ -32,6 +37,7 @@ import {
SavedObjectsPermissionControl,
SavedObjectsPermissionControlContract,
} from './permission_control/client';
import { updateDashboardAdminStateForRequest } from './utils';
import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper';

export class WorkspacePlugin implements Plugin<WorkspacePluginSetup, WorkspacePluginStart> {
Expand Down Expand Up @@ -64,12 +70,80 @@ export class WorkspacePlugin implements Plugin<WorkspacePluginSetup, WorkspacePl
});
}

private async setupPermission(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to declare as a async function here I think

core: CoreSetup,
{ applicationConfig }: AppPluginSetupDependencies
) {
this.permissionControl = new SavedObjectsPermissionControl(this.logger);

core.http.registerOnPostAuth(async (request, response, toolkit) => {
let groups: string[];
let users: string[];

// There may be calls to saved objects client before user get authenticated, need to add a try catch here as `getPrincipalsFromRequest` will throw error when user is not authenticated.
try {
({ groups = [], users = [] } = this.permissionControl!.getPrincipalsFromRequest(request));
} catch (e) {
return toolkit.next();
}
if (groups.length === 0 && users.length === 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the cluster does not require authentication, anyone can be dashboardAdmin I think.
@ruanyl @wanglam @raintygao what do you think?

Copy link
Collaborator

@raintygao raintygao Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, a bit like OSD without security plugin installed that will have almost admin permissions. Also want to hear other's thoughts.

Copy link
Author

@yubonluo yubonluo Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean to update isDashboardAdmin state to true if groups.length === 0 && users.length === 0 when both the security plugin and savedObject.permission.enabled are open.

updateWorkspaceState(request, {
isDashboardAdmin: false,
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we use updateDashboardAdminStateForRequest method here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If users/groups is null, we can prevent the program from running down in advance. Meanwhile, configGroups/configUsers has not been obtained.

return toolkit.next();
}

this.logger.info('Dynamic application configuration enabled:' + !!applicationConfig);
if (!!applicationConfig) {
const [coreStart] = await core.getStartServices();
const scopeClient = coreStart.opensearch.client.asScoped(request);
const applicationConfigClient = applicationConfig.getConfigurationClient(scopeClient);

const [configGroups, configUsers] = await Promise.all([
applicationConfigClient
.getEntityConfig('opensearchDashboards.dashboardAdmin.groups')
.catch(() => undefined),
applicationConfigClient
.getEntityConfig('opensearchDashboards.dashboardAdmin.users')
.catch(() => undefined),
]);

updateDashboardAdminStateForRequest(
request,
groups,
users,
configGroups ? [configGroups] : [],
configUsers ? [configUsers] : []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
configGroups ? [configGroups] : [],
configUsers ? [configUsers] : []
configGroups ? configGroups : [],
configUsers ? configUsers : []

configGroups and configUsers should already be array I think?

);
return toolkit.next();
}

const globalConfig: SharedGlobalConfig = await this.globalConfig$.pipe(first()).toPromise();
const configGroups = (globalConfig.opensearchDashboards.dashboardAdmin.groups ||
[]) as string[];
const configUsers = (globalConfig.opensearchDashboards.dashboardAdmin.users ||
[]) as string[];
updateDashboardAdminStateForRequest(request, groups, users, configGroups, configUsers);
return toolkit.next();
});

this.workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper(
this.permissionControl
);

core.savedObjects.addClientWrapper(
0,
WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
this.workspaceSavedObjectsClientWrapper.wrapperFactory
);
}

constructor(initializerContext: PluginInitializerContext) {
this.logger = initializerContext.logger.get('plugins', 'workspace');
this.globalConfig$ = initializerContext.config.legacy.globalConfig$;
}

public async setup(core: CoreSetup) {
public async setup(core: CoreSetup, { applicationConfig }: AppPluginSetupDependencies) {
this.logger.debug('Setting up Workspaces service');
const globalConfig = await this.globalConfig$.pipe(first()).toPromise();
const isPermissionControlEnabled = globalConfig.savedObjects.permission.enabled === true;
Expand All @@ -94,19 +168,7 @@ export class WorkspacePlugin implements Plugin<WorkspacePluginSetup, WorkspacePl
);

this.logger.info('Workspace permission control enabled:' + isPermissionControlEnabled);
if (isPermissionControlEnabled) {
this.permissionControl = new SavedObjectsPermissionControl(this.logger);

this.workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper(
this.permissionControl
);

core.savedObjects.addClientWrapper(
0,
WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
this.workspaceSavedObjectsClientWrapper.wrapperFactory
);
}
if (isPermissionControlEnabled) await this.setupPermission(core, { applicationConfig });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems the await is not needed here.


registerRoutes({
http: core.http,
Expand Down
Loading
Loading