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(rbac): add type checks with generics for audit log #1789

Merged
merged 4 commits into from
Jun 5, 2024
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
10 changes: 6 additions & 4 deletions plugins/audit-log-node/src/DefaultAuditLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 = {
Expand Down
14 changes: 7 additions & 7 deletions plugins/audit-log-node/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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>;

/**
Expand All @@ -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>;
}
6 changes: 3 additions & 3 deletions plugins/rbac-backend/src/audit-log/audit-logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -91,7 +90,8 @@ export const ConditionEvents = {
};

export type ConditionAuditInfo = {
condition: RoleConditionalPolicyDecision<PermissionInfo>;
conditionId?: number;
condition: RoleConditionalPolicyDecision<PermissionAction>;
};

export const RBAC_BACKEND = 'rbac-backend';
Expand All @@ -111,7 +111,7 @@ export function createPermissionEvaluationOptions(
userEntityRef: string,
request: PolicyQuery,
policyDecision?: PolicyDecision,
): AuditLogOptions {
): AuditLogOptions<EvaluationAuditInfo> {
const auditInfo: EvaluationAuditInfo = {
userEntityRef,
permissionName: request.permission.name,
Expand Down
10 changes: 6 additions & 4 deletions plugins/rbac-backend/src/file-permissions/csv-file-watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion plugins/rbac-backend/src/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions plugins/rbac-backend/src/service/permission-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
25 changes: 14 additions & 11 deletions plugins/rbac-backend/src/service/policies-rest-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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' },
Expand Down Expand Up @@ -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' },
Expand Down Expand Up @@ -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' },
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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,
Expand Down
Loading