-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
WIP: http_filter: REST-based external authentication filter #2621
Closed
Closed
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
0dfa5a9
Build extauth filter as part of Envoy core
845ad8b
Support path_prefix for extauth.
25fe00a
Support adding headers from the auth server, with configuration speci…
e578680
Correctly support invalidating the route cache when adding headers.
1f45772
Rename request to reqmsg just for my own sanity in keeping track of w…
8d95f25
Invert the allowed-header check so that we're iterating the vector an…
891006a
Add comments, remove dead code, clean up logging.
b7ce768
Do not overwrite the Host header when talking to the extauth service …
70e29a5
Re-sort for check_format. Sigh.
0f18d0b
Let's see about fixing the clang errors from CI.
69d1d97
Merge remote-tracking branch 'upstream/master' into datawire/extauth
7a0d470
filter_auth: initial code review changes
3e020b6
filter_auth: code review changes
44ddcb9
filter_auth: changed server config to use std make_shared
e332b8a
filter_auth: fixed sanitizer errors
8585489
filter_auth: request_headers_ initialization
f36184e
forcing ci.
546f6bb
Merge branch 'datawire/extauth' of github.com:datawire/envoy into dat…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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,288 @@ | ||
#include "common/http/filter/extauth.h" | ||
|
||
#include "common/common/assert.h" | ||
#include "common/common/enum_to_int.h" | ||
#include "common/http/message_impl.h" | ||
#include "common/http/utility.h" | ||
|
||
#include "absl/strings/str_cat.h" | ||
|
||
namespace Envoy { | ||
namespace Http { | ||
|
||
namespace { | ||
|
||
const LowerCaseString header_to_add() { | ||
CONSTRUCT_ON_FIRST_USE(LowerCaseString, "x-ambassador-calltype"); | ||
} | ||
|
||
const std::string value_to_add() { CONSTRUCT_ON_FIRST_USE(std::string, "extauth-request"); } | ||
|
||
} // namespace | ||
|
||
ExtAuth::ExtAuth(ExtAuthConfigConstSharedPtr config) : config_(std::move(config)) {} | ||
|
||
ExtAuth::~ExtAuth() { ASSERT(!auth_request_); } | ||
|
||
void ExtAuth::dumpHeaders(const char* what, HeaderMap* headers) { | ||
#ifndef NVLOG | ||
ENVOY_STREAM_LOG(trace, "ExtAuth headers ({}):", *callbacks_, what); | ||
if (headers) { | ||
headers->iterate( | ||
[](const HeaderEntry& header, void* context) -> HeaderMap::Iterate { | ||
ENVOY_STREAM_LOG(trace, " '{}':'{}'", | ||
*static_cast<StreamDecoderFilterCallbacks*>(context), | ||
header.key().c_str(), header.value().c_str()); | ||
return HeaderMap::Iterate::Continue; | ||
}, | ||
static_cast<void*>(callbacks_)); | ||
} | ||
#endif | ||
} | ||
|
||
// TODO(gsagula): at the end of this PR, most of the comments inside member functions should be | ||
// removed. | ||
FilterHeadersStatus ExtAuth::decodeHeaders(HeaderMap& headers, bool) { | ||
|
||
// decodeHeaders is called at the point that the HTTP machinery handling | ||
// the request has parsed the HTTP headers for this request. | ||
// Our primary job here is to construct the request to the auth service | ||
// and start it executing, but we also have to be sure to save a pointer | ||
// to the incoming request headers in case we need to modify them in | ||
// flight. | ||
|
||
// Remember that we have _not_ finished talking to the auth service... | ||
|
||
auth_complete_ = false; | ||
|
||
// ...and hang onto a pointer to the original request headers. | ||
// | ||
// Note that using a reference here won't work. Move semantics are the | ||
// root of this issue, I think. | ||
|
||
request_headers_ = &headers; | ||
|
||
// Debugging. | ||
dumpHeaders("decodeHeaders", request_headers_); | ||
|
||
// OK, time to get the auth-service request set up. Create a | ||
// RequestMessageImpl to hold all the details, and start it off as a | ||
// copy of the incoming request's headers. | ||
|
||
MessagePtr request_message{new RequestMessageImpl{HeaderMapPtr{new HeaderMapImpl{headers}}}}; | ||
|
||
// We do need to tweak a couple of things. To start with, has a change | ||
// to the path we hand to the auth service been configured? | ||
|
||
if (!config_->path_prefix_.empty()) { | ||
// Yes, it has. Go ahead and prepend it to the request_message path. | ||
std::string path; | ||
absl::StrAppend(&path, config_->path_prefix_, | ||
request_message->headers().insertPath().value().getString()); | ||
request_message->headers().insertPath().value(path); | ||
} | ||
|
||
// https://github.com/datawire/ambassador/issues/154 | ||
// We used to reset the Host: header to match the cluster name we're about | ||
// to send the auth request to. That, however, causes trouble for anyone | ||
// who wants to make auth decisions based on the host to which the client | ||
// started out trying to talk to. | ||
// | ||
// We may need to make this configurable later, so I'm leaving this line | ||
// in for reference. | ||
// request_message->headers().insertHost().value(config_->cluster_); | ||
|
||
// After setting up whatever headers we need to, make sure the body is | ||
// correctly marked as empty. | ||
|
||
request_message->headers().insertContentLength().value(uint64_t(0)); | ||
|
||
// Finally, we mark the request as being an Ambassador auth request. | ||
|
||
request_message->headers().addReference(header_to_add(), value_to_add()); | ||
|
||
// Fire the request up. When it's finished, we'll get a call to | ||
// either onSuccess() or onFailure(). | ||
|
||
ENVOY_STREAM_LOG(trace, "ExtAuth contacting auth server", *callbacks_); | ||
|
||
auth_request_ = config_->cm_.httpAsyncClientForCluster(config_->cluster_) | ||
.send(std::move(request_message), *this, | ||
Optional<std::chrono::milliseconds>(config_->timeout_)); | ||
|
||
// It'll take some time for our auth call to complete. Stop | ||
// filtering while we wait for it. | ||
|
||
return FilterHeadersStatus::StopIteration; | ||
} | ||
|
||
FilterDataStatus ExtAuth::decodeData(Buffer::Instance&, bool) { | ||
// decodeHeaders is called at the point that the HTTP machinery handling | ||
// the request has parsed the HTTP body for this request. We don't need | ||
// to do anything special here; we just need to make sure that we don't | ||
// let things proceed until our auth call is done. | ||
if (auth_complete_) { | ||
return FilterDataStatus::Continue; | ||
} | ||
return FilterDataStatus::StopIterationAndBuffer; | ||
} | ||
|
||
FilterTrailersStatus ExtAuth::decodeTrailers(HeaderMap&) { | ||
// decodeTrailers is called at the point that the HTTP machinery handling | ||
// the request has parsed the HTTP trailers for this request. We don't need | ||
// to do anything special here; we just need to make sure that we don't | ||
// let things proceed until our auth call is done. | ||
if (auth_complete_) { | ||
return FilterTrailersStatus::Continue; | ||
} | ||
return FilterTrailersStatus::StopIteration; | ||
} | ||
|
||
ExtAuthStats ExtAuth::generateStats(const std::string& prefix, Stats::Scope& scope) { | ||
std::string final_prefix = prefix + "extauth."; | ||
return {ALL_EXTAUTH_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))}; | ||
} | ||
|
||
void ExtAuth::onSuccess(Http::MessagePtr&& response) { | ||
|
||
// onSuccess is called when our asynch auth request succeeds, meaning | ||
// "the HTTP protocol was successfully followed to completion" -- it | ||
// could still be the case that the auth server gave us a failure | ||
// response. | ||
|
||
// We're done with our auth request, so make sure it gets shredded. | ||
auth_request_ = nullptr; | ||
|
||
dumpHeaders("onSuccess", request_headers_); | ||
|
||
// What did we get back from the auth server? | ||
|
||
uint64_t response_code = Http::Utility::getResponseStatus(response->headers()); | ||
std::string response_body = response->bodyAsString(); | ||
|
||
ENVOY_STREAM_LOG(trace, "ExtAuth Auth responded with code {}", *callbacks_, response_code); | ||
|
||
if (!response->body()->length()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I forgot to roll back this. I will do it on the next pass. |
||
ENVOY_STREAM_LOG(trace, "ExtAuth Auth said: {}", *callbacks_, response->bodyAsString()); | ||
} | ||
|
||
// By definition, any response code other than 200, "OK", means we deny | ||
// this request. | ||
|
||
if (response_code != enumToInt(Http::Code::OK)) { | ||
ENVOY_STREAM_LOG(debug, "ExtAuth rejecting request", *callbacks_); | ||
|
||
// Bump the rejection count... | ||
|
||
config_->stats_.rq_rejected_.inc(); | ||
|
||
// ...and ditch our pointer to the request headers. | ||
|
||
request_headers_ = nullptr; | ||
|
||
// Whatever the auth server replied, we're going to hand that back to the | ||
// original requestor. That means both the header and the body; start by | ||
// copying the headers... | ||
|
||
Http::HeaderMapPtr response_headers{new HeaderMapImpl(response->headers())}; | ||
|
||
callbacks_->encodeHeaders(std::move(response_headers), response_body.empty()); | ||
|
||
// ...and then copy the body, as well, if there is one. | ||
|
||
if (!response_body.empty()) { | ||
Buffer::OwnedImpl buffer(response_body); | ||
callbacks_->encodeData(buffer, true); | ||
} | ||
|
||
// ...ahhhhnd we're done. | ||
|
||
return; | ||
} | ||
|
||
ENVOY_STREAM_LOG(debug, "ExtAuth accepting request", *callbacks_); | ||
|
||
// OK, we're going to approve this request, great! Next up: the filter can | ||
// be configured to copy headers from the auth server to the requester. | ||
// If that's configured, we need to take care of that now -- and if we actually | ||
// copy any headers, we'll need to be sure to invalidate the route cache. | ||
// (If we don't copy any headers, we should leave the route cache alone.) | ||
|
||
bool addedHeaders = false; | ||
|
||
// Do we have any headers configured to copy? | ||
|
||
if (!config_->allowed_headers_.empty()) { | ||
// Yup. Let's see if any of them are present. | ||
|
||
for (const std::string& allowed_header : config_->allowed_headers_) { | ||
LowerCaseString key(allowed_header); | ||
|
||
// OK, do we have this header? | ||
|
||
const HeaderEntry* hdr = response->headers().get(key); | ||
|
||
if (hdr) { | ||
// Well, the header exists at all. Does it have a value? | ||
|
||
const HeaderString& value = hdr->value(); | ||
|
||
if (!value.empty()) { | ||
// Not empty! Copy it into our request_headers_. | ||
|
||
std::string valstr{value.c_str()}; | ||
|
||
ENVOY_STREAM_LOG(trace, "ExtAuth allowing response header {}: {}", *callbacks_, | ||
allowed_header, valstr); | ||
addedHeaders = true; | ||
request_headers_->addCopy(key, valstr); | ||
} | ||
} | ||
} | ||
} | ||
|
||
if (addedHeaders) { | ||
// Yup, we added headers. Invalidate the route cache in case any of | ||
// the headers will affect routing decisions. | ||
|
||
dumpHeaders("ExtAuth invalidating route cache", request_headers_); | ||
|
||
callbacks_->clearRouteCache(); | ||
} | ||
|
||
// Finally done. Bump the "passed" stat... | ||
config_->stats_.rq_passed_.inc(); | ||
|
||
// ...remember that auth is done... | ||
auth_complete_ = true; | ||
|
||
// ...clear our request-header pointer now that we're finished with | ||
// this request... | ||
request_headers_ = nullptr; | ||
|
||
// ...and allow everything to continue. | ||
callbacks_->continueDecoding(); | ||
} | ||
|
||
void ExtAuth::onFailure(Http::AsyncClient::FailureReason) { | ||
auth_request_ = nullptr; | ||
request_headers_ = nullptr; | ||
ENVOY_STREAM_LOG(warn, "ExtAuth Auth request failed", *callbacks_); | ||
config_->stats_.rq_failed_.inc(); | ||
Http::Utility::sendLocalReply(*callbacks_, false, Http::Code::ServiceUnavailable, | ||
std::string("Auth request failed.")); | ||
} | ||
|
||
void ExtAuth::onDestroy() { | ||
if (auth_request_) { | ||
auth_request_->cancel(); | ||
auth_request_ = nullptr; | ||
} | ||
} | ||
|
||
void ExtAuth::setDecoderFilterCallbacks(StreamDecoderFilterCallbacks& callbacks) { | ||
callbacks_ = &callbacks; | ||
} | ||
|
||
} // namespace Http | ||
} // namespace Envoy |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you generally want to avoid reference members, it's easier to have dangling references when you can't explicitly null/null check them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see reference members are pretty common in envoy. I stand by my comment but convention is convention...