From b82ab7c75cf0d526f03b005a25fa55cc465a3642 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Tue, 10 Sep 2024 13:21:53 +0530 Subject: [PATCH] fix: remove authorize from login/logout webscope issue - oauth user fails to login with error - no authorization header passed added authorize check in /o/login handler returns unauthorized error for users not having login priviledge --- Cargo.lock | 4 ++-- server/Cargo.toml | 6 +++--- server/src/handlers/http/modal/server.rs | 6 ++---- server/src/handlers/http/oidc.rs | 13 +++++++++++-- server/src/storage/s3.rs | 4 ++-- server/src/utils/arrow.rs | 6 ++---- 6 files changed, 22 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6f96b76d8..d4644b671 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -707,9 +707,9 @@ dependencies = [ [[package]] name = "async-trait" -version = "0.1.80" +version = "0.1.82" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c6fa2087f2753a7da8cc1c0dbfcf89579dd57458e36769de5ac750b4671737ca" +checksum = "a27b8a3a6e1a44fa4c8baf1f653e4172e81486d4941f2237e20dc2d0cf4ddff1" dependencies = [ "proc-macro2", "quote", diff --git a/server/Cargo.toml b/server/Cargo.toml index 0bc623a79..29a490db1 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -34,7 +34,7 @@ mime = "0.3.17" ### other dependencies anyhow = { version = "1.0", features = ["backtrace"] } argon2 = "0.5.0" -async-trait = "0.1" +async-trait = "0.1.82" base64 = "0.22.0" lazy_static = "1.4" bytes = "1.4" @@ -71,7 +71,7 @@ prometheus = { version = "0.13", features = ["process"] } rand = "0.8" regex = "1.7.3" relative-path = { version = "1.7", features = ["serde"] } -reqwest = { version = "0.11.27", default_features = false, features = [ +reqwest = { version = "0.11.27", default-features = false, features = [ "rustls-tls", "json", ] } # cannot update cause rustls is not latest `see rustls` @@ -113,7 +113,7 @@ sha1_smol = { version = "1.0", features = ["std"] } static-files = "0.2" ureq = "2.6" vergen = { version = "8.1", features = ["build", "git", "cargo", "gitcl"] } -zip = { version = "1.1.1", default_features = false, features = ["deflate"] } +zip = { version = "1.1.1", default-features = false, features = ["deflate"] } url = "2.4.0" prost-build = "0.12.3" diff --git a/server/src/handlers/http/modal/server.rs b/server/src/handlers/http/modal/server.rs index 08fd8eee2..a58a843dd 100644 --- a/server/src/handlers/http/modal/server.rs +++ b/server/src/handlers/http/modal/server.rs @@ -430,10 +430,8 @@ impl Server { // get the oauth webscope pub fn get_oauth_webscope(oidc_client: Option) -> Scope { let oauth = web::scope("/o") - .service(resource("/login").route(web::get().to(oidc::login).authorize(Action::Login))) - .service( - resource("/logout").route(web::get().to(oidc::logout).authorize(Action::Login)), - ) + .service(resource("/login").route(web::get().to(oidc::login))) + .service(resource("/logout").route(web::get().to(oidc::logout))) .service(resource("/code").route(web::get().to(oidc::reply_login))); if let Some(client) = oidc_client { diff --git a/server/src/handlers/http/oidc.rs b/server/src/handlers/http/oidc.rs index bcc749225..90c68aa54 100644 --- a/server/src/handlers/http/oidc.rs +++ b/server/src/handlers/http/oidc.rs @@ -35,6 +35,7 @@ use crate::{ oidc::{Claims, DiscoveredClient}, option::CONFIG, rbac::{ + self, map::{SessionKey, DEFAULT_ROLE}, user::{self, User, UserType}, Users, @@ -64,13 +65,18 @@ pub async fn login( ) -> Result { let oidc_client = req.app_data::>(); let session_key = extract_session_key_from_req(&req).ok(); - let (session_key, oidc_client) = match (session_key, oidc_client) { (None, None) => return Ok(redirect_no_oauth_setup(query.redirect.clone())), (None, Some(client)) => return Ok(redirect_to_oidc(query, client)), (Some(session_key), client) => (session_key, client), }; - + // try authorize + match Users.authorize(session_key.clone(), rbac::role::Action::Login, None, None) { + rbac::Response::Authorized => (), + rbac::Response::UnAuthorized | rbac::Response::ReloadRequired => { + return Err(OIDCError::Unauthorized) + } + } match session_key { // We can exchange basic auth for session cookie SessionKey::BasicAuth { username, password } => match Users.get_user(&username) { @@ -358,6 +364,8 @@ pub enum OIDCError { Serde(#[from] serde_json::Error), #[error("Bad Request")] BadRequest, + #[error("Unauthorized")] + Unauthorized, } impl actix_web::ResponseError for OIDCError { @@ -366,6 +374,7 @@ impl actix_web::ResponseError for OIDCError { Self::ObjectStorageError(_) => StatusCode::INTERNAL_SERVER_ERROR, Self::Serde(_) => StatusCode::INTERNAL_SERVER_ERROR, Self::BadRequest => StatusCode::BAD_REQUEST, + Self::Unauthorized => StatusCode::UNAUTHORIZED, } } diff --git a/server/src/storage/s3.rs b/server/src/storage/s3.rs index 8f4b6a257..d359a11a3 100644 --- a/server/src/storage/s3.rs +++ b/server/src/storage/s3.rs @@ -322,7 +322,7 @@ impl S3 { stream_json_check.push(task); } - stream_json_check.try_collect().await?; + stream_json_check.try_collect::<()>().await?; Ok(dirs.into_iter().map(|name| LogStream { name }).collect()) } @@ -633,7 +633,7 @@ impl ObjectStorage for S3 { stream_json_check.push(task); } - stream_json_check.try_collect().await?; + stream_json_check.try_collect::<()>().await?; Ok(dirs.into_iter().map(|name| LogStream { name }).collect()) } diff --git a/server/src/utils/arrow.rs b/server/src/utils/arrow.rs index 4bb9cde93..f88be6048 100644 --- a/server/src/utils/arrow.rs +++ b/server/src/utils/arrow.rs @@ -98,10 +98,8 @@ pub fn record_batches_to_json(records: &[&RecordBatch]) -> Result> = match serde_json::from_reader(buf.as_slice()) { - Ok(json) => json, - Err(_) => vec![], - }; + let json_rows: Vec> = + serde_json::from_reader(buf.as_slice()).unwrap_or_default(); Ok(json_rows) }