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 7 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 and objects inside OpenSearch Dashboards.
# workspace.dashboardAdmin.groups: ["dashboard_admin"]
# workspace.dashboardAdmin.users: ["dashboard_admin"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

The naming workspace.dashboardAdmin may confuse me, it sounds like the admin for dashboard type only. I come few names like workspace.dashboardsAdmin,
workspace.admin or workpsace.superAdmin

Copy link
Collaborator

Choose a reason for hiding this comment

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

workspace.admin may be confusing as admin of workspace stands for the the users who have write permission on specific workspaces, vote for superAdmin.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If there is no configuration named workspace.admin, would workspace.superAdmin also be confused?

1 change: 1 addition & 0 deletions src/legacy/server/osd_server.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ declare module '@hapi/hapi' {
interface PluginsStates {
workspace?: {
id?: string;
isDashboardAdmin?: boolean;
};
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/plugins/workspace/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ export const configSchema = schema.object({
permission: schema.object({
enabled: schema.boolean({ defaultValue: true }),
}),
dashboardAdmin: schema.object({
groups: schema.arrayOf(schema.string(), {
defaultValue: [],
}),
users: schema.arrayOf(schema.string(), {
defaultValue: [],
}),
}),
});

export type WorkspacePluginConfigType = TypeOf<typeof configSchema>;
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"]
}
40 changes: 36 additions & 4 deletions src/plugins/workspace/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,22 @@ import {
WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
} from '../common/constants';
import { IWorkspaceClientImpl } from './types';
import { AppPluginSetupDependencies, IWorkspaceClientImpl } from './types';
import { WorkspaceClient } from './workspace_client';
import { registerRoutes } from './routes';
import { WorkspaceSavedObjectsClientWrapper } from './saved_objects';
import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils';
import {
cleanWorkspaceId,
getWorkspaceIdFromUrl,
updateWorkspaceState,
} from '../../../core/server/utils';
import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict';
import {
SavedObjectsPermissionControl,
SavedObjectsPermissionControlContract,
} from './permission_control/client';
import { WorkspacePluginConfigType } from '../config';
import { isRequestByDashboardAdmin } from './saved_objects/workspace_saved_objects_client_wrapper';

export class WorkspacePlugin implements Plugin<{}, {}> {
private readonly logger: Logger;
Expand Down Expand Up @@ -60,7 +65,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
this.config$ = initializerContext.config.create<WorkspacePluginConfigType>();
}

public async setup(core: CoreSetup) {
public async setup(core: CoreSetup, { applicationConfig }: AppPluginSetupDependencies) {
this.logger.debug('Setting up Workspaces service');
const config: WorkspacePluginConfigType = await this.config$.pipe(first()).toPromise();
const isPermissionControlEnabled =
Expand All @@ -83,8 +88,35 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
if (isPermissionControlEnabled) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shall we move the permission control logic to a separate method like setupPermission? It seems over 30 lines.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I have moved the related logic to a separate setupPermission method.

this.permissionControl = new SavedObjectsPermissionControl(this.logger);

this.logger.info('Dynamic application configuration enabled:' + !!applicationConfig);
if (!!applicationConfig) {
core.http.registerOnPostAuth(async (request, response, toolkit) => {
const [coreStart] = await core.getStartServices();
SuZhou-Joe marked this conversation as resolved.
Show resolved Hide resolved
const scopeClient = coreStart.opensearch.client.asScoped(request);
const configClient = applicationConfig.getConfigurationClient(scopeClient);

const [adminGroups, adminUsers] = await Promise.all([
configClient.getEntityConfig('workspace.dashboardAdmin.groups').catch(() => undefined),
configClient.getEntityConfig('workspace.dashboardAdmin.users').catch(() => undefined),
]);

const isDashboardAdmin = isRequestByDashboardAdmin(
request,
adminGroups ? [adminGroups] : [],
adminUsers ? [adminUsers] : [],
this.permissionControl!
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we use assert here, permissionControl is inited above and its type is fixed.

Copy link
Author

Choose a reason for hiding this comment

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

You are right, the relevant logic has been deleted.

);
updateWorkspaceState(request, {
isDashboardAdmin,
});
return toolkit.next();
});
}

this.workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper(
this.permissionControl
this.permissionControl,
{ config$: this.config$ },
!!applicationConfig
);

core.savedObjects.addClientWrapper(
Expand Down
Loading
Loading