[yew-devel] [PATCH yew-comp] http wasm client: load csrf token from global Proxmox object

[Thomas Lamprecht]

Am 10.12.25 um 14:50 schrieb Shannon Sterz:
> previously csrf tokens were only loaded from session storage. this
> could lead to two scenarios:
> 
> - the csrf token expired while the newer authentication ticket was
> still valid.
> - when restoring a session, session cookies were restored, but session
> storage seemingly isn't always restored (tested in chromium and ff).
> this lead to no csrf token being loaded here, the
> `unwrap_or_default()` would then return the empty string as a csrf
> token.
> 
> both scenarios would lead to a situation where a valid authentication
> cookie was present, but the csrf token was empty or expired. the
> result is that all GET requests would work properly, as we don't check
> csrf tokens there. however, the first non-GET request would lead to a
> logout.
> 
> to fix this, we first load the token from the Proxmox object that is
> injected via a script in the index.html for all proxmox products. we
> prefer this token over the one in session storage.
> authentication_from_cookie (that calls the extract_auth_from_cookie
> function), is and should only be called when the page was just loaded.
> so the csrf token in the Proxmox object will always be fresher than the
> one in the session storage.
> 
> additionally, if no csrf token can be found, return None to trigger a
> fresh log in.

Many thanks for this patch, the basic approach checks out AFAICT, some
comment w.r.t. (pre-existing) code structure inline though.


> Signed-off-by: Shannon Sterz <s.sterz at proxmox.com>
> ---
> 
> tested this by opening and closing chromium and firefox after logging in
> with active session restore. without this patch an empty csrf token
> would be loaded (and then persisted to session storage).
> 
> note, i intentionally did not put this in load_csrf_token() in lib.rs as
> that could break the assumption that it will always return the same
> token last stored with store_csrf_token().
> 
>  src/http_client_wasm.rs | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/http_client_wasm.rs b/src/http_client_wasm.rs
> index 25d0dd2..ae99a99 100644
> --- a/src/http_client_wasm.rs
> +++ b/src/http_client_wasm.rs
> @@ -3,9 +3,11 @@ use std::pin::Pin;
>  use std::rc::Rc;
>  use std::sync::Mutex;
> 
> +use js_sys::Reflect;
>  use percent_encoding::percent_decode_str;
>  use serde::Serialize;
>  use serde_json::Value;
> +use wasm_bindgen::JsValue;
> 
>  use proxmox_client::{Error, HttpApiClient, HttpApiResponse, HttpApiResponseStream};
>  use proxmox_login::{Authentication, Login, Ticket, TicketResult};
> @@ -55,8 +57,16 @@ fn extract_auth_from_cookie(project: &dyn ProjectInfo) -> Option<(String, String
>              if key == name {
>                  let items: Vec<&str> = value.split(':').take(2).collect();
>                  if prefixes.contains(&items[0]) {
> -                    let csrf_token = crate::load_csrf_token().unwrap_or_default();
> -                    return Some((value.to_string(), csrf_token));

Might be nice to have a comment here, like:

    // prefer the token from the index template's JS object, which is the most     
    // recent one, fallback to the one from the session storage (possibly outdated)

That said, I wonder if the function should be split into one to get the ticket and one to
get the CSRF token which then could be called separately in authentication_from_cookie
as this already feels like two different functions to me, it was just slightly hidden
by the csrf extraction being just a short call to another fn previously.

This doesn't mean you have to put it in the lib's load_csrf_token though, can be a
helper local to this module with a short doc-comment describing the background.

> +                    let window = gloo_utils::window();
> +                    if let Ok(proxmox) = Reflect::get(&window, &JsValue::from_str("Proxmox")) {
> +                        if let Ok(token) =
> +                            Reflect::get(&proxmox, &JsValue::from_str("CSRFPreventionToken"))
> +                        {
> +                            return token.as_string().map(|t| (value.to_string(), t));
> +                        }
> +                    }
> +
> +                    return crate::load_csrf_token().map(|t| (value.to_string(), t));
>                  }
>              }
>          }
> --
> 2.47.3