Skip to content

Commit 4301af2

Browse files
committed
Fix OIDC metadata caching: simple periodic refresh approach
- Add RwLock wrapper around OidcClient for atomic updates - Spawn background task that refreshes provider metadata every 6 hours - Replace entire client atomically when new metadata is available - Use blocking reads for client access (fast, simple) - Log refresh attempts and failures for operational visibility This fixes the key rotation vulnerability where OIDC provider metadata was fetched only once at startup. The solution is explicit, minimal, and much simpler than complex on-demand caching approaches. Changes: ~77 lines vs 450+ in previous complex implementation
1 parent 627b478 commit 4301af2

File tree

1 file changed

+77
-19
lines changed

1 file changed

+77
-19
lines changed

src/webserver/oidc.rs

Lines changed: 77 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
use std::collections::HashSet;
22
use std::future::ready;
3-
use std::{future::Future, pin::Pin, str::FromStr, sync::Arc};
3+
use std::{
4+
future::Future,
5+
pin::Pin,
6+
str::FromStr,
7+
sync::{Arc, RwLock},
8+
time::Duration,
9+
};
410

511
use crate::webserver::http_client::get_http_client_from_appdata;
612
use crate::{app_config::AppConfig, AppState};
@@ -132,7 +138,7 @@ fn get_app_host(config: &AppConfig) -> String {
132138

133139
pub struct OidcState {
134140
pub config: Arc<OidcConfig>,
135-
pub client: Arc<OidcClient>,
141+
pub client: Arc<RwLock<OidcClient>>,
136142
}
137143

138144
pub async fn initialize_oidc_state(
@@ -149,10 +155,61 @@ pub async fn initialize_oidc_state(
149155
discover_provider_metadata(&http_client, oidc_cfg.issuer_url.clone()).await?;
150156
let client = make_oidc_client(&oidc_cfg, provider_metadata)?;
151157

152-
Ok(Some(Arc::new(OidcState {
153-
config: oidc_cfg,
154-
client: Arc::new(client),
155-
})))
158+
let oidc_state = Arc::new(OidcState {
159+
config: oidc_cfg.clone(),
160+
client: Arc::new(RwLock::new(client)),
161+
});
162+
163+
// Start background refresh task
164+
let refresh_state = Arc::clone(&oidc_state);
165+
let refresh_config = Arc::clone(app_config);
166+
tokio::spawn(async move {
167+
refresh_oidc_metadata_periodically(refresh_state, refresh_config).await;
168+
});
169+
170+
Ok(Some(oidc_state))
171+
}
172+
173+
/// Background task that refreshes OIDC provider metadata every 6 hours
174+
async fn refresh_oidc_metadata_periodically(
175+
oidc_state: Arc<OidcState>,
176+
app_config: Arc<AppConfig>,
177+
) {
178+
let mut interval = tokio::time::interval(Duration::from_secs(6 * 60 * 60)); // 6 hours
179+
interval.tick().await; // Skip first tick (already initialized)
180+
181+
loop {
182+
interval.tick().await;
183+
184+
log::info!("Refreshing OIDC provider metadata");
185+
186+
match refresh_oidc_client(&oidc_state, &app_config).await {
187+
Ok(()) => {
188+
log::info!("Successfully refreshed OIDC provider metadata");
189+
}
190+
Err(e) => {
191+
log::warn!("Failed to refresh OIDC provider metadata: {}", e);
192+
// Continue with existing client
193+
}
194+
}
195+
}
196+
}
197+
198+
/// Refresh the OIDC client with new provider metadata
199+
async fn refresh_oidc_client(
200+
oidc_state: &Arc<OidcState>,
201+
app_config: &AppConfig,
202+
) -> anyhow::Result<()> {
203+
let http_client = make_http_client(app_config)?;
204+
let provider_metadata =
205+
discover_provider_metadata(&http_client, oidc_state.config.issuer_url.clone()).await?;
206+
let new_client = make_oidc_client(&oidc_state.config, provider_metadata)?;
207+
208+
// Replace the client atomically
209+
let mut client_guard = oidc_state.client.write().unwrap();
210+
*client_guard = new_client;
211+
212+
Ok(())
156213
}
157214

158215
pub struct OidcMiddleware {
@@ -239,29 +296,31 @@ where
239296

240297
log::debug!("Redirecting to OIDC provider");
241298

242-
let response = build_auth_provider_redirect_response(
243-
&self.oidc_state.client,
244-
&self.oidc_state.config,
245-
&request,
246-
);
299+
let client = self.oidc_state.client.read().unwrap();
300+
let response =
301+
build_auth_provider_redirect_response(&*client, &self.oidc_state.config, &request);
247302
Box::pin(async move { Ok(request.into_response(response)) })
248303
}
249304

250305
fn handle_oidc_callback(
251306
&self,
252307
request: ServiceRequest,
253308
) -> LocalBoxFuture<Result<ServiceResponse<BoxBody>, Error>> {
254-
let oidc_client = Arc::clone(&self.oidc_state.client);
255-
let oidc_config = Arc::clone(&self.oidc_state.config);
309+
let oidc_state = Arc::clone(&self.oidc_state);
256310

257311
Box::pin(async move {
312+
let client = oidc_state.client.read().unwrap();
258313
let query_string = request.query_string();
259-
match process_oidc_callback(&oidc_client, &oidc_config, query_string, &request).await {
314+
match process_oidc_callback(&*client, &oidc_state.config, query_string, &request).await
315+
{
260316
Ok(response) => Ok(request.into_response(response)),
261317
Err(e) => {
262318
log::error!("Failed to process OIDC callback with params {query_string}: {e}");
263-
let resp =
264-
build_auth_provider_redirect_response(&oidc_client, &oidc_config, &request);
319+
let resp = build_auth_provider_redirect_response(
320+
&*client,
321+
&oidc_state.config,
322+
&request,
323+
);
265324
Ok(request.into_response(resp))
266325
}
267326
}
@@ -296,9 +355,8 @@ where
296355
fn call(&self, request: ServiceRequest) -> Self::Future {
297356
log::trace!("Started OIDC middleware request handling");
298357

299-
let oidc_client = Arc::clone(&self.oidc_state.client);
300-
let oidc_config = Arc::clone(&self.oidc_state.config);
301-
match get_authenticated_user_info(&oidc_client, &oidc_config, &request) {
358+
let client = self.oidc_state.client.read().unwrap();
359+
match get_authenticated_user_info(&*client, &self.oidc_state.config, &request) {
302360
Ok(Some(claims)) => {
303361
if request.path() == SQLPAGE_REDIRECT_URI {
304362
return handle_authenticated_oidc_callback(request);

0 commit comments

Comments
 (0)