Improve webhook request validation and test coverage

Author: alexjweil

lightspark/Cargo.toml

@@ -21,15 +21,13 @@ client = ["base", "objects", "dep:reqwest"]
 
 [dependencies]
 reqwest = { version = "0.12", default-features = false, features = ["blocking", "json", "rustls-tls", "zstd"], optional = true }
-futures = "0.3"
 base64 = "0.21.0"
 serde_json = "1.0.94"
 serde = { version = "1.0.155", features = ["derive"] }
 regex = "1.7.1"
 chrono = "0.4.35"
 aes-gcm = "0.10.1"
 rand = "0.8.5"
-block-modes = "0.9.1"
 os-version = "0.2.0"
 version_check = "0.9.4"
 hex = "0.4.3"

lightspark/src/objects/webhook_event_type.rs

@@ -4,7 +4,7 @@ use serde_json::Value;
 use std::fmt;
 
 /// This is an enum of the potential event types that can be associated with your Lightspark wallets.
-#[derive(Debug, Clone, Deserialize, Serialize)]
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
 pub enum WebhookEventType {
     #[serde(rename = "PAYMENT_FINISHED")]
     PaymentFinished,

lightspark/src/webhooks.rs

@@ -9,7 +9,7 @@ use chrono::{DateTime, Utc};
 
 pub const SIGNATURE_HEADER: &str = "lightspark-signature";
 
-#[derive(Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct WebhookEvent {
     pub event_type: WebhookEventType,
     pub event_id: String,
@@ -24,46 +24,37 @@ pub struct WebhookEvent {
 impl WebhookEvent {
     pub fn verify_and_parse(
         data: &[u8],
-        hexdigest: &str,
+        hex_digest: &str,
         webhook_secret: &str,
     ) -> Result<WebhookEvent, Error> {
-        let mut hmac: Hmac<Sha256> =
-            Hmac::new_from_slice(webhook_secret.as_bytes()).expect("HMAC can take key of any size");
-        hmac.update(data);
-        let result = hmac.finalize();
-        let code_bytes = result.into_bytes();
-        let hex_string = hex::encode(code_bytes);
-        if !hex_string
-            .to_ascii_lowercase()
-            .eq(&hexdigest.to_ascii_lowercase())
-        {
-            return Err(Error::WebhookSignatureError);
-        }
-        Self::parse(data)
-    }
+        if let Ok(digest_bytes) = hex::decode(hex_digest) {
+            let hmac: Hmac<Sha256> = Hmac::new_from_slice(webhook_secret.as_bytes())
+                .expect("HMAC can take key of any size")
+                .chain_update(data);
 
-    fn parse(data: &[u8]) -> Result<WebhookEvent, Error> {
-        let event: WebhookEvent = serde_json::from_slice(data).map_err(Error::JsonError)?;
-        Ok(event)
+            if hmac.verify_slice(digest_bytes.as_slice()).is_ok() {
+                return serde_json::from_slice(data).map_err(Error::JsonError);
+            }
+        }
+        Err(Error::WebhookSignatureError)
     }
 }
 
 #[cfg(test)]
 mod tests {
+    use crate::error::Error;
+
     #[test]
     fn test_verify_and_parse() {
         let data = "{\"event_type\": \"NODE_STATUS\", \"event_id\": \"1615c8be5aa44e429eba700db2ed8ca5\", \"timestamp\": \"2023-05-17T23:56:47.874449+00:00\", \"entity_id\": \"lightning_node:01882c25-157a-f96b-0000-362d42b64397\"}";
-        let hexdigest = "62a8829aeb48b4142533520b1f7f86cdb1ee7d718bf3ea15bc1c662d4c453b74";
+        let hex_digest = "62a8829aeb48b4142533520b1f7f86cdb1ee7d718bf3ea15bc1c662d4c453b74";
         let webhook_secret = "3gZ5oQQUASYmqQNuEk0KambNMVkOADDItIJjzUlAWjX";
 
         let result =
-            super::WebhookEvent::verify_and_parse(data.as_bytes(), hexdigest, webhook_secret)
+            super::WebhookEvent::verify_and_parse(data.as_bytes(), hex_digest, webhook_secret)
                 .expect("Success case");
 
-        assert_eq!(
-            result.event_type.to_string(),
-            super::WebhookEventType::NodeStatus.to_string()
-        );
+        assert_eq!(result.event_type, super::WebhookEventType::NodeStatus);
         assert_eq!(result.event_id, "1615c8be5aa44e429eba700db2ed8ca5");
         assert_eq!(
             result.entity_id,
@@ -75,6 +66,30 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_invalid_signature() {
+        let data = "{\"event_type\": \"NODE_STATUS\", \"event_id\": \"1615c8be5aa44e429eba700db2ed8ca5\", \"timestamp\": \"2023-05-17T23:56:47.874449+00:00\", \"entity_id\": \"lightning_node:01882c25-157a-f96b-0000-362d42b64397\"}";
+        let hex_digest = "deadbeef";
+        let webhook_secret = "3gZ5oQQUASYmqQNuEk0KambNMVkOADDItIJjzUlAWjX";
+
+        let result =
+            super::WebhookEvent::verify_and_parse(data.as_bytes(), hex_digest, webhook_secret);
+
+        result.expect_err(Error::WebhookSignatureError.to_string().as_str());
+    }
+
+    #[test]
+    fn test_invalid_digest_bytes() {
+        let data = "{\"event_type\": \"NODE_STATUS\", \"event_id\": \"1615c8be5aa44e429eba700db2ed8ca5\", \"timestamp\": \"2023-05-17T23:56:47.874449+00:00\", \"entity_id\": \"lightning_node:01882c25-157a-f96b-0000-362d42b64397\"}";
+        let hex_digest = "NotAHexDigest";
+        let webhook_secret = "3gZ5oQQUASYmqQNuEk0KambNMVkOADDItIJjzUlAWjX";
+
+        let result =
+            super::WebhookEvent::verify_and_parse(data.as_bytes(), hex_digest, webhook_secret);
+
+        result.expect_err(Error::WebhookSignatureError.to_string().as_str());
+    }
+
     #[test]
     fn test_remote_signing_webhook() {
         let data = "{\"event_type\": \"REMOTE_SIGNING\", \"event_id\": \"1615c8be5aa44e429eba700db2ed8ca5\", \"timestamp\": \"2023-05-17T23:56:47.874449+00:00\", \"entity_id\": \"lightning_node:01882c25-157a-f96b-0000-362d42b64397\", \"data\": {\"sub_event_type\": \"ECDH\", \"public_key\": \"027c4b09ffb985c298afe7e5813266cbfcb7780b480ac294b0b43dc21f2be3d13c\"}}";
@@ -85,10 +100,7 @@ mod tests {
             super::WebhookEvent::verify_and_parse(data.as_bytes(), hexdigest, webhook_secret)
                 .expect("Success case");
 
-        assert_eq!(
-            result.event_type.to_string(),
-            super::WebhookEventType::RemoteSigning.to_string()
-        );
+        assert_eq!(result.event_type, super::WebhookEventType::RemoteSigning);
 
         let data = result.data.expect("Data is missing");
         let sub_event_type = data["sub_event_type"]