* [pbs-devel] [PATCH proxmox-backup 0/2] add support for HttpOnly cookies for OpenID authentication flow
@ 2025-08-07 10:49 Shannon Sterz
2025-08-07 10:49 ` [pbs-devel] [PATCH proxmox-backup 1/2] api: openid: allow users of openid to opt into the HttpOnly cookies Shannon Sterz
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Shannon Sterz @ 2025-08-07 10:49 UTC (permalink / raw)
To: pbs-devel
these two patches do the following:
1. add a `http-only` parameter to the OpenID login endpoint, so clients
can opt into receive the authentication ticket via a HttpOnly cookie
2. opt the ui dialog into using this new HttpOnly parameter
this should fix a bug where users were instantly logged out again after
a successful OpenID authentication.
Shannon Sterz (2):
api: openid: allow users of openid to opt into the HttpOnly cookies
ui: opt open id authentication flows into the new http only flow
src/api2/access/openid.rs | 346 ++++++++++++++++++++++----------------
www/LoginView.js | 1 +
2 files changed, 206 insertions(+), 141 deletions(-)
--
2.47.2
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pbs-devel] [PATCH proxmox-backup 1/2] api: openid: allow users of openid to opt into the HttpOnly cookies 2025-08-07 10:49 [pbs-devel] [PATCH proxmox-backup 0/2] add support for HttpOnly cookies for OpenID authentication flow Shannon Sterz @ 2025-08-07 10:49 ` Shannon Sterz 2025-08-07 11:46 ` Mira Limbeck 2025-08-07 10:49 ` [pbs-devel] [PATCH proxmox-backup 2/2] ui: opt open id authentication flows into the new http only flow Shannon Sterz 2025-08-07 12:05 ` [pbs-devel] Superseded: [PATCH proxmox-backup 0/2] add support for HttpOnly cookies for OpenID authentication flow Shannon Sterz 2 siblings, 1 reply; 6+ messages in thread From: Shannon Sterz @ 2025-08-07 10:49 UTC (permalink / raw) To: pbs-devel add a `http-only` api parameter that allows users to opt into the the HttpOnly cookie based authentication flow. here the server will set a cookie via a `Set-Cookie` header instead of providing it in the response's body. this protects users better against cookie stealing attacks and other similar attacks. note that this has the side effect of always returning extjs-like responses here. Analyzed-by: Mira Limbeck <m.limbeck@proxmox.com> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com> --- src/api2/access/openid.rs | 346 ++++++++++++++++++++++---------------- 1 file changed, 205 insertions(+), 141 deletions(-) diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs index 8e39cbc9..1beb77bc 100644 --- a/src/api2/access/openid.rs +++ b/src/api2/access/openid.rs @@ -1,13 +1,16 @@ //! OpenID redirect/login API use anyhow::{bail, format_err, Error}; +use hyper::http::request::Parts; +use hyper::Response; use serde_json::{json, Value}; use proxmox_auth_api::api::ApiTicket; use proxmox_auth_api::ticket::Ticket; use proxmox_router::{ - http_err, list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap, + http_err, list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission, + Router, RpcEnvironment, SubdirMap, }; -use proxmox_schema::api; +use proxmox_schema::{api, BooleanSchema, ObjectSchema, ParameterSchema, StringSchema}; use proxmox_sortable_macro::sortable; use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig}; @@ -58,167 +61,228 @@ fn openid_authenticator( OpenIdAuthenticator::discover(&config, redirect_url) } -#[api( - input: { - properties: { - state: { - description: "OpenId state.", - type: String, - }, - code: { - description: "OpenId authorization code.", - type: String, - }, - "redirect-url": { - description: "Redirection Url. The client should set this to used server url.", - type: String, - }, - }, - }, - returns: { - properties: { - username: { - type: String, - description: "User name.", - }, - ticket: { - type: String, - description: "Auth ticket.", - }, - CSRFPreventionToken: { - type: String, - description: "Cross Site Request Forgery Prevention Token.", - }, - }, - }, - protected: true, - access: { - permission: &Permission::World, - }, -)] -/// Verify OpenID authorization code and create a ticket -/// -/// Returns: An authentication ticket with additional infos. -pub fn openid_login( - state: String, - code: String, - redirect_url: String, - rpcenv: &mut dyn RpcEnvironment, -) -> Result<Value, Error> { - use proxmox_rest_server::RestEnvironment; +#[sortable] +pub const API_METHOD_OPENID_LOGIN: ApiMethod = ApiMethod::new_full( + &ApiHandler::AsyncHttpBodyParameters(&create_ticket_http_only), + ParameterSchema::Object(&ObjectSchema::new( + "Get a new ticket as an HttpOnly cookie. Supports tickets via cookies.", + &sorted!([ + ("state", false, &StringSchema::new("OpenId state.").schema()), + ( + "code", + false, + &StringSchema::new("OpenId authorization code.").schema(), + ), + ( + "redirect-url", + false, + &StringSchema::new( + "Redirection Url. The client should set this to used server url.", + ) + .schema(), + ), + ( + "http-only", + true, + &BooleanSchema::new("Whether the HttpOnly authentication flow should be used.") + .default(false) + .schema(), + ), + ]), + )), +) +.returns(::proxmox_schema::ReturnType::new( + false, + &ObjectSchema::new( + "An authentication ticket with additional infos.", + &sorted!([ + ("username", false, &StringSchema::new("User name.").schema()), + ( + "ticket", + true, + &StringSchema::new( + "Auth ticket, present if http-only was not provided or is false." + ) + .schema() + ), + ("ticket-info", + true, + &StringSchema::new( + "Informational ticket, can only be used to check if the ticket is expired. Present if http-only was true." + ).schema()), + ( + "CSRFPreventionToken", + false, + &StringSchema::new("Cross Site Request Forgery Prevention Token.").schema(), + ), + ]), + ) + .schema(), +)) +.protected(true) +.access(None, &Permission::World) +.reload_timezone(true); - let env: &RestEnvironment = rpcenv - .as_any() - .downcast_ref::<RestEnvironment>() - .ok_or_else(|| format_err!("detected wrong RpcEnvironment type"))?; +fn create_ticket_http_only( + _parts: Parts, + param: Value, + _info: &ApiMethod, + rpcenv: Box<dyn RpcEnvironment>, +) -> ApiResponseFuture { + Box::pin(async move { + use proxmox_rest_server::RestEnvironment; - let user_info = CachedUserInfo::new()?; + let code = param["code"] + .as_str() + .ok_or_else(|| format_err!("missing non-optional parameter: code"))? + .to_owned(); + let state = param["state"] + .as_str() + .ok_or_else(|| format_err!("missing non-optional parameter: state"))? + .to_owned(); + let redirect_url = param["redirect-url"] + .as_str() + .ok_or_else(|| format_err!("missing non-optional parameter: redirect-url"))? + .to_owned(); - let mut tested_username = None; + let env: &RestEnvironment = rpcenv + .as_any() + .downcast_ref::<RestEnvironment>() + .ok_or_else(|| format_err!("detected wrong RpcEnvironment type"))?; - let result = proxmox_lang::try_block!({ - let (realm, private_auth_state) = - OpenIdAuthenticator::verify_public_auth_state(PROXMOX_BACKUP_RUN_DIR_M!(), &state)?; + let user_info = CachedUserInfo::new()?; - let (domains, _digest) = pbs_config::domains::config()?; - let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?; + let mut tested_username = None; - let open_id = openid_authenticator(&config, &redirect_url)?; + let result = proxmox_lang::try_block!({ + let (realm, private_auth_state) = + OpenIdAuthenticator::verify_public_auth_state(PROXMOX_BACKUP_RUN_DIR_M!(), &state)?; - let info = open_id.verify_authorization_code_simple(&code, &private_auth_state)?; + let (domains, _digest) = pbs_config::domains::config()?; + let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?; - // eprintln!("VERIFIED {:?}", info); + let open_id = openid_authenticator(&config, &redirect_url)?; - let name_attr = config.username_claim.as_deref().unwrap_or("sub"); + let info = open_id.verify_authorization_code_simple(&code, &private_auth_state)?; - // Try to be compatible with previous versions - let try_attr = match name_attr { - "subject" => Some("sub"), - "username" => Some("preferred_username"), - _ => None, - }; + // eprintln!("VERIFIED {:?}", info); - let unique_name = match info[name_attr].as_str() { - Some(name) => name.to_owned(), - None => { - if let Some(try_attr) = try_attr { - match info[try_attr].as_str() { - Some(name) => name.to_owned(), - None => bail!("missing claim '{}'", name_attr), - } - } else { - bail!("missing claim '{}'", name_attr); - } - } - }; + let name_attr = config.username_claim.as_deref().unwrap_or("sub"); - let user_id = Userid::try_from(format!("{}@{}", unique_name, realm))?; - tested_username = Some(unique_name); + // Try to be compatible with previous versions + let try_attr = match name_attr { + "subject" => Some("sub"), + "username" => Some("preferred_username"), + _ => None, + }; - if !user_info.is_active_user_id(&user_id) { - if config.autocreate.unwrap_or(false) { - use pbs_config::user; - let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; - - let firstname = info["given_name"] - .as_str() - .map(|n| n.to_string()) - .filter(|n| FIRST_NAME_SCHEMA.parse_simple_value(n).is_ok()); - - let lastname = info["family_name"] - .as_str() - .map(|n| n.to_string()) - .filter(|n| LAST_NAME_SCHEMA.parse_simple_value(n).is_ok()); - - let email = info["email"] - .as_str() - .map(|n| n.to_string()) - .filter(|n| EMAIL_SCHEMA.parse_simple_value(n).is_ok()); - - let user = User { - userid: user_id.clone(), - comment: None, - enable: None, - expire: None, - firstname, - lastname, - email, - }; - let (mut config, _digest) = user::config()?; - if let Ok(old_user) = config.lookup::<User>("user", user.userid.as_str()) { - if let Some(false) = old_user.enable { - bail!("user '{}' is disabled.", user.userid); + let unique_name = match info[name_attr].as_str() { + Some(name) => name.to_owned(), + None => { + if let Some(try_attr) = try_attr { + match info[try_attr].as_str() { + Some(name) => name.to_owned(), + None => bail!("missing claim '{}'", name_attr), + } } else { - bail!("autocreate user failed - '{}' already exists.", user.userid); + bail!("missing claim '{}'", name_attr); } } - config.set_data(user.userid.as_str(), "user", &user)?; - user::save_config(&config)?; - } else { - bail!("user account '{}' missing, disabled or expired.", user_id); + }; + + let user_id = Userid::try_from(format!("{}@{}", unique_name, realm))?; + tested_username = Some(unique_name); + + if !user_info.is_active_user_id(&user_id) { + if config.autocreate.unwrap_or(false) { + use pbs_config::user; + let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; + + let firstname = info["given_name"] + .as_str() + .map(|n| n.to_string()) + .filter(|n| FIRST_NAME_SCHEMA.parse_simple_value(n).is_ok()); + + let lastname = info["family_name"] + .as_str() + .map(|n| n.to_string()) + .filter(|n| LAST_NAME_SCHEMA.parse_simple_value(n).is_ok()); + + let email = info["email"] + .as_str() + .map(|n| n.to_string()) + .filter(|n| EMAIL_SCHEMA.parse_simple_value(n).is_ok()); + + let user = User { + userid: user_id.clone(), + comment: None, + enable: None, + expire: None, + firstname, + lastname, + email, + }; + let (mut config, _digest) = user::config()?; + if let Ok(old_user) = config.lookup::<User>("user", user.userid.as_str()) { + if let Some(false) = old_user.enable { + bail!("user '{}' is disabled.", user.userid); + } else { + bail!("autocreate user failed - '{}' already exists.", user.userid); + } + } + config.set_data(user.userid.as_str(), "user", &user)?; + user::save_config(&config)?; + } else { + bail!("user account '{}' missing, disabled or expired.", user_id); + } } - } - let api_ticket = ApiTicket::Full(user_id.clone()); - let ticket = Ticket::new("PBS", &api_ticket)?.sign(private_auth_keyring(), None)?; - let token = assemble_csrf_prevention_token(csrf_secret(), &user_id); + let api_ticket = ApiTicket::Full(user_id.clone()); + let ticket = Ticket::new("PBS", &api_ticket)?; + let token = assemble_csrf_prevention_token(csrf_secret(), &user_id); - env.log_auth(user_id.as_str()); + env.log_auth(user_id.as_str()); - Ok(json!({ - "username": user_id, - "ticket": ticket, - "CSRFPreventionToken": token, - })) - }); + Ok((user_id, ticket, token)) + }); - if let Err(ref err) = result { - let msg = err.to_string(); - env.log_failed_auth(tested_username, &msg); - return Err(http_err!(UNAUTHORIZED, "{}", msg)); - } + let (user_id, mut ticket, token) = result.map_err(|err| { + let msg = err.to_string(); + env.log_failed_auth(tested_username, &msg); + http_err!(UNAUTHORIZED, "{}", msg) + })?; - result + let mut response = + Response::builder().header(hyper::http::header::CONTENT_TYPE, "application/json"); + + let data = if Value::Bool(true) == param["http-only"] { + let cookie = format!( + "{}={}; Secure; SameSite=Lax; HttpOnly; Path=/;", + get_auth_cookie_name(), + ticket.sign(private_auth_keyring(), None)?, + ); + + response = response.header(hyper::header::SET_COOKIE, cookie); + + json!({ + "username": user_id, + "ticket_info": ticket.ticket_info(), + "CSRFPreventionToken": token + }) + } else { + json!({ + "username": user_id, + "ticket": ticket.sign(private_auth_keyring(), None)?, + "CSRFPreventionToken": token + }) + }; + + Ok(response.body( + json!({"data": data, "status": 200, "success": true }) + .to_string() + .into(), + )?) + }) } #[api( -- 2.47.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 1/2] api: openid: allow users of openid to opt into the HttpOnly cookies 2025-08-07 10:49 ` [pbs-devel] [PATCH proxmox-backup 1/2] api: openid: allow users of openid to opt into the HttpOnly cookies Shannon Sterz @ 2025-08-07 11:46 ` Mira Limbeck 2025-08-07 11:51 ` Shannon Sterz 0 siblings, 1 reply; 6+ messages in thread From: Mira Limbeck @ 2025-08-07 11:46 UTC (permalink / raw) To: pbs-devel On 8/7/25 12:49, Shannon Sterz wrote: > add a `http-only` api parameter that allows users to opt into the the > HttpOnly cookie based authentication flow. here the server will set a > cookie via a `Set-Cookie` header instead of providing it in the > response's body. this protects users better against cookie stealing > attacks and other similar attacks. > > note that this has the side effect of always returning extjs-like > responses here. > > Analyzed-by: Mira Limbeck <m.limbeck@proxmox.com> > Signed-off-by: Shannon Sterz <s.sterz@proxmox.com> > --- > src/api2/access/openid.rs | 346 ++++++++++++++++++++++---------------- > 1 file changed, 205 insertions(+), 141 deletions(-) > > diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs > index 8e39cbc9..1beb77bc 100644 > --- a/src/api2/access/openid.rs > +++ b/src/api2/access/openid.rs > @@ -1,13 +1,16 @@ > //! OpenID redirect/login API > use anyhow::{bail, format_err, Error}; > +use hyper::http::request::Parts; > +use hyper::Response; > use serde_json::{json, Value}; > > use proxmox_auth_api::api::ApiTicket; > use proxmox_auth_api::ticket::Ticket; > use proxmox_router::{ > - http_err, list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap, > + http_err, list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission, > + Router, RpcEnvironment, SubdirMap, > }; > -use proxmox_schema::api; > +use proxmox_schema::{api, BooleanSchema, ObjectSchema, ParameterSchema, StringSchema}; > use proxmox_sortable_macro::sortable; > > use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig}; > @@ -58,167 +61,228 @@ fn openid_authenticator( > OpenIdAuthenticator::discover(&config, redirect_url) > } > > -#[api( > - input: { > - properties: { > - state: { > - description: "OpenId state.", > - type: String, > - }, > - code: { > - description: "OpenId authorization code.", > - type: String, > - }, > - "redirect-url": { > - description: "Redirection Url. The client should set this to used server url.", > - type: String, > - }, > - }, > - }, > - returns: { > - properties: { > - username: { > - type: String, > - description: "User name.", > - }, > - ticket: { > - type: String, > - description: "Auth ticket.", > - }, > - CSRFPreventionToken: { > - type: String, > - description: "Cross Site Request Forgery Prevention Token.", > - }, > - }, > - }, > - protected: true, > - access: { > - permission: &Permission::World, > - }, > -)] > -/// Verify OpenID authorization code and create a ticket > -/// > -/// Returns: An authentication ticket with additional infos. > -pub fn openid_login( > - state: String, > - code: String, > - redirect_url: String, > - rpcenv: &mut dyn RpcEnvironment, > -) -> Result<Value, Error> { > - use proxmox_rest_server::RestEnvironment; > +#[sortable] > +pub const API_METHOD_OPENID_LOGIN: ApiMethod = ApiMethod::new_full( > + &ApiHandler::AsyncHttpBodyParameters(&create_ticket_http_only), > + ParameterSchema::Object(&ObjectSchema::new( > + "Get a new ticket as an HttpOnly cookie. Supports tickets via cookies.", > + &sorted!([ > + ("state", false, &StringSchema::new("OpenId state.").schema()), > + ( > + "code", > + false, > + &StringSchema::new("OpenId authorization code.").schema(), > + ), > + ( > + "redirect-url", > + false, > + &StringSchema::new( > + "Redirection Url. The client should set this to used server url.", > + ) > + .schema(), > + ), > + ( > + "http-only", > + true, > + &BooleanSchema::new("Whether the HttpOnly authentication flow should be used.") > + .default(false) > + .schema(), > + ), > + ]), > + )), > +) > +.returns(::proxmox_schema::ReturnType::new( > + false, > + &ObjectSchema::new( > + "An authentication ticket with additional infos.", > + &sorted!([ > + ("username", false, &StringSchema::new("User name.").schema()), > + ( > + "ticket", > + true, > + &StringSchema::new( > + "Auth ticket, present if http-only was not provided or is false." > + ) > + .schema() > + ), > + ("ticket-info", > + true, > + &StringSchema::new( > + "Informational ticket, can only be used to check if the ticket is expired. Present if http-only was true." > + ).schema()), > + ( > + "CSRFPreventionToken", > + false, > + &StringSchema::new("Cross Site Request Forgery Prevention Token.").schema(), > + ), > + ]), > + ) > + .schema(), > +)) > +.protected(true) > +.access(None, &Permission::World) > +.reload_timezone(true); > > - let env: &RestEnvironment = rpcenv > - .as_any() > - .downcast_ref::<RestEnvironment>() > - .ok_or_else(|| format_err!("detected wrong RpcEnvironment type"))?; > +fn create_ticket_http_only( > + _parts: Parts, > + param: Value, > + _info: &ApiMethod, > + rpcenv: Box<dyn RpcEnvironment>, > +) -> ApiResponseFuture { > + Box::pin(async move { > + use proxmox_rest_server::RestEnvironment; > > - let user_info = CachedUserInfo::new()?; > + let code = param["code"] > + .as_str() > + .ok_or_else(|| format_err!("missing non-optional parameter: code"))? > + .to_owned(); > + let state = param["state"] > + .as_str() > + .ok_or_else(|| format_err!("missing non-optional parameter: state"))? > + .to_owned(); > + let redirect_url = param["redirect-url"] > + .as_str() > + .ok_or_else(|| format_err!("missing non-optional parameter: redirect-url"))? > + .to_owned(); > > - let mut tested_username = None; > + let env: &RestEnvironment = rpcenv > + .as_any() > + .downcast_ref::<RestEnvironment>() > + .ok_or_else(|| format_err!("detected wrong RpcEnvironment type"))?; > > - let result = proxmox_lang::try_block!({ > - let (realm, private_auth_state) = > - OpenIdAuthenticator::verify_public_auth_state(PROXMOX_BACKUP_RUN_DIR_M!(), &state)?; > + let user_info = CachedUserInfo::new()?; > > - let (domains, _digest) = pbs_config::domains::config()?; > - let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?; > + let mut tested_username = None; > > - let open_id = openid_authenticator(&config, &redirect_url)?; > + let result = proxmox_lang::try_block!({ > + let (realm, private_auth_state) = > + OpenIdAuthenticator::verify_public_auth_state(PROXMOX_BACKUP_RUN_DIR_M!(), &state)?; > > - let info = open_id.verify_authorization_code_simple(&code, &private_auth_state)?; > + let (domains, _digest) = pbs_config::domains::config()?; > + let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?; > > - // eprintln!("VERIFIED {:?}", info); > + let open_id = openid_authenticator(&config, &redirect_url)?; > > - let name_attr = config.username_claim.as_deref().unwrap_or("sub"); > + let info = open_id.verify_authorization_code_simple(&code, &private_auth_state)?; > > - // Try to be compatible with previous versions > - let try_attr = match name_attr { > - "subject" => Some("sub"), > - "username" => Some("preferred_username"), > - _ => None, > - }; > + // eprintln!("VERIFIED {:?}", info); > > - let unique_name = match info[name_attr].as_str() { > - Some(name) => name.to_owned(), > - None => { > - if let Some(try_attr) = try_attr { > - match info[try_attr].as_str() { > - Some(name) => name.to_owned(), > - None => bail!("missing claim '{}'", name_attr), > - } > - } else { > - bail!("missing claim '{}'", name_attr); > - } > - } > - }; > + let name_attr = config.username_claim.as_deref().unwrap_or("sub"); > > - let user_id = Userid::try_from(format!("{}@{}", unique_name, realm))?; > - tested_username = Some(unique_name); > + // Try to be compatible with previous versions > + let try_attr = match name_attr { > + "subject" => Some("sub"), > + "username" => Some("preferred_username"), > + _ => None, > + }; > > - if !user_info.is_active_user_id(&user_id) { > - if config.autocreate.unwrap_or(false) { > - use pbs_config::user; > - let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; > - > - let firstname = info["given_name"] > - .as_str() > - .map(|n| n.to_string()) > - .filter(|n| FIRST_NAME_SCHEMA.parse_simple_value(n).is_ok()); > - > - let lastname = info["family_name"] > - .as_str() > - .map(|n| n.to_string()) > - .filter(|n| LAST_NAME_SCHEMA.parse_simple_value(n).is_ok()); > - > - let email = info["email"] > - .as_str() > - .map(|n| n.to_string()) > - .filter(|n| EMAIL_SCHEMA.parse_simple_value(n).is_ok()); > - > - let user = User { > - userid: user_id.clone(), > - comment: None, > - enable: None, > - expire: None, > - firstname, > - lastname, > - email, > - }; > - let (mut config, _digest) = user::config()?; > - if let Ok(old_user) = config.lookup::<User>("user", user.userid.as_str()) { > - if let Some(false) = old_user.enable { > - bail!("user '{}' is disabled.", user.userid); > + let unique_name = match info[name_attr].as_str() { > + Some(name) => name.to_owned(), > + None => { > + if let Some(try_attr) = try_attr { > + match info[try_attr].as_str() { > + Some(name) => name.to_owned(), > + None => bail!("missing claim '{}'", name_attr), > + } > } else { > - bail!("autocreate user failed - '{}' already exists.", user.userid); > + bail!("missing claim '{}'", name_attr); > } > } > - config.set_data(user.userid.as_str(), "user", &user)?; > - user::save_config(&config)?; > - } else { > - bail!("user account '{}' missing, disabled or expired.", user_id); > + }; > + > + let user_id = Userid::try_from(format!("{}@{}", unique_name, realm))?; > + tested_username = Some(unique_name); > + > + if !user_info.is_active_user_id(&user_id) { > + if config.autocreate.unwrap_or(false) { > + use pbs_config::user; > + let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; > + > + let firstname = info["given_name"] > + .as_str() > + .map(|n| n.to_string()) > + .filter(|n| FIRST_NAME_SCHEMA.parse_simple_value(n).is_ok()); > + > + let lastname = info["family_name"] > + .as_str() > + .map(|n| n.to_string()) > + .filter(|n| LAST_NAME_SCHEMA.parse_simple_value(n).is_ok()); > + > + let email = info["email"] > + .as_str() > + .map(|n| n.to_string()) > + .filter(|n| EMAIL_SCHEMA.parse_simple_value(n).is_ok()); > + > + let user = User { > + userid: user_id.clone(), > + comment: None, > + enable: None, > + expire: None, > + firstname, > + lastname, > + email, > + }; > + let (mut config, _digest) = user::config()?; > + if let Ok(old_user) = config.lookup::<User>("user", user.userid.as_str()) { > + if let Some(false) = old_user.enable { > + bail!("user '{}' is disabled.", user.userid); > + } else { > + bail!("autocreate user failed - '{}' already exists.", user.userid); > + } > + } > + config.set_data(user.userid.as_str(), "user", &user)?; > + user::save_config(&config)?; > + } else { > + bail!("user account '{}' missing, disabled or expired.", user_id); > + } > } > - } > > - let api_ticket = ApiTicket::Full(user_id.clone()); > - let ticket = Ticket::new("PBS", &api_ticket)?.sign(private_auth_keyring(), None)?; > - let token = assemble_csrf_prevention_token(csrf_secret(), &user_id); > + let api_ticket = ApiTicket::Full(user_id.clone()); > + let ticket = Ticket::new("PBS", &api_ticket)?; > + let token = assemble_csrf_prevention_token(csrf_secret(), &user_id); > > - env.log_auth(user_id.as_str()); > + env.log_auth(user_id.as_str()); > > - Ok(json!({ > - "username": user_id, > - "ticket": ticket, > - "CSRFPreventionToken": token, > - })) > - }); > + Ok((user_id, ticket, token)) > + }); > > - if let Err(ref err) = result { > - let msg = err.to_string(); > - env.log_failed_auth(tested_username, &msg); > - return Err(http_err!(UNAUTHORIZED, "{}", msg)); > - } > + let (user_id, mut ticket, token) = result.map_err(|err| { > + let msg = err.to_string(); > + env.log_failed_auth(tested_username, &msg); > + http_err!(UNAUTHORIZED, "{}", msg) > + })?; > > - result > + let mut response = > + Response::builder().header(hyper::http::header::CONTENT_TYPE, "application/json"); > + > + let data = if Value::Bool(true) == param["http-only"] { > + let cookie = format!( > + "{}={}; Secure; SameSite=Lax; HttpOnly; Path=/;", > + get_auth_cookie_name(), > + ticket.sign(private_auth_keyring(), None)?, > + ); > + > + response = response.header(hyper::header::SET_COOKIE, cookie); > + > + json!({ > + "username": user_id, > + "ticket_info": ticket.ticket_info(), > + "CSRFPreventionToken": token > + }) > + } else { > + json!({ > + "username": user_id, > + "ticket": ticket.sign(private_auth_keyring(), None)?, > + "CSRFPreventionToken": token > + }) > + }; > + > + Ok(response.body( > + json!({"data": data, "status": 200, "success": true }) > + .to_string() > + .into(), > + )?) > + }) > } > > #[api( It looks like this only sets 1 cookie (__Host-PBSAuthCookie) rather than both (PBSAuthCookie and __Host-PBSAuthCookie). This results in the login mask showing after refreshing the browser tab. _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 1/2] api: openid: allow users of openid to opt into the HttpOnly cookies 2025-08-07 11:46 ` Mira Limbeck @ 2025-08-07 11:51 ` Shannon Sterz 0 siblings, 0 replies; 6+ messages in thread From: Shannon Sterz @ 2025-08-07 11:51 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Mira Limbeck On Thu Aug 7, 2025 at 1:46 PM CEST, Mira Limbeck wrote: > On 8/7/25 12:49, Shannon Sterz wrote: >> add a `http-only` api parameter that allows users to opt into the the >> HttpOnly cookie based authentication flow. here the server will set a >> cookie via a `Set-Cookie` header instead of providing it in the >> response's body. this protects users better against cookie stealing >> attacks and other similar attacks. >> >> note that this has the side effect of always returning extjs-like >> responses here. >> >> Analyzed-by: Mira Limbeck <m.limbeck@proxmox.com> >> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com> >> --- >> src/api2/access/openid.rs | 346 ++++++++++++++++++++++---------------- >> 1 file changed, 205 insertions(+), 141 deletions(-) >> >> diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs >> index 8e39cbc9..1beb77bc 100644 >> --- a/src/api2/access/openid.rs >> +++ b/src/api2/access/openid.rs >> @@ -1,13 +1,16 @@ >> //! OpenID redirect/login API >> use anyhow::{bail, format_err, Error}; >> +use hyper::http::request::Parts; >> +use hyper::Response; >> use serde_json::{json, Value}; >> >> use proxmox_auth_api::api::ApiTicket; >> use proxmox_auth_api::ticket::Ticket; >> use proxmox_router::{ >> - http_err, list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap, >> + http_err, list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission, >> + Router, RpcEnvironment, SubdirMap, >> }; >> -use proxmox_schema::api; >> +use proxmox_schema::{api, BooleanSchema, ObjectSchema, ParameterSchema, StringSchema}; >> use proxmox_sortable_macro::sortable; >> >> use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig}; >> @@ -58,167 +61,228 @@ fn openid_authenticator( >> OpenIdAuthenticator::discover(&config, redirect_url) >> } >> >> -#[api( >> - input: { >> - properties: { >> - state: { >> - description: "OpenId state.", >> - type: String, >> - }, >> - code: { >> - description: "OpenId authorization code.", >> - type: String, >> - }, >> - "redirect-url": { >> - description: "Redirection Url. The client should set this to used server url.", >> - type: String, >> - }, >> - }, >> - }, >> - returns: { >> - properties: { >> - username: { >> - type: String, >> - description: "User name.", >> - }, >> - ticket: { >> - type: String, >> - description: "Auth ticket.", >> - }, >> - CSRFPreventionToken: { >> - type: String, >> - description: "Cross Site Request Forgery Prevention Token.", >> - }, >> - }, >> - }, >> - protected: true, >> - access: { >> - permission: &Permission::World, >> - }, >> -)] >> -/// Verify OpenID authorization code and create a ticket >> -/// >> -/// Returns: An authentication ticket with additional infos. >> -pub fn openid_login( >> - state: String, >> - code: String, >> - redirect_url: String, >> - rpcenv: &mut dyn RpcEnvironment, >> -) -> Result<Value, Error> { >> - use proxmox_rest_server::RestEnvironment; >> +#[sortable] >> +pub const API_METHOD_OPENID_LOGIN: ApiMethod = ApiMethod::new_full( >> + &ApiHandler::AsyncHttpBodyParameters(&create_ticket_http_only), >> + ParameterSchema::Object(&ObjectSchema::new( >> + "Get a new ticket as an HttpOnly cookie. Supports tickets via cookies.", >> + &sorted!([ >> + ("state", false, &StringSchema::new("OpenId state.").schema()), >> + ( >> + "code", >> + false, >> + &StringSchema::new("OpenId authorization code.").schema(), >> + ), >> + ( >> + "redirect-url", >> + false, >> + &StringSchema::new( >> + "Redirection Url. The client should set this to used server url.", >> + ) >> + .schema(), >> + ), >> + ( >> + "http-only", >> + true, >> + &BooleanSchema::new("Whether the HttpOnly authentication flow should be used.") >> + .default(false) >> + .schema(), >> + ), >> + ]), >> + )), >> +) >> +.returns(::proxmox_schema::ReturnType::new( >> + false, >> + &ObjectSchema::new( >> + "An authentication ticket with additional infos.", >> + &sorted!([ >> + ("username", false, &StringSchema::new("User name.").schema()), >> + ( >> + "ticket", >> + true, >> + &StringSchema::new( >> + "Auth ticket, present if http-only was not provided or is false." >> + ) >> + .schema() >> + ), >> + ("ticket-info", >> + true, >> + &StringSchema::new( >> + "Informational ticket, can only be used to check if the ticket is expired. Present if http-only was true." >> + ).schema()), >> + ( >> + "CSRFPreventionToken", >> + false, >> + &StringSchema::new("Cross Site Request Forgery Prevention Token.").schema(), >> + ), >> + ]), >> + ) >> + .schema(), >> +)) >> +.protected(true) >> +.access(None, &Permission::World) >> +.reload_timezone(true); >> >> - let env: &RestEnvironment = rpcenv >> - .as_any() >> - .downcast_ref::<RestEnvironment>() >> - .ok_or_else(|| format_err!("detected wrong RpcEnvironment type"))?; >> +fn create_ticket_http_only( >> + _parts: Parts, >> + param: Value, >> + _info: &ApiMethod, >> + rpcenv: Box<dyn RpcEnvironment>, >> +) -> ApiResponseFuture { >> + Box::pin(async move { >> + use proxmox_rest_server::RestEnvironment; >> >> - let user_info = CachedUserInfo::new()?; >> + let code = param["code"] >> + .as_str() >> + .ok_or_else(|| format_err!("missing non-optional parameter: code"))? >> + .to_owned(); >> + let state = param["state"] >> + .as_str() >> + .ok_or_else(|| format_err!("missing non-optional parameter: state"))? >> + .to_owned(); >> + let redirect_url = param["redirect-url"] >> + .as_str() >> + .ok_or_else(|| format_err!("missing non-optional parameter: redirect-url"))? >> + .to_owned(); >> >> - let mut tested_username = None; >> + let env: &RestEnvironment = rpcenv >> + .as_any() >> + .downcast_ref::<RestEnvironment>() >> + .ok_or_else(|| format_err!("detected wrong RpcEnvironment type"))?; >> >> - let result = proxmox_lang::try_block!({ >> - let (realm, private_auth_state) = >> - OpenIdAuthenticator::verify_public_auth_state(PROXMOX_BACKUP_RUN_DIR_M!(), &state)?; >> + let user_info = CachedUserInfo::new()?; >> >> - let (domains, _digest) = pbs_config::domains::config()?; >> - let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?; >> + let mut tested_username = None; >> >> - let open_id = openid_authenticator(&config, &redirect_url)?; >> + let result = proxmox_lang::try_block!({ >> + let (realm, private_auth_state) = >> + OpenIdAuthenticator::verify_public_auth_state(PROXMOX_BACKUP_RUN_DIR_M!(), &state)?; >> >> - let info = open_id.verify_authorization_code_simple(&code, &private_auth_state)?; >> + let (domains, _digest) = pbs_config::domains::config()?; >> + let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?; >> >> - // eprintln!("VERIFIED {:?}", info); >> + let open_id = openid_authenticator(&config, &redirect_url)?; >> >> - let name_attr = config.username_claim.as_deref().unwrap_or("sub"); >> + let info = open_id.verify_authorization_code_simple(&code, &private_auth_state)?; >> >> - // Try to be compatible with previous versions >> - let try_attr = match name_attr { >> - "subject" => Some("sub"), >> - "username" => Some("preferred_username"), >> - _ => None, >> - }; >> + // eprintln!("VERIFIED {:?}", info); >> >> - let unique_name = match info[name_attr].as_str() { >> - Some(name) => name.to_owned(), >> - None => { >> - if let Some(try_attr) = try_attr { >> - match info[try_attr].as_str() { >> - Some(name) => name.to_owned(), >> - None => bail!("missing claim '{}'", name_attr), >> - } >> - } else { >> - bail!("missing claim '{}'", name_attr); >> - } >> - } >> - }; >> + let name_attr = config.username_claim.as_deref().unwrap_or("sub"); >> >> - let user_id = Userid::try_from(format!("{}@{}", unique_name, realm))?; >> - tested_username = Some(unique_name); >> + // Try to be compatible with previous versions >> + let try_attr = match name_attr { >> + "subject" => Some("sub"), >> + "username" => Some("preferred_username"), >> + _ => None, >> + }; >> >> - if !user_info.is_active_user_id(&user_id) { >> - if config.autocreate.unwrap_or(false) { >> - use pbs_config::user; >> - let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; >> - >> - let firstname = info["given_name"] >> - .as_str() >> - .map(|n| n.to_string()) >> - .filter(|n| FIRST_NAME_SCHEMA.parse_simple_value(n).is_ok()); >> - >> - let lastname = info["family_name"] >> - .as_str() >> - .map(|n| n.to_string()) >> - .filter(|n| LAST_NAME_SCHEMA.parse_simple_value(n).is_ok()); >> - >> - let email = info["email"] >> - .as_str() >> - .map(|n| n.to_string()) >> - .filter(|n| EMAIL_SCHEMA.parse_simple_value(n).is_ok()); >> - >> - let user = User { >> - userid: user_id.clone(), >> - comment: None, >> - enable: None, >> - expire: None, >> - firstname, >> - lastname, >> - email, >> - }; >> - let (mut config, _digest) = user::config()?; >> - if let Ok(old_user) = config.lookup::<User>("user", user.userid.as_str()) { >> - if let Some(false) = old_user.enable { >> - bail!("user '{}' is disabled.", user.userid); >> + let unique_name = match info[name_attr].as_str() { >> + Some(name) => name.to_owned(), >> + None => { >> + if let Some(try_attr) = try_attr { >> + match info[try_attr].as_str() { >> + Some(name) => name.to_owned(), >> + None => bail!("missing claim '{}'", name_attr), >> + } >> } else { >> - bail!("autocreate user failed - '{}' already exists.", user.userid); >> + bail!("missing claim '{}'", name_attr); >> } >> } >> - config.set_data(user.userid.as_str(), "user", &user)?; >> - user::save_config(&config)?; >> - } else { >> - bail!("user account '{}' missing, disabled or expired.", user_id); >> + }; >> + >> + let user_id = Userid::try_from(format!("{}@{}", unique_name, realm))?; >> + tested_username = Some(unique_name); >> + >> + if !user_info.is_active_user_id(&user_id) { >> + if config.autocreate.unwrap_or(false) { >> + use pbs_config::user; >> + let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; >> + >> + let firstname = info["given_name"] >> + .as_str() >> + .map(|n| n.to_string()) >> + .filter(|n| FIRST_NAME_SCHEMA.parse_simple_value(n).is_ok()); >> + >> + let lastname = info["family_name"] >> + .as_str() >> + .map(|n| n.to_string()) >> + .filter(|n| LAST_NAME_SCHEMA.parse_simple_value(n).is_ok()); >> + >> + let email = info["email"] >> + .as_str() >> + .map(|n| n.to_string()) >> + .filter(|n| EMAIL_SCHEMA.parse_simple_value(n).is_ok()); >> + >> + let user = User { >> + userid: user_id.clone(), >> + comment: None, >> + enable: None, >> + expire: None, >> + firstname, >> + lastname, >> + email, >> + }; >> + let (mut config, _digest) = user::config()?; >> + if let Ok(old_user) = config.lookup::<User>("user", user.userid.as_str()) { >> + if let Some(false) = old_user.enable { >> + bail!("user '{}' is disabled.", user.userid); >> + } else { >> + bail!("autocreate user failed - '{}' already exists.", user.userid); >> + } >> + } >> + config.set_data(user.userid.as_str(), "user", &user)?; >> + user::save_config(&config)?; >> + } else { >> + bail!("user account '{}' missing, disabled or expired.", user_id); >> + } >> } >> - } >> >> - let api_ticket = ApiTicket::Full(user_id.clone()); >> - let ticket = Ticket::new("PBS", &api_ticket)?.sign(private_auth_keyring(), None)?; >> - let token = assemble_csrf_prevention_token(csrf_secret(), &user_id); >> + let api_ticket = ApiTicket::Full(user_id.clone()); >> + let ticket = Ticket::new("PBS", &api_ticket)?; >> + let token = assemble_csrf_prevention_token(csrf_secret(), &user_id); >> >> - env.log_auth(user_id.as_str()); >> + env.log_auth(user_id.as_str()); >> >> - Ok(json!({ >> - "username": user_id, >> - "ticket": ticket, >> - "CSRFPreventionToken": token, >> - })) >> - }); >> + Ok((user_id, ticket, token)) >> + }); >> >> - if let Err(ref err) = result { >> - let msg = err.to_string(); >> - env.log_failed_auth(tested_username, &msg); >> - return Err(http_err!(UNAUTHORIZED, "{}", msg)); >> - } >> + let (user_id, mut ticket, token) = result.map_err(|err| { >> + let msg = err.to_string(); >> + env.log_failed_auth(tested_username, &msg); >> + http_err!(UNAUTHORIZED, "{}", msg) >> + })?; >> >> - result >> + let mut response = >> + Response::builder().header(hyper::http::header::CONTENT_TYPE, "application/json"); >> + >> + let data = if Value::Bool(true) == param["http-only"] { >> + let cookie = format!( >> + "{}={}; Secure; SameSite=Lax; HttpOnly; Path=/;", >> + get_auth_cookie_name(), >> + ticket.sign(private_auth_keyring(), None)?, >> + ); >> + >> + response = response.header(hyper::header::SET_COOKIE, cookie); >> + >> + json!({ >> + "username": user_id, >> + "ticket_info": ticket.ticket_info(), >> + "CSRFPreventionToken": token >> + }) >> + } else { >> + json!({ >> + "username": user_id, >> + "ticket": ticket.sign(private_auth_keyring(), None)?, >> + "CSRFPreventionToken": token >> + }) >> + }; >> + >> + Ok(response.body( >> + json!({"data": data, "status": 200, "success": true }) >> + .to_string() >> + .into(), >> + )?) >> + }) >> } >> >> #[api( > > It looks like this only sets 1 cookie (__Host-PBSAuthCookie) rather than > both (PBSAuthCookie and __Host-PBSAuthCookie). This results in the login > mask showing after refreshing the browser tab. yep thanks for noticing that, the fix for that is rather trivial: `ticket_info` in the response json of the endpoint, should have been `ticket-info`. i'll send a v2 in a minute. > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] ui: opt open id authentication flows into the new http only flow 2025-08-07 10:49 [pbs-devel] [PATCH proxmox-backup 0/2] add support for HttpOnly cookies for OpenID authentication flow Shannon Sterz 2025-08-07 10:49 ` [pbs-devel] [PATCH proxmox-backup 1/2] api: openid: allow users of openid to opt into the HttpOnly cookies Shannon Sterz @ 2025-08-07 10:49 ` Shannon Sterz 2025-08-07 12:05 ` [pbs-devel] Superseded: [PATCH proxmox-backup 0/2] add support for HttpOnly cookies for OpenID authentication flow Shannon Sterz 2 siblings, 0 replies; 6+ messages in thread From: Shannon Sterz @ 2025-08-07 10:49 UTC (permalink / raw) To: pbs-devel otherwise the cookie would not be properly set, leading to users potentially getting logged out instantly again. Analyzed-by: Mira Limbeck <m.limbeck@proxmox.com> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com> --- www/LoginView.js | 1 + 1 file changed, 1 insertion(+) diff --git a/www/LoginView.js b/www/LoginView.js index cf6c2cf7..08786080 100644 --- a/www/LoginView.js +++ b/www/LoginView.js @@ -197,6 +197,7 @@ Ext.define('PBS.LoginView', { state: auth.state, code: auth.code, 'redirect-url': redirectURL, + 'http-only': true, }, method: 'POST', failure: function (response) { -- 2.47.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] Superseded: [PATCH proxmox-backup 0/2] add support for HttpOnly cookies for OpenID authentication flow 2025-08-07 10:49 [pbs-devel] [PATCH proxmox-backup 0/2] add support for HttpOnly cookies for OpenID authentication flow Shannon Sterz 2025-08-07 10:49 ` [pbs-devel] [PATCH proxmox-backup 1/2] api: openid: allow users of openid to opt into the HttpOnly cookies Shannon Sterz 2025-08-07 10:49 ` [pbs-devel] [PATCH proxmox-backup 2/2] ui: opt open id authentication flows into the new http only flow Shannon Sterz @ 2025-08-07 12:05 ` Shannon Sterz 2 siblings, 0 replies; 6+ messages in thread From: Shannon Sterz @ 2025-08-07 12:05 UTC (permalink / raw) To: Shannon Sterz, pbs-devel Superseded-by: https://lore.proxmox.com/all/20250807120308.257071-2-s.sterz@proxmox.com/T/#t On Thu Aug 7, 2025 at 12:49 PM CEST, Shannon Sterz wrote: > these two patches do the following: > > 1. add a `http-only` parameter to the OpenID login endpoint, so clients > can opt into receive the authentication ticket via a HttpOnly cookie > 2. opt the ui dialog into using this new HttpOnly parameter > > this should fix a bug where users were instantly logged out again after > a successful OpenID authentication. > > Shannon Sterz (2): > api: openid: allow users of openid to opt into the HttpOnly cookies > ui: opt open id authentication flows into the new http only flow > > src/api2/access/openid.rs | 346 ++++++++++++++++++++++---------------- > www/LoginView.js | 1 + > 2 files changed, 206 insertions(+), 141 deletions(-) > > -- > 2.47.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-07 12:03 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-08-07 10:49 [pbs-devel] [PATCH proxmox-backup 0/2] add support for HttpOnly cookies for OpenID authentication flow Shannon Sterz 2025-08-07 10:49 ` [pbs-devel] [PATCH proxmox-backup 1/2] api: openid: allow users of openid to opt into the HttpOnly cookies Shannon Sterz 2025-08-07 11:46 ` Mira Limbeck 2025-08-07 11:51 ` Shannon Sterz 2025-08-07 10:49 ` [pbs-devel] [PATCH proxmox-backup 2/2] ui: opt open id authentication flows into the new http only flow Shannon Sterz 2025-08-07 12:05 ` [pbs-devel] Superseded: [PATCH proxmox-backup 0/2] add support for HttpOnly cookies for OpenID authentication flow Shannon Sterz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox