Skip to content

github: Use mockall to generate MockGitHubClient implementation #9946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
4 changes: 4 additions & 0 deletions crates/crates_io_github/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
1 change: 1 addition & 0 deletions crates/crates_io_github/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use serde::Deserialize;

type Result<T> = std::result::Result<T, GitHubError>;

#[cfg_attr(feature = "mock", mockall::automock)]
#[async_trait]
pub trait GitHubClient: Send + Sync {
async fn current_user(&self, auth: &AccessToken) -> Result<GithubUser>;
Expand Down
30 changes: 26 additions & 4 deletions src/tests/github_secret_scanning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
98 changes: 30 additions & 68 deletions src/tests/util/github.rs
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -51,30 +49,34 @@ pub(crate) const MOCK_GITHUB_DATA: MockData = MockData {
email: "[email protected]",
},
],
// 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<GithubUser, GitHubError> {
let user = &self.data.users[0];
fn current_user(&self) -> Result<GithubUser, GitHubError> {
let user = &self.users[0];
Ok(GithubUser {
id: user.id,
login: user.login.into(),
Expand All @@ -84,13 +86,8 @@ impl GitHubClient for MockGitHubClient {
})
}

async fn org_by_name(
&self,
org_name: &str,
_auth: &AccessToken,
) -> Result<GitHubOrganization, GitHubError> {
fn org_by_name(&self, org_name: &str) -> Result<GitHubOrganization, GitHubError> {
let org = self
.data
.orgs
.iter()
.find(|org| org.name == org_name.to_lowercase())
Expand All @@ -101,14 +98,8 @@ impl GitHubClient for MockGitHubClient {
})
}

async fn team_by_name(
&self,
org_name: &str,
team_name: &str,
auth: &AccessToken,
) -> Result<GitHubTeam, GitHubError> {
fn team_by_name(&self, org_name: &str, team_name: &str) -> Result<GitHubTeam, GitHubError> {
let team = self
.data
.orgs
.iter()
.find(|org| org.name == org_name.to_lowercase())
Expand All @@ -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<GitHubTeamMembership, GitHubError> {
let team = self
.data
.orgs
.iter()
.find(|org| org.id == org_id)
Expand All @@ -150,14 +139,12 @@ impl GitHubClient for MockGitHubClient {
}
}

async fn org_membership(
fn org_membership(
&self,
org_id: i32,
username: &str,
_auth: &AccessToken,
) -> Result<GitHubOrgMembership, GitHubError> {
let org = self
.data
.orgs
.iter()
.find(|org| org.id == org_id)
Expand All @@ -180,14 +167,6 @@ impl GitHubClient for MockGitHubClient {
Err(not_found())
}
}

async fn public_keys(
&self,
_username: &str,
_password: &str,
) -> Result<Vec<GitHubPublicKey>, GitHubError> {
Ok(self.data.public_keys.iter().map(Into::into).collect())
}
}

fn not_found() -> GitHubError {
Expand All @@ -197,7 +176,6 @@ fn not_found() -> GitHubError {
pub(crate) struct MockData {
orgs: &'static [MockOrg],
users: &'static [MockUser],
public_keys: &'static [MockPublicKey],
}

struct MockUser {
Expand All @@ -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,
}
}
}
19 changes: 13 additions & 6 deletions src/tests/util/test_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -103,6 +104,7 @@ impl TestApp {
build_job_runner: false,
use_chaos_proxy: false,
team_repo: MockTeamRepo::new(),
github: None,
}
}

Expand Down Expand Up @@ -252,6 +254,7 @@ pub struct TestAppBuilder {
build_job_runner: bool,
use_chaos_proxy: bool,
team_repo: MockTeamRepo,
github: Option<MockGitHubClient>,
}

impl TestAppBuilder {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -487,14 +495,13 @@ fn simple_config() -> config::Server {
}
}

fn build_app(config: config::Server) -> (Arc<App>, axum::Router) {
fn build_app(config: config::Server, github: Option<MockGitHubClient>) -> (Arc<App>, 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);

Expand Down