Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 25 deletions enterprise/authentication/authentication.cc
Original file line number Diff line number Diff line change
Expand Up @@ -359,27 +359,6 @@ auto collect_jwt_identifiers(const std::span<const std::byte> metadata,
keys.emplace(reinterpret_cast<const char *>(metadata.data() + cursor), count);
}

auto derive_discovery_url(const std::string_view issuer) -> std::string {
std::string result{issuer};
if (!result.empty() && result.back() == '/') {
result.pop_back();
}

result += "/.well-known/openid-configuration";
return result;
}

auto parse_jwks_uri(const std::string_view body) -> std::optional<std::string> {
const auto document{sourcemeta::core::try_parse_json(body)};
if (!document.has_value() || !document.value().is_object() ||
!document.value().defines("jwks_uri") ||
!document.value().at("jwks_uri").is_string()) {
return std::nullopt;
}

return document.value().at("jwks_uri").to_string();
}

} // namespace

namespace sourcemeta::one {
Expand Down Expand Up @@ -575,17 +554,17 @@ struct Authentication::Impl {

std::string location;
if (jwks_uri.empty()) {
const auto metadata{this->fetcher_(derive_discovery_url(issuer))};
const auto metadata{this->fetcher_(discovery_url(issuer))};
if (!metadata.has_value()) {
return nullptr;
}

auto resolved{parse_jwks_uri(metadata.value().body)};
if (!resolved.has_value()) {
auto document{discovery_parse(metadata.value().body)};
if (!document.has_value() || !document.value().jwks_uri.has_value()) {
return nullptr;
}

location = std::move(resolved).value();
location = std::move(document.value().jwks_uri).value();
} else {
location = jwks_uri;
}
Expand Down
52 changes: 52 additions & 0 deletions enterprise/authentication/discovery.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#include <sourcemeta/one/authentication_discovery.h>

#include <sourcemeta/core/json.h>

#include <optional> // std::optional, std::nullopt
#include <string> // std::string
#include <string_view> // std::string_view

namespace {

auto read_endpoint(const sourcemeta::core::JSON &document,
const std::string_view property)
-> std::optional<std::string> {
const auto *endpoint{document.try_at(property)};
if (endpoint == nullptr || !endpoint->is_string()) {
return std::nullopt;
}

return endpoint->to_string();
}

} // namespace

namespace sourcemeta::one {

auto discovery_url(const std::string_view issuer) -> std::string {
std::string result{issuer};
if (!result.empty() && result.back() == '/') {
result.pop_back();
}

result += "/.well-known/openid-configuration";
return result;
}

auto discovery_parse(const std::string_view body)
-> std::optional<DiscoveryDocument> {
const auto document{sourcemeta::core::try_parse_json(body)};
if (!document.has_value() || !document.value().is_object()) {
return std::nullopt;
}

return DiscoveryDocument{
.jwks_uri = read_endpoint(document.value(), "jwks_uri"),
.authorization_endpoint =
read_endpoint(document.value(), "authorization_endpoint"),
.token_endpoint = read_endpoint(document.value(), "token_endpoint"),
.userinfo_endpoint =
read_endpoint(document.value(), "userinfo_endpoint")};
}

} // namespace sourcemeta::one
2 changes: 1 addition & 1 deletion enterprise/unit/authentication/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
sourcemeta_test(NAMESPACE sourcemeta PROJECT one NAME enterprise_authentication
SOURCES authentication_test.cc session_test.cc)
SOURCES authentication_test.cc session_test.cc discovery_test.cc)

target_link_libraries(sourcemeta_one_enterprise_authentication_unit
PRIVATE sourcemeta::one::authentication)
Expand Down
88 changes: 88 additions & 0 deletions enterprise/unit/authentication/discovery_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
#include <sourcemeta/one/authentication.h>

#include <sourcemeta/core/test.h>

TEST(discovery_url_appends_the_well_known_path) {
EXPECT_EQ(sourcemeta::one::discovery_url("https://acme.test"),
"https://acme.test/.well-known/openid-configuration");
}

TEST(discovery_url_strips_a_trailing_slash) {
EXPECT_EQ(sourcemeta::one::discovery_url("https://acme.test/"),
"https://acme.test/.well-known/openid-configuration");
}

TEST(discovery_url_of_an_empty_issuer) {
EXPECT_EQ(sourcemeta::one::discovery_url(""),
"/.well-known/openid-configuration");
}

TEST(discovery_parse_reads_every_endpoint) {
const auto document{sourcemeta::one::discovery_parse(R"JSON({
"jwks_uri": "https://acme.test/keys",
"authorization_endpoint": "https://acme.test/authorize",
"token_endpoint": "https://acme.test/token",
"userinfo_endpoint": "https://acme.test/userinfo"
})JSON")};
EXPECT_TRUE(document.has_value());
EXPECT_TRUE(document.value().jwks_uri.has_value());
EXPECT_EQ(document.value().jwks_uri.value(), "https://acme.test/keys");
EXPECT_TRUE(document.value().authorization_endpoint.has_value());
EXPECT_EQ(document.value().authorization_endpoint.value(),
"https://acme.test/authorize");
EXPECT_TRUE(document.value().token_endpoint.has_value());
EXPECT_EQ(document.value().token_endpoint.value(), "https://acme.test/token");
EXPECT_TRUE(document.value().userinfo_endpoint.has_value());
EXPECT_EQ(document.value().userinfo_endpoint.value(),
"https://acme.test/userinfo");
}

TEST(discovery_parse_reads_a_document_with_only_a_key_set_location) {
const auto document{sourcemeta::one::discovery_parse(R"JSON({
"jwks_uri": "https://acme.test/keys"
})JSON")};
EXPECT_TRUE(document.has_value());
EXPECT_TRUE(document.value().jwks_uri.has_value());
EXPECT_EQ(document.value().jwks_uri.value(), "https://acme.test/keys");
EXPECT_FALSE(document.value().authorization_endpoint.has_value());
EXPECT_FALSE(document.value().token_endpoint.has_value());
EXPECT_FALSE(document.value().userinfo_endpoint.has_value());
}

TEST(discovery_parse_reads_an_empty_document) {
const auto document{sourcemeta::one::discovery_parse("{}")};
EXPECT_TRUE(document.has_value());
EXPECT_FALSE(document.value().jwks_uri.has_value());
EXPECT_FALSE(document.value().authorization_endpoint.has_value());
EXPECT_FALSE(document.value().token_endpoint.has_value());
EXPECT_FALSE(document.value().userinfo_endpoint.has_value());
}

TEST(discovery_parse_ignores_a_non_string_endpoint) {
const auto document{sourcemeta::one::discovery_parse(R"JSON({
"jwks_uri": 1,
"authorization_endpoint": [ "https://acme.test/authorize" ],
"token_endpoint": null,
"userinfo_endpoint": "https://acme.test/userinfo"
})JSON")};
EXPECT_TRUE(document.has_value());
EXPECT_FALSE(document.value().jwks_uri.has_value());
EXPECT_FALSE(document.value().authorization_endpoint.has_value());
EXPECT_FALSE(document.value().token_endpoint.has_value());
EXPECT_TRUE(document.value().userinfo_endpoint.has_value());
EXPECT_EQ(document.value().userinfo_endpoint.value(),
"https://acme.test/userinfo");
}

TEST(discovery_parse_denies_a_document_that_is_not_an_object) {
EXPECT_FALSE(sourcemeta::one::discovery_parse("[]").has_value());
EXPECT_FALSE(
sourcemeta::one::discovery_parse("\"https://acme.test\"").has_value());
EXPECT_FALSE(sourcemeta::one::discovery_parse("null").has_value());
}

TEST(discovery_parse_denies_a_document_that_is_not_json) {
EXPECT_FALSE(sourcemeta::one::discovery_parse("").has_value());
EXPECT_FALSE(sourcemeta::one::discovery_parse("<html></html>").has_value());
EXPECT_FALSE(sourcemeta::one::discovery_parse("{").has_value());
}
2 changes: 1 addition & 1 deletion enterprise/unit/authentication/session_test.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include <sourcemeta/one/authentication_session.h>
#include <sourcemeta/one/authentication.h>

#include <sourcemeta/core/test.h>

Expand Down
9 changes: 5 additions & 4 deletions src/authentication/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
if(ONE_ENTERPRISE)
sourcemeta_library(NAMESPACE sourcemeta PROJECT one NAME authentication
PRIVATE_HEADERS error.h session.h
PRIVATE_HEADERS error.h session.h discovery.h
SOURCES
"${PROJECT_SOURCE_DIR}/enterprise/authentication/authentication.cc"
"${PROJECT_SOURCE_DIR}/enterprise/authentication/authentication_save.cc"
"${PROJECT_SOURCE_DIR}/enterprise/authentication/session.cc")
"${PROJECT_SOURCE_DIR}/enterprise/authentication/session.cc"
"${PROJECT_SOURCE_DIR}/enterprise/authentication/discovery.cc")
target_compile_definitions(sourcemeta_one_authentication PUBLIC SOURCEMETA_ONE_ENTERPRISE)
target_link_libraries(sourcemeta_one_authentication PRIVATE sourcemeta::core::crypto)
else()
sourcemeta_library(NAMESPACE sourcemeta PROJECT one NAME authentication
PRIVATE_HEADERS error.h session.h
SOURCES authentication.cc session.cc)
PRIVATE_HEADERS error.h session.h discovery.h
SOURCES authentication.cc session.cc discovery.cc)
endif()

target_link_libraries(sourcemeta_one_authentication PUBLIC sourcemeta::core::io)
Expand Down
19 changes: 19 additions & 0 deletions src/authentication/discovery.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#include <sourcemeta/one/authentication_discovery.h>

#include <optional> // std::optional, std::nullopt
#include <string> // std::string
#include <string_view> // std::string_view

namespace sourcemeta::one {

// Provider metadata only arises from token-based and interactive
// authentication, which are enterprise features, so this edition never
// locates a metadata document and never reads one
auto discovery_url(const std::string_view) -> std::string { return {}; }

auto discovery_parse(const std::string_view)
-> std::optional<DiscoveryDocument> {
return std::nullopt;
}

} // namespace sourcemeta::one
2 changes: 2 additions & 0 deletions src/authentication/include/sourcemeta/one/authentication.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
#include <sourcemeta/one/authentication_export.h>
#endif

#include <sourcemeta/one/authentication_discovery.h>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The two new includes (authentication_discovery.h and authentication_session.h) are unused by the Authentication class interface. They add unnecessary compilation dependencies to this public header — any change to either file triggers a rebuild of every translation unit that includes authentication.h. Add them only when the class declaration actually references types or functions from these headers.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/authentication/include/sourcemeta/one/authentication.h, line 8:

<comment>The two new includes (`authentication_discovery.h` and `authentication_session.h`) are unused by the `Authentication` class interface. They add unnecessary compilation dependencies to this public header — any change to either file triggers a rebuild of every translation unit that includes `authentication.h`. Add them only when the class declaration actually references types or functions from these headers.</comment>

<file context>
@@ -5,7 +5,9 @@
 #include <sourcemeta/one/authentication_export.h>
 #endif
 
+#include <sourcemeta/one/authentication_discovery.h>
 #include <sourcemeta/one/authentication_error.h>
+#include <sourcemeta/one/authentication_session.h>
</file context>

#include <sourcemeta/one/authentication_error.h>
#include <sourcemeta/one/authentication_session.h>
#include <sourcemeta/one/configuration.h>

#include <sourcemeta/core/jose.h>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#ifndef SOURCEMETA_ONE_AUTHENTICATION_DISCOVERY_H
#define SOURCEMETA_ONE_AUTHENTICATION_DISCOVERY_H

#ifndef SOURCEMETA_ONE_AUTHENTICATION_EXPORT
#include <sourcemeta/one/authentication_export.h>
#endif

#include <optional> // std::optional
#include <string> // std::string
#include <string_view> // std::string_view

namespace sourcemeta::one {

// The provider endpoints that a metadata document advertises, each present
// only when the document declares it as a string
struct DiscoveryDocument {
std::optional<std::string> jwks_uri;
std::optional<std::string> authorization_endpoint;
std::optional<std::string> token_endpoint;
std::optional<std::string> userinfo_endpoint;
};

// The well-known location of an issuer's metadata document
auto SOURCEMETA_ONE_AUTHENTICATION_EXPORT discovery_url(std::string_view issuer)
-> std::string;

// Read the provider endpoints out of a metadata document, returning nothing
// for a body that is not a JSON object
auto SOURCEMETA_ONE_AUTHENTICATION_EXPORT discovery_parse(std::string_view body)
-> std::optional<DiscoveryDocument>;

} // namespace sourcemeta::one

#endif
Loading