feat56: refresh oidc discovery

integration-test/regression/tokens.ts

@@ -476,7 +476,7 @@ describe('Persistent token', () => {
         user = undefined!;
     });
 
-    it('An failed login with invalid authorization shall not change the current user', async () => {
+    it('A failed login with invalid authorization shall not change the current user', async () => {
         const user = await TestUser.createGuest();
         const response = await api.request
             .loginWithToken(user.tid!, user.sid!, null, null, false)

service/src/auth/auth_service.rs

@@ -47,6 +47,8 @@ pub struct OIDCConfig {
     pub client_secret: String,
     pub scopes: Vec<String>,
     pub ignore_certificates: Option<bool>,
+    /// Maximum time to store the discovered OIDC client information, like JWKS.
+    pub ttl_client: Option<usize>,
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
@@ -58,9 +60,13 @@ pub struct AuthSessionConfig {
     pub external_login_cookie_secret: String,
     pub token_cookie_secret: String,
 
+    /// The maximum time to line of a session in seconds
     pub ttl_session: usize,
+    /// The maximum time to line of an access (remember me) token in seconds
     pub ttl_access_token: usize,
+    /// The maximum time to line of a single access (one-time-use) token in seconds
     pub ttl_single_access: usize,
+    /// The maximum time to line of an api-key in seconds
     pub ttl_api_key: usize,
 }
 
@@ -120,6 +126,8 @@ pub enum AuthBuildError {
     InvalidUserInfoUrl(String),
     #[error("Invalid redirect url: {0}")]
     RedirectUrl(String),
+    #[error("Invalid key cache time: {0}")]
+    InvalidKeyCacheTime(#[source] TryFromIntError),
     #[error("Failed to discover open id: {0}")]
     OIDCDiscovery(OIDCDiscoveryError),
     #[error("Failed to create http client")]

service/src/auth/oidc/oidc_client.rs

@@ -1,34 +1,42 @@
 use crate::auth::{async_http_client, AuthBuildError, OIDCConfig};
-use async_once_cell::OnceCell;
 use oauth2::{reqwest::AsyncHttpClientError, ClientId, ClientSecret, HttpRequest, HttpResponse, RedirectUrl, Scope};
 use openidconnect::{
     core::{CoreClient, CoreProviderMetadata},
     IssuerUrl,
 };
 use reqwest::Client as HttpClient;
 use serde::Serialize;
-use std::sync::Arc;
+use std::{num::TryFromIntError, time::Duration as StdDuration};
+use std::{sync::Arc, time::Instant};
 use thiserror::Error as ThisError;
+use tokio::sync::Mutex;
 use url::Url;
 
 struct ClientInfo {
     client_id: ClientId,
     client_secret: ClientSecret,
     discovery_url: IssuerUrl,
     redirect_url: RedirectUrl,
+    ttl_client: StdDuration,
 }
 
 #[derive(ThisError, Debug, Serialize)]
 #[error("OpenId Connect discovery failed")]
 #[serde(rename_all = "camelCase")]
 pub struct OIDCDiscoveryError(pub String);
 
+#[derive(Clone)]
+struct CachedClient {
+    client: CoreClient,
+    created_at: Instant,
+}
+
 pub(in crate::auth) struct OIDCClient {
     pub provider: String,
     pub scopes: Vec<Scope>,
     client_info: ClientInfo,
     http_client: HttpClient,
-    client: Arc<OnceCell<CoreClient>>,
+    cached_client: Arc<Mutex<Option<CachedClient>>>,
 }
 
 impl OIDCClient {
@@ -48,6 +56,13 @@ impl OIDCClient {
         let discovery_url = IssuerUrl::new(config.discovery_url.clone())
             .map_err(|err| AuthBuildError::InvalidIssuer(format!("{err}")))?;
 
+        let ttl_client = config
+            .ttl_client
+            .map(|sec| Ok::<_, TryFromIntError>(StdDuration::from_secs(u64::try_from(sec)?)))
+            .transpose()
+            .map_err(AuthBuildError::InvalidKeyCacheTime)?
+            .unwrap_or(StdDuration::from_secs(15 * 60));
+
         let ignore_certificates = config.ignore_certificates.unwrap_or(false);
         let http_client = HttpClient::builder()
             .redirect(reqwest::redirect::Policy::none())
@@ -63,9 +78,10 @@ impl OIDCClient {
                 client_secret,
                 discovery_url,
                 redirect_url,
+                ttl_client,
             },
             http_client,
-            client: Arc::new(OnceCell::new()),
+            cached_client: Arc::new(Mutex::new(None)),
         };
 
         if let Err(err) = client.client().await {
@@ -79,30 +95,54 @@ impl OIDCClient {
         Ok(Some(client))
     }
 
-    pub async fn client(&self) -> Result<&CoreClient, OIDCDiscoveryError> {
+    pub async fn client(&self) -> Result<CoreClient, OIDCDiscoveryError> {
         let client_info = &self.client_info;
-        self.client
-            .get_or_try_init(async {
-                let provider_metadata =
-                    match CoreProviderMetadata::discover_async(client_info.discovery_url.clone(), |request| async {
-                        async_http_client(&self.http_client, request).await
-                    })
-                    .await
-                    {
-                        Ok(meta) => meta,
-                        Err(err) => {
-                            log::warn!("Discovery failed for {}: {:#?}", self.provider, err);
-                            return Err(OIDCDiscoveryError(format!("{err:#?}")));
-                        }
-                    };
-                Ok(CoreClient::from_provider_metadata(
-                    provider_metadata,
-                    client_info.client_id.clone(),
-                    Some(client_info.client_secret.clone()),
-                )
-                .set_redirect_uri(client_info.redirect_url.clone()))
-            })
-            .await
+
+        // happy path, try to get the current client
+        {
+            let cached_client = self.cached_client.lock().await;
+            if let Some(cached_client) = &*cached_client {
+                let age = cached_client.created_at.elapsed();
+                if age < self.client_info.ttl_client {
+                    return Ok(cached_client.client.clone());
+                }
+                log::warn!("Discovery expired({}s) for {} ", self.provider, age.as_secs());
+            }
+        }
+
+        // get client configuration from discovery
+        let client = {
+            let provider_metadata =
+                match CoreProviderMetadata::discover_async(client_info.discovery_url.clone(), |request| async {
+                    async_http_client(&self.http_client, request).await
+                })
+                .await
+                {
+                    Ok(meta) => meta,
+                    Err(err) => {
+                        log::warn!("Discovery failed for {}: {:#?}", self.provider, err);
+                        return Err(OIDCDiscoveryError(format!("{err:#?}")));
+                    }
+                };
+
+            CoreClient::from_provider_metadata(
+                provider_metadata,
+                client_info.client_id.clone(),
+                Some(client_info.client_secret.clone()),
+            )
+            .set_redirect_uri(client_info.redirect_url.clone())
+        };
+
+        // cache the new client (last writer wins)
+        {
+            let mut cached_client = self.cached_client.lock().await;
+            *cached_client = Some(CachedClient {
+                created_at: Instant::now(),
+                client: client.clone(),
+            });
+        }
+
+        Ok(client)
     }
 
     pub async fn send_request(&self, request: HttpRequest) -> Result<HttpResponse, AsyncHttpClientError> {