From 309e523cfe35d00fecdb61575bc339b1bf35024f Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 4 Jun 2024 18:30:11 +0100 Subject: [PATCH] test: add more tests to forge-github crate --- Cargo.toml | 2 +- crates/config/src/server.rs | 12 +- crates/forge-forgejo/src/tests.rs | 1 + crates/forge-forgejo/src/webhook/register.rs | 2 + .../forge-forgejo/src/webhook/unregister.rs | 6 +- crates/forge-github/Cargo.toml | 6 +- crates/forge-github/src/commit.rs | 27 +- crates/forge-github/src/lib.rs | 22 +- crates/forge-github/src/tests/mod.rs | 756 ++++++++++++++++-- crates/forge-github/src/webhook/authorised.rs | 39 +- crates/forge-github/src/webhook/list.rs | 69 +- crates/forge-github/src/webhook/mod.rs | 3 + crates/forge-github/src/webhook/register.rs | 12 +- crates/forge-github/src/webhook/unregister.rs | 12 +- justfile | 4 + 15 files changed, 830 insertions(+), 143 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d6c0192c..425dc185 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -90,7 +90,7 @@ inotify = "0.10" # Actors actix = "0.13" actix-rt = "2.9" -tokio = { version = "1.37" } +tokio = { version = "1.37", features = ["rt", "macros"] } # Testing assert2 = "0.3" diff --git a/crates/config/src/server.rs b/crates/config/src/server.rs index f08f83f5..d139c1a2 100644 --- a/crates/config/src/server.rs +++ b/crates/config/src/server.rs @@ -1,5 +1,6 @@ // use actix::prelude::*; +use derive_more::Constructor; use std::{ collections::HashMap, @@ -93,7 +94,16 @@ impl Webhook { } /// The URL for the webhook where forges should send their updates -#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize, derive_more::AsRef)] +#[derive( + Clone, + Debug, + PartialEq, + Eq, + serde::Serialize, + serde::Deserialize, + derive_more::AsRef, + Constructor, +)] pub struct WebhookUrl(String); /// The directory to store server data, such as cloned repos diff --git a/crates/forge-forgejo/src/tests.rs b/crates/forge-forgejo/src/tests.rs index 814cc00a..014a0c90 100644 --- a/crates/forge-forgejo/src/tests.rs +++ b/crates/forge-forgejo/src/tests.rs @@ -1,3 +1,4 @@ +// use git_next_config as config; use git_next_git as git; diff --git a/crates/forge-forgejo/src/webhook/register.rs b/crates/forge-forgejo/src/webhook/register.rs index 1c924cc7..aa127c7f 100644 --- a/crates/forge-forgejo/src/webhook/register.rs +++ b/crates/forge-forgejo/src/webhook/register.rs @@ -59,6 +59,8 @@ pub async fn register( match result { Ok(response) => { let Some(hook) = response.response_body() else { + #[cfg(not(tarpaulin_include))] + // request response is Json so response_body never returns None return Err(git::forge::webhook::Error::NetworkResponseEmpty); }; info!(webhook_id = %hook.id, "Webhook registered"); diff --git a/crates/forge-forgejo/src/webhook/unregister.rs b/crates/forge-forgejo/src/webhook/unregister.rs index 3b3e6467..ea92bec9 100644 --- a/crates/forge-forgejo/src/webhook/unregister.rs +++ b/crates/forge-forgejo/src/webhook/unregister.rs @@ -26,5 +26,9 @@ pub async fn unregister( network::NetRequestLogging::None, ); let result = net.delete(request).await; - Ok(result.map(|_| ())?) + if let Err(e) = result { + tracing::warn!("Failed to unregister webhook"); + return Err(git::forge::webhook::Error::FailedToRegister(e.to_string())); + } + Ok(()) } diff --git a/crates/forge-github/Cargo.toml b/crates/forge-github/Cargo.toml index d8fafc62..74cbfc47 100644 --- a/crates/forge-github/Cargo.toml +++ b/crates/forge-github/Cargo.toml @@ -7,7 +7,7 @@ edition = { workspace = true } git-next-config = { workspace = true } git-next-git = { workspace = true } -# own version +# own version for UserAgent requests to github.com clap = { workspace = true } # logging @@ -45,6 +45,10 @@ derive_more = { workspace = true } # # Actors tokio = { workspace = true } +[dev-dependencies] +# Testing +assert2 = { workspace = true } +rand = { workspace = true } [lints.clippy] nursery = { level = "warn", priority = -1 } diff --git a/crates/forge-github/src/commit.rs b/crates/forge-github/src/commit.rs index b75998ce..96944876 100644 --- a/crates/forge-github/src/commit.rs +++ b/crates/forge-github/src/commit.rs @@ -2,7 +2,7 @@ use crate::{self as github, GithubState}; use git::forge::commit::Status; use git_next_git as git; -use github::GitHubStatus; +use github::GithubStatus; use kxio::network; /// Checks the results of any (e.g. CI) status checks for the commit. @@ -10,12 +10,13 @@ use kxio::network; /// GitHub: https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#list-commit-statuses-for-a-reference pub async fn status(github: &github::Github, commit: &git::Commit) -> git::forge::commit::Status { let repo_details = &github.repo_details; + let hostname = repo_details.forge.hostname(); let repo_path = &repo_details.repo_path; let api_token = &repo_details.forge.token(); use secrecy::ExposeSecret; let token = api_token.expose_secret(); let url = network::NetUrl::new(format!( - "https://api.github.com/repos/{repo_path}/commits/{commit}/statuses" + "https://api.{hostname}/repos/{repo_path}/commits/{commit}/statuses" )); let headers = network::NetRequestHeaders::new() @@ -31,14 +32,11 @@ pub async fn status(github: &github::Github, commit: &git::Commit) -> git::forge None, network::NetRequestLogging::None, ); - let result = github.net.get::>(request).await; + let result = github.net.get::>(request).await; match result { - Ok(response) => response.response_body().map_or_else( - || { - tracing::warn!("No status found for commit"); - git::forge::commit::Status::Pending // assume issue is transient and allow retry - }, - |statuses| { + Ok(response) => response + .response_body() + .and_then(|statuses| { statuses .into_iter() .map(|status| match status.state { @@ -55,12 +53,11 @@ pub async fn status(github: &github::Github, commit: &git::Commit) -> git::forge (_, Status::Pending) => Status::Pending, (Status::Pending, _) => Status::Pending, }) - .unwrap_or_else(|| { - tracing::warn!("No status checks configured for 'next' branch",); - Status::Pass - }) - }, - ), + }) + .unwrap_or_else(|| { + tracing::warn!("No status checks configured for 'next' branch",); + Status::Pass + }), Err(e) => { tracing::warn!(?e, "Failed to get commit status"); Status::Pending // assume issue is transient and allow retry diff --git a/crates/forge-github/src/lib.rs b/crates/forge-github/src/lib.rs index 9a34b41f..9fc660e5 100644 --- a/crates/forge-github/src/lib.rs +++ b/crates/forge-github/src/lib.rs @@ -44,9 +44,9 @@ impl git::ForgeLike for Github { async fn list_webhooks( &self, - _webhook_url: &config::server::WebhookUrl, + webhook_url: &config::server::WebhookUrl, ) -> git::forge::webhook::Result> { - github::webhook::list(self, _webhook_url).await + github::webhook::list(self, webhook_url).await } async fn unregister_webhook( @@ -65,12 +65,12 @@ impl git::ForgeLike for Github { } } -#[derive(Debug, serde::Deserialize)] -struct GitHubStatus { +#[derive(Debug, serde::Deserialize, serde::Serialize)] +struct GithubStatus { pub state: GithubState, // other fields that we ignore } -#[derive(Debug, serde::Deserialize)] +#[derive(Clone, Debug, serde::Deserialize, serde::Serialize)] enum GithubState { #[serde(rename = "success")] Success, @@ -86,8 +86,16 @@ enum GithubState { #[derive(Debug, serde::Deserialize)] struct GithubHook { - pub id: u64, - pub config: Config, + id: u64, + config: Config, +} +impl GithubHook { + pub fn id(&self) -> config::WebhookId { + config::WebhookId::new(format!("{}", self.id)) + } + pub fn url(&self) -> config::server::WebhookUrl { + config::server::WebhookUrl::new(self.config.url.clone()) + } } #[derive(Debug, serde::Deserialize)] struct Config { diff --git a/crates/forge-github/src/tests/mod.rs b/crates/forge-github/src/tests/mod.rs index 662e92b9..403b9b68 100644 --- a/crates/forge-github/src/tests/mod.rs +++ b/crates/forge-github/src/tests/mod.rs @@ -1,73 +1,703 @@ // - -use git::ForgeLike; use git_next_config as config; use git_next_git as git; -use std::collections::HashMap; +mod github { + use super::*; -type TestResult = Result<(), Box>; + use assert2::let_assert; + use std::collections::BTreeMap; -#[test] -fn accepts_valid_webhook_signature() -> TestResult { - //given - // we registered a webhook with this secret: - let webhook_auth = config::WebhookAuth::new("01HZ598CS1K9E0C193ND175XHJ")?; - // then recorded the following test message from github: - let headers = HashMap::from([( - "x-hub-signature-256".to_string(), - "sha256=6c801b0730b1ce06bf38f901de40206d3b0e93ef7b9bf09a5cf28ad9c4221bab".to_string(), - )]); - let payload = config::webhook::message::Body::new(include_str!("payload.json").to_string()); - // this reproduces that message: - let message = message(headers, payload); + use crate::Github; + use config::{ + webhook::message::Body, ForgeAlias, ForgeConfig, ForgeType, GitDir, RepoAlias, + RepoBranches, ServerRepoConfig, WebhookAuth, WebhookMessage, + }; + use git::ForgeLike as _; - //when - // now, we attempt to recreate the signature in the header given the same message: - let result = forge().is_message_authorised(&message, &webhook_auth); + #[test] + fn should_return_name() { + let forge = given::a_github_forge(&given::repo_details(), given::a_network()); + assert_eq!(forge.name(), "github"); + } - //then - // if we succeed: then result will be true: - assert!(result); + mod is_message_authorised { - Ok(()) -} - -fn message( - headers: HashMap, - payload: config::webhook::message::Body, -) -> config::WebhookMessage { - config::WebhookMessage::new( - config::ForgeAlias::new("".to_string()), - config::RepoAlias::new(""), - headers, - payload, - ) -} - -fn forge() -> crate::Github { - crate::Github::new( - git::RepoDetails::new( - git::Generation::new(), - &config::RepoAlias::new(""), - &config::ServerRepoConfig::new( - "a".to_string(), - "b".to_string(), - None, - None, - None, - None, - ), - &config::ForgeAlias::new("c".to_string()), - &config::ForgeConfig::new( - config::ForgeType::GitHub, - "d".to_string(), - "e".to_string(), - "f".to_string(), - std::collections::BTreeMap::default(), - ), - config::GitDir::default(), - ), - kxio::network::Network::new_mock(), - ) + use super::*; + + #[test] + fn should_return_true_with_valid_header() { + let forge = given::a_github_forge(&given::repo_details(), given::a_network()); + let auth = given::a_webhook_auth(); + let message = given::a_webhook_message(given::Header::Valid( + auth.clone(), + given::a_webhook_message_body(), + )); + assert!(forge.is_message_authorised(&message, &auth)); + } + #[test] + fn should_return_false_with_missing_header() { + let forge = given::a_github_forge(&given::repo_details(), given::a_network()); + let auth = given::a_webhook_auth(); + let message = given::a_webhook_message(given::Header::Missing); + assert!(!forge.is_message_authorised(&message, &auth)); + } + #[test] + fn should_return_false_with_invalid_header() { + let forge = given::a_github_forge(&given::repo_details(), given::a_network()); + let auth = given::a_webhook_auth(); + let message = given::a_webhook_message(given::Header::Invalid); + assert!(!forge.is_message_authorised(&message, &auth)); + } + } + + mod parse_webhook_body { + + use serde_json::json; + + use super::*; + + #[test] + fn should_parse_valid_body() { + let forge = given::a_github_forge(&given::repo_details(), given::a_network()); + let repo_branches = given::repo_branches(); + let next = repo_branches.next(); + let sha = given::a_name(); + let message = given::a_name(); + let body = Body::new( + json!({"ref":format!("refs/heads/{next}"),"after":sha,"head_commit":{"message":message}}) + .to_string(), + ); + let_assert!(Ok(push) = forge.parse_webhook_body(&body)); + assert_eq!(push.sha(), sha); + assert_eq!(push.message(), message); + assert_eq!( + push.branch(&repo_branches), + Some(config::webhook::push::Branch::Next) + ); + } + + #[test] + fn should_error_invalid_body() { + let forge = given::a_github_forge(&given::repo_details(), given::a_network()); + let body = Body::new(r#"{"type":"invalid"}"#.to_string()); + let_assert!(Err(_) = forge.parse_webhook_body(&body)); + } + } + + mod commit_status { + use git_next_git::forge::commit::Status; + use kxio::network::StatusCode; + + use crate::GithubState; + + use super::*; + + macro_rules! status_tests{ + ($($name:ident: $value:expr, )*) => { + $( + #[tokio::test] + async fn $name() { + let (states, expected) = $value; + let repo_details = given::repo_details(); + let commit = given::a_commit(); + let mut net = given::a_network(); + given::commit_states(&states, &mut net, &repo_details, &commit); + let forge = given::a_github_forge(&repo_details, net); + assert_eq!(forge.commit_status(&commit).await, expected); + } + )* + } + } + + status_tests!( + pass_when_success: ([GithubState::Success], Status::Pass), + pending_when_pending: ([GithubState::Pending], Status::Pending), + fail_when_failure: ([GithubState::Failure], Status::Fail), + fail_when_error: ([GithubState::Error], Status::Fail), + pending_when_blank: ([GithubState::Blank], Status::Pending), + pass_wneh_no_checks: ([], Status::Pass), + pass_when_all_success: ([GithubState::Success, GithubState::Success], Status::Pass), + fail_when_only_alpha_fails: ([GithubState::Failure, GithubState::Success], Status::Fail), + fail_when_only_beta_fails: ([GithubState::Success, GithubState::Failure], Status::Fail), + pending_when_all_pending: ([GithubState::Pending, GithubState::Pending], Status::Pending), + pending_when_only_alpha_pending: ([GithubState::Pending, GithubState::Success], Status::Pending), + pending_when_only_beta_pending: ([GithubState::Success, GithubState::Pending], Status::Pending), + + ); + + #[tokio::test] + async fn should_return_pending_for_no_statuses() { + let repo_details = given::repo_details(); + let commit = given::a_commit(); + let mut net = given::a_network(); + net.add_get_response( + &given::a_commit_status_url(&repo_details, &commit), + StatusCode::OK, + "", + ); + let forge = given::a_github_forge(&repo_details, net); + assert_eq!(forge.commit_status(&commit).await, Status::Pending); + } + #[tokio::test] + async fn should_return_pending_for_network_error() { + let repo_details = given::repo_details(); + let commit = given::a_commit(); + let mut net = given::a_network(); + net.add_get_error( + &given::a_commit_status_url(&repo_details, &commit), + "boom today", + ); + let forge = given::a_github_forge(&repo_details, net); + assert_eq!(forge.commit_status(&commit).await, Status::Pending); + } + } + + mod list_webhooks { + use git_next_config::WebhookId; + use git_next_git::ForgeLike as _; + + use crate::tests::github::with; + + use super::*; + + #[tokio::test] + async fn should_return_a_list_of_matching_webhooks() { + let repo_details = given::repo_details(); + let webhook_url = given::any_webhook_url(); + let hostname = repo_details.forge.hostname(); + let repo_path = &repo_details.repo_path; + use secrecy::ExposeSecret; + let token = repo_details.forge.token().expose_secret(); + let hook_id_1 = given::a_github_webhook_id(); + let hook_id_2 = given::a_github_webhook_id(); + let hook_id_3 = given::a_github_webhook_id(); + let mut net = given::a_network(); + + let mut args = with::WebhookArgs { + net: &mut net, + hostname, + repo_path, + token, + }; + // page 1 with three items + with::get_webhooks_by_page( + 1, + &[ + with::ReturnedWebhook::new(hook_id_1, &webhook_url), + with::ReturnedWebhook::new(hook_id_2, &webhook_url), + with::ReturnedWebhook::new(hook_id_3, &given::any_webhook_url()), + ], + &mut args, + ); + // page 2 with no items - stops pagination + with::get_webhooks_by_page(2, &[], &mut args); + + let forge = given::a_github_forge(&repo_details, net); + + let_assert!(Ok(result) = forge.list_webhooks(&webhook_url).await); + assert_eq!( + result, + vec![ + WebhookId::new(format!("{hook_id_1}")), + WebhookId::new(format!("{hook_id_2}")) + ] + ); + } + + #[tokio::test] + async fn should_return_any_network_error() { + let repo_details = given::repo_details(); + let webhook_url = given::a_webhook_url(&given::a_forge_alias(), &given::a_repo_alias()); + let hostname = repo_details.forge.hostname(); + let repo_path = &repo_details.repo_path; + let mut net = given::a_network(); + + net.add_get_error( + format!("https://api.{hostname}/repos/{repo_path}/hooks?page=1").as_str(), + "error_message", + ); + + let forge = given::a_github_forge(&repo_details, net); + + let_assert!(Err(_) = forge.list_webhooks(&webhook_url).await); + } + } + + mod unregister_webhook { + use super::*; + use git_next_git::ForgeLike; + use kxio::network::StatusCode; + + #[tokio::test] + async fn should_delete_webhook() { + let repo_details = given::repo_details(); + let hostname = repo_details.forge.hostname(); + let repo_path = &repo_details.repo_path; + let webhook_id = given::a_webhook_id(); + let mut net = given::a_network(); + + net.add_delete_response( + format!("https://api.{hostname}/repos/{repo_path}/hooks/{webhook_id}").as_str(), + StatusCode::OK, + "", + ); + + let forge = given::a_github_forge(&repo_details, net); + + let_assert!(Ok(_) = forge.unregister_webhook(&webhook_id).await); + } + #[tokio::test] + async fn should_return_error_on_network_error() { + let repo_details = given::repo_details(); + assert!( + repo_details.repo_config.is_some(), + "repo_details needs to have repo_config for this test" + ); + let hostname = repo_details.forge.hostname(); + let repo_path = &repo_details.repo_path; + + let mut net = given::a_network(); + + // unregister the webhook will return empty response + net.add_delete_error( + format!("https://api.{hostname}/repos/{repo_path}/hooks").as_str(), + "error", + ); + + let forge = given::a_github_forge(&repo_details, net); + + let webhook_id = given::a_webhook_id(); + let_assert!(Err(err) = forge.unregister_webhook(&webhook_id).await); + assert!( + matches!(err, git::forge::webhook::Error::FailedToRegister(_)), + "{err:?}" + ); + } + } + + mod register_webhook { + use crate::tests::github::with; + + use super::*; + + use git_next_config::WebhookId; + use kxio::network::StatusCode; + use serde_json::json; + + #[tokio::test] + async fn should_register_a_new_webhook() { + let repo_details = given::repo_details(); + assert!( + repo_details.repo_config.is_some(), + "repo_details needs to have repo_config for this test" + ); + let webhook_url = + given::a_webhook_url(repo_details.forge.forge_alias(), &repo_details.repo_alias); + let hostname = repo_details.forge.hostname(); + let repo_path = &repo_details.repo_path; + use secrecy::ExposeSecret; + let token = repo_details.forge.token().expose_secret(); + + let mut net = given::a_network(); + let mut args = with::WebhookArgs { + net: &mut net, + hostname, + repo_path, + token, + }; + + // there are no existing matching webhooks + with::get_webhooks_by_page(1, &[], &mut args); + // register the webhook will succeed + let webhook_id = given::a_github_webhook_id(); + net.add_post_response( + format!("https://api.{hostname}/repos/{repo_path}/hooks").as_str(), + StatusCode::OK, + json!({"id": webhook_id, "config":{"url": webhook_url}}) + .to_string() + .as_str(), + ); + + let forge = given::a_github_forge(&repo_details, net); + + let_assert!(Ok(registered_webhook) = forge.register_webhook(&webhook_url).await); + assert_eq!( + registered_webhook.id(), + &WebhookId::new(format!("{webhook_id}")) + ); + } + + #[tokio::test] + async fn should_abort_if_repo_config_missing() { + let mut repo_details = given::repo_details(); + repo_details.repo_config.take(); + assert!( + repo_details.repo_config.is_none(), + "repo_details needs to NOT have repo_config for this test" + ); + let webhook_url = + given::a_webhook_url(repo_details.forge.forge_alias(), &repo_details.repo_alias); + let net = given::a_network(); + let forge = given::a_github_forge(&repo_details, net); + let_assert!(Err(err) = forge.register_webhook(&webhook_url).await); + assert!( + matches!(err, git::forge::webhook::Error::NoRepoConfig), + "{err:?}" + ); + } + + #[tokio::test] + async fn should_unregister_existing_webhooks_before_registering() { + let repo_details = given::repo_details(); + assert!( + repo_details.repo_config.is_some(), + "repo_details needs to have repo_config for this test" + ); + let webhook_url = + given::a_webhook_url(repo_details.forge.forge_alias(), &repo_details.repo_alias); + let hostname = repo_details.forge.hostname(); + let repo_path = &repo_details.repo_path; + use secrecy::ExposeSecret; + let token = repo_details.forge.token().expose_secret(); + + let mut net = given::a_network(); + let mut args = with::WebhookArgs { + net: &mut net, + hostname, + repo_path, + token, + }; + let hook1 = with::ReturnedWebhook::new(given::a_github_webhook_id(), &webhook_url); + let hook2 = with::ReturnedWebhook::new(given::a_github_webhook_id(), &webhook_url); + let hook3 = + with::ReturnedWebhook::new(given::a_github_webhook_id(), &given::any_webhook_url()); + let hooks = [hook1, hook2, hook3]; + + // there are three existing webhooks, two are matching webhooks + with::get_webhooks_by_page(1, &hooks, &mut args); + with::get_webhooks_by_page(2, &[], &mut args); + // should unregister 1 and 2, but not 3 + with::unregister_webhook(&hooks[0], &mut args); + with::unregister_webhook(&hooks[1], &mut args); + // register the webhook will succeed + let webhook_id = given::a_github_webhook_id(); + net.add_post_response( + format!("https://api.{hostname}/repos/{repo_path}/hooks").as_str(), + StatusCode::OK, + json!({"id": webhook_id, "config":{"url": webhook_url}}) + .to_string() + .as_str(), + ); + + let forge = given::a_github_forge(&repo_details, net); + + let_assert!(Ok(registered_webhook) = forge.register_webhook(&webhook_url).await); + assert_eq!( + registered_webhook.id(), + &WebhookId::new(format!("{webhook_id}")) + ); + } + + #[tokio::test] + async fn should_return_error_if_empty_network_response() { + let repo_details = given::repo_details(); + assert!( + repo_details.repo_config.is_some(), + "repo_details needs to have repo_config for this test" + ); + let webhook_url = + given::a_webhook_url(repo_details.forge.forge_alias(), &repo_details.repo_alias); + let hostname = repo_details.forge.hostname(); + let repo_path = &repo_details.repo_path; + use secrecy::ExposeSecret; + let token = repo_details.forge.token().expose_secret(); + + let mut net = given::a_network(); + let mut args = with::WebhookArgs { + net: &mut net, + hostname, + repo_path, + token, + }; + + // there are no existing matching webhooks + with::get_webhooks_by_page(1, &[], &mut args); + // register the webhook will return empty response + net.add_post_response( + format!("https://api.{hostname}/repos/{repo_path}/hooks").as_str(), + StatusCode::OK, + json!({}).to_string().as_str(), // empty response + ); + + let forge = given::a_github_forge(&repo_details, net); + + let_assert!(Err(err) = forge.register_webhook(&webhook_url).await); + assert!( + matches!(err, git::forge::webhook::Error::FailedToRegister(_)), + "{err:?}" + ); + } + + #[tokio::test] + async fn should_return_error_on_network_error() { + let repo_details = given::repo_details(); + assert!( + repo_details.repo_config.is_some(), + "repo_details needs to have repo_config for this test" + ); + let webhook_url = + given::a_webhook_url(repo_details.forge.forge_alias(), &repo_details.repo_alias); + let hostname = repo_details.forge.hostname(); + let repo_path = &repo_details.repo_path; + use secrecy::ExposeSecret; + let token = repo_details.forge.token().expose_secret(); + + let mut net = given::a_network(); + let mut args = with::WebhookArgs { + net: &mut net, + hostname, + repo_path, + token, + }; + + // there are no existing matching webhooks + with::get_webhooks_by_page(1, &[], &mut args); + // register the webhook will return empty response + net.add_post_error( + format!("https://api.{hostname}/repos/{repo_path}/hooks").as_str(), + "error", + ); + + let forge = given::a_github_forge(&repo_details, net); + + let_assert!(Err(err) = forge.register_webhook(&webhook_url).await); + assert!( + matches!(err, git::forge::webhook::Error::FailedToRegister(_)), + "{err:?}" + ); + } + } + + mod with { + use serde::Serialize; + use serde_json::json; + + use git_next_config::{server::WebhookUrl, Hostname, RepoPath}; + use kxio::network::{self, StatusCode}; + + pub fn get_webhooks_by_page( + page: u8, + response: &[ReturnedWebhook], + args: &mut WebhookArgs, + ) { + let hostname = args.hostname; + let repo_path = args.repo_path; + args.net.add_get_response( + format!("https://api.{hostname}/repos/{repo_path}/hooks?page={page}").as_str(), + StatusCode::OK, + json!(response).to_string().as_str(), + ); + } + + pub fn unregister_webhook(hook1: &ReturnedWebhook, args: &mut WebhookArgs) { + let webhook_id = hook1.id; + let hostname = args.hostname; + let repo_path = args.repo_path; + args.net.add_delete_response( + format!("https://api.{hostname}/repos/{repo_path}/hooks/{webhook_id}").as_str(), + StatusCode::OK, + "", + ); + } + pub struct WebhookArgs<'a> { + pub net: &'a mut network::MockNetwork, + pub hostname: &'a Hostname, + pub repo_path: &'a RepoPath, + pub token: &'a str, + } + #[derive(Debug, Serialize)] + pub struct ReturnedWebhook { + pub id: i64, + pub config: Config, + } + impl ReturnedWebhook { + pub fn new(id: i64, url: &WebhookUrl) -> Self { + Self { + id, + config: Config { url: url.clone() }, + } + } + } + #[derive(Debug, Serialize)] + pub struct Config { + pub url: WebhookUrl, + } + } + + mod given { + use std::collections::HashMap; + + use git_next_config::{server::Webhook, WebhookId}; + use kxio::network::{MockNetwork, StatusCode}; + use rand::RngCore; + use serde_json::json; + + use crate::{GithubState, GithubStatus}; + + use super::*; + + pub fn commit_states( + states: &[GithubState], + net: &mut MockNetwork, + repo_details: &git::RepoDetails, + commit: &git::Commit, + ) { + let response = json!(states + .iter() + .map(|state| GithubStatus { + state: state.to_owned() + }) + .collect::>()); + net.add_get_response( + a_commit_status_url(repo_details, commit).as_str(), + StatusCode::OK, + response.to_string().as_str(), + ) + } + + pub fn a_commit_status_url( + repo_details: &git::RepoDetails, + commit: &git::Commit, + ) -> String { + let hostname = repo_details.forge.hostname(); + let repo_path = &repo_details.repo_path; + format!("https://api.{hostname}/repos/{repo_path}/commits/{commit}/statuses") + } + + pub fn a_webhook_auth() -> WebhookAuth { + WebhookAuth::generate() + } + + pub enum Header { + Valid(WebhookAuth, Body), + Missing, + Invalid, + } + pub fn a_webhook_message(header: Header) -> WebhookMessage { + let body = match &header { + Header::Valid(_, body) => body.clone(), + _ => given::a_webhook_message_body(), + }; + WebhookMessage::new( + given::a_forge_alias(), + given::a_repo_alias(), + given::webhook_headers(header), + body, + ) + } + pub fn webhook_headers(header: Header) -> HashMap { + let mut headers = HashMap::new(); + match header { + Header::Valid(auth, body) => { + if let Some(sig) = crate::webhook::sign_body(&auth, &body) { + headers.insert("x-hub-signature-256".to_string(), format!("sha256={sig}")); + } + } + Header::Missing => { /* don't add any header */ } + Header::Invalid => { + headers.insert( + "x-hub-signature-256".to_string(), + format!("{}", WebhookAuth::generate()), + ); + } + } + headers + } + pub fn a_webhook_message_body() -> Body { + Body::new(a_name()) + } + pub fn a_commit() -> git::Commit { + git::Commit::new( + git::commit::Sha::new(a_name()), + git::commit::Message::new(a_name()), + ) + } + + pub fn repo_branches() -> RepoBranches { + RepoBranches::new(a_name(), a_name(), a_name()) + } + + pub fn a_github_forge( + repo_details: &git::RepoDetails, + net: impl Into, + ) -> Github { + Github::new(repo_details.clone(), net.into()) + } + pub fn repo_details() -> git::RepoDetails { + git::RepoDetails::new( + git::Generation::new(), + &a_repo_alias(), + &ServerRepoConfig::new( + format!("{}/{}", a_name(), a_name()), // repo path: owner/repo + a_name(), + None, + Some(a_name()), + Some(a_name()), + Some(a_name()), + ), + &a_forge_alias(), + &ForgeConfig::new( + ForgeType::ForgeJo, + a_name(), + a_name(), + a_name(), + BTreeMap::default(), + ), + GitDir::default(), + ) + } + + pub fn a_forge_alias() -> ForgeAlias { + ForgeAlias::new(a_name()) + } + + pub fn a_repo_alias() -> RepoAlias { + RepoAlias::new(a_name()) + } + pub fn a_network() -> kxio::network::MockNetwork { + kxio::network::MockNetwork::new() + } + + pub fn a_webhook_url( + forge_alias: &ForgeAlias, + repo_alias: &RepoAlias, + ) -> git_next_config::server::WebhookUrl { + Webhook::new(a_name()).url(forge_alias, repo_alias) + } + pub fn any_webhook_url() -> git_next_config::server::WebhookUrl { + given::a_webhook_url(&given::a_forge_alias(), &given::a_repo_alias()) + } + + pub fn a_name() -> String { + use rand::Rng; + use std::iter; + + fn generate(len: usize) -> String { + const CHARSET: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + let mut rng = rand::thread_rng(); + let one_char = || CHARSET[rng.gen_range(0..CHARSET.len())] as char; + iter::repeat_with(one_char).take(len).collect() + } + generate(5) + } + + pub fn a_webhook_id() -> WebhookId { + WebhookId::new(given::a_name()) + } + + pub fn a_github_webhook_id() -> i64 { + rand::thread_rng().next_u32().into() + } + } } diff --git a/crates/forge-github/src/webhook/authorised.rs b/crates/forge-github/src/webhook/authorised.rs index d4154a4d..fbde6b57 100644 --- a/crates/forge-github/src/webhook/authorised.rs +++ b/crates/forge-github/src/webhook/authorised.rs @@ -2,25 +2,32 @@ use git_next_config as config; pub fn is_authorised(msg: &config::WebhookMessage, webhook_auth: &config::WebhookAuth) -> bool { - let Some(github_signature) = msg - .header("x-hub-signature-256") + msg.header("x-hub-signature-256") .map(|x| x.trim_matches('"').to_string()) .and_then(|sha| sha.strip_prefix("sha256=").map(|k| k.to_string())) - else { - tracing::warn!("no signature header found"); - return false; - }; - let Ok(gh_sig) = hex::decode(github_signature) else { - eprintln!("can't decode github signature"); - return false; - }; - let payload = msg.body().as_str(); + .and_then(|github_signature| hex::decode(github_signature).ok()) + .and_then(|gh_sig| { + let payload = &msg.body().as_str(); + + use hmac::Mac; + type HmacSha256 = hmac::Hmac; + let mut hmac = HmacSha256::new_from_slice(webhook_auth.to_string().as_bytes()).ok()?; + hmac::Mac::update(&mut hmac, payload.as_ref()); + Some(hmac::Mac::verify_slice(hmac, gh_sig.as_ref()).is_ok()) + }) + .unwrap_or_default() +} + +#[cfg(test)] +pub fn sign_body( + webhook_auth: &config::WebhookAuth, + body: &config::webhook::message::Body, +) -> std::option::Option { + let payload = body.as_str(); use hmac::Mac; type HmacSha256 = hmac::Hmac; - let Ok(mut hmac) = HmacSha256::new_from_slice(webhook_auth.to_string().as_bytes()) else { - tracing::error!("failed to parse webhook auth token"); - return false; - }; + let mut hmac = HmacSha256::new_from_slice(webhook_auth.to_string().as_bytes()).ok()?; hmac::Mac::update(&mut hmac, payload.as_ref()); - hmac::Mac::verify_slice(hmac, gh_sig.as_ref()).is_ok() + let f = hmac::Mac::finalize(hmac); + Some(hex::encode(f.into_bytes())) } diff --git a/crates/forge-github/src/webhook/list.rs b/crates/forge-github/src/webhook/list.rs index 3fc91003..3771d25d 100644 --- a/crates/forge-github/src/webhook/list.rs +++ b/crates/forge-github/src/webhook/list.rs @@ -9,36 +9,45 @@ pub async fn list( github: &github::Github, webhook_url: &config::server::WebhookUrl, ) -> git::forge::webhook::Result> { - let net = &github.net; + let mut ids: Vec = vec![]; let repo_details = &github.repo_details; - let request = network::NetRequest::new( - network::RequestMethod::Delete, - network::NetUrl::new(format!( - "https://api.github.com/repos/{}/hooks/", - repo_details.repo_path, - )), - github::webhook::headers(repo_details.forge.token()), - network::RequestBody::None, - network::ResponseType::None, - None, - network::NetRequestLogging::None, - ); - match net.post_json::>(request).await { - Err(e) => { - tracing::warn!("Failed to list webhooks"); - Err(git::forge::webhook::Error::FailedToList(e.to_string())) - } - Ok(response) => response.response_body().map_or_else( - || Ok(vec![]), - |hooks| { - Ok(hooks - .into_iter() - .filter(|hook| &hook.config.url == webhook_url.as_ref()) - .map(|hook| hook.id) - .map(|id| format!("{id}")) - .map(config::WebhookId::new) - .collect::>()) - }, - ), + let hostname = &repo_details.forge.hostname(); + let net = &github.net; + let mut page = 1; + loop { + let request = network::NetRequest::new( + network::RequestMethod::Get, + network::NetUrl::new(format!( + "https://api.{hostname}/repos/{}/hooks?page={page}", + repo_details.repo_path, + )), + github::webhook::headers(repo_details.forge.token()), + network::RequestBody::None, + network::ResponseType::Json, + None, + network::NetRequestLogging::None, + ); + let result = net.get::>(request).await; + match result { + Ok(response) => { + let Some(list) = response.response_body() else { + #[cfg(not(tarpaulin_include))] + // request response is Json so response_body never returns None + return Err(git::forge::webhook::Error::NetworkResponseEmpty); + }; + if list.is_empty() { + return Ok(ids); + } + for hook in list { + if hook.url().as_ref().starts_with(webhook_url.as_ref()) { + ids.push(hook.id()); + } + } + page += 1; + } + Err(e) => { + return Err(git::forge::webhook::Error::Network(e)); + } + }; } } diff --git a/crates/forge-github/src/webhook/mod.rs b/crates/forge-github/src/webhook/mod.rs index 7b0fba26..5e0a06a8 100644 --- a/crates/forge-github/src/webhook/mod.rs +++ b/crates/forge-github/src/webhook/mod.rs @@ -14,6 +14,9 @@ pub use parse::parse_body; pub use register::register; pub use unregister::unregister; +#[cfg(test)] +pub use authorised::sign_body; + pub fn headers(token: &config::ApiToken) -> kxio::network::NetRequestHeaders { use secrecy::ExposeSecret; kxio::network::NetRequestHeaders::default() diff --git a/crates/forge-github/src/webhook/register.rs b/crates/forge-github/src/webhook/register.rs index 461c860a..bca17be3 100644 --- a/crates/forge-github/src/webhook/register.rs +++ b/crates/forge-github/src/webhook/register.rs @@ -10,13 +10,19 @@ pub async fn register( github: &github::Github, webhook_url: &config::server::WebhookUrl, ) -> git::forge::webhook::Result { - let net = &github.net; let repo_details = &github.repo_details; + if repo_details.repo_config.is_none() { + return Err(git::forge::webhook::Error::NoRepoConfig); + }; + + let net = &github.net; + + let hostname = repo_details.forge.hostname(); let authorisation = config::WebhookAuth::generate(); let request = network::NetRequest::new( network::RequestMethod::Post, network::NetUrl::new(format!( - "https://api.github.com/repos/{}/hooks", + "https://api.{hostname}/repos/{}/hooks", repo_details.repo_path )), github::webhook::headers(repo_details.forge.token()), @@ -39,6 +45,8 @@ pub async fn register( match result { Ok(response) => { let Some(hook) = response.response_body() else { + #[cfg(not(tarpaulin_include))] + // request response is Json so response_body never returns None return Err(git::forge::webhook::Error::NetworkResponseEmpty); }; tracing::info!(webhook_id = %hook.id, "Webhook registered"); diff --git a/crates/forge-github/src/webhook/unregister.rs b/crates/forge-github/src/webhook/unregister.rs index e7318bcf..7bc0ac46 100644 --- a/crates/forge-github/src/webhook/unregister.rs +++ b/crates/forge-github/src/webhook/unregister.rs @@ -12,10 +12,11 @@ pub async fn unregister( ) -> git::forge::webhook::Result<()> { let net = &github.net; let repo_details = &github.repo_details; + let hostname = repo_details.forge.hostname(); let request = network::NetRequest::new( network::RequestMethod::Delete, network::NetUrl::new(format!( - "https://api.github.com/repos/{}/hooks/{}", + "https://api.{hostname}/repos/{}/hooks/{}", repo_details.repo_path, webhook_id )), github::webhook::headers(repo_details.forge.token()), @@ -24,9 +25,8 @@ pub async fn unregister( None, network::NetRequestLogging::None, ); - if let Err(e) = net.post_json::(request).await { - tracing::warn!("Failed to register webhook"); - return Err(git::forge::webhook::Error::FailedToRegister(e.to_string())); - } - Ok(()) + net.delete(request) + .await + .map_err(|e| git::forge::webhook::Error::FailedToRegister(e.to_string())) + .map(|_| ()) } diff --git a/justfile b/justfile index 661f7039..071a1f77 100644 --- a/justfile +++ b/justfile @@ -22,6 +22,10 @@ coverage-update: cargo tarpaulin --lib --out html echo "Now:\n\topen tarpaulin-report.html" +coverage crate test: + cargo tarpaulin -p {{crate}} --lib --out html -- {{test}} + # cargo tarpaulin --skip-clean -p {{crate}} --lib --out html -- {{test}} + grcov-coverage: #!/usr/bin/env bash set -e