feat(rbac): add type checks with generics for audit log (#1789)

* feat(rbac): add type checks with generics for audit log Signed-off-by: Oleksandr Andriienko <[email protected]> * feat(rbac): add id for condition delete event Signed-off-by: Oleksandr Andriienko <[email protected]> * feat(rbac): clean up unused import Signed-off-by: Oleksandr Andriienko <[email protected]> * feat(rbac): delete condition id it is present in the condition Signed-off-by: Oleksandr Andriienko <[email protected]> --------- Signed-off-by: Oleksandr Andriienko <[email protected]>
janus-idp · Jun 5, 2024 · ac69838 · ac69838
1 parent acaf2b4
commit ac69838
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 32 deletions.
diff --git 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<T extends JsonValue>(
+    options: AuditLogDetailsOptions<T>,
   ): Promise<AuditLogDetails> {
     const { eventName, stage, metadata, request, response, status } = options;
 
@@ -106,7 +106,9 @@ export class DefaultAuditLogger implements AuditLogger {
       status,
     };
   }
-  async auditLog(options: AuditLogOptions): Promise<void> {
+  async auditLog<T extends JsonValue>(
+    options: AuditLogOptions<T>,
+  ): Promise<void> {
     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
@@ -54,22 +54,22 @@ export type AuditLogDetails = {
   isAuditLog: true;
 } & AuditLogStatus;
 
-export type AuditLogDetailsOptions = {
+export type AuditLogDetailsOptions<T extends JsonValue> = {
   eventName: string;
   stage: string;
-  metadata?: JsonValue;
+  metadata?: T;
   response?: AuditResponse;
   actorId?: string;
   request?: Request;
 } & (AuditLogSuccessStatus | AuditLogUnknownFailureStatus);
 
-export type AuditLogOptions = {
+export type AuditLogOptions<T extends JsonValue> = {
   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<T extends JsonValue>(
+    options: AuditLogDetailsOptions<T>,
   ): Promise<AuditLogDetails>;
 
   /**
@@ -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<void>;
+  auditLog<T extends JsonValue>(options: AuditLogOptions<T>): Promise<void>;
 }
diff --git a/plugins/rbac-backend/src/audit-log/audit-logger.ts b/plugins/rbac-backend/src/audit-log/audit-logger.ts
@@ -8,7 +8,6 @@ import { PolicyQuery } from '@backstage/plugin-permission-node';
 import { AuditLogOptions } from '@janus-idp/backstage-plugin-audit-log-node';
 import {
   PermissionAction,
-  PermissionInfo,
   RoleConditionalPolicyDecision,
   Source,
   toPermissionAction,
@@ -91,7 +90,8 @@ export const ConditionEvents = {
 };
 
 export type ConditionAuditInfo = {
-  condition: RoleConditionalPolicyDecision<PermissionInfo>;
+  conditionId?: number;
+  condition: RoleConditionalPolicyDecision<PermissionAction>;
 };
 
 export const RBAC_BACKEND = 'rbac-backend';
@@ -111,7 +111,7 @@ export function createPermissionEvaluationOptions(
   userEntityRef: string,
   request: PolicyQuery,
   policyDecision?: PolicyDecision,
-): AuditLogOptions {
+): AuditLogOptions<EvaluationAuditInfo> {
   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
@@ -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<PermissionAuditInfo>({
           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<PermissionAuditInfo>({
         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<RoleAuditInfo>({
           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<RoleAuditInfo>({
           actorId: RBAC_BACKEND,
           message,
           eventName,

diff --git 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<RoleAuditInfo>({
     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
@@ -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<RoleAuditInfo>({
     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<PermissionAuditInfo>({
     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
@@ -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<PermissionAuditInfo>({
           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<PermissionAuditInfo>({
         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<PermissionAuditInfo>({
           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<RoleAuditInfo>({
         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<RoleAuditInfo>({
         message,
         eventName: RoleEvents.UPDATE_ROLE,
         metadata: {
@@ -733,7 +736,7 @@ export class PoliciesServer {
           false,
         );
 
-        await this.aLog.auditLog({
+        await this.aLog.auditLog<RoleAuditInfo>({
           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<ConditionAuditInfo>({
         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<ConditionAuditInfo>({
         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<ConditionAuditInfo>({
         message: `Updated conditional permission policy`,
         eventName: ConditionEvents.UPDATE_CONDITION,
-        metadata: { conditionId: id, condition: conditionToUpdate },
+        metadata: { condition: roleConditionPolicy },
         stage: SEND_RESPONSE_STAGE,
         status: 'succeeded',
         request,