Skip to content

Commit

Permalink
feat: address concerns on ensureRawRequest
Browse files Browse the repository at this point in the history
Signed-off-by: SuZhou-Joe <[email protected]>
  • Loading branch information
SuZhou-Joe committed Mar 1, 2024
1 parent 898fe4c commit a9e672e
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 38 deletions.
1 change: 0 additions & 1 deletion src/core/server/http/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ export {
RouteValidationResultFactory,
DestructiveRouteMethod,
SafeRouteMethod,
ensureRawRequest,
} from './router';
export { BasePathProxyServer } from './base_path_proxy_server';
export { OnPreRoutingHandler, OnPreRoutingToolkit } from './lifecycle/on_pre_routing';
Expand Down
1 change: 0 additions & 1 deletion src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ export {
SessionStorageFactory,
DestructiveRouteMethod,
SafeRouteMethod,
ensureRawRequest,
} from './http';

export {
Expand Down
43 changes: 28 additions & 15 deletions src/plugins/workspace/server/permission_control/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,18 @@

import { loggerMock } from '@osd/logging/target/mocks';
import { SavedObjectsPermissionControl } from './client';
import { httpServerMock, savedObjectsClientMock } from '../../../../core/server/mocks';
import {
httpServerMock,
httpServiceMock,
savedObjectsClientMock,
} from '../../../../core/server/mocks';
import * as utilsExports from '../utils';

describe('workspace utils', () => {
describe('PermissionControl', () => {
jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation(() => ({
users: ['bar'],
}));
const mockAuth = httpServiceMock.createAuth();
it('should return principals when calling getPrincipalsOfObjects', async () => {
const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create());
const getScopedClient = jest.fn();
Expand All @@ -27,7 +36,7 @@ describe('workspace utils', () => {
});
return clientMock;
});
permissionControlClient.setup(getScopedClient);
permissionControlClient.setup(getScopedClient, mockAuth);
const result = await permissionControlClient.getPrincipalsOfObjects(
httpServerMock.createOpenSearchDashboardsRequest(),
[]
Expand All @@ -50,7 +59,7 @@ describe('workspace utils', () => {
getScopedClient.mockImplementation((request) => {
return clientMock;
});
permissionControlClient.setup(getScopedClient);
permissionControlClient.setup(getScopedClient, mockAuth);
clientMock.bulkGet.mockResolvedValue({
saved_objects: [],
});
Expand All @@ -69,7 +78,7 @@ describe('workspace utils', () => {
getScopedClient.mockImplementation((request) => {
return clientMock;
});
permissionControlClient.setup(getScopedClient);
permissionControlClient.setup(getScopedClient, mockAuth);

clientMock.bulkGet.mockResolvedValue({
saved_objects: [
Expand Down Expand Up @@ -102,7 +111,7 @@ describe('workspace utils', () => {
getScopedClient.mockImplementation((request) => {
return clientMock;
});
permissionControlClient.setup(getScopedClient);
permissionControlClient.setup(getScopedClient, mockAuth);

clientMock.bulkGet.mockResolvedValue({
saved_objects: [
Expand All @@ -126,19 +135,23 @@ describe('workspace utils', () => {
],
});
const batchValidateResult = await permissionControlClient.batchValidate(
httpServerMock.createOpenSearchDashboardsRequest({
auth: {
credentials: {
authInfo: {
user_name: 'bar',
},
},
} as any,
}),
httpServerMock.createOpenSearchDashboardsRequest(),
[],
['read']
);
expect(batchValidateResult.success).toEqual(true);
expect(batchValidateResult.result).toEqual(true);
});

describe('getPrincipalsFromRequest', () => {
const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create());
const getScopedClient = jest.fn();
permissionControlClient.setup(getScopedClient, mockAuth);

it('should return normally when calling getPrincipalsFromRequest', () => {
const mockRequest = httpServerMock.createOpenSearchDashboardsRequest();
const result = permissionControlClient.getPrincipalsFromRequest(mockRequest);
expect(result.users).toEqual(['bar']);
});
});
});
11 changes: 9 additions & 2 deletions src/plugins/workspace/server/permission_control/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
SavedObject,
WORKSPACE_TYPE,
Permissions,
HttpAuth,
} from '../../../../core/server';
import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../../common/constants';
import { getPrincipalsFromRequest } from '../utils';
Expand All @@ -29,6 +30,7 @@ export type SavedObjectsPermissionModes = string[];
export class SavedObjectsPermissionControl {
private readonly logger: Logger;
private _getScopedClient?: SavedObjectsServiceStart['getScopedClient'];
private auth?: HttpAuth;
private getScopedClient(request: OpenSearchDashboardsRequest) {
return this._getScopedClient?.(request, {
excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID],
Expand All @@ -46,8 +48,9 @@ export class SavedObjectsPermissionControl {
) {
return (await this.getScopedClient?.(request)?.bulkGet(savedObjects))?.saved_objects || [];
}
public async setup(getScopedClient: SavedObjectsServiceStart['getScopedClient']) {
public async setup(getScopedClient: SavedObjectsServiceStart['getScopedClient'], auth: HttpAuth) {
this._getScopedClient = getScopedClient;
this.auth = auth;
}
public async validate(
request: OpenSearchDashboardsRequest,
Expand Down Expand Up @@ -76,6 +79,10 @@ export class SavedObjectsPermissionControl {
);
}

public getPrincipalsFromRequest(request: OpenSearchDashboardsRequest) {
return getPrincipalsFromRequest(request, this.auth);
}

public validateSavedObjectsACL(
savedObjects: Array<Pick<SavedObject<unknown>, 'id' | 'type' | 'workspaces' | 'permissions'>>,
principals: Principals,
Expand Down Expand Up @@ -136,7 +143,7 @@ export class SavedObjectsPermissionControl {
};
}

const principals = getPrincipalsFromRequest(request);
const principals = this.getPrincipalsFromRequest(request);
const deniedObjects: Array<
Pick<SavedObjectsBulkGetObject, 'id' | 'type'> & {
workspaces?: string[];
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/workspace/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
http: core.http,
logger: this.logger,
client: this.client as IWorkspaceClientImpl,
permissionControlClient: this.permissionControl,
});

core.capabilities.registerProvider(() => ({
Expand All @@ -98,7 +99,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> {

public start(core: CoreStart) {
this.logger.debug('Starting Workspace service');
this.permissionControl?.setup(core.savedObjects.getScopedClient);
this.permissionControl?.setup(core.savedObjects.getScopedClient, core.http.auth);
this.client?.setSavedObjects(core.savedObjects);
this.workspaceSavedObjectsClientWrapper?.setScopedClient(core.savedObjects.getScopedClient);

Expand Down
14 changes: 8 additions & 6 deletions src/plugins/workspace/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
*/

import { schema } from '@osd/config-schema';
import { CoreSetup, Logger, ensureRawRequest } from '../../../../core/server';
import { CoreSetup, Logger } from '../../../../core/server';
import { WorkspacePermissionMode } from '../../common/constants';
import { AuthInfo, IWorkspaceClientImpl, WorkspacePermissionItem } from '../types';
import { IWorkspaceClientImpl, WorkspacePermissionItem } from '../types';
import { SavedObjectsPermissionControlContract } from '../permission_control/client';

const WORKSPACES_API_BASE_URL = '/api/workspaces';

Expand Down Expand Up @@ -44,10 +45,12 @@ export function registerRoutes({
client,
logger,
http,
permissionControlClient,
}: {
client: IWorkspaceClientImpl;
logger: Logger;
http: CoreSetup['http'];
permissionControlClient?: SavedObjectsPermissionControlContract;
}) {
const router = http.createRouter();
router.post(
Expand Down Expand Up @@ -124,8 +127,7 @@ export function registerRoutes({
},
router.handleLegacyErrors(async (context, req, res) => {
const { attributes, permissions: permissionsInRequest } = req.body;
const rawRequest = ensureRawRequest(req);
const authInfo = rawRequest?.auth?.credentials?.authInfo as AuthInfo | null;
const authInfo = permissionControlClient?.getPrincipalsFromRequest(req);
let permissions: WorkspacePermissionItem[] = [];
if (permissionsInRequest) {
permissions = Array.isArray(permissionsInRequest)
Expand All @@ -134,10 +136,10 @@ export function registerRoutes({
}

// Assign workspace owner to current user
if (!!authInfo?.user_name) {
if (!!authInfo?.users?.length) {
permissions.push({
type: 'user',
userId: authInfo.user_name,
userId: authInfo.users[0],
modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write],
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import {
SavedObjectsClientContract,
} from '../../../../core/server';
import { SavedObjectsPermissionControlContract } from '../permission_control/client';
import { getPrincipalsFromRequest } from '../utils';
import {
WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
WorkspacePermissionMode,
Expand Down Expand Up @@ -172,7 +171,7 @@ export class WorkspaceSavedObjectsClientWrapper {
if (savedObject.permissions) {
hasPermission = await this.permissionControl.validateSavedObjectsACL(
[savedObject],
getPrincipalsFromRequest(request),
this.permissionControl.getPrincipalsFromRequest(request),
objectPermissionModes
);
}
Expand Down Expand Up @@ -400,7 +399,7 @@ export class WorkspaceSavedObjectsClientWrapper {
const findWithWorkspacePermissionControl = async <T = unknown>(
options: SavedObjectsFindOptions
) => {
const principals = getPrincipalsFromRequest(wrapperOptions.request);
const principals = this.permissionControl.getPrincipalsFromRequest(wrapperOptions.request);
if (!options.ACLSearchParams) {
options.ACLSearchParams = {};
}
Expand Down
21 changes: 12 additions & 9 deletions src/plugins/workspace/server/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

import crypto from 'crypto';
import {
ensureRawRequest,
AuthStatus,
HttpAuth,
OpenSearchDashboardsRequest,
Principals,
PrincipalType,
Expand All @@ -19,25 +20,27 @@ export const generateRandomId = (size: number) => {
return crypto.randomBytes(size).toString('base64url').slice(0, size);
};

export const getPrincipalsFromRequest = (request: OpenSearchDashboardsRequest): Principals => {
const rawRequest = ensureRawRequest(request);
const authInfo = rawRequest?.auth?.credentials?.authInfo as AuthInfo | null;
export const getPrincipalsFromRequest = (
request: OpenSearchDashboardsRequest,
auth?: HttpAuth
): Principals => {
const payload: Principals = {};
if (!authInfo) {
const authInfoResp = auth?.get(request);
if (authInfoResp?.status === AuthStatus.unknown) {
/**
* Login user have access to all the workspaces when no authentication is presented.
* The logic will be used when users create workspaces with authentication enabled but turn off authentication for any reason.
*/
return payload;
}
if (!authInfo?.backend_roles?.length && !authInfo.user_name) {

if (authInfoResp?.status === AuthStatus.unauthenticated) {
/**
* It means OSD can not recognize who the user is even if authentication is enabled,
* use a fake user that won't be granted permission explicitly.
* use a fake user that won't be granted permission explicitly when authenticated error.
*/
payload[PrincipalType.Users] = [`_user_fake_${Date.now()}_`];
return payload;
}
const authInfo = authInfoResp?.state as AuthInfo | null;
if (authInfo?.backend_roles) {
payload[PrincipalType.Groups] = authInfo.backend_roles;
}
Expand Down

0 comments on commit a9e672e

Please sign in to comment.