Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(jans-cedarling): consolidate JwtService initialization into the module #9980

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions jans-cedarling/cedarling/src/init/jwt_algorithm.rs

This file was deleted.

2 changes: 0 additions & 2 deletions jans-cedarling/cedarling/src/init/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
//! - load Cedar Policies
//! - get keys for JWT validation

mod jwt_algorithm;
pub(crate) mod policy_store;
pub(crate) mod service_config;
pub(crate) mod service_factory;

pub(crate) use service_factory::ServiceFactory;
8 changes: 1 addition & 7 deletions jans-cedarling/cedarling/src/init/service_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,19 @@
* Copyright (c) 2024, Gluu, Inc.
*/

use super::jwt_algorithm::parse_jwt_algorithms;
use super::policy_store::{load_policy_store, PolicyStoreLoadError};
use crate::bootstrap_config;
use crate::common::policy_store::PolicyStore;
use crate::{bootstrap_config, jwt};
use bootstrap_config::BootstrapConfig;

/// Configuration that hold validated infomation from bootstrap config
#[derive(typed_builder::TypedBuilder, Clone)]
pub(crate) struct ServiceConfig {
pub policy_store: PolicyStore,
pub jwt_algorithms: Vec<jwt::Algorithm>,
}

#[derive(thiserror::Error, Debug)]
pub enum ServiceConfigError {
/// Parse jwt algorithm error.
#[error("could not parse an algorithim defined in the config: {0}")]
ParseAlgorithm(#[from] jwt::Error),
/// Error that may occur during loading the policy store.
#[error("Could not load policy: {0}")]
PolicyStore(#[from] PolicyStoreLoadError),
Expand All @@ -31,7 +26,6 @@ pub enum ServiceConfigError {
impl ServiceConfig {
pub fn new(bootstrap: &BootstrapConfig) -> Result<Self, ServiceConfigError> {
let builder = ServiceConfig::builder()
.jwt_algorithms(parse_jwt_algorithms(bootstrap)?)
.policy_store(load_policy_store(&bootstrap.policy_store_config)?);

Ok(builder.build())
Expand Down
17 changes: 10 additions & 7 deletions jans-cedarling/cedarling/src/init/service_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,20 @@ impl<'a> ServiceFactory<'a> {
if let Some(jwt_service) = &self.container.jwt_service {
jwt_service.clone()
} else {
let config = match self.bootstrap_config.jwt_config {
let config = match &self.bootstrap_config.jwt_config {
crate::JwtConfig::Disabled => JwtServiceConfig::WithoutValidation,
crate::JwtConfig::Enabled { .. } => JwtServiceConfig::WithValidation {
supported_algs: self.service_config.jwt_algorithms.clone(),
crate::JwtConfig::Enabled { signature_algorithms } => JwtServiceConfig::WithValidation {
supported_algs: signature_algorithms.to_vec(),
trusted_idps: self.policy_store().trusted_issuers.expect("Expected trusted issuers to be present for JWT validation, but found None. Ensure that the policy store is properly initialized with trusted issuers before using JWT validation.").clone(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not panic!. We need to get value from service_config

},
};

let service = Arc::new(JwtService::new_with_config(config));
self.container.jwt_service = Some(service.clone());
service

// TODO: handle intialization errors
let jwt_service = Arc::new(
abaghinyan marked this conversation as resolved.
Show resolved Hide resolved
JwtService::new_with_config(config).expect("should initialize jwt service"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not panic!

Copy link
Contributor Author

@rmarinn rmarinn Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know we shouldn't panic but look at the code calling it...

    // get jwt service
    pub fn jwt_service(&mut self) -> Arc<JwtService> {
        if let Some(jwt_service) = &self.container.jwt_service {
            jwt_service.clone()
        } else {
            let config = match &self.bootstrap_config.jwt_config {
                crate::JwtConfig::Disabled => JwtServiceConfig::WithoutValidation,
                crate::JwtConfig::Enabled { signature_algorithms } => JwtServiceConfig::WithValidation {
                    supported_algs: signature_algorithms.to_vec(),
                    trusted_idps: self.policy_store().trusted_issuers.expect("Expected trusted issuers to be present for JWT validation, but found None. Ensure that the policy store is properly initialized with trusted issuers before using JWT validation.").clone(),
                },
            };
            
            // TODO: handle intialization errors 
            let jwt_service = Arc::new(
                JwtService::new_with_config(config).expect("should initialize jwt service"),
            );
            self.container.jwt_service = Some(jwt_service.clone());
            jwt_service
        }
    }

ServiceFactory::jwt_service doesn't return a Result when initializing...

);
self.container.jwt_service = Some(jwt_service.clone());
jwt_service
}
}

Expand Down
108 changes: 95 additions & 13 deletions jans-cedarling/cedarling/src/jwt/decoding_strategy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
pub mod key_service;
use crate::common::policy_store::TrustedIssuer;

use super::Error;
use super::InitError;
use jsonwebtoken as jwt;
pub use key_service::*;
pub use key_service::KeyService;
use serde::de::DeserializeOwned;

/// Represents the decoding strategy for JWT tokens.
Expand Down Expand Up @@ -47,15 +47,17 @@ impl DecodingStrategy {
/// # Errors
/// Returns an error if the specified algorithm is unrecognized or the key service initialization fails.
pub fn new_with_validation(
config_algs: Vec<jwt::Algorithm>,
config_algs: Vec<String>,
trusted_idps: Vec<TrustedIssuer>,
) -> Result<Self, Error> {
) -> Result<Self, InitError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should get already validated value and do not return any errors

// initialize the key service with OpenID configuration endpoints
let openid_conf_endpoints = trusted_idps
.iter()
.map(|x| x.openid_configuration_endpoint.as_ref())
.collect();
let key_service = KeyService::new(openid_conf_endpoints).map_err(Error::KeyService)?;
let key_service = KeyService::new(openid_conf_endpoints)?;

let config_algs = parse_jwt_algorithms(config_algs).map_err(InitError::DecodingStrategy)?;

Ok(Self::WithValidation {
key_service,
Expand All @@ -69,7 +71,7 @@ impl DecodingStrategy {
iss: Option<impl ToString>,
aud: Option<impl ToString>,
req_sub: bool,
) -> Result<T, Error> {
) -> Result<T, super::Error> {
match self {
DecodingStrategy::WithoutValidation => self.extract_claims(jwt),
DecodingStrategy::WithValidation {
Expand All @@ -86,7 +88,7 @@ impl DecodingStrategy {
///
/// # Errors
/// Returns an error if the claims cannot be extracted.
pub fn extract_claims<T: DeserializeOwned>(&self, jwt_str: &str) -> Result<T, Error> {
pub fn extract_claims<T: DeserializeOwned>(&self, jwt_str: &str) -> Result<T, super::Error> {
let mut validator = jwt::Validation::default();
validator.insecure_disable_signature_validation();
validator.validate_exp = false;
Expand Down Expand Up @@ -122,12 +124,12 @@ fn decode_and_validate_jwt<T: DeserializeOwned>(
req_sub: bool,
supported_algs: &[jwt::Algorithm],
key_service: &KeyService,
) -> Result<T, Error> {
) -> Result<T, super::Error> {
let header = jwt::decode_header(jwt).map_err(Error::Parsing)?;

// reject unsupported algorithms early
if !supported_algs.contains(&header.alg) {
return Err(Error::TokenSignedWithUnsupportedAlgorithm(header.alg));
return Err(Error::TokenSignedWithUnsupportedAlgorithm(header.alg).into());
}

// set up validation rules
Expand All @@ -149,8 +151,9 @@ fn decode_and_validate_jwt<T: DeserializeOwned>(
let kid = &header
.kid
.ok_or_else(|| Error::MissingRequiredHeader("kid".into()))?;
let key = key_service.get_key(kid).map_err(Error::KeyService)?;
// TODO: handle tokens without a `kid` in the header
let key = key_service.get_key(kid)?;
// TODO: figure out how to handle tokens without a `kid` in the header
// there's no plans yet if we need to support this or not.

// decode and validate the jwt
let claims = jwt::decode::<T>(jwt, &key, &validator)
Expand All @@ -159,6 +162,33 @@ fn decode_and_validate_jwt<T: DeserializeOwned>(
Ok(claims)
}

fn parse_jwt_algorithms(algorithms: Vec<String>) -> Result<Vec<jwt::Algorithm>, Error> {
let parsing_results = algorithms
.iter()
.map(|alg| string_to_alg(alg))
.collect::<Vec<Result<jwt::Algorithm, Box<str>>>>();

let (successes, errors): (Vec<_>, Vec<_>) =
parsing_results.into_iter().partition(Result::is_ok);

// Collect all errors into a single error message or return them as a vector.
if !errors.is_empty() {
let unsupported_algs = errors
.into_iter()
.filter_map(Result::err)
.collect::<Vec<Box<str>>>()
.join(", ");
return Err(Error::UnimplementedAlgorithm(unsupported_algs));
}

let algorithms = successes
.into_iter()
.filter_map(Result::ok)
.collect::<Vec<jwt::Algorithm>>();

Ok(algorithms)
}

/// Converts a string representation of an algorithm to a `jwt::Algorithm` enum.
///
/// This function maps algorithm names (e.g., "HS256", "RS256") to corresponding
Expand All @@ -169,7 +199,7 @@ fn decode_and_validate_jwt<T: DeserializeOwned>(
///
/// # Errors
/// Returns an error if the algorithm is not implemented.
pub fn string_to_alg(algorithm: &str) -> Result<jwt::Algorithm, Error> {
pub fn string_to_alg(algorithm: &str) -> Result<jwt::Algorithm, Box<str>> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we return error as string?

match algorithm {
"HS256" => Ok(jwt::Algorithm::HS256),
"HS384" => Ok(jwt::Algorithm::HS384),
Expand All @@ -183,6 +213,58 @@ pub fn string_to_alg(algorithm: &str) -> Result<jwt::Algorithm, Error> {
"PS384" => Ok(jwt::Algorithm::PS384),
"PS512" => Ok(jwt::Algorithm::PS512),
"EdDSA" => Ok(jwt::Algorithm::EdDSA),
_ => Err(Error::UnimplementedAlgorithm(algorithm.into())),
_ => Err(algorithm.into()),
}
}

/// Error type for issues encountered during JWT decoding and validation.
///
/// The `DecodingStrategy` is responsible for parsing, validating, and verifying JWTs.
/// This enum represents various errors that can occur during these processes, such as
/// issues with parsing the token, unsupported algorithms, or validation failures.
#[derive(thiserror::Error, Debug)]
pub enum Error {
/// An error occurred while parsing the JWT.
///
/// This error occurs when the provided JWT cannot be properly parsed.
/// This might happen if the token is malformed or contains invalid data
/// that does not conform to the JWT structure.
#[error("Error parsing the JWT: {0}")]
Parsing(#[source] jsonwebtoken::errors::Error),

/// The token was signed using an unsupported algorithm.
///
/// This error occurs when the JWT's header specifies an algorithm that
/// is not supported by the current validation configuration. Common causes
/// include encountering an algorithm that is disallowed or not yet implemented
/// by the decoding strategy.
#[error("The JWT is signed with an unsupported algorithm: {0:?}")]
TokenSignedWithUnsupportedAlgorithm(jsonwebtoken::Algorithm),

/// A required header is missing from the JWT.
///
/// This error occurs when a necessary header is missing from the JWT's header section,
/// preventing proper verification or validation of the token.
///
/// *Certain headers, such as `kid` (Key ID), are required for proper JWT processing.*
#[error("The JWT is missing a required header: {0}")]
MissingRequiredHeader(String),

/// JWT validation failed.
///
/// This error occurs when the JWT fails to pass validation checks. Common causes
/// include an invalid signature, expired tokens, or claims that do not meet
/// the expected criteria. The underlying validation error provides more context
/// on why the token was considered invalid.
#[error("Failed to validate the JWT: {0}")]
Validation(#[source] jsonwebtoken::errors::Error),

/// The configuration defines an unsupported or unimplemented algorithm.
///
/// This error occurs when the validation configuration specifies an algorithm
/// that is either not implemented by the `DecodingStrategy` or not recognized
/// by the JWT service. This typically happens when the application tries to use
/// a newer or less common algorithm that has not been added to the supported set.
#[error("An algorithm(s) defined in the configuration is not yet implemented: {0}")]
UnimplementedAlgorithm(String),
}
Loading
Loading