Skip to content

Commit

Permalink
oidc: allow http scheme during discovery (#2642)
Browse files Browse the repository at this point in the history
This is to enable entirely local stacks of Element X to work correctly. It's mostly seamless (no ffi changes) because it ties into `ClientBuilder.insecure_server_name_no_tls()`.

---------

Co-authored-by: Benjamin Bouvier <[email protected]>
  • Loading branch information
kegsay and bnjbvr authored Sep 29, 2023
1 parent 9cc9221 commit 58e15f8
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 12 deletions.
10 changes: 6 additions & 4 deletions crates/matrix-sdk/src/client/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ impl ClientBuilder {
}

/// Set the server name to discover the homeserver from, assuming an HTTP
/// (not secured) scheme.
/// (not secured) scheme. This also relaxes OIDC discovery checks to allow
/// HTTP schemes.
///
/// This method is mutually exclusive with
/// [`homeserver_url()`][Self::homeserver_url], if you set both whatever was
Expand Down Expand Up @@ -371,7 +372,7 @@ impl ClientBuilder {
let http_client = HttpClient::new(inner_http_client.clone(), self.request_config);

#[cfg(feature = "experimental-oidc")]
let mut authentication_server_info = None;
let (mut authentication_server_info, mut allow_insecure_oidc) = (None, false);

#[cfg(feature = "experimental-sliding-sync")]
let mut sliding_sync_proxy: Option<Url> = None;
Expand Down Expand Up @@ -411,6 +412,7 @@ impl ClientBuilder {
#[cfg(feature = "experimental-oidc")]
{
authentication_server_info = well_known.authentication;
allow_insecure_oidc = matches!(protocol, UrlScheme::Http);
}

#[cfg(feature = "experimental-sliding-sync")]
Expand All @@ -437,7 +439,7 @@ impl ClientBuilder {
reload_session_callback: OnceCell::default(),
save_session_callback: OnceCell::default(),
#[cfg(feature = "experimental-oidc")]
oidc: OidcCtx::new(authentication_server_info),
oidc: OidcCtx::new(authentication_server_info, allow_insecure_oidc),
});

let inner = Arc::new(ClientInner::new(
Expand All @@ -457,7 +459,7 @@ impl ClientBuilder {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Copy, Debug)]
enum UrlScheme {
Http,
Https,
Expand Down
6 changes: 5 additions & 1 deletion crates/matrix-sdk/src/oidc/backend/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,11 @@ impl MockImpl {

#[async_trait::async_trait]
impl OidcBackend for MockImpl {
async fn discover(&self, issuer: &str) -> Result<VerifiedProviderMetadata, OidcError> {
async fn discover(
&self,
issuer: &str,
_insecure: bool,
) -> Result<VerifiedProviderMetadata, OidcError> {
Ok(ProviderMetadata {
issuer: Some(self.issuer.clone()),
authorization_endpoint: Some(Url::parse(&self.authorization_endpoint).unwrap()),
Expand Down
6 changes: 5 additions & 1 deletion crates/matrix-sdk/src/oidc/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ pub(super) struct RefreshedSessionTokens {

#[async_trait::async_trait]
pub(super) trait OidcBackend: std::fmt::Debug + Send + Sync {
async fn discover(&self, issuer: &str) -> Result<VerifiedProviderMetadata, OidcError>;
async fn discover(
&self,
issuer: &str,
insecure: bool,
) -> Result<VerifiedProviderMetadata, OidcError>;

async fn register_client(
&self,
Expand Down
14 changes: 11 additions & 3 deletions crates/matrix-sdk/src/oidc/backend/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use mas_oidc_client::{
access_token_with_authorization_code, build_par_authorization_url,
AuthorizationRequestData, AuthorizationValidationData,
},
discovery::discover,
discovery::{discover, insecure_discover},
jose::{fetch_jwks, JwtVerificationData},
refresh_token::refresh_access_token,
registration::register_client,
Expand Down Expand Up @@ -71,8 +71,16 @@ impl OidcServer {

#[async_trait::async_trait]
impl OidcBackend for OidcServer {
async fn discover(&self, issuer: &str) -> Result<VerifiedProviderMetadata, OidcError> {
discover(&self.http_service(), issuer).await.map_err(Into::into)
async fn discover(
&self,
issuer: &str,
insecure: bool,
) -> Result<VerifiedProviderMetadata, OidcError> {
if insecure {
insecure_discover(&self.http_service(), issuer).await.map_err(Into::into)
} else {
discover(&self.http_service(), issuer).await.map_err(Into::into)
}
}

async fn trade_authorization_code_for_tokens(
Expand Down
13 changes: 10 additions & 3 deletions crates/matrix-sdk/src/oidc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,19 @@ pub(crate) struct OidcCtx {
/// Note: only required because we're using the crypto store that might not
/// be present before reloading a session.
deferred_cross_process_lock_init: Mutex<Option<String>>,

/// Whether to allow HTTP issuer URLs.
insecure_discover: bool,
}

impl OidcCtx {
pub(crate) fn new(authentication_server_info: Option<AuthenticationServerInfo>) -> Self {
pub(crate) fn new(
authentication_server_info: Option<AuthenticationServerInfo>,
insecure_discover: bool,
) -> Self {
Self {
authentication_server_info,
insecure_discover,
cross_process_token_refresh_manager: Default::default(),
deferred_cross_process_lock_init: Default::default(),
}
Expand Down Expand Up @@ -420,7 +427,7 @@ impl Oidc {
&self,
issuer: &str,
) -> Result<VerifiedProviderMetadata, OidcError> {
self.backend.discover(issuer).await
self.backend.discover(issuer, self.ctx().insecure_discover).await
}

/// Fetch the OpenID Connect metadata of the issuer.
Expand Down Expand Up @@ -639,7 +646,7 @@ impl Oidc {
client_metadata: VerifiedClientMetadata,
software_statement: Option<String>,
) -> Result<ClientRegistrationResponse, OidcError> {
let provider_metadata = self.backend.discover(issuer).await?;
let provider_metadata = self.backend.discover(issuer, self.ctx().insecure_discover).await?;

let registration_endpoint = provider_metadata
.registration_endpoint
Expand Down

0 comments on commit 58e15f8

Please sign in to comment.