Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error messages in 5xx responces are hidden #272

Merged
merged 6 commits into from
Aug 21, 2023
Merged
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
37 changes: 21 additions & 16 deletions mpc-recovery/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,25 @@ impl MpcError {
Self::SignNodeRejection(error) => error.code(),
}
}

pub fn safe_error_message(&self) -> String {
if self.status().is_server_error() {
"Internal Server Error: Unexpected issue occurred. The backend team was notified."
.to_string()
} else {
match self {
Self::JsonExtractorRejection(json_rejection) => json_rejection.body_text(),
Self::LeaderNodeRejection(error) => error.to_string(),
Self::SignNodeRejection(error) => error.to_string(),
}
}
}
}

// We implement `IntoResponse` so MpcError can be used as a response
impl axum::response::IntoResponse for MpcError {
fn into_response(self) -> Response {
let (status, message) = match self {
Self::JsonExtractorRejection(json_rejection) => {
(json_rejection.status(), json_rejection.body_text())
}
Self::LeaderNodeRejection(error) => (error.code(), error.to_string()),
Self::SignNodeRejection(error) => (error.code(), error.to_string()),
};

(status, axum::Json(message)).into_response()
(self.status(), axum::Json(self.safe_error_message())).into_response()
}
}

Expand Down Expand Up @@ -87,14 +92,14 @@ impl LeaderNodeError {
LeaderNodeError::SignatureVerificationFailed(_) => StatusCode::BAD_REQUEST,
LeaderNodeError::OidcVerificationFailed(_) => StatusCode::UNAUTHORIZED,
LeaderNodeError::MalformedAccountId(_, _) => StatusCode::BAD_REQUEST,
LeaderNodeError::RelayerError(_) => StatusCode::INTERNAL_SERVER_ERROR,
LeaderNodeError::RelayerError(_) => StatusCode::FAILED_DEPENDENCY,
LeaderNodeError::TimeoutGatheringPublicKeys => StatusCode::INTERNAL_SERVER_ERROR,
LeaderNodeError::RecoveryKeyCanNotBeDeleted(_) => StatusCode::BAD_REQUEST,
LeaderNodeError::FailedToRetrieveRecoveryPk(_) => StatusCode::UNAUTHORIZED,
LeaderNodeError::Other(_) => StatusCode::INTERNAL_SERVER_ERROR,
LeaderNodeError::NetworkRejection(err) => {
err.status().unwrap_or(StatusCode::INTERNAL_SERVER_ERROR)
err.status().unwrap_or(StatusCode::REQUEST_TIMEOUT)
}
LeaderNodeError::Other(_) => StatusCode::INTERNAL_SERVER_ERROR,
}
}
}
Expand All @@ -106,7 +111,7 @@ pub enum SignNodeError {
#[error("malformed public key {0}: {1}")]
MalformedPublicKey(String, ParseKeyError),
#[error("failed to verify signature: {0}")]
SignatureVerificationFailed(anyhow::Error),
DigestSignatureVerificationFailed(anyhow::Error),
#[error("failed to verify oidc token: {0}")]
OidcVerificationFailed(anyhow::Error),
#[error("oidc token {0:?} already claimed with another key")]
Expand All @@ -127,10 +132,10 @@ impl SignNodeError {
pub fn code(&self) -> StatusCode {
match self {
// TODO: this case was not speicifically handled before. Check if it is the right code
Self::MalformedAccountId(_, _) => StatusCode::INTERNAL_SERVER_ERROR,
Self::MalformedAccountId(_, _) => StatusCode::BAD_REQUEST,
Self::MalformedPublicKey(_, _) => StatusCode::BAD_REQUEST,
Self::SignatureVerificationFailed(_) => StatusCode::BAD_REQUEST,
Self::OidcVerificationFailed(_) => StatusCode::UNAUTHORIZED,
Self::DigestSignatureVerificationFailed(_) => StatusCode::UNAUTHORIZED,
Self::OidcVerificationFailed(_) => StatusCode::BAD_REQUEST,
Self::OidcTokenAlreadyClaimed(_) => StatusCode::UNAUTHORIZED,
Self::OidcTokenClaimedWithAnotherKey(_) => StatusCode::UNAUTHORIZED,
Self::OidcTokenNotClaimed(_) => StatusCode::UNAUTHORIZED,
Expand Down
6 changes: 3 additions & 3 deletions mpc-recovery/src/sign_node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ async fn process_commit<T: OAuthTokenVerifier>(

match check_digest_signature(&public_key, &request.signature, &request_digest) {
Ok(()) => tracing::debug!("claim oidc token digest signature verified"),
Err(e) => return Err(SignNodeError::SignatureVerificationFailed(e)),
Err(e) => return Err(SignNodeError::DigestSignatureVerificationFailed(e)),
};

// Save info about token in the database, if it's present, throw an error
Expand Down Expand Up @@ -226,7 +226,7 @@ async fn process_commit<T: OAuthTokenVerifier>(
sign_request_digest(&request.delegate_action, &request.oidc_token, &frp_pk)?;
match check_digest_signature(&frp_pk, &request.frp_signature, &digest) {
Ok(()) => tracing::debug!("sign request digest signature verified"),
Err(e) => return Err(SignNodeError::SignatureVerificationFailed(e)),
Err(e) => return Err(SignNodeError::DigestSignatureVerificationFailed(e)),
};

// Check if this OIDC token was claimed
Expand Down Expand Up @@ -403,7 +403,7 @@ async fn process_public_key<T: OAuthTokenVerifier>(
let digest = user_credentials_request_digest(&request.oidc_token, &frp_pk)?;
match check_digest_signature(&frp_pk, &request.frp_signature, &digest) {
Ok(()) => tracing::debug!("user credentials digest signature verified"),
Err(e) => return Err(SignNodeError::SignatureVerificationFailed(e)),
Err(e) => return Err(SignNodeError::DigestSignatureVerificationFailed(e)),
};

// Check if this OIDC token was claimed
Expand Down