mirrors/syncstorage-rs
1
0
Fork 0
mirror of https://github.com/mozilla-services/syncstorage-rs.git synced 2026-05-05 12:16:21 +02:00
Code Issues Actions 4 Packages Projects Releases Wiki Activity

bug: convert DbErrors to TokenserverErrors (#1327)

Browse Source 
Closes #1316
This commit is contained in:
Ethan Donowitz 2022-06-10 17:30:45 -04:00 committed by GitHub
parent 03c059cd9e
commit 9bea32803c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 201 additions and 156 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Cargo.lock generated
View File

@ -3270,8 +3270,10 @@ name = "tokenserver-common"
version = "0.10.2"
dependencies = [
"actix-web",
"backtrace",
"serde 1.0.135",
"serde_json",
"syncstorage-db-common",
"thiserror",
]

18
syncstorage/src/tokenserver/auth/browserid.rs
View File

@ -81,7 +81,7 @@ impl VerifyToken for RemoteVerifier {
"Unknown error occurred during BrowserID request to FxA: {}",
e
),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
}
}
})?;
@ -124,22 +124,22 @@ impl VerifyToken for RemoteVerifier {
reason: Some(reason),
} => Err(TokenserverError {
context: format!("BrowserID verification error: {}", reason),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
}),
VerifyResponse::Failure { .. } => Err(TokenserverError {
context: "Unknown BrowserID verification error".to_owned(),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
}),
VerifyResponse::Okay { issuer, .. } if issuer != self.issuer => Err(TokenserverError {
context: "BrowserID issuer mismatch".to_owned(),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
}),
VerifyResponse::Okay {
idp_claims: Some(claims),
..
} if !claims.token_verified() => Err(TokenserverError {
context: "BrowserID assertion not verified".to_owned(),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
}),
VerifyResponse::Okay {
email,
@ -238,7 +238,7 @@ impl IdpClaims {
Some(Some(_)) | None => Ok(self.keys_changed_at.flatten()),
// If the fxa-keysChangedAt claim is null, return an error.
Some(None) => Err(TokenserverError {
description: "invalid keysChangedAt",
description: "invalid keysChangedAt".to_owned(),
status: "invalid-credentials",
location: ErrorLocation::Body,
context: "null fxa-keysChangedAt claim in BrowserID assertion".to_owned(),
@ -407,7 +407,7 @@ mod tests {
let expected_error = TokenserverError {
context: "BrowserID verification error: something broke".to_owned(),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
};
assert_eq!(expected_error, error);
}
@ -423,7 +423,7 @@ mod tests {
let expected_error = TokenserverError {
context: "Unknown BrowserID verification error".to_owned(),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
};
assert_eq!(expected_error, error);
}
@ -450,7 +450,7 @@ mod tests {
let expected_error = TokenserverError {
context: "BrowserID issuer mismatch".to_owned(),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
};
let verifier = RemoteVerifier::try_from(&Settings {
fxa_browserid_audience: AUDIENCE.to_owned(),

17
syncstorage/src/tokenserver/auth/mod.rs
View File

@ -3,7 +3,6 @@ pub mod oauth;
use std::fmt;
use actix_web::Error;
use async_trait::async_trait;
use dyn_clone::{self, DynClone};
use pyo3::{
@ -13,8 +12,6 @@ use pyo3::{
use serde::{Deserialize, Serialize};
use tokenserver_common::error::TokenserverError;
use crate::error::{ApiError, ApiErrorKind};
/// Represents the origin of the token used by Sync clients to access their data.
#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
#[serde(rename_all = "lowercase")]
@ -82,7 +79,7 @@ impl Tokenlib {
pub fn get_token_and_derived_secret(
plaintext: MakeTokenPlaintext,
shared_secret: &str,
) -> Result<(String, String), Error> {
) -> Result<(String, String), TokenserverError> {
Python::with_gil(|py| {
// `import tokenlib`
let module = PyModule::import(py, "tokenlib").map_err(|e| {
@ -112,7 +109,7 @@ impl Tokenlib {
// `return (token, derived_secret)`
Ok((token, derived_secret))
})
.map_err(pyerr_to_actix_error)
.map_err(pyerr_to_tokenserver_error)
}
}
@ -142,11 +139,13 @@ impl<T: Clone + Send + Sync> VerifyToken for MockVerifier<T> {
async fn verify(&self, _token: String) -> Result<T, TokenserverError> {
self.valid
.then(|| self.verify_output.clone())
.ok_or_else(|| TokenserverError::invalid_credentials("Unauthorized"))
.ok_or_else(|| TokenserverError::invalid_credentials("Unauthorized".to_owned()))
}
}
fn pyerr_to_actix_error(e: PyErr) -> Error {
let api_error: ApiError = ApiErrorKind::Internal(e.to_string()).into();
api_error.into()
fn pyerr_to_tokenserver_error(e: PyErr) -> TokenserverError {
TokenserverError {
context: e.to_string(),
..TokenserverError::internal_error()
}
}

13
syncstorage/src/tokenserver/auth/oauth.rs
View File

@ -1,4 +1,3 @@
use actix_web::Error;
use async_trait::async_trait;
use futures::TryFutureExt;
use pyo3::{
@ -40,9 +39,9 @@ impl Verifier {
}
impl TryFrom<&Settings> for Verifier {
type Error = Error;
type Error = TokenserverError;
fn try_from(settings: &Settings) -> Result<Self, Error> {
fn try_from(settings: &Settings) -> Result<Self, TokenserverError> {
let inner: Py<PyAny> = Python::with_gil::<_, Result<Py<PyAny>, PyErr>>(|py| {
let code = include_str!("verify.py");
let module = PyModule::from_code(py, code, Self::FILENAME, Self::FILENAME)?;
@ -80,7 +79,7 @@ impl TryFrom<&Settings> for Verifier {
Ok(object)
})
.map_err(super::pyerr_to_actix_error)?;
.map_err(super::pyerr_to_tokenserver_error)?;
Ok(Self {
inner,
@ -122,7 +121,7 @@ impl VerifyToken for Verifier {
})
.map_err(|e| TokenserverError {
context: format!("pyo3 error in OAuth verifier: {}", e),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
})?;
match maybe_verify_output_string {
@ -130,13 +129,13 @@ impl VerifyToken for Verifier {
serde_json::from_str::<VerifyOutput>(&verify_output_string).map_err(|e| {
TokenserverError {
context: format!("Invalid OAuth verify output: {}", e),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
}
})
}
None => Err(TokenserverError {
context: "Invalid OAuth token".to_owned(),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
}),
}
};

167
syncstorage/src/tokenserver/extractors.rs
View File

@ -10,7 +10,7 @@ use actix_web::{
dev::Payload,
http::StatusCode,
web::{Data, Query},
Error, FromRequest, HttpRequest,
FromRequest, HttpRequest,
};
use futures::future::{self, LocalBoxFuture, Ready};
use hex;
@ -25,7 +25,7 @@ use super::{
db::{models::Db, params, pool::DbPool, results},
LogItemsMutator, NodeType, ServerState, TokenserverMetrics,
};
use crate::{error::ApiError, server::metrics, settings::Secrets};
use crate::{server::metrics, settings::Secrets};
lazy_static! {
static ref CLIENT_STATE_REGEX: Regex = Regex::new("^[a-zA-Z0-9._-]{1,32}$").unwrap();
@ -104,7 +104,7 @@ impl TokenserverRequest {
.old_client_states
.contains(&self.auth_data.client_state)
{
let error_message = "Unacceptable client-state value stale value";
let error_message = "Unacceptable client-state value stale value".to_owned();
return Err(TokenserverError::invalid_client_state(error_message));
}
@ -114,7 +114,7 @@ impl TokenserverRequest {
&& opt_cmp!(auth_generation <= user_generation)
{
let error_message =
"Unacceptable client-state value new value with no generation change";
"Unacceptable client-state value new value with no generation change".to_owned();
return Err(TokenserverError::invalid_client_state(error_message));
}
@ -124,7 +124,8 @@ impl TokenserverRequest {
&& opt_cmp!(auth_keys_changed_at <= user_keys_changed_at)
{
let error_message =
"Unacceptable client-state value new value with no keys_changed_at change";
"Unacceptable client-state value new value with no keys_changed_at change"
.to_owned();
return Err(TokenserverError::invalid_client_state(error_message));
}
@ -152,7 +153,7 @@ impl TokenserverRequest {
impl FromRequest for TokenserverRequest {
type Config = ();
type Error = Error;
type Error = TokenserverError;
type Future = LocalBoxFuture<'static, Result<Self, Self::Error>>;
fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future {
@ -202,16 +203,14 @@ impl FromRequest for TokenserverRequest {
db.get_service_id(params::GetServiceId {
service: SYNC_SERVICE_NAME.to_owned(),
})
.await
.map_err(ApiError::from)?
.await?
.id,
)
} else {
return Err(TokenserverError::unsupported(
"Unsupported application version",
"Unsupported application version".to_owned(),
version.to_owned(),
)
.into());
));
}
} else {
// NOTE: It would probably be better to include the name of the unsupported
@ -220,10 +219,9 @@ impl FromRequest for TokenserverRequest {
// new Tokenservers as close as possible, we defer to the error message from
// the old Tokenserver.
return Err(TokenserverError::unsupported(
"Unsupported application",
"Unsupported application".to_owned(),
"application".to_owned(),
)
.into());
));
}
};
let user = db
@ -235,12 +233,20 @@ impl FromRequest for TokenserverRequest {
keys_changed_at: auth_data.keys_changed_at,
capacity_release_rate: state.node_capacity_release_rate,
})
.await
.map_err(ApiError::from)?;
.await?;
log_items_mutator.insert("first_seen_at".to_owned(), user.first_seen_at.to_string());
let duration = {
let params = Query::<QueryParams>::extract(&req).await?;
let params =
Query::<QueryParams>::extract(&req)
.await
.map_err(|_| TokenserverError {
description: "invalid query params".to_owned(),
context: "invalid query params".to_owned(),
http_status: StatusCode::BAD_REQUEST,
location: ErrorLocation::Url,
..Default::default()
})?;
// An error in the "duration" query parameter should never cause a request to fail.
// Instead, we should simply resort to using the default token duration.
@ -279,7 +285,7 @@ struct QueryParams {
impl FromRequest for Box<dyn Db> {
type Config = ();
type Error = Error;
type Error = TokenserverError;
type Future = LocalBoxFuture<'static, Result<Self, Self::Error>>;
fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future {
@ -290,12 +296,9 @@ impl FromRequest for Box<dyn Db> {
.await?
.get()
.await
.map_err(|e| {
TokenserverError {
context: format!("Couldn't acquire a database connection: {}", e),
..TokenserverError::internal_error()
}
.into()
.map_err(|e| TokenserverError {
context: format!("Couldn't acquire a database connection: {}", e),
..TokenserverError::internal_error()
})
})
}
@ -303,7 +306,7 @@ impl FromRequest for Box<dyn Db> {
impl FromRequest for Box<dyn DbPool> {
type Config = ();
type Error = Error;
type Error = TokenserverError;
type Future = LocalBoxFuture<'static, Result<Self, Self::Error>>;
fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future {
@ -328,7 +331,7 @@ pub enum Token {
impl FromRequest for Token {
type Config = ();
type Error = Error;
type Error = TokenserverError;
type Future = LocalBoxFuture<'static, Result<Self, Self::Error>>;
fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future {
@ -340,14 +343,14 @@ impl FromRequest for Token {
.headers()
.get("Authorization")
.ok_or_else(|| TokenserverError {
description: "Unauthorized",
description: "Unauthorized".to_owned(),
location: ErrorLocation::Body,
context: "No Authorization header".to_owned(),
..Default::default()
})?
.to_str()
.map_err(|e| TokenserverError {
description: "Unauthorized",
description: "Unauthorized".to_owned(),
location: ErrorLocation::Body,
context: format!(
"Authorization header contains invalid ASCII characters: {}",
@ -366,22 +369,20 @@ impl FromRequest for Token {
} else {
// The request must use a Bearer token or BrowserID token
Err(TokenserverError {
description: "Unsupported",
description: "Unsupported".to_owned(),
location: ErrorLocation::Body,
context: "Invalid authorization scheme".to_owned(),
..Default::default()
}
.into())
})
}
} else {
// Headers that are not of the format "[AUTH TYPE] [TOKEN]" are invalid
Err(TokenserverError {
description: "Unauthorized",
description: "Unauthorized".to_owned(),
location: ErrorLocation::Body,
context: "Invalid Authorization header format".to_owned(),
..Default::default()
}
.into())
})
}
})
}
@ -400,7 +401,7 @@ pub struct AuthData {
impl FromRequest for AuthData {
type Config = ();
type Error = Error;
type Error = TokenserverError;
type Future = LocalBoxFuture<'static, Result<Self, Self::Error>>;
fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future {
@ -467,7 +468,7 @@ struct XClientStateHeader(Option<String>);
impl FromRequest for XClientStateHeader {
type Config = ();
type Error = Error;
type Error = TokenserverError;
type Future = LocalBoxFuture<'static, Result<Self, Self::Error>>;
fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future {
@ -485,12 +486,12 @@ impl FromRequest for XClientStateHeader {
return Err(TokenserverError {
status: "error",
location: ErrorLocation::Header,
description: "Invalid client state value",
description: "Invalid client state value".to_owned(),
name: "X-Client-State".to_owned(),
http_status: StatusCode::BAD_REQUEST,
context: "Invalid client state value".to_owned(),
}
.into());
..Default::default()
});
}
}
@ -509,7 +510,7 @@ struct KeyId {
impl FromRequest for KeyId {
type Config = ();
type Error = Error;
type Error = TokenserverError;
type Future = LocalBoxFuture<'static, Result<Self, Self::Error>>;
fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future {
@ -521,15 +522,19 @@ impl FromRequest for KeyId {
// The X-KeyID header must be present for requests using OAuth
let x_key_id = headers
.get("X-KeyID")
.ok_or_else(|| TokenserverError::invalid_key_id("Missing X-KeyID header"))?
.ok_or_else(|| {
TokenserverError::invalid_key_id("Missing X-KeyID header".to_owned())
})?
.to_str()
.map_err(|_| TokenserverError::invalid_key_id("Invalid X-KeyID header"))?;
.map_err(|_| {
TokenserverError::invalid_key_id("Invalid X-KeyID header".to_owned())
})?;
// The X-KeyID header is of the format `[keys_changed_at]-[base64-encoded client state]` (e.g. `00000000000001234-qqo`)
let (keys_changed_at_string, encoded_client_state) =
x_key_id.split_once('-').ok_or_else(|| TokenserverError {
context: "X-KeyID header has invalid format".to_owned(),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
})?;
let client_state = {
@ -544,7 +549,7 @@ impl FromRequest for KeyId {
"Failed to decode client state base64 in X-KeyID: {}",
e
),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
})?;
hex::encode(bytes)
@ -562,8 +567,7 @@ impl FromRequest for KeyId {
location: ErrorLocation::Body,
context: "Client state mismatch in X-Client-State header".to_owned(),
..TokenserverError::default()
}
.into());
});
}
}
@ -575,7 +579,7 @@ impl FromRequest for KeyId {
.parse::<i64>()
.map_err(|e| TokenserverError {
context: format!("Non-integral keys_changed_at in X-KeyID: {}", e),
..TokenserverError::invalid_credentials("Unauthorized")
..TokenserverError::invalid_credentials("Unauthorized".to_owned())
})?;
Ok(KeyId {
@ -588,7 +592,7 @@ impl FromRequest for KeyId {
impl FromRequest for TokenserverMetrics {
type Config = ();
type Error = Error;
type Error = TokenserverError;
type Future = Ready<Result<Self, Self::Error>>;
fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future {
@ -606,28 +610,25 @@ impl FromRequest for TokenserverMetrics {
}
}
fn get_server_state(req: &HttpRequest) -> Result<&Data<Option<ServerState>>, Error> {
req.app_data::<Data<Option<ServerState>>>().ok_or_else(|| {
Error::from(TokenserverError {
fn get_server_state(req: &HttpRequest) -> Result<&Data<Option<ServerState>>, TokenserverError> {
req.app_data::<Data<Option<ServerState>>>()
.ok_or_else(|| TokenserverError {
context: "Failed to load the application state".to_owned(),
..TokenserverError::internal_error()
})
})
}
fn get_secret(req: &HttpRequest) -> Result<String, Error> {
let secrets = req.app_data::<Data<Arc<Secrets>>>().ok_or_else(|| {
Error::from(TokenserverError {
fn get_secret(req: &HttpRequest) -> Result<String, TokenserverError> {
let secrets = req
.app_data::<Data<Arc<Secrets>>>()
.ok_or_else(|| TokenserverError {
context: "Failed to load the application secrets".to_owned(),
..TokenserverError::internal_error()
})
})?;
})?;
String::from_utf8(secrets.master_secret.clone()).map_err(|e| {
Error::from(TokenserverError {
context: format!("Failed to read the master secret: {}", e),
..TokenserverError::internal_error()
})
String::from_utf8(secrets.master_secret.clone()).map_err(|e| TokenserverError {
context: format!("Failed to read the master secret: {}", e),
..TokenserverError::internal_error()
})
}
@ -766,7 +767,7 @@ mod tests {
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
let expected_error = TokenserverError::invalid_credentials("Unauthorized");
let expected_error = TokenserverError::invalid_credentials("Unauthorized".to_owned());
let body = extract_body_as_str(ServiceResponse::new(request, response));
assert_eq!(body, serde_json::to_string(&expected_error).unwrap());
}
@ -811,8 +812,10 @@ mod tests {
assert_eq!(response.status(), StatusCode::NOT_FOUND);
let expected_error =
TokenserverError::unsupported("Unsupported application version", "1.0".to_owned());
let expected_error = TokenserverError::unsupported(
"Unsupported application version".to_owned(),
"1.0".to_owned(),
);
let body = extract_body_as_str(ServiceResponse::new(request, response));
assert_eq!(body, serde_json::to_string(&expected_error).unwrap());
}
@ -831,8 +834,10 @@ mod tests {
assert_eq!(response.status(), StatusCode::NOT_FOUND);
let expected_error =
TokenserverError::unsupported("Unsupported application", "application".to_owned());
let expected_error = TokenserverError::unsupported(
"Unsupported application".to_owned(),
"application".to_owned(),
);
let body = extract_body_as_str(ServiceResponse::new(request, response));
assert_eq!(body, serde_json::to_string(&expected_error).unwrap());
}
@ -851,8 +856,10 @@ mod tests {
assert_eq!(response.status(), StatusCode::NOT_FOUND);
let expected_error =
TokenserverError::unsupported("Unsupported application", "application".to_owned());
let expected_error = TokenserverError::unsupported(
"Unsupported application".to_owned(),
"application".to_owned(),
);
let body = extract_body_as_str(ServiceResponse::new(request, response));
assert_eq!(body, serde_json::to_string(&expected_error).unwrap());
}
@ -902,7 +909,8 @@ mod tests {
let response: HttpResponse = KeyId::extract(&request).await.unwrap_err().into();
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
let expected_error = TokenserverError::invalid_key_id("Missing X-KeyID header");
let expected_error =
TokenserverError::invalid_key_id("Missing X-KeyID header".to_owned());
let body = extract_body_as_str(ServiceResponse::new(request, response));
assert_eq!(body, serde_json::to_string(&expected_error).unwrap());
}
@ -915,7 +923,8 @@ mod tests {
let response: HttpResponse = KeyId::extract(&request).await.unwrap_err().into();
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
let expected_error = TokenserverError::invalid_key_id("Invalid X-KeyID header");
let expected_error =
TokenserverError::invalid_key_id("Invalid X-KeyID header".to_owned());
let body = extract_body_as_str(ServiceResponse::new(request, response));
assert_eq!(body, serde_json::to_string(&expected_error).unwrap());
}
@ -928,7 +937,7 @@ mod tests {
let response: HttpResponse = KeyId::extract(&request).await.unwrap_err().into();
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
let expected_error = TokenserverError::invalid_credentials("Unauthorized");
let expected_error = TokenserverError::invalid_credentials("Unauthorized".to_owned());
let body = extract_body_as_str(ServiceResponse::new(request, response));
assert_eq!(body, serde_json::to_string(&expected_error).unwrap());
}
@ -941,7 +950,7 @@ mod tests {
let response: HttpResponse = KeyId::extract(&request).await.unwrap_err().into();
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
let expected_error = TokenserverError::invalid_credentials("Unauthorized");
let expected_error = TokenserverError::invalid_credentials("Unauthorized".to_owned());
let body = extract_body_as_str(ServiceResponse::new(request, response));
assert_eq!(body, serde_json::to_string(&expected_error).unwrap());
}
@ -954,7 +963,8 @@ mod tests {
let response: HttpResponse = KeyId::extract(&request).await.unwrap_err().into();
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
let expected_error = TokenserverError::invalid_key_id("Invalid X-KeyID header");
let expected_error =
TokenserverError::invalid_key_id("Invalid X-KeyID header".to_owned());
let body = extract_body_as_str(ServiceResponse::new(request, response));
assert_eq!(body, serde_json::to_string(&expected_error).unwrap());
}
@ -967,7 +977,7 @@ mod tests {
let response: HttpResponse = KeyId::extract(&request).await.unwrap_err().into();
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
let expected_error = TokenserverError::invalid_credentials("Unauthorized");
let expected_error = TokenserverError::invalid_credentials("Unauthorized".to_owned());
let body = extract_body_as_str(ServiceResponse::new(request, response));
assert_eq!(body, serde_json::to_string(&expected_error).unwrap());
}
@ -1182,7 +1192,7 @@ mod tests {
};
let error = tokenserver_request.validate().unwrap_err();
let error_message = "Unacceptable client-state value stale value";
let error_message = "Unacceptable client-state value stale value".to_owned();
assert_eq!(error, TokenserverError::invalid_client_state(error_message));
}
@ -1219,7 +1229,8 @@ mod tests {
};
let error = tokenserver_request.validate().unwrap_err();
let error_message = "Unacceptable client-state value new value with no generation change";
let error_message =
"Unacceptable client-state value new value with no generation change".to_owned();
assert_eq!(error, TokenserverError::invalid_client_state(error_message));
}
@ -1257,7 +1268,7 @@ mod tests {
let error = tokenserver_request.validate().unwrap_err();
let error_message =
"Unacceptable client-state value new value with no keys_changed_at change";
"Unacceptable client-state value new value with no keys_changed_at change".to_owned();
assert_eq!(error, TokenserverError::invalid_client_state(error_message));
}

8
syncstorage/src/tokenserver/handlers.rs
View File

@ -18,7 +18,6 @@ use super::{
extractors::TokenserverRequest,
NodeType, TokenserverMetrics,
};
use crate::error::ApiError;
#[derive(Debug, Serialize)]
pub struct TokenserverResult {
@ -36,7 +35,7 @@ pub async fn get_tokenserver_result(
req: TokenserverRequest,
db: Box<dyn Db>,
TokenserverMetrics(mut metrics): TokenserverMetrics,
) -> Result<HttpResponse, Error> {
) -> Result<HttpResponse, TokenserverError> {
let updates = update_user(&req, db).await?;
let (token, derived_secret) = {
@ -113,7 +112,10 @@ struct UserUpdates {
uid: i64,
}
async fn update_user(req: &TokenserverRequest, db: Box<dyn Db>) -> Result<UserUpdates, ApiError> {
async fn update_user(
req: &TokenserverRequest,
db: Box<dyn Db>,
) -> Result<UserUpdates, TokenserverError> {
// If the keys_changed_at in the request is larger than that stored on the user record,
// update to the value in the request.
let keys_changed_at =

2
tokenserver-common/Cargo.toml
View File

@ -5,6 +5,8 @@ edition = "2021"
[dependencies]
actix-web = "3"
backtrace = "0.3.61"
serde = "1.0"
serde_json = { version = "1.0", features = ["arbitrary_precision"] }
syncstorage-db-common = { path = "../syncstorage-db-common" }
thiserror = "1.0.26"

87
tokenserver-common/src/error.rs
View File

@ -1,23 +1,43 @@
use std::fmt;
use std::{cmp::PartialEq, fmt};
use actix_web::{http::StatusCode, HttpResponse, ResponseError};
use backtrace::Backtrace;
use serde::{
ser::{SerializeMap, Serializer},
Serialize,
};
use thiserror::Error;
use syncstorage_db_common::error::DbError;
#[derive(Clone, Debug, Error, PartialEq)]
#[error("{context}")]
#[derive(Clone, Debug)]
pub struct TokenserverError {
pub status: &'static str,
pub location: ErrorLocation,
pub name: String,
pub description: &'static str,
pub description: String,
pub http_status: StatusCode,
/// For internal use only. Used to report any additional context behind an error to
/// distinguish between similar errors in Sentry.
pub context: String,
pub backtrace: Backtrace,
}
// We implement `PartialEq` manually here because `Backtrace` doesn't implement `PartialEq`, so we
// can't derive it
impl PartialEq for TokenserverError {
fn eq(&self, other: &Self) -> bool {
self.status == other.status
&& self.location == other.location
&& self.name == other.name
&& self.description == other.description
&& self.http_status == other.http_status
&& self.context == other.context
}
}
impl fmt::Display for TokenserverError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.context)
}
}
impl Default for TokenserverError {
@ -26,9 +46,10 @@ impl Default for TokenserverError {
status: "error",
location: ErrorLocation::default(),
name: "".to_owned(),
description: "Unauthorized",
description: "Unauthorized".to_owned(),
http_status: StatusCode::UNAUTHORIZED,
context: "Unauthorized".to_owned(),
backtrace: Backtrace::new(),
}
}
}
@ -52,31 +73,31 @@ impl TokenserverError {
}
}
pub fn invalid_key_id(description: &'static str) -> Self {
pub fn invalid_key_id(description: String) -> Self {
Self {
status: "invalid-key-id",
context: description.clone(),
description,
context: description.to_owned(),
..Self::default()
}
}
pub fn invalid_credentials(description: &'static str) -> Self {
pub fn invalid_credentials(description: String) -> Self {
Self {
status: "invalid-credentials",
location: ErrorLocation::Body,
context: description.clone(),
description,
context: description.to_owned(),
..Self::default()
}
}
pub fn invalid_client_state(description: &'static str) -> Self {
pub fn invalid_client_state(description: String) -> Self {
Self {
status: "invalid-client-state",
context: description.clone(),
description,
name: "X-Client-State".to_owned(),
context: description.to_owned(),
..Self::default()
}
}
@ -85,7 +106,7 @@ impl TokenserverError {
Self {
status: "internal-error",
location: ErrorLocation::Internal,
description: "Server error",
description: "Server error".to_owned(),
http_status: StatusCode::INTERNAL_SERVER_ERROR,
context: "Internal error".to_owned(),
..Self::default()
@ -95,29 +116,30 @@ impl TokenserverError {
pub fn resource_unavailable() -> Self {
Self {
location: ErrorLocation::Body,
description: "Resource is not available",
description: "Resource is not available".to_owned(),
http_status: StatusCode::SERVICE_UNAVAILABLE,
context: "Resource is not available".to_owned(),
..Default::default()
}
}
pub fn unsupported(description: &'static str, name: String) -> Self {
pub fn unsupported(description: String, name: String) -> Self {
Self {
status: "error",
location: ErrorLocation::Url,
context: description.clone(),
description,
name,
http_status: StatusCode::NOT_FOUND,
context: description.to_owned(),
..Self::default()
}
}
pub fn unauthorized(description: &'static str) -> Self {
pub fn unauthorized(description: String) -> Self {
Self {
location: ErrorLocation::Body,
context: description.clone(),
description,
context: description.to_owned(),
..Self::default()
}
}
@ -166,7 +188,7 @@ struct ErrorResponse {
struct ErrorInstance {
location: ErrorLocation,
name: String,
description: &'static str,
description: String,
}
impl From<&TokenserverError> for ErrorResponse {
@ -176,7 +198,7 @@ impl From<&TokenserverError> for ErrorResponse {
errors: [ErrorInstance {
location: error.location,
name: error.name.clone(),
description: error.description,
description: error.description.clone(),
}],
}
}
@ -215,3 +237,28 @@ impl Serialize for TokenserverError {
ErrorResponse::from(self).serialize(serializer)
}
}
impl From<DbError> for TokenserverError {
fn from(db_error: DbError) -> Self {
TokenserverError {
description: db_error.to_string(),
context: db_error.to_string(),
backtrace: db_error.backtrace,
http_status: if db_error.status.is_server_error() {
// Use the status code from the DbError if it already suggests an internal error;
// it might be more specific than `StatusCode::SERVICE_UNAVAILABLE`
db_error.status
} else {
StatusCode::SERVICE_UNAVAILABLE
},
// An unhandled DbError in the Tokenserver code is an internal error
..TokenserverError::internal_error()
}
}
}
impl From<TokenserverError> for HttpResponse {
fn from(inner: TokenserverError) -> Self {
ResponseError::error_response(&inner)
}
}

29
tools/integration_tests/tokenserver/test_interchangeability.py
View File

@ -1,29 +0,0 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at http://mozilla.org/MPL/2.0/.
import unittest
from tokenserver.test_support import TestCase
class TestInterchangeability(TestCase, unittest.TestCase):
def setUp(self):
super(TestInterchangeability, self).setUp()
def tearDown(self):
super(TestInterchangeability, self).tearDown()
def test_generation_change(self):
pass
def test_keys_changed_at_change(self):
pass
def test_client_state_change(self):
pass
def test_replaced_at(self):
pass
def test_node_reassignment(self):
pass

14
tools/integration_tests/tokenserver/test_node_assignment.py
View File

@ -131,4 +131,16 @@ class TestNodeAssignment(TestCase, unittest.TestCase):
client_state='aaaa')
# All of these nodes are completely full, and no capacity can be
# released
self.app.get('/1.0/sync/1.5', headers=headers, status=503)
res = self.app.get('/1.0/sync/1.5', headers=headers, status=503)
# The response has the expected body
expected_error_response = {
'errors': [
{
'description': 'Unexpected error: unable to get a node',
'location': 'internal',
'name': ''
}
],
'status': 'internal-error'
}
self.assertEqual(res.json, expected_error_response)