Commit 499566af authored by Sophie Wenzel-Teuber's avatar Sophie Wenzel-Teuber
Browse files

More flexible error creation

All different kinds of errors our server sends are in compliance with Amazon S3 errors. They are now stored in one place and should be accessed from anywhere.

Changelog:
* add the file s3_errors.h that contains const objects for all errors that might occur (most of them are not checked yet)
* change struct error_info into a class and add a function to create the XML from the object's variables
* Use these errors in the request handlers
* Add an error object to the s3 authorisation that will always contain "Access Denied" or another error if applicable
* Update tests to test the code generation and error return values
parent 3a557d35
Pipeline #1526 passed with stages
in 7 minutes and 29 seconds
......@@ -50,6 +50,7 @@ The implementation of this class follows the guidelines from [https://docs.aws.a
| [m_queries](#fiphoboserver-s3_utilities-S3_authorisation-m_queries) | a (sorted) list of all queries split up into key and value |
| [m_signature](#fiphoboserver-s3_utilities-S3_authorisation-m_signature) | the signature stored in the request |
| [m_payload](#fiphoboserver-s3_utilities-S3_authorisation-m_payload) | the payload of the request |
| [m_error](#fiphoboserver-s3_utilities-S3_authorisation-m_error) | the error description if one occurred |
## Public Functions
......@@ -58,6 +59,7 @@ The implementation of this class follows the guidelines from [https://docs.aws.a
| [authorise](#fiphoboserver-s3_utilities-S3_authorisation-authorise) | main method to run the authorisation algorithm |
| [add_chunk](#fiphoboserver-s3_utilities-S3_authorisation-add_chunk) | add a chunk of data to the payload |
| [is_valid](#fiphoboserver-s3_utilities-S3_authorisation-is_valid) | checks if the authorisation was successful |
| [get_error](#fiphoboserver-s3_utilities-S3_authorisation-get_error) | returns the internal error |
......@@ -581,6 +583,19 @@ This is the whole body since we do not support multiple chunked signatures
[Go to Top](#fiphoboserver-s3_utilities-S3_authorisation)
### <a name='fiphoboserver-s3_utilities-S3_authorisation-m_error' /> private fiphoboserver::s3_utilities::S3_authorisation::m_error = access_denied
the error description if one occurred
[Go to Top](#fiphoboserver-s3_utilities-S3_authorisation)
## Public Functions
......@@ -665,6 +680,36 @@ checks if the authorisation was successful
#### Qualifiers:
* const
* inline
[Go to Top](#fiphoboserver-s3_utilities-S3_authorisation)
### <a name='fiphoboserver-s3_utilities-S3_authorisation-get_error' /> public [s3_error_info][fiphoboserver-s3_utilities-s3_error_info] fiphoboserver::s3_utilities::S3_authorisation::get_error () const
returns the internal error
#### Returns:
| Type | Description |
| ---- | ---- |
| [s3_error_info][fiphoboserver-s3_utilities-s3_error_info] | the error that might have occurred in the process |
#### Qualifiers:
* const
* inline
......@@ -682,3 +727,4 @@ checks if the authorisation was successful
[fiphoboserver-s3_utilities-S3_authorisation-m_user_identifier]:./S3_authorisation.md#fiphoboserver-s3_utilities-S3_authorisation-m_user_identifier
[fiphoboserver-s3_utilities-S3_authorisation-m_user_key]:./S3_authorisation.md#fiphoboserver-s3_utilities-S3_authorisation-m_user_key
[fiphoboserver-s3_utilities-S3_header]:./S3_header.md
[fiphoboserver-s3_utilities-s3_error_info]:./s3_error_info.md
......@@ -18,7 +18,7 @@ class to extract the S3 specific information from proxygens HTTPMessage headers
| [get_bucket](#fiphoboserver-s3_utilities-S3_header-get_bucket) | extracts the name of the S3 bucket requested in the HTTP message |
| [get_key_without_bucket](#fiphoboserver-s3_utilities-S3_header-get_key_without_bucket) | extracts the name of the key id requested in the HTTP message |
| [get_key](#fiphoboserver-s3_utilities-S3_header-get_key) | extracts the name of the key id requested in the HTTP message |
| [is_create_bucket_request](#fiphoboserver-s3_utilities-S3_header-is_create_bucket_request) | checks if the message we got is one for just creating a bucket |
| [is_bucket_only_request](#fiphoboserver-s3_utilities-S3_header-is_bucket_only_request) | checks if the message we got is only concerning a bucket or a key inside a bucket |
| [get_body_length](#fiphoboserver-s3_utilities-S3_header-get_body_length) | gets the total length of all body chunks that is expected |
| [get_meta_data](#fiphoboserver-s3_utilities-S3_header-get_meta_data) | gets the S3 metadata stored in the HTTP headers |
| [print_all_headers](#fiphoboserver-s3_utilities-S3_header-print_all_headers) | print all headers to std out |
......@@ -162,9 +162,9 @@ This is the whole path argument without the leading '/' but including the bucket
[Go to Top](#fiphoboserver-s3_utilities-S3_header)
### <a name='fiphoboserver-s3_utilities-S3_header-is_create_bucket_request' /> public bool fiphoboserver::s3_utilities::S3_header::is_create_bucket_request () const
### <a name='fiphoboserver-s3_utilities-S3_header-is_bucket_only_request' /> public bool fiphoboserver::s3_utilities::S3_header::is_bucket_only_request () const
checks if the message we got is one for just creating a bucket
checks if the message we got is only concerning a bucket or a key inside a bucket
......@@ -172,7 +172,7 @@ checks if the message we got is one for just creating a bucket
#### Returns:
| Type | Description |
| ---- | ---- |
| bool | true if these headers belong to a create bucket request, false for all other requests |
| bool | true if the path in these headers only has a bucket name false if it contains a key |
......@@ -180,7 +180,7 @@ checks if the message we got is one for just creating a bucket
This is convenient since we don't really support buckets themselves and can easily stop working if this returns true
The way this is decided currently is by checking if there is a '/' in the path as this would split the bucket name from the key. This might not be ideal...
......
......@@ -9,7 +9,7 @@ namespace for S3 specific utilities
| Name | Description |
| ---- | ---- |
| [S3_authorisation](./S3_authorisation.md) | class to perform an authorisation of an S3 request. |
| [s3_error_info](./s3_error_info.md) | struct to contain all the information needed to send a proper s3 error message |
| [s3_error_info](./s3_error_info.md) | class to contain all the information needed to send a proper s3 error message |
| [S3_header](./S3_header.md) | class to extract the S3 specific information from proxygens HTTPMessage headers |
......@@ -54,7 +54,7 @@ enum to store all the different states a [fiphoboserver::s3_utilities::S3_author
[Go to Top](#fiphoboserver-s3_utilities)
## Functions
### <a name='fiphoboserver-s3_utilities-create_s3_error' /> public [s3_error_info][fiphoboserver-s3_utilities-s3_error_info] fiphoboserver::s3_utilities::create_s3_error (const int return_value, const std::string request)
### <a name='fiphoboserver-s3_utilities-create_s3_error' /> public [s3_error_info][fiphoboserver-s3_utilities-s3_error_info] fiphoboserver::s3_utilities::create_s3_error (const int return_value)
create a [s3_error_info][fiphoboserver-s3_utilities-s3_error_info] depending on the error value
......@@ -64,8 +64,7 @@ create a [s3_error_info][fiphoboserver-s3_utilities-s3_error_info] depending on
#### Parameters:
| Type | Name | Description |
| ---- | ---- | ---- |
| const int | return_value | the error code (errno, etc) that the function returned that raised the exception that preceded the call to this function |
| const std::string | request | the request path that was being processed |
| const int | return_value | the error code (errno, etc) that the function returned that raised the exception that preceded the call to this function |
#### Returns:
| Type | Description |
......@@ -240,4 +239,4 @@ create a key hashed message
[Go to Top](#fiphoboserver-s3_utilities)
[fiphoboserver-s3_utilities-S3_authorisation]:./S3_authorisation.md
[fiphoboserver-s3_utilities-s3_error_info]:./s3_error_info.md#fiphoboserver-s3_utilities-s3_error_info
[fiphoboserver-s3_utilities-s3_error_info]:./s3_error_info.md
# <a name='fiphoboserver-s3_utilities-s3_error_info' /> public fiphoboserver::s3_utilities::s3_error_info
struct to contain all the information needed to send a proper s3 error message
class to contain all the information needed to send a proper s3 error message
......@@ -10,7 +10,15 @@ struct to contain all the information needed to send a proper s3 error message
| ---- | ---- |
| [https_error_code](#fiphoboserver-s3_utilities-s3_error_info-https_error_code) | https error code (400s or 500s) |
| [https_error_identifier](#fiphoboserver-s3_utilities-s3_error_info-https_error_identifier) | readable string for the code [https_error_code][fiphoboserver-s3_utilities-s3_error_info-https_error_code] |
| [error_xml](#fiphoboserver-s3_utilities-s3_error_info-error_xml) | XML encoded message containing more information on the error. |
| [https_error_message](#fiphoboserver-s3_utilities-s3_error_info-https_error_message) | human readable message with information on the error |
## Public Functions
| Name | Description |
| ---- | ---- |
| [get_xml](#fiphoboserver-s3_utilities-s3_error_info-get_xml) | created a XML message containing all the error information |
| [operator==](#fiphoboserver-s3_utilities-s3_error_info-operator==) | equality operator |
| [operator!=](#fiphoboserver-s3_utilities-s3_error_info-operator!=) | inequality operator |
......@@ -41,17 +49,125 @@ readable string for the code [https_error_code][fiphoboserver-s3_utilities-s3_er
[Go to Top](#fiphoboserver-s3_utilities-s3_error_info)
### <a name='fiphoboserver-s3_utilities-s3_error_info-error_xml' /> public fiphoboserver::s3_utilities::s3_error_info::error_xml
### <a name='fiphoboserver-s3_utilities-s3_error_info-https_error_message' /> public fiphoboserver::s3_utilities::s3_error_info::https_error_message
human readable message with information on the error
[Go to Top](#fiphoboserver-s3_utilities-s3_error_info)
## Public Functions
### <a name='fiphoboserver-s3_utilities-s3_error_info-get_xml' /> public std::string fiphoboserver::s3_utilities::s3_error_info::get_xml (const std::string request) const
created a XML message containing all the error information
#### Parameters:
| Type | Name | Description |
| ---- | ---- | ---- |
| const std::string | request | the request that was processed when this error occurred |
#### Returns:
| Type | Description |
| ---- | ---- |
| std::string | a string containg an S3 compatible XML error message |
#### Qualifiers:
* const
[Go to Top](#fiphoboserver-s3_utilities-s3_error_info)
### <a name='fiphoboserver-s3_utilities-s3_error_info-operator==' /> public bool fiphoboserver::s3_utilities::s3_error_info::operator== (const s3_error_info &other) const
equality operator
#### Parameters:
| Type | Name | Description |
| ---- | ---- | ---- |
| const [s3_error_info][fiphoboserver-s3_utilities-s3_error_info] & | other | the other error info object to compare |
#### Returns:
| Type | Description |
| ---- | ---- |
| bool | |
return true if all inner members are equal, false otherwise
#### Qualifiers:
* const
* inline
[Go to Top](#fiphoboserver-s3_utilities-s3_error_info)
### <a name='fiphoboserver-s3_utilities-s3_error_info-operator!=' /> public bool fiphoboserver::s3_utilities::s3_error_info::operator!= (const s3_error_info &other) const
inequality operator
#### Parameters:
| Type | Name | Description |
| ---- | ---- | ---- |
| const [s3_error_info][fiphoboserver-s3_utilities-s3_error_info] & | other | the other error info object to compare |
#### Returns:
| Type | Description |
| ---- | ---- |
| bool | |
XML encoded message containing more information on the error.
return true if any inner member is different, false otherwise (all equal)
#### Qualifiers:
* const
* inline
[Go to Top](#fiphoboserver-s3_utilities-s3_error_info)
[fiphoboserver-s3_utilities-s3_error_info]:./s3_error_info.md
[fiphoboserver-s3_utilities-s3_error_info-https_error_code]:./s3_error_info.md#fiphoboserver-s3_utilities-s3_error_info-https_error_code
......@@ -36,8 +36,12 @@ void GetRequestHandler::onRequest(
s3_utilities::S3_authorisation auth;
if (auth.authorise(m_s3_header)
!= s3_utilities::Authorisation_status::valid) {
s3_utilities::s3_error_info auth_error = auth.get_error();
proxygen::ResponseBuilder(downstream_)
.status(403, "Access Denied")
.status(
auth_error.https_error_code, auth_error.https_error_identifier)
.body(auth_error.get_xml(m_s3_header.get_key()))
.sendWithEOM();
return;
}
......@@ -69,11 +73,11 @@ void GetRequestHandler::onRequest(
catch (const FIPhoboServerException& ex) {
std::cerr << "Caught exception: " << ex.what() << '\n';
s3_utilities::s3_error_info error = s3_utilities::create_s3_error(
ex.get_inner_error(), m_s3_header.get_key());
s3_utilities::s3_error_info error =
s3_utilities::create_s3_error(ex.get_inner_error());
proxygen::ResponseBuilder(downstream_)
.status(error.https_error_code, error.https_error_identifier)
.body(error.error_xml)
.body(error.get_xml(m_s3_header.get_key()))
.sendWithEOM();
return;
......@@ -129,13 +133,13 @@ void GetRequestHandler::read_file(folly::EventBase* evb)
}
catch (const FIPhoboServerException& ex) {
std::cerr << "Read error=" << ex.what() << '\n';
s3_utilities::s3_error_info error = s3_utilities::create_s3_error(
ex.get_inner_error(), m_s3_header.get_key());
s3_utilities::s3_error_info error =
s3_utilities::create_s3_error(ex.get_inner_error());
evb->runInEventBaseThread([this, error] {
proxygen::ResponseBuilder(downstream_)
.status(
error.https_error_code, error.https_error_identifier)
.body(error.error_xml)
.body(error.get_xml(m_s3_header.get_key()))
.sendWithEOM();
});
break;
......
......@@ -34,8 +34,12 @@ void GetmdRequestHandler::onRequest(
s3_utilities::S3_authorisation auth;
if (auth.authorise(m_s3_header)
!= s3_utilities::Authorisation_status::valid) {
s3_utilities::s3_error_info auth_error = auth.get_error();
proxygen::ResponseBuilder(downstream_)
.status(403, "Access Denied")
.status(
auth_error.https_error_code, auth_error.https_error_identifier)
.body(auth_error.get_xml(m_s3_header.get_key()))
.sendWithEOM();
return;
}
......@@ -56,11 +60,11 @@ void GetmdRequestHandler::onRequest(
catch (const FIPhoboServerException& ex) {
std::cerr << "Caught exception: " << ex.what() << '\n';
s3_utilities::s3_error_info error = s3_utilities::create_s3_error(
ex.get_inner_error(), m_s3_header.get_key());
s3_utilities::s3_error_info error =
s3_utilities::create_s3_error(ex.get_inner_error());
proxygen::ResponseBuilder(downstream_)
.status(error.https_error_code, error.https_error_identifier)
.body(error.error_xml)
.body(error.get_xml(m_s3_header.get_key()))
.sendWithEOM();
return;
}
......
......@@ -31,14 +31,18 @@ void PutRequestHandler::onRequest(
if (m_auth.authorise(m_s3_header)
!= s3_utilities::Authorisation_status::waiting_for_payload) {
s3_utilities::s3_error_info auth_error = m_auth.get_error();
proxygen::ResponseBuilder(downstream_)
.status(403, "Access Denied")
.status(
auth_error.https_error_code, auth_error.https_error_identifier)
.body(auth_error.get_xml(m_s3_header.get_key()))
.sendWithEOM();
m_stop_processing = true;
return;
}
if (m_s3_header.is_create_bucket_request()) {
if (m_s3_header.is_bucket_only_request()) {
/*
* Ignore, since we don't really have buckets.
* Or do we want a list of the actual buckets to give an error, when a
......@@ -64,11 +68,10 @@ void PutRequestHandler::onRequest(
}
catch (const FIPhoboServerException& ex) {
std::cerr << "Caught exception: " << ex.what() << '\n';
s3_utilities::s3_error_info error = s3_utilities::create_s3_error(
ex.get_inner_error(), m_s3_header.get_key());
s3_utilities::s3_error_info error = s3_utilities::no_such_bucket;
proxygen::ResponseBuilder(downstream_)
.status(error.https_error_code, error.https_error_identifier)
.body(error.error_xml)
.body(error.get_xml(m_s3_header.get_bucket()))
.sendWithEOM();
m_stop_processing = true;
return;
......@@ -92,11 +95,11 @@ void PutRequestHandler::onBody(std::unique_ptr<folly::IOBuf> body) noexcept
}
catch (const FIPhoboServerException& ex) {
std::cerr << "Caught an exception in put: " << ex.what() << '\n';
s3_utilities::s3_error_info error = s3_utilities::create_s3_error(
ex.get_inner_error(), m_s3_header.get_key());
s3_utilities::s3_error_info error =
s3_utilities::create_s3_error(ex.get_inner_error());
proxygen::ResponseBuilder(downstream_)
.status(error.https_error_code, error.https_error_identifier)
.body(error.error_xml)
.body(error.get_xml(m_s3_header.get_key()))
.sendWithEOM();
m_stop_processing = true;
return;
......@@ -114,8 +117,11 @@ void PutRequestHandler::onEOM() noexcept
if (m_auth.authorise(m_s3_header)
!= s3_utilities::Authorisation_status::valid) {
// TODO: Delete object from phobos or revert put! (Issue #45)
s3_utilities::s3_error_info auth_error = m_auth.get_error();
proxygen::ResponseBuilder(downstream_)
.status(403, "Access Denied")
.status(
auth_error.https_error_code, auth_error.https_error_identifier)
.body(auth_error.get_xml(m_s3_header.get_key()))
.sendWithEOM();
return;
......@@ -128,11 +134,11 @@ void PutRequestHandler::onEOM() noexcept
catch (const FIPhoboServerException& ex) {
std::cerr << "Caught an exception in finish put: " << ex.what()
<< '\n';
s3_utilities::s3_error_info error = s3_utilities::create_s3_error(
ex.get_inner_error(), m_s3_header.get_key());
s3_utilities::s3_error_info error =
s3_utilities::create_s3_error(ex.get_inner_error());
proxygen::ResponseBuilder(downstream_)
.status(error.https_error_code, error.https_error_identifier)
.body(error.error_xml)
.body(error.get_xml(m_s3_header.get_key()))
.sendWithEOM();
return;
}
......
......@@ -64,8 +64,9 @@ bool S3_authorisation::set_authorisation_info(const S3_header& headers)
headers.get_header_by_name("Authorization");
if (full_authorisation.rfind("AWS4-HMAC-SHA256", 0) != 0) {
std::cerr << "Could not find AWS4-HMAC-SHA256 at the beginning of ";
std::cerr << full_authorisation << std::endl;
// std::cerr << "Could not find AWS4-HMAC-SHA256 at the beginning of ";
// std::cerr << full_authorisation << std::endl;
m_error = invalid_signature_type;
return false;
}
......@@ -77,6 +78,7 @@ bool S3_authorisation::set_authorisation_info(const S3_header& headers)
if (credentials.empty() || all_signed_headers.empty()
|| m_signature.empty()) {
m_error = malformed_authorisation;
return false;
}
......@@ -174,12 +176,14 @@ bool S3_authorisation::search_for_user()
// TODO: Proper database: Issue #41
std::ifstream user_database("/tmp/users.txt");
if (!user_database.good()) {
m_error = internal_error;
return false;
}
std::string user;
while (user_database >> user) {
int colon = user.find(':');
if (colon == std::string::npos) {
m_error = internal_error;
return false; // Database corrupted
}
std::string user_id = user.substr(0, colon);
......@@ -189,19 +193,18 @@ bool S3_authorisation::search_for_user()
return true;
}
}
m_error = s3_error_info(access_denied);
return false;
}
bool S3_authorisation::is_chunked(const S3_header& headers) const
{
std::string content_md5 = headers.get_header_by_name("content-md5");
std::string content_length = headers.get_header_by_name("Content-Length");
if (!content_md5.empty() || !content_length.empty()) {
return true;
if (content_length.empty()) {
return false;
}
return false;
return true;
}
void S3_authorisation::print_info() const
......@@ -307,7 +310,6 @@ std::string S3_authorisation::create_string_to_sign(
std::string timestamp = headers.get_header_by_name("x-amz-date");
if (timestamp.empty()) {
// I don't have to check this return value anywhere because it will
// let the signature fail in the end
return "";
}
result_stream << timestamp << '\n';
......
#pragma once
#include "s3_errors.h"
#include "s3_header.h"
#include <string>
......@@ -59,6 +60,13 @@ class S3_authorisation {
///
bool is_valid() const { return m_status == Authorisation_status::valid; }
///
/// @brief returns the internal error
///
/// @return the error that might have occurred in the process
///
s3_error_info get_error() const { return m_error; }
private:
/// @name Helper functions
///
......@@ -227,6 +235,8 @@ class S3_authorisation {
/// This is the whole body since we do not support multiple chunked
/// signatures
std::string m_payload = "";
/// @brief the error description if one occurred
s3_error_info m_error = access_denied;
};
} // namespace s3_utilities
......
#pragma once
#include <string>
#include "s3_utilities.h"
namespace fiphoboserver {
namespace s3_utilities {
const s3_error_info access_denied = {403, "AccessDenied", "Access Denied"};
const s3_error_info bucket_exists = {
409, "BucketAlreadyExists",
"The requested bucket name is not available. The bucket namespace is shared by all users of the system. Please select a different name and try again. "};
const s3_error_info bucket_not_empty = {
409, "BucketNotEmpty", "The bucket you tried to delete is not empty."};
const s3_error_info incomplete_body = {
400, "IncompleteBody",
"You did not provide the number of bytes specified by the Content-Length HTTP header. "};
const s3_error_info internal_error = {
500, "InternalError",
"We encountered an internal error. Please try again."};
const s3_error_info invalid_argument = {400, "InvalidArgument",
"Invalid Argument"};
const s3_error_info invalid_bucketname = {400, "InvalidBucketName",
"The specified bucket is not valid."};
const s3_error_info invalid_key = {403, "InvalidAccessKeyId",
"The given kay is not valid"};
const s3_error_info invalid_signature_type = {400, "InvalidRequest",
"Please use AWS4-HMAC-SHA256."};
const s3_error_info malformed_authorisation = {
400, "AuthorizationHeaderMalformed",
"The authorization header you provided is invalid."};
const s3_error_info missing_content_length = {
411, "MissingContentLength",
"You must provide the Content-Length HTTP header."};
const s3_error_info no_such_bucket = {404, "NoSuchBucket",
"The specified bucket does not exist."};
const s3_error_info no_such_key = {404, "NoSuchKey",
"The specified key does not exist."};
const s3_error_info no_such_version = {
404, "NoSuchVersion",
"The version ID specified in the request does not match any existing version. "};
const s3_error_info not_implemented = {
404, "NotImplemented",
"A header you provided implies functionality that is not implemented. "};
const s3_error_info signature_incorrect = {
403, "SignatureDoesNotMatch",
"The request signature we calculated does not match the signature you provided."};
} // namespace s3_utilities
} // namespace fiphoboserver
......@@ -56,11 +56,17 @@ class S3_header {
if (!m_headers) {
return "";
}
std::string path = m_headers->getPath();
path = path.substr(1, path