diff options
| author | Joel Klinghed <the_jk@spawned.biz> | 2025-01-26 23:55:50 +0100 |
|---|---|---|
| committer | Joel Klinghed <the_jk@spawned.biz> | 2025-01-26 23:55:50 +0100 |
| commit | d1647b7a056f04ad5828976dd5a7e2e06b431feb (patch) | |
| tree | 11a4e1ad3f40bf653b7971bd40e54568f507ffb5 | |
| parent | 4fe3e610d5697aca4dfdc1033261130ec2b431ee (diff) | |
Stop using current user in git hooks
Want to support any authentication for the git server, so use git
commiter as username for creating reviews instead of the local user
that logged in to git.
Also verify that pushed commits has a valid author in pre-receive.
This is tricky as pre-receive must do this check in the hook, because
pre-receive runs when before the objects are pushed so the server
can't read the commits, the hook must do this.
| -rw-r--r-- | docker/dev/docker-compose.yaml | 4 | ||||
| -rw-r--r-- | server/.sqlx/query-12328898305c1a7e73acfcd01239c0c6dc5ac84289a2e7c6487f9817fb5f4f1c.json | 25 | ||||
| -rw-r--r-- | server/Cargo.lock | 11 | ||||
| -rw-r--r-- | server/Cargo.toml | 1 | ||||
| -rw-r--r-- | server/src/git.rs | 64 | ||||
| -rw-r--r-- | server/src/git_root.rs | 46 | ||||
| -rw-r--r-- | server/src/git_socket.rs | 5 | ||||
| -rw-r--r-- | server/src/githook.rs | 35 |
8 files changed, 147 insertions, 44 deletions
diff --git a/docker/dev/docker-compose.yaml b/docker/dev/docker-compose.yaml index bfb6ab6..dc8cf74 100644 --- a/docker/dev/docker-compose.yaml +++ b/docker/dev/docker-compose.yaml @@ -7,8 +7,8 @@ services: environment: - LDAP_ADMIN_USERNAME=admin - LDAP_ADMIN_PASSWORD=adminpassword - - LDAP_USERS=the_jk,user01,user02 - - LDAP_PASSWORDS=password,password1,password2 + - LDAP_USERS=user01,user02 + - LDAP_PASSWORDS=password1,password2 volumes: - 'openldap_data:/bitnami/openldap' mariadb: diff --git a/server/.sqlx/query-12328898305c1a7e73acfcd01239c0c6dc5ac84289a2e7c6487f9817fb5f4f1c.json b/server/.sqlx/query-12328898305c1a7e73acfcd01239c0c6dc5ac84289a2e7c6487f9817fb5f4f1c.json new file mode 100644 index 0000000..6e676f4 --- /dev/null +++ b/server/.sqlx/query-12328898305c1a7e73acfcd01239c0c6dc5ac84289a2e7c6487f9817fb5f4f1c.json @@ -0,0 +1,25 @@ +{ + "db_name": "MySQL", + "query": "SELECT id FROM users WHERE id=? AND dn IS NOT NULL", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": { + "type": "VarString", + "flags": "NOT_NULL | PRIMARY_KEY | NO_DEFAULT_VALUE", + "char_set": 224, + "max_size": 512 + } + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false + ] + }, + "hash": "12328898305c1a7e73acfcd01239c0c6dc5ac84289a2e7c6487f9817fb5f4f1c" +} diff --git a/server/Cargo.lock b/server/Cargo.lock index e9c42de..09ff182 100644 --- a/server/Cargo.lock +++ b/server/Cargo.lock @@ -563,7 +563,6 @@ dependencies = [ "testdir", "time", "tokio", - "users", "utoipa", "utoipa-swagger-ui", ] @@ -2996,16 +2995,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "daf8dba3b7eb870caf1ddeed7bc9d2a049f3cfdfae7cb521b087cc33ae4c49da" [[package]] -name = "users" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "24cc0f6d6f267b73e5a2cadf007ba8f9bc39c6a6f9666f8cf25ea809a153b032" -dependencies = [ - "libc", - "log", -] - -[[package]] name = "utf16_iter" version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" diff --git a/server/Cargo.toml b/server/Cargo.toml index aabcd82..5b337e0 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -16,7 +16,6 @@ serde = { version = "1.0", features = ["derive"] } sqlx = { version = "0.7.0", default-features = false, features = ["macros", "migrate"] } time = "0.3.34" tokio = { version = "1", features = ["process"] } -users = "0.11.0" utoipa = { version = "5", features = ["rocket_extras"] } utoipa-swagger-ui = { version = "9", features = ["rocket", "vendored"], default-features = false } diff --git a/server/src/git.rs b/server/src/git.rs index 9fb0593..7add644 100644 --- a/server/src/git.rs +++ b/server/src/git.rs @@ -31,6 +31,8 @@ impl fmt::Display for Error { impl std::error::Error for Error {} +pub const EMPTY: &str = "0000000000000000000000000000000000000000"; + struct RepoData { // Only one fetch at a time, and they should be in queue fetch_semaphore: Semaphore, @@ -50,10 +52,30 @@ pub struct Repository { lock: RwLock<RepoData>, } +#[allow(dead_code)] +pub struct User { + pub name: String, + pub email: String, + // Part before '@' in email + pub username: String, +} + fn io_err(action: &str, e: std::io::Error) -> Error { Error::new(format!("{action}: {e}")) } +fn parse_user(output: String) -> User { + let mut lines = output.lines(); + let name = lines.next().unwrap_or("").to_string(); + let username = lines.next().unwrap_or("").to_string(); + let email = lines.next().unwrap_or("").to_string(); + User { + name, + email, + username, + } +} + impl RepoData { fn new() -> Self { Self { @@ -246,6 +268,34 @@ impl RepoData { self.check(&mut cmd).await } + async fn get_author(&self, repo: &Repository, commit: &str) -> Result<User, Error> { + self.get_log_format(repo, commit, "%an%n%al%n%ae") + .map_ok(|output| parse_user(output)) + .await + } + + async fn get_commiter(&self, repo: &Repository, commit: &str) -> Result<User, Error> { + self.get_log_format(repo, commit, "%cn%n%cl%n%ce") + .map_ok(|output| parse_user(output)) + .await + } + + async fn get_log_format( + &self, + repo: &Repository, + commit: &str, + format: &str, + ) -> Result<String, Error> { + let mut cmd = self.git_cmd(repo); + cmd.arg("log") + .arg("-1") + .arg("--no-decorate") + .arg("--no-mailmap") + .arg(format!("--pretty=format:{format}")) + .arg(commit); + self.output(&mut cmd).await + } + fn git_cmd(&self, repo: &Repository) -> Command { let mut cmd = Command::new("git"); // Run as if git was started in <path> instead of the current working directory. @@ -445,6 +495,20 @@ impl Repository { data.is_equal_content(self, commit1.as_str(), commit2.as_str()) .await } + + pub async fn get_author(&self, commit: impl Into<String>) -> Result<User, Error> { + let commit = commit.into(); + let data = self.lock.read().await; + + data.get_author(self, commit.as_str()).await + } + + pub async fn get_commiter(&self, commit: impl Into<String>) -> Result<User, Error> { + let commit = commit.into(); + let data = self.lock.read().await; + + data.get_commiter(self, commit.as_str()).await + } } #[cfg(not(test))] diff --git a/server/src/git_root.rs b/server/src/git_root.rs index c6ee1fb..8102b18 100644 --- a/server/src/git_root.rs +++ b/server/src/git_root.rs @@ -84,8 +84,6 @@ impl std::fmt::Display for IoError { impl std::error::Error for IoError {} -const EMPTY: &str = "0000000000000000000000000000000000000000"; - fn is_printable(name: &str) -> bool { name.as_bytes().iter().all(|c| c.is_ascii_graphic()) } @@ -116,7 +114,30 @@ async fn git_process_prehook( let branch = row.reference.strip_prefix("refs/heads/").unwrap(); - if row.old_value == EMPTY { + if row.new_value != git::EMPTY { + match row.commiter { + Some(ref commiter) => match sqlx::query!( + "SELECT id FROM users WHERE id=? AND dn IS NOT NULL", + commiter, + ) + .fetch_one(&mut *db) + .map_err(|_| IoError::new(format!("{branch}: Unknown commiter {}", commiter))) + .await + { + Ok(_) => {} + Err(e) => { + errors.push(e.message); + continue; + } + }, + None => { + errors.push(format!("{branch}: Missing commiter")); + continue; + } + } + } + + if row.old_value == git::EMPTY { // Creating new review, nothing to check (yet). continue; } @@ -136,7 +157,7 @@ async fn git_process_prehook( .map_err(|_| IoError::new(format!("{branch}: Unknown branch"))) .await; - if row.new_value == EMPTY { + if row.new_value == git::EMPTY { // Do not allow to delete branch if there is a review connected to the branch. // All branches should be connected to a branch, but in case of errors this might // be relevant. @@ -209,7 +230,6 @@ async fn git_process_prehook( async fn git_process_posthook( repo: &git::Repository, mut db: DbConnection, - user_id: &String, receive: &Vec<git_socket::GitReceive>, ) -> git_socket::GitHookResponse { let mut messages: Vec<String> = Vec::new(); @@ -218,12 +238,20 @@ async fn git_process_posthook( for row in receive { let branch = row.reference.strip_prefix("refs/heads/").unwrap(); - if row.old_value == EMPTY { + if row.old_value == git::EMPTY { + let commiter = match repo.get_commiter(row.reference.as_str()).await { + Ok(user) => user, + Err(e) => { + messages.push(format!("{branch}: {e}")); + continue; + } + }; + // Create review match sqlx::query!( "INSERT INTO reviews (project, owner, title, branch) VALUES (?, ?, ?, ?)", repo.project_id(), - user_id, + commiter.username, "Unnamed", branch ) @@ -242,7 +270,7 @@ async fn git_process_posthook( messages.push(format!("{branch}: Error {e}",)); } }; - } else if row.new_value == EMPTY { + } else if row.new_value == git::EMPTY { // Delete branch, prehook already checked that it is not connected to a branch. } else { match sqlx::query!( @@ -312,7 +340,7 @@ async fn git_socket_process( let response = if request.pre { git_process_prehook(repo, db, &request.receive).await? } else { - git_process_posthook(repo, db, &request.user, &request.receive).await + git_process_posthook(repo, db, &request.receive).await }; task::spawn_blocking(move || { diff --git a/server/src/git_socket.rs b/server/src/git_socket.rs index 90f9dc2..a4805be 100644 --- a/server/src/git_socket.rs +++ b/server/src/git_socket.rs @@ -5,12 +5,15 @@ pub struct GitReceive { pub old_value: String, pub new_value: String, pub reference: String, + // Only set for pre hooks, because server can't read the objects the pre-hook has not yet + // accepted, so to be able to validate the commiter, send them. Also only set if new_value + // is not empty. + pub commiter: Option<String>, } #[derive(Deserialize, Serialize)] pub struct GitHookRequest { pub pre: bool, - pub user: String, pub receive: Vec<GitReceive>, } diff --git a/server/src/githook.rs b/server/src/githook.rs index 057ee47..f0e872a 100644 --- a/server/src/githook.rs +++ b/server/src/githook.rs @@ -6,7 +6,6 @@ use std::os::unix::net::UnixStream; use std::path::PathBuf; use tokio::io::{self, AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::task; -use users::get_current_username; mod fs_utils; mod git; @@ -33,11 +32,6 @@ impl fmt::Display for IoError { impl Error for IoError {} -async fn get_socket() -> Result<String, git::Error> { - let repo = git::Repository::new(PathBuf::from("."), true, None::<String>, None::<String>); - repo.config_get("eyeballs.socket").await -} - #[tokio::main] async fn main() -> Result<(), Box<dyn Error>> { let pre = match std::env::current_exe()? @@ -49,40 +43,41 @@ async fn main() -> Result<(), Box<dyn Error>> { _ => return Err(Box::<dyn Error>::from("Invalid hook executable name")), }; - let user = match get_current_username() { - Some(username) => match username.into_string() { - Ok(valid_username) => valid_username, - Err(_) => return Err(Box::<dyn Error>::from("Invalid username for current user")), - }, - None => { - return Err(Box::<dyn Error>::from( - "Unable to get username of current user", - )) - } - }; - let input = io::stdin(); let reader = BufReader::new(input); let mut lines = reader.lines(); let mut request = git_socket::GitHookRequest { pre, - user, receive: Vec::new(), }; + + let repo = git::Repository::new(PathBuf::from("."), true, None::<String>, None::<String>); + while let Some(line) = lines.next_line().await? { let data: Vec<&str> = line.split(' ').collect(); if data.len() == 3 { + let mut commiter: Option<String> = None; + if pre && data[1] != git::EMPTY { + match repo.get_commiter(data[1]).await { + Ok(user) => { + commiter = Some(user.username); + } + Err(_) => {} + } + } + request.receive.push(git_socket::GitReceive { old_value: data[0].to_string(), new_value: data[1].to_string(), reference: data[2].to_string(), + commiter: commiter, }) } } - let socket = PathBuf::from(get_socket().await?); + let socket = PathBuf::from(repo.config_get("eyeballs.socket").await?); let response = task::spawn_blocking(move || { let stream = UnixStream::connect(socket).map_err(|e| IoError::new(e.to_string()))?; |
