Skip to content

Commit a9d80b8

Browse files
dennisoelkersdennisoelkers
authored
Turn permission error into validation error for empty streams in event definition. (#22778)
* Cleaning up `EventConditionForm`. * Turn permission error into validation error for empty streams in event definition. * Adding changelog snippet. * Fixing test. * Adjusting `AggregationEventProcessorConfigTest`. * Fixing cases where plugin types are missing. * Fixing linter hint.
1 parent 36e9a86 commit a9d80b8

12 files changed

Lines changed: 128 additions & 84 deletions

File tree

‎changelog/unreleased/pr-22778.toml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
type = "f"
2+
message = "Turn permission error into validation error for empty streams in event definition."
3+
4+
issues = ["Graylog2/graylog-plugin-enterprise#10869"]
5+
pulls = ["22778"]

‎graylog2-server/src/main/java/org/graylog/events/notifications/NotificationTestData.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.graylog.events.event.EventOriginContext;
2525
import org.graylog.events.processor.EventDefinitionDto;
2626
import org.graylog.events.processor.EventProcessorConfig;
27+
import org.graylog.security.UserContext;
2728
import org.graylog2.contentpacks.EntityDescriptorIds;
2829
import org.graylog2.plugin.Tools;
2930
import org.graylog2.plugin.rest.ValidationResult;
@@ -61,7 +62,7 @@ public String type() {
6162
return "test-dummy-v1";
6263
}
6364
@Override
64-
public ValidationResult validate() {
65+
public ValidationResult validate(UserContext userContext) {
6566
return null;
6667
}
6768
@Override

‎graylog2-server/src/main/java/org/graylog/events/processor/EventDefinitionDto.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import com.fasterxml.jackson.annotation.JsonAutoDetect;
2020
import com.fasterxml.jackson.annotation.JsonCreator;
21-
import com.fasterxml.jackson.annotation.JsonIgnore;
2221
import com.fasterxml.jackson.annotation.JsonInclude;
2322
import com.fasterxml.jackson.annotation.JsonProperty;
2423
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
@@ -37,6 +36,7 @@
3736
import org.graylog.events.notifications.EventNotificationSettings;
3837
import org.graylog.events.processor.storage.EventStorageHandler;
3938
import org.graylog.events.processor.storage.PersistToStreamsStorageHandler;
39+
import org.graylog.security.UserContext;
4040
import org.graylog2.contentpacks.ContentPackable;
4141
import org.graylog2.contentpacks.EntityDescriptorIds;
4242
import org.graylog2.contentpacks.model.ModelId;
@@ -163,17 +163,17 @@ public static Builder builder() {
163163

164164
public abstract Builder toBuilder();
165165

166-
@JsonIgnore
167166
public ValidationResult validate(@Nullable EventDefinitionDto oldEventDefinitionDto,
168-
EventDefinitionConfiguration eventDefinitionConfiguration) {
167+
EventDefinitionConfiguration eventDefinitionConfiguration,
168+
UserContext userContext) {
169169
final ValidationResult validation = new ValidationResult();
170170

171171
if (title().isEmpty()) {
172172
validation.addError(FIELD_TITLE, "Event Definition title cannot be empty.");
173173
}
174174

175175
try {
176-
validation.addAll(config().validate());
176+
validation.addAll(config().validate(userContext));
177177
validation.addAll(config().validate(
178178
Optional.ofNullable(oldEventDefinitionDto).map(EventDefinitionDto::config).orElse(null),
179179
eventDefinitionConfiguration));

‎graylog2-server/src/main/java/org/graylog/events/processor/EventProcessorConfig.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.graylog.plugins.views.search.searchfilters.model.UsedSearchFilter;
2424
import org.graylog.scheduler.JobDefinitionConfig;
2525
import org.graylog.scheduler.clock.JobSchedulerClock;
26+
import org.graylog.security.UserContext;
2627
import org.graylog2.contentpacks.ContentPackable;
2728
import org.graylog2.contentpacks.EntityDescriptorIds;
2829
import org.graylog2.plugin.rest.ValidationResult;
@@ -76,8 +77,7 @@ default ValidationResult validate(@Nullable EventProcessorConfig oldEventProcess
7677
*
7778
* @return the validation result
7879
*/
79-
@JsonIgnore
80-
ValidationResult validate();
80+
ValidationResult validate(UserContext userContext);
8181

8282
/**
8383
* Returns the permissions that are required to create the event processor configuration. (e.g. stream permissions)
@@ -128,7 +128,7 @@ public String type() {
128128
}
129129

130130
@Override
131-
public ValidationResult validate() {
131+
public ValidationResult validate(UserContext userContext) {
132132
throw new UnsupportedOperationException();
133133
}
134134

‎graylog2-server/src/main/java/org/graylog/events/processor/aggregation/AggregationEventProcessorConfig.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.graylog.scheduler.schedule.CronJobSchedule;
4242
import org.graylog.scheduler.schedule.CronUtils;
4343
import org.graylog.scheduler.schedule.IntervalJobSchedule;
44+
import org.graylog.security.UserContext;
4445
import org.graylog2.contentpacks.EntityDescriptorIds;
4546
import org.graylog2.contentpacks.model.ModelId;
4647
import org.graylog2.contentpacks.model.ModelTypes;
@@ -130,11 +131,6 @@ public abstract class AggregationEventProcessorConfig implements EventProcessorC
130131

131132
@Override
132133
public Set<String> requiredPermissions() {
133-
// When there are no streams the event processor will search in all streams so we need to require the
134-
// generic stream permission.
135-
if (streams().isEmpty()) {
136-
return Collections.singleton(RestPermissions.STREAMS_READ);
137-
}
138134
return streams().stream()
139135
.map(streamId -> String.join(":", RestPermissions.STREAMS_READ, streamId))
140136
.collect(Collectors.toSet());
@@ -242,7 +238,7 @@ private boolean isConditionsEmpty() {
242238
}
243239

244240
@Override
245-
public ValidationResult validate() {
241+
public ValidationResult validate(UserContext userContext) {
246242
final ValidationResult validationResult = new ValidationResult();
247243

248244
if (searchWithinMs() <= 0) {
@@ -273,6 +269,10 @@ public ValidationResult validate() {
273269
}
274270
});
275271

272+
if (streams().isEmpty() && !userContext.isPermitted(RestPermissions.STREAMS_READ)) {
273+
validationResult.addError(FIELD_STREAMS, "At least one stream must have been selected.");
274+
}
275+
276276
if (useCronScheduling()) {
277277
if (cronExpression() == null || cronExpression().isEmpty()) {
278278
validationResult.addError(FIELD_CRON_EXPRESSION, "Cron expression must not be empty when using cron scheduling");

‎graylog2-server/src/main/java/org/graylog/events/processor/systemnotification/SystemNotificationEventProcessorConfig.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.google.common.graph.MutableGraph;
2424
import org.graylog.events.contentpack.entities.EventProcessorConfigEntity;
2525
import org.graylog.events.processor.EventProcessorConfig;
26+
import org.graylog.security.UserContext;
2627
import org.graylog2.contentpacks.EntityDescriptorIds;
2728
import org.graylog2.contentpacks.model.entities.EntityDescriptor;
2829
import org.graylog2.plugin.rest.ValidationResult;
@@ -51,7 +52,7 @@ public static Builder create() {
5152
}
5253

5354
@Override
54-
public ValidationResult validate() {
55+
public ValidationResult validate(UserContext userContext) {
5556
// Nothing to validate
5657
return new ValidationResult();
5758
}

‎graylog2-server/src/main/java/org/graylog/events/rest/EventDefinitionsResource.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ public Response create(@ApiParam("schedule") @QueryParam("schedule") @DefaultVal
285285
final EventDefinitionDto dto = unwrappedCreateEntityRequest.getEntity();
286286
checkEventDefinitionPermissions(dto, "create");
287287

288-
final ValidationResult result = dto.validate(null, eventDefinitionConfiguration);
288+
final ValidationResult result = dto.validate(null, eventDefinitionConfiguration, userContext);
289289
if (result.failed()) {
290290
return Response.status(Response.Status.BAD_REQUEST).entity(result).build();
291291
}
@@ -314,7 +314,7 @@ public Response update(@ApiParam(name = "definitionId") @PathParam("definitionId
314314
.orElseThrow(() -> new NotFoundException("Event definition <" + definitionId + "> doesn't exist"));
315315
checkProcessorConfig(oldDto, dto);
316316

317-
final ValidationResult result = dto.validate(oldDto, eventDefinitionConfiguration);
317+
final ValidationResult result = dto.validate(oldDto, eventDefinitionConfiguration, userContext);
318318
if (!definitionId.equals(dto.id())) {
319319
result.addError("id", "Event definition IDs don't match");
320320
}
@@ -473,9 +473,10 @@ public EventDefinitionDto duplicate(@ApiParam(name = "definitionId") @PathParam(
473473
@ApiOperation(value = "Validate an event definition")
474474
@RequiresPermissions(RestPermissions.EVENT_DEFINITIONS_CREATE)
475475
public ValidationResult validate(@ApiParam(name = "JSON body", required = true)
476-
@Valid @NotNull EventDefinitionDto toValidate) {
476+
@Valid @NotNull EventDefinitionDto toValidate,
477+
@Context UserContext userContext) {
477478
EventProcessorConfig oldConfig = dbService.get(toValidate.id()).map(EventDefinition::config).orElse(null);
478-
ValidationResult validationResult = toValidate.config().validate();
479+
ValidationResult validationResult = toValidate.config().validate(userContext);
479480
validationResult.addAll(toValidate.config().validate(oldConfig, eventDefinitionConfiguration));
480481
return validationResult;
481482
}

‎graylog2-server/src/test/java/org/graylog/events/TestEventProcessorConfig.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@
2323
import org.graylog.events.contentpack.entities.EventProcessorConfigEntity;
2424
import org.graylog.events.processor.EventDefinition;
2525
import org.graylog.events.processor.EventProcessorConfig;
26+
import org.graylog.events.processor.EventProcessorExecutionJob;
2627
import org.graylog.events.processor.EventProcessorSchedulerConfig;
2728
import org.graylog.scheduler.clock.JobSchedulerClock;
28-
import org.graylog.events.processor.EventProcessorExecutionJob;
2929
import org.graylog.scheduler.schedule.IntervalJobSchedule;
30+
import org.graylog.security.UserContext;
3031
import org.graylog2.contentpacks.EntityDescriptorIds;
3132
import org.graylog2.plugin.indexer.searches.timeranges.AbsoluteRange;
3233
import org.graylog2.plugin.rest.ValidationResult;
@@ -107,7 +108,7 @@ public EventProcessorConfigEntity toContentPackEntity(EntityDescriptorIds entity
107108
}
108109

109110
@Override
110-
public ValidationResult validate() {
111+
public ValidationResult validate(UserContext userContext) {
111112
return new ValidationResult();
112113
}
113114
}

‎graylog2-server/src/test/java/org/graylog/events/processor/EventDefinitionDtoTest.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.graylog.events.fields.EventFieldSpec;
2424
import org.graylog.events.notifications.EventNotificationSettings;
2525
import org.graylog.events.processor.aggregation.AggregationEventProcessorConfig;
26+
import org.graylog.security.UserContext;
2627
import org.graylog2.plugin.rest.ValidationResult;
2728
import org.graylog2.shared.bindings.providers.ObjectMapperProvider;
2829
import org.junit.Before;
@@ -41,7 +42,7 @@ public class EventDefinitionDtoTest {
4142
@Before
4243
public void setUp() throws Exception {
4344
final AggregationEventProcessorConfig configMock = mock(AggregationEventProcessorConfig.class);
44-
when(configMock.validate()).thenReturn(new ValidationResult());
45+
when(configMock.validate(any(UserContext.class))).thenReturn(new ValidationResult());
4546
when(configMock.validate(any(), any())).thenReturn(new ValidationResult());
4647

4748
testSubject = EventDefinitionDto.builder()
@@ -80,7 +81,7 @@ public void testValidateWithInvalidConfig() {
8081
final AggregationEventProcessorConfig configMock = mock(AggregationEventProcessorConfig.class);
8182
final ValidationResult mockedValidationResult = new ValidationResult();
8283
mockedValidationResult.addError("foo", "bar");
83-
when(configMock.validate()).thenReturn(mockedValidationResult);
84+
when(configMock.validate(any(UserContext.class))).thenReturn(mockedValidationResult);
8485
when(configMock.validate(any(), any())).thenReturn(mockedValidationResult);
8586

8687
final EventDefinitionDto invalidEventDefinition = testSubject.toBuilder()
@@ -161,6 +162,6 @@ public void testEventDefinitionHtmlSanitization() throws JsonProcessingException
161162
}
162163

163164
private static ValidationResult validate(EventDefinitionDto eventDefinitionDto) {
164-
return eventDefinitionDto.validate(null, new EventDefinitionConfiguration());
165+
return eventDefinitionDto.validate(null, new EventDefinitionConfiguration(), mock(UserContext.class));
165166
}
166167
}

‎graylog2-server/src/test/java/org/graylog/events/processor/aggregation/AggregationEventProcessorConfigTest.java‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.graylog.plugins.views.search.searchtypes.pivot.series.Max;
4141
import org.graylog.plugins.views.search.searchtypes.pivot.series.Sum;
4242
import org.graylog.scheduler.schedule.IntervalJobSchedule;
43+
import org.graylog.security.UserContext;
4344
import org.graylog.security.entities.EntityOwnershipService;
4445
import org.graylog.testing.mongodb.MongoDBFixtures;
4546
import org.graylog.testing.mongodb.MongoDBInstance;
@@ -48,6 +49,7 @@
4849
import org.graylog2.plugin.indexer.searches.timeranges.AbsoluteRange;
4950
import org.graylog2.plugin.rest.ValidationResult;
5051
import org.graylog2.shared.bindings.providers.ObjectMapperProvider;
52+
import org.graylog2.shared.security.RestPermissions;
5153
import org.joda.time.DateTime;
5254
import org.joda.time.DateTimeZone;
5355
import org.junit.Before;
@@ -67,7 +69,9 @@
6769
import static org.graylog.events.processor.aggregation.AggregationEventProcessorConfig.FIELD_SERIES;
6870
import static org.junit.Assert.assertEquals;
6971
import static org.junit.Assert.assertTrue;
72+
import static org.mockito.ArgumentMatchers.anyString;
7073
import static org.mockito.Mockito.mock;
74+
import static org.mockito.Mockito.when;
7175

7276
public class AggregationEventProcessorConfigTest {
7377
public static final EventDefinitionConfiguration EVENT_DEFINITION_CONFIGURATION = new EventDefinitionConfiguration();
@@ -85,6 +89,9 @@ public class AggregationEventProcessorConfigTest {
8589
@Mock
8690
private DBEventProcessorStateService stateService;
8791

92+
@Mock
93+
private UserContext userContext;
94+
8895
private DBEventDefinitionService dbService;
8996
private JobSchedulerTestClock clock;
9097

@@ -95,6 +102,8 @@ public void setUp() throws Exception {
95102
objectMapper.registerSubtypes(new NamedType(TemplateFieldValueProvider.Config.class, TemplateFieldValueProvider.Config.TYPE_NAME));
96103
objectMapper.registerSubtypes(new NamedType(PersistToStreamsStorageHandler.Config.class, PersistToStreamsStorageHandler.Config.TYPE_NAME));
97104

105+
when(userContext.isPermitted(anyString())).thenReturn(true);
106+
98107
final MongoJackObjectMapperProvider mapperProvider = new MongoJackObjectMapperProvider(objectMapper);
99108
final MongoCollections mongoCollections = new MongoCollections(mapperProvider, mongodb.mongoConnection());
100109
this.dbService = new DBEventDefinitionService(mongoCollections, stateService,
@@ -167,15 +176,15 @@ public void testValidateWithInvalidTimeRange() {
167176
.searchWithinMs(-1)
168177
.build();
169178

170-
final ValidationResult validationResult1 = invalidConfig1.validate();
179+
final ValidationResult validationResult1 = invalidConfig1.validate(userContext);
171180
assertThat(validationResult1.failed()).isTrue();
172181
assertThat(validationResult1.getErrors()).containsOnlyKeys("search_within_ms");
173182

174183
final AggregationEventProcessorConfig invalidConfig2 = invalidConfig1.toBuilder()
175184
.searchWithinMs(0)
176185
.build();
177186

178-
final ValidationResult validationResult2 = invalidConfig2.validate();
187+
final ValidationResult validationResult2 = invalidConfig2.validate(userContext);
179188
assertThat(validationResult2.failed()).isTrue();
180189
assertThat(validationResult2.getErrors()).containsOnlyKeys("search_within_ms");
181190
}
@@ -189,7 +198,7 @@ public void testValidateWithAggregationSeriesMissingFields() {
189198
.conditions(trueConditionThatDoesNotMatter)
190199
.build();
191200

192-
ValidationResult validationResult = configWithSingleSeriesWithoutField.validate();
201+
ValidationResult validationResult = configWithSingleSeriesWithoutField.validate(userContext);
193202
assertTrue(validationResult.failed());
194203
assertEquals(1, validationResult.getErrors().get(FIELD_SERIES).size());
195204
assertThat(validationResult.getErrors()).containsOnlyKeys(FIELD_SERIES);
@@ -208,7 +217,7 @@ public void testValidateWithAggregationSeriesMissingFields() {
208217
.conditions(trueConditionThatDoesNotMatter)
209218
.build();
210219

211-
validationResult = configWithMultipleSeriesWithoutField.validate();
220+
validationResult = configWithMultipleSeriesWithoutField.validate(userContext);
212221
assertTrue(validationResult.failed());
213222
assertThat(validationResult.getErrors()).containsOnlyKeys(FIELD_SERIES);
214223
assertEquals(5, validationResult.getErrors().get(FIELD_SERIES).size());
@@ -221,15 +230,15 @@ public void testValidateWithInvalidExecutionTime() {
221230
.executeEveryMs(-1)
222231
.build();
223232

224-
final ValidationResult validationResult1 = invalidConfig1.validate();
233+
final ValidationResult validationResult1 = invalidConfig1.validate(userContext);
225234
assertThat(validationResult1.failed()).isTrue();
226235
assertThat(validationResult1.getErrors()).containsOnlyKeys("execute_every_ms");
227236

228237
final AggregationEventProcessorConfig invalidConfig2 = invalidConfig1.toBuilder()
229238
.executeEveryMs(0)
230239
.build();
231240

232-
final ValidationResult validationResult2 = invalidConfig2.validate();
241+
final ValidationResult validationResult2 = invalidConfig2.validate(userContext);
233242
assertThat(validationResult2.failed()).isTrue();
234243
assertThat(validationResult2.getErrors()).containsOnlyKeys("execute_every_ms");
235244
}
@@ -289,30 +298,30 @@ public void testValidateWithIncompleteAggregationOptions() {
289298
.groupBy(ImmutableList.of("foo"))
290299
.build();
291300

292-
ValidationResult validationResult = invalidConfig.validate();
301+
ValidationResult validationResult = invalidConfig.validate(userContext);
293302
assertThat(validationResult.failed()).isTrue();
294303
assertThat(validationResult.getErrors()).containsOnlyKeys("series", "conditions");
295304

296305
invalidConfig = getConfig().toBuilder()
297306
.series(ImmutableList.of(this.getSeries()))
298307
.build();
299308

300-
validationResult = invalidConfig.validate();
309+
validationResult = invalidConfig.validate(userContext);
301310
assertThat(validationResult.failed()).isTrue();
302311
assertThat(validationResult.getErrors()).containsOnlyKeys("conditions");
303312

304313
invalidConfig = getConfig().toBuilder()
305314
.conditions(this.getConditions())
306315
.build();
307316

308-
validationResult = invalidConfig.validate();
317+
validationResult = invalidConfig.validate(userContext);
309318
assertThat(validationResult.failed()).isTrue();
310319
assertThat(validationResult.getErrors()).containsOnlyKeys("series");
311320
}
312321

313322
@Test
314323
public void testValidConfiguration() {
315-
final ValidationResult validationResult = getConfig().validate();
324+
final ValidationResult validationResult = getConfig().validate(userContext);
316325
assertThat(validationResult.failed()).isFalse();
317326
assertThat(validationResult.getErrors().size()).isEqualTo(0);
318327
}
@@ -324,7 +333,7 @@ public void testValidFilterConfiguration() {
324333
.streams(ImmutableSet.of("1", "2"))
325334
.build();
326335

327-
final ValidationResult validationResult = config.validate();
336+
final ValidationResult validationResult = config.validate(userContext);
328337
assertThat(validationResult.failed()).isFalse();
329338
assertThat(validationResult.getErrors().size()).isEqualTo(0);
330339
}
@@ -337,7 +346,7 @@ public void testValidAggregationConfiguration() {
337346
.conditions(this.getConditions())
338347
.build();
339348

340-
final ValidationResult validationResult = config.validate();
349+
final ValidationResult validationResult = config.validate(userContext);
341350
assertThat(validationResult.failed()).isFalse();
342351
assertThat(validationResult.getErrors().size()).isEqualTo(0);
343352
}
@@ -352,7 +361,9 @@ public void requiredPermissions() {
352361
@Test
353362
@MongoDBFixtures("aggregation-processors.json")
354363
public void requiredPermissionsWithEmptyStreams() {
364+
when(userContext.isPermitted(RestPermissions.STREAMS_READ)).thenReturn(false);
365+
355366
assertThat(dbService.get("54e3deadbeefdeadbeefafff")).get().satisfies(definition ->
356-
assertThat(definition.config().requiredPermissions()).containsOnly("streams:read"));
367+
assertThat(definition.config().validate(userContext).getErrors()).containsKey("streams"));
357368
}
358369
}

0 commit comments

Comments
 (0)