From dc761d6a6d5af507b9ff69bbdad6f9cdde54e79a Mon Sep 17 00:00:00 2001 From: Jonathan Buttner Date: Tue, 20 Apr 2021 11:07:03 -0400 Subject: [PATCH] Switching ensure to owner array --- .../server/authorization/audit_logger.ts | 44 ++++++------------- .../server/authorization/authorization.ts | 32 ++++++++------ .../cases/server/client/cases/create.ts | 4 +- x-pack/plugins/cases/server/client/utils.ts | 14 +++--- 4 files changed, 42 insertions(+), 52 deletions(-) diff --git a/x-pack/plugins/cases/server/authorization/audit_logger.ts b/x-pack/plugins/cases/server/authorization/audit_logger.ts index 3c890a2c7ad5be..2a739ea6e81067 100644 --- a/x-pack/plugins/cases/server/authorization/audit_logger.ts +++ b/x-pack/plugins/cases/server/authorization/audit_logger.ts @@ -20,19 +20,19 @@ export class AuthorizationAuditLogger { this.auditLogger = logger; } - private createMessage({ + private static createMessage({ result, - owner, + owners, operation, }: { result: AuthorizationResult; - owner?: string; + owners?: string[]; operation: OperationDetails; }): string { - const ownerMsg = owner == null ? 'of any owner' : `with "${owner}" as the owner`; + const ownerMsg = owners == null ? 'of any owner' : `with owners: "${owners.join(', ')}"`; /** * This will take the form: - * `Unauthorized to create case with "securitySolution" as the owner` + * `Unauthorized to create case with owners: "securitySolution, observability"` * `Unauthorized to find cases of any owner`. */ return `${result} to ${operation.verbs.present} ${operation.docType} ${ownerMsg}`; @@ -65,16 +65,16 @@ export class AuthorizationAuditLogger { public failure({ username, - owner, + owners, operation, }: { username?: string; - owner?: string; + owners?: string[]; operation: OperationDetails; }): string { - const message = this.createMessage({ + const message = AuthorizationAuditLogger.createMessage({ result: AuthorizationResult.Unauthorized, - owner, + owners, operation, }); this.auditLogger?.log({ @@ -96,24 +96,6 @@ export class AuthorizationAuditLogger { } public success({ - username, - operation, - owner, - }: { - username: string; - owner: string; - operation: OperationDetails; - }): string { - const message = this.createMessage({ - result: AuthorizationResult.Authorized, - owner, - operation, - }); - this.logSuccessEvent({ message, operation, username }); - return message; - } - - public bulkSuccess({ username, operation, owners, @@ -122,9 +104,11 @@ export class AuthorizationAuditLogger { owners: string[]; operation: OperationDetails; }): string { - const message = `${AuthorizationResult.Authorized} to ${operation.verbs.present} ${ - operation.docType - } of owner: ${owners.join(', ')}`; + const message = AuthorizationAuditLogger.createMessage({ + result: AuthorizationResult.Authorized, + owners, + operation, + }); this.logSuccessEvent({ message, operation, username }); return message; } diff --git a/x-pack/plugins/cases/server/authorization/authorization.ts b/x-pack/plugins/cases/server/authorization/authorization.ts index d72abaf1fd3b89..adb684c60a1bd2 100644 --- a/x-pack/plugins/cases/server/authorization/authorization.ts +++ b/x-pack/plugins/cases/server/authorization/authorization.ts @@ -79,19 +79,21 @@ export class Authorization { return this.securityAuth?.mode?.useRbacForRequest(this.request) ?? false; } - public async ensureAuthorized(owner: string, operation: OperationDetails) { + public async ensureAuthorized(owners: string[], operation: OperationDetails) { const { securityAuth } = this; - const isOwnerAvailable = this.featureCaseOwners.has(owner); + const areAllOwnersAvailable = owners.every((owner) => this.featureCaseOwners.has(owner)); if (securityAuth && this.shouldCheckAuthorization()) { - const requiredPrivileges: string[] = [securityAuth.actions.cases.get(owner, operation.name)]; + const requiredPrivileges: string[] = owners.map((owner) => + securityAuth.actions.cases.get(owner, operation.name) + ); const checkPrivileges = securityAuth.checkPrivilegesDynamicallyWithRequest(this.request); const { hasAllRequested, username } = await checkPrivileges({ kibana: requiredPrivileges, }); - if (!isOwnerAvailable) { + if (!areAllOwnersAvailable) { /** * Under most circumstances this would have been caught by `checkPrivileges` as * a user can't have Privileges to an unknown owner, but super users @@ -99,16 +101,16 @@ export class Authorization { * as Privileged. * This check will ensure we don't accidentally let these through */ - throw Boom.forbidden(this.auditLogger.failure({ username, owner, operation })); + throw Boom.forbidden(this.auditLogger.failure({ username, owners, operation })); } if (hasAllRequested) { - this.auditLogger.success({ username, operation, owner }); + this.auditLogger.success({ username, operation, owners }); } else { - throw Boom.forbidden(this.auditLogger.failure({ owner, operation, username })); + throw Boom.forbidden(this.auditLogger.failure({ owners, operation, username })); } - } else if (!isOwnerAvailable) { - throw Boom.forbidden(this.auditLogger.failure({ owner, operation })); + } else if (!areAllOwnersAvailable) { + throw Boom.forbidden(this.auditLogger.failure({ owners, operation })); } // else security is disabled so let the operation proceed @@ -129,12 +131,14 @@ export class Authorization { filter: getOwnersFilter(operation.savedObjectType, authorizedOwners), ensureSavedObjectIsAuthorized: (owner: string) => { if (!authorizedOwners.includes(owner)) { - throw Boom.forbidden(this.auditLogger.failure({ username, operation, owner })); + throw Boom.forbidden( + this.auditLogger.failure({ username, operation, owners: [owner] }) + ); } }, logSuccessfulAuthorization: () => { if (authorizedOwners.length) { - this.auditLogger.bulkSuccess({ username, owners: authorizedOwners, operation }); + this.auditLogger.success({ username, owners: authorizedOwners, operation }); } }, }; @@ -156,11 +160,11 @@ export class Authorization { const { securityAuth, featureCaseOwners } = this; if (securityAuth && this.shouldCheckAuthorization()) { const checkPrivileges = securityAuth.checkPrivilegesDynamicallyWithRequest(this.request); - const requiredPrivileges = new Map(); + const requiredPrivileges = new Map(); for (const owner of featureCaseOwners) { for (const operation of operations) { - requiredPrivileges.set(securityAuth.actions.cases.get(owner, operation.name), [owner]); + requiredPrivileges.set(securityAuth.actions.cases.get(owner, operation.name), owner); } } @@ -175,7 +179,7 @@ export class Authorization { ? Array.from(featureCaseOwners) : privileges.kibana.reduce((authorizedOwners, { authorized, privilege }) => { if (authorized && requiredPrivileges.has(privilege)) { - const [owner] = requiredPrivileges.get(privilege)!; + const owner = requiredPrivileges.get(privilege)!; authorizedOwners.push(owner); } diff --git a/x-pack/plugins/cases/server/client/cases/create.ts b/x-pack/plugins/cases/server/client/cases/create.ts index 117f90aca95b53..2109424575ed35 100644 --- a/x-pack/plugins/cases/server/client/cases/create.ts +++ b/x-pack/plugins/cases/server/client/cases/create.ts @@ -91,10 +91,10 @@ export const create = async ({ await ensureAuthorized({ operation: Operations.createCase, - owner: query.owner, + owners: [query.owner], authorization: auth, auditLogger, - savedObjectID, + savedObjectIDs: [savedObjectID], }); // log that we're attempting to create a case diff --git a/x-pack/plugins/cases/server/client/utils.ts b/x-pack/plugins/cases/server/client/utils.ts index dfff3ca449a276..0dcbf61fa08942 100644 --- a/x-pack/plugins/cases/server/client/utils.ts +++ b/x-pack/plugins/cases/server/client/utils.ts @@ -453,22 +453,24 @@ export const sortToSnake = (sortField: string | undefined): SortFieldCase => { * on a failure. */ export async function ensureAuthorized({ - owner, + owners, operation, - savedObjectID, + savedObjectIDs, authorization, auditLogger, }: { - owner: string; + owners: string[]; operation: OperationDetails; - savedObjectID: string; + savedObjectIDs: string[]; authorization: PublicMethodsOf; auditLogger?: AuditLogger; }) { try { - return await authorization.ensureAuthorized(owner, operation); + return await authorization.ensureAuthorized(owners, operation); } catch (error) { - auditLogger?.log(createAuditMsg({ operation, error, savedObjectID })); + for (const savedObjectID of savedObjectIDs) { + auditLogger?.log(createAuditMsg({ operation, error, savedObjectID })); + } throw error; } }