diff --git a/crates/handlers/src/oauth2/discovery.rs b/crates/handlers/src/oauth2/discovery.rs index 2c5853f7..a79b5c45 100644 --- a/crates/handlers/src/oauth2/discovery.rs +++ b/crates/handlers/src/oauth2/discovery.rs @@ -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()); diff --git a/crates/oauth2-types/src/oidc.rs b/crates/oauth2-types/src/oidc.rs index 002acd91..b856bfef 100644 --- a/crates/oauth2-types/src/oidc.rs +++ b/crates/oauth2-types/src/oidc.rs @@ -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, + pub issuer: Option, /// 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 { 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(); } diff --git a/crates/oidc-client/src/requests/discovery.rs b/crates/oidc-client/src/requests/discovery.rs index c62ef742..6a185078 100644 --- a/crates/oidc-client/src/requests/discovery.rs +++ b/crates/oidc-client/src/requests/discovery.rs @@ -31,11 +31,11 @@ use crate::{ /// Fetch the provider metadata. async fn discover_inner( http_service: &HttpService, - issuer: &Url, + issuer: Url, ) -> Result { 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 { - 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 { - 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()?) } diff --git a/crates/oidc-client/src/requests/jose.rs b/crates/oidc-client/src/requests/jose.rs index 081f5b23..03a5018d 100644 --- a/crates/oidc-client/src/requests/jose.rs +++ b/crates/oidc-client/src/requests/jose.rs @@ -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)?; diff --git a/crates/oidc-client/tests/it/main.rs b/crates/oidc-client/tests/it/main.rs index ce5da2e3..7b27876b 100644 --- a/crates/oidc-client/tests/it/main.rs +++ b/crates/oidc-client/tests/it/main.rs @@ -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); diff --git a/crates/oidc-client/tests/it/requests/authorization_code.rs b/crates/oidc-client/tests/it/requests/authorization_code.rs index caa1bc88..52dea447 100644 --- a/crates/oidc-client/tests/it/requests/authorization_code.rs +++ b/crates/oidc-client/tests/it/requests/authorization_code.rs @@ -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, diff --git a/crates/oidc-client/tests/it/requests/discovery.rs b/crates/oidc-client/tests/it/requests/discovery.rs index bc575601..f90e3bc7 100644 --- a/crates/oidc-client/tests/it/requests/discovery.rs +++ b/crates/oidc-client/tests/it/requests/discovery.rs @@ -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(_)); } diff --git a/crates/oidc-client/tests/it/requests/jose.rs b/crates/oidc-client/tests/it/requests/jose.rs index f3950b83..e21edb8a 100644 --- a/crates/oidc-client/tests/it/requests/jose.rs +++ b/crates/oidc-client/tests/it/requests/jose.rs @@ -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, auth_time: Option>, ) -> (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, diff --git a/crates/oidc-client/tests/it/requests/userinfo.rs b/crates/oidc-client/tests/it/requests/userinfo.rs index 7bd720a1..11c56506 100644 --- a/crates/oidc-client/tests/it/requests/userinfo.rs +++ b/crates/oidc-client/tests/it/requests/userinfo.rs @@ -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"))