Re: [pbs-devel] [PATCH proxmox 1/3] rest-server: remove auth cookies via http header on unauthorized request

[Dominik Csapak <d.csapak@proxmox.com>]

Looks good to me.

Tested by invalidating my cookie and sending any http request that 
returns a 401 subsequently.
That successfully deleted my http-only cookie.


Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>

On 7/25/25 13:24, Shannon Sterz wrote:
> previously the behaviour of our javascript clients was to remove
> authentication cookies if the api returned a 401 UNAUTHORIZED
> response. with the switch to httponly cookies, this is no longer
> possible. add an option to the ApiConfig to allow the rest-server to
> remove such cookies
> 
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>   proxmox-rest-server/src/api_config.rs |  9 +++++++++
>   proxmox-rest-server/src/rest.rs       | 25 ++++++++++++++++++++++++-
>   2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/proxmox-rest-server/src/api_config.rs b/proxmox-rest-server/src/api_config.rs
> index 0b847a0c..0a67231e 100644
> --- a/proxmox-rest-server/src/api_config.rs
> +++ b/proxmox-rest-server/src/api_config.rs
> @@ -33,6 +33,9 @@ pub struct ApiConfig {
>       auth_handler: Option<AuthHandler>,
>       index_handler: Option<IndexHandler>,
>       pub(crate) privileged_addr: Option<PrivilegedAddr>,
> +    // Name of the auth cookie that should be unset on 401 request. If `None` no cookie will be
> +    // removed.
> +    pub(crate) auth_cookie_name: Option<String>,
>   
>       #[cfg(feature = "templates")]
>       templates: templates::Templates,
> @@ -62,6 +65,7 @@ impl ApiConfig {
>               auth_handler: None,
>               index_handler: None,
>               privileged_addr: None,
> +            auth_cookie_name: None,
>   
>               #[cfg(feature = "templates")]
>               templates: templates::Templates::with_escape_fn(),
> @@ -82,6 +86,11 @@ impl ApiConfig {
>           self.auth_handler(AuthHandler::from_fn(func))
>       }
>   
> +    pub fn auth_cookie_name(mut self, auth_cookie_name: String) -> Self {
> +        self.auth_cookie_name = Some(auth_cookie_name);
> +        self
> +    }
> +
>       /// This is used for `protected` API calls to proxy to a more privileged service.
>       pub fn privileged_addr(mut self, addr: impl Into<PrivilegedAddr>) -> Self {
>           self.privileged_addr = Some(addr.into());
> diff --git a/proxmox-rest-server/src/rest.rs b/proxmox-rest-server/src/rest.rs
> index bff90882..035a9537 100644
> --- a/proxmox-rest-server/src/rest.rs
> +++ b/proxmox-rest-server/src/rest.rs
> @@ -357,8 +357,21 @@ impl Service<Request<Incoming>> for ApiService {
>               Some(proxied_peer) => proxied_peer,
>               None => self.peer,
>           };
> +
> +        let header = self.api_config
> +            .auth_cookie_name
> +            .as_ref()
> +            .map(|name|{
> +                let host_cookie = format!("{name}=; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Secure; SameSite=Lax; HttpOnly; Path=/;");
> +
> +                // SAFETY: this can only fail if the cookie name is not valid in http headers.
> +                // since this is about an authentication cookie, this should never happen.
> +                hyper::header::HeaderValue::from_str(&host_cookie)
> +                    .expect("auth cookie name has characters that are not valid for http headers")
> +             });
> +
>           async move {
> -            let response = match Arc::clone(&config).handle_request(req, &peer).await {
> +            let mut response = match Arc::clone(&config).handle_request(req, &peer).await {
>                   Ok(response) => response,
>                   Err(err) => {
>                       let (err, code) = match err.downcast_ref::<HttpError>() {
> @@ -371,6 +384,16 @@ impl Service<Request<Incoming>> for ApiService {
>                           .body(err.into())?
>                   }
>               };
> +
> +            if let Some(cookie_header) = header {
> +                // remove auth cookies that javascript based clients can not unset
> +                if response.status() == StatusCode::UNAUTHORIZED {
> +                    response
> +                        .headers_mut()
> +                        .insert(hyper::header::SET_COOKIE, cookie_header);
> +                }
> +            }
> +
>               let logger = config.get_access_log();
>               log_response(logger, &peer, method, &path, &response, user_agent);
>               Ok(response)



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel