# HG changeset patch # User Alain Mazy # Date 1752252107 -7200 # Node ID 2b09f8e98cfe91d6ccb1958f9877f3024b220a20 # Parent ff21632f3ab6b0924a9bc75b544d6f13030ad13c# Parent d18a5deb19cf2579403de94144b01086db74da96 merge default -> inbox diff -r ff21632f3ab6 -r 2b09f8e98cfe NEWS --- a/NEWS Fri Jul 11 15:38:38 2025 +0200 +++ b/NEWS Fri Jul 11 18:41:47 2025 +0200 @@ -16,6 +16,11 @@ * The User profile can now contain an "id" field if the auth-service provides it. * New experimental feature: audit-logs (TODO) +* Fixed a security issue: the entries in the cache token->permissions were kept too long in the cache + allowing users to have access to generic routes even with an expired token. + These entries are now stored maximum for 10 seconds. + Note that the validity duration of the token->user-profile entries is determined by the auth-service; + typically 60 seconds. 2025-06-11 - v 0.9.3 diff -r ff21632f3ab6 -r 2b09f8e98cfe Plugin/AuthorizationWebService.cpp --- a/Plugin/AuthorizationWebService.cpp Fri Jul 11 15:38:38 2025 +0200 +++ b/Plugin/AuthorizationWebService.cpp Fri Jul 11 18:41:47 2025 +0200 @@ -464,8 +464,7 @@ } } - bool AuthorizationWebService::HasUserPermissionInternal(unsigned int& validity, - const std::string& permission, + bool AuthorizationWebService::HasUserPermissionInternal(const std::string& permission, const UserProfile& profile) { const std::set& permissions = profile.permissions; diff -r ff21632f3ab6 -r 2b09f8e98cfe Plugin/AuthorizationWebService.h --- a/Plugin/AuthorizationWebService.h Fri Jul 11 15:38:38 2025 +0200 +++ b/Plugin/AuthorizationWebService.h Fri Jul 11 18:41:47 2025 +0200 @@ -49,8 +49,7 @@ const Token* token, const std::string& tokenValue) ORTHANC_OVERRIDE; - virtual bool HasUserPermissionInternal(unsigned int& validity, - const std::string& permission, + virtual bool HasUserPermissionInternal(const std::string& permission, const UserProfile& profile) ORTHANC_OVERRIDE; public: diff -r ff21632f3ab6 -r 2b09f8e98cfe Plugin/BaseAuthorizationService.h --- a/Plugin/BaseAuthorizationService.h Fri Jul 11 15:38:38 2025 +0200 +++ b/Plugin/BaseAuthorizationService.h Fri Jul 11 18:41:47 2025 +0200 @@ -44,8 +44,7 @@ const Token* token, const std::string& tokenValue) = 0; - virtual bool HasUserPermissionInternal(unsigned int& validity, - const std::string& permission, + virtual bool HasUserPermissionInternal(const std::string& permission, const UserProfile& profile) = 0; public: @@ -83,8 +82,7 @@ return GetUserProfileInternal(validity, profile, NULL, ""); } - virtual bool HasUserPermission(unsigned int& validity /* out */, - const std::set& anyOfPermissions, + virtual bool HasUserPermission(const std::set& anyOfPermissions, const UserProfile& profile) ORTHANC_OVERRIDE { if (anyOfPermissions.size() == 0) @@ -94,7 +92,7 @@ for (std::set::const_iterator it = anyOfPermissions.begin(); it != anyOfPermissions.end(); ++it) { - if (HasUserPermissionInternal(validity, *it, profile)) + if (HasUserPermissionInternal(*it, profile)) { return true; } diff -r ff21632f3ab6 -r 2b09f8e98cfe Plugin/CachedAuthorizationService.cpp --- a/Plugin/CachedAuthorizationService.cpp Fri Jul 11 15:38:38 2025 +0200 +++ b/Plugin/CachedAuthorizationService.cpp Fri Jul 11 18:41:47 2025 +0200 @@ -160,8 +160,7 @@ return false; } - bool CachedAuthorizationService::HasUserPermissionInternal(unsigned int& validity, - const std::string& permission, + bool CachedAuthorizationService::HasUserPermissionInternal(const std::string& permission, const UserProfile& profile) { assert(decorated_.get() != NULL); @@ -176,28 +175,10 @@ return (value == "1"); } - bool granted = decorated_->HasUserPermissionInternal(validity, permission, profile); + bool granted = decorated_->HasUserPermissionInternal(permission, profile); + + cache_->Store(key, (granted ? "1" : "0"), 10); // don't cache for more than 10 seconds - it's the result of a quite easy computation anyway - if (granted) - { - if (validity > 0) - { - cache_->Store(key, "1", validity); - } - - return true; - } - else - { - if (validity > 0) - { - cache_->Store(key, "0", validity); - } - - return false; - } + return granted; } - - - } diff -r ff21632f3ab6 -r 2b09f8e98cfe Plugin/CachedAuthorizationService.h --- a/Plugin/CachedAuthorizationService.h Fri Jul 11 15:38:38 2025 +0200 +++ b/Plugin/CachedAuthorizationService.h Fri Jul 11 18:41:47 2025 +0200 @@ -58,8 +58,7 @@ const Token* token, const std::string& tokenValue) ORTHANC_OVERRIDE; - virtual bool HasUserPermissionInternal(unsigned int& validity, - const std::string& permission, + virtual bool HasUserPermissionInternal(const std::string& permission, const UserProfile& profile) ORTHANC_OVERRIDE; diff -r ff21632f3ab6 -r 2b09f8e98cfe Plugin/IAuthorizationService.h --- a/Plugin/IAuthorizationService.h Fri Jul 11 15:38:38 2025 +0200 +++ b/Plugin/IAuthorizationService.h Fri Jul 11 18:41:47 2025 +0200 @@ -94,8 +94,7 @@ virtual bool GetAnonymousUserProfile(unsigned int& validity /* out */, UserProfile& profile /* out */) = 0; - virtual bool HasUserPermission(unsigned int& validity /* out */, - const std::set& anyOfPermissions, + virtual bool HasUserPermission(const std::set& anyOfPermissions, const UserProfile& profile) = 0; virtual bool CreateToken(CreatedToken& response, diff -r ff21632f3ab6 -r 2b09f8e98cfe Plugin/Plugin.cpp --- a/Plugin/Plugin.cpp Fri Jul 11 15:38:38 2025 +0200 +++ b/Plugin/Plugin.cpp Fri Jul 11 18:41:47 2025 +0200 @@ -353,8 +353,7 @@ const OrthancPlugins::AssociativeArray& getArguments ) { - unsigned int validity; // ignored - if (authorizationService_->HasUserPermission(validity, requiredPermissions, profile)) + if (authorizationService_->HasUserPermission(requiredPermissions, profile)) { LOG(INFO) << msg << " -> granted to user '" << profile.name << "'"; hasUserRequiredPermissions = true;