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

Use new headers crate instead of hyper-old-types #158

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,14 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Changed

- *BREAKING* - Remove dependency and re-export of `hyper-old-types` which is no longer maintained.
- This changes the inner types of the `AuthData` enum and thus the various methods on it.
- The `hyper_old_types` are no longer re-exported, and the enums just wrap `String`s.
- The `auth::make_headers` function now returns an `Option<Credentials>` from the `headers` crate.

### Added

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ serde_json = { version = "1.0", optional = true }
hyper = "0.14"
slog = { version = "2", features = [ "max_level_trace", "release_max_level_debug"] }
uuid = { version = "0.8", features = ["serde", "v4"] }
hyper-old-types = "0.11.0"
futures = "0.3"
headers = "0.3"

# Conversion
frunk = { version = "0.3.0", optional = true }
Expand Down
40 changes: 15 additions & 25 deletions src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@

use crate::context::Push;
use futures::future::FutureExt;
pub use headers::authorization::{Basic, Bearer, Credentials};
use headers::Authorization as Header;
use hyper::header::AUTHORIZATION;
use hyper::service::Service;
use hyper::{HeaderMap, Request};
pub use hyper_old_types::header::Authorization as Header;
use hyper_old_types::header::Header as HeaderTrait;
pub use hyper_old_types::header::{Basic, Bearer};
use hyper_old_types::header::{Raw, Scheme};
use std::collections::BTreeSet;
use std::marker::PhantomData;
use std::string::ToString;
Expand Down Expand Up @@ -53,28 +51,26 @@ pub struct Authorization {
/// request authentication, and for authenticating outgoing client requests.
#[derive(Clone, Debug, PartialEq)]
pub enum AuthData {
/// HTTP Basic auth.
Basic(Basic),
/// HTTP Bearer auth, used for OAuth2.
Bearer(Bearer),
/// HTTP Basic auth - username and password.
Basic(String, String),
/// HTTP Bearer auth, used for OAuth2 - token.
Bearer(String),
/// Header-based or query parameter-based API key auth.
ApiKey(String),
}

impl AuthData {
/// Set Basic authentication
pub fn basic(username: &str, password: &str) -> Self {
AuthData::Basic(Basic {
username: username.to_owned(),
password: Some(password.to_owned()),
})
let basic = Header::basic(username, password);
AuthData::Basic(basic.username().to_owned(), basic.password().to_owned())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just realised this is a pretty pointless implementation - I'm cnnstucting the header for no reason. I feel like ideally, we would return https://docs.rs/headers/latest/headers/authorization/struct.Basic.html here - but that has no constructor beyond a decode from a HeaderValue. (Same applies to the Bearer variant). We could make the inner type an authorization header but that feels wrong too.

Thoughts welcome!

}

/// Set Bearer token authentication
pub fn bearer(token: &str) -> Self {
AuthData::Bearer(Bearer {
token: token.to_owned(),
})
/// Set Bearer token authentication. Returns None if the token was invalid.
pub fn bearer(token: &str) -> Option<Self> {
Some(AuthData::Bearer(
Header::bearer(token).ok()?.token().to_owned(),
))
}

/// Set ApiKey authentication
Expand Down Expand Up @@ -211,16 +207,10 @@ where
}

/// Retrieve an authorization scheme data from a set of headers
pub fn from_headers<S: Scheme>(headers: &HeaderMap) -> Option<S>
where
S: std::str::FromStr + 'static,
S::Err: 'static,
{
pub fn from_headers<C: Credentials>(headers: &HeaderMap) -> Option<C> {
headers
.get(AUTHORIZATION)
.and_then(|v| v.to_str().ok())
.and_then(|s| Header::<S>::parse_header(&Raw::from(s)).ok())
.map(|a| a.0)
.and_then(|s| Credentials::decode(s))
}

/// Retrieve an API key from a header
Expand Down
3 changes: 1 addition & 2 deletions src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ impl XSpanIdString {
let x_span_id = req.headers().get(X_SPAN_ID);

x_span_id
.map(|x| x.to_str().ok())
.flatten()
.and_then(|x| x.to_str().ok())
.map(|x| XSpanIdString(x.to_string()))
.unwrap_or_default()
}
Expand Down