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

Prevent deletion of event definitions which are still being referenced #14792

Merged
merged 17 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelog/unreleased/issue-14302.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
type = "c"
message = "Prevent deletion of event definitions that are still referenced in other definitions."

issues = ["14302"]
pulls = ["#14792", "Graylog2/graylog-plugin-enterprise#4765"]

details.user = """
An event definition d1 may be referenced from another definition d2, specifically a correlation event.
Deleting d1 at this time results in unexpected behavior. We now block deletion and show a warning,
displaying the list of dependent events from which the definition must be removed, prior to deletion.
"""
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.graylog.events;

import com.google.inject.assistedinject.FactoryModuleBuilder;
import com.google.inject.multibindings.OptionalBinder;
import org.graylog.events.audit.EventsAuditEventTypes;
import org.graylog.events.contentpack.entities.AggregationEventProcessorConfigEntity;
import org.graylog.events.contentpack.entities.EmailEventNotificationConfigEntity;
Expand All @@ -39,9 +40,11 @@
import org.graylog.events.notifications.types.HTTPEventNotification;
import org.graylog.events.notifications.types.HTTPEventNotificationConfig;
import org.graylog.events.periodicals.EventNotificationStatusCleanUp;
import org.graylog.events.processor.DefaultEventResolver;
import org.graylog.events.processor.EventProcessorEngine;
import org.graylog.events.processor.EventProcessorExecutionJob;
import org.graylog.events.processor.EventProcessorExecutionMetrics;
import org.graylog.events.processor.EventResolver;
import org.graylog.events.processor.aggregation.AggregationEventProcessor;
import org.graylog.events.processor.aggregation.AggregationEventProcessorConfig;
import org.graylog.events.processor.aggregation.AggregationEventProcessorParameters;
Expand Down Expand Up @@ -85,6 +88,9 @@ protected void configure() {
bind(EventNotificationExecutionMetrics.class).asEagerSingleton();
bind(SystemNotificationRenderService.class).asEagerSingleton();

OptionalBinder.newOptionalBinder(binder(), EventResolver.class)
.setDefault().to(DefaultEventResolver.class);

addEntityScope(SystemNotificationEventEntityScope.class);

addSystemRestResource(AvailableEntityTypesResource.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.slf4j.LoggerFactory;

import javax.inject.Inject;
import javax.validation.constraints.NotNull;
import java.util.List;
import java.util.Locale;
import java.util.function.Predicate;
Expand Down Expand Up @@ -124,4 +125,12 @@ public List<EventDefinitionDto> getByNotificationId(String notificationId) {
public List<EventDefinitionDto> getSystemEventDefinitions() {
return ImmutableList.copyOf((db.find(DBQuery.is(EventDefinitionDto.FIELD_SCOPE, SystemNotificationEventEntityScope.NAME)).iterator()));
}

/**
* Returns the list of event definitions that contain the given value in the specified array field
*/
@NotNull
public List<EventDefinitionDto> getByArrayValue(String arrayField, String field, String value) {
return ImmutableList.copyOf((db.find(DBQuery.elemMatch(arrayField, DBQuery.is(field, value))).iterator()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (C) 2020 Graylog, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the Server Side Public License, version 1,
* as published by MongoDB, Inc.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* Server Side Public License for more details.
*
* You should have received a copy of the Server Side Public License
* along with this program. If not, see
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
package org.graylog.events.processor;

import java.util.ArrayList;
import java.util.List;

public class DefaultEventResolver implements EventResolver {
@Override
public List<EventDefinitionDto> dependentEvents(String definitionId) {
return new ArrayList<>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public abstract class EventDefinitionDto extends ScopedEntity implements EventDe
public static final String FIELD_NOTIFICATIONS = "notifications";
private static final String FIELD_PRIORITY = "priority";
private static final String FIELD_ALERT = "alert";
private static final String FIELD_CONFIG = "config";
public static final String FIELD_CONFIG = "config";
private static final String FIELD_FIELD_SPEC = "field_spec";
private static final String FIELD_KEY_SPEC = "key_spec";
private static final String FIELD_NOTIFICATION_SETTINGS = "notification_settings";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright (C) 2020 Graylog, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the Server Side Public License, version 1,
* as published by MongoDB, Inc.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* Server Side Public License for more details.
*
* You should have received a copy of the Server Side Public License
* along with this program. If not, see
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
package org.graylog.events.processor;

import javax.validation.constraints.NotNull;
import java.util.List;

/**
* Resolves dependencies between events
*/
public interface EventResolver {
/**
* Returns IDs of dependent event definitions
*
* @param definitionId an event definition ID
* @return the dependent event definitions
*/
@NotNull
List<EventDefinitionDto> dependentEvents(String definitionId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
import org.apache.commons.lang.StringUtils;
import org.apache.shiro.authz.annotation.RequiresAuthentication;
import org.apache.shiro.authz.annotation.RequiresPermissions;
import org.graylog.events.audit.EventsAuditEventTypes;
Expand All @@ -35,6 +36,7 @@
import org.graylog.events.processor.EventProcessorException;
import org.graylog.events.processor.EventProcessorParameters;
import org.graylog.events.processor.EventProcessorParametersWithTimerange;
import org.graylog.events.processor.EventResolver;
import org.graylog.grn.GRNTypes;
import org.graylog.plugins.views.startpage.recentActivities.RecentActivityService;
import org.graylog.security.UserContext;
Expand All @@ -43,6 +45,7 @@
import org.graylog2.audit.jersey.NoAuditEvent;
import org.graylog2.database.PaginatedList;
import org.graylog2.plugin.rest.PluginRestResource;
import org.graylog2.plugin.rest.ValidationFailureException;
import org.graylog2.plugin.rest.ValidationResult;
import org.graylog2.rest.bulk.AuditParams;
import org.graylog2.rest.bulk.BulkExecutor;
Expand Down Expand Up @@ -126,6 +129,7 @@ public class EventDefinitionsResource extends RestResource implements PluginRest
private final BulkExecutor<EventDefinitionDto, UserContext> bulkDeletionExecutor;
private final BulkExecutor<EventDefinitionDto, UserContext> bulkScheduleExecutor;
private final BulkExecutor<EventDefinitionDto, UserContext> bulkUnscheduleExecutor;
private final EventResolver eventResolver;

@Inject
public EventDefinitionsResource(DBEventDefinitionService dbService,
Expand All @@ -134,7 +138,8 @@ public EventDefinitionsResource(DBEventDefinitionService dbService,
EventProcessorEngine engine,
RecentActivityService recentActivityService,
AuditEventSender auditEventSender,
ObjectMapper objectMapper
ObjectMapper objectMapper,
EventResolver eventResolver
) {
this.dbService = dbService;
this.eventDefinitionHandler = eventDefinitionHandler;
Expand All @@ -145,6 +150,7 @@ public EventDefinitionsResource(DBEventDefinitionService dbService,
this.bulkDeletionExecutor = new SequentialBulkExecutor<>(this::delete, auditEventSender, objectMapper);
this.bulkScheduleExecutor = new SequentialBulkExecutor<>(this::schedule, auditEventSender, objectMapper);
this.bulkUnscheduleExecutor = new SequentialBulkExecutor<>(this::unschedule, auditEventSender, objectMapper);
this.eventResolver = eventResolver;
}


Expand Down Expand Up @@ -228,9 +234,9 @@ public EventDefinitionDto get(@ApiParam(name = "definitionId") @PathParam("defin
public Map<String, Object> getWithContext(@ApiParam(name = "definitionId") @PathParam("definitionId") @NotBlank String definitionId) {
checkPermission(RestPermissions.EVENT_DEFINITIONS_READ, definitionId);
return dbService.get(definitionId)
.map(evenDefinition -> ImmutableMap.of(
"event_definition", evenDefinition,
"context", contextService.contextFor(evenDefinition)
.map(eventDefinition -> ImmutableMap.of(
"event_definition", eventDefinition,
"context", contextService.contextFor(eventDefinition)
))
.orElseThrow(() -> new NotFoundException("Event definition <" + definitionId + "> doesn't exist"));
}
Expand Down Expand Up @@ -286,7 +292,21 @@ public Response update(@ApiParam(name = "definitionId") @PathParam("definitionId
public EventDefinitionDto delete(@ApiParam(name = "definitionId") @PathParam("definitionId") @NotBlank String definitionId,
@Context UserContext userContext) {
checkPermission(RestPermissions.EVENT_DEFINITIONS_DELETE, definitionId);

final Optional<EventDefinitionDto> eventDefinitionDto = dbService.get(definitionId);
final String dependencyTitle = eventDefinitionDto.isPresent() ? eventDefinitionDto.get().title() : definitionId;
thll marked this conversation as resolved.
Show resolved Hide resolved
final List<EventDefinitionDto> dependentEventDtoList = eventResolver.dependentEvents(definitionId);
if (!dependentEventDtoList.isEmpty()) {
final List<String> dependenciesTitles = dependentEventDtoList.stream().map(EventDefinitionDto::title).toList();
final List<String> dependenciesIds = dependentEventDtoList.stream().map(EventDefinitionDto::id).toList();
String msg = "Unable to delete event definition <" + dependencyTitle
+ "> - please remove all references from event definitions: " + StringUtils.join(dependenciesTitles, ",");
ValidationResult validationResult = new ValidationResult()
.addError("dependency", msg)
.addContext("dependency_ids", dependenciesIds);
throw new ValidationFailureException(validationResult, msg);
}

eventDefinitionDto.ifPresent(d ->
recentActivityService.delete(d.id(), GRNTypes.EVENT_DEFINITION, d.title(), userContext.getUser())
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ public ValidationFailureException(ValidationResult validationResult) {
this.validationResult = validationResult;
}

public ValidationFailureException(ValidationResult validationResult, String message) {
super(message);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that message used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need that message to return an error result from EventDefinitionsResource::delete.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm missing something, it's really not being used, so I think we should rather remove that constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

throw new ValidationFailureException(validationResult, msg);

this.validationResult = validationResult;
}

public ValidationResult getValidationResult() {
return validationResult;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.graylog.events.context.EventDefinitionContextService;
import org.graylog.events.notifications.EventNotificationSettings;
import org.graylog.events.processor.DBEventDefinitionService;
import org.graylog.events.processor.DefaultEventResolver;
import org.graylog.events.processor.EventDefinitionDto;
import org.graylog.events.processor.EventDefinitionHandler;
import org.graylog.events.processor.EventProcessorConfig;
Expand Down Expand Up @@ -62,11 +63,14 @@ public class EventDefinitionsResourceTest {
AuditEventSender auditEventSender;
@Mock
ObjectMapper objectMapper;

EventDefinitionsResource resource;

@Before
public void setup() {
resource = new EventDefinitionsResource(dbService, eventDefinitionHandler, contextService, engine, recentActivityService, auditEventSender, objectMapper);
resource = new EventDefinitionsResource(
dbService, eventDefinitionHandler, contextService, engine, recentActivityService,
auditEventSender, objectMapper, new DefaultEventResolver());
when(config1.type()).thenReturn(CONFIG_TYPE_1);
when(config2.type()).thenReturn(CONFIG_TYPE_2);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { Button } from 'components/bootstrap';
import StringUtils from 'util/StringUtils';

type Props = {
selectedDefintionsIds: Array<string>,
selectedDefinitionsIds: Array<string>,
setSelectedEventDefinitionsIds: (definitionIds: Array<string>) => void
};
const ACTION_TYPES = {
Expand All @@ -40,27 +40,27 @@ const getDescriptor = (count: number) => StringUtils.pluralize(count, 'event def
const ACTION_TEXT = {
[ACTION_TYPES.DELETE]: {
dialogTitle: 'Delete Event Definitions',
dialogBody: (count: number) => `Are you sure you want to delete ${getDescriptor(count)}"?`,
dialogBody: (count: number) => `Are you sure you want to delete ${count} ${getDescriptor(count)}?`,
bulkActionUrl: ApiRoutes.EventDefinitionsApiController.bulkDelete().url,

},
[ACTION_TYPES.DISABLE]: {
dialogTitle: 'Disable Event Definitions',
dialogBody: (count: number) => `Are you sure you want to disable ${getDescriptor(count)}"?`,
dialogBody: (count: number) => `Are you sure you want to disable ${count} ${getDescriptor(count)}?`,
bulkActionUrl: ApiRoutes.EventDefinitionsApiController.bulkUnschedule().url,
},
[ACTION_TYPES.ENABLE]: {
dialogTitle: 'Enable Event Definitions',
dialogBody: (count: number) => `Are you sure you want to enable ${getDescriptor(count)}"?`,
dialogBody: (count: number) => `Are you sure you want to enable ${count} ${getDescriptor(count)}?`,
bulkActionUrl: ApiRoutes.EventDefinitionsApiController.bulkSchedule().url,
},
};

const BulkActions = ({ selectedDefintionsIds, setSelectedEventDefinitionsIds }: Props) => {
const BulkActions = ({ selectedDefinitionsIds, setSelectedEventDefinitionsIds }: Props) => {
const queryClient = useQueryClient();
const [showDialog, setShowDialog] = useState(false);
const [actionType, setActionType] = useState(null);
const selectedItemsAmount = selectedDefintionsIds?.length;
const selectedItemsAmount = selectedDefinitionsIds?.length;
const refetchEventDefinitions = useCallback(() => queryClient.invalidateQueries(['eventDefinition', 'overview']), [queryClient]);

const updateState = ({ show, type }) => {
Expand Down Expand Up @@ -92,15 +92,19 @@ const BulkActions = ({ selectedDefintionsIds, setSelectedEventDefinitionsIds }:
refetchEventDefinitions();
};

const getErrorExplanation = (failures: Array<{entity_id: string, failure_explanation: string}>) => {
return failures?.reduce((acc, failure) => `${acc} ${failure?.failure_explanation}`, '');
};

const onAction = useCallback(() => {
fetch('POST',
qualifyUrl(ACTION_TEXT[actionType].bulkActionUrl),
{ entity_ids: selectedDefintionsIds },
{ entity_ids: selectedDefinitionsIds },
).then(({ failures }) => {
if (failures?.length) {
const notUpdatedDefinitionIds = failures.map(({ entity_id }) => entity_id);
setSelectedEventDefinitionsIds(notUpdatedDefinitionIds);
UserNotification.error(`${notUpdatedDefinitionIds.length} out of ${selectedItemsAmount} selected ${getDescriptor(selectedItemsAmount)} could not be ${actionType}d.`);
UserNotification.error(`${notUpdatedDefinitionIds.length} out of ${selectedItemsAmount} selected ${getDescriptor(selectedItemsAmount)} could not be ${actionType}d. ${getErrorExplanation(failures)}`);
} else {
setSelectedEventDefinitionsIds([]);
UserNotification.success(`${selectedItemsAmount} ${getDescriptor(selectedItemsAmount)} ${StringUtils.pluralize(selectedItemsAmount, 'was', 'were')} ${actionType}d successfully.`, 'Success');
Expand All @@ -111,7 +115,7 @@ const BulkActions = ({ selectedDefintionsIds, setSelectedEventDefinitionsIds }:
}).finally(() => {
refetchEventDefinitions();
});
}, [actionType, refetchEventDefinitions, selectedDefintionsIds, selectedItemsAmount, setSelectedEventDefinitionsIds]);
}, [actionType, refetchEventDefinitions, selectedDefinitionsIds, selectedItemsAmount, setSelectedEventDefinitionsIds]);

const handleConfirm = () => {
onAction();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ const EventDefinitionActions = ({ eventDefinition, refetchEventDefinitions }: Pr
`Event Definition "${eventDefinition.title}" was deleted successfully.`);
},
(error) => {
UserNotification.error(`Deleting Event Definition "${eventDefinition.title}" failed with status: ${error}`,
const errorStatus = error?.additional?.body?.errors?.dependency.join(' ') || error;

UserNotification.error(`Deleting Event Definition "${eventDefinition.title}" failed with status: ${errorStatus}`,
'Could not delete Event Definition');
},
).finally(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ const EventDefinitionsContainer = () => {
selectedEventDefinitionsIds: Array<string>,
setSelectedEventDefinitionsIds: (eventDefinitionsId: Array<string>) => void,
) => (
<BulkActions selectedDefintionsIds={selectedEventDefinitionsIds}
<BulkActions selectedDefinitionsIds={selectedEventDefinitionsIds}
setSelectedEventDefinitionsIds={setSelectedEventDefinitionsIds} />
);

Expand Down