-
-
Notifications
You must be signed in to change notification settings - Fork 49
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
refactor(server)!: cleanup authorization boilerplate (#199)
* refactor!: use `actix-web-grants` to protect endpoints * fix: filter out blank strings * doc: add documentation for a function * fix: don't return body for not exposed endpoints * test: add fixtures * test: fix naming * test: remove extra step in teardown
- Loading branch information
Showing
13 changed files
with
341 additions
and
105 deletions.
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
[server] | ||
address = "127.0.0.1:8000" | ||
max_content_length = "10MB" | ||
upload_path = "./upload" | ||
expose_list = false | ||
|
||
[paste] | ||
random_url = { type = "petname", words = 2, separator = "-" } | ||
default_extension = "txt" | ||
duplicate_files = true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
#!/usr/bin/env bash | ||
|
||
setup() { | ||
:; | ||
} | ||
|
||
run_test() { | ||
result=$(curl -s --write-out "%{http_code}" http://localhost:8000/list) | ||
test "404" = "$result" | ||
} | ||
|
||
teardown() { | ||
rm -r upload | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
[server] | ||
address = "127.0.0.1:8000" | ||
max_content_length = "10MB" | ||
upload_path = "./upload" | ||
|
||
[paste] | ||
default_extension = "txt" | ||
duplicate_files = false |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
#!/usr/bin/env bash | ||
|
||
content="topsecret" | ||
|
||
setup() { | ||
echo "$content" > file | ||
} | ||
|
||
run_test() { | ||
result=$(curl -s -F "file=@file" localhost:8000) | ||
test "unauthorized" != "$result" | ||
test "$content" = "$(cat upload/file.txt)" | ||
test "$content" = "$(curl -s $result)" | ||
} | ||
|
||
teardown() { | ||
rm file | ||
rm -r upload | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
[server] | ||
address = "127.0.0.1:8000" | ||
max_content_length = "10MB" | ||
upload_path = "./upload" | ||
|
||
[paste] | ||
default_extension = "txt" | ||
duplicate_files = false |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
#!/usr/bin/env bash | ||
|
||
content="test data" | ||
|
||
setup() { | ||
echo "$content" > file | ||
} | ||
|
||
run_test() { | ||
file_url=$(curl -s -F "file=@file" localhost:8000) | ||
test "$file_url" = "http://localhost:8000/file.txt" | ||
|
||
result=$(curl -s --write-out "%{http_code}" -X DELETE http://localhost:8000/file.txt) | ||
test "404" = "$result" | ||
} | ||
|
||
teardown() { | ||
rm file | ||
rm -r upload | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
[server] | ||
address = "127.0.0.1:8000" | ||
max_content_length = "10MB" | ||
upload_path = "./upload" | ||
expose_version = false | ||
|
||
[paste] | ||
random_url = { type = "petname", words = 2, separator = "-" } | ||
default_extension = "txt" | ||
duplicate_files = true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
#!/usr/bin/env bash | ||
|
||
setup() { | ||
echo "$content" > file | ||
} | ||
|
||
run_test() { | ||
result=$(curl -s --write-out "%{http_code}" http://localhost:8000/version) | ||
test "404" = "$result" | ||
} | ||
|
||
teardown() { | ||
rm file | ||
rm -r upload | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,55 +1,163 @@ | ||
use actix_web::http::header::{HeaderMap, AUTHORIZATION}; | ||
use actix_web::{error, Error}; | ||
use crate::config::{Config, TokenType}; | ||
use actix_web::dev::{ServiceRequest, ServiceResponse}; | ||
use actix_web::http::header::AUTHORIZATION; | ||
use actix_web::http::Method; | ||
use actix_web::middleware::ErrorHandlerResponse; | ||
use actix_web::{error, web, Error}; | ||
use std::collections::HashSet; | ||
use std::sync::RwLock; | ||
|
||
/// Checks the authorization header for the specified token. | ||
/// Extracts the tokens from the authorization header by token type. | ||
/// | ||
/// `Authorization: (type) <token>` | ||
pub fn check(host: &str, headers: &HeaderMap, tokens: Option<Vec<String>>) -> Result<(), Error> { | ||
if let Some(tokens) = tokens { | ||
let auth_header = headers | ||
.get(AUTHORIZATION) | ||
.map(|v| v.to_str().unwrap_or_default()) | ||
.map(|v| v.split_whitespace().last().unwrap_or_default()); | ||
if !tokens.iter().any(|v| v == auth_header.unwrap_or_default()) { | ||
#[cfg(debug_assertions)] | ||
warn!( | ||
"authorization failure for {host} (token: {})", | ||
auth_header.unwrap_or("none"), | ||
); | ||
#[cfg(not(debug_assertions))] | ||
warn!("authorization failure for {host}"); | ||
return Err(error::ErrorUnauthorized("unauthorized\n")); | ||
pub(crate) async fn extract_tokens(req: &ServiceRequest) -> Result<HashSet<TokenType>, Error> { | ||
let config = req | ||
.app_data::<web::Data<RwLock<Config>>>() | ||
.map(|cfg| cfg.read()) | ||
.and_then(Result::ok) | ||
.ok_or_else(|| error::ErrorInternalServerError("cannot acquire config"))?; | ||
|
||
let mut user_tokens = HashSet::with_capacity(2); | ||
|
||
let auth_header = req | ||
.headers() | ||
.get(AUTHORIZATION) | ||
.map(|v| v.to_str().unwrap_or_default()) | ||
.map(|v| v.split_whitespace().last().unwrap_or_default()); | ||
|
||
for token_type in [TokenType::Auth, TokenType::Delete] { | ||
let maybe_tokens = config.get_tokens(token_type); | ||
if let Some(configured_tokens) = maybe_tokens { | ||
if configured_tokens.contains(auth_header.unwrap_or_default()) { | ||
user_tokens.insert(token_type); | ||
} | ||
} else if token_type == TokenType::Auth { | ||
// not configured `auth_tokens` means that the user is allowed to access the endpoints | ||
warn!("auth_tokens not configured, allowing the request without auth header"); | ||
user_tokens.insert(token_type); | ||
} else if token_type == TokenType::Delete && req.method() == Method::DELETE { | ||
// explicitly disable `DELETE` methods if no `delete_tokens` are set | ||
warn!("delete endpoints is not served because there are no delete_tokens set"); | ||
Err(error::ErrorNotFound(""))?; | ||
} | ||
} | ||
Ok(()) | ||
|
||
Ok(user_tokens) | ||
} | ||
|
||
/// Returns `HttpResponse` with unauthorized (`401`) error and `unauthorized\n` as body. | ||
pub(crate) fn unauthorized_error() -> actix_web::HttpResponse { | ||
error::ErrorUnauthorized("unauthorized\n").into() | ||
} | ||
|
||
/// Log all unauthorized requests. | ||
pub(crate) fn handle_unauthorized_error<B>( | ||
res: ServiceResponse<B>, | ||
) -> actix_web::Result<ErrorHandlerResponse<B>> { | ||
let connection = res.request().connection_info().clone(); | ||
let host = connection.realip_remote_addr().unwrap_or("unknown host"); | ||
|
||
#[cfg(debug_assertions)] | ||
{ | ||
let auth_header = res | ||
.request() | ||
.headers() | ||
.get(AUTHORIZATION) | ||
.and_then(|v| v.to_str().ok()) | ||
.unwrap_or("none"); | ||
|
||
warn!("authorization failure for {host} (token: {auth_header})",); | ||
} | ||
#[cfg(not(debug_assertions))] | ||
warn!("authorization failure for {host}"); | ||
|
||
Ok(ErrorHandlerResponse::Response(res.map_into_left_body())) | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
use actix_web::http::header::HeaderValue; | ||
use actix_web::test::TestRequest; | ||
use actix_web::web::Data; | ||
use actix_web::HttpResponse; | ||
use awc::http::StatusCode; | ||
|
||
#[actix_web::test] | ||
async fn test_extract_tokens() -> Result<(), Error> { | ||
let mut config = Config::default(); | ||
|
||
// request without configured auth-tokens | ||
let request = TestRequest::default() | ||
.app_data(Data::new(RwLock::new(config.clone()))) | ||
.insert_header((AUTHORIZATION, HeaderValue::from_static("basic test_token"))) | ||
.to_srv_request(); | ||
let tokens = extract_tokens(&request).await?; | ||
assert_eq!(HashSet::from([TokenType::Auth]), tokens); | ||
|
||
// request with configured auth-tokens | ||
config.server.auth_tokens = Some(["test_token".to_string()].into()); | ||
let request = TestRequest::default() | ||
.app_data(Data::new(RwLock::new(config.clone()))) | ||
.insert_header((AUTHORIZATION, HeaderValue::from_static("basic test_token"))) | ||
.to_srv_request(); | ||
let tokens = extract_tokens(&request).await?; | ||
assert_eq!(HashSet::from([TokenType::Auth]), tokens); | ||
|
||
// request with configured auth-tokens but wrong token in request | ||
config.server.auth_tokens = Some(["test_token".to_string()].into()); | ||
let request = TestRequest::default() | ||
.app_data(Data::new(RwLock::new(config.clone()))) | ||
.insert_header(( | ||
AUTHORIZATION, | ||
HeaderValue::from_static("basic invalid_token"), | ||
)) | ||
.to_srv_request(); | ||
let tokens = extract_tokens(&request).await?; | ||
assert_eq!(HashSet::new(), tokens); | ||
|
||
// DELETE request without configured delete-tokens | ||
let request = TestRequest::default() | ||
.method(Method::DELETE) | ||
.app_data(Data::new(RwLock::new(config.clone()))) | ||
.insert_header((AUTHORIZATION, HeaderValue::from_static("basic test_token"))) | ||
.to_srv_request(); | ||
let res = extract_tokens(&request).await; | ||
assert!(res.is_err()); | ||
assert_eq!( | ||
Some(StatusCode::NOT_FOUND), | ||
res.err() | ||
.as_ref() | ||
.map(Error::error_response) | ||
.as_ref() | ||
.map(HttpResponse::status) | ||
); | ||
|
||
// DELETE request with configured delete-tokens | ||
config.server.delete_tokens = Some(["delete_token".to_string()].into()); | ||
let request = TestRequest::default() | ||
.method(Method::DELETE) | ||
.app_data(Data::new(RwLock::new(config.clone()))) | ||
.insert_header(( | ||
AUTHORIZATION, | ||
HeaderValue::from_static("basic delete_token"), | ||
)) | ||
.to_srv_request(); | ||
let tokens = extract_tokens(&request).await?; | ||
assert_eq!(HashSet::from([TokenType::Delete]), tokens); | ||
|
||
// DELETE request with configured delete-tokens but wrong token in request | ||
let request = TestRequest::default() | ||
.method(Method::DELETE) | ||
.app_data(Data::new(RwLock::new(config.clone()))) | ||
.insert_header(( | ||
AUTHORIZATION, | ||
HeaderValue::from_static("basic invalid_token"), | ||
)) | ||
.to_srv_request(); | ||
let tokens = extract_tokens(&request).await?; | ||
assert_eq!(HashSet::new(), tokens); | ||
|
||
#[test] | ||
fn test_check_auth() -> Result<(), Error> { | ||
let mut headers = HeaderMap::new(); | ||
headers.insert(AUTHORIZATION, HeaderValue::from_static("basic test_token")); | ||
assert!(check("", &headers, Some(vec!["test_token".to_string()])).is_ok()); | ||
assert!(check("", &headers, Some(vec!["invalid_token".to_string()])).is_err()); | ||
assert!(check( | ||
"", | ||
&headers, | ||
Some(vec!["invalid1".to_string(), "test_token".to_string()]) | ||
) | ||
.is_ok()); | ||
assert!(check( | ||
"", | ||
&headers, | ||
Some(vec!["invalid1".to_string(), "invalid2".to_string()]) | ||
) | ||
.is_err()); | ||
assert!(check("", &headers, None).is_ok()); | ||
assert!(check("", &HeaderMap::new(), None).is_ok()); | ||
assert!(check("", &HeaderMap::new(), Some(vec!["token".to_string()])).is_err()); | ||
Ok(()) | ||
} | ||
} |
Oops, something went wrong.