Commit 5 Contents:

1-Unit test for hmac/gsa auth
2-Review comments

notification-core/src/main/java/org/opengroup/osdu/notification/api/PubsubEndpoint.java

@@ -22,6 +22,7 @@ import org.opengroup.osdu.notification.provider.interfaces.IPubsubRequestBodyExt
 import org.opengroup.osdu.notification.service.NotificationHandler;
 import org.opengroup.osdu.notification.utils.Config;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.bind.annotation.PostMapping;
@@ -51,17 +52,12 @@ public class PubsubEndpoint {
         String notificationId = this.pubsubRequestBodyExtractor.extractNotificationIdFromRequestBody();
         String pubsubMessage = this.pubsubRequestBodyExtractor.extractDataFromRequestBody();
         Map<String, String> headerAttributes = this.pubsubRequestBodyExtractor.extractAttributesFromRequestBody();
-        try {
-            HttpResponse response = notificationHandler.notifySubscriber(notificationId, pubsubMessage, headerAttributes);
-            if (!response.isSuccessCode()) {
-                this.log.error(NOT_ACKNOWLEDGE);
-                return ResponseEntity.badRequest().body(NOT_ACKNOWLEDGE);
-            }
-            this.log.info(ACKNOWLEDGE);
-            return ResponseEntity.ok(ACKNOWLEDGE);
-        } catch (Exception e) {
-            this.log.error("An exception occurred: " + e);
-            return ResponseEntity.badRequest().body(NOT_ACKNOWLEDGE);
+        HttpResponse response = notificationHandler.notifySubscriber(notificationId, pubsubMessage, headerAttributes);
+        if (!response.isSuccessCode()) {
+            this.log.error(NOT_ACKNOWLEDGE);
+            return new ResponseEntity<String>(NOT_ACKNOWLEDGE, HttpStatus.INTERNAL_SERVER_ERROR);
         }
+        this.log.info(ACKNOWLEDGE);
+        return new ResponseEntity<String>(ACKNOWLEDGE, HttpStatus.OK);
     }
 }

notification-core/src/main/java/org/opengroup/osdu/notification/auth/GsaAuth.java

@@ -25,6 +25,7 @@ import org.opengroup.osdu.notification.provider.interfaces.IGoogleServiceAccount
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
+import java.util.HashMap;
 import java.util.Map;
 
 @Component
@@ -40,17 +41,26 @@ public class GsaAuth implements SecretAuth {
     }
 
     @Override
-    public String getPushUrl(String endpoint) {
+    public Secret getSecret() {
+        return this.gsaSecret;
+    }
+
+    @Override
+    public String getPushUrl(String endpoint) throws Exception {
         return endpoint;
     }
 
     @Override
-    public void getRequestHeaders(Map<String, String> requestHeader) {
-        GsaSecretValue gsaSecretValue = gsaSecret.getValue();
-        JsonParser jsonParser = new JsonParser();
-        JsonElement root = jsonParser.parse(gsaSecretValue.getKey());
-        String keyString = root.getAsJsonObject().toString();
-        String idToken = this.gsaTokenProvider.getIdToken(keyString, gsaSecretValue.getAudience());
-        requestHeader.put("Authorization", idToken);
+    public Map<String, String> getRequestHeaders() {
+        Map<String, String> requestHeader = new HashMap<>();
+        if (gsaSecret != null) {
+            GsaSecretValue gsaSecretValue = gsaSecret.getValue();
+            JsonParser jsonParser = new JsonParser();
+            JsonElement root = jsonParser.parse(gsaSecretValue.getKey());
+            String keyString = root.getAsJsonObject().toString();
+            String idToken = this.gsaTokenProvider.getIdToken(keyString, gsaSecretValue.getAudience());
+            requestHeader.put("Authorization", idToken);
+        }
+        return requestHeader;
     }
 }

notification-core/src/main/java/org/opengroup/osdu/notification/auth/HmacAuth.java

@@ -22,6 +22,7 @@ import org.opengroup.osdu.notification.auth.interfaces.SecretAuth;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
+import java.util.HashMap;
 import java.util.Map;
 
 @Component
@@ -37,19 +38,26 @@ public class HmacAuth implements SecretAuth {
     }
 
     @Override
-    public String getPushUrl(String endpoint) {
+    public Secret getSecret() {
+        return this.hmacSecret;
+    }
+
+    @Override
+    public String getPushUrl(String endpoint) throws Exception {
         String pushUrl = endpoint;
         try {
             String signedjwt = this.signatureService.getSignedSignature(endpoint, hmacSecret.getValue());
             pushUrl += "?hmac=" + signedjwt;
         } catch (Exception e) {
-            System.out.println("An exception occurred in signature service: " + e);
+            throw e;
         }
         return pushUrl;
     }
 
     @Override
-    public void getRequestHeaders(Map<String, String> requestHeader) {
-        // No Op
+    public Map<String, String> getRequestHeaders() {
+        Map<String, String> requestHeader = new HashMap<>();
+        return requestHeader;
+
     }
 }

notification-core/src/main/java/org/opengroup/osdu/notification/auth/factory/AuthFactory.java

@@ -15,6 +15,7 @@
  */
 package org.opengroup.osdu.notification.auth.factory;
 
+import org.opengroup.osdu.core.common.logging.JaxRsDpsLog;
 import org.opengroup.osdu.core.common.model.notification.Secret;
 import org.opengroup.osdu.notification.auth.GsaAuth;
 import org.opengroup.osdu.notification.auth.HmacAuth;
@@ -28,6 +29,8 @@ public class AuthFactory {
     private HmacAuth hmacAuth;
     @Autowired
     private GsaAuth gsaAuth;
+    @Autowired
+    private JaxRsDpsLog log;
 
     public SecretAuth getSecret(String secretType) {
         switch (secretType) {
@@ -36,6 +39,7 @@ public class AuthFactory {
             case "GSA":
                 return gsaAuth;
         }
+        log.error("Unrecognized Secret Type.");
         return null;
     }
 }

notification-core/src/main/java/org/opengroup/osdu/notification/auth/interfaces/SecretAuth.java

@@ -20,9 +20,11 @@ import org.opengroup.osdu.core.common.model.notification.Secret;
 import java.util.Map;
 
 public interface SecretAuth {
-    String getPushUrl(String endpoint);
+    String getPushUrl(String endpoint) throws Exception;
 
     void setSecret(Secret secret);
 
-    void getRequestHeaders(Map<String, String> requestHeader);
+    Secret getSecret();
+
+    Map<String, String> getRequestHeaders();
 }

notification-core/src/main/java/org/opengroup/osdu/notification/service/NotificationHandler.java

@@ -25,6 +25,7 @@ import org.opengroup.osdu.core.common.model.notification.*;
 import org.opengroup.osdu.notification.auth.factory.AuthFactory;
 import org.opengroup.osdu.notification.auth.interfaces.SecretAuth;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 
 import java.util.HashMap;
@@ -40,8 +41,8 @@ public class NotificationHandler {
     private SubscriptionHandler subscriptionHandler;
     @Autowired
     private AuthFactory authFactory;
-
-    private final int WAITING_TIME = 30000;
+    @Value("${app.expireTime:30000}")
+    private int WAITING_TIME ;
 
     public HttpResponse notifySubscriber(String notificationId, String pubsubMessage, Map<String, String> headerAttributes) throws Exception {
         Subscription subscription = subscriptionHandler.getSubscriptionFromCache(notificationId);
@@ -51,10 +52,11 @@ public class NotificationHandler {
         String pushUrl = "";
         Map<String, String> requestHeader = new HashMap<>();
         // Authentication Secret
+
         SecretAuth secretAuth = authFactory.getSecret(secretType);
         secretAuth.setSecret(secret);
         pushUrl = secretAuth.getPushUrl(endpoint);
-        secretAuth.getRequestHeaders(requestHeader);
+        requestHeader = secretAuth.getRequestHeaders();
 
         requestHeader.put(DpsHeaders.CONTENT_TYPE, "application/json");
         requestHeader.put(DpsHeaders.CORRELATION_ID, headerAttributes.get(DpsHeaders.CORRELATION_ID));

notification-core/src/test/java/org/opengroup/osdu/notification/api/PubsubEndpointTests.java

@@ -24,11 +24,16 @@ import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.opengroup.osdu.core.common.http.HttpResponse;
 import org.opengroup.osdu.core.common.logging.JaxRsDpsLog;
+import org.opengroup.osdu.core.common.model.http.DpsHeaders;
+import org.opengroup.osdu.core.common.notification.SubscriptionException;
 import org.opengroup.osdu.notification.provider.interfaces.IPubsubRequestBodyExtractor;
 import org.opengroup.osdu.notification.service.NotificationHandler;
 import org.powermock.modules.junit4.PowerMockRunner;
 import org.springframework.http.ResponseEntity;
 
+import java.util.HashMap;
+
+import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.when;
@@ -57,7 +62,7 @@ public class PubsubEndpointTests {
     @Test
     public void should_return200_whenPubsubMessageValidAndSuccessCodeReturnedFromNotificationHandler() throws Exception {
         response.setResponseCode(200);
-        when(this.notificationHandler.notifySubscriber(anyString(), anyString(), any())).thenReturn(response);
+        when(this.notificationHandler.notifySubscriber(NOTIFICATION_ID, PUBSUB_MESSAGE, new HashMap<>())).thenReturn(response);
         ResponseEntity responseEntity = this.sut.recordChanged();
         Assert.assertEquals(200, responseEntity.getStatusCode().value());
         Assert.assertEquals("message acknowledged by client", responseEntity.getBody().toString());
@@ -66,18 +71,17 @@ public class PubsubEndpointTests {
     @Test
     public void should_return400_whenPubsubMessageValidAndFailureCodeReturnedFromNotificationHandler() throws Exception {
         response.setResponseCode(400);
-        when(this.notificationHandler.notifySubscriber(anyString(), anyString(), any())).thenReturn(response);
+        when(this.notificationHandler.notifySubscriber(NOTIFICATION_ID, PUBSUB_MESSAGE, new HashMap<>())).thenReturn(response);
         ResponseEntity responseEntity = this.sut.recordChanged();
         Assert.assertEquals(400, responseEntity.getStatusCode().value());
         Assert.assertEquals("message not acknowledged by client", responseEntity.getBody().toString());
     }
 
-    @Test
+    @Test(expected = Exception.class)
     public void should_return400_whenPubsubMessageValidAndNotificationHandlerThrowsException() throws Exception {
         response.setResponseCode(400);
-        when(this.notificationHandler.notifySubscriber(anyString(), anyString(), any())).thenThrow(new Exception("error"));
-        ResponseEntity responseEntity = this.sut.recordChanged();
-        Assert.assertEquals(400, responseEntity.getStatusCode().value());
-        Assert.assertEquals("message not acknowledged by client", responseEntity.getBody().toString());
+        when(this.notificationHandler.notifySubscriber(NOTIFICATION_ID, PUBSUB_MESSAGE, new HashMap<>())).thenThrow(new Exception("error"));
+        this.sut.recordChanged();
+        fail("should throw Exception");
     }
 }

notification-core/src/test/java/org/opengroup/osdu/notification/auth/GsaAuthTest.java

+/*
+ *   Copyright 2017-2020, Schlumberger
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License");
+ *   you may not use this file except in compliance with the License.
+ *   You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   Unless required by applicable law or agreed to in writing, software
+ *   distributed under the License is distributed on an "AS IS" BASIS,
+ *   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *   See the License for the specific language governing permissions and
+ *   limitations under the License.
+ */
+
+package org.opengroup.osdu.notification.auth;
+
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.opengroup.osdu.core.common.model.notification.GsaSecret;
+import org.opengroup.osdu.core.common.model.notification.GsaSecretValue;
+import org.opengroup.osdu.core.common.model.notification.Subscription;
+import org.opengroup.osdu.notification.provider.interfaces.IGoogleServiceAccount;
+import org.powermock.modules.junit4.PowerMockRunner;
+import org.powermock.utils.Asserts;
+
+import java.util.Map;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.when;
+
+@RunWith(PowerMockRunner.class)
+public class GsaAuthTest {
+    @Mock
+    private IGoogleServiceAccount gsaTokenProvider;
+    @InjectMocks
+    private GsaAuth sut;
+
+    private static Subscription gsa_subscription;
+    private static final String GOOGLE_ID_TOKEN = "testHeader.testPayload.testSignature";
+
+    @BeforeClass
+    public static void setup() {
+        setGsa_subscription();
+    }
+
+    @Test
+    public void should_return_valid_EndpointAndHeaders_gsaClient() throws Exception {
+        GsaSecret secret = (GsaSecret) gsa_subscription.getSecret();
+        when(this.gsaTokenProvider.getIdToken(any(), any())).thenReturn(GOOGLE_ID_TOKEN);
+        sut.setSecret(gsa_subscription.getSecret());
+        Asserts.assertNotNull(sut.getSecret(), "Unable to set Secret");
+        Map<String, String> headers = sut.getRequestHeaders();
+        Assert.assertEquals(GOOGLE_ID_TOKEN, headers.get("Authorization"));
+        String pushUrl = sut.getPushUrl(gsa_subscription.getPushEndpoint());
+        Assert.assertEquals(pushUrl, gsa_subscription.getPushEndpoint());
+    }
+
+    @Test
+    public void should_return_emptyHeaders_when_secret_is_null() throws Exception {
+        GsaSecret secret = (GsaSecret) gsa_subscription.getSecret();
+        when(this.gsaTokenProvider.getIdToken(any(), any())).thenReturn(GOOGLE_ID_TOKEN);
+        Map<String, String> headers = sut.getRequestHeaders();
+        Assert.assertEquals(0, headers.size());
+        String pushUrl = sut.getPushUrl(gsa_subscription.getPushEndpoint());
+        Assert.assertEquals(pushUrl, gsa_subscription.getPushEndpoint());
+    }
+
+    private static void setGsa_subscription() {
+        gsa_subscription = new Subscription();
+        gsa_subscription.setName("gsa_test_subscription");
+        gsa_subscription.setPushEndpoint("http:///gsa-challenge");
+        gsa_subscription.setDescription("Description");
+        gsa_subscription.setTopic("records-changed");
+        gsa_subscription.setNotificationId("test-notification-id");
+        gsa_subscription.setId("id_1");
+        gsa_subscription.setCreatedBy("test@test.com");
+        GsaSecret secret = new GsaSecret();
+        GsaSecretValue value = new GsaSecretValue();
+        value.setAudience("audience");
+        value.setKey("{\"keyFile\":{\"key\":\"gsa\"}}");
+        secret.setValue(value);
+        gsa_subscription.setSecret(secret);
+    }
+
+}

notification-core/src/test/java/org/opengroup/osdu/notification/auth/HmacAuthTest.java

+/*
+ *   Copyright 2017-2020, Schlumberger
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License");
+ *   you may not use this file except in compliance with the License.
+ *   You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   Unless required by applicable law or agreed to in writing, software
+ *   distributed under the License is distributed on an "AS IS" BASIS,
+ *   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *   See the License for the specific language governing permissions and
+ *   limitations under the License.
+ */
+
+package org.opengroup.osdu.notification.auth;
+
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.opengroup.osdu.core.common.cryptographic.ISignatureService;
+import org.opengroup.osdu.core.common.model.notification.HmacSecret;
+import org.opengroup.osdu.core.common.model.notification.Subscription;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.util.Map;
+
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.when;
+
+@RunWith(PowerMockRunner.class)
+public class HmacAuthTest {
+    @Mock
+    private ISignatureService signatureService;
+    @InjectMocks
+    private HmacAuth sut;
+
+    private static Subscription hmac_subscription;
+    private static final String SIGNED_SIGNATURE = "testEncodedInfo.testSignature";
+
+    @BeforeClass
+    public static void setup() {
+        setHmac_subscription();
+    }
+
+    @Test
+    public void should_return_valid_EndpointAndHeaders() throws Exception {
+        HmacSecret secret = (HmacSecret) hmac_subscription.getSecret();
+        when(this.signatureService.getSignedSignature(hmac_subscription.getPushEndpoint(), secret.getValue())).thenReturn(SIGNED_SIGNATURE);
+        sut.setSecret(hmac_subscription.getSecret());
+        Assert.assertEquals(secret, sut.getSecret());
+        Map<String, String> headers = sut.getRequestHeaders();
+        Assert.assertEquals(0, headers.size());
+        String pushUrl = sut.getPushUrl(hmac_subscription.getPushEndpoint());
+        Assert.assertEquals(hmac_subscription.getPushEndpoint() + "?hmac=" + SIGNED_SIGNATURE, pushUrl);
+    }
+
+    @Test(expected = Exception.class)
+    public void should_throwException_whenErrorGeneratingSignature_hmac() throws Exception {
+        Exception ex = new Exception("Error generating signed signature");
+        HmacSecret secret = (HmacSecret) hmac_subscription.getSecret();
+        when(this.signatureService.getSignedSignature(hmac_subscription.getPushEndpoint(), secret.getValue())).thenThrow(ex);
+        sut.getPushUrl(hmac_subscription.getPushEndpoint());
+        fail("should throw Exception");
+    }
+
+    private static void setHmac_subscription() {
+        hmac_subscription = new Subscription();
+        hmac_subscription.setName("hamc_test_subscription");
+        hmac_subscription.setPushEndpoint("http://challenge");
+        hmac_subscription.setDescription("Description");
+        hmac_subscription.setTopic("records-changed");
+        hmac_subscription.setNotificationId("test-notification-id");
+        hmac_subscription.setId("id_1");
+        hmac_subscription.setCreatedBy("test@test.com");
+        HmacSecret secret = new HmacSecret();
+        secret.setValue("testsecret");
+        hmac_subscription.setSecret(secret);
+    }
+}

notification-core/src/test/java/org/opengroup/osdu/notification/service/NotificationHandlerTests.java

@@ -22,7 +22,6 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
-import org.mockito.Mockito;
 import org.opengroup.osdu.core.common.http.HttpClient;
 import org.opengroup.osdu.core.common.http.HttpResponse;
 import org.opengroup.osdu.core.common.logging.JaxRsDpsLog;
@@ -74,7 +73,7 @@ public class NotificationHandlerTests {
         when(this.authFactory.getSecret(any())).thenReturn(secretAuth);
         when(this.httpClient.send(any())).thenReturn(response);
         when(this.secretAuth.getPushUrl(gsa_subscription.getPushEndpoint())).thenReturn(gsa_subscription.getPushEndpoint());
-        Mockito.doNothing().when(secretAuth).getRequestHeaders(headers);
+        when(this.secretAuth.getRequestHeaders()).thenReturn(headers);
         HttpResponse response = this.sut.notifySubscriber(this.NOTIFICATION_ID, this.PUBSUB_MESSAGE, headers);
         Assert.assertEquals(200, response.getResponseCode());
     }
@@ -87,7 +86,7 @@ public class NotificationHandlerTests {
         when(this.authFactory.getSecret(any())).thenReturn(secretAuth);
         when(this.httpClient.send(any())).thenReturn(response);
         when(this.secretAuth.getPushUrl(hmac_subscription.getPushEndpoint())).thenReturn(hmac_subscription.getPushEndpoint());
-        Mockito.doNothing().when(secretAuth).getRequestHeaders(headers);
+        when(this.secretAuth.getRequestHeaders()).thenReturn(headers);
         HttpResponse response = this.sut.notifySubscriber(this.NOTIFICATION_ID, this.PUBSUB_MESSAGE, headers);
         Assert.assertEquals(200, response.getResponseCode());
     }
@@ -98,7 +97,7 @@ public class NotificationHandlerTests {
         when(this.authFactory.getSecret(any())).thenReturn(secretAuth);
         when(this.httpClient.send(any())).thenReturn(response);
         when(this.secretAuth.getPushUrl(gsa_subscription.getPushEndpoint())).thenReturn(gsa_subscription.getPushEndpoint());
-        Mockito.doNothing().when(secretAuth).getRequestHeaders(headers);
+        when(this.secretAuth.getRequestHeaders()).thenReturn(headers);
         when(subscriptionHandler.getSubscriptionFromCache(this.NOTIFICATION_ID)).thenThrow(new SubscriptionException("error", response));
         this.sut.notifySubscriber(this.NOTIFICATION_ID, this.PUBSUB_MESSAGE, headers);
         fail("should throw SubscriptionException");

notification-core/src/test/java/org/opengroup/osdu/notification/service/SubscriptionHandlerTests.java

@@ -51,8 +51,6 @@ public class SubscriptionHandlerTests {
     @Mock
     private SubscriptionCacheFactory subscriptionCacheFactory;
     @Mock
-    private JaxRsDpsLog log;
-    @Mock
     private ObjectMapper objectMapper;
     @InjectMocks
     private SubscriptionHandler sut;