1
0
mirror of https://github.com/matrix-org/matrix-authentication-service.git synced 2025-07-29 22:01:14 +03:00

Make the OIDC issuer a string instead of a URL

This commit is contained in:
Quentin Gliech
2022-12-02 15:38:53 +01:00
parent 68b477cae1
commit 95a879585b
9 changed files with 80 additions and 65 deletions

View File

@ -47,7 +47,7 @@ pub(crate) async fn get(
let jwt_signing_alg_values_supported = Some(key_store.available_signing_algorithms());
// Prepare all the endpoints
let issuer = Some(url_builder.oidc_issuer());
let issuer = Some(url_builder.oidc_issuer().into());
let authorization_endpoint = Some(url_builder.oauth_authorization_endpoint());
let token_endpoint = Some(url_builder.oauth_token_endpoint());
let jwks_uri = Some(url_builder.jwks_uri());

View File

@ -190,7 +190,7 @@ pub struct ProviderMetadata {
/// This field is required. The URL must use a `https` scheme, and must not
/// contain a query or fragment. It must match the one used to build the
/// well-known URI to query this metadata.
pub issuer: Option<Url>,
pub issuer: Option<String>,
/// URL of the authorization server's [authorization endpoint].
///
@ -489,7 +489,7 @@ impl ProviderMetadata {
/// [OpenID Connect Discovery Spec 1.0]: https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata
pub fn validate(
self,
issuer: &Url,
issuer: &str,
) -> Result<VerifiedProviderMetadata, ProviderMetadataVerificationError> {
let metadata = self.insecure_verify_metadata()?;
@ -499,7 +499,10 @@ impl ProviderMetadata {
validate_url(
"issuer",
metadata.issuer(),
&metadata
.issuer()
.parse()
.map_err(|_| ProviderMetadataVerificationError::IssuerNotUrl)?,
ExtraUrlRestrictions::NoQueryOrFragment,
)?;
@ -784,7 +787,7 @@ impl ProviderMetadata {
/// use url::Url;
/// # use oauth2_types::oidc::{ProviderMetadata, ProviderMetadataVerificationError};
/// # let metadata = ProviderMetadata::default();
/// # let issuer = Url::parse("http://localhost").unwrap();
/// # let issuer = "http://localhost/";
/// let verified_metadata = metadata.validate(&issuer)?;
///
/// // The endpoint is required during validation so this is not an `Option`.
@ -809,7 +812,7 @@ pub struct VerifiedProviderMetadata {
impl VerifiedProviderMetadata {
/// Authorization server's issuer identifier URL.
#[must_use]
pub fn issuer(&self) -> &Url {
pub fn issuer(&self) -> &str {
match &self.issuer {
Some(u) => u,
None => unreachable!(),
@ -889,6 +892,10 @@ pub enum ProviderMetadataVerificationError {
#[error("issuer is missing")]
MissingIssuer,
/// The issuer is not a valid URL.
#[error("issuer is not a valid URL")]
IssuerNotUrl,
/// The authorization endpoint is missing.
#[error("authorization endpoint is missing")]
MissingAuthorizationEndpoint,
@ -1074,8 +1081,8 @@ mod tests {
use super::*;
fn valid_provider_metadata() -> (ProviderMetadata, Url) {
let issuer = Url::parse("https://localhost").unwrap();
fn valid_provider_metadata() -> (ProviderMetadata, String) {
let issuer = "https://localhost".to_owned();
let metadata = ProviderMetadata {
issuer: Some(issuer.clone()),
authorization_endpoint: Some(Url::parse("https://localhost/auth").unwrap()),
@ -1109,45 +1116,52 @@ mod tests {
Err(ProviderMetadataVerificationError::MissingIssuer)
);
// Err - Not an url
metadata.issuer = Some("not-an-url".to_owned());
assert_matches!(
metadata.clone().validate("not-an-url"),
Err(ProviderMetadataVerificationError::IssuerNotUrl)
);
// Err - Wrong issuer
metadata.issuer = Some(Url::parse("https://example.com").unwrap());
metadata.issuer = Some("https://example.com/".to_owned());
assert_matches!(
metadata.clone().validate(&issuer),
Err(ProviderMetadataVerificationError::IssuerUrlsDontMatch)
);
// Err - Not https
let issuer = Url::parse("http://localhost").unwrap();
let issuer = "http://localhost/".to_owned();
metadata.issuer = Some(issuer.clone());
let (field, url) = assert_matches!(
metadata.clone().validate(&issuer),
Err(ProviderMetadataVerificationError::UrlNonHttpsScheme(field, url)) => (field, url)
);
assert_eq!(field, "issuer");
assert_eq!(url, issuer);
assert_eq!(url.as_str(), issuer);
// Err - Query
let issuer = Url::parse("https://localhost/?query").unwrap();
let issuer = "https://localhost/?query".to_owned();
metadata.issuer = Some(issuer.clone());
let (field, url) = assert_matches!(
metadata.clone().validate(&issuer),
Err(ProviderMetadataVerificationError::UrlWithQuery(field, url)) => (field, url)
);
assert_eq!(field, "issuer");
assert_eq!(url, issuer);
assert_eq!(url.as_str(), issuer);
// Err - Fragment
let issuer = Url::parse("https://localhost/#fragment").unwrap();
let issuer = "https://localhost/#fragment".to_owned();
metadata.issuer = Some(issuer.clone());
let (field, url) = assert_matches!(
metadata.clone().validate(&issuer),
Err(ProviderMetadataVerificationError::UrlWithFragment(field, url)) => (field, url)
);
assert_eq!(field, "issuer");
assert_eq!(url, issuer);
assert_eq!(url.as_str(), issuer);
// Ok - Path
let issuer = Url::parse("https://localhost/issuer1").unwrap();
let issuer = "https://localhost/issuer1".to_owned();
metadata.issuer = Some(issuer.clone());
metadata.validate(&issuer).unwrap();
}

View File

@ -31,11 +31,11 @@ use crate::{
/// Fetch the provider metadata.
async fn discover_inner(
http_service: &HttpService,
issuer: &Url,
issuer: Url,
) -> Result<ProviderMetadata, DiscoveryError> {
tracing::debug!("Fetching provider metadata...");
let mut config_url = issuer.clone();
let mut config_url = issuer;
// If the path doesn't end with a slash, the last segment is removed when
// using `join`.
@ -69,9 +69,9 @@ async fn discover_inner(
#[tracing::instrument(skip_all, fields(issuer))]
pub async fn discover(
http_service: &HttpService,
issuer: &Url,
issuer: &str,
) -> Result<VerifiedProviderMetadata, DiscoveryError> {
let provider_metadata = discover_inner(http_service, issuer).await?;
let provider_metadata = discover_inner(http_service, issuer.parse()?).await?;
Ok(provider_metadata.validate(issuer)?)
}
@ -101,9 +101,9 @@ pub async fn discover(
#[tracing::instrument(skip_all, fields(issuer))]
pub async fn insecure_discover(
http_service: &HttpService,
issuer: &Url,
issuer: &str,
) -> Result<VerifiedProviderMetadata, DiscoveryError> {
let provider_metadata = discover_inner(http_service, issuer).await?;
let provider_metadata = discover_inner(http_service, issuer.parse()?).await?;
Ok(provider_metadata.insecure_verify_metadata()?)
}

View File

@ -66,7 +66,7 @@ pub async fn fetch_jwks(
#[derive(Clone, Copy)]
pub struct JwtVerificationData<'a> {
/// The URL of the issuer that generated the ID Token.
pub issuer: &'a Url,
pub issuer: &'a str,
/// The issuer's JWKS.
pub jwks: &'a PublicJsonWebKeySet,
@ -127,7 +127,7 @@ pub fn verify_signed_jwt<'a>(
let (header, mut claims) = jwt.clone().into_parts();
// Must have the proper issuer.
claims::ISS.extract_required_with_options(&mut claims, issuer.as_str())?;
claims::ISS.extract_required_with_options(&mut claims, issuer)?;
// Must have the proper audience.
claims::AUD.extract_required_with_options(&mut claims, client_id)?;

View File

@ -87,7 +87,7 @@ fn keystore(alg: &JsonWebSignatureAlg) -> Keystore {
}
/// Generate an ID token.
fn id_token(issuer: &Url) -> (IdToken, PublicJsonWebKeySet) {
fn id_token(issuer: &str) -> (IdToken, PublicJsonWebKeySet) {
let signing_alg = ID_TOKEN_SIGNING_ALG;
let keystore = keystore(&signing_alg);

View File

@ -262,9 +262,9 @@ async fn pass_access_token_with_authorization_code() {
code_challenge_verifier: Some(CODE_VERIFIER.to_owned()),
};
let (id_token, jwks) = id_token(&issuer);
let (id_token, jwks) = id_token(issuer.as_str());
let id_token_verification_data = JwtVerificationData {
issuer: &issuer,
issuer: issuer.as_str(),
jwks: &jwks,
client_id: &CLIENT_ID.to_owned(),
signing_algorithm: &ID_TOKEN_SIGNING_ALG,
@ -321,9 +321,9 @@ async fn fail_access_token_with_authorization_code_wrong_nonce() {
code_challenge_verifier: Some(CODE_VERIFIER.to_owned()),
};
let (id_token, jwks) = id_token(&issuer);
let (id_token, jwks) = id_token(issuer.as_str());
let id_token_verification_data = JwtVerificationData {
issuer: &issuer,
issuer: issuer.as_str(),
jwks: &jwks,
client_id: &CLIENT_ID.to_owned(),
signing_algorithm: &ID_TOKEN_SIGNING_ALG,
@ -385,7 +385,7 @@ async fn fail_access_token_with_authorization_code_no_id_token() {
};
let id_token_verification_data = JwtVerificationData {
issuer: &issuer,
issuer: issuer.as_str(),
jwks: &PublicJsonWebKeySet::default(),
client_id: &CLIENT_ID.to_owned(),
signing_algorithm: &ID_TOKEN_SIGNING_ALG,

View File

@ -30,7 +30,7 @@ use crate::init_test;
fn provider_metadata(issuer: &Url) -> ProviderMetadata {
ProviderMetadata {
issuer: Some(issuer.clone()),
issuer: Some(issuer.as_str().to_owned()),
authorization_endpoint: issuer.join("authorize").ok(),
token_endpoint: issuer.join("token").ok(),
jwks_uri: issuer.join("jwks").ok(),
@ -52,16 +52,18 @@ async fn pass_discover() {
.mount(&mock_server)
.await;
let provider_metadata = insecure_discover(&http_service, &issuer).await.unwrap();
let provider_metadata = insecure_discover(&http_service, issuer.as_str())
.await
.unwrap();
assert_eq!(*provider_metadata.issuer(), issuer);
assert_eq!(provider_metadata.issuer(), issuer.as_str());
}
#[tokio::test]
async fn fail_discover_404() {
let (http_service, _mock_server, issuer) = init_test().await;
let error = discover(&http_service, &issuer).await.unwrap_err();
let error = discover(&http_service, issuer.as_str()).await.unwrap_err();
assert_matches!(error, DiscoveryError::Http(_));
}
@ -76,7 +78,7 @@ async fn fail_discover_not_json() {
.mount(&mock_server)
.await;
let error = discover(&http_service, &issuer).await.unwrap_err();
let error = discover(&http_service, issuer.as_str()).await.unwrap_err();
assert_matches!(error, DiscoveryError::FromJson(_));
}
@ -91,7 +93,7 @@ async fn fail_discover_invalid_metadata() {
.mount(&mock_server)
.await;
let error = discover(&http_service, &issuer).await.unwrap_err();
let error = discover(&http_service, issuer.as_str()).await.unwrap_err();
assert_matches!(error, DiscoveryError::Validation(_));
}

View File

@ -28,7 +28,6 @@ use mas_oidc_client::{
requests::jose::{verify_id_token, JwtVerificationData},
types::IdToken,
};
use url::Url;
use crate::{keystore, now, CLIENT_ID, ID_TOKEN_SIGNING_ALG, SUBJECT_IDENTIFIER};
@ -40,7 +39,7 @@ enum IdTokenFlag {
/// Generate an ID token with the given settings.
fn id_token(
issuer: &Url,
issuer: &str,
flag: Option<IdTokenFlag>,
auth_time: Option<DateTime<Utc>>,
) -> (IdToken, PublicJsonWebKeySet) {
@ -91,13 +90,13 @@ fn id_token(
#[tokio::test]
async fn pass_verify_id_token() {
let issuer = Url::parse("http://localhost/").unwrap();
let issuer = "http://localhost/";
let now = now();
let (auth_id_token, _) = id_token(&issuer, None, Some(now));
let (id_token, jwks) = id_token(&issuer, None, Some(now));
let (auth_id_token, _) = id_token(issuer, None, Some(now));
let (id_token, jwks) = id_token(issuer, None, Some(now));
let verification_data = JwtVerificationData {
issuer: &issuer,
issuer,
jwks: &jwks,
client_id: &CLIENT_ID.to_owned(),
signing_algorithm: &ID_TOKEN_SIGNING_ALG,
@ -114,13 +113,13 @@ async fn pass_verify_id_token() {
#[tokio::test]
async fn fail_verify_id_token_wrong_issuer() {
let issuer = Url::parse("http://localhost/").unwrap();
let wrong_issuer = Url::parse("http://distanthost/").unwrap();
let (id_token, jwks) = id_token(&issuer, None, None);
let issuer = "http://localhost/";
let wrong_issuer = "http://distanthost/";
let (id_token, jwks) = id_token(issuer, None, None);
let now = now();
let verification_data = JwtVerificationData {
issuer: &wrong_issuer,
issuer: wrong_issuer,
jwks: &jwks,
client_id: &CLIENT_ID.to_owned(),
signing_algorithm: &ID_TOKEN_SIGNING_ALG,
@ -139,12 +138,12 @@ async fn fail_verify_id_token_wrong_issuer() {
#[tokio::test]
async fn fail_verify_id_token_wrong_audience() {
let issuer = Url::parse("http://localhost/").unwrap();
let (id_token, jwks) = id_token(&issuer, None, None);
let issuer = "http://localhost/";
let (id_token, jwks) = id_token(issuer, None, None);
let now = now();
let verification_data = JwtVerificationData {
issuer: &issuer,
issuer,
jwks: &jwks,
client_id: &"wrong_client_id".to_owned(),
signing_algorithm: &ID_TOKEN_SIGNING_ALG,
@ -163,12 +162,12 @@ async fn fail_verify_id_token_wrong_audience() {
#[tokio::test]
async fn fail_verify_id_token_wrong_signing_algorithm() {
let issuer = Url::parse("http://localhost/").unwrap();
let (id_token, jwks) = id_token(&issuer, None, None);
let issuer = "http://localhost/";
let (id_token, jwks) = id_token(issuer, None, None);
let now = now();
let verification_data = JwtVerificationData {
issuer: &issuer,
issuer,
jwks: &jwks,
client_id: &CLIENT_ID.to_owned(),
signing_algorithm: &JsonWebSignatureAlg::Unknown("wrong_algorithm".to_owned()),
@ -184,12 +183,12 @@ async fn fail_verify_id_token_wrong_signing_algorithm() {
#[tokio::test]
async fn fail_verify_id_token_wrong_expiration() {
let issuer = Url::parse("http://localhost/").unwrap();
let (id_token, jwks) = id_token(&issuer, Some(IdTokenFlag::WrongExpiration), None);
let issuer = "http://localhost/";
let (id_token, jwks) = id_token(issuer, Some(IdTokenFlag::WrongExpiration), None);
let now = now();
let verification_data = JwtVerificationData {
issuer: &issuer,
issuer,
jwks: &jwks,
client_id: &CLIENT_ID.to_owned(),
signing_algorithm: &ID_TOKEN_SIGNING_ALG,
@ -202,13 +201,13 @@ async fn fail_verify_id_token_wrong_expiration() {
#[tokio::test]
async fn fail_verify_id_token_wrong_subject() {
let issuer = Url::parse("http://localhost/").unwrap();
let issuer = "http://localhost/";
let now = now();
let (auth_id_token, _) = id_token(&issuer, None, Some(now));
let (id_token, jwks) = id_token(&issuer, Some(IdTokenFlag::WrongSubject), None);
let (auth_id_token, _) = id_token(issuer, None, Some(now));
let (id_token, jwks) = id_token(issuer, Some(IdTokenFlag::WrongSubject), None);
let verification_data = JwtVerificationData {
issuer: &issuer,
issuer,
jwks: &jwks,
client_id: &CLIENT_ID.to_owned(),
signing_algorithm: &ID_TOKEN_SIGNING_ALG,
@ -227,13 +226,13 @@ async fn fail_verify_id_token_wrong_subject() {
#[tokio::test]
async fn fail_verify_id_token_wrong_auth_time() {
let issuer = Url::parse("http://localhost/").unwrap();
let issuer = "http://localhost/";
let now = now();
let (auth_id_token, _) = id_token(&issuer, None, Some(now));
let (id_token, jwks) = id_token(&issuer, None, Some(now + Duration::hours(1)));
let (auth_id_token, _) = id_token(issuer, None, Some(now));
let (id_token, jwks) = id_token(issuer, None, Some(now + Duration::hours(1)));
let verification_data = JwtVerificationData {
issuer: &issuer,
issuer,
jwks: &jwks,
client_id: &CLIENT_ID.to_owned(),
signing_algorithm: &ID_TOKEN_SIGNING_ALG,

View File

@ -29,7 +29,7 @@ use crate::{id_token, init_test, ACCESS_TOKEN, SUBJECT_IDENTIFIER};
async fn pass_fetch_userinfo() {
let (http_service, mock_server, issuer) = init_test().await;
let userinfo_endpoint = issuer.join("userinfo").unwrap();
let (auth_id_token, _) = id_token(&issuer);
let (auth_id_token, _) = id_token(issuer.as_str());
Mock::given(method("GET"))
.and(path("/userinfo"))
@ -61,7 +61,7 @@ async fn pass_fetch_userinfo() {
async fn fail_wrong_subject_identifier() {
let (http_service, mock_server, issuer) = init_test().await;
let userinfo_endpoint = issuer.join("userinfo").unwrap();
let (auth_id_token, _) = id_token(&issuer);
let (auth_id_token, _) = id_token(issuer.as_str());
Mock::given(method("GET"))
.and(path("/userinfo"))