From 1d80f98d8abb8ce4d78740a05cf4854252afd4ba Mon Sep 17 00:00:00 2001 From: Erik Drolshammer Date: Thu, 16 Mar 2017 15:27:37 +0100 Subject: [PATCH] #62 - first draft for validation of clientSecret for sync/checkForUpdate. --- pom.xml | 3 +- .../no/cantara/cs/client/ClientResource.java | 21 ++++++-- .../no/cantara/cs/client/ClientService.java | 15 ++++-- src/main/resources/application.properties | 2 + .../cantara/cs/client/CheckForUpdateTest.java | 49 +++++++++++++++++++ 5 files changed, 82 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index 2d60f125..74561008 100755 --- a/pom.xml +++ b/pom.xml @@ -25,7 +25,6 @@ scm:git:ssh://git@github.com/cantara/ConfigService.git scm:git:https://github.com/cantara/ConfigService.git - HEAD @@ -57,7 +56,7 @@ no.cantara.jau configservice-sdk - 0.12.3 + 0.13-SNAPSHOT diff --git a/src/main/java/no/cantara/cs/client/ClientResource.java b/src/main/java/no/cantara/cs/client/ClientResource.java index 724a9a16..8a30733d 100644 --- a/src/main/java/no/cantara/cs/client/ClientResource.java +++ b/src/main/java/no/cantara/cs/client/ClientResource.java @@ -1,6 +1,7 @@ package no.cantara.cs.client; import com.fasterxml.jackson.databind.ObjectMapper; +import no.cantara.cs.config.ConstrettoConfig; import no.cantara.cs.dto.CheckForUpdateRequest; import no.cantara.cs.dto.ClientConfig; import no.cantara.cs.dto.ClientRegistrationRequest; @@ -25,10 +26,12 @@ public class ClientResource { private static final ObjectMapper mapper = new ObjectMapper(); private final ClientService clientService; + private final boolean clientSecretValidationEnabled; @Autowired public ClientResource(ClientService clientService) { this.clientService = clientService; + this.clientSecretValidationEnabled = ConstrettoConfig.getBoolean("client.secret.validation.enabled"); } @POST @@ -72,18 +75,26 @@ public Response registerClient(String json) { @Path("/{clientId}/sync") @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) - public Response sync(@PathParam("clientId") String clientId, String json) { + public Response sync(@PathParam("clientId") String clientId, String checkForUpdateRequestJson) { log.trace("checkForUpdate with clientId={}", clientId); + CheckForUpdateRequest checkForUpdateRequest; try { - checkForUpdateRequest = mapper.readValue(json, CheckForUpdateRequest.class); + checkForUpdateRequest = fromJson(checkForUpdateRequestJson); } catch (IOException e) { - log.error("Error parsing json. {}, json={}", e.getMessage(), json); + log.error("Error parsing json. {}, json={}", e.getMessage(), checkForUpdateRequestJson); return Response.status(Response.Status.BAD_REQUEST).entity("Could not parse json.").build(); } + if (clientSecretValidationEnabled && !clientService.validateClientSecret(checkForUpdateRequest)) { + log.warn("Invalid clientSecret={} for clientId={}. Returning {}", + checkForUpdateRequest.clientSecret, checkForUpdateRequest.clientId, Response.Status.UNAUTHORIZED); + return Response.status(Response.Status.UNAUTHORIZED).entity("Invalid or mismatching client id and/or secret.").build(); + } + clientService.processEvents(clientId, checkForUpdateRequest.eventsStore); + ClientConfig clientConfig = clientService.checkForUpdatedClientConfig(clientId, checkForUpdateRequest); if (clientConfig == null) { String msg = "No ClientConfig could be found. Not registered? clientId=" + clientId + ", configLastChanged=" + checkForUpdateRequest.configLastChanged; @@ -107,4 +118,8 @@ public Response sync(@PathParam("clientId") String clientId, String json) { clientId, clientConfig.config.getLastChanged(), checkForUpdateRequest.configLastChanged); return Response.ok(jsonResult).build(); } + + static CheckForUpdateRequest fromJson(String checkForUpdateRequestJson) throws IOException { + return mapper.readValue(checkForUpdateRequestJson, CheckForUpdateRequest.class); + } } diff --git a/src/main/java/no/cantara/cs/client/ClientService.java b/src/main/java/no/cantara/cs/client/ClientService.java index df7f4d48..db6cc58e 100644 --- a/src/main/java/no/cantara/cs/client/ClientService.java +++ b/src/main/java/no/cantara/cs/client/ClientService.java @@ -12,6 +12,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import java.util.Objects; import java.util.UUID; /** @@ -37,11 +38,11 @@ public ClientService(ApplicationConfigDao dao, EventsDao eventsDao, ClientDao cl } /** - * @return null if request is valid, but no Config can be found or a new ClientConfig containing the Config and a ClientId. + * @return null if request is valid, but no Config can be found, + * else return a new ClientConfig containing the ApplicationConfig and a ClientId. * @throws IllegalArgumentException if request does not contain enough information */ public ClientConfig registerClient(ClientRegistrationRequest registration) { - Client client; ApplicationConfig config; @@ -69,7 +70,7 @@ public ClientConfig registerClient(ClientRegistrationRequest registration) { clientDao.saveClient(client); clientDao.saveClientHeartbeatData(client.clientId, new ClientHeartbeatData(registration, config)); clientDao.saveClientEnvironment(client.clientId, new ClientEnvironment(registration.envInfo)); - return new ClientConfig(client.clientId, config) ; + return new ClientConfig(client.clientId, client.clientSecret, config) ; } public ClientConfig checkForUpdatedClientConfig(String clientId, CheckForUpdateRequest checkForUpdateRequest) { @@ -113,4 +114,12 @@ public void processEvents(String clientId, ExtractedEventsStore eventsStore) { log.error("Failed to register heartbeat in CloudWatch", e); } } + + boolean validateClientSecret(CheckForUpdateRequest checkForUpdateRequest) { + Client client = clientDao.getClient(checkForUpdateRequest.clientId); + if (client.clientSecret == null || client.clientSecret.isEmpty()) { + return false; + } + return Objects.equals(client.clientSecret, checkForUpdateRequest.clientSecret); + } } diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index cc17c9f6..f897a3c6 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -5,6 +5,8 @@ login.password=baretillesing login.admin.user=admin login.admin.password=configservice +client.secret.validation.enabled=false + mapdb.path=./db/serviceConfig.db # The AWS region to use, e.g., "eu-west-1" diff --git a/src/test/java/no/cantara/cs/client/CheckForUpdateTest.java b/src/test/java/no/cantara/cs/client/CheckForUpdateTest.java index 85120b62..04f92f45 100644 --- a/src/test/java/no/cantara/cs/client/CheckForUpdateTest.java +++ b/src/test/java/no/cantara/cs/client/CheckForUpdateTest.java @@ -60,4 +60,53 @@ public void testCheckForUpdateUnregisteredClient() throws IOException { } } + @Test + public void testCheckForUpdateRequestParsingBackwardCompatible() throws Exception { + /* + CheckForUpdateRequest request = new CheckForUpdateRequest("configLastChanged"); + Map envInfo = new HashMap<>(); + envInfo.put("envKey1", "envValue1"); + envInfo.put("envKey2", "envValue2"); + request.envInfo = envInfo; + request.tags = "tagA, tabB, tagC"; + request.clientName = "clientName123"; + //request.eventsStore = new ExtractedEventsStore(); + //request.eventsStore.addEvents(Collections.singletonList(new Event(43, "this is a log statement"))); + + String jsonResult = new ObjectMapper().writeValueAsString(request); + */ + String configLastChanged1 = "configLastChanged"; + String json1 = "{\n" + + " \"configLastChanged\": \"" + configLastChanged1 + "\",\n" + + " \"envInfo\": {\n" + + " \"envKey1\": \"envValue1\",\n" + + " \"envKey2\": \"envValue2\"\n" + + " },\n" + + " \"tags\": \"tagA, tabB, tagC\",\n" + + " \"clientName\": \"clientName123\",\n" + + " \"eventsStore\": null\n" + + "}"; + CheckForUpdateRequest checkForUpdateRequest = ClientResource.fromJson(json1); + assertEquals(checkForUpdateRequest.configLastChanged, configLastChanged1); + + String clientId2 = "clientId2"; + String clientSecret2 = "clientSecret2"; + String configLastChanged2 = "configLastChanged2"; + String json2 = "{\n" + + " \"clientId\": \"" + clientId2 + "\",\n" + + " \"clientSecret\": \"" + clientSecret2 + "\",\n" + + " \"configLastChanged\": \"" + configLastChanged2 + "\",\n" + + " \"envInfo\": {\n" + + " \"envKey1\": \"envValue1\",\n" + + " \"envKey2\": \"envValue2\"\n" + + " },\n" + + " \"tags\": \"tagA, tabB, tagC\",\n" + + " \"clientName\": \"clientName123\",\n" + + " \"eventsStore\": null\n" + + "}"; + CheckForUpdateRequest checkForUpdateRequest2 = ClientResource.fromJson(json2); + assertEquals(checkForUpdateRequest2.clientId, clientId2); + assertEquals(checkForUpdateRequest2.clientSecret, clientSecret2); + assertEquals(checkForUpdateRequest2.configLastChanged, configLastChanged2); + } }