Make misuse harder across the OAuth, OIDC, JOSE and HTTP APIs

src/core/http/include/sourcemeta/core/http_message.h

@@ -151,21 +151,26 @@ inline auto http_parse_headers(const std::string_view input, Callback callback)
 /// with the name and value. A request cookie header carries only names and
 /// values, never attributes. Surrounding whitespace is trimmed, values are
 /// otherwise reported verbatim, and pairs that lack a `=` or have an empty name
-/// are skipped. For example:
+/// are skipped. Neither argument allocates, as both are borrowed from the
+/// input, so anything the callback keeps must not outlive it. For example:
 ///
 /// ```cpp
 /// #include <sourcemeta/core/http.h>
 /// #include <cassert>
 /// #include <string_view>
 ///
-/// std::string_view last_value;
+/// std::size_t count{0};
 /// sourcemeta::core::http_parse_cookies(
 ///     "session=abc; theme=dark",
-///     [&last_value](const std::string_view, const std::string_view value) {
-///       last_value = value;
+///     [&count](const std::string_view, const std::string_view) {
+///       count += 1;
 ///     });
-/// assert(last_value == "dark");
+/// assert(count == 2);
 /// ```
+///
+/// A header may carry several cookies under one name, so a callback that
+/// assigns to a single variable keeps whichever happens to come last. Use
+/// `http_cookie_values` to look one up by name.
 template <typename Callback>
   requires std::invocable<Callback, std::string_view, std::string_view>
 inline auto http_parse_cookies(const std::string_view input, Callback callback)
@@ -199,9 +204,8 @@ inline auto http_parse_cookies(const std::string_view input, Callback callback)
 /// @ingroup http
 /// Parse the value of an RFC 6265 §4.2 `Cookie` request header, given without
 /// the field name, into any container of name and value pairs. The names and
-/// values are forwarded as views that borrow from `input`, so a container of
-/// `std::string_view` pairs collects them without copying, while a container of
-/// `std::string` pairs owns them. For example:
+/// values are borrowed from `input` rather than copied, so a container of views
+/// must not outlive it, while an owning container may. For example:
 ///
 /// ```cpp
 /// #include <sourcemeta/core/http.h>
@@ -229,6 +233,49 @@ inline auto http_parse_cookies(const std::string_view input, Container &cookies)
                      });
 }
 
+/// @ingroup http
+/// Collect every value carried under the given cookie name, in the order the
+/// header presents them. A request may carry several cookies with one name,
+/// since a parent domain and the host itself can each set one and RFC 6265
+/// §4.2.2 notes the server "cannot determine from the Cookie header alone [...]
+/// for which hosts the cookie is valid". That section also warns that servers
+/// "SHOULD NOT rely upon the order in which these cookies appear", so a caller
+/// verifying a signed cookie tries every value rather than any single one.
+/// Naming a cookie with the RFC 6265bis §4.1.3.2 `__Host-` prefix prevents the
+/// collision at the source.
+///
+/// The values are borrowed from `input` rather than copied, so a container of
+/// views must not outlive it, while an owning container may. For example:
+///
+/// ```cpp
+/// #include <sourcemeta/core/http.h>
+/// #include <cassert>
+/// #include <string_view>
+/// #include <vector>
+///
+/// std::vector<std::string_view> values;
+/// sourcemeta::core::http_cookie_values("session=abc; theme=dark; session=xyz",
+///                                      "session", values);
+/// assert(values.size() == 2);
+/// assert(values.at(0) == "abc");
+/// assert(values.at(1) == "xyz");
+/// ```
+template <typename Container>
+  requires requires(Container container, std::string_view entry) {
+    container.emplace_back(entry);
+  }
+inline auto http_cookie_values(const std::string_view input,
+                               const std::string_view name, Container &values)
+    -> void {
+  http_parse_cookies(input,
+                     [&name, &values](const std::string_view cookie,
+                                      const std::string_view value) -> void {
+                       if (cookie == name) {
+                         values.emplace_back(value);
+                       }
+                     });
+}
+
 /// @ingroup http
 /// Parse the field lines of a raw message header block, skipping the start
 /// line, into any container of name and value pairs, normalising names to

src/core/http/include/sourcemeta/core/http_system.h

@@ -232,7 +232,10 @@ class SOURCEMETA_CORE_HTTP_EXPORT HTTPSystemRequest {
       -> HTTPSystemRequest &;
 
   /// Perform the request. A failure to obtain a response is reported as an
-  /// error, while unsuccessful status codes are returned on the result
+  /// error, while unsuccessful status codes are returned on the result. This
+  /// blocks the calling thread until the response arrives or a timeout elapses,
+  /// so it must not run on an event loop or any other thread serving unrelated
+  /// work.
   [[nodiscard]] auto send() const -> HTTPResponse;
 
 private:

src/core/jose/include/sourcemeta/core/jose_jwks_provider.h

@@ -92,15 +92,33 @@ class SOURCEMETA_CORE_JOSE_EXPORT JWKSProvider {
   /// keys as needed. Returns no value when the token is fully valid, otherwise
   /// the first failing step. The current time and the clock skew tolerance are
   /// the provider's own concern, so a caller supplies only what identifies the
-  /// token: an optional expected subject and an optional expected type for the
-  /// access token profile.
+  /// token: the expected subject, or no value to accept any, and the expected
+  /// `typ` header, or no value to accept any.
+  ///
+  /// Neither is defaulted. RFC 9068 Section 4 requires a resource server to
+  /// "verify that the `typ` header value is `at+jwt` or `application/at+jwt`
+  /// and reject tokens carrying any other value", and Section 5 explains that
+  /// this is what keeps an OpenID Connect ID Token from being accepted as an
+  /// access token. A default of no value would make skipping that check the
+  /// thing a caller gets for writing less, so the decision is spelled at the
+  /// call site instead. Prefer `verify_access_token` when the token is an
+  /// access token.
   [[nodiscard]] auto
   verify(const JWT &token,
          const std::span<const JWSAlgorithm> allowed_algorithms,
          const std::string_view expected_issuer,
          const std::string_view expected_audience,
-         const std::optional<std::string_view> expected_subject = std::nullopt,
-         const std::optional<std::string_view> expected_type = std::nullopt)
+         const std::optional<std::string_view> expected_subject,
+         const std::optional<std::string_view> expected_type)
+      -> std::optional<JWTVerificationError>;
+
+  /// Verify a token as an RFC 9068 JWT access token, pinning the `typ` header
+  /// to `at+jwt` so the profile's Section 4 requirement cannot be omitted.
+  [[nodiscard]] auto verify_access_token(
+      const JWT &token, const std::span<const JWSAlgorithm> allowed_algorithms,
+      const std::string_view expected_issuer,
+      const std::string_view expected_audience,
+      const std::optional<std::string_view> expected_subject = std::nullopt)
       -> std::optional<JWTVerificationError>;
 
   /// Verify a token exactly as the other overload does, additionally exposing

src/core/jose/include/sourcemeta/core/jose_verify.h

@@ -244,15 +244,14 @@ enum class JWTVerificationError : std::uint8_t {
 /// assert(error.has_value());
 /// ```
 SOURCEMETA_CORE_JOSE_EXPORT
-auto jwt_verify(
-    const JWT &token, const JWKS &keys,
-    const std::span<const JWSAlgorithm> allowed_algorithms,
-    const std::string_view expected_issuer,
-    const std::string_view expected_audience,
-    const std::chrono::system_clock::time_point now,
-    const JWTClockSkew clock_skew = {},
-    const std::optional<std::string_view> expected_subject = std::nullopt,
-    const std::optional<std::string_view> expected_type = std::nullopt)
+auto jwt_verify(const JWT &token, const JWKS &keys,
+                const std::span<const JWSAlgorithm> allowed_algorithms,
+                const std::string_view expected_issuer,
+                const std::string_view expected_audience,
+                const std::chrono::system_clock::time_point now,
+                const JWTClockSkew clock_skew,
+                const std::optional<std::string_view> expected_subject,
+                const std::optional<std::string_view> expected_type)
     -> std::optional<JWTVerificationError>;
 
 } // namespace sourcemeta::core

src/core/jose/jose_jwks_provider.cc

@@ -134,6 +134,20 @@ auto JWKSProvider::verify(
                          this->clock_());
 }
 
+auto JWKSProvider::verify_access_token(
+    const JWT &token, const std::span<const JWSAlgorithm> allowed_algorithms,
+    const std::string_view expected_issuer,
+    const std::string_view expected_audience,
+    const std::optional<std::string_view> expected_subject)
+    -> std::optional<JWTVerificationError> {
+  // RFC 9068 Section 2.1: "it is RECOMMENDED that the "application/" prefix be
+  // omitted. Therefore, the "typ" value used SHOULD be "at+jwt"". The
+  // comparison accepts either spelling, so pinning the short one satisfies
+  // Section 4
+  return this->verify(token, allowed_algorithms, expected_issuer,
+                      expected_audience, expected_subject, "at+jwt");
+}
+
 auto JWKSProvider::verify(
     const JWT &token, const std::span<const JWSAlgorithm> allowed_algorithms,
     const std::string_view expected_issuer,

src/core/jose/jose_jwt_verify.cc

@@ -44,8 +44,10 @@ auto jwt_verify(const JWT &token, const JWKS &keys,
                 const std::optional<std::string_view> expected_subject,
                 const std::optional<std::string_view> expected_type)
     -> std::optional<JWTVerificationError> {
-  // The algorithm allow-list is enforced before any key is touched, per step 3
-  // of the Sourcemeta One validation algorithm
+  // RFC 8725 Section 3.1: "Libraries MUST enable the caller to specify a
+  // supported set of algorithms and MUST NOT use any other algorithms when
+  // performing cryptographic operations", so the allow-list is enforced before
+  // any key is touched
   const auto algorithm{token.algorithm()};
   if (!algorithm.has_value() ||
       std::ranges::find(allowed_algorithms, algorithm.value()) ==

src/core/oauth/include/sourcemeta/core/oauth_metadata.h

@@ -60,6 +60,25 @@ auto oauth_well_known_url(const std::string_view identifier,
                           const OAuthWellKnownKind kind, std::string &sink)
     -> bool;
 
+/// @ingroup oauth
+/// Whether a URL is usable as an endpoint a client sends requests to: the https
+/// scheme, compared case-insensitively per RFC 3986 Section 3.1, a non-empty
+/// host, and no fragment, which RFC 6749 Section 3.1 forbids on an endpoint. A
+/// query is permitted. This is the rule the metadata parsers apply, named for
+/// the endpoint case rather than as a general https test, since the fragment
+/// prohibition comes from the endpoint specifications and does not hold of
+/// https URLs at large. For example:
+///
+/// ```cpp
+/// #include <sourcemeta/core/oauth.h>
+/// #include <cassert>
+///
+/// assert(sourcemeta::core::oauth_is_endpoint_url("https://example.com/token"));
+/// assert(!sourcemeta::core::oauth_is_endpoint_url("http://example.com/token"));
+/// ```
+SOURCEMETA_CORE_OAUTH_EXPORT
+auto oauth_is_endpoint_url(const std::string_view value) -> bool;
+
 #if defined(_MSC_VER)
 #pragma warning(disable : 4251)
 #endif

src/core/oauth/oauth_metadata.cc

@@ -237,6 +237,23 @@ auto validated_resource_metadata(JSON &&data, const std::string_view resource)
 
 } // namespace
 
+// RFC 6749 Section 3.1: "the authorization server MUST require the use of TLS
+// as described in Section 1.6 when sending requests to the authorization
+// endpoint", which over HTTP means the https scheme, and "The endpoint URI MUST
+// NOT include a fragment component", while it "MAY include an
+// "application/x-www-form-urlencoded" formatted (per Appendix B) query
+// component". Unlike an identifier, which RFC 8414 Section 4 compares as "a
+// Unicode code-point-to-code-point equality comparison", an endpoint is a
+// location to dereference, so RFC 3986 Section 3.1 governs and makes its scheme
+// case-insensitive
+auto oauth_is_endpoint_url(const std::string_view value) -> bool {
+  const auto uri{oauth_try_parse_uri(value)};
+  return uri.has_value() && uri->scheme().has_value() &&
+         equals_ignore_case(uri->scheme().value(), "https") &&
+         uri->host().has_value() && !uri->host().value().empty() &&
+         !uri->fragment().has_value();
+}
+
 auto oauth_well_known_url(const std::string_view identifier,
                           const OAuthWellKnownKind kind, std::string &sink)
     -> bool {

src/core/oauth/oauth_registration.cc

@@ -2,6 +2,7 @@
 
 #include <sourcemeta/core/json.h>
 #include <sourcemeta/core/oauth_error.h>
+#include <sourcemeta/core/oauth_metadata.h>
 
 #include "oauth_json.h"
 #include "oauth_syntax.h"

src/core/oauth/oauth_syntax.h

@@ -60,23 +60,6 @@ inline auto oauth_is_advertised_issuer(const std::string_view value) -> bool {
          !uri->query().has_value() && !uri->fragment().has_value();
 }
 
-// RFC 6749 Section 3.1: "the authorization server MUST require the use of TLS
-// as described in Section 1.6 when sending requests to the authorization
-// endpoint", which over HTTP means the https scheme, and "The endpoint URI MUST
-// NOT include a fragment component", while it "MAY include an
-// "application/x-www-form-urlencoded" formatted (per Appendix B) query
-// component". Unlike an identifier, which RFC 8414 Section 4 compares as "a
-// Unicode code-point-to-code-point equality comparison", an endpoint is a
-// location to dereference, so RFC 3986 Section 3.1 governs and makes its scheme
-// case-insensitive
-inline auto oauth_is_endpoint_url(const std::string_view value) -> bool {
-  const auto uri{oauth_try_parse_uri(value)};
-  return uri.has_value() && uri->scheme().has_value() &&
-         equals_ignore_case(uri->scheme().value(), "https") &&
-         uri->host().has_value() && !uri->host().value().empty() &&
-         !uri->fragment().has_value();
-}
-
 // RFC 3986 Section 2.3: "unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"",
 // the character set RFC 7636 reuses for the PKCE verifier and challenge
 inline auto oauth_is_unreserved(const char character) noexcept -> bool {

src/core/oidc/include/sourcemeta/core/oidc_metadata.h

@@ -52,11 +52,44 @@ class SOURCEMETA_CORE_OIDC_EXPORT OIDCProviderMetadata {
   /// throwing when it is invalid. The document is moved in.
   OIDCProviderMetadata(JSON &&data, const std::string_view issuer);
 
+  /// Apply the OpenID Connect layer to a document already parsed and validated
+  /// as OAuth authorization server metadata, throwing when the OpenID Connect
+  /// requirements are not met. The document is moved in and the OAuth checks
+  /// are not repeated.
+  explicit OIDCProviderMetadata(OAuthServerMetadata &&oauth);
+
   /// Construct and validate a metadata document for an expected issuer,
   /// returning no value when it is invalid. The document is moved in.
   [[nodiscard]] static auto from(JSON &&data, const std::string_view issuer)
       -> std::optional<OIDCProviderMetadata>;
 
+  /// Apply the OpenID Connect layer to a document already parsed and validated
+  /// as OAuth authorization server metadata, returning no value when the
+  /// OpenID Connect requirements are not met. This is what a caching resolver
+  /// hands back, so lifting it costs no reparse and no repeated OAuth
+  /// validation. For example:
+  ///
+  /// ```cpp
+  /// #include <sourcemeta/core/oidc.h>
+  /// #include <cassert>
+  ///
+  /// auto document{sourcemeta::core::parse_json(R"JSON({
+  ///   "issuer":"https://example.com",
+  ///   "jwks_uri":"https://example.com/jwks",
+  ///   "response_types_supported":[ "code" ],
+  ///   "subject_types_supported":[ "public" ],
+  ///   "id_token_signing_alg_values_supported":[ "RS256" ]
+  /// })JSON")};
+  /// auto oauth{sourcemeta::core::OAuthServerMetadata::from(
+  ///     std::move(document), "https://example.com")};
+  /// assert(oauth.has_value());
+  /// const auto metadata{
+  ///     sourcemeta::core::OIDCProviderMetadata::from(std::move(oauth).value())};
+  /// assert(metadata.has_value());
+  /// ```
+  [[nodiscard]] static auto from(OAuthServerMetadata &&oauth)
+      -> std::optional<OIDCProviderMetadata>;
+
   /// The issuer identifier (OpenID Connect Discovery 1.0 Section 3).
   [[nodiscard]] auto issuer() const -> std::string_view;
 

src/core/oidc/oidc_id_token.cc

@@ -236,7 +236,8 @@ auto oidc_validate_id_token(
   // audience, expiration, not-before, and issued-at (OpenID Connect Core 1.0
   // Section 3.1.3.7 steps 6 through 9)
   const auto error{jwt_verify(token, keys, allowed_algorithms, issuer,
-                              client_id, now, clock_skew)};
+                              client_id, now, clock_skew, std::nullopt,
+                              std::nullopt)};
   if (error.has_value()) {
     return std::nullopt;
   }

src/core/oidc/oidc_metadata.cc

@@ -164,6 +164,22 @@ OIDCProviderMetadata::OIDCProviderMetadata(JSON &&data,
   validate_provider_metadata(this->oauth_);
 }
 
+OIDCProviderMetadata::OIDCProviderMetadata(OAuthServerMetadata &&oauth)
+    : oauth_{std::move(oauth)} {
+  validate_provider_metadata(this->oauth_);
+}
+
+auto OIDCProviderMetadata::from(OAuthServerMetadata &&oauth)
+    -> std::optional<OIDCProviderMetadata> {
+  // The OAuth layer validated itself on the way in, so only the OpenID Connect
+  // requirements can fail here
+  try {
+    return OIDCProviderMetadata{std::move(oauth)};
+  } catch (const OIDCMetadataParseError &) {
+    return std::nullopt;
+  }
+}
+
 auto OIDCProviderMetadata::from(JSON &&data, const std::string_view issuer)
     -> std::optional<OIDCProviderMetadata> {
   try {

test/http/CMakeLists.txt

@@ -10,6 +10,7 @@ sourcemeta_test(NAMESPACE sourcemeta PROJECT core NAME http
           http_header_find_test.cc http_error_test.cc
           http_parse_bearer_test.cc http_cache_control_max_age_test.cc
           http_serialize_cookie_test.cc http_parse_cookies_test.cc
+          http_cookie_values_test.cc
           http_cookie_valid_test.cc http_aws_sigv4_test.cc
           http_syntax_test.cc
           http_system_request_test.cc)