feat: use user id instead of username

Author: vohoanglong0107

src/bors/handlers/trybuild.rs

@@ -262,7 +262,7 @@ async fn check_try_permissions<Client: RepositoryClient>(
 ) -> anyhow::Result<bool> {
     let result = if !repo
         .permissions_resolver
-        .has_permission(&author.username, PermissionType::Try)
+        .has_permission(&author.id, PermissionType::Try)
         .await
     {
         tracing::info!("Permission denied");

src/github/mod.rs

@@ -2,6 +2,7 @@
 //! for working with (GitHub) repositories.
 use std::fmt::{Debug, Display, Formatter};
 
+use octocrab::models::UserId;
 use url::Url;
 
 pub mod api;
@@ -46,6 +47,7 @@ impl Display for GithubRepoName {
 
 #[derive(Debug, PartialEq)]
 pub struct GithubUser {
+    pub id: UserId,
     pub username: String,
     pub html_url: Url,
 }

src/github/webhook.rs

@@ -282,6 +282,7 @@ fn parse_comment_from_pr_review(
 
 fn parse_user(user: Author) -> GithubUser {
     GithubUser {
+        id: user.id,
         username: user.login,
         html_url: user.html_url,
     }
@@ -381,6 +382,9 @@ mod tests {
                             name: "bors-kindergarten",
                         },
                         author: GithubUser {
+                            id: UserId(
+                                4539057,
+                            ),
                             username: "Kobzol",
                             html_url: Url {
                                 scheme: "https",
@@ -424,6 +428,9 @@ mod tests {
                             name: "bors-kindergarten",
                         },
                         author: GithubUser {
+                            id: UserId(
+                                4539057,
+                            ),
                             username: "Kobzol",
                             html_url: Url {
                                 scheme: "https",
@@ -467,6 +474,9 @@ mod tests {
                             name: "bors-kindergarten",
                         },
                         author: GithubUser {
+                            id: UserId(
+                                4539057,
+                            ),
                             username: "Kobzol",
                             html_url: Url {
                                 scheme: "https",

src/permissions.rs

@@ -1,4 +1,5 @@
 use axum::async_trait;
+use octocrab::models::UserId;
 use std::collections::HashSet;
 use tokio::sync::Mutex;
 
@@ -14,7 +15,7 @@ pub enum PermissionType {
 /// Decides if a GitHub user can perform various actions using the bot.
 #[async_trait]
 pub trait PermissionResolver {
-    async fn has_permission(&self, username: &str, permission: PermissionType) -> bool;
+    async fn has_permission(&self, _username: &UserId, _permission: PermissionType) -> bool;
     async fn reload(&self);
 }
 
@@ -46,11 +47,11 @@ impl TeamApiPermissionResolver {
 
 #[async_trait]
 impl PermissionResolver for TeamApiPermissionResolver {
-    async fn has_permission(&self, username: &str, permission: PermissionType) -> bool {
+    async fn has_permission(&self, user_id: &UserId, permission: PermissionType) -> bool {
         self.permissions
             .lock()
             .await
-            .has_permission(username, permission)
+            .has_permission(user_id, permission)
     }
 
     async fn reload(&self) {
@@ -59,15 +60,15 @@ impl PermissionResolver for TeamApiPermissionResolver {
 }
 
 pub struct UserPermissions {
-    review_users: HashSet<String>,
-    try_users: HashSet<String>,
+    review_users: HashSet<UserId>,
+    try_users: HashSet<UserId>,
 }
 
 impl UserPermissions {
-    fn has_permission(&self, username: &str, permission: PermissionType) -> bool {
+    fn has_permission(&self, user_id: &UserId, permission: PermissionType) -> bool {
         match permission {
-            PermissionType::Review => self.review_users.contains(username),
-            PermissionType::Try => self.try_users.contains(username),
+            PermissionType::Review => self.review_users.contains(user_id),
+            PermissionType::Try => self.try_users.contains(user_id),
         }
     }
 }
@@ -90,14 +91,14 @@ async fn load_permissions(repo: &GithubRepoName) -> anyhow::Result<UserPermissio
 
 #[derive(serde::Deserialize)]
 struct UserPermissionsResponse {
-    github_users: HashSet<String>,
+    github_ids: HashSet<UserId>,
 }
 
 /// Loads users that are allowed to perform try/review from the Rust Team API.
 async fn load_users_from_team_api(
     repository_name: &str,
     permission: PermissionType,
-) -> anyhow::Result<HashSet<String>> {
+) -> anyhow::Result<HashSet<UserId>> {
     let permission = match permission {
         PermissionType::Review => "review",
         PermissionType::Try => "try",
@@ -112,5 +113,5 @@ async fn load_users_from_team_api(
         .json::<UserPermissionsResponse>()
         .await
         .map_err(|error| anyhow::anyhow!("Cannot deserialize users from team API: {error:?}"))?;
-    Ok(users.github_users)
+    Ok(users.github_ids)
 }

src/tests/event.rs

@@ -1,5 +1,5 @@
 use derive_builder::Builder;
-use octocrab::models::RunId;
+use octocrab::models::{RunId, UserId};
 
 use crate::bors::event::PullRequestComment;
 use crate::bors::{event, CheckSuite, CheckSuiteStatus};
@@ -9,6 +9,7 @@ use crate::tests::state::{default_merge_sha, default_repo_name};
 
 fn default_user() -> GithubUser {
     GithubUser {
+        id: UserId(1),
         username: "<user>".to_string(),
         html_url: "https://user.com".parse().unwrap(),
     }

src/tests/permissions.rs

@@ -2,12 +2,13 @@ use std::sync::{Arc, Mutex};
 
 use crate::permissions::{PermissionResolver, PermissionType};
 use axum::async_trait;
+use octocrab::models::UserId;
 
 pub struct NoPermissions;
 
 #[async_trait]
 impl PermissionResolver for NoPermissions {
-    async fn has_permission(&self, _username: &str, _permission: PermissionType) -> bool {
+    async fn has_permission(&self, _username: &UserId, _permission: PermissionType) -> bool {
         false
     }
     async fn reload(&self) {}
@@ -17,7 +18,7 @@ pub struct AllPermissions;
 
 #[async_trait]
 impl PermissionResolver for AllPermissions {
-    async fn has_permission(&self, _username: &str, _permission: PermissionType) -> bool {
+    async fn has_permission(&self, _username: &UserId, _permission: PermissionType) -> bool {
         true
     }
     async fn reload(&self) {}
@@ -37,7 +38,7 @@ impl Default for MockPermissions {
 
 #[async_trait]
 impl PermissionResolver for Arc<Mutex<MockPermissions>> {
-    async fn has_permission(&self, _username: &str, _permission: PermissionType) -> bool {
+    async fn has_permission(&self, _username: &UserId, _permission: PermissionType) -> bool {
         false
     }
     async fn reload(&self) {

src/tests/state.rs

@@ -7,7 +7,7 @@ use std::time::Duration;
 use crate::config::RepositoryConfig;
 use axum::async_trait;
 use derive_builder::Builder;
-use octocrab::models::RunId;
+use octocrab::models::{RunId, UserId};
 
 use super::permissions::AllPermissions;
 use crate::bors::event::{
@@ -29,6 +29,8 @@ use crate::tests::github::{default_base_branch, PRBuilder};
 
 pub fn test_bot_user() -> GithubUser {
     GithubUser {
+        // just a random one, to reduce the chance of duplicate id
+        id: UserId(517237103),
         username: "<test-bot>".to_string(),
         html_url: "https://test-bors.bot.com".parse().unwrap(),
     }