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

refactor: overhaul errors #395

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ serde_json = { version = "1.0.51", optional = true }
serde_crate = { version = "1.0.106", features = ["derive"], optional = true, package = "serde" }
serde_urlencoded = { version = "0.7.0", optional = true}
serde_qs = { version = "0.9.1", optional = true }

miette = "3"
thiserror = "1.0.29"

[dev-dependencies]
http = "0.2.0"
Expand Down
6 changes: 3 additions & 3 deletions src/auth/authentication_scheme.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::fmt::{self, Display};
use std::str::FromStr;

use crate::bail_status as bail;
use crate::errors::AuthError;

/// HTTP Mutual Authentication Algorithms
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
Expand Down Expand Up @@ -47,7 +47,7 @@ impl Display for AuthenticationScheme {
}

impl FromStr for AuthenticationScheme {
type Err = crate::Error;
type Err = AuthError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
// NOTE(yosh): matching here is lowercase as specified by RFC2617#section-1.2
Expand All @@ -65,7 +65,7 @@ impl FromStr for AuthenticationScheme {
"scram-sha-1" => Ok(Self::ScramSha1),
"scram-sha-256" => Ok(Self::ScramSha256),
"vapid" => Ok(Self::Vapid),
s => bail!(400, "`{}` is not a recognized authentication scheme", s),
s => Err(AuthError::SchemeUnrecognized(s.to_string())),
}
}
}
12 changes: 7 additions & 5 deletions src/auth/authorization.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::auth::AuthenticationScheme;
use crate::bail_status as bail;
use crate::errors::AuthError;
use crate::headers::{Header, HeaderName, HeaderValue, Headers, AUTHORIZATION};

/// Credentials to authenticate a user agent with a server.
Expand Down Expand Up @@ -60,8 +60,8 @@ impl Authorization {
let scheme = iter.next();
let credential = iter.next();
let (scheme, credentials) = match (scheme, credential) {
(None, _) => bail!(400, "Could not find scheme"),
(Some(_), None) => bail!(400, "Could not find credentials"),
(None, _) => return Err(AuthError::SchemeMissing.into()),
(Some(_), None) => return Err(AuthError::CredentialsMissing.into()),
(Some(scheme), Some(credentials)) => (scheme.parse()?, credentials.to_owned()),
};

Expand Down Expand Up @@ -107,8 +107,10 @@ impl Header for Authorization {

#[cfg(test)]
mod test {
use super::*;
use crate::headers::Headers;
use crate::StatusCode;

use super::*;

#[test]
fn smoke() -> crate::Result<()> {
Expand All @@ -133,6 +135,6 @@ mod test {
.insert(AUTHORIZATION, "<nori ate the tag. yum.>")
.unwrap();
let err = Authorization::from_headers(headers).unwrap_err();
assert_eq!(err.status(), 400);
assert_eq!(err.associated_status_code(), Some(StatusCode::BadRequest));
}
}
39 changes: 27 additions & 12 deletions src/auth/basic_auth.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use crate::errors::AuthError;
use crate::headers::{HeaderName, HeaderValue, Headers, AUTHORIZATION};
use crate::Status;
use crate::{
auth::{AuthenticationScheme, Authorization},
headers::Header,
};
use crate::{bail_status as bail, ensure_status as ensure};

/// HTTP Basic authorization.
///
Expand Down Expand Up @@ -60,28 +59,42 @@ impl BasicAuth {
};

let scheme = auth.scheme();
ensure!(
internal_ensure!(
matches!(scheme, AuthenticationScheme::Basic),
400,
"Expected basic auth scheme found `{}`",
scheme
AuthError::SchemeUnexpected(AuthenticationScheme::Basic, scheme.to_string())
);
Self::from_credentials(auth.credentials()).map(Some)
}

/// Create a new instance from the base64 encoded credentials.
pub fn from_credentials(credentials: impl AsRef<[u8]>) -> crate::Result<Self> {
let bytes = base64::decode(credentials).status(400)?;
let credentials = String::from_utf8(bytes).status(400)?;
let bytes = base64::decode(credentials).map_err(|_| {
AuthError::CredentialsInvalid(AuthenticationScheme::Basic, "invalid base64")
})?;
let credentials = String::from_utf8(bytes).map_err(|_| {
AuthError::CredentialsInvalid(AuthenticationScheme::Basic, "invalid utf8 from base64")
})?;

let mut iter = credentials.splitn(2, ':');
let username = iter.next();
let password = iter.next();

let (username, password) = match (username, password) {
(Some(username), Some(password)) => (username.to_string(), password.to_string()),
(Some(_), None) => bail!(400, "Expected basic auth to contain a password"),
(None, _) => bail!(400, "Expected basic auth to contain a username"),
(Some(_), None) => {
return Err(AuthError::CredentialsInvalid(
AuthenticationScheme::Basic,
"missing password",
)
.into())
}
(None, _) => {
return Err(AuthError::CredentialsInvalid(
AuthenticationScheme::Basic,
"missing username",
)
.into())
}
};

Ok(Self { username, password })
Expand Down Expand Up @@ -113,8 +126,10 @@ impl Header for BasicAuth {

#[cfg(test)]
mod test {
use super::*;
use crate::headers::Headers;
use crate::StatusCode;

use super::*;

#[test]
fn smoke() -> crate::Result<()> {
Expand All @@ -139,6 +154,6 @@ mod test {
.insert(AUTHORIZATION, "<nori ate the tag. yum.>")
.unwrap();
let err = BasicAuth::from_headers(headers).unwrap_err();
assert_eq!(err.status(), 400);
assert_eq!(err.associated_status_code(), Some(StatusCode::BadRequest));
}
}
16 changes: 9 additions & 7 deletions src/auth/www_authenticate.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::bail_status as bail;
use crate::errors::{AuthError, HeaderError};
use crate::headers::{HeaderName, HeaderValue, Headers, WWW_AUTHENTICATE};
use crate::{auth::AuthenticationScheme, headers::Header};

Expand Down Expand Up @@ -63,15 +63,15 @@ impl WwwAuthenticate {
let scheme = iter.next();
let credential = iter.next();
let (scheme, realm) = match (scheme, credential) {
(None, _) => bail!(400, "Could not find scheme"),
(Some(_), None) => bail!(400, "Could not find realm"),
(None, _) => return Err(AuthError::SchemeMissing.into()),
(Some(_), None) => return Err(AuthError::RealmMissing.into()),
(Some(scheme), Some(realm)) => (scheme.parse()?, realm.to_owned()),
};

let realm = realm.trim_start();
let realm = match realm.strip_prefix(r#"realm=""#) {
Some(realm) => realm,
None => bail!(400, "realm not found"),
None => return Err(AuthError::RealmMissing.into()),
};

let mut chars = realm.chars();
Expand All @@ -87,7 +87,7 @@ impl WwwAuthenticate {
})
.collect();
if !closing_quote {
bail!(400, r"Expected a closing quote");
return Err(HeaderError::WWWAuthenticateInvalid("Expected a closing quote").into());
}

Ok(Some(Self { scheme, realm }))
Expand Down Expand Up @@ -129,8 +129,10 @@ impl Header for WwwAuthenticate {

#[cfg(test)]
mod test {
use super::*;
use crate::headers::Headers;
use crate::StatusCode;

use super::*;

#[test]
fn smoke() -> crate::Result<()> {
Expand Down Expand Up @@ -160,6 +162,6 @@ mod test {
.insert(WWW_AUTHENTICATE, "<nori ate the tag. yum.>")
.unwrap();
let err = WwwAuthenticate::from_headers(headers).unwrap_err();
assert_eq!(err.status(), 400);
assert_eq!(err.associated_status_code(), Some(StatusCode::BadRequest));
}
}
Loading