Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow use of URL-namespaced claims #94

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Allow use of URL-namespaced claims #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
840cc21
Fixes for check-style of existing code
willthames Mar 28, 2023
80c042d
Allow use of URL-namespaced claims
willthames Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions src/struct_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@

#include "jwt_verify_lib/struct_utils.h"

#include <string>

#include "absl/strings/str_cat.h"
#include "absl/strings/str_join.h"
#include "absl/strings/str_split.h"

namespace google {
Expand Down Expand Up @@ -105,8 +109,29 @@ StructUtils::FindResult StructUtils::GetStringList(

StructUtils::FindResult StructUtils::GetValue(
const std::string& nested_names, const google::protobuf::Value*& found) {
const std::vector<absl::string_view> name_vector =
absl::StrSplit(nested_names, '.');
std::vector<std::string> name_vector;

// Handle namespaced custom claims that by convention start with a URL - as
// most URLs contain dots, we extract the URL up to the FQDN and then split
// only the URL path by dots.
//
// e.g. for nested_names "https://example.com/claims.nested.key", name_vector
// would be {"https://example.com/claims", "nested", "key"}
if (absl::StartsWith(nested_names, "http://") ||
absl::StartsWith(nested_names, "https://")) {
const std::vector<std::string> url_components =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wary of doing this URL parsing manually with string manipulation instead of using an actual URL parser.

There's many things that can go wrong. At the very least, we should at least unescape the URL as %2F is the same as /.

abseil/abseil-cpp#158 (comment)

Copy link

@CelsoSantos CelsoSantos Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While that's not wrong, I am thinking: Is there ever a situation where the contents of the token could potentially be URLencoded after having been b64-decoded from the Authorization header? Other than some user error maybe. And in that case, don't we want to fail because the token is not properly formatted?

Or is the assumption that the token is always obtained from said header wrong, for this use case, and we could actually run into this issue AND still want to process it as valid?

EDIT:
Actually, now that I'm revisiting this, I DO have one concern: What if the claim is a "url-like" string, but not starting with http:// or https://, something like example.org/some_claim instead of http://example.org/some_claim? What then? Gracefully fail or let it try to unnest?

absl::StrSplit(nested_names, absl::MaxSplits('/', 3));
const std::string left = absl::StrJoin(
{url_components[0], url_components[1], url_components[2]}, "/");
const std::vector<std::string> path_components =
absl::StrSplit(url_components[3], '.');
name_vector = {absl::StrCat(left, "/", path_components[0])};
name_vector.insert(std::end(name_vector),
std::next(std::begin(path_components)),
std::end(path_components));
} else {
name_vector = absl::StrSplit(nested_names, '.');
}

const google::protobuf::Struct* current_struct = &struct_pb_;
for (int i = 0; i < name_vector.size(); ++i) {
Expand Down
Loading