diff --git a/frontend/packages/policy-editor/src/PolicyEditor.tsx b/frontend/packages/policy-editor/src/PolicyEditor.tsx index 87657cb9c08..a5ba826111c 100644 --- a/frontend/packages/policy-editor/src/PolicyEditor.tsx +++ b/frontend/packages/policy-editor/src/PolicyEditor.tsx @@ -178,7 +178,6 @@ export const PolicyEditor = ({ const policyEditorRules: PolicyRule[] = rules.map((pr) => mapPolicyRuleToPolicyRuleBackendObject( subjects, - actions, pr, `${resourceType}:${usageType === 'app' ? 'example' : resourceId}:ruleid:${pr.ruleId}`, // TODO - find out if ID should be hardcoded. Issue: #10893 ), diff --git a/frontend/packages/policy-editor/src/components/ExpandablePolicyCard/ExpandablePolicyCard.tsx b/frontend/packages/policy-editor/src/components/ExpandablePolicyCard/ExpandablePolicyCard.tsx index 295edf90ec0..7c9fc6555b8 100644 --- a/frontend/packages/policy-editor/src/components/ExpandablePolicyCard/ExpandablePolicyCard.tsx +++ b/frontend/packages/policy-editor/src/components/ExpandablePolicyCard/ExpandablePolicyCard.tsx @@ -18,7 +18,7 @@ import type { PolicySubject, PolicyEditorUsage, } from '../../types'; -import { createNewPolicyResource } from '../../utils'; +import { createNewPolicyResource, findSubjectByPolicyRuleSubject } from '../../utils'; import { getActionOptions, getPolicyRuleIdString, @@ -244,8 +244,13 @@ export const ExpandablePolicyCard = ({ * Displays the selected subjects */ const displaySubjects = policyRule.subject.map((s, i) => { + const subject: PolicySubject = findSubjectByPolicyRuleSubject(subjects, s); return ( - handleRemoveSubject(i, s)} /> + handleRemoveSubject(i, s)} + /> ); }); diff --git a/frontend/packages/policy-editor/src/data-mocks/policySubjectMocks.ts b/frontend/packages/policy-editor/src/data-mocks/policySubjectMocks.ts index 69be9a7b8fb..7b95416b5fc 100644 --- a/frontend/packages/policy-editor/src/data-mocks/policySubjectMocks.ts +++ b/frontend/packages/policy-editor/src/data-mocks/policySubjectMocks.ts @@ -1,23 +1,27 @@ import type { PolicySubject } from '../types'; +export const mockSubjectId1: string = 's1'; +export const mockSubjectId2: string = 's2'; +export const mockSubjectId3: string = 's3'; + export const mockSubjectTitle1: string = 'Subject 1'; export const mockSubjectTitle2: string = 'Subject 2'; export const mockSubjectTitle3: string = 'Subject 3'; export const mockSubject1: PolicySubject = { - subjectId: 's1', + subjectId: mockSubjectId1, subjectSource: 'Subject1', subjectTitle: mockSubjectTitle1, subjectDescription: '', }; export const mockSubject2: PolicySubject = { - subjectId: 's2', + subjectId: mockSubjectId2, subjectSource: 'Subject2', subjectTitle: mockSubjectTitle2, subjectDescription: '', }; export const mockSubject3: PolicySubject = { - subjectId: 's3', + subjectId: mockSubjectId3, subjectSource: 'Subject3', subjectTitle: mockSubjectTitle3, subjectDescription: '', diff --git a/frontend/packages/policy-editor/src/utils/PolicyEditorUtils.test.ts b/frontend/packages/policy-editor/src/utils/PolicyEditorUtils.test.ts index e0574b45c5f..68da2c6f1d8 100644 --- a/frontend/packages/policy-editor/src/utils/PolicyEditorUtils.test.ts +++ b/frontend/packages/policy-editor/src/utils/PolicyEditorUtils.test.ts @@ -10,6 +10,7 @@ import { convertSubjectStringToSubjectId, createNewSubjectFromSubjectString, convertSubjectStringToSubjectSource, + findSubjectByPolicyRuleSubject, } from './index'; import { mockAction1, @@ -32,10 +33,12 @@ import { mockSubject3, mockSubjectBackendString1, mockSubjectBackendString3, + mockSubjectId1, mockSubjectTitle1, mockSubjectTitle3, mockSubjects, } from '../data-mocks'; +import type { PolicySubject } from '../types'; describe('PolicyEditorUtils', () => { describe('mapPolicySubjectToSubjectTitle', () => { @@ -75,13 +78,17 @@ describe('PolicyEditorUtils', () => { expect(result).toBe(mockSubjectBackendString1); }); + + it('should return nothing when there is no subject matching the subject title', () => { + const result = mapSubjectTitleToSubjectString(mockSubjects, 'invalidTitle'); + expect(result).toBe(undefined); + }); }); describe('mapPolicyRuleToPolicyRuleBackendObject', () => { it('should map a policy rule card to a policy rule backend object', () => { const result = mapPolicyRuleToPolicyRuleBackendObject( mockSubjects, - mockActions, mockPolicyRuleCard1, mockRuleId1, ); @@ -163,4 +170,17 @@ describe('PolicyEditorUtils', () => { expect(subjectSource).toBe(mockSubject1.subjectSource); }); }); + + describe('findSubjectByPolicyRuleSubject', () => { + it('returns a subject when the policy rule subject is in the subject options list', () => { + const subject: PolicySubject = findSubjectByPolicyRuleSubject(mockSubjects, mockSubjectId1); + expect(subject.subjectTitle).toEqual(mockSubjectTitle1); + expect(subject.subjectId).toEqual(mockSubjectId1); + }); + + it('returns undefined when the policy rule subject is not in the subject options list', () => { + const subject: PolicySubject = findSubjectByPolicyRuleSubject(mockSubjects, 's4'); + expect(subject).toEqual(undefined); + }); + }); }); diff --git a/frontend/packages/policy-editor/src/utils/index.ts b/frontend/packages/policy-editor/src/utils/index.ts index 8ffc0868ed0..5fd9051e860 100644 --- a/frontend/packages/policy-editor/src/utils/index.ts +++ b/frontend/packages/policy-editor/src/utils/index.ts @@ -42,6 +42,7 @@ export const mapPolicySubjectToSubjectTitle = ( return subjectOption?.subjectTitle || subjectId; }); }; + const findSubjectOptionBySubjectId = ( subjectOptions: PolicySubject[], subjectId: string, @@ -115,6 +116,7 @@ export const mapSubjectTitleToSubjectString = ( const subject: PolicySubject = subjectOptions.find( (s) => s.subjectTitle.toLowerCase() === subjectTitle.toLowerCase(), ); + if (!subject) return; return `urn:${subject.subjectSource}:${subject.subjectId}`; }; @@ -123,7 +125,6 @@ export const mapSubjectTitleToSubjectString = ( * to be sent to backend where all fields are strings. * * @param subjectOptions the possible subjects to select from - * @param actionOptions the possible actions to select from * @param policyRule the policy rule to map * @param ruleId the id of the rule * @@ -131,7 +132,6 @@ export const mapSubjectTitleToSubjectString = ( */ export const mapPolicyRuleToPolicyRuleBackendObject = ( subjectOptions: PolicySubject[], - actionOptions: PolicyAction[], policyRule: PolicyRuleCard, ruleId: string, ): PolicyRule => { @@ -231,8 +231,7 @@ export const mergeSubjectsFromPolicyWithSubjectOptions = ( rules.forEach((rule) => { rule.subject.forEach((subjectString) => { const subjectId = convertSubjectStringToSubjectId(subjectString); - - if (!existingSubjectIds.includes(subjectId)) { + if (!existingSubjectIds.includes(subjectId.toLowerCase())) { const newSubject: PolicySubject = createNewSubjectFromSubjectString(subjectString); copiedSubjects.push(newSubject); existingSubjectIds.push(subjectId); @@ -265,3 +264,12 @@ export const convertSubjectStringToSubjectSource = (subjectString: string): stri // Starting at 1 to remove 'urn', and excluding the final to remove the id return subjectString.slice(firstColonIndex + 1, lastColonIndex); }; + +export const findSubjectByPolicyRuleSubject = ( + subjectOptions: PolicySubject[], + policyRuleSubject: string, +): PolicySubject => { + return subjectOptions.find( + (subject) => subject.subjectId.toLowerCase() === policyRuleSubject.toLowerCase(), + ); +};