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

Fix dagl and regna bug in policy editor #12247

Merged
13 commits merged into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from 11 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
1 change: 0 additions & 1 deletion frontend/packages/policy-editor/src/PolicyEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
PolicySubject,
PolicyEditorUsage,
} from '../../types';
import { createNewPolicyResource } from '../../utils';
import { createNewPolicyResource, findSubjectByPolicyRuleSubject } from '../../utils';
import {
getActionOptions,
getPolicyRuleIdString,
Expand Down Expand Up @@ -244,8 +244,13 @@
* Displays the selected subjects
*/
const displaySubjects = policyRule.subject.map((s, i) => {
const subject: PolicySubject = findSubjectByPolicyRuleSubject(subjects, s);
return (
<ActionAndSubjectListItem key={i} title={s} onRemove={() => handleRemoveSubject(i, s)} />
<ActionAndSubjectListItem
key={i}
title={subject?.subjectTitle || s}
onRemove={() => handleRemoveSubject(i, s)}

Check warning on line 252 in frontend/packages/policy-editor/src/components/ExpandablePolicyCard/ExpandablePolicyCard.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/packages/policy-editor/src/components/ExpandablePolicyCard/ExpandablePolicyCard.tsx#L252

Added line #L252 was not covered by tests
/>
);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -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: '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
convertSubjectStringToSubjectId,
createNewSubjectFromSubjectString,
convertSubjectStringToSubjectSource,
findSubjectByPolicyRuleSubject,
} from './index';
import {
mockAction1,
Expand All @@ -32,10 +33,12 @@ import {
mockSubject3,
mockSubjectBackendString1,
mockSubjectBackendString3,
mockSubjectId1,
mockSubjectTitle1,
mockSubjectTitle3,
mockSubjects,
} from '../data-mocks';
import type { PolicySubject } from '../types';

describe('PolicyEditorUtils', () => {
describe('mapPolicySubjectToSubjectTitle', () => {
Expand Down Expand Up @@ -75,13 +78,17 @@ describe('PolicyEditorUtils', () => {

expect(result).toBe(mockSubjectBackendString1);
});

it('should return nothing when there is no subject mathing the subject title', () => {
This conversation was marked as resolved.
Show resolved Hide resolved
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,
);
Expand Down Expand Up @@ -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);
});
});
});
16 changes: 12 additions & 4 deletions frontend/packages/policy-editor/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const mapPolicySubjectToSubjectTitle = (
return subjectOption?.subjectTitle || subjectId;
});
};

const findSubjectOptionBySubjectId = (
subjectOptions: PolicySubject[],
subjectId: string,
Expand Down Expand Up @@ -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}`;
};

Expand All @@ -123,15 +125,13 @@ 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
*
* @returns a mapped object ready to be sent to backend
*/
export const mapPolicyRuleToPolicyRuleBackendObject = (
subjectOptions: PolicySubject[],
actionOptions: PolicyAction[],
policyRule: PolicyRuleCard,
ruleId: string,
): PolicyRule => {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(),
);
};
Loading