Skip to content

Commit

Permalink
fix: remove authorize from login/logout webscope (#922)
Browse files Browse the repository at this point in the history
This PR fixes the issue where oauth user fails to login with 
error - "no authorization header passed". The authorize 
check in /o/login handler returns unauthorized error for users 
not having login privilege.
  • Loading branch information
nikhilsinhaparseable authored Sep 11, 2024
1 parent 75cda6b commit c916f1f
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 6 deletions.
6 changes: 2 additions & 4 deletions server/src/handlers/http/modal/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,8 @@ impl Server {
// get the oauth webscope
pub fn get_oauth_webscope(oidc_client: Option<OpenIdClient>) -> 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 {
Expand Down
13 changes: 11 additions & 2 deletions server/src/handlers/http/oidc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use crate::{
oidc::{Claims, DiscoveredClient},
option::CONFIG,
rbac::{
self,
map::{SessionKey, DEFAULT_ROLE},
user::{self, User, UserType},
Users,
Expand Down Expand Up @@ -64,13 +65,18 @@ pub async fn login(
) -> Result<HttpResponse, OIDCError> {
let oidc_client = req.app_data::<Data<DiscoveredClient>>();
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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
}
}

Expand Down

0 comments on commit c916f1f

Please sign in to comment.