Skip to content

Commit

Permalink
crypto: fix OIDC cross-signing reset flows after backend authorizatio…
Browse files Browse the repository at this point in the history
…n failure response change (#3933)
  • Loading branch information
stefanceriu authored Sep 3, 2024
1 parent a737421 commit 5b14fe6
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 83 deletions.
5 changes: 1 addition & 4 deletions bindings/matrix-sdk-ffi/src/encryption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,15 +462,12 @@ impl From<&matrix_sdk::encryption::CrossSigningResetAuthType> for CrossSigningRe

#[derive(uniffi::Record)]
pub struct OidcCrossSigningResetInfo {
/// The error message we received from the homeserver after we attempted to
/// reset the cross-signing keys.
pub error: String,
/// The URL where the user can approve the reset of the cross-signing keys.
pub approval_url: String,
}

impl From<&matrix_sdk::encryption::OidcCrossSigningResetInfo> for OidcCrossSigningResetInfo {
fn from(value: &matrix_sdk::encryption::OidcCrossSigningResetInfo) -> Self {
Self { error: value.error.to_owned(), approval_url: value.approval_url.to_string() }
Self { approval_url: value.approval_url.to_string() }
}
}
80 changes: 42 additions & 38 deletions crates/matrix-sdk/src/encryption/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ use matrix_sdk_base::crypto::{
use matrix_sdk_common::executor::spawn;
use ruma::{
api::client::{
error::{ErrorBody, ErrorKind},
keys::{
get_keys, upload_keys, upload_signatures::v3::Request as UploadSignaturesRequest,
upload_signing_keys::v3::Request as UploadSigningKeysRequest,
Expand All @@ -57,6 +56,7 @@ use ruma::{
},
DeviceId, OwnedDeviceId, OwnedUserId, TransactionId, UserId,
};
use serde::Deserialize;
use tokio::sync::{Mutex, RwLockReadGuard};
use tracing::{debug, error, instrument, trace, warn};
use url::Url;
Expand Down Expand Up @@ -279,13 +279,12 @@ impl CrossSigningResetHandle {
let mut upload_request = self.upload_request.clone();
upload_request.auth = auth;

// TODO: Do we want to put a limit on this infinite loop? 🤷
while let Err(e) = self.client.send(upload_request.clone(), None).await {
if *self.is_cancelled.lock().await {
return Ok(());
}

if e.client_api_error_kind() != Some(&ErrorKind::Unrecognized) {
if e.as_uiaa_response().is_none() {
return Err(e.into());
}
}
Expand Down Expand Up @@ -313,15 +312,22 @@ pub enum CrossSigningResetAuthType {
}

impl CrossSigningResetAuthType {
async fn new(client: &Client, error: &HttpError) -> Result<Option<Self>> {
#[allow(clippy::unused_async)]
async fn new(
#[allow(unused_variables)] client: &Client,
error: &HttpError,
) -> Result<Option<Self>> {
if let Some(auth_info) = error.as_uiaa_response() {
#[cfg(feature = "experimental-oidc")]
if client.oidc().issuer().is_some() {
OidcCrossSigningResetInfo::from_auth_info(client, auth_info)
.map(|t| Some(CrossSigningResetAuthType::Oidc(t)))
} else {
Ok(Some(CrossSigningResetAuthType::Uiaa(auth_info.clone())))
}

#[cfg(not(feature = "experimental-oidc"))]
Ok(Some(CrossSigningResetAuthType::Uiaa(auth_info.clone())))
} else if let Some(ErrorBody::Standard { kind, message }) =
error.as_client_api_error().map(|e| &e.body)
{
OidcCrossSigningResetInfo::from_matrix_error(client, kind, message)
.await
.map(|t| t.map(CrossSigningResetAuthType::Oidc))
} else {
Ok(None)
}
Expand All @@ -330,45 +336,43 @@ impl CrossSigningResetAuthType {

/// OIDC specific information about the required authentication for the upload
/// of cross-signing keys.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Deserialize)]
pub struct OidcCrossSigningResetInfo {
/// The error message we received from the homeserver after we attempted to
/// reset the cross-signing keys.
pub error: String,
/// The URL where the user can approve the reset of the cross-signing keys.
pub approval_url: Url,
}

impl OidcCrossSigningResetInfo {
#[allow(clippy::unused_async)]
async fn from_matrix_error(
#[cfg(feature = "experimental-oidc")]
fn from_auth_info(
// This is used if the OIDC feature is enabled.
#[allow(unused_variables)] client: &Client,
kind: &ErrorKind,
message: &str,
) -> Result<Option<Self>> {
#[cfg(feature = "experimental-oidc")]
use mas_oidc_client::requests::account_management::AccountManagementActionFull;
auth_info: &UiaaInfo,
) -> Result<Self> {
let parameters =
serde_json::from_str::<OidcCrossSigningResetUiaaParameters>(auth_info.params.get())?;

if kind == &ErrorKind::Unrecognized {
#[cfg(feature = "experimental-oidc")]
let approval_url = client
.oidc()
.account_management_url(Some(AccountManagementActionFull::CrossSigningReset))
.await?;
Ok(OidcCrossSigningResetInfo { approval_url: parameters.reset.url })
}
}

#[cfg(not(feature = "experimental-oidc"))]
let approval_url = None;
/// The parsed `parameters` part of a [`ruma::api::client::uiaa::UiaaInfo`]
/// response
#[cfg(feature = "experimental-oidc")]
#[derive(Debug, Deserialize)]
struct OidcCrossSigningResetUiaaParameters {
/// The URL where the user can approve the reset of the cross-signing keys.
#[serde(rename = "org.matrix.cross_signing_reset")]
reset: OidcCrossSigningResetUiaaResetParameter,
}

if let Some(approval_url) = approval_url {
Ok(Some(OidcCrossSigningResetInfo { error: message.to_owned(), approval_url }))
} else {
Ok(None)
}
} else {
Ok(None)
}
}
/// The `org.matrix.cross_signing_reset` part of the Uiaa response `parameters``
/// dictionary.
#[cfg(feature = "experimental-oidc")]
#[derive(Debug, Deserialize)]
struct OidcCrossSigningResetUiaaResetParameter {
/// The URL where the user can approve the reset of the cross-signing keys.
url: Url,
}

impl Client {
Expand Down
55 changes: 14 additions & 41 deletions crates/matrix-sdk/tests/integration/encryption/cross_signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,32 +141,6 @@ async fn test_reset_oidc() {

let (client, server) = no_retry_test_client_with_server().await;

let auth_issuer_body = json!({
"issuer": server.uri(),
"authorization_endpoint": format!("{}/authorize", server.uri()),
"token_endpoint": format!("{}/oauth2/token", server.uri()),
"jwks_uri": format!("{}/oauth2/keys.json", server.uri()),
"response_types_supported": [
"code",
],
"response_modes_supported": [
"fragment"
],
"subject_types_supported": [
"public"
],
"id_token_signing_alg_values_supported": [
"RS256",
],
"claim_types_supported": [
"normal"
],
"account_management_uri": format!("{}/account/", server.uri()),
"account_management_actions_supported": [
"org.matrix.cross_signing_reset"
]
});

pub fn mock_registered_client_data() -> (ClientCredentials, VerifiedClientMetadata) {
(
ClientCredentials::None { client_id: CLIENT_ID.to_owned() },
Expand Down Expand Up @@ -227,21 +201,23 @@ async fn test_reset_oidc() {
.mount(&server)
.await;

Mock::given(method("GET"))
.and(path(".well-known/openid-configuration"))
.respond_with(ResponseTemplate::new(200).set_body_json(auth_issuer_body))
.expect(1)
.named("Auth issuer discovery")
.mount(&server)
.await;
let uiaa_response_body = json!({
"session": "dummy",
"flows": [{
"stages": [ "org.matrix.cross_signing_reset" ]
}],
"params": {
"org.matrix.cross_signing_reset": {
"url": format!("{}/account/?action=org.matrix.cross_signing_reset", server.uri())
}
},
"msg": "To reset your end-to-end encryption cross-signing identity, you first need to approve it and then try again."
});

let reset_handle = {
let _guard = Mock::given(method("POST"))
.and(path("/_matrix/client/unstable/keys/device_signing/upload"))
.respond_with(ResponseTemplate::new(501).set_body_json(json!({
"errcode": "M_UNRECOGNIZED",
"error": "To reset your cross-signing keys you first need to approve it in your auth issuer settings",
})))
.respond_with(ResponseTemplate::new(401).set_body_json(uiaa_response_body.clone()))
.expect(1)
.named("Initial cross-signing upload attempt")
.mount_as_scoped(&server)
Expand Down Expand Up @@ -279,10 +255,7 @@ async fn test_reset_oidc() {
if current_value >= 4 {
ResponseTemplate::new(200).set_body_json(json!({}))
} else {
ResponseTemplate::new(501).set_body_json(json!({
"errcode": "M_UNRECOGNIZED",
"error": "",
}))
ResponseTemplate::new(401).set_body_json(uiaa_response_body.clone())
}
}
})
Expand Down

0 comments on commit 5b14fe6

Please sign in to comment.