From 8bf4699bd616c3dea815228040feacf9996eef6a Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sat, 26 Nov 2022 15:31:02 +0100 Subject: [PATCH] fix getting email from github --- backend/oauth_login.json | 2 +- backend/sqlx-data.json | 65 +++++++++++++++---- backend/windmill-api/src/oauth2.rs | 34 ++++++++-- backend/windmill-api/src/users.rs | 52 ++++++++++++++- .../src/routes/user/accept_invite@user.svelte | 2 +- .../routes/user/create_workspace@user.svelte | 2 +- 6 files changed, 133 insertions(+), 24 deletions(-) diff --git a/backend/oauth_login.json b/backend/oauth_login.json index b5cd675105..cdcc8c3191 100644 --- a/backend/oauth_login.json +++ b/backend/oauth_login.json @@ -3,7 +3,7 @@ "auth_url": "https://github.com/login/oauth/authorize", "token_url": "https://github.com/login/oauth/access_token", "userinfo_url": "https://api.github.com/user", - "scopes": ["read:user", "user:email"] + "scopes": ["user:email"] }, "gitlab": { "auth_url": "https://gitlab.com/oauth/authorize", diff --git a/backend/sqlx-data.json b/backend/sqlx-data.json index 56285836bb..f9911357ab 100644 --- a/backend/sqlx-data.json +++ b/backend/sqlx-data.json @@ -445,18 +445,6 @@ }, "query": "SELECT dependency_job FROM flow WHERE path = $1 AND workspace_id = $2" }, - "1ad8677694aca94ee0e6da287d7cc028dcf673583a0e3e4fedd0e5d6766c5860": { - "describe": { - "columns": [], - "nullable": [], - "parameters": { - "Left": [ - "Text" - ] - } - }, - "query": "DELETE FROM usr WHERE email = $1" - }, "1b31847d6187d6969deac5aa7b2feb169ef963449ac2d3ea06e1ed785f6d42e7": { "describe": { "columns": [ @@ -740,6 +728,18 @@ }, "query": "UPDATE workspace_settings SET slack_command_script = $1 WHERE workspace_id = $2" }, + "29785ae8f0cd2dbadc9fd294dc2d6eb396df0d8c5ce23184d5a20a1bdd6f3993": { + "describe": { + "columns": [], + "nullable": [], + "parameters": { + "Left": [ + "Text" + ] + } + }, + "query": "DELETE FROM usr_to_group WHERE usr = $1" + }, "2a4be8334db7d39f3d954193a8b0169cc4a4a07e081d2fa61d8764879d6a8ff5": { "describe": { "columns": [], @@ -1969,6 +1969,26 @@ }, "query": "SELECT email, login_type::text, verified, super_admin, name, company from password LIMIT $1 OFFSET $2" }, + "7a511ce8dbbf761423b527672ab02156aba5594623dc269992b34398673ca387": { + "describe": { + "columns": [ + { + "name": "username", + "ordinal": 0, + "type_info": "Varchar" + } + ], + "nullable": [ + false + ], + "parameters": { + "Left": [ + "Text" + ] + } + }, + "query": "DELETE FROM usr WHERE email = $1 RETURNING username" + }, "7aef087f9e10dd32417477109b97c99b85d68ce1be2bafdc145aed0aa8e5d989": { "describe": { "columns": [], @@ -2250,6 +2270,27 @@ }, "query": "SELECT * from resource_type WHERE (workspace_id = $1 OR workspace_id = 'starter') ORDER BY name" }, + "8c11511a74a41a65f448249a00ebe6964a61d00c2f7b4875a55e64741bf1f0ca": { + "describe": { + "columns": [ + { + "name": "exists", + "ordinal": 0, + "type_info": "Bool" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT EXISTS(SELECT 1 FROM usr WHERE workspace_id = $1 AND email = $2)" + }, "8caa01546506f42740b7973a3a45e40093a06714b8524570c5143b77af4a8e19": { "describe": { "columns": [ diff --git a/backend/windmill-api/src/oauth2.rs b/backend/windmill-api/src/oauth2.rs index 5449d6e28c..fd3a91fd98 100644 --- a/backend/windmill-api/src/oauth2.rs +++ b/backend/windmill-api/src/oauth2.rs @@ -813,9 +813,24 @@ async fn login_callback( let userinfo_url = client_w_config.userinfo_url.as_ref().ok_or_else(|| { Error::BadConfig(format!("Missing userinfo_url in client {client_name}")) })?; - let user = http_get_user_info(&http_client, userinfo_url, token).await?; + let user = http_get_user_info::(&http_client, userinfo_url, token).await?; - let email = user.email; + let email = match client_name.as_str() { + "github" => http_get_user_info::>( + &http_client, + "https://api.github.com/user/emails", + token, + ) + .await? + .iter() + .find(|x| x.primary && x.verified) + .ok_or(error::Error::BadRequest(format!( + "user does not have any primary and verified address" + )))? + .email + .to_string(), + _ => user.email, + }; if let Some(domains) = &client_w_config.allowed_domains { if !domains.iter().any(|d| email.ends_with(d)) { @@ -937,11 +952,18 @@ async fn exchange_code( .map_err(|e| error::Error::InternalErr(format!("{:?}", e))) } -async fn http_get_user_info( +#[derive(Deserialize)] +pub struct GHEmailInfo { + email: String, + verified: bool, + primary: bool, +} + +async fn http_get_user_info( http_client: &Client, url: &str, token: &str, -) -> error::Result { +) -> error::Result { Ok(http_client .get(url) .bearer_auth(token) @@ -949,10 +971,10 @@ async fn http_get_user_info( .await .map_err(to_anyhow) .context("failed to fetch user info")? - .json() + .json::() .await .map_err(to_anyhow) - .context("failed to decode email from user info")?) + .context("failed to decode json from user info")?) } fn oauth_redirect( diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs index f766d5f45a..f5ff4544e8 100644 --- a/backend/windmill-api/src/users.rs +++ b/backend/windmill-api/src/users.rs @@ -895,6 +895,38 @@ async fn add_user_to_workspace<'c>( is_admin: bool, mut tx: sqlx::Transaction<'c, sqlx::Postgres>, ) -> error::Result> { + let already_exists_username = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM usr WHERE workspace_id = $1 AND username = $2)", + &w_id, + username, + ) + .fetch_one(&mut tx) + .await? + .unwrap_or(false); + + if already_exists_username { + return Err(Error::BadRequest(format!( + "user with username {} already exists in workspace {}", + username, w_id + ))); + } + + let already_exists_email = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM usr WHERE workspace_id = $1 AND email = $2)", + &w_id, + username, + ) + .fetch_one(&mut tx) + .await? + .unwrap_or(false); + + if already_exists_email { + return Err(Error::BadRequest(format!( + "user with email {} already exists in workspace {}", + email, w_id + ))); + } + sqlx::query!( "INSERT INTO usr (workspace_id, email, username, is_admin) @@ -1006,11 +1038,18 @@ async fn delete_user( require_super_admin(&mut tx, email.clone()).await?; - sqlx::query!("DELETE FROM usr WHERE email = $1", &email_to_delete) + let username = sqlx::query_scalar!( + "DELETE FROM usr WHERE email = $1 RETURNING username", + &email_to_delete + ) + .fetch_one(&mut tx) + .await?; + + sqlx::query!("DELETE FROM password WHERE email = $1", &email_to_delete) .execute(&mut tx) .await?; - sqlx::query!("DELETE FROM password WHERE email = $1", &email_to_delete) + sqlx::query!("DELETE FROM usr_to_group WHERE usr = $1", &username) .execute(&mut tx) .await?; @@ -1092,7 +1131,14 @@ async fn delete_workspace_user( let email_to_delete = not_found_if_none(email_to_delete_o, "User", &username_to_delete)?; - sqlx::query!("DELETE FROM usr WHERE email = $1", email_to_delete) + let username = sqlx::query_scalar!( + "DELETE FROM usr WHERE email = $1 RETURNING username", + email_to_delete + ) + .fetch_one(&mut tx) + .await?; + + sqlx::query!("DELETE FROM usr_to_group WHERE usr = $1", &username) .execute(&mut tx) .await?; diff --git a/frontend/src/routes/user/accept_invite@user.svelte b/frontend/src/routes/user/accept_invite@user.svelte index 6ded384cb6..768b65f937 100644 --- a/frontend/src/routes/user/accept_invite@user.svelte +++ b/frontend/src/routes/user/accept_invite@user.svelte @@ -68,7 +68,7 @@ class:input-error={errorUsername != ''} /> -
+
diff --git a/frontend/src/routes/user/create_workspace@user.svelte b/frontend/src/routes/user/create_workspace@user.svelte index 19b3adb42e..0bd8cecb91 100644 --- a/frontend/src/routes/user/create_workspace@user.svelte +++ b/frontend/src/routes/user/create_workspace@user.svelte @@ -105,7 +105,7 @@ {/if} -
+