Skip to content

Commit

Permalink
HWKALERTS-239 Query parameter validation
Browse files Browse the repository at this point in the history
- Apply query param validation to all criteria-based services
- add new QueryParamValidation annotation
- add new ResponseUtil.populateQueryParamsMap util for gathering
    query param names for annotated services.
- add new ResponseUtil.checkForUnknownQueryParams for validating requests
  • Loading branch information
jshaughn committed May 3, 2017
1 parent d2e73c6 commit 1510aa8
Show file tree
Hide file tree
Showing 12 changed files with 284 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import org.junit.After
import org.junit.Before
import org.junit.BeforeClass

import groovy.json.internal.Charsets
import groovyx.net.http.ContentType
import groovyx.net.http.RESTClient
import groovyx.net.http.HttpResponseDecorator
Expand All @@ -42,12 +43,23 @@ class AbstractITestBase {
static final AtomicInteger TENANT_ID_COUNTER = new AtomicInteger(0)
static cluster = System.getProperty('cluster') ? true : false

static String failureEntity;

@BeforeClass
static void initClient() {

client = new RESTClient(baseURI, ContentType.JSON)
// this prevents 404 from being wrapped in an Exception, just return the response, better for testing
client.handler.failure = { it }
client.handler.failure = { resp ->
failureEntity = null
if (resp.entity != null && resp.entity.contentLength != 0) {
def baos = new ByteArrayOutputStream()
resp.entity.writeTo(baos)
failureEntity = new String(baos.toByteArray(), Charsets.UTF_8)
}
return resp
}

/*
client.handler.failure = { resp, data ->
resp.setData(data)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,28 @@ class AlertsITest extends AbstractITestBase {
assert resp.status == 200 : resp.status
}

@Test
void findAlertsUnknownParams() {
String now = String.valueOf(System.currentTimeMillis());
def resp = client.get(path: "", query: [
startTime:"0", endTime:now,
startAckTime:"0", endAckTime:now,
startResolvedTime:"0", endResolvedTime:now,
alertIds:"Alert-01", triggerIds:"Trigger-01,Trigger-02", statuses: "OPEN", severities: "LOW",
tags: "a|b", tagQuery: "foo", thin: true] )
assert resp.status == 200 : resp.status

resp = client.get(path: "", query: [
startyTime:"0", endTime:now,
alertIds:"Alert-01", triggrIds:"Trigger-01,Trigger-02", statuses: "OPEN"])
assert resp.status == 400 : resp.status
assert failureEntity.contains("startyTime")
assert failureEntity.contains("triggrIds")
assert !failureEntity.contains("endTime")
assert !failureEntity.contains("alertIds")
assert !failureEntity.contains("statuses")
}

@Test
void deleteAlerts() {
String now = String.valueOf(System.currentTimeMillis());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2016 Red Hat, Inc. and/or its affiliates
* Copyright 2015-2017 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -120,7 +120,7 @@ class CORSITest extends AbstractITestBase {

// Now query for the event
response = client.get(path: "events",
query: [ids: "cors-test-event-id",tags: "cors-test-tag-name|cors-test-tag-value"],
query: [eventIds: "cors-test-event-id",tags: "cors-test-tag-name|cors-test-tag-value"],
headers: [(tenantHeaderName): tenantId])

assertEquals(200, response.status)
Expand Down Expand Up @@ -149,7 +149,7 @@ class CORSITest extends AbstractITestBase {
assertEquals(responseHeaders, (72 * 60 * 60) + "", response.headers[ACCESS_CONTROL_MAX_AGE].value)

//Requery "metrics" endpoint to make sure data gets returned and check headers
response = client.get(path: "events", query: [ids: "cors-test-event-id"],
response = client.get(path: "events", query: [eventIds: "cors-test-event-id"],
headers: [
(tenantHeaderName): tenantId,
(ORIGIN): testOrigin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class EventsITest extends AbstractITestBase {
assertEquals(200, resp.status)

resp = client.put(path: "events/delete",
query: [endTime:now, startTime:"0",alertIds:"Trigger-01|"+now+","+"Trigger-02|"+now] )
query: [endTime:now, startTime:"0", eventIds:"Trigger-01|"+now+","+"Trigger-02|"+now] )
assertEquals(200, resp.status)

resp = client.put(path: "events/delete",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -68,6 +69,12 @@
public class ActionsHandler {
private final Logger log = Logger.getLogger(ActionsHandler.class);

private static final Map<String, Set<String>> queryParamValidationMap = new HashMap<>();

static {
ResponseUtil.populateQueryParamsMap(ActionsHandler.class, queryParamValidationMap);
}

@HeaderParam(TENANT_HEADER_NAME)
String tenantId;

Expand Down Expand Up @@ -286,6 +293,7 @@ public Response deleteActionDefinition(
@ApiResponse(code = 400, message = "Bad Request/Invalid Parameters.", response = ApiError.class),
@ApiResponse(code = 500, message = "Internal server error.", response = ApiError.class)
})
@QueryParamValidation(name = "findActionsHistory")
public Response findActionsHistory(
@ApiParam(required = false, value = "Filter out actions created before this time.",
allowableValues = "Timestamp in millisecond since epoch.")
Expand Down Expand Up @@ -316,8 +324,10 @@ public Response findActionsHistory(
final Boolean thin,
@Context
final UriInfo uri) {
Pager pager = RequestUtil.extractPaging(uri);
try {
ResponseUtil.checkForUnknownQueryParams(uri, queryParamValidationMap.get("findActionsHistory"));
Pager pager = RequestUtil.extractPaging(uri);

ActionsCriteria criteria = buildCriteria(startTime, endTime, actionPlugins, actionIds, alertIds, results,
thin);
Page<Action> actionPage = actions.getActions(tenantId, criteria, pager);
Expand All @@ -343,6 +353,7 @@ public Response findActionsHistory(
@ApiResponse(code = 400, message = "Bad Request/Invalid Parameters.", response = ApiError.class),
@ApiResponse(code = 500, message = "Internal server error.", response = ApiError.class)
})
@QueryParamValidation(name = "deleteActionsHistory")
public Response deleteActionsHistory(
@ApiParam(required = false, value = "Filter out actions created before this time.",
allowableValues = "Timestamp in millisecond since epoch.")
Expand All @@ -367,8 +378,12 @@ public Response deleteActionsHistory(
@ApiParam(required = false, value = "Filter out alerts for unspecified result. ",
allowableValues = "Comma separated list of action results.")
@QueryParam("results")
final String results) {
final String results,
@Context
final UriInfo uri) {
try {
ResponseUtil.checkForUnknownQueryParams(uri, queryParamValidationMap.get("deleteActionsHistory"));

ActionsCriteria criteria = buildCriteria(startTime, endTime, actionPlugins, actionIds, alertIds, results,
false);
int numDeleted = actions.deleteActions(tenantId, criteria);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.ejb.EJB;
import javax.ws.rs.Consumes;
Expand Down Expand Up @@ -72,6 +74,12 @@
public class AlertsHandler {
private final Logger log = Logger.getLogger(AlertsHandler.class);

private static final Map<String, Set<String>> queryParamValidationMap = new HashMap<>();

static {
ResponseUtil.populateQueryParamsMap(AlertsHandler.class, queryParamValidationMap);
}

@HeaderParam(TENANT_HEADER_NAME)
String tenantId;

Expand Down Expand Up @@ -118,6 +126,7 @@ public AlertsHandler() {
@ApiResponse(code = 400, message = "Bad Request/Invalid Parameters.", response = ApiError.class),
@ApiResponse(code = 500, message = "Internal server error.", response = ApiError.class)
})
@QueryParamValidation(name = "findAlerts")
public Response findAlerts(
@ApiParam(required = false, value = "Filter out alerts created before this time.",
allowableValues = "Timestamp in millisecond since epoch.")
Expand Down Expand Up @@ -181,8 +190,10 @@ public Response findAlerts(
final Boolean thin,
@Context
final UriInfo uri) {
Pager pager = RequestUtil.extractPaging(uri);
try {
ResponseUtil.checkForUnknownQueryParams(uri, queryParamValidationMap.get("findAlerts"));
Pager pager = RequestUtil.extractPaging(uri);

/*
We maintain old tags criteria as deprecated (it can be removed in a next major version).
If present, the tags criteria has precedence over tagQuery parameter.
Expand Down Expand Up @@ -246,6 +257,7 @@ We maintain old tags criteria as deprecated (it can be removed in a next major v
@ApiResponse(code = 200, message = "Stream of alerts.", response = Alert.class),
@ApiResponse(code = 200, message = "Errors will close the stream. Description is sent before stream is closed.", response = ResponseUtil.ApiError.class)
})
@QueryParamValidation(name = "watchAlerts")
public Response watchAlerts(
@ApiParam(required = false, value = "Filter out alerts created before this time.",
allowableValues = "Timestamp in millisecond since epoch.")
Expand Down Expand Up @@ -313,16 +325,29 @@ public Response watchAlerts(
final Boolean thin,
@Context
final UriInfo uri) {
String unifiedTagQuery;
if (!isEmpty(tags)) {
unifiedTagQuery = parseTagQuery(parseTags(tags));
} else {
unifiedTagQuery = tagQuery;
try {
ResponseUtil.checkForUnknownQueryParams(uri, queryParamValidationMap.get("watchAlerts"));

String unifiedTagQuery;
if (!isEmpty(tags)) {
unifiedTagQuery = parseTagQuery(parseTags(tags));
} else {
unifiedTagQuery = tagQuery;
}
AlertsCriteria criteria = new AlertsCriteria(startTime, endTime, alertIds, triggerIds, statuses,
severities,
unifiedTagQuery, startResolvedTime, endResolvedTime, startAckTime, endAckTime, startStatusTime,
endStatusTime, thin);
return Response.ok(streamWatcher.watchAlerts(Collections.singleton(tenantId), criteria, watchInterval))
.build();
} catch (Exception e) {
log.debug(e.getMessage(), e);
if (e instanceof IllegalArgumentException ||
(e.getCause() != null && e.getCause() instanceof IllegalArgumentException)) {
return ResponseUtil.badRequest("Bad arguments: " + e.getMessage());
}
return ResponseUtil.internalError(e);
}
AlertsCriteria criteria = new AlertsCriteria(startTime, endTime, alertIds, triggerIds, statuses, severities,
unifiedTagQuery, startResolvedTime, endResolvedTime, startAckTime, endAckTime, startStatusTime,
endStatusTime, thin);
return Response.ok(streamWatcher.watchAlerts(Collections.singleton(tenantId), criteria, watchInterval)).build();
}

@PUT
Expand Down Expand Up @@ -550,6 +575,7 @@ public Response deleteAlert(
@ApiResponse(code = 400, message = "Bad Request/Invalid Parameters.", response = ApiError.class),
@ApiResponse(code = 500, message = "Internal server error.", response = ApiError.class)
})
@QueryParamValidation(name = "deleteAlerts")
public Response deleteAlerts(
@ApiParam(required = false, value = "Filter out alerts created before this time.",
allowableValues = "Timestamp in millisecond since epoch.")
Expand Down Expand Up @@ -607,9 +633,13 @@ public Response deleteAlerts(
@ApiParam(required = false, value = "Filter out alerts with some lifecycle after this time.",
allowableValues = "Timestamp in millisecond since epoch.")
@QueryParam("endStatusTime")
final Long endStatusTime
final Long endStatusTime,
@Context
final UriInfo uri
) {
try {
ResponseUtil.checkForUnknownQueryParams(uri, queryParamValidationMap.get("deleteAlerts"));

/*
We maintain old tags criteria as deprecated (it can be removed in a next major version).
If present, the tags criteria has precedence over tagQuery parameter.
Expand Down

0 comments on commit 1510aa8

Please sign in to comment.