Skip to content

Commit

Permalink
Switching ensure to owner array
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathan-buttner committed Apr 20, 2021
1 parent bac104e commit dc761d6
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 52 deletions.
44 changes: 14 additions & 30 deletions x-pack/plugins/cases/server/authorization/audit_logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand Down Expand Up @@ -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({
Expand All @@ -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,
Expand All @@ -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;
}
Expand Down
32 changes: 18 additions & 14 deletions x-pack/plugins/cases/server/authorization/authorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,36 +79,38 @@ 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
* don't actually get "privilege checked" so the made up owner *will* return
* 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
Expand All @@ -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 });
}
},
};
Expand All @@ -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<string, [string]>();
const requiredPrivileges = new Map<string, string>();

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);
}
}

Expand All @@ -175,7 +179,7 @@ export class Authorization {
? Array.from(featureCaseOwners)
: privileges.kibana.reduce<string[]>((authorizedOwners, { authorized, privilege }) => {
if (authorized && requiredPrivileges.has(privilege)) {
const [owner] = requiredPrivileges.get(privilege)!;
const owner = requiredPrivileges.get(privilege)!;
authorizedOwners.push(owner);
}

Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/cases/server/client/cases/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 8 additions & 6 deletions x-pack/plugins/cases/server/client/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Authorization>;
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;
}
}
Expand Down

0 comments on commit dc761d6

Please sign in to comment.