fix(server): invalid webhook authorisations

Parameters had been passed in wrong order. Added strong types to prevent
a repeat.

Cargo.toml

@@ -3,7 +3,7 @@ resolver = "2"
 members = ["crates/cli", "crates/server", "crates/config", "crates/git"]
 
 [workspace.package]
-version = "0.5.0"
+version = "0.5.1"
 edition = "2021"
 
 [workspace.lints.clippy]

crates/server/src/actors/repo/webhook.rs

@@ -24,8 +24,9 @@ pub struct WebhookId(String);
 pub struct WebhookAuth(ulid::Ulid);
 impl WebhookAuth {
     pub fn from_str(authorisation: &str) -> Result<Self, DecodeError> {
-        let id = ulid::Ulid::from_str(authorisation);
+        let id = ulid::Ulid::from_str(authorisation)?;
-        Ok(Self(id?))
+        info!("Parse auth token: {}", id);
+        Ok(Self(id))
     }
 
     fn generate() -> Self {
@@ -186,23 +187,16 @@ impl Handler<WebhookMessage> for RepoActor {
             warn!("Don't know what authorization to expect");
             return;
         };
-        let Some(received_authorization) = &msg.authorisation() else {
+        if msg.authorisation() != expected_authorization {
-            warn!("Missing authorization token");
-            return;
-        };
-        if received_authorization != expected_authorization {
             warn!(
                 "Invalid authorization - expected {}",
                 expected_authorization
             );
             return;
         }
-        let id = msg.id();
-        let span = tracing::info_span!("handle", %id);
-        let _guard = span.enter();
         let body = msg.body();
-        match serde_json::from_str::<Push>(body) {
+        match serde_json::from_str::<Push>(body.as_str()) {
-            Err(err) => warn!(?err, %body, "Not a 'push'"),
+            Err(err) => warn!(?err, ?body, "Not a 'push'"),
             Ok(push) => {
                 if let Some(config) = &self.details.repo_config {
                     match push.branch(config.branches()) {
@@ -262,6 +256,10 @@ impl Handler<WebhookMessage> for RepoActor {
     }
 }
 
+pub fn split_ref(reference: &str) -> (&str, &str) {
+    reference.split_at(11)
+}
+
 #[derive(Debug, serde::Deserialize)]
 struct Push {
     #[serde(rename = "ref")]
@@ -298,12 +296,8 @@ impl Push {
     }
 }
 
-pub fn split_ref(reference: &str) -> (&str, &str) {
-    reference.split_at(11)
-}
-
 #[derive(Debug)]
-enum Branch {
+pub enum Branch {
     Main,
     Next,
     Dev,

crates/server/src/actors/webhook/message.rs

@@ -1,27 +1,33 @@
 //
 use actix::prelude::*;
+use git_next_config::RepoAlias;
 
 use crate::actors::repo::webhook::WebhookAuth;
 
 #[derive(Message, Debug, Clone, derive_more::Constructor)]
 #[rtype(result = "()")]
 pub struct WebhookMessage {
-    id: String,
+    // forge // TODO: (#58) differentiate between multiple forges
-    path: String,
+    repo_alias: RepoAlias,
-    authorisation: String,
+    authorisation: WebhookAuth,
-    body: String,
+    body: Body,
 }
 impl WebhookMessage {
-    pub const fn id(&self) -> &String {
+    pub const fn repo_alias(&self) -> &RepoAlias {
-        &self.id
+        &self.repo_alias
     }
-    pub const fn path(&self) -> &String {
+    pub const fn body(&self) -> &Body {
-        &self.path
-    }
-    pub const fn body(&self) -> &String {
         &self.body
     }
-    pub fn authorisation(&self) -> Option<WebhookAuth> {
+    pub const fn authorisation(&self) -> &WebhookAuth {
-        WebhookAuth::from_str(&self.authorisation).ok()
+        &self.authorisation
+    }
+}
+
+#[derive(Clone, Debug, derive_more::Constructor)]
+pub struct Body(String);
+impl Body {
+    pub fn as_str(&self) -> &str {
+        self.0.as_str()
     }
 }

crates/server/src/actors/webhook/router.rs

@@ -28,9 +28,9 @@ impl Handler<WebhookMessage> for WebhookRouter {
 
     fn handle(&mut self, msg: WebhookMessage, _ctx: &mut Self::Context) -> Self::Result {
         let _gaurd = self.span.enter();
-        let repo_alias = RepoAlias::new(msg.path());
+        let repo_alias = msg.repo_alias();
         debug!(repo = %repo_alias, "Router...");
-        if let Some(recipient) = self.repos.get(&repo_alias) {
+        if let Some(recipient) = self.repos.get(repo_alias) {
             info!(repo = %repo_alias, "Sending to Recipient");
             recipient.do_send(msg);
         }

crates/server/src/actors/webhook/server.rs

@@ -2,13 +2,17 @@ use std::net::SocketAddr;
 
 use actix::prelude::*;
 
+use git_next_config::RepoAlias;
 use tracing::{info, warn};
+use warp::reject::Rejection;
 
-use crate::actors::webhook::message::WebhookMessage;
+use crate::actors::{repo::webhook::WebhookAuth, webhook::message::WebhookMessage};
+
+use super::message;
 
 pub async fn start(
     socket_addr: SocketAddr,
-    address: actix::prelude::Recipient<super::message::WebhookMessage>,
+    address: actix::prelude::Recipient<message::WebhookMessage>,
 ) {
     // start webhook server
     use warp::Filter;
@@ -21,35 +25,24 @@ pub async fn start(
         .and(warp::body::bytes())
         .and_then(
             |recipient: Recipient<WebhookMessage>,
-             path,
+             path: String,
              // query: String,
              headers: warp::http::HeaderMap,
              body: bytes::Bytes| async move {
                 info!("POST received");
+                let repo_alias = RepoAlias::new(path);
                 let bytes = body.to_vec();
-                let request_data = String::from_utf8_lossy(&bytes).to_string();
+                let body = message::Body::new(String::from_utf8_lossy(&bytes).to_string());
-                let id = ulid::Ulid::new().to_string();
+                headers.get("Authorization").map_or_else(
-                match headers.get("Authorization") {
+                    || {
-                    Some(auhorisation) => {
+                        warn!("No Authorization header");
-                        info!(id, path, "Received webhook");
+                        Err(warp::reject())
-                        let authorisation = auhorisation
+                    },
-                            .to_str()
+                    |authorisation_header| {
-                            .map_err(|e| {
+                        info!(?repo_alias, ?authorisation_header, "Received webhook",);
-                                warn!("Invalid value in authorization: {:?}", e);
+                        match parse_auth(authorisation_header) {
-                                warp::reject()
+                            Ok(authorisation) => {
-                            })? // valid characters
+                                let message = WebhookMessage::new(repo_alias, authorisation, body);
-                            .strip_prefix("Basic ")
-                            .ok_or_else(|| {
-                                warn!("Authorization must be 'Basic'");
-                                warp::reject()
-                            })? // must start with "Basic "
-                            .to_string();
-                        let message = WebhookMessage::new(
-                            id,
-                            path,
-                            /* query, headers, */ request_data,
-                            authorisation,
-                        );
                                 recipient
                                     .try_send(message)
                                     .map(|_| {
@@ -61,15 +54,41 @@ pub async fn start(
                                         warp::reject()
                                     })
                             }
-                    _ => {
+                            Err(e) => {
-                        warn!("No Authorization header");
+                                warn!(?e, "Failed to decode authorization header");
                                 Err(warp::reject())
                             }
                         }
                     },
+                )
+            },
         );
 
     // Start the server
     info!("Starting webhook server: {}", socket_addr);
     warp::serve(route).run(socket_addr).await;
 }
+
+fn parse_auth(authorization_header: &warp::http::HeaderValue) -> Result<WebhookAuth, Rejection> {
+    WebhookAuth::from_str(
+        authorization_header
+            .to_str()
+            .map_err(|e| {
+                warn!("Invalid non-ascii value in authorization: {:?}", e);
+                warp::reject()
+            }) // valid characters
+            .map(|v| {
+                info!("raw auth header: {}", v);
+                v
+            })?
+            .strip_prefix("Basic ")
+            .ok_or_else(|| {
+                warn!("Authorization must be 'Basic'");
+                warp::reject()
+            })?, // must start with "Basic "
+    )
+    .map_err(|e| {
+        warn!(?e, "decode error");
+        warp::reject()
+    })
+}