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

feat: address concerns on ensureRawRequest #4

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -282,15 +282,15 @@ export function getQueryParams({

if (ACLSearchParams) {
const shouldClause: any = [];
if (ACLSearchParams.permissionModes && ACLSearchParams.principals) {
if (ACLSearchParams.permissionModes?.length && ACLSearchParams.principals) {
const permissionDSL = ACL.generateGetPermittedSavedObjectsQueryDSL(
ACLSearchParams.permissionModes,
ACLSearchParams.principals
);
shouldClause.push(permissionDSL.query);
}

if (ACLSearchParams.workspaces) {
if (ACLSearchParams.workspaces?.length) {
shouldClause.push({
terms: {
workspaces: ACLSearchParams.workspaces,
Expand All @@ -301,7 +301,28 @@ export function getQueryParams({
if (shouldClause.length) {
bool.filter.push({
bool: {
should: shouldClause,
should: [
/**
* Return those objects without workspaces field and permissions field to keep find find API backward compatible
*/
{
bool: {
must_not: [
{
exists: {
field: 'workspaces',
},
},
{
exists: {
field: 'permissions',
},
},
],
},
},
...shouldClause,
],
},
});
}
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 @@ -137,7 +144,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 @@ -28,7 +28,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 @@ -190,7 +189,7 @@ export class WorkspaceSavedObjectsClientWrapper {
if (savedObject.permissions) {
hasPermission = await this.permissionControl.validateSavedObjectsACL(
[savedObject],
getPrincipalsFromRequest(request),
this.permissionControl.getPrincipalsFromRequest(request),
objectPermissionModes
);
}
Expand Down Expand Up @@ -422,7 +421,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 Expand Up @@ -497,7 +496,6 @@ export class WorkspaceSavedObjectsClientWrapper {
* Select all the docs that
* 1. ACL matches read / write / user passed permission OR
* 2. workspaces matches library_read or library_write OR
* 3. Advanced settings
*/
options.workspaces = undefined;
options.ACLSearchParams.workspaces = permittedWorkspaceIds;
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
Loading