Mercurial > hg > orthanc-authorization
changeset 248:d18a5deb19cf
fixed permissions cache validity that was too long
| author | Alain Mazy <am@orthanc.team> |
|---|---|
| date | Fri, 11 Jul 2025 18:20:32 +0200 |
| parents | 1877694ab4f7 |
| children | 2b09f8e98cfe d5b1714ad7f0 e957ca443917 |
| files | NEWS Plugin/AuthorizationWebService.cpp Plugin/AuthorizationWebService.h Plugin/BaseAuthorizationService.h Plugin/CachedAuthorizationService.cpp Plugin/CachedAuthorizationService.h Plugin/IAuthorizationService.h Plugin/Plugin.cpp |
| diffstat | 8 files changed, 31 insertions(+), 46 deletions(-) [+] |
line wrap: on
line diff
--- a/NEWS Wed Jun 11 10:55:44 2025 +0200 +++ b/NEWS Fri Jul 11 18:20:32 2025 +0200 @@ -1,3 +1,17 @@ +Pending changes in the mainline +=============================== + +=> Minimum Orthanc version: 1.11.3 <= +=> Recommended SDK version: 1.12.4 <= +=> Minimum SDK version: 1.11.3 <= + +* 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 ====================
--- a/Plugin/AuthorizationWebService.cpp Wed Jun 11 10:55:44 2025 +0200 +++ b/Plugin/AuthorizationWebService.cpp Fri Jul 11 18:20:32 2025 +0200 @@ -446,8 +446,7 @@ } } - bool AuthorizationWebService::HasUserPermissionInternal(unsigned int& validity, - const std::string& permission, + bool AuthorizationWebService::HasUserPermissionInternal(const std::string& permission, const UserProfile& profile) { const std::set<std::string>& permissions = profile.permissions;
--- a/Plugin/AuthorizationWebService.h Wed Jun 11 10:55:44 2025 +0200 +++ b/Plugin/AuthorizationWebService.h Fri Jul 11 18:20:32 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:
--- a/Plugin/BaseAuthorizationService.h Wed Jun 11 10:55:44 2025 +0200 +++ b/Plugin/BaseAuthorizationService.h Fri Jul 11 18:20:32 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<std::string>& anyOfPermissions, + virtual bool HasUserPermission(const std::set<std::string>& anyOfPermissions, const UserProfile& profile) ORTHANC_OVERRIDE { if (anyOfPermissions.size() == 0) @@ -94,7 +92,7 @@ for (std::set<std::string>::const_iterator it = anyOfPermissions.begin(); it != anyOfPermissions.end(); ++it) { - if (HasUserPermissionInternal(validity, *it, profile)) + if (HasUserPermissionInternal(*it, profile)) { return true; } @@ -102,8 +100,7 @@ return false; } - virtual bool HasAnonymousUserPermission(unsigned int& validity /* out */, - const std::set<std::string>& anyOfPermissions) ORTHANC_OVERRIDE + virtual bool HasAnonymousUserPermission(const std::set<std::string>& anyOfPermissions) ORTHANC_OVERRIDE { if (anyOfPermissions.size() == 0) { @@ -115,7 +112,7 @@ for (std::set<std::string>::const_iterator it = anyOfPermissions.begin(); it != anyOfPermissions.end(); ++it) { - if (HasUserPermissionInternal(validity, *it, anonymousUserProfile)) + if (HasUserPermissionInternal(*it, anonymousUserProfile)) { return true; }
--- a/Plugin/CachedAuthorizationService.cpp Wed Jun 11 10:55:44 2025 +0200 +++ b/Plugin/CachedAuthorizationService.cpp Fri Jul 11 18:20:32 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; } - - - }
--- a/Plugin/CachedAuthorizationService.h Wed Jun 11 10:55:44 2025 +0200 +++ b/Plugin/CachedAuthorizationService.h Fri Jul 11 18:20:32 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;
--- a/Plugin/IAuthorizationService.h Wed Jun 11 10:55:44 2025 +0200 +++ b/Plugin/IAuthorizationService.h Fri Jul 11 18:20:32 2025 +0200 @@ -92,12 +92,10 @@ virtual bool GetAnonymousUserProfile(unsigned int& validity /* out */, UserProfile& profile /* out */) = 0; - virtual bool HasUserPermission(unsigned int& validity /* out */, - const std::set<std::string>& anyOfPermissions, + virtual bool HasUserPermission(const std::set<std::string>& anyOfPermissions, const UserProfile& profile) = 0; - virtual bool HasAnonymousUserPermission(unsigned int& validity /* out */, - const std::set<std::string>& anyOfPermissions) = 0; + virtual bool HasAnonymousUserPermission(const std::set<std::string>& anyOfPermissions) = 0; virtual bool CreateToken(CreatedToken& response, const std::string& tokenType,
--- a/Plugin/Plugin.cpp Wed Jun 11 10:55:44 2025 +0200 +++ b/Plugin/Plugin.cpp Fri Jul 11 18:20:32 2025 +0200 @@ -327,8 +327,7 @@ LOG(INFO) << msg; - unsigned int validity; // ignored - if (authorizationService_->HasAnonymousUserPermission(validity, requiredPermissions)) + if (authorizationService_->HasAnonymousUserPermission(requiredPermissions)) { LOG(INFO) << msg << " -> granted"; hasUserRequiredPermissions = true; @@ -351,8 +350,7 @@ unsigned int validityNotUsed; authorizationService_->GetUserProfile(validityNotUsed, profile, authTokens[i].GetToken(), authTokens[i].GetValue()); - unsigned int validity; // ignored - if (authorizationService_->HasUserPermission(validity, requiredPermissions, profile)) + if (authorizationService_->HasUserPermission(requiredPermissions, profile)) { LOG(INFO) << msg << " -> granted"; hasUserRequiredPermissions = true;