From 31467396ae565fefa355136385ecaddc6e4a6a48 Mon Sep 17 00:00:00 2001 From: Lukas Potthast Date: Fri, 7 Feb 2025 14:22:12 +0100 Subject: [PATCH] Gracefully handle type changes by capturing storage decode errors; Log all unhandled storage errors --- .../src/internal/code_verifier_manager.rs | 27 ++--- .../src/internal/jwk_set_manager.rs | 39 ++++---- .../src/internal/oidc_config_manager.rs | 25 +++-- .../src/internal/token_manager.rs | 22 +++-- leptos-keycloak-auth/src/lib.rs | 1 + leptos-keycloak-auth/src/storage.rs | 99 +++++++++++++++++++ 6 files changed, 165 insertions(+), 48 deletions(-) create mode 100644 leptos-keycloak-auth/src/storage.rs diff --git a/leptos-keycloak-auth/src/internal/code_verifier_manager.rs b/leptos-keycloak-auth/src/internal/code_verifier_manager.rs index 74dc035..6020f5d 100644 --- a/leptos-keycloak-auth/src/internal/code_verifier_manager.rs +++ b/leptos-keycloak-auth/src/internal/code_verifier_manager.rs @@ -1,8 +1,9 @@ use crate::code_verifier; use crate::code_verifier::{CodeChallenge, CodeVerifier}; +use crate::storage::{use_storage_with_options_and_error_handler, UseStorageReturn}; use codee::string::JsonSerdeCodec; use leptos::prelude::*; -use leptos_use::storage::{use_storage_with_options, StorageType, UseStorageOptions}; +use leptos_use::storage::StorageType; #[derive(Debug, Clone, Copy)] pub struct CodeVerifierManager { @@ -21,17 +22,19 @@ impl CodeVerifierManager { // our app and need the same code_verifier later to do the token exchange, giving us no other // way than storing it. // TODO: Can we provide an "iframe" mode in which the login page is shown in an iframe while our Leptos application stays running in the background? - let (code_verifier, set_code_verifier, _remove_code_verifier_from_storage) = - use_storage_with_options::>, JsonSerdeCodec>( - // Forcing session storage, because this data point must be as secure as possible, - // and we do not care that we may lose the code from a page-refresh or tab-close. - StorageType::Session, - "leptos_keycloak_auth__code_verifier", - UseStorageOptions::default() - .initial_value(None) - .delay_during_hydration(false) - .on_error(|err| tracing::error!(?err, "code_verifier storage error")), - ); + let UseStorageReturn { + read: code_verifier, + write: set_code_verifier, + remove: _remove_code_verifier_from_storage, + .. + } = use_storage_with_options_and_error_handler::>, JsonSerdeCodec>( + // Forcing session storage, because this data point must be as secure as possible, + // and we do not care that we may lose the code from a page-refresh or tab-close. + StorageType::Session, + "leptos_keycloak_auth__code_verifier", + None, + ); + if code_verifier.read_untracked().is_none() { tracing::trace!("No code_verifier found in session storage, generating new one..."); set_code_verifier.set(Some(CodeVerifier::<128>::generate())); diff --git a/leptos-keycloak-auth/src/internal/jwk_set_manager.rs b/leptos-keycloak-auth/src/internal/jwk_set_manager.rs index 3daad03..ed8d3ec 100644 --- a/leptos-keycloak-auth/src/internal/jwk_set_manager.rs +++ b/leptos-keycloak-auth/src/internal/jwk_set_manager.rs @@ -2,11 +2,12 @@ use crate::config::Options; use crate::internal::derived_urls::DerivedUrlError; use crate::internal::JwkSetWithTimestamp; use crate::request::RequestError; +use crate::storage::{use_storage_with_options_and_error_handler, UseStorageReturn}; use crate::time_ext::TimeDurationExt; use crate::{action, JwkSetEndpoint}; use codee::string::JsonSerdeCodec; use leptos::prelude::*; -use leptos_use::storage::{use_storage_with_options, StorageType, UseStorageOptions}; +use leptos_use::storage::StorageType; use leptos_use::{use_interval, UseIntervalReturn}; use std::time::Duration as StdDuration; use time::OffsetDateTime; @@ -31,14 +32,16 @@ impl JwkSetManager { jwk_set_endpoint: Signal>, handle_req_error: Callback>, ) -> Self { - let (jwk_set_old, set_jwk_set_old, _remove_jwk_set_old_from_storage) = - use_storage_with_options::, JsonSerdeCodec>( - StorageType::Local, - "leptos_keycloak_auth__jwk_set_old", - UseStorageOptions::default() - .initial_value(None) - .delay_during_hydration(false), - ); + let UseStorageReturn { + read: jwk_set_old, + write: set_jwk_set_old, + remove: _remove_jwk_set_old_from_storage, + .. + } = use_storage_with_options_and_error_handler::, JsonSerdeCodec>( + StorageType::Local, + "leptos_keycloak_auth__jwk_set_old", + None, + ); // Immediately forget the previously cached value when the discovery endpoint changed! if let Some(source) = jwk_set_old.get_untracked().map(|it| it.source) { @@ -50,14 +53,16 @@ impl JwkSetManager { } } - let (jwk_set, set_jwk_set, _remove_jwk_set_from_storage) = - use_storage_with_options::, JsonSerdeCodec>( - StorageType::Local, - "leptos_keycloak_auth__jwk_set", - UseStorageOptions::default() - .initial_value(None) - .delay_during_hydration(false), - ); + let UseStorageReturn { + read: jwk_set, + write: set_jwk_set, + remove: _remove_jwk_set_from_storage, + .. + } = use_storage_with_options_and_error_handler::, JsonSerdeCodec>( + StorageType::Local, + "leptos_keycloak_auth__jwk_set", + None, + ); // Immediately forget the previously cached value when the discovery endpoint changed! if let Some(source) = jwk_set.get_untracked().map(|it| it.source) { diff --git a/leptos-keycloak-auth/src/internal/oidc_config_manager.rs b/leptos-keycloak-auth/src/internal/oidc_config_manager.rs index b08ea8d..ebd06da 100644 --- a/leptos-keycloak-auth/src/internal/oidc_config_manager.rs +++ b/leptos-keycloak-auth/src/internal/oidc_config_manager.rs @@ -2,11 +2,13 @@ use crate::config::Options; use crate::internal::derived_urls::DerivedUrls; use crate::internal::OidcConfigWithTimestamp; use crate::request::RequestError; +use crate::storage::{use_storage_with_options_and_error_handler, UseStorageReturn}; use crate::time_ext::TimeDurationExt; use codee::string::JsonSerdeCodec; use leptos::prelude::*; -use leptos_use::storage::{use_storage_with_options, StorageType, UseStorageOptions}; +use leptos_use::storage::StorageType; use leptos_use::{use_interval, UseIntervalReturn}; +use std::fmt::Debug; use std::time::Duration as StdDuration; use time::OffsetDateTime; @@ -27,14 +29,19 @@ impl OidcConfigManager { options: StoredValue, handle_req_error: Callback>, ) -> Self { - let (oidc_config, set_oidc_config, _remove_oidc_config_from_storage) = - use_storage_with_options::, JsonSerdeCodec>( - StorageType::Local, - "leptos_keycloak_auth__oidc_config", - UseStorageOptions::default() - .initial_value(None) - .delay_during_hydration(false), - ); + let UseStorageReturn { + read: oidc_config, + write: set_oidc_config, + remove: _remove_oidc_config_from_storage, + .. + } = use_storage_with_options_and_error_handler::< + Option, + JsonSerdeCodec, + >( + StorageType::Local, + "leptos_keycloak_auth__oidc_config", + None, + ); // Immediately forget the previously cached value when the discovery endpoint changed! if let Some(source) = oidc_config.get_untracked().map(|it| it.source) { diff --git a/leptos-keycloak-auth/src/internal/token_manager.rs b/leptos-keycloak-auth/src/internal/token_manager.rs index 68e6fdd..f0f0740 100644 --- a/leptos-keycloak-auth/src/internal/token_manager.rs +++ b/leptos-keycloak-auth/src/internal/token_manager.rs @@ -3,12 +3,13 @@ use crate::code_verifier::CodeVerifier; use crate::config::Options; use crate::internal::derived_urls::DerivedUrlError; use crate::request::RequestError; +use crate::storage::{use_storage_with_options_and_error_handler, UseStorageReturn}; use crate::time_ext::TimeDurationExt; use crate::token::TokenData; use crate::{action, AuthorizationCode, SessionState, TokenEndpoint}; use codee::string::JsonSerdeCodec; use leptos::prelude::*; -use leptos_use::storage::{use_storage_with_options, StorageType, UseStorageOptions}; +use leptos_use::storage::StorageType; use leptos_use::{use_interval, UseIntervalReturn}; use std::fmt::{Debug, Formatter}; use std::time::Duration as StdDuration; @@ -69,15 +70,16 @@ impl TokenManager { handle_req_error: Callback>, token_endpoint: Signal>, ) -> Self { - let (token, set_token, _remove_token_from_storage) = - use_storage_with_options::, JsonSerdeCodec>( - StorageType::Local, - "leptos_keycloak_auth__token", - UseStorageOptions::default() - .initial_value(None) - .delay_during_hydration(false) - .on_error(|err| tracing::error!(?err, "token storage error")), - ); + let UseStorageReturn { + read: token, + write: set_token, + remove: _remove_token_from_storage, + .. + } = use_storage_with_options_and_error_handler::, JsonSerdeCodec>( + StorageType::Local, + "leptos_keycloak_auth__token", + None, + ); // Immediately forget the previously cached value when the discovery endpoint changed! if let Some(source) = token.get_untracked().map(|it| it.source) { diff --git a/leptos-keycloak-auth/src/lib.rs b/leptos-keycloak-auth/src/lib.rs index e0c5968..a43533b 100644 --- a/leptos-keycloak-auth/src/lib.rs +++ b/leptos-keycloak-auth/src/lib.rs @@ -124,6 +124,7 @@ mod time_ext; mod token; mod token_claims; mod token_validation; +mod storage; // Library exports (additional to pub modules). pub use authenticated_client::*; diff --git a/leptos-keycloak-auth/src/storage.rs b/leptos-keycloak-auth/src/storage.rs new file mode 100644 index 0000000..2e0a294 --- /dev/null +++ b/leptos-keycloak-auth/src/storage.rs @@ -0,0 +1,99 @@ +use codee::{CodecError, Decoder, Encoder}; +use leptos::prelude::{signal, Effect, Get, GetUntracked, LocalStorage, ReadSignal, Set, Signal, UpdateUntracked, WriteSignal}; +use leptos_use::core::MaybeRwSignal; +use leptos_use::storage::{ + use_storage_with_options, StorageType, UseStorageError, UseStorageOptions, +}; +use std::fmt::Debug; + +pub(crate) struct UseStorageReturn +where + T: Send + Sync + 'static, + Remover: Fn() + Clone + Send + Sync, +{ + pub(crate) read: Signal, + pub(crate) write: WriteSignal, + pub(crate) remove: Remover, + + #[expect(unused)] + decode_err: (ReadSignal, WriteSignal), + #[expect(unused)] + effect: Effect, +} + +pub(crate) fn use_storage_with_options_and_error_handler( + storage_type: StorageType, + key: impl Into> + 'static, + // Read once on creation. Reused through cloning when a decode error must be resolved by using the initial value again. + initial_value: impl Into>, +) -> UseStorageReturn +where + T: Default + Debug + Clone + PartialEq + Send + Sync, + C: Encoder + Decoder, + >::Error: Debug, + >::Error: Debug, +{ + let (decode_err, set_decode_err) = signal(false); + + let key = key.into(); + + let initial_value_signal = initial_value.into(); + let initial_value = match &initial_value_signal { + MaybeRwSignal::Static(s) => s.clone(), + MaybeRwSignal::DynamicRw(r, _) => r.get_untracked(), + MaybeRwSignal::DynamicRead(r) => r.get_untracked(), + }; + let options = UseStorageOptions::default() + .initial_value(initial_value_signal) + .listen_to_storage_changes(true) + .delay_during_hydration(false) + .on_error(move |err| { + let log_as_error = match &err { + UseStorageError::StorageNotAvailable(_) => true, + UseStorageError::StorageReturnedNone => true, + UseStorageError::GetItemFailed(_) => true, + UseStorageError::SetItemFailed(_) => true, + UseStorageError::RemoveItemFailed(_) => true, + UseStorageError::NotifyItemChangedFailed(_) => true, + UseStorageError::ItemCodecError(codec_err) => match codec_err { + CodecError::Encode(_) => true, + CodecError::Decode(_decode_err) => { + // Only schedule deletion (and log) once! + // We saw that these decode errors may come in multiple times in quick + // succession, without our effect being able to handle it in between. + if !decode_err.get_untracked() { + // Note: A "decode" error will always come up if we break the + // type, e.g. by adding a new field that wasn't previously + // persisted. + tracing::debug!(?err, "Data format of '{}' changed. Scheduling removal of previously persisted value.", key.get()); + set_decode_err.set(true); + } + false + } + }, + }; + if log_as_error { + tracing::error!(?err, "Error reading '{}' from storage.", key.get()); + } + }); + + let (read, write, remove) = use_storage_with_options::(storage_type, key, options); + + let remove_clone = remove.clone(); + let effect = Effect::new(move |_| { + if decode_err.get() { + tracing::trace!("Removing previously persisted value of '{}' due to a decode error. Using initial value: {initial_value:?}", key.get()); + remove_clone(); + write.set(initial_value.clone()); + set_decode_err.update_untracked(|it| *it = false); + } + }); + + UseStorageReturn { + read, + write, + remove, + decode_err: (decode_err, set_decode_err), + effect, + } +}