Skip to content

Commit

Permalink
Refactor the Uri class and introduce new PathQueryFragment class
Browse files Browse the repository at this point in the history 
[Issue #60]

Signed-off-by: Tyler Schultz <[email protected]>
  • Loading branch information
@tylerschultz
Peter Chen authored and tylerschultz committed Mar 12, 2020
1 parent 9b378f6 commit 278cba3
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 126 deletions.
64 changes: 37 additions & 27 deletions src/common/http/http.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ absl::optional<std::map<std::string, std::string>> Http::DecodeCookies(
return result;
}

Uri::Uri(absl::string_view uri) {
Uri::Uri(absl::string_view uri) : pathQueryFragment_("/") {
if (uri.find(https_prefix_) != 0) { // must start with https://
throw std::runtime_error(absl::StrCat("uri must be https scheme: ", uri));
}
Expand All @@ -256,11 +256,13 @@ Uri::Uri(absl::string_view uri) {
}
host_and_port = std::string(uri_without_scheme.substr(0, end_of_host_and_port_index).data(),
end_of_host_and_port_index);
pathQueryFragment_ = std::string(uri_without_scheme.substr(end_of_host_and_port_index).data());
if (!absl::StartsWith(pathQueryFragment_, "/")) {
pathQueryFragment_ = "/" + pathQueryFragment_;
pathQueryFragmentString_ = std::string(uri_without_scheme.substr(end_of_host_and_port_index).data());
if (!absl::StartsWith(pathQueryFragmentString_, "/")) {
pathQueryFragmentString_ = "/" + pathQueryFragmentString_;
}

pathQueryFragment_ = http::PathQueryFragment(pathQueryFragmentString_);

auto colon_position = host_and_port.find(':');
if (colon_position == 0) {
throw std::runtime_error(absl::StrCat("no host in uri: ", uri));
Expand All @@ -283,48 +285,56 @@ Uri::Uri(absl::string_view uri) {

const std::string Uri::https_prefix_ = "https://";

Uri& Uri::operator=(Uri &&uri) noexcept {
Uri &Uri::operator=(Uri &&uri) noexcept {
host_ = uri.host_;
port_ = uri.port_;
pathQueryFragmentString_ = uri.pathQueryFragmentString_;
pathQueryFragment_ = uri.pathQueryFragment_;
return *this;
}

Uri::Uri(const Uri &uri) {
host_ = uri.host_;
port_ = uri.port_;
pathQueryFragment_ = uri.pathQueryFragment_;
Uri::Uri(const Uri &uri)
: host_(uri.host_),
port_(uri.port_),
pathQueryFragmentString_(uri.pathQueryFragmentString_),
pathQueryFragment_(uri.pathQueryFragment_) {
}

Uri Http::ParseUri(absl::string_view uri) {
return Uri(uri);
std::string Uri::Path() {
return pathQueryFragment_.Path();
}

std::array<std::string, 3> Http::DecodePath(absl::string_view path) {
std::string Uri::Fragment() {
return pathQueryFragment_.Fragment();
}

std::string Uri::Query() {
return pathQueryFragment_.Query();
}

PathQueryFragment::PathQueryFragment(absl::string_view path_query_fragment) {
// See https://tools.ietf.org/html/rfc3986#section-3.4 and https://tools.ietf.org/html/rfc3986#section-3.5
std::array<std::string, 3> result;
auto question_mark_position = path.find('?');
auto hashtag_position = path.find("#");
auto question_mark_position = path_query_fragment.find('?');
auto hashtag_position = path_query_fragment.find("#");
if (question_mark_position == absl::string_view::npos && hashtag_position == absl::string_view::npos) {
result[0] = std::string(path.data());
path_ = std::string(path_query_fragment.data());
} else if (question_mark_position == absl::string_view::npos) {
result[0] = std::string(path.substr(0, hashtag_position).data(), hashtag_position);
result[2] = std::string(path.substr(hashtag_position + 1).data());
path_ = std::string(path_query_fragment.substr(0, hashtag_position).data(), hashtag_position);
fragment_ = std::string(path_query_fragment.substr(hashtag_position + 1).data());
} else if (hashtag_position == absl::string_view::npos) {
result[0] = std::string(path.substr(0, question_mark_position).data(), question_mark_position);
result[1] = std::string(path.substr(question_mark_position + 1).data());
path_ = std::string(path_query_fragment.substr(0, question_mark_position).data(), question_mark_position);
query_ = std::string(path_query_fragment.substr(question_mark_position + 1).data());
} else {
if (question_mark_position < hashtag_position) {
auto query_length = hashtag_position - question_mark_position - 1;
result[0] = std::string(path.substr(0, question_mark_position).data(), question_mark_position);
result[1] = std::string(path.substr(question_mark_position + 1, query_length).data(), query_length);
result[2] = std::string(path.substr(hashtag_position + 1).data());
path_ = std::string(path_query_fragment.substr(0, question_mark_position).data(), question_mark_position);
query_ = std::string(path_query_fragment.substr(question_mark_position + 1, query_length).data(), query_length);
fragment_ = std::string(path_query_fragment.substr(hashtag_position + 1).data());
} else {
result[0] = std::string(path.substr(0, hashtag_position).data(), hashtag_position);
result[2] = std::string(path.substr(hashtag_position + 1).data());
path_ = std::string(path_query_fragment.substr(0, hashtag_position).data(), hashtag_position);
fragment_ = std::string(path_query_fragment.substr(hashtag_position + 1).data());
}
}
return result;
}

response_t HttpImpl::Post(absl::string_view uri,
Expand All @@ -349,7 +359,7 @@ response_t HttpImpl::Post(absl::string_view uri,
}
}

auto parsed_uri = http::Http::ParseUri(uri);
auto parsed_uri = http::Uri(uri);

tcp::resolver resolver(ioc);
beast::ssl_stream<beast::tcp_stream> stream(ioc, ctx);
Expand Down
61 changes: 44 additions & 17 deletions src/common/http/http.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,68 @@ typedef std::shared_ptr<Http> ptr_t;
typedef std::unique_ptr<beast::http::response<beast::http::string_body>>
response_t;

class PathQueryFragment {
private:
std::string path_;
std::string query_;
std::string fragment_;

public:
explicit PathQueryFragment(absl::string_view path_query_fragment);

inline const std::string &Path() const {
return path_;
}

inline const std::string &Query() const {
return query_;
}

inline bool HasQuery() const {
return !query_.empty();
}

inline const std::string &Fragment() const {
return fragment_;
}

inline bool HasFragment() const {
return !fragment_.empty();
}
};

class Uri {
private:
static const std::string https_prefix_;
std::string host_;
int32_t port_ = 443;
std::string pathQueryFragment_; // includes the path, query, and fragment (if any)
std::string pathQueryFragmentString_; // includes the path, query, and fragment (if any)
PathQueryFragment pathQueryFragment_;

public:
explicit Uri(absl::string_view uri);

Uri(const Uri &uri);

Uri& operator=(Uri &&uri) noexcept;
Uri &operator=(Uri &&uri) noexcept;

inline std::string Scheme() { return "https"; }

inline std::string Host() { return host_; }

inline int32_t Port() { return port_; }

inline std::string PathQueryFragment() { return pathQueryFragment_; }
inline std::string PathQueryFragment() { return pathQueryFragmentString_; }

std::string Path();

std::string Query();

inline bool HasQuery() const { return pathQueryFragment_.HasQuery(); };

std::string Fragment();

inline bool HasFragment() const { return pathQueryFragment_.HasFragment(); };
};

class Http {
Expand Down Expand Up @@ -131,20 +172,6 @@ class Http {
static absl::optional<std::map<std::string, std::string>> DecodeCookies(
absl::string_view cookies);

/**
* Decode a uri into a scheme, host, port, and path.
* @param uri string
* @return the decoded Uri
*/
static Uri ParseUri(absl::string_view uri);

/**
* Decode a path into a path, query and fragment triple.
* @param path the path to decode
* @return the decoded triple
*/
static std::array<std::string, 3> DecodePath(absl::string_view path);

/**
* Virtual destructor
*/
Expand Down
17 changes: 8 additions & 9 deletions src/config/get_config.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,24 @@
#include "src/common/http/http.h"
#include "absl/strings/string_view.h"
#include <fmt/ostream.h>
#include <memory>

using namespace std;
using namespace google::protobuf::util;

namespace authservice {
namespace config {

void validateUri(absl::string_view uri, absl::string_view uri_name) {
array<string, 3> path_query_fragment_array;
void ValidateUri(absl::string_view uri, absl::string_view uri_name) {
unique_ptr<common::http::Uri> parsed_uri;
try {
auto parsed_uri = common::http::Http::ParseUri(uri);
path_query_fragment_array = common::http::Http::DecodePath(parsed_uri.PathQueryFragment());
parsed_uri = unique_ptr<common::http::Uri>(new common::http::Uri(uri));
} catch (runtime_error &e) {
throw runtime_error(fmt::format("invalid {}: ", uri_name) + e.what());
}
if (!path_query_fragment_array[1].empty() || !path_query_fragment_array[2].empty()) {
if (parsed_uri->HasQuery() || parsed_uri->HasFragment()) {
throw runtime_error(fmt::format("invalid {}: query params and fragments not allowed: {}", uri_name, uri));
}

}

unique_ptr<Config> GetConfig(const string &configFileName) {
Expand All @@ -50,9 +49,9 @@ unique_ptr<Config> GetConfig(const string &configFileName) {
}

for (const auto &chain : config->chains()) {
validateUri(chain.filters(0).oidc().authorization_uri(), "authorization_uri");
validateUri(chain.filters(0).oidc().callback_uri(), "callback_uri");
validateUri(chain.filters(0).oidc().token_uri(), "token_uri");
ValidateUri(chain.filters(0).oidc().authorization_uri(), "authorization_uri");
ValidateUri(chain.filters(0).oidc().callback_uri(), "callback_uri");
ValidateUri(chain.filters(0).oidc().token_uri(), "token_uri");
}

return config;
Expand Down
10 changes: 5 additions & 5 deletions src/filters/oidc/oidc_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,11 +355,11 @@ bool OidcFilter::MatchesLogoutRequest(const ::envoy::service::auth::v2::CheckReq
}

std::string OidcFilter::RequestPath(const CheckRequest *request) {
return common::http::Http::DecodePath(request->attributes().request().http().path())[0];
return common::http::PathQueryFragment(request->attributes().request().http().path()).Path();
}

std::string OidcFilter::RequestQueryString(const CheckRequest *request) {
return common::http::Http::DecodePath(request->attributes().request().http().path())[1];
return common::http::PathQueryFragment(request->attributes().request().http().path()).Query();
}

bool OidcFilter::MatchesCallbackRequest(const ::envoy::service::auth::v2::CheckRequest *request) {
Expand All @@ -368,9 +368,9 @@ bool OidcFilter::MatchesCallbackRequest(const ::envoy::service::auth::v2::CheckR
auto scheme = request->attributes().request().http().scheme();
spdlog::trace("{}: checking handler for {}://{}{}", __func__, scheme, request_host, path);

auto request_path_parts = common::http::Http::DecodePath(path);
auto request_path_parts = common::http::PathQueryFragment(path);
auto configured_uri = idp_config_.callback_uri();
auto parsed_uri = common::http::Http::ParseUri(configured_uri);
auto parsed_uri = common::http::Uri(configured_uri);
auto configured_port = parsed_uri.Port();
auto configured_hostname = parsed_uri.Host();
auto configured_scheme = parsed_uri.Scheme();
Expand All @@ -381,7 +381,7 @@ bool OidcFilter::MatchesCallbackRequest(const ::envoy::service::auth::v2::CheckR

std::string configured_callback_host_with_port = buf.str();

bool path_matches = request_path_parts[0] == configured_path;
bool path_matches = request_path_parts.Path() == configured_path;

bool host_matches = request_host == configured_callback_host_with_port ||
(configured_scheme == "https" && configured_port == 443 && request_host == configured_hostname);
Expand Down
2 changes: 1 addition & 1 deletion src/service/service_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ ::grpc::Status AuthServiceImpl::Check(
::envoy::service::auth::v2::CheckResponse *response) {
spdlog::trace("{}", __func__);
try {
auto request_path = common::http::Http::DecodePath(request->attributes().request().http().path())[0];
auto request_path = common::http::PathQueryFragment(request->attributes().request().http().path()).Path();
if (!common::utilities::trigger_rules::TriggerRuleMatchesPath(request_path, trigger_rules_config_)) {
spdlog::debug(
"{}: no matching trigger rule, so allowing request to proceed without any authservice functionality {}://{}{} ",
Expand Down
Loading

0 comments on commit 278cba3

Please sign in to comment.