From 669879367a950540fa159673f46216aaf9fcac0e Mon Sep 17 00:00:00 2001 From: Kory Draughn Date: Fri, 1 Aug 2025 01:34:39 -0400 Subject: [PATCH 1/4] [311] Make client request extend lifetime of bearer token --- README.md | 5 +- .../irods/private/http_api/process_stash.hpp | 10 ++ core/src/common.cpp | 132 ++++++++++-------- core/src/process_stash.cpp | 15 ++ 4 files changed, 101 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index 297ca7b4..f80ef39f 100644 --- a/README.md +++ b/README.md @@ -247,8 +247,9 @@ Notice how some of the configuration values are wrapped in angle brackets (e.g. // Defines options for the "Basic" authentication scheme. "basic": { - // The amount of time before a user's authentication - // token expires. + // The amount of time before a user's authentication token expires. + // The lifetime of a token will be extended by this amount on each + // use as long as the token has not expired. "timeout_in_seconds": 3600 }, diff --git a/core/include/irods/private/http_api/process_stash.hpp b/core/include/irods/private/http_api/process_stash.hpp index 7ed4e2bf..b41a9060 100644 --- a/core/include/irods/private/http_api/process_stash.hpp +++ b/core/include/irods/private/http_api/process_stash.hpp @@ -45,6 +45,16 @@ namespace irods::http::process_stash /// \since 4.2.12 auto find(const std::string& _key) -> std::optional; + /// Atomically executes operation on value mapped to a specific key. + /// + /// This function is thread-safe. + /// + /// \param[in] _key The string which maps to the value of interest. + /// \param[in] _visitor The operation to execute on the value mapped to the key. + /// + /// \returns A boolean indicating whether the operation was executed. + auto visit(const std::string& _key, const std::function& _visitor) -> bool; + /// Removes a value from the process stash. /// /// This function is thread-safe. diff --git a/core/src/common.cpp b/core/src/common.cpp index 6a3aad3e..32d0d738 100644 --- a/core/src/common.cpp +++ b/core/src/common.cpp @@ -339,81 +339,95 @@ namespace irods::http boost::trim(bearer_token); logging::debug("{}: Bearer token: [{}]", __func__, bearer_token); + const auto& config = irods::http::globals::configuration(); + + bool token_expired = false; + authenticated_client_info client_info_copy; + // Verify the bearer token is known to the server. If not, return an error. - auto mapped_value{irods::http::process_stash::find(bearer_token)}; - if (!mapped_value.has_value()) { - const auto& config = irods::http::globals::configuration(); - - // It's possible that the admin didn't include the OIDC configuration stanza. - // This use-case is allowed, therefore we check for the OIDC configuration before - // attempting to access it. Without this logic, the server would crash. - static const auto oidc_conf_exists{ - config.contains(nlohmann::json::json_pointer{"/http_server/authentication/openid_connect"})}; - if (oidc_conf_exists) { - nlohmann::json json_res; - const auto& validation_method{irods::http::globals::oidc_configuration() - .at("access_token_validation_method") - .get_ref()}; - - // Try parsing token as JWT Access Token - if (validation_method == "local_validation") { - try { - auto token{jwt::decode(bearer_token)}; - auto possible_json_res{openid::validate_using_local_validation(token)}; - - if (possible_json_res) { - json_res = *possible_json_res; - } - } - // Parsing of the token failed, this is not a JWT access token - catch (const std::exception& e) { - logging::debug("{}: {}", __func__, e.what()); - } - } + const auto found_token = irods::http::process_stash::visit(bearer_token, [&](boost::any& _v) { + auto& client_info = boost::any_cast(_v); + const auto now = std::chrono::steady_clock::now(); + + if (now >= client_info.expires_at) { + token_expired = true; + return; + } + + // Refresh the lifetime of the bearer token. + static const auto token_lifetime = + config.at(nlohmann::json::json_pointer{"/http_server/authentication/basic/timeout_in_seconds"}) + .get(); + client_info.expires_at = now + std::chrono::seconds{token_lifetime}; + + client_info_copy = client_info; + }); + + if (found_token) { + if (token_expired) { + logging::error("{}: Session for bearer token [{}] has expired.", __func__, bearer_token); + return {.response = fail(status_type::unauthorized)}; + } + + logging::trace("{}: Client is authenticated.", __func__); + return {.client_info = std::move(client_info_copy)}; + } + + // It's possible that the admin didn't include the OIDC configuration stanza. + // This use-case is allowed, therefore we check for the OIDC configuration before + // attempting to access it. Without this logic, the server would crash. + static const auto oidc_conf_exists{ + config.contains(nlohmann::json::json_pointer{"/http_server/authentication/openid_connect"})}; + if (oidc_conf_exists) { + nlohmann::json json_res; + const auto& validation_method{irods::http::globals::oidc_configuration() + .at("access_token_validation_method") + .get_ref()}; + + // Try parsing token as JWT Access Token + if (validation_method == "local_validation") { + try { + auto token{jwt::decode(bearer_token)}; + auto possible_json_res{openid::validate_using_local_validation(token)}; - // Use introspection endpoint if it exists - static const auto introspection_endpoint_exists{ - irods::http::globals::oidc_endpoint_configuration().contains("introspection_endpoint")}; - if (introspection_endpoint_exists && validation_method == "introspection") { - auto possible_json_res{openid::validate_using_introspection_endpoint(bearer_token)}; if (possible_json_res) { json_res = *possible_json_res; } } - - if (json_res.empty()) { - logging::error("{}: Could not find bearer token matching [{}].", __func__, bearer_token); - return {.response = fail(status_type::unauthorized)}; + // Parsing of the token failed, this is not a JWT access token + catch (const std::exception& e) { + logging::debug("{}: {}", __func__, e.what()); } + } - // Do mapping of user to irods user - auto user{map_json_to_user(json_res)}; - if (user) { - return {.client_info = {.username = *std::move(user)}}; + // Use introspection endpoint if it exists + static const auto introspection_endpoint_exists{ + irods::http::globals::oidc_endpoint_configuration().contains("introspection_endpoint")}; + if (introspection_endpoint_exists && validation_method == "introspection") { + auto possible_json_res{openid::validate_using_introspection_endpoint(bearer_token)}; + if (possible_json_res) { + json_res = *possible_json_res; } - - logging::warn("{}: Could not find a matching user.", __func__); - return {.response = fail(status_type::unauthorized)}; } - logging::debug("{}: No 'openid_connect' stanza found in server configuration.", __func__); - logging::error("{}: Could not find bearer token matching [{}].", __func__, bearer_token); - return {.response = fail(status_type::unauthorized)}; - } + if (json_res.empty()) { + logging::error("{}: Could not find bearer token matching [{}].", __func__, bearer_token); + return {.response = fail(status_type::unauthorized)}; + } - auto* client_info{boost::any_cast(&*mapped_value)}; - if (client_info == nullptr) { - logging::error("{}: Could not find bearer token matching [{}].", __func__, bearer_token); - return {.response = fail(status_type::unauthorized)}; - } + // Do mapping of user to irods user + auto user{map_json_to_user(json_res)}; + if (user) { + return {.client_info = {.username = *std::move(user)}}; + } - if (std::chrono::steady_clock::now() >= client_info->expires_at) { - logging::error("{}: Session for bearer token [{}] has expired.", __func__, bearer_token); + logging::warn("{}: Could not find a matching user.", __func__); return {.response = fail(status_type::unauthorized)}; } - logging::trace("{}: Client is authenticated.", __func__); - return {.client_info = std::move(*client_info)}; + logging::debug("{}: No [openid_connect] stanza found in server configuration.", __func__); + logging::error("{}: Could not find bearer token matching [{}].", __func__, bearer_token); + return {.response = fail(status_type::unauthorized)}; } // resolve_client_identity auto execute_operation( diff --git a/core/src/process_stash.cpp b/core/src/process_stash.cpp index 166aa514..506e8a44 100644 --- a/core/src/process_stash.cpp +++ b/core/src/process_stash.cpp @@ -52,6 +52,21 @@ namespace irods::http::process_stash return std::nullopt; } // find + auto visit(const std::string& _key, const std::function& _visitor) -> bool + { + std::shared_lock read_lock{g_mtx}; + if (g_stash.find(_key) != std::end(g_stash)) { + read_lock.unlock(); + std::lock_guard write_lock{g_mtx}; + if (auto iter = g_stash.find(_key); iter != std::end(g_stash)) { + _visitor(iter->second); + return true; + } + } + + return false; + } // visit + auto erase(const std::string& _key) -> bool { std::lock_guard lock{g_mtx}; From c349ee74921b4eaf59016104b670d80b804e5d28 Mon Sep 17 00:00:00 2001 From: Kory Draughn Date: Fri, 1 Aug 2025 01:48:34 -0400 Subject: [PATCH 2/4] [43] Report unknown bearer token as INFO level message --- core/src/common.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/common.cpp b/core/src/common.cpp index 32d0d738..642f75ff 100644 --- a/core/src/common.cpp +++ b/core/src/common.cpp @@ -365,7 +365,7 @@ namespace irods::http if (found_token) { if (token_expired) { - logging::error("{}: Session for bearer token [{}] has expired.", __func__, bearer_token); + logging::info("{}: Session for bearer token [{}] has expired.", __func__, bearer_token); return {.response = fail(status_type::unauthorized)}; } @@ -410,10 +410,10 @@ namespace irods::http } } - if (json_res.empty()) { - logging::error("{}: Could not find bearer token matching [{}].", __func__, bearer_token); - return {.response = fail(status_type::unauthorized)}; - } + if (json_res.empty()) { + logging::info("{}: Could not find bearer token matching [{}].", __func__, bearer_token); + return {.response = fail(status_type::unauthorized)}; + } // Do mapping of user to irods user auto user{map_json_to_user(json_res)}; @@ -426,7 +426,7 @@ namespace irods::http } logging::debug("{}: No [openid_connect] stanza found in server configuration.", __func__); - logging::error("{}: Could not find bearer token matching [{}].", __func__, bearer_token); + logging::info("{}: Could not find bearer token matching [{}].", __func__, bearer_token); return {.response = fail(status_type::unauthorized)}; } // resolve_client_identity From c36fcae593f2d181a3a3cc1758a2e1adfd0f017f Mon Sep 17 00:00:00 2001 From: Kory Draughn Date: Sat, 16 Aug 2025 16:39:16 -0400 Subject: [PATCH 3/4] [43] Remove unused header include directive --- endpoints/authentication/src/main.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/endpoints/authentication/src/main.cpp b/endpoints/authentication/src/main.cpp index 0292442c..85019765 100644 --- a/endpoints/authentication/src/main.cpp +++ b/endpoints/authentication/src/main.cpp @@ -4,7 +4,6 @@ #include "irods/private/http_api/globals.hpp" #include "irods/private/http_api/log.hpp" #include "irods/private/http_api/process_stash.hpp" -#include "irods/private/http_api/openid.hpp" #include "irods/private/http_api/session.hpp" #include "irods/private/http_api/transport.hpp" #include "irods/private/http_api/version.hpp" From 4358600159f8840446f868f9350dd2f789de55e3 Mon Sep 17 00:00:00 2001 From: Kory Draughn Date: Sat, 16 Aug 2025 16:54:08 -0400 Subject: [PATCH 4/4] squash w/ 311 --- core/src/process_stash.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/core/src/process_stash.cpp b/core/src/process_stash.cpp index 506e8a44..2607aa0a 100644 --- a/core/src/process_stash.cpp +++ b/core/src/process_stash.cpp @@ -54,14 +54,10 @@ namespace irods::http::process_stash auto visit(const std::string& _key, const std::function& _visitor) -> bool { - std::shared_lock read_lock{g_mtx}; - if (g_stash.find(_key) != std::end(g_stash)) { - read_lock.unlock(); - std::lock_guard write_lock{g_mtx}; - if (auto iter = g_stash.find(_key); iter != std::end(g_stash)) { - _visitor(iter->second); - return true; - } + std::lock_guard write_lock{g_mtx}; + if (auto iter = g_stash.find(_key); iter != std::end(g_stash)) { + _visitor(iter->second); + return true; } return false;