Signature extractor is now required for ensuring verification

actix/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "http-signature-normalization-actix"
 description = "An HTTP Signatures library that leaves the signing to you"
-version = "0.7.2"
+version = "0.8.0"
 authors = ["asonix <asonix@asonix.dog>"]
 license = "AGPL-3.0"
 readme = "README.md"

actix/README.md

@@ -16,7 +16,7 @@ This crate provides extensions the ClientRequest type from Actix Web, and provid
 actix-rt = "2.6.0"
 actix-web = "4.0.0"
 thiserror = "0.1"
-http-signature-normalization-actix = { version = "0.6.0", default-features = false, features = ["sha-2"] }
+http-signature-normalization-actix = { version = "0.8.0", default-features = false, features = ["sha-2"] }
 sha2 = "0.9"
 ```
 
@@ -91,7 +91,8 @@ impl SignatureVerify for MyVerify {
 }
 
 async fn index(
-    (_, sig_verified): (DigestVerified, SignatureVerified),
+    _: DigestVerified,
+    sig_verified: SignatureVerified,
     req: HttpRequest,
     _body: web::Bytes,
 ) -> &'static str {
@@ -116,7 +117,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
     HttpServer::new(move || {
         App::new()
             .wrap(VerifyDigest::new(Sha256::new()).optional())
-            .wrap(VerifySignature::new(MyVerify, config.clone()).optional())
+            .wrap(VerifySignature::new(MyVerify, config.clone()))
             .wrap(TracingLogger::default())
             .route("/", web::post().to(index))
     })

actix/examples/server.rs

@@ -42,7 +42,8 @@ impl SignatureVerify for MyVerify {
 }
 
 async fn index(
-    (_, sig_verified): (DigestVerified, SignatureVerified),
+    _: DigestVerified,
+    sig_verified: SignatureVerified,
     req: HttpRequest,
     _body: web::Bytes,
 ) -> &'static str {
@@ -67,7 +68,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
     HttpServer::new(move || {
         App::new()
             .wrap(VerifyDigest::new(Sha256::new()).optional())
-            .wrap(VerifySignature::new(MyVerify, config.clone()).optional())
+            .wrap(VerifySignature::new(MyVerify, config.clone()))
             .wrap(TracingLogger::default())
             .route("/", web::post().to(index))
     })

actix/src/middleware.rs

@@ -11,6 +11,7 @@ use futures_util::future::LocalBoxFuture;
 use std::{
     collections::HashSet,
     future::{ready, Ready},
+    rc::Rc,
     task::{Context, Poll},
 };
 use tracing::{debug, Span};
@@ -18,8 +19,7 @@ use tracing_error::SpanTrace;
 use tracing_futures::Instrument;
 
 #[derive(Clone, Debug)]
-/// A marker type that can be used to guard routes when the signature middleware is set to
+/// A marker type that is used to guard routes
-/// 'optional'
 pub struct SignatureVerified(String);
 
 impl SignatureVerified {
@@ -36,9 +36,7 @@ impl SignatureVerified {
 /// The Verify signature middleware
 ///
 /// ```rust,ignore
-/// let middleware = VerifySignature::new(MyVerifier::new(), Config::default())
+/// let middleware = VerifySignature::new(MyVerifier::new(), Config::default()).authorization();
-///     .authorization()
-///     .optional();
 ///
 /// HttpServer::new(move || {
 ///     App::new()
@@ -47,11 +45,11 @@ impl SignatureVerified {
 ///         .route("/unprotected", web::post().to(|| "No verification required"))
 /// })
 /// ```
-pub struct VerifySignature<T>(T, Config, HeaderKind, bool);
+pub struct VerifySignature<T>(T, Config, HeaderKind);
 
-#[derive(Clone, Debug)]
+#[derive(Debug)]
 #[doc(hidden)]
-pub struct VerifyMiddleware<T, S>(S, Config, HeaderKind, bool, T);
+pub struct VerifyMiddleware<T, S>(Rc<S>, Config, HeaderKind, T);
 
 #[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
 enum HeaderKind {
@@ -112,9 +110,6 @@ enum VerifyErrorKind {
     #[error("Signature is not a valid string")]
     ParseSignature,
 
-    #[error("Signature is invalid")]
-    Validate,
-
     #[error("Request extension not present")]
     Extension,
 
@@ -141,21 +136,12 @@ where
     /// By default, this middleware expects to verify Signature headers, and requires the presence
     /// of the header
     pub fn new(verify_signature: T, config: Config) -> Self {
-        VerifySignature(verify_signature, config, HeaderKind::Signature, false)
+        VerifySignature(verify_signature, config, HeaderKind::Signature)
     }
 
     /// Verify Authorization headers instead of Signature headers
     pub fn authorization(self) -> Self {
-        VerifySignature(self.0, self.1, HeaderKind::Authorization, self.3)
+        VerifySignature(self.0, self.1, HeaderKind::Authorization)
-    }
-
-    /// Mark the presence of a Signature or Authorization header as optional
-    ///
-    /// If a header is present, it will be verified, but if there is not one present, the request
-    /// is passed through. This can be used to set a global middleware, and then guard each route
-    /// handler with the [`SignatureVerified`] type.
-    pub fn optional(self) -> Self {
-        VerifySignature(self.0, self.1, self.2, true)
     }
 }
 
@@ -219,31 +205,25 @@ where
         let algorithm = unverified.algorithm().cloned();
         let key_id = unverified.key_id().to_owned();
 
-        let f1 = unverified.verify(|signature, signing_string| {
+        let verify_fut = unverified.verify(|signature, signing_string| {
-            let fut = span.in_scope(|| {
+            span.in_scope(|| {
-                self.4.clone().signature_verify(
+                self.3.clone().signature_verify(
                     algorithm,
                     key_id.clone(),
                     signature.to_string(),
                     signing_string.to_string(),
                 )
+            })
         });
 
-            fut.instrument(span.clone())
+        let service = Rc::clone(&self.0);
-        });
-
-        req.extensions_mut().insert(SignatureVerified(key_id));
-
-        let f2 = self.0.call(req);
 
         Box::pin(async move {
-            let span = span;
+            if verify_fut.instrument(span).await? {
-
+                req.extensions_mut().insert(SignatureVerified(key_id));
-            if f1.await? {
-                f2.await
-            } else {
-                Err(VerifyError::new(&span, VerifyErrorKind::Validate).into())
             }
+
+            service.call(req).await
         })
     }
 }
@@ -292,10 +272,9 @@ where
 
     fn new_transform(&self, service: S) -> Self::Future {
         ready(Ok(VerifyMiddleware(
-            service,
+            Rc::new(service),
             self.1.clone(),
             self.2,
-            self.3,
             self.0.clone(),
         )))
     }
@@ -321,7 +300,6 @@ where
             "Signature Verification",
             signature.kind = tracing::field::Empty,
             signature.expected_kind = tracing::field::display(&self.2),
-            signature.optional = tracing::field::display(&self.3),
         );
         let authorization = req.headers().get("Authorization").is_some();
         let signature = req.headers().get("Signature").is_some();
@@ -340,17 +318,9 @@ where
             }
         } else {
             span.record("signature.kind", &tracing::field::display("None"));
-
-            if self.3 {
-                return Box::pin(self.0.call(req));
-            }
         }
 
-        Box::pin(ready(Err(VerifyError::new(
+        Box::pin(self.0.call(req))
-            &span,
-            VerifyErrorKind::MissingSignature,
-        )
-        .into())))
     }
 }