Skip to content

Commit

Permalink
fix(api/http): Use SecretStrings for HTTP authorization secrets
Browse files Browse the repository at this point in the history 
Fixes #556
  • Loading branch information
@gakonst
gakonst committed Dec 21, 2019
1 parent f07dd5f commit 7485916
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 56 deletions.
16 changes: 8 additions & 8 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/interledger-api/Cargo.toml
View file
Expand Up @@ -31,7 +31,7 @@ reqwest = { version = "0.9.22", default-features = false, features = ["default-t
url = { version = "2.1.0", default-features = false, features = ["serde"] }
uuid = { version = "0.8.1", default-features = false}
warp = { version = "0.1.20", default-features = false }
secrecy = { version = "0.5.1", default-features = false, features = ["serde"] }
secrecy = { version = "0.5.2", default-features = false, features = ["serde"] }
lazy_static = "1.4.0"

[badges]
Expand Down
79 changes: 45 additions & 34 deletions crates/interledger-api/src/routes/accounts.rs
View file
Expand Up @@ -18,6 +18,7 @@ use interledger_settlement::core::types::SettlementAccount;
use interledger_spsp::{pay, SpspResponder};
use interledger_stream::{PaymentNotification, StreamNotificationsStore};
use log::{debug, error, trace};
use secrecy::{ExposeSecret, SecretString};
use serde::{Deserialize, Serialize};
use serde_json::json;
use std::convert::TryFrom;
Expand Down Expand Up @@ -66,14 +67,16 @@ where

// Helper filters
let admin_auth_header = format!("Bearer {}", admin_api_token);
let admin_only = warp::header::<String>("authorization")
.and_then(move |authorization| -> Result<(), Rejection> {
if authorization == admin_auth_header {
Ok(())
} else {
Err(ApiError::unauthorized().into())
}
})
let admin_only = warp::header::<SecretString>("authorization")
.and_then(
move |authorization: SecretString| -> Result<(), Rejection> {
if authorization.expose_secret() == &admin_auth_header {
Ok(())
} else {
Err(ApiError::unauthorized().into())
}
},
)
// This call makes it so we do not pass on a () value on
// success to the next filter, it just gets rid of it
.untuple_one()
Expand Down Expand Up @@ -121,12 +124,15 @@ where
// unauthorized that the the request actually should not cause.
// This function needs parameters which can be prepared by `account_username`.
let admin_or_authorized_user_only = warp::filters::ext::get::<Username>()
.and(warp::header::<String>("authorization"))
.and(warp::header::<SecretString>("authorization"))
.and(with_store.clone())
.and(with_admin_auth_header.clone())
.and_then(
|path_username: Username, auth_string: String, store: S, admin_auth_header: String| {
if auth_string.len() < BEARER_TOKEN_START {
|path_username: Username,
auth_string: SecretString,
store: S,
admin_auth_header: String| {
if auth_string.expose_secret().len() < BEARER_TOKEN_START {
return Either::A(err(ApiError::bad_request().into()));
}
Either::B(store.get_account_id_from_username(&path_username).then(
Expand All @@ -137,14 +143,14 @@ where
));
}
let account_id = account_id.unwrap();
if auth_string == admin_auth_header {
if auth_string.expose_secret() == &admin_auth_header {
return Either::A(ok(account_id));
}
Either::B(
store
.get_account_from_http_auth(
&path_username,
&auth_string[BEARER_TOKEN_START..],
&auth_string.expose_secret()[BEARER_TOKEN_START..],
)
.then(move |authorized_account: Result<A, _>| {
if authorized_account.is_err() {
Expand All @@ -167,28 +173,33 @@ where
// The same structure as `admin_or_authorized_user_only`.
// This function needs parameters which can be prepared by `account_username`.
let authorized_user_only = warp::filters::ext::get::<Username>()
.and(warp::header::<String>("authorization"))
.and(warp::header::<SecretString>("authorization"))
.and(with_store.clone())
.and_then(|path_username: Username, auth_string: String, store: S| {
if auth_string.len() < BEARER_TOKEN_START {
return Either::A(err(ApiError::bad_request().into()));
}
Either::B(
store
.get_account_from_http_auth(&path_username, &auth_string[BEARER_TOKEN_START..])
.then(move |authorized_account: Result<A, _>| {
if authorized_account.is_err() {
return err::<A, Rejection>(ApiError::unauthorized().into());
}
let authorized_account = authorized_account.unwrap();
if &path_username == authorized_account.username() {
ok(authorized_account)
} else {
err(ApiError::unauthorized().into())
}
}),
)
})
.and_then(
|path_username: Username, auth_string: SecretString, store: S| {
if auth_string.expose_secret().len() < BEARER_TOKEN_START {
return Either::A(err(ApiError::bad_request().into()));
}
Either::B(
store
.get_account_from_http_auth(
&path_username,
&auth_string.expose_secret()[BEARER_TOKEN_START..],
)
.then(move |authorized_account: Result<A, _>| {
if authorized_account.is_err() {
return err::<A, Rejection>(ApiError::unauthorized().into());
}
let authorized_account = authorized_account.unwrap();
if &path_username == authorized_account.username() {
ok(authorized_account)
} else {
err(ApiError::unauthorized().into())
}
}),
)
},
)
.boxed();

// POST /accounts
Expand Down
19 changes: 11 additions & 8 deletions crates/interledger-api/src/routes/node_settings.rs
View file
Expand Up @@ -11,6 +11,7 @@ use interledger_service::{Account, Username};
use interledger_service_util::{BalanceStore, ExchangeRateStore};
use interledger_settlement::core::types::SettlementAccount;
use log::{error, trace};
use secrecy::{ExposeSecret, SecretString};
use serde::Serialize;
use std::{
collections::HashMap,
Expand Down Expand Up @@ -44,14 +45,16 @@ where
{
// Helper filters
let admin_auth_header = format!("Bearer {}", admin_api_token);
let admin_only = warp::header::<String>("authorization")
.and_then(move |authorization| -> Result<(), Rejection> {
if authorization == admin_auth_header {
Ok(())
} else {
Err(ApiError::unauthorized().into())
}
})
let admin_only = warp::header::<SecretString>("authorization")
.and_then(
move |authorization: SecretString| -> Result<(), Rejection> {
if authorization.expose_secret() == &admin_auth_header {
Ok(())
} else {
Err(ApiError::unauthorized().into())
}
},
)
// This call makes it so we do not pass on a () value on
// success to the next filter, it just gets rid of it
.untuple_one()
Expand Down
2 changes: 1 addition & 1 deletion crates/interledger-http/Cargo.toml
View file
Expand Up @@ -24,7 +24,7 @@ chrono = { version = "0.4.9", features = ["clock"], default-features = false }
regex = { version ="1.3.1", default-features = false, features = ["std"] }
lazy_static = { version ="1.4.0", default-features = false }
mime = { version ="0.3.14", default-features = false }
secrecy = "0.5.1"
secrecy = "0.5.2"

[dev-dependencies]
uuid = { version = "0.8.1", features=["v4"]}
12 changes: 8 additions & 4 deletions crates/interledger-http/src/server.rs
View file
Expand Up @@ -8,6 +8,7 @@ use interledger_packet::Prepare;
use interledger_service::Username;
use interledger_service::{IncomingRequest, IncomingService};
use log::error;
use secrecy::{ExposeSecret, SecretString};
use std::{convert::TryFrom, net::SocketAddr};
use warp::{self, Filter, Rejection};

Expand Down Expand Up @@ -44,14 +45,17 @@ where
.and(warp::path::param2::<Username>())
.and(warp::path("ilp"))
.and(warp::path::end())
.and(warp::header::<String>("authorization"))
.and_then(move |path_username: Username, password: String| {
if password.len() < BEARER_TOKEN_START {
.and(warp::header::<SecretString>("authorization"))
.and_then(move |path_username: Username, password: SecretString| {
if password.expose_secret().len() < BEARER_TOKEN_START {
return Either::A(err(ApiError::bad_request().into()));
}
Either::B(
store
.get_account_from_http_auth(&path_username, &password[BEARER_TOKEN_START..])
.get_account_from_http_auth(
&path_username,
&password.expose_secret()[BEARER_TOKEN_START..],
)
.map_err(move |_| -> Rejection {
error!("Invalid authorization provided for user: {}", path_username);
ApiError::unauthorized().into()
Expand Down

0 comments on commit 7485916

Please sign in to comment.