diff --git a/Cargo.lock b/Cargo.lock index e7c62d43eec..dfe85d42692 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1181,6 +1181,7 @@ version = "0.0.0" dependencies = [ "anyhow", "async-trait", + "mockall", "oauth2", "reqwest 0.12.9", "serde", diff --git a/Cargo.toml b/Cargo.toml index f09c6d947b8..8856cd55bb1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -122,6 +122,7 @@ unicode-xid = "=0.2.6" [dev-dependencies] bytes = "=1.8.0" +crates_io_github = { path = "crates/crates_io_github", features = ["mock"] } crates_io_index = { path = "crates/crates_io_index", features = ["testing"] } crates_io_tarball = { path = "crates/crates_io_tarball", features = ["builder"] } crates_io_team_repo = { path = "crates/crates_io_team_repo", features = ["mock"] } diff --git a/crates/crates_io_github/Cargo.toml b/crates/crates_io_github/Cargo.toml index 85fdf3a69e0..ad2b263aea0 100644 --- a/crates/crates_io_github/Cargo.toml +++ b/crates/crates_io_github/Cargo.toml @@ -7,9 +7,13 @@ edition = "2021" [lints] workspace = true +[features] +mock = ["dep:mockall"] + [dependencies] anyhow = "=1.0.93" async-trait = "=0.1.83" +mockall = { version = "=0.13.0", optional = true } oauth2 = { version = "=4.4.2", default-features = false } reqwest = { version = "=0.12.9", features = ["json"] } serde = { version = "=1.0.215", features = ["derive"] } diff --git a/crates/crates_io_github/src/lib.rs b/crates/crates_io_github/src/lib.rs index 644fb62b440..3520d3899c8 100644 --- a/crates/crates_io_github/src/lib.rs +++ b/crates/crates_io_github/src/lib.rs @@ -16,6 +16,7 @@ use serde::Deserialize; type Result = std::result::Result; +#[cfg_attr(feature = "mock", mockall::automock)] #[async_trait] pub trait GitHubClient: Send + Sync { async fn current_user(&self, auth: &AccessToken) -> Result; diff --git a/src/tests/github_secret_scanning.rs b/src/tests/github_secret_scanning.rs index e6f73411323..b6c9eeb64ac 100644 --- a/src/tests/github_secret_scanning.rs +++ b/src/tests/github_secret_scanning.rs @@ -2,6 +2,7 @@ use crate::tests::util::MockRequestExt; use crate::tests::{RequestHelper, TestApp}; use crate::util::token::HashedToken; use crate::{models::ApiToken, schema::api_tokens}; +use crates_io_github::{GitHubPublicKey, MockGitHubClient}; use diesel::prelude::*; use diesel_async::RunQueryDsl; use googletest::prelude::*; @@ -13,13 +14,34 @@ static URL: &str = "/api/github/secret-scanning/verify"; // Test request and signature from https://docs.github.com/en/developers/overview/secret-scanning-partner-program#create-a-secret-alert-service static GITHUB_ALERT: &[u8] = br#"[{"token":"some_token","type":"some_type","url":"some_url","source":"some_source"}]"#; + static GITHUB_PUBLIC_KEY_IDENTIFIER: &str = "f9525bf080f75b3506ca1ead061add62b8633a346606dc5fe544e29231c6ee0d"; + +/// Test key from https://docs.github.com/en/developers/overview/secret-scanning-partner-program#create-a-secret-alert-service +static GITHUB_PUBLIC_KEY: &str = "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEsz9ugWDj5jK5ELBK42ynytbo38gP\nHzZFI03Exwz8Lh/tCfL3YxwMdLjB+bMznsanlhK0RwcGP3IDb34kQDIo3Q==\n-----END PUBLIC KEY-----"; + static GITHUB_PUBLIC_KEY_SIGNATURE: &str = "MEUCIFLZzeK++IhS+y276SRk2Pe5LfDrfvTXu6iwKKcFGCrvAiEAhHN2kDOhy2I6eGkOFmxNkOJ+L2y8oQ9A2T9GGJo6WJY="; +fn github_mock() -> MockGitHubClient { + let mut mock = MockGitHubClient::new(); + + mock.expect_public_keys().returning(|_, _| { + let key = GitHubPublicKey { + key_identifier: GITHUB_PUBLIC_KEY_IDENTIFIER.to_string(), + key: GITHUB_PUBLIC_KEY.to_string(), + is_current: true, + }; + + Ok(vec![key]) + }); + + mock +} + #[tokio::test(flavor = "multi_thread")] async fn github_secret_alert_revokes_token() { - let (app, anon, user, token) = TestApp::init().with_token(); + let (app, anon, user, token) = TestApp::init().with_github(github_mock()).with_token(); let mut conn = app.async_db_conn().await; // Ensure no emails were sent up to this point @@ -77,7 +99,7 @@ async fn github_secret_alert_revokes_token() { #[tokio::test(flavor = "multi_thread")] async fn github_secret_alert_for_revoked_token() { - let (app, anon, user, token) = TestApp::init().with_token(); + let (app, anon, user, token) = TestApp::init().with_github(github_mock()).with_token(); let mut conn = app.async_db_conn().await; // Ensure no emails were sent up to this point @@ -138,7 +160,7 @@ async fn github_secret_alert_for_revoked_token() { #[tokio::test(flavor = "multi_thread")] async fn github_secret_alert_for_unknown_token() { - let (app, anon, user, token) = TestApp::init().with_token(); + let (app, anon, user, token) = TestApp::init().with_github(github_mock()).with_token(); let mut conn = app.async_db_conn().await; // Ensure no emails were sent up to this point @@ -180,7 +202,7 @@ async fn github_secret_alert_for_unknown_token() { #[tokio::test(flavor = "multi_thread")] async fn github_secret_alert_invalid_signature_fails() { - let (_, anon) = TestApp::init().empty(); + let (_, anon) = TestApp::init().with_github(github_mock()).empty(); // No headers or request body let request = anon.post_request(URL); diff --git a/src/tests/util/github.rs b/src/tests/util/github.rs index 5d5e0697fb1..18bd36c82a2 100644 --- a/src/tests/util/github.rs +++ b/src/tests/util/github.rs @@ -1,10 +1,8 @@ use anyhow::anyhow; -use async_trait::async_trait; use crates_io_github::{ - GitHubClient, GitHubError, GitHubOrgMembership, GitHubOrganization, GitHubPublicKey, - GitHubTeam, GitHubTeamMembership, GithubUser, + GitHubError, GitHubOrgMembership, GitHubOrganization, GitHubTeam, GitHubTeamMembership, + GithubUser, MockGitHubClient, }; -use oauth2::AccessToken; use std::sync::atomic::{AtomicUsize, Ordering}; static NEXT_GH_ID: AtomicUsize = AtomicUsize::new(0); @@ -51,30 +49,34 @@ pub(crate) const MOCK_GITHUB_DATA: MockData = MockData { email: "owner@example.com", }, ], - // Test key from https://docs.github.com/en/developers/overview/secret-scanning-partner-program#create-a-secret-alert-service - public_keys: &[ - MockPublicKey { - key_identifier: "f9525bf080f75b3506ca1ead061add62b8633a346606dc5fe544e29231c6ee0d", - key: "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEsz9ugWDj5jK5ELBK42ynytbo38gP\nHzZFI03Exwz8Lh/tCfL3YxwMdLjB+bMznsanlhK0RwcGP3IDb34kQDIo3Q==\n-----END PUBLIC KEY-----", - is_current: true, - }, - ], }; -pub(crate) struct MockGitHubClient { - data: &'static MockData, -} +impl MockData { + pub fn as_mock_client(&'static self) -> MockGitHubClient { + let mut mock = MockGitHubClient::new(); + + mock.expect_current_user() + .returning(|_auth| self.current_user()); + + mock.expect_org_by_name() + .returning(|org_name, _auth| self.org_by_name(org_name)); -impl MockGitHubClient { - pub(crate) fn new(data: &'static MockData) -> Self { - Self { data } + mock.expect_team_by_name() + .returning(|org_name, team_name, _auth| self.team_by_name(org_name, team_name)); + + mock.expect_team_membership() + .returning(|org_id, team_id, username, _auth| { + self.team_membership(org_id, team_id, username) + }); + + mock.expect_org_membership() + .returning(|org_id, username, _auth| self.org_membership(org_id, username)); + + mock } -} -#[async_trait] -impl GitHubClient for MockGitHubClient { - async fn current_user(&self, _auth: &AccessToken) -> Result { - let user = &self.data.users[0]; + fn current_user(&self) -> Result { + let user = &self.users[0]; Ok(GithubUser { id: user.id, login: user.login.into(), @@ -84,13 +86,8 @@ impl GitHubClient for MockGitHubClient { }) } - async fn org_by_name( - &self, - org_name: &str, - _auth: &AccessToken, - ) -> Result { + fn org_by_name(&self, org_name: &str) -> Result { let org = self - .data .orgs .iter() .find(|org| org.name == org_name.to_lowercase()) @@ -101,14 +98,8 @@ impl GitHubClient for MockGitHubClient { }) } - async fn team_by_name( - &self, - org_name: &str, - team_name: &str, - auth: &AccessToken, - ) -> Result { + fn team_by_name(&self, org_name: &str, team_name: &str) -> Result { let team = self - .data .orgs .iter() .find(|org| org.name == org_name.to_lowercase()) @@ -120,19 +111,17 @@ impl GitHubClient for MockGitHubClient { Ok(GitHubTeam { id: team.id, name: Some(team.name.into()), - organization: self.org_by_name(org_name, auth).await?, + organization: self.org_by_name(org_name)?, }) } - async fn team_membership( + fn team_membership( &self, org_id: i32, team_id: i32, username: &str, - _auth: &AccessToken, ) -> Result { let team = self - .data .orgs .iter() .find(|org| org.id == org_id) @@ -150,14 +139,12 @@ impl GitHubClient for MockGitHubClient { } } - async fn org_membership( + fn org_membership( &self, org_id: i32, username: &str, - _auth: &AccessToken, ) -> Result { let org = self - .data .orgs .iter() .find(|org| org.id == org_id) @@ -180,14 +167,6 @@ impl GitHubClient for MockGitHubClient { Err(not_found()) } } - - async fn public_keys( - &self, - _username: &str, - _password: &str, - ) -> Result, GitHubError> { - Ok(self.data.public_keys.iter().map(Into::into).collect()) - } } fn not_found() -> GitHubError { @@ -197,7 +176,6 @@ fn not_found() -> GitHubError { pub(crate) struct MockData { orgs: &'static [MockOrg], users: &'static [MockUser], - public_keys: &'static [MockPublicKey], } struct MockUser { @@ -219,19 +197,3 @@ struct MockTeam { name: &'static str, members: &'static [&'static str], } - -struct MockPublicKey { - key_identifier: &'static str, - key: &'static str, - is_current: bool, -} - -impl From<&'static MockPublicKey> for GitHubPublicKey { - fn from(k: &'static MockPublicKey) -> Self { - Self { - key_identifier: k.key_identifier.to_string(), - key: k.key.to_string(), - is_current: k.is_current, - } - } -} diff --git a/src/tests/util/test_app.rs b/src/tests/util/test_app.rs index 508c0caf9b1..d0eff98107b 100644 --- a/src/tests/util/test_app.rs +++ b/src/tests/util/test_app.rs @@ -9,9 +9,10 @@ use crate::rate_limiter::{LimitedAction, RateLimiterConfig}; use crate::schema::users; use crate::storage::StorageConfig; use crate::tests::util::chaosproxy::ChaosProxy; -use crate::tests::util::github::{MockGitHubClient, MOCK_GITHUB_DATA}; +use crate::tests::util::github::MOCK_GITHUB_DATA; use crate::worker::{Environment, RunnerExt}; use crate::{App, Emails, Env}; +use crates_io_github::MockGitHubClient; use crates_io_index::testing::UpstreamIndex; use crates_io_index::{Credentials, RepositoryConfig}; use crates_io_team_repo::MockTeamRepo; @@ -103,6 +104,7 @@ impl TestApp { build_job_runner: false, use_chaos_proxy: false, team_repo: MockTeamRepo::new(), + github: None, } } @@ -252,6 +254,7 @@ pub struct TestAppBuilder { build_job_runner: bool, use_chaos_proxy: bool, team_repo: MockTeamRepo, + github: Option, } impl TestAppBuilder { @@ -296,7 +299,7 @@ impl TestAppBuilder { (primary_proxy, replica_proxy) }; - let (app, router) = build_app(self.config); + let (app, router) = build_app(self.config, self.github); let runner = if self.build_job_runner { let index = self @@ -397,6 +400,11 @@ impl TestAppBuilder { self } + pub fn with_github(mut self, github: MockGitHubClient) -> Self { + self.github = Some(github); + self + } + pub fn with_team_repo(mut self, team_repo: MockTeamRepo) -> Self { self.team_repo = team_repo; self @@ -487,14 +495,13 @@ fn simple_config() -> config::Server { } } -fn build_app(config: config::Server) -> (Arc, axum::Router) { +fn build_app(config: config::Server, github: Option) -> (Arc, axum::Router) { // Use the in-memory email backend for all tests, allowing tests to analyze the emails sent by // the application. This will also prevent cluttering the filesystem. let emails = Emails::new_in_memory(); - // Use a custom mock for the GitHub client, allowing to define the GitHub users and - // organizations without actually having to create GitHub accounts. - let github = Box::new(MockGitHubClient::new(&MOCK_GITHUB_DATA)); + let github = github.unwrap_or_else(|| MOCK_GITHUB_DATA.as_mock_client()); + let github = Box::new(github); let app = App::new(config, emails, github);