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

[backend/frontend] Danger zone: Remove FF (#8284) #8905

Merged
merged 4 commits into from
Nov 7, 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
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const DangerZoneBlock: FunctionComponent<DangerZoneBlockProps> = ({ title, compo
if (isSensitive) {
currentTitle = (
<>
{title}<DangerZoneChip style={{ marginTop: 0 }} />
{title}<DangerZoneChip />
</>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ const RulesListComponent = ({ relay, data, keyword }) => {
<DangerZoneBlock
type={'rules'}
title={t_i18n(rule.name)}
sx={{ title: { textWrap: 'nowrap' } }}
sx={{ title: { textWrap: 'nowrap', display: 'flex', alignItems: 'center' } }}
component={({ disabled, style }) => (
<Paper
variant="outlined"
Expand Down Expand Up @@ -549,7 +549,7 @@ const RulesListComponent = ({ relay, data, keyword }) => {
<Paper
variant="outlined"
classes={{ root: classes.paper }}
style={{ marginTop: 25 }}
style={{ marginTop: 23, overflowX: 'auto' }}
>
<div className={classes.definition}>
<div className={classes.left}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ const Group = ({ groupData }: { groupData: Group_group$key }) => {
</Grid>
<GroupEdition
groupId={group.id}
disabled={!isAllowed}
disabled={!isAllowed && isSensitive}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change ?
I would naively think that isAllowed is always true if isSensitive is false (a non-sensitive zone is always granted).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the edit button was disabled for non sensitive Group/Role if user was not allowed (not allowed to modified sensitive Group/Role )

/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ const Role = ({
return (
<RoleEdition
role={props.role}
disabled={!isAllowed}
disabled={!isAllowed && isSensitive}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,42 +1,15 @@
import { describe, it, vi, expect, afterAll } from 'vitest';
import { describe, it, expect } from 'vitest';
import { UserContextType } from './useAuth';
import { createMockUserContext, testRenderHook } from '../tests/test-render';
import useSensitiveModifications from './useSensitiveModifications';

let FEATURE_ENABLED = false;

vi.mock('./useHelper.ts', () => {
return {
default: () => {
return {
isFeatureEnable: vi.fn().mockReturnValue(FEATURE_ENABLED),
};
},
};
});

describe('Hook: useSensitiveModifications', () => {
const baseUserContext = {
me: { can_manage_sensitive_config: false },
settings: { platform_protected_sensitive_config: { enabled: true } },
} as unknown as UserContextType;

afterAll(() => {
vi.restoreAllMocks();
});

it('should be allowed if sensitive feature disabled', () => {
const { hook } = testRenderHook(
() => useSensitiveModifications(),
{ userContext: createMockUserContext(baseUserContext) },
);
const { isAllowed, isSensitive } = hook.result.current;
expect(isAllowed).toEqual(true);
expect(isSensitive).toEqual(false);
});
it('should be allowed if sensitive config disabled', () => {
FEATURE_ENABLED = true;

const { hook } = testRenderHook(
() => useSensitiveModifications(),
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import useAuth from './useAuth';
import useHelper from './useHelper';

const PROTECT_SENSITIVE_CHANGES_FF = 'PROTECT_SENSITIVE_CHANGES';

export type SensitiveConfigType = 'ce_ee_toggle' | 'file_indexing' | 'groups' | 'markings' | 'platform_organization' | 'roles' | 'rules';

const useSensitiveModifications = (type?: SensitiveConfigType, id?: string) => {
const { me, settings } = useAuth();
const sensitiveConfig = settings.platform_protected_sensitive_config;

const { isFeatureEnable } = useHelper();
const isSensitiveConfigEnabled = isFeatureEnable(PROTECT_SENSITIVE_CHANGES_FF) && sensitiveConfig.enabled;
const isSensitiveConfigEnabled = sensitiveConfig.enabled;

if (!isSensitiveConfigEnabled) {
return {
Expand Down
1 change: 0 additions & 1 deletion opencti-platform/opencti-graphql/config/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"FILIGRAN_LOADER",
"CONTAINERS_AUTHORIZED_MEMBERS",
"TELEMETRY_COUNT_ACTIVE_USERS",
"PROTECT_SENSITIVE_CHANGES",
"CONTENT_FROM_TEMPLATE"
],
"https_cert": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isFeatureEnabled, logApp } from '../config/conf';
import { logApp } from '../config/conf';

Check warning on line 1 in opencti-platform/opencti-graphql/src/database/data-initialization.js

View check run for this annotation

Codecov / codecov/patch

opencti-platform/opencti-graphql/src/database/data-initialization.js#L1

Added line #L1 was not covered by tests
import { addSettings } from '../domain/settings';
import { BYPASS, ROLE_ADMINISTRATOR, ROLE_DEFAULT, SYSTEM_USER } from '../utils/access';
import { initCreateEntitySettings } from '../modules/entitySetting/entitySetting-domain';
Expand All @@ -10,7 +10,7 @@
import { builtInOv, openVocabularies } from '../modules/vocabulary/vocabulary-utils';
import { addVocabulary } from '../modules/vocabulary/vocabulary-domain';
import { addAllowedMarkingDefinition } from '../domain/markingDefinition';
import { addCapability, addGroup, addRole, PROTECT_SENSITIVE_CHANGES_FF } from '../domain/grant';
import { addCapability, addGroup, addRole } from '../domain/grant';
import { GROUP_DEFAULT, groupAddRelation } from '../domain/group';
import { TAXIIAPI } from '../domain/user';
import { KNOWLEDGE_COLLABORATION, KNOWLEDGE_DELETE, KNOWLEDGE_FRONTEND_EXPORT, KNOWLEDGE_MANAGE_AUTH_MEMBERS, KNOWLEDGE_UPDATE } from '../schema/general';
Expand Down Expand Up @@ -244,17 +244,12 @@
await createCapabilities(context, CAPABILITIES);

// Create Default(s) Role and Group
let defaultRoleInput = await addRole(context, SYSTEM_USER, {
const defaultRoleInput = await addRole(context, SYSTEM_USER, {

Check warning on line 247 in opencti-platform/opencti-graphql/src/database/data-initialization.js

View check run for this annotation

Codecov / codecov/patch

opencti-platform/opencti-graphql/src/database/data-initialization.js#L247

Added line #L247 was not covered by tests
name: ROLE_DEFAULT,
description: 'Default role associated to the default group',
capabilities: [KNOWLEDGE_CAPABILITY],
can_manage_sensitive_config: false,

Check warning on line 251 in opencti-platform/opencti-graphql/src/database/data-initialization.js

View check run for this annotation

Codecov / codecov/patch

opencti-platform/opencti-graphql/src/database/data-initialization.js#L251

Added line #L251 was not covered by tests
});
if (isFeatureEnabled((PROTECT_SENSITIVE_CHANGES_FF))) {
defaultRoleInput = {
...defaultRoleInput,
can_manage_sensitive_config: false
};
}

const defaultGroup = await addGroup(context, SYSTEM_USER, {
name: GROUP_DEFAULT,
Expand All @@ -268,17 +263,13 @@
await groupAddRelation(context, SYSTEM_USER, defaultGroup.id, defaultRoleRelationInput);

// Create Administrator(s) Role and Group
let administratorRoleInput = {
const administratorRoleInput = {

Check warning on line 266 in opencti-platform/opencti-graphql/src/database/data-initialization.js

View check run for this annotation

Codecov / codecov/patch

opencti-platform/opencti-graphql/src/database/data-initialization.js#L266

Added line #L266 was not covered by tests
name: ROLE_ADMINISTRATOR,
description: 'Administrator role that bypass every capabilities',
capabilities: [BYPASS],
can_manage_sensitive_config: false,

Check warning on line 270 in opencti-platform/opencti-graphql/src/database/data-initialization.js

View check run for this annotation

Codecov / codecov/patch

opencti-platform/opencti-graphql/src/database/data-initialization.js#L270

Added line #L270 was not covered by tests
};
if (isFeatureEnabled((PROTECT_SENSITIVE_CHANGES_FF))) {
administratorRoleInput = {
...administratorRoleInput,
can_manage_sensitive_config: false
};
}

const administratorRole = await addRole(context, SYSTEM_USER, administratorRoleInput);

const administratorGroup = await addGroup(context, SYSTEM_USER, {
Expand All @@ -293,7 +284,7 @@
await groupAddRelation(context, SYSTEM_USER, administratorGroup.id, administratorRoleRelationInput);

// Create Connector(s) Role and Group
let connectorRoleInput = {
const connectorRoleInput = {

Check warning on line 287 in opencti-platform/opencti-graphql/src/database/data-initialization.js

View check run for this annotation

Codecov / codecov/patch

opencti-platform/opencti-graphql/src/database/data-initialization.js#L287

Added line #L287 was not covered by tests
name: 'Connector',
description: 'Connector role that has the recommended capabilities',
capabilities: [
Expand All @@ -311,15 +302,9 @@
'SETTINGS_SETMARKINGS',
'SETTINGS_SETLABELS',
],
can_manage_sensitive_config: false

Check warning on line 305 in opencti-platform/opencti-graphql/src/database/data-initialization.js

View check run for this annotation

Codecov / codecov/patch

opencti-platform/opencti-graphql/src/database/data-initialization.js#L305

Added line #L305 was not covered by tests
};

if (isFeatureEnabled((PROTECT_SENSITIVE_CHANGES_FF))) {
connectorRoleInput = {
...connectorRoleInput,
can_manage_sensitive_config: false
};
}

const connectorRole = await addRole(context, SYSTEM_USER, connectorRoleInput);
// Create default group with default role

Expand Down
19 changes: 5 additions & 14 deletions opencti-platform/opencti-graphql/src/domain/grant.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ import { ENTITY_TYPE_CAPABILITY, ENTITY_TYPE_GROUP, ENTITY_TYPE_ROLE } from '../
import { RELATION_HAS_CAPABILITY } from '../schema/internalRelationship';
import { generateStandardId } from '../schema/identifier';
import { publishUserAction } from '../listener/UserActionListener';
import { isFeatureEnabled } from '../config/conf';

export const PROTECT_SENSITIVE_CHANGES_FF = 'PROTECT_SENSITIVE_CHANGES';

export const addCapability = async (context, user, capability) => {
return createEntity(context, user, capability, ENTITY_TYPE_CAPABILITY);
Expand All @@ -20,17 +17,11 @@ export const addRole = async (context, user, role) => {
dissoc('capabilities'),
)(role);

let completeRoleToCreate;
if (isFeatureEnabled(PROTECT_SENSITIVE_CHANGES_FF)) {
completeRoleToCreate = {
...roleToCreate,
can_manage_sensitive_config: role.can_manage_sensitive_config ?? false, // default when undefined is false
};
} else {
completeRoleToCreate = {
...roleToCreate
};
}
const completeRoleToCreate = {
...roleToCreate,
can_manage_sensitive_config: role.can_manage_sensitive_config ?? false, // default when undefined is false
};

const { element, isCreation } = await createEntity(context, user, completeRoleToCreate, ENTITY_TYPE_ROLE, { complete: true });
for (let index = 0; index < capabilities.length; index += 1) {
const capability = capabilities[index];
Expand Down
7 changes: 2 additions & 5 deletions opencti-platform/opencti-graphql/src/domain/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import {
} from '../utils/access';
import { ASSIGNEE_FILTER, CREATOR_FILTER, PARTICIPANT_FILTER } from '../utils/filtering/filtering-constants';
import { now, utcDate } from '../utils/format';
import { addGroup, PROTECT_SENSITIVE_CHANGES_FF } from './grant';
import { addGroup } from './grant';
import { defaultMarkingDefinitionsFromGroups, findAll as findGroups } from './group';
import { addIndividual } from './individual';
import { ENTITY_TYPE_IDENTITY_ORGANIZATION } from '../modules/organization/organization-types';
Expand Down Expand Up @@ -1376,10 +1376,7 @@ export const buildCompleteUser = async (context, client) => {
const no_creators = groups.filter((g) => g.no_creators).length === groups.length;
const restrict_delete = !isByPass && groups.filter((g) => g.restrict_delete).length === groups.length;

let canManageSensitiveConfig = null;
if (isFeatureEnabled(PROTECT_SENSITIVE_CHANGES_FF)) {
canManageSensitiveConfig = { can_manage_sensitive_config: isSensitiveChangesAllowed(client.id, roles) };
}
const canManageSensitiveConfig = { can_manage_sensitive_config: isSensitiveChangesAllowed(client.id, roles) };

return {
...client,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ const internalObjectsAttributes: { [k: string]: Array<AttributeDefinition> } = {
[ENTITY_TYPE_ROLE]: [
{ name: 'name', label: 'Name', type: 'string', format: 'short', mandatoryType: 'external', editDefault: true, multiple: false, upsert: false, isFilterable: true },
{ name: 'description', label: 'Description', type: 'string', format: 'text', mandatoryType: 'no', editDefault: false, multiple: false, upsert: false, isFilterable: true },
{ name: 'can_manage_sensitive_config', label: 'Is sensitive changes allowed', type: 'boolean', mandatoryType: 'no', editDefault: false, multiple: false, upsert: false, isFilterable: false, featureFlag: 'PROTECT_SENSITIVE_CHANGES' },
{ name: 'can_manage_sensitive_config', label: 'Is sensitive changes allowed', type: 'boolean', mandatoryType: 'no', editDefault: false, multiple: false, upsert: false, isFilterable: false },
],
[ENTITY_TYPE_RULE]: [
{ name: 'active', label: 'Status', type: 'boolean', mandatoryType: 'no', editDefault: false, multiple: false, upsert: true, isFilterable: true }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { ADMIN_USER, testContext, queryAsAdmin, TESTING_ROLES } from '../../util
import { elLoadById } from '../../../src/database/engine';
import { ENTITY_TYPE_CAPABILITY } from '../../../src/schema/internalObject';
import { generateStandardId } from '../../../src/schema/identifier';
import { isFeatureEnabled } from '../../../src/config/conf';
import { PROTECT_SENSITIVE_CHANGES_FF } from '../../../src/domain/grant';

const LIST_QUERY = gql`
query roles($first: Int, $after: ID, $orderBy: RolesOrdering, $orderMode: OrderingMode, $search: String) {
Expand Down Expand Up @@ -59,11 +57,7 @@ describe('Role resolver standard behavior', () => {
expect(role).not.toBeNull();
expect(role.data.roleAdd).not.toBeNull();
expect(role.data.roleAdd.name).toEqual('Role');
if (isFeatureEnabled(PROTECT_SENSITIVE_CHANGES_FF)) {
expect(role.data.roleAdd.can_manage_sensitive_config).toBeFalsy('New role should have the can_manage_sensitive_config to false by default');
} else {
expect(role.data.roleAdd.can_manage_sensitive_config).toBeUndefined('New role should not have the can_manage_sensitive_config when it is not enabled');
}
expect(role.data.roleAdd.can_manage_sensitive_config).toBeFalsy('New role should have the can_manage_sensitive_config to false by default');

roleInternalId = role.data.roleAdd.id;
});
Expand Down