From 58d3343bec61d30fd06c5effb88750cf7d26ca83 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Wed, 5 Jun 2024 16:56:59 +0300 Subject: [PATCH] feat(rbac): add type checks with generics for audit log Signed-off-by: Oleksandr Andriienko --- .../audit-log-node/src/DefaultAuditLogger.ts | 10 +++++--- plugins/audit-log-node/src/types.ts | 14 +++++------ .../src/audit-log/audit-logger.ts | 5 ++-- .../src/file-permissions/csv-file-watcher.ts | 10 +++++--- plugins/rbac-backend/src/helper.ts | 3 ++- .../src/service/permission-policy.ts | 6 +++-- .../src/service/policies-rest-api.ts | 25 +++++++++++-------- 7 files changed, 42 insertions(+), 31 deletions(-) diff --git a/plugins/audit-log-node/src/DefaultAuditLogger.ts b/plugins/audit-log-node/src/DefaultAuditLogger.ts index 27751d781e..e8fd04daa5 100644 --- a/plugins/audit-log-node/src/DefaultAuditLogger.ts +++ b/plugins/audit-log-node/src/DefaultAuditLogger.ts @@ -4,7 +4,7 @@ import { LoggerService, } from '@backstage/backend-plugin-api'; import { ErrorLike } from '@backstage/errors'; -import { JsonObject } from '@backstage/types'; +import { JsonObject, JsonValue } from '@backstage/types'; import { Request } from 'express'; import { cloneDeep } from 'lodash'; @@ -51,8 +51,8 @@ export class DefaultAuditLogger implements AuditLogger { return undefined; } } - async createAuditLogDetails( - options: AuditLogDetailsOptions, + async createAuditLogDetails( + options: AuditLogDetailsOptions, ): Promise { const { eventName, stage, metadata, request, response, status } = options; @@ -106,7 +106,9 @@ export class DefaultAuditLogger implements AuditLogger { status, }; } - async auditLog(options: AuditLogOptions): Promise { + async auditLog( + options: AuditLogOptions, + ): Promise { let auditLogDetails: AuditLogDetails; const logLevel = options.level || 'info'; const auditLogCommonDetails = { diff --git a/plugins/audit-log-node/src/types.ts b/plugins/audit-log-node/src/types.ts index 4284056acb..67ccb2cc4f 100644 --- a/plugins/audit-log-node/src/types.ts +++ b/plugins/audit-log-node/src/types.ts @@ -54,22 +54,22 @@ export type AuditLogDetails = { isAuditLog: true; } & AuditLogStatus; -export type AuditLogDetailsOptions = { +export type AuditLogDetailsOptions = { eventName: string; stage: string; - metadata?: JsonValue; + metadata?: T; response?: AuditResponse; actorId?: string; request?: Request; } & (AuditLogSuccessStatus | AuditLogUnknownFailureStatus); -export type AuditLogOptions = { +export type AuditLogOptions = { eventName: string; message: string; stage: string; level?: 'info' | 'debug' | 'warn' | 'error'; actorId?: string; - metadata?: JsonValue; + metadata?: T; response?: AuditResponse; request?: Request; } & (AuditLogSuccessStatus | AuditLogUnknownFailureStatus); @@ -94,8 +94,8 @@ export interface AuditLogger { * Secrets in the metadata field and request body, params and query field should be redacted by the user before passing in the request object * @public */ - createAuditLogDetails( - options: AuditLogDetailsOptions, + createAuditLogDetails( + options: AuditLogDetailsOptions, ): Promise; /** @@ -105,5 +105,5 @@ export interface AuditLogger { * Secrets in the metadata field and request body, params and query field should be redacted by the user before passing in the request object * @public */ - auditLog(options: AuditLogOptions): Promise; + auditLog(options: AuditLogOptions): Promise; } diff --git a/plugins/rbac-backend/src/audit-log/audit-logger.ts b/plugins/rbac-backend/src/audit-log/audit-logger.ts index ccc7be79fb..cc0a49f6b8 100644 --- a/plugins/rbac-backend/src/audit-log/audit-logger.ts +++ b/plugins/rbac-backend/src/audit-log/audit-logger.ts @@ -91,7 +91,8 @@ export const ConditionEvents = { }; export type ConditionAuditInfo = { - condition: RoleConditionalPolicyDecision; + conditionId?: number; + condition: RoleConditionalPolicyDecision; }; export const RBAC_BACKEND = 'rbac-backend'; @@ -111,7 +112,7 @@ export function createPermissionEvaluationOptions( userEntityRef: string, request: PolicyQuery, policyDecision?: PolicyDecision, -): AuditLogOptions { +): AuditLogOptions { const auditInfo: EvaluationAuditInfo = { userEntityRef, permissionName: request.permission.name, diff --git a/plugins/rbac-backend/src/file-permissions/csv-file-watcher.ts b/plugins/rbac-backend/src/file-permissions/csv-file-watcher.ts index 5a12b47887..6331de25b8 100644 --- a/plugins/rbac-backend/src/file-permissions/csv-file-watcher.ts +++ b/plugins/rbac-backend/src/file-permissions/csv-file-watcher.ts @@ -10,8 +10,10 @@ import fs from 'fs'; import { HANDLE_RBAC_DATA_STAGE, + PermissionAuditInfo, PermissionEvents, RBAC_BACKEND, + RoleAuditInfo, RoleEvents, } from '../audit-log/audit-logger'; import { @@ -308,7 +310,7 @@ export class CSVFileWatcher { try { await this.enforcer.addOrUpdatePolicy(policy, 'csv-file'); - await this.auditLogger.auditLog({ + await this.auditLogger.auditLog({ actorId: RBAC_BACKEND, message: `Created or updated policy`, eventName: PermissionEvents.CREATE_OR_UPDATE_POLICY, @@ -333,7 +335,7 @@ export class CSVFileWatcher { try { await this.enforcer.removePolicies(this.csvFilePolicies.removedPolicies); - await this.auditLogger.auditLog({ + await this.auditLogger.auditLog({ actorId: RBAC_BACKEND, message: `Deleted policies`, eventName: PermissionEvents.DELETE_POLICY, @@ -405,7 +407,7 @@ export class CSVFileWatcher { ? RoleEvents.UPDATE_ROLE : RoleEvents.CREATE_ROLE; const message = currentMetadata ? 'Updated role' : 'Created role'; - await this.auditLogger.auditLog({ + await this.auditLogger.auditLog({ actorId: RBAC_BACKEND, message, eventName, @@ -459,7 +461,7 @@ export class CSVFileWatcher { const message = isRolePresent ? 'Updated role: deleted members' : 'Deleted role'; - await this.auditLogger.auditLog({ + await this.auditLogger.auditLog({ actorId: RBAC_BACKEND, message, eventName, diff --git a/plugins/rbac-backend/src/helper.ts b/plugins/rbac-backend/src/helper.ts index c3bff21911..98334e1621 100644 --- a/plugins/rbac-backend/src/helper.ts +++ b/plugins/rbac-backend/src/helper.ts @@ -17,6 +17,7 @@ import { import { HANDLE_RBAC_DATA_STAGE, RBAC_BACKEND, + RoleAuditInfo, RoleEvents, } from './audit-log/audit-logger'; import { EnforcerDelegate } from './service/enforcer-delegate'; @@ -73,7 +74,7 @@ export async function removeTheDifference( remainingMembers.length > 0 ? RoleEvents.UPDATE_ROLE : RoleEvents.DELETE_ROLE; - await auditLogger.auditLog({ + await auditLogger.auditLog({ actorId: RBAC_BACKEND, message, eventName, diff --git a/plugins/rbac-backend/src/service/permission-policy.ts b/plugins/rbac-backend/src/service/permission-policy.ts index fe73cbf43a..d0db6fa2a3 100644 --- a/plugins/rbac-backend/src/service/permission-policy.ts +++ b/plugins/rbac-backend/src/service/permission-policy.ts @@ -31,8 +31,10 @@ import { EVALUATE_PERMISSION_ACCESS_STAGE, EvaluationEvents, HANDLE_RBAC_DATA_STAGE, + PermissionAuditInfo, PermissionEvents, RBAC_BACKEND, + RoleAuditInfo, RoleEvents, } from '../audit-log/audit-logger'; import { ConditionalStorage } from '../database/conditional-storage'; @@ -107,7 +109,7 @@ const useAdminsFromConfig = async ( getAdminRoleMetadata(), ); - await auditLogger.auditLog({ + await auditLogger.auditLog({ actorId: RBAC_BACKEND, message: `Created or updated role`, eventName: RoleEvents.CREATE_OR_UPDATE_ROLE, @@ -147,7 +149,7 @@ const addAdminPermission = async ( ) => { await enf.addOrUpdatePolicy(policy, 'configuration'); - await auditLogger.auditLog({ + await auditLogger.auditLog({ actorId: RBAC_BACKEND, message: `Created or updated policy`, eventName: PermissionEvents.CREATE_OR_UPDATE_POLICY, diff --git a/plugins/rbac-backend/src/service/policies-rest-api.ts b/plugins/rbac-backend/src/service/policies-rest-api.ts index 8145bba2ee..35e1bae052 100644 --- a/plugins/rbac-backend/src/service/policies-rest-api.ts +++ b/plugins/rbac-backend/src/service/policies-rest-api.ts @@ -47,10 +47,13 @@ import { import { PluginIdProvider } from '@janus-idp/backstage-plugin-rbac-node'; import { + ConditionAuditInfo, ConditionEvents, ListConditionEvents, ListPluginPoliciesEvents, + PermissionAuditInfo, PermissionEvents, + RoleAuditInfo, RoleEvents, SEND_RESPONSE_STAGE, } from '../audit-log/audit-logger'; @@ -256,7 +259,7 @@ export class PoliciesServer { await this.enforcer.removePolicies(processedPolicies); - await this.aLog.auditLog({ + await this.aLog.auditLog({ message: `Deleted permission policies`, eventName: PermissionEvents.DELETE_POLICY, metadata: { policies: processedPolicies, source: 'rest' }, @@ -297,7 +300,7 @@ export class PoliciesServer { await this.enforcer.addPolicies(processedPolicies, 'rest'); - await this.aLog.auditLog({ + await this.aLog.auditLog({ message: `Created permission policies`, eventName: PermissionEvents.CREATE_POLICY, metadata: { policies: processedPolicies, source: 'rest' }, @@ -385,7 +388,7 @@ export class PoliciesServer { 'rest', ); - await this.aLog.auditLog({ + await this.aLog.auditLog({ message: `Updated permission policies`, eventName: PermissionEvents.UPDATE_POLICY, metadata: { policies: processedNewPolicy, source: 'rest' }, @@ -523,7 +526,7 @@ export class PoliciesServer { await this.enforcer.addGroupingPolicies(roles, metadata); - await this.aLog.auditLog({ + await this.aLog.auditLog({ message: `Created ${metadata.roleEntityRef}`, eventName: RoleEvents.CREATE_ROLE, metadata: { @@ -663,7 +666,7 @@ export class PoliciesServer { if (newMetadata.roleEntityRef !== oldMetadata.roleEntityRef) { message = `${message}. Role entity reference renamed to ${newMetadata.roleEntityRef}`; } - await this.aLog.auditLog({ + await this.aLog.auditLog({ message, eventName: RoleEvents.UPDATE_ROLE, metadata: { @@ -733,7 +736,7 @@ export class PoliciesServer { false, ); - await this.aLog.auditLog({ + await this.aLog.auditLog({ message: `Deleted ${metadata.roleEntityRef}`, eventName: RoleEvents.DELETE_ROLE, metadata: { @@ -863,10 +866,10 @@ export class PoliciesServer { const body = { id: id }; - await this.aLog.auditLog({ + await this.aLog.auditLog({ message: `Created conditional permission policy`, eventName: ConditionEvents.CREATE_CONDITION, - metadata: { condition: conditionToCreate }, + metadata: { condition: roleConditionPolicy }, stage: SEND_RESPONSE_STAGE, status: 'succeeded', request, @@ -942,7 +945,7 @@ export class PoliciesServer { await this.conditionalStorage.deleteCondition(id); - await this.aLog.auditLog({ + await this.aLog.auditLog({ message: `Deleted conditional permission policy`, eventName: ConditionEvents.DELETE_CONDITION, metadata: { condition: conditionToDelete }, @@ -983,10 +986,10 @@ export class PoliciesServer { await this.conditionalStorage.updateCondition(id, conditionToUpdate); - await this.aLog.auditLog({ + await this.aLog.auditLog({ message: `Updated conditional permission policy`, eventName: ConditionEvents.UPDATE_CONDITION, - metadata: { conditionId: id, condition: conditionToUpdate }, + metadata: { conditionId: id, condition: roleConditionPolicy }, stage: SEND_RESPONSE_STAGE, status: 'succeeded', request,