feat(jans-cedarling): improve error handling for JWKS responses (#9982)

feat(jans-cedarling): add graceful error handling for unsupported algorithms in JWKs

- added logic to skip over JWKs that use unsupported algorithms without breaking the initialization process
- updated error handling to avoid stopping service initialization when encountering unknown algorithm variants in JWKs

Signed-off-by: rmarinn <[email protected]>

refactor(jans-cedarling): extract repeated code into a method for better code reusability

Signed-off-by: rmarinn <[email protected]>

chore(jans-cedarling): fix misspellings

Signed-off-by: rmarinn <[email protected]>

fix(jans-cedarling): add error handling to update_jwks_for_iss

Signed-off-by: rmarinn <[email protected]>

test(jans-cedarling): improve test assertions and messages

Signed-off-by: rmarinn <[email protected]>

test(jans-cedarling): improve panic message in test utils

Signed-off-by: rmarinn <[email protected]>

test(jans-cedarling): Make generate_token_using_claims return a Result

- Make gernerate_token_using_claims return a Result instead of panicking
  for improved error management in tests.

Signed-off-by: rmarinn <[email protected]>

feat(jans-cedarling): add retry mechanism for KeyService HTTP requests

Signed-off-by: rmarinn <[email protected]>

chore(jans-cedarling): add missing license header

Signed-off-by: rmarinn <[email protected]>

chore(jans-cedarling): resolve clippy issues

Signed-off-by: rmarinn <[email protected]>

chore(jans-cedarling): update example for authorize_with_jwt_validation.rs

Signed-off-by: rmarinn <[email protected]>

refactor(jans-cedarling): remove `exp` and `nbf` requirement for userinfo_token

Signed-off-by: rmarinn <[email protected]>

jans-cedarling/cedarling/examples/.gitignore

@@ -0,0 +1 @@
+tokens.json

jans-cedarling/cedarling/examples/authorize_with_jwt_validation.rs

@@ -9,21 +9,37 @@ use cedarling::{
     BootstrapConfig, Cedarling, JwtConfig, LogConfig, LogTypeConfig, PolicyStoreConfig,
     PolicyStoreSource, Request, ResourceData,
 };
+use serde::Deserialize;
 use std::collections::HashMap;
 
+// Load a JSON policy store file, containing policies and trusted issuers, at compile time.
+// This file defines access control policies for different resources and actions.
 static POLICY_STORE_RAW: &str =
     include_str!("../../test_files/policy-store_with_trusted_issuers_ok.json");
 
+// Load example tokens from a JSON file, also at compile time.
+// NOTE: `tokens.json` is ignored in version control for security reasons.
+// To run this example, create a `tokens.json` file based on `tokens.example.json`.
+static TOKENS: &str = include_str!("./tokens.json");
+
+#[derive(Deserialize)]
+struct Tokens {
+    access_token: String,
+    userinfo_token: String,
+    id_token: String,
+}
+
 fn main() -> Result<(), Box<dyn std::error::Error>> {
-    // Configure the JwtService to validate signatures using the specified algorithms:
-    // `HS256` and `RS256`.
-    //
-    // Tokens signed with an algorithm not in `signature_algorithms`
-    // will be automatically marked as invalid.
+    // Configure JWT validation settings. Enable the JwtService to validate JWT tokens
+    // using specific algorithms: `HS256` and `RS256`. Only tokens signed with these algorithms
+    // will be accepted; others will be marked as invalid during validation.
     let jwt_config = JwtConfig::Enabled {
         signature_algorithms: vec!["HS256".to_string(), "RS256".to_string()],
     };
 
+    // Initialize the main Cedarling instance, responsible for policy-based authorization.
+    // This setup includes basic application information, logging configuration, and
+    // policy store configuration.
     let cedarling = Cedarling::new(BootstrapConfig {
         application_name: "test_app".to_string(),
         log_config: LogConfig {
@@ -35,48 +51,18 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         jwt_config,
     })?;
 
-    // access_token claims:
-    // {
-    //   "iss": "https://admin-ui-test.gluu.org",
-    //   "aud": "some_audience",
-    //   "sub": "some_subject",
-    //   "exp": 2724945978, ->  May 8, 2056 01:26:18 GMT+0800
-    //   "iat": 1624832259  ->  June 28, 2021 06:17:39 GMT+0800
-    // }
-    let access_token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJodHRwczovL2FkbWluLXVpLXRlc3QuZ2x1dS5vcmciLCJhdWQiOiJzb21lX2F1ZGllbmNlIiwic3ViIjoic29tZV9zdWJqZWN0IiwiZXhwIjoyNzI0OTQ1OTc4LCJpYXQiOjE2MjQ4MzIyNTl9.oZKCdPvtvA8yJ5BQhP5725TYf0CAzcOZEhPQmom7cOc".to_string();
-
-    // id_token claims:
-    // {
-    //   "iss": "https://admin-ui-test.gluu.org",
-    //   "aud": "some_audience",
-    //   "sub": "some_subject",
-    //   "exp": 2724945978, ->  May 8, 2056 01:26:18 GMT+0800
-    //   "iat": 1624832259  ->  June 28, 2021 06:17:39 GMT+0800
-    //   "nonce": "123123123",
-    //   "name": "Mr. admin",
-    //   "email": "[email protected]"
-    // }
-    let id_token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJodHRwczovL2FkbWluLXVpLXRlc3QuZ2x1dS5vcmciLCJhdWQiOiJzb21lX2F1ZGllbmNlIiwic3ViIjoic29tZV9zdWJqZWN0IiwiZXhwIjoyNzI0OTQ1OTc4LCJpYXQiOjE2MjQ4MzIyNTksIm5vbmNlIjoiMTIzMTIzMTIzIiwibmFtZSI6Ik1yLiBhZG1pbiIsImVtYWlsIjoiYWRtaW5AZ2x1dS5vcmcifQ.Zzx3gz3d3YK2geb0aCPLyiOEvFviuMsGbf1urNnmPDU".to_string();
-
-    // userinfo_token claims:
-    // {
-    //   "iss": "https://admin-ui-test.gluu.org",
-    //   "aud": "some_audience",
-    //   "sub": "some_subject",
-    //   "exp": 2724945978, ->  May 8, 2056 01:26:18 GMT+0800
-    //   "iat": 1624832259  ->  June 28, 2021 06:17:39 GMT+0800
-    //   "nonce": "123123123",
-    //   "name": "Mr. admin",
-    //   "email": "[email protected]",
-    //   "email_verified": true,
-    //   "locale": "en_US",
-    // }
-    let userinfo_token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJodHRwczovL2FkbWluLXVpLXRlc3QuZ2x1dS5vcmciLCJhdWQiOiJzb21lX2F1ZGllbmNlIiwic3ViIjoic29tZV9zdWJqZWN0IiwiZXhwIjoyNzI0OTQ1OTc4LCJpYXQiOjE2MjQ4MzIyNTksIm5vbmNlIjoiMTIzMTIzMTIzIiwibmFtZSI6Ik1yLiBhZG1pbiIsImVtYWlsIjoiYWRtaW5AZ2x1dS5vcmciLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwibG9jYWxlIjoiZW5fVVMifQ.HvX2s_ZWUfyvRHLUl5CWaSPOIp9zVpwP2LbF5U6tNGA".to_string();
+    // Parse the tokens from the JSON string loaded from `tokens.json`.
+    // This will create a `Tokens` struct populated with `access_token`, `userinfo_token`, and `id_token`.
+    let tokens = serde_json::from_str::<Tokens>(TOKENS).expect("should deserialize tokens");
 
+    // Perform an authorization request to Cedarling.
+    // This request checks if the provided tokens have sufficient permission to perform an action
+    // on a specific resource. Each token (access, ID, and userinfo) is required for the
+    // authorization process, alongside resource and action details.
     let result = cedarling.authorize(Request {
-        access_token,
-        id_token,
-        userinfo_token,
+        access_token: tokens.access_token,
+        id_token: tokens.id_token,
+        userinfo_token: tokens.userinfo_token,
         action: "Jans::Action::\"Update\"".to_string(),
         context: serde_json::json!({}),
         resource: ResourceData {
@@ -88,6 +74,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             )]),
         },
     });
+
+    // Handle authorization result. If there's an error, print it.
     if let Err(ref e) = &result {
         eprintln!("Error while authorizing: {:?}\n\n", e)
     }

jans-cedarling/cedarling/examples/tokens.example.json

@@ -0,0 +1,5 @@
+{
+  "access_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...",
+  "id_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6IjE2...",
+  "userinfo_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9..."
+}

jans-cedarling/cedarling/src/jwt/decoding_strategy.rs

@@ -136,7 +136,11 @@ fn decode_and_validate_jwt<T: DeserializeOwned>(
 
     // set up validation rules
     let mut validator = jwt::Validation::new(header.alg);
-    validator.set_required_spec_claims(&["iss", "sub", "aud", "exp"]);
+    let mut required_spec_claims = vec!["iss", "sub", "aud"];
+    if decoding_args.validate_exp {
+        required_spec_claims.push("exp");
+    }
+    validator.set_required_spec_claims(&required_spec_claims);
     validator.validate_nbf = decoding_args.validate_nbf;
     validator.validate_exp = decoding_args.validate_exp;
     if let Some(iss) = decoding_args.iss {

jans-cedarling/cedarling/src/jwt/decoding_strategy/error.rs

@@ -1,3 +1,10 @@
+/*
+ * This software is available under the Apache-2.0 license.
+ * See https://www.apache.org/licenses/LICENSE-2.0.txt for full text.
+ *
+ * Copyright (c) 2024, Gluu, Inc.
+ */
+
 use super::key_service;
 use jsonwebtoken as jwt;
 

jans-cedarling/cedarling/src/jwt/decoding_strategy/key_service.rs

@@ -9,16 +9,59 @@ mod error;
 mod openid_config;
 
 pub use error::Error;
-use jsonwebtoken::jwk::JwkSet;
+use jsonwebtoken::jwk::Jwk;
 use jsonwebtoken::DecodingKey;
 use openid_config::*;
 use reqwest::blocking::Client;
+use serde::Deserialize;
 use std::collections::HashMap;
 use std::sync::Arc;
+use std::thread::sleep;
+use std::time::Duration;
 
+/// A wrapper around `reqwest::blocking::Client` providing HTTP request functionality with retry logic.
+///
+/// The `HttpClient` struct allows for sending GET requests with a retry mechanism that attempts to
+/// fetch the requested resource up to a maximum number of times if an error occurs.
+struct HttpClient(reqwest::blocking::Client);
+
+impl HttpClient {
+    /// Sends a GET request to the specified URI with retry logic.
+    ///
+    /// This method will attempt to fetch the resource up to `MAX_RETRIES` times, with an increasing delay
+    /// between each attempt. The delay duration is adjusted for testing and non-testing environments.
+    fn get(&self, uri: &str) -> Result<reqwest::blocking::Response, Error> {
+        const MAX_RETRIES: u32 = 3;
+        #[cfg(test)]
+        const RETRY_DELAY: Duration = Duration::from_millis(10);
+        #[cfg(not(test))]
+        const RETRY_DELAY: Duration = Duration::from_secs(1);
+
+        // Fetch the JWKS from the jwks_uri
+        let mut attempts = 0;
+        let response = loop {
+            match self.0.get(uri).send() {
+                Ok(response) => break response, // Exit loop on success
+                Err(e) if attempts < MAX_RETRIES => {
+                    attempts += 1;
+                    // TODO: pass this message to the logger
+                    eprintln!(
+                        "Request failed (attempt {} of {}): {}. Retrying...",
+                        attempts, MAX_RETRIES, e
+                    );
+                    sleep(RETRY_DELAY * attempts);
+                },
+                Err(e) => return Err(Error::MaxHttpRetriesReached(e)), // Exit if max retries exceeded
+            }
+        };
+        response.error_for_status().map_err(Error::HttpStatus)
+    }
+}
+
+/// A service that manages key retrieval and caching for OpenID Connect configurations.
 pub struct KeyService {
     idp_configs: HashMap<Box<str>, OpenIdConfig>, // <issuer (`iss`), OpenIdConfig>
-    http_client: Client,
+    http_client: HttpClient,
 }
 
 #[allow(unused)]
@@ -30,42 +73,23 @@ impl KeyService {
     /// failures will return a corresponding `Error`.
     pub fn new(openid_conf_endpoints: Vec<&str>) -> Result<Self, Error> {
         let mut idp_configs = HashMap::new();
-        let http_client = Client::builder().build().map_err(Error::Http)?;
+        let http_client = HttpClient(
+            Client::builder()
+                .build()
+                .map_err(Error::HttpClientInitialization)?,
+        );
 
         // fetch IDP configs
         for endpoint in openid_conf_endpoints {
-            let conf_src: OpenIdConfigSource = http_client
-                .get(endpoint)
-                .send()
-                .map_err(Error::Http)?
-                .error_for_status()
-                .map_err(Error::Http)?
-                .json()
-                .map_err(Error::RequestDeserialization)?;
+            let response = http_client.get(endpoint)?;
+            let conf_src: OpenIdConfigSource =
+                response.json().map_err(Error::RequestDeserialization)?;
             let (issuer, conf) = OpenIdConfig::from_source(conf_src);
             idp_configs.insert(issuer, conf);
         }
 
-        /// retrieves a decoding key based on the provided key ID (`kid`).
-        ///
-        /// this method first attempts to retrieve the key from the local key store. if the key
-        /// is not found, it will refresh the JWKS and try again. if the key is still not found,
-        /// an error of type `KeyNotFound` is returned.
         for (iss, conf) in &mut idp_configs {
-            let jwks: JwkSet = http_client
-                .get(&*conf.jwks_uri)
-                .send()
-                .map_err(Error::Http)?
-                .error_for_status()
-                .map_err(Error::Http)?
-                .json()
-                .map_err(Error::RequestDeserialization)?;
-            let mut decoding_keys = conf.decoding_keys.write().map_err(|_| Error::Lock)?;
-            for jwk in jwks.keys {
-                let decoding_key = DecodingKey::from_jwk(&jwk).map_err(Error::KeyParsing)?;
-                let key_id = jwk.common.key_id.ok_or(Error::MissingKeyId)?;
-                decoding_keys.insert(key_id.into(), Arc::new(decoding_key));
-            }
+            Self::update_decoding_keys(&http_client, conf)?;
         }
 
         Ok(Self {
@@ -89,7 +113,7 @@ impl KeyService {
                 eprintln!("could not find {}, updating jwks", kid);
                 // if the key is not found in the local keystore, update
                 // the local keystore and try again
-                self.update_jwks(iss);
+                self.update_jwks_for_iss(iss)?;
                 if let Some(key) = self.get_key_from_iss(iss, kid)? {
                     return Ok(key.clone());
                 }
@@ -115,35 +139,63 @@ impl KeyService {
     ///
     /// this method fetches a fresh set of keys from the JWKS URI of the given issuer
     /// and updates the local key store.
-    fn update_jwks(&self, iss: &str) -> Result<(), Error> {
-        let conf = match self.idp_configs.get(iss) {
-            Some(conf) => conf,
+    fn update_jwks_for_iss(&self, iss: &str) -> Result<(), Error> {
+        match self.idp_configs.get(iss) {
+            Some(conf) => Ok(Self::update_decoding_keys(&self.http_client, conf)?),
             // do nothing if the issuer isn't in the current store
-            None => return Ok(()),
-        };
+            None => Ok(()),
+        }
+    }
+
+    /// Updates the keys in the given config
+    fn update_decoding_keys(http_client: &HttpClient, conf: &OpenIdConfig) -> Result<(), Error> {
+        let mut fetched_keys = HashMap::new();
+
+        // Fetch the JWKS from the jwks_uri
+        let response = http_client.get(&conf.jwks_uri)?;
+
+        // Deserialize the response into a Jwks
+        let jwks: Jwks = response.json().map_err(Error::RequestDeserialization)?;
 
-        // fetch fresh keys
-        let mut new_keys = HashMap::new();
-        let jwks: JwkSet = self
-            .http_client
-            .get(&*conf.jwks_uri)
-            .send()
-            .map_err(Error::Http)?
-            .error_for_status()
-            .map_err(Error::Http)?
-            .json()
-            .map_err(Error::RequestDeserialization)?;
+        // Deserialize the Jwk into multiple Decoding Keys then store them
         for jwk in jwks.keys {
-            let decoding_key = DecodingKey::from_jwk(&jwk).map_err(Error::KeyParsing)?;
-            let key_id = jwk.common.key_id.ok_or(Error::MissingKeyId)?;
-            new_keys.insert(key_id.into(), Arc::new(decoding_key));
+            let jwk = serde_json::from_str::<Jwk>(&jwk.to_string());
+
+            match jwk {
+                Ok(jwk) => {
+                    // convert the parsed JWK to a DecodingKey and insert it into the decoding_keys map
+                    // if the JWK does not have a key ID (kid), return a MissingKeyId error
+                    let decoding_key = DecodingKey::from_jwk(&jwk).map_err(Error::KeyParsing)?;
+                    let key_id = jwk.common.key_id.ok_or(Error::MissingKeyId)?;
+
+                    // insert the key into the map, using the key ID as the map's key
+                    fetched_keys.insert(key_id.into(), Arc::new(decoding_key));
+                },
+                Err(e) => {
+                    // if the error indicates an unknown variant, we can safely ignore it.
+                    //
+                    // TODO: also print it in the logging
+                    if !e.to_string().contains("unknown variant") {
+                        return Err(Error::KeyParsing(e.into()));
+                    }
+                },
+            };
         }
 
-        // we reassign the keys after fetching and deserializing the keys
-        // so we don't lose the old ones in case the process fails
         let mut decoding_keys = conf.decoding_keys.write().map_err(|_| Error::Lock)?;
-        *decoding_keys = new_keys;
+        *decoding_keys = fetched_keys;
 
         Ok(())
     }
 }
+
+/// A simple struct to deserialize a collection of JWKs (JSON Web Keys).
+///
+/// This struct holds the raw keys in a vector of `serde_json::Value`, allowing
+/// the keys to be processed or validated individually. It is primarily used
+/// as an intermediary step for deserialization before iterating over each key
+/// to check for errors or unsupported algorithms and skipping any invalid keys.
+#[derive(Deserialize)]
+struct Jwks {
+    keys: Vec<serde_json::Value>,
+}

jans-cedarling/cedarling/src/jwt/decoding_strategy/key_service/error.rs

@@ -12,14 +12,22 @@ pub enum Error {
     #[error("No key with `kid`=\"{0}\" found in the JWKS.")]
     KeyNotFound(String),
 
-    /// Represents an HTTP error during the request.
-    #[error("HTTP error occurred: {0}")]
-    Http(#[source] reqwest::Error),
+    /// Indicates an HTTP error response received from an endpoint.
+    #[error("Received error HTTP status: {0}")]
+    HttpStatus(#[source] reqwest::Error),
+
+    /// Indicates a failure to reach the endpoint after 3 attempts.
+    #[error("Could not reach endpoint after trying 3 times: {0}")]
+    MaxHttpRetriesReached(#[source] reqwest::Error),
 
     /// Indicates failure to deserialize the response from the HTTP request.
     #[error("Failed to deserialize the response from the HTTP request: {0}")]
     RequestDeserialization(#[source] reqwest::Error),
 
+    /// Indicates failure to initialize the HTTP client.
+    #[error("Failed to initilize HTTP client: {0}")]
+    HttpClientInitialization(#[source] reqwest::Error),
+
     /// Indicates an error in parsing the decoding key from the JWKS JSON.
     #[error("Error parsing decoding key from JWKS JSON: {0}")]
     KeyParsing(#[source] jsonwebtoken::errors::Error),

jans-cedarling/cedarling/src/jwt/decoding_strategy/key_service/openid_config.rs

@@ -36,8 +36,7 @@ pub struct OpenIdConfigSource {
 /// represents the OpenID configuration for an identity provider.
 pub struct OpenIdConfig {
     pub jwks_uri: Box<str>,
-    // <key_id (`kid`), DecodingKey>
-    pub decoding_keys: Arc<RwLock<HashMap<Box<str>, Arc<DecodingKey>>>>,
+    pub decoding_keys: Arc<RwLock<HashMap<Box<str>, Arc<DecodingKey>>>>, // <key_id (`kid`), DecodingKey>
 }
 
 impl OpenIdConfig {