Skip to content
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

Implement setup for bitbucket #3

Merged
merged 5 commits into from
Oct 7, 2023
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
50 changes: 50 additions & 0 deletions vibi-dpu/src/bitbucket/auth.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use std::env;
use std::str;
use std::process::Command;
use std::time::{SystemTime, UNIX_EPOCH};
use reqwest::Client;
use crate::db::auth::{save_auth_info_to_db, auth_info};
use crate::utils::auth::AuthInfo;

pub async fn get_access_token_from_bitbucket(code: &str) -> Option<AuthInfo> {
let client = Client::new();
let bitbucket_client_id = env::var("BITBUCKET_CLIENT_ID").unwrap();
let bitbucket_client_secret = env::var("BITBUCKET_CLIENT_SECRET").unwrap();
Comment on lines +11 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of unwrap() on environment variables can lead to a panic if the variables are not set. It's better to handle these errors gracefully.

-    let bitbucket_client_id = env::var("BITBUCKET_CLIENT_ID").unwrap();
-    let bitbucket_client_secret = env::var("BITBUCKET_CLIENT_SECRET").unwrap();
+    let bitbucket_client_id = match env::var("BITBUCKET_CLIENT_ID") {
+        Ok(val) => val,
+        Err(_) => return None,
+    };
+    let bitbucket_client_secret = match env::var("BITBUCKET_CLIENT_SECRET") {
+        Ok(val) => val,
+        Err(_) => return None,
+    };

let mut params = std::collections::HashMap::new();
let redirect_uri = format!("{}/api/bitbucket/callbacks/install",
env::var("SERVER_URL").expect("SERVER_URL must be set"));
params.insert("client_id", bitbucket_client_id);
params.insert("client_secret", bitbucket_client_secret);
params.insert("code", code.to_owned());
params.insert("grant_type", "authorization_code".to_owned());
params.insert("redirect_uri", redirect_uri);
println!("params = {:?}", &params);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using println! for logging in production code. Consider using a logging library like log or env_logger which provides different log levels (debug, info, warn, error).

-    println!("params = {:?}", &params);
+    debug!("params = {:?}", &params);

let response = client
.post("https://bitbucket.org/site/oauth2/access_token")
.form(&params)
.send()
.await;
if response.is_err() {
let response_err = response.expect_err("No error in access token response");
eprintln!("error in calling api : {:?}", &response_err);
return None;
}
let res = response.expect("Uncaught error in response");
if !res.status().is_success() {
eprintln!(
"Failed to exchange code for access token. Status code: {}, Response content: {}",
res.status(),
res.text().await.expect("No text in response")
);
return None;
}
let json_res = res.json::<AuthInfo>().await;
if json_res.is_err() {
let json_error = json_res.expect_err("Error not found in json");
eprintln!("error deserializing : {:?}", json_error);
return None;
}
Comment on lines +41 to +46
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error handling could be improved by using the ? operator to propagate errors up the call stack. This would allow the caller to decide how to handle them.

-    if response.is_err() {
-        let response_err = response.expect_err("No error in access token response");
-        eprintln!("error in calling api : {:?}", &response_err);
-        return None;
-    }
-    let res = response.expect("Uncaught error in response");
-    if !res.status().is_success() {
-        eprintln!(
-            "Failed to exchange code for access token. Status code: {}, Response content: {}",
-            res.status(),
-            res.text().await.expect("No text in response")
-        );
-        return None;
-    }
-    let json_res = res.json::<AuthInfo>().await;
-    if json_res.is_err() {
-        let json_error = json_res.expect_err("Error not found in json");
-        eprintln!("error deserializing : {:?}", json_error);
-        return None;
-    }
+    let res = response?;
+    if !res.status().is_success() {
+        return Err(anyhow::anyhow!(
+            "Failed to exchange code for access token. Status code: {}, Response content: {}",
+            res.status(),
+            res.text().await?
+        ));
+    }
+    let response_json = res.json::<AuthInfo>().await?;

let mut response_json = json_res.expect("Uncaught error in deserializing response json");
save_auth_info_to_db(&mut response_json);
return Some(response_json);
}
106 changes: 106 additions & 0 deletions vibi-dpu/src/bitbucket/config.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
use std::{env, collections::HashMap};

use reqwest::{Response, header::{HeaderMap, HeaderValue}};
use serde_json::Value;

pub fn bitbucket_base_url() -> String {
env::var("BITBUCKET_BASE_URL").expect("BITBUCKET_BASE_URL must be set")
}

pub async fn get_api(url: &str, access_token: &str, params: Option<HashMap<&str, &str>> ) -> Vec<Value> {
let response_opt = call_get_api(url, access_token, &params).await;
println!("response of get_api = {:?}", &response_opt);
let (mut response_values, next_url) = deserialize_response(response_opt).await;
if next_url.is_some() {
let mut page_values = get_all_pages(next_url, access_token, &params).await;
response_values.append(&mut page_values);
}
return response_values;
}
Comment on lines +10 to +19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function get_api is making a call to the Bitbucket API and handling pagination. However, it's using println! for logging which is not recommended in production code. Consider using a proper logging framework like log or slog for better control over log levels and outputs.


pub async fn call_get_api(url: &str, access_token: &str, params: &Option<HashMap<&str, &str>> ) -> Option<Response>{
println!("GET api url = {}", url);
let client = reqwest::Client::new();
let mut headers = reqwest::header::HeaderMap::new();
headers.insert( reqwest::header::AUTHORIZATION,
format!("Bearer {}", access_token).parse().expect("Invalid auth header"), );
headers.insert("Accept",
"application/json".parse().expect("Invalid Accept header"));
match params {
Some(params) => {
match client.get(url).headers(headers).query(params).send().await {
Ok(response) => {
if response.status().is_success() {
return Some(response);
}
else { eprintln!("Failed to call API {}, status: {}", url, response.status()); }
},
Err(e) => { eprintln!("Error sending GET request to {}, error: {}", url, e); },
};
},
None => {
match client.get(url).headers(headers).send().await {
Ok(response) => {
if response.status().is_success() {
return Some(response);
}
else { eprintln!("Failed to call API {}, status: {}", url, response.status()); }
},
Err(e) => { eprintln!("Error sending GET request to {}, error: {}", url, e); },
};
}
};
return None;
}
Comment on lines +21 to +54
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function call_get_api is responsible for making GET requests to the Bitbucket API. It has some issues:

  1. The function is repeating code for the cases when params is Some and None. This repetition can be avoided by building the request separately from sending it.

  2. Error handling could be improved. Currently, the function prints an error message and returns None if the request fails. It would be more robust to return a Result type and let the caller decide how to handle errors.

  3. The function uses println! for logging which is not recommended in production code. Consider using a proper logging framework like log or slog.

Here's a suggested refactor:

pub async fn call_get_api(url: &str, access_token: &str, params: &Option<HashMap<&str, &str>> ) -> Result<Response, reqwest::Error> {
    let client = reqwest::Client::new();
    let mut headers = HeaderMap::new(); 
    headers.insert( reqwest::header::AUTHORIZATION, format!("Bearer {}", access_token).parse().expect("Invalid auth header"));
    headers.insert("Accept", "application/json".parse().expect("Invalid Accept header"));

    let request_builder = client.get(url).headers(headers);
    let request_builder = match params {
        Some(params) => request_builder.query(params),
        None => request_builder,
    };

    let response = request_builder.send().await?;
    if response.status().is_success() {
        Ok(response)
    } else {
        Err(reqwest::Error::new(reqwest::StatusCode::from_u16(response.status().as_u16()).unwrap(), "Failed to call API"))
    }
}


async fn deserialize_response(response_opt: Option<Response>) -> (Vec<Value>, Option<String>) {
let values_vec = Vec::new();
match response_opt {
Some(response) => {
match response.json::<serde_json::Value>().await {
Ok(response_json) => {
let mut values_vec = Vec::new();
if let Some(values) = response_json["values"].as_array() {
for value in values {
values_vec.push(value.to_owned());
}
return (values_vec, Some(response_json["next"].to_string()));
}
}
Err(e) => { eprintln!("Unable to deserialize response: {}", e); }
};
},
None => { eprintln!("Response is None");}
};
return (values_vec, None);
}
Comment on lines +56 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function deserialize_response is responsible for deserializing the response from the Bitbucket API. It has some issues:

  1. The function uses println! for logging which is not recommended in production code. Consider using a proper logging framework like log or slog.

  2. Error handling could be improved. Currently, the function prints an error message and returns an empty vector and None if deserialization fails. It would be more robust to return a Result type and let the caller decide how to handle errors.

  3. The function assumes that the next field in the response is always present. This might not be the case and could lead to runtime errors. Consider checking if the next field exists before trying to access it.


async fn get_all_pages(next_url: Option<String>, access_token: &str, params: &Option<HashMap<&str, &str>>) -> Vec<Value>{
let mut values_vec = Vec::new();
let mut next_url = next_url;
while next_url.is_some() {
let url = next_url.as_ref().expect("next_url is none").trim_matches('"');
if url != "null" {
let response_opt = call_get_api(url, access_token, params).await;
let (mut response_values, url_opt) = deserialize_response(response_opt).await;
next_url = url_opt.clone();
values_vec.append(&mut response_values);
} else {
break;
}
}
return values_vec;
}
Comment on lines +78 to +93
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function get_all_pages is responsible for fetching all pages of data from the Bitbucket API. It has some issues:

  1. The function uses expect on next_url.as_ref(), which will panic if next_url is None. This is not good practice as it can cause the program to crash. Consider using pattern matching or the if let construct to safely handle the Option.

  2. The function assumes that the next field in the response is always present. This might not be the case and could lead to runtime errors. Consider checking if the next field exists before trying to access it.

  3. The function checks if url is equal to "null". This is a bit of a hack and could lead to bugs if the API changes. Consider checking if url_opt is None instead.


pub fn prepare_auth_headers(access_token: &str) -> HeaderMap{
let mut headers_map = HeaderMap::new();
let auth_header = format!("Bearer {}", access_token);
let headervalres = HeaderValue::from_str(&auth_header);
match headervalres {
Ok(headerval) => {
headers_map.insert("Authorization", headerval);
},
Err(e) => panic!("Could not parse header value: {}", e),
};
return headers_map;
}
Comment on lines +95 to +106
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function prepare_auth_headers prepares the authorization headers for Bitbucket API calls. It has some issues:

  1. The function panics if it fails to parse the header value. This is not good practice as it can cause the program to crash. Consider returning a Result type and let the caller decide how to handle errors.

  2. The function only sets the Authorization header. Other headers like Accept might also be required for API calls. Consider making the function more flexible by allowing it to set multiple headers.

Here's a suggested refactor:

pub fn prepare_auth_headers(access_token: &str) -> Result<HeaderMap, reqwest::header::InvalidHeaderValue> {
    let mut headers_map = HeaderMap::new();
    let auth_header = format!("Bearer {}", access_token);
    let headerval = HeaderValue::from_str(&auth_header)?;
    headers_map.insert("Authorization", headerval);
    Ok(headers_map)
}

6 changes: 6 additions & 0 deletions vibi-dpu/src/bitbucket/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pub mod auth;
pub mod workspace;
pub mod repo;
mod config;
pub mod webhook;
pub mod user;
30 changes: 30 additions & 0 deletions vibi-dpu/src/bitbucket/repo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use serde_json::Value;

use crate::db::repo::save_repo_to_db;
use crate::utils::repo::Repository;
use super::config::{bitbucket_base_url, get_api};

pub async fn get_workspace_repos(workspace: &str, access_token: &str) -> Option<Vec<Repository>> {
let repos_url = format!("{}/repositories/{}", bitbucket_base_url(), workspace);
let response_json = get_api(&repos_url, access_token, None).await;
let mut repos_data = Vec::new();
for repo_json in response_json {
let val = Repository::new(
repo_json["name"].to_string().trim_matches('"').to_string(),
repo_json["uuid"].to_string().trim_matches('"').to_string(),
repo_json["owner"]["username"].to_string().trim_matches('"').to_string(),
repo_json["is_private"].as_bool().unwrap_or(false),
repo_json["links"]["clone"].as_array()
.expect("Unable to convert clone to array").iter().filter(|clone_val| {
clone_val["name".to_string()].as_str() == Some("ssh")
}).collect::<Vec<&Value>>()[0]["href"].to_string().replace('\"',""),
repo_json["project"]["name"].to_string().trim_matches('"').to_string(),
repo_json["workspace"]["slug"].to_string().trim_matches('"').to_string(),
None,
"bitbucket".to_string(),
);
save_repo_to_db(&val);
repos_data.push(val);
}
Some(repos_data)
}
23 changes: 23 additions & 0 deletions vibi-dpu/src/bitbucket/user.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use chrono::{DateTime, Utc, FixedOffset};
use crate::db::auth::auth_info;
use crate::db::user::{save_user_to_db, user_from_db};
use crate::utils::auth::AuthInfo;
use crate::utils::user::{User, Provider, ProviderEnum};
use super::config::{bitbucket_base_url, get_api, call_get_api};

pub async fn get_and_save_workspace_users(workspace_id: &str, access_token: &str) {
let base_url = bitbucket_base_url();
let members_url = format!("{}/workspaces/{}/members", &base_url, workspace_id);
let response_json = get_api(&members_url, access_token, None).await;
Comment on lines +9 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The get_api function is called without error handling. If the API call fails, it could cause a panic or unexpected behavior. Consider adding error handling to ensure that the program can recover gracefully from errors.

for user_json in response_json {
let provider_id = user_json["user"]["uuid"].to_string().replace('"', "");
let user = User::new(
Provider::new(
provider_id,
ProviderEnum::Bitbucket),
user_json["user"]["display_name"].to_string().replace('"', ""),
user_json["workspace"]["slug"].to_string().replace('"', ""),
None);
save_user_to_db(&user);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The save_user_to_db function is called without checking its result. If saving to the database fails, this could lead to data loss. Consider checking the result of this function and handling any errors that may occur.

}
Comment on lines +12 to +22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code assumes that the JSON response will always have the expected structure and fields. If the API response changes or if there's an error in the response, this could lead to panics due to accessing non-existent fields. It would be better to use a library like serde_json to safely parse the JSON into a struct.

}
86 changes: 86 additions & 0 deletions vibi-dpu/src/bitbucket/webhook.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use std::env;

use reqwest::{header::HeaderValue, Response, Error};
use serde_json::json;

use crate::{db::webhook::save_webhook_to_db, utils::webhook::{Webhook, WebhookResponse}, bitbucket::config::{bitbucket_base_url, get_api}};

use super::config::prepare_auth_headers;


pub async fn get_webhooks_in_repo(workspace_slug: &str, repo_slug: &str, access_token: &str) -> Vec<Webhook> {
let url = format!("{}/repositories/{}/{}/hooks", bitbucket_base_url(), workspace_slug, repo_slug);
println!("Getting webhooks from {}", url);
let response_json = get_api(&url, access_token, None).await;
let mut webhooks = Vec::new();
for webhook_json in response_json {
let active = matches!(webhook_json["active"].to_string().trim_matches('"'), "true" | "false");
let webhook = Webhook::new(
webhook_json["uuid"].to_string(),
active,
webhook_json["created_at"].to_string().replace('"', ""),
webhook_json["events"].as_array().expect("Unable to deserialize events").into_iter()
.map(|events| events.as_str().expect("Unable to convert event").to_string()).collect(),
webhook_json["links"]["self"]["href"].to_string().replace('"', ""),
webhook_json["url"].to_string().replace('"', ""),
);
Comment on lines +17 to +26
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code assumes that all fields in the webhook JSON are present and correctly formatted. If any of these assumptions are not met (for example, if a field is missing or is not the expected type), the program will panic. It would be better to handle these cases gracefully, perhaps by skipping over malformed webhooks or returning an error.

webhooks.push(webhook);
}
return webhooks;
}
Comment on lines +11 to +30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function get_webhooks_in_repo does not handle potential errors that may occur during the API call in line 14. If the API call fails or returns an error status, this function will still attempt to process the response as if it were successful, which could lead to unexpected behavior or panics. Consider adding error handling for the API call.


pub async fn add_webhook(workspace_slug: &str, repo_slug: &str, access_token: &str) {
let url = format!(
"{}/repositories/{}/{}/hooks",
bitbucket_base_url(), workspace_slug, repo_slug
);

let mut headers_map = prepare_auth_headers(&access_token);
headers_map.insert("Accept", HeaderValue::from_static("application/vnd.github+json"));
let callback_url = format!("{}/api/bitbucket/callbacks/webhook",
env::var("SERVER_URL").expect("SERVER_URL must be set"));
let payload = json!({
"description": "Webhook for PRs when raised and when something is pushed to the open PRs",
"url": callback_url,
"active": true,
"events": ["pullrequest:created", "pullrequest:updated"]
});

let response = reqwest::Client::new()
.post(&url)
.headers(headers_map)
.json(&payload)
.send()
.await;
process_add_webhook_response(response).await;
}
Comment on lines +32 to +56
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function add_webhook constructs a new webhook and sends a POST request to add it to the repository. However, it does not check whether a webhook with the same URL already exists in the repository. This could result in duplicate webhooks being created if this function is called multiple times with the same parameters. Consider checking for existing webhooks before creating a new one.


async fn process_add_webhook_response(response: Result<Response, Error>){
if response.is_err() {
let err = response.expect_err("No error in response");
eprintln!("Error in api call: {:?}", err);
return;
}
let res = response.expect("Uncaught error in response");
if !res.status().is_success() {
println!("Failed to add webhook. Status code: {}, Text: {:?}",
res.status(), res.text().await);
return;
}
let webhook_res = res.json::<WebhookResponse>().await;
if webhook_res.is_err() {
let err = webhook_res.expect_err("No error in webhook response");
return;
}

let webhook = webhook_res.expect("Uncaught error in webhook response");
let webhook_data = Webhook::new(
webhook.uuid().to_string(),
webhook.active(),
webhook.created_at().to_string(),
webhook.events().to_owned(),
webhook.links()["self"]["href"].clone(),
webhook.url().to_string(),
);
save_webhook_to_db(&webhook_data);
}
Comment on lines +58 to +86
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function process_add_webhook_response processes the response from the add_webhook function. However, it does not handle potential errors that may occur during the deserialization of the response body into a WebhookResponse object in line 70. If the response body is not valid JSON or does not match the structure of the WebhookResponse struct, the program will panic. Consider adding error handling for the deserialization step.

14 changes: 14 additions & 0 deletions vibi-dpu/src/bitbucket/workspace.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use super::config::{bitbucket_base_url, get_api};
use crate::db::owner::save_workspace_to_db;
use crate::utils::owner::Workspace;
pub async fn get_bitbucket_workspaces(access_token: &str) -> Vec<Workspace> {
let user_url = format!("{}/workspaces", bitbucket_base_url());
let response = get_api(&user_url, access_token, None).await;
let mut workspace_vec = Vec::new();
for workspace_json in response {
let val = serde_json::from_value::<Workspace>(workspace_json.clone()).expect("Unable to deserialize workspace");
save_workspace_to_db(&val);
workspace_vec.push(val);
}
return workspace_vec;
}
Comment on lines +1 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function get_bitbucket_workspaces is making an API call and directly saving the response to the database without any error handling. If the API call fails or returns an error, this could lead to unexpected behavior or crashes. Consider adding error handling for the API call and the database operation.

    let user_url = format!("{}/workspaces", bitbucket_base_url());
    let response = match get_api(&user_url, access_token, None).await {
        Ok(res) => res,
        Err(e) => {
            eprintln!("Failed to fetch workspaces: {}", e);
            return Vec::new();
        }
    };
    let mut workspace_vec = Vec::new();
    for workspace_json in response {
        let val = serde_json::from_value::<Workspace>(workspace_json.clone()).expect("Unable to deserialize workspace");
        if let Err(e) = save_workspace_to_db(&val) {
            eprintln!("Failed to save workspace to DB: {}", e);
            continue;
        }
        workspace_vec.push(val);
    }
    return workspace_vec;
}

1 change: 1 addition & 0 deletions vibi-dpu/src/core/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod setup;
Loading
Loading