Skip to content
Open
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
4 changes: 3 additions & 1 deletion docs/cedarling/reference/cedarling-policy-store.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ Each trusted issuer file includes:
- **`name`**: Human-readable name for the issuer (used as namespace for `TrustedIssuer` entity).
- **`description`**: Optional description of the issuer.
- **`openid_configuration_endpoint`**: HTTPS URL for the OpenID Connect discovery endpoint. For backward compatibility, `configuration_endpoint` is also accepted.
- **`issuer`**: Base URL of the issuer, used when `openid_configuration_endpoint` is not given: the discovery endpoint is then `<issuer>/.well-known/openid-configuration`. Any path on the issuer is kept, so `https://jans.example.com/realms/jans` resolves to `https://jans.example.com/realms/jans/.well-known/openid-configuration`. As an OpenID Connect issuer identifier it must use the `https` scheme and must not carry a query or fragment component; anything else is rejected, since the appended discovery path would otherwise land in the query or fragment instead of the path. One of `openid_configuration_endpoint` or `issuer` is required, and `openid_configuration_endpoint` wins when both are present.
- **`token_metadata`**: Map of token names to their metadata configuration (see [Token Metadata Schema](#token-metadata-schema)).
- **`id`**: Optional issuer ID at the top level. If absent, the ID is derived from the filename with the `.json` suffix removed.

Expand Down Expand Up @@ -523,7 +524,8 @@ This record contains the information needed to validate tokens from this issuer:

- **name** : (_String_) The name of the trusted issuer.
- **description** : (_String_) A brief description of the trusted issuer, providing context for administrators.
- **openid_configuration_endpoint** : (_String_) The HTTPS URL for the OpenID Connect configuration endpoint (usually found at `/.well-known/openid-configuration`).
- **openid_configuration_endpoint** : (_String_) The HTTPS URL for the OpenID Connect configuration endpoint (usually found at `/.well-known/openid-configuration`). Takes precedence over `issuer` when both are given.
- **issuer** : (_String_) The base URL of the issuer. Supply this instead of `openid_configuration_endpoint` to have the discovery endpoint derived as `<issuer>/.well-known/openid-configuration`. One of the two is required.
- **trusted_issuer_id** : (_Object_, _optional_) Metadata related to a particular issuer. You can add as many trusted issuers you want. Furthermore, the name this object is what will be used as the entity ID of the [Trusted Issuer](./cedarling-entities.md#trusted-issuer) that Cedarling automatically creates at startup.
- **token_metadata** : (_Object_, _optional_) Tokens metadata in a map of _token name_ -> _token metadata_. See [Token Metadata Schema](#token-metadata-schema).

Expand Down
44 changes: 44 additions & 0 deletions jans-cedarling/cedarling/src/common/policy_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,50 @@ impl TrustedIssuer {
}
}

/// The well-known path an `OpenID` Connect provider serves its discovery document from.
const OIDC_DISCOVERY_PATH: &str = ".well-known/openid-configuration";

/// Why an `issuer` cannot be used as an `OpenID` Connect issuer identifier.
#[derive(Debug, thiserror::Error)]
pub(crate) enum IssuerUrlError {
#[error("not a valid url: {0}")]
Parse(#[from] url::ParseError),
#[error("must use the https scheme, got `{0}`")]
NotHttps(String),
#[error("must not carry a query component")]
HasQuery,
#[error("must not carry a fragment component")]
HasFragment,
}

/// Derives a trusted issuer's `OpenID` configuration endpoint from its base url.
///
/// Follows `OpenID` Connect Discovery: the well-known path is appended to the issuer, keeping any
/// path the issuer carries, so `https://host/realms/jans` resolves to
/// `https://host/realms/jans/.well-known/openid-configuration`.
///
/// The issuer is validated first. A query or fragment would swallow the appended path instead of
/// extending it, silently yielding a discovery url that points somewhere else entirely, so such
/// issuers are rejected rather than derived from.
pub(crate) fn derive_oidc_endpoint(issuer: &str) -> Result<Url, IssuerUrlError> {
let parsed = Url::parse(issuer)?;

if parsed.scheme() != "https" {
return Err(IssuerUrlError::NotHttps(parsed.scheme().to_string()));
}
if parsed.query().is_some() {
return Err(IssuerUrlError::HasQuery);
}
if parsed.fragment().is_some() {
return Err(IssuerUrlError::HasFragment);
}

Ok(Url::parse(&format!(
"{}/{OIDC_DISCOVERY_PATH}",
issuer.trim_end_matches('/')
))?)
}

/// Container for compiled Cedar policies and their descriptions.
#[derive(Debug, Clone)]
pub(crate) struct PoliciesContainer {
Expand Down
8 changes: 8 additions & 0 deletions jans-cedarling/cedarling/src/common/policy_store/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ pub(crate) enum TrustedIssuerErrorType {
reason: String,
},

/// Invalid issuer base URL, from which the OIDC endpoint is derived
#[error("Issuer '{issuer_id}': invalid issuer URL '{url}': {reason}")]
InvalidIssuerUrl {
issuer_id: String,
url: String,
reason: String,
},

/// Token metadata is not an object
#[error("Issuer '{issuer_id}': token_metadata must be a JSON object")]
TokenMetadataNotAnObject { issuer_id: String },
Expand Down
247 changes: 235 additions & 12 deletions jans-cedarling/cedarling/src/common/policy_store/issuer_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
//! ensuring they conform to the required schema with proper token metadata and required fields.

use super::errors::{PolicyStoreError, TrustedIssuerErrorType};
use super::{TokenEntityMetadata, TrustedIssuer};
use super::{TokenEntityMetadata, TrustedIssuer, derive_oidc_endpoint};
use serde_json::Value as JsonValue;
use std::collections::HashMap;
use url::Url;
Expand Down Expand Up @@ -80,28 +80,56 @@ impl IssuerParser {
.and_then(|v| v.as_str())
.unwrap_or("");

let oidc_endpoint_str = obj
// Kept as the raw value: a present-but-non-string endpoint is a configuration error, and
// collapsing it to `None` here would silently derive from `issuer` instead of reporting it.
let explicit_endpoint = obj
.get("openid_configuration_endpoint") // canonical and more readable key
.or_else(|| obj.get("configuration_endpoint")) // key that was used in RFC
.and_then(|v| v.as_str())
.ok_or_else(|| PolicyStoreError::TrustedIssuerError {
file: filename.to_string(),
err: TrustedIssuerErrorType::MissingRequiredField {
issuer_id: issuer_id.clone(),
field: "openid_configuration_endpoint".to_string(),
},
})?;
.or_else(|| obj.get("configuration_endpoint")); // key that was used in RFC

let oidc_endpoint = if let Some(endpoint_value) = explicit_endpoint {
let oidc_endpoint_str =
endpoint_value
.as_str()
.ok_or_else(|| PolicyStoreError::TrustedIssuerError {
file: filename.to_string(),
err: TrustedIssuerErrorType::InvalidOidcEndpoint {
issuer_id: issuer_id.clone(),
url: endpoint_value.to_string(),
reason: "must be a string".to_string(),
},
})?;

let oidc_endpoint =
Url::parse(oidc_endpoint_str).map_err(|e| PolicyStoreError::TrustedIssuerError {
file: filename.to_string(),
err: TrustedIssuerErrorType::InvalidOidcEndpoint {
issuer_id: issuer_id.clone(),
url: oidc_endpoint_str.to_string(),
reason: e.to_string(),
},
})?
} else {
// No discovery endpoint given: derive it from the issuer's base url, which is all an
// issuer that follows OpenID Connect Discovery needs to be identified by.
let issuer_url = obj.get("issuer").and_then(|v| v.as_str()).ok_or_else(|| {
PolicyStoreError::TrustedIssuerError {
file: filename.to_string(),
err: TrustedIssuerErrorType::MissingRequiredField {
issuer_id: issuer_id.clone(),
field: "openid_configuration_endpoint".to_string(),
},
}
})?;

derive_oidc_endpoint(issuer_url).map_err(|e| PolicyStoreError::TrustedIssuerError {
file: filename.to_string(),
err: TrustedIssuerErrorType::InvalidIssuerUrl {
issuer_id: issuer_id.clone(),
url: issuer_url.to_string(),
reason: e.to_string(),
},
})?
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Parse token_metadata (optional but recommended)
let token_metadata = if let Some(metadata_json) = obj.get("token_metadata") {
Self::parse_token_metadata(metadata_json, &issuer_id, filename)?
Expand Down Expand Up @@ -376,6 +404,201 @@ mod tests {
);
}

#[test]
fn test_parse_issuer_with_issuer_base_url() {
let content = r#"{
"name": "Test Issuer",
"description": "Only the issuer base url is given",
"issuer": "https://accounts.test.com"
}"#;

let parsed = IssuerParser::parse_issuer(content, "issuer3.json")
.expect("Should parse an issuer given only its base url");

assert_eq!(
parsed[0].issuer.oidc_endpoint.as_str(),
"https://accounts.test.com/.well-known/openid-configuration",
"Should derive the discovery endpoint from the issuer base url"
);
}

#[test]
fn test_parse_issuer_with_issuer_base_url_keeps_path() {
let content = r#"{
"name": "Test Issuer",
"description": "Issuer with a path and a trailing slash",
"issuer": "https://accounts.test.com/realms/jans/"
}"#;

let parsed = IssuerParser::parse_issuer(content, "issuer4.json")
.expect("Should parse an issuer whose base url carries a path");

assert_eq!(
parsed[0].issuer.oidc_endpoint.as_str(),
"https://accounts.test.com/realms/jans/.well-known/openid-configuration",
"Should keep the issuer path and drop its trailing slash"
);
}

#[test]
fn test_parse_issuer_endpoint_takes_precedence_over_issuer() {
let content = r#"{
"name": "Test Issuer",
"description": "Both fields are given",
"issuer": "https://accounts.test.com",
"openid_configuration_endpoint": "https://accounts.test.com/custom/openid-configuration"
}"#;

let parsed = IssuerParser::parse_issuer(content, "issuer5.json")
.expect("Should parse when both the endpoint and the issuer are given");

assert_eq!(
parsed[0].issuer.oidc_endpoint.as_str(),
"https://accounts.test.com/custom/openid-configuration",
"Should prefer the explicitly configured endpoint over the derived one"
);
}

#[test]
fn test_parse_issuer_rejects_non_https_issuer() {
let content = r#"{
"name": "Test Issuer",
"description": "Issuer served over http",
"issuer": "http://accounts.test.com"
}"#;

let err = IssuerParser::parse_issuer(content, "http-issuer.json")
.expect_err("An OIDC issuer identifier must use https");

assert!(
matches!(
&err,
PolicyStoreError::TrustedIssuerError {
err: TrustedIssuerErrorType::InvalidIssuerUrl { url, .. },
..
} if url == "http://accounts.test.com"
),
"Expected InvalidIssuerUrl for `http://accounts.test.com`, got: {err:?}"
);
}

#[test]
fn test_parse_issuer_rejects_issuer_with_query() {
let content = r#"{
"name": "Test Issuer",
"description": "Issuer carrying a query",
"issuer": "https://accounts.test.com/realms?tenant=a"
}"#;

let err = IssuerParser::parse_issuer(content, "query-issuer.json")
.expect_err("An issuer carrying a query cannot identify an OIDC provider");

assert!(
matches!(
&err,
PolicyStoreError::TrustedIssuerError {
err: TrustedIssuerErrorType::InvalidIssuerUrl { url, .. },
..
} if url == "https://accounts.test.com/realms?tenant=a"
),
"Expected InvalidIssuerUrl for `https://accounts.test.com/realms?tenant=a`, got: {err:?}"
);
}

#[test]
fn test_parse_issuer_rejects_issuer_with_fragment() {
let content = r#"{
"name": "Test Issuer",
"description": "Issuer carrying a fragment",
"issuer": "https://accounts.test.com/realms#frag"
}"#;

let err = IssuerParser::parse_issuer(content, "fragment-issuer.json")
.expect_err("An issuer carrying a fragment cannot identify an OIDC provider");

assert!(
matches!(
&err,
PolicyStoreError::TrustedIssuerError {
err: TrustedIssuerErrorType::InvalidIssuerUrl { url, .. },
..
} if url == "https://accounts.test.com/realms#frag"
),
"Expected InvalidIssuerUrl for `https://accounts.test.com/realms#frag`, got: {err:?}"
);
}

#[test]
fn test_parse_issuer_rejects_null_endpoint() {
let content = r#"{
"name": "Test Issuer",
"description": "Endpoint is present but null",
"issuer": "https://accounts.test.com",
"openid_configuration_endpoint": null
}"#;

let err = IssuerParser::parse_issuer(content, "null-endpoint.json")
.expect_err("A present but null endpoint must not fall back to the issuer");

assert!(
matches!(
&err,
PolicyStoreError::TrustedIssuerError {
err: TrustedIssuerErrorType::InvalidOidcEndpoint { reason, .. },
..
} if reason == "must be a string"
),
"Expected InvalidOidcEndpoint for a null endpoint, got: {err:?}"
);
}

#[test]
fn test_parse_issuer_rejects_numeric_endpoint() {
let content = r#"{
"name": "Test Issuer",
"description": "Endpoint is present but numeric",
"issuer": "https://accounts.test.com",
"openid_configuration_endpoint": 42
}"#;

let err = IssuerParser::parse_issuer(content, "numeric-endpoint.json")
.expect_err("A present but numeric endpoint must not fall back to the issuer");

assert!(
matches!(
&err,
PolicyStoreError::TrustedIssuerError {
err: TrustedIssuerErrorType::InvalidOidcEndpoint { reason, .. },
..
} if reason == "must be a string"
),
"Expected InvalidOidcEndpoint for a numeric endpoint, got: {err:?}"
);
}

#[test]
fn test_parse_issuer_invalid_issuer_url() {
let content = r#"{
"name": "Test",
"description": "Invalid issuer",
"issuer": "not a valid url"
}"#;

let result = IssuerParser::parse_issuer(content, "bad.json");
let err = result.expect_err("Should fail on invalid issuer url");

assert!(
matches!(
&err,
PolicyStoreError::TrustedIssuerError {
file,
err: TrustedIssuerErrorType::InvalidIssuerUrl { issuer_id, url, .. }
} if file == "bad.json" && issuer_id == "bad" && url == "not a valid url"
),
"Expected InvalidIssuerUrl error, got: {err:?}"
);
}

#[test]
fn test_parse_issuer_invalid_url() {
let content = r#"{
Expand Down
Loading