changeset 115:0eed78c1e177

cache the UserProfile + updated http filter logic
author Alain Mazy <am@osimis.io>
date Fri, 08 Sep 2023 09:52:21 +0200
parents 546aea509427
children 89eddd4b2f6a
files Plugin/AuthorizationWebService.cpp Plugin/AuthorizationWebService.h Plugin/CachedAuthorizationService.cpp Plugin/DefaultAuthorizationParser.cpp Plugin/DefaultAuthorizationParser.h Plugin/DefaultConfiguration.json Plugin/IAuthorizationParser.h Plugin/Plugin.cpp Plugin/Token.cpp
diffstat 9 files changed, 159 insertions(+), 72 deletions(-) [+]
line wrap: on
line diff
--- a/Plugin/AuthorizationWebService.cpp	Wed Sep 06 17:02:41 2023 +0200
+++ b/Plugin/AuthorizationWebService.cpp	Fri Sep 08 09:52:21 2023 +0200
@@ -33,7 +33,7 @@
   static const char* PERMISSIONS = "permissions";
   static const char* AUTHORIZED_LABELS = "authorized-labels";
   static const char* USER_NAME = "name";
-
+  
 
   bool AuthorizationWebService::IsGrantedInternal(unsigned int& validity,
                                                   OrthancPluginHttpMethod method,
@@ -321,6 +321,41 @@
 
   }
 
+  void AuthorizationWebService::ToJson(Json::Value& jsonProfile, const UserProfile& profile)
+  {
+    jsonProfile = Json::objectValue;
+    jsonProfile[USER_NAME] = profile.name;
+    Orthanc::SerializationToolbox::WriteSetOfStrings(jsonProfile, profile.authorizedLabels, AUTHORIZED_LABELS);
+    Orthanc::SerializationToolbox::WriteSetOfStrings(jsonProfile, profile.permissions, PERMISSIONS);
+  }
+    
+  void AuthorizationWebService::FromJson(UserProfile& profile, const Json::Value& jsonProfile)
+  {
+    if (jsonProfile.type() != Json::objectValue ||
+        !jsonProfile.isMember(PERMISSIONS) ||
+        !jsonProfile.isMember(AUTHORIZED_LABELS) ||
+        !jsonProfile.isMember(USER_NAME) ||
+        jsonProfile[PERMISSIONS].type() != Json::arrayValue ||
+        jsonProfile[AUTHORIZED_LABELS].type() != Json::arrayValue ||
+        jsonProfile[USER_NAME].type() != Json::stringValue)
+    {
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat,
+                                      "Syntax error in the result of the Auth Web service, the format of the UserProfile is invalid");
+    }
+
+    profile.name = jsonProfile[USER_NAME].asString();
+
+    for (Json::ArrayIndex i = 0; i < jsonProfile[PERMISSIONS].size(); ++i)
+    {
+      profile.permissions.insert(jsonProfile[PERMISSIONS][i].asString());
+    }
+    for (Json::ArrayIndex i = 0; i < jsonProfile[AUTHORIZED_LABELS].size(); ++i)
+    {
+      profile.authorizedLabels.insert(jsonProfile[AUTHORIZED_LABELS][i].asString());
+    }
+  }
+
+
 
   bool AuthorizationWebService::GetUserProfileInternal(unsigned int& validity,
                                                        UserProfile& profile /* out */,
@@ -372,35 +407,18 @@
       Json::Value jsonProfile;
       authClient.ApplyAndThrowException(jsonProfile);
 
-      if (jsonProfile.type() != Json::objectValue ||
-          !jsonProfile.isMember(PERMISSIONS) ||
-          !jsonProfile.isMember(VALIDITY) ||
-          !jsonProfile.isMember(AUTHORIZED_LABELS) ||
-          !jsonProfile.isMember(USER_NAME) ||
-          jsonProfile[PERMISSIONS].type() != Json::arrayValue ||
-          jsonProfile[AUTHORIZED_LABELS].type() != Json::arrayValue ||
-          jsonProfile[VALIDITY].type() != Json::intValue ||
-          jsonProfile[USER_NAME].type() != Json::stringValue)
+      if (!jsonProfile.isMember(VALIDITY) ||
+        jsonProfile[VALIDITY].type() != Json::intValue)
       {
-        throw Orthanc::OrthancException(Orthanc::ErrorCode_NetworkProtocol,
+        throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat,
                                         "Syntax error in the result of the Auth Web service, the format of the UserProfile is invalid");
       }
-
       validity = jsonProfile[VALIDITY].asUInt();
-      
-      profile.name = jsonProfile[USER_NAME].asString();
       profile.tokenKey = token->GetKey();
       profile.tokenType = token->GetType();
       profile.tokenValue = tokenValue;
 
-      for (Json::ArrayIndex i = 0; i < jsonProfile[PERMISSIONS].size(); ++i)
-      {
-        profile.permissions.insert(jsonProfile[PERMISSIONS][i].asString());
-      }
-      for (Json::ArrayIndex i = 0; i < jsonProfile[AUTHORIZED_LABELS].size(); ++i)
-      {
-        profile.authorizedLabels.insert(jsonProfile[AUTHORIZED_LABELS][i].asString());
-      }
+      FromJson(profile, jsonProfile);
 
       if (profile.authorizedLabels.size() == 0)
       {
--- a/Plugin/AuthorizationWebService.h	Wed Sep 06 17:02:41 2023 +0200
+++ b/Plugin/AuthorizationWebService.h	Fri Sep 08 09:52:21 2023 +0200
@@ -93,5 +93,9 @@
                              const std::string& tokenKey, 
                              const std::string& tokenValue);
 
+    static void ToJson(Json::Value& output, const UserProfile& profile);
+    
+    static void FromJson(UserProfile& profile, const Json::Value& input);
+
   };
 }
--- a/Plugin/CachedAuthorizationService.cpp	Wed Sep 06 17:02:41 2023 +0200
+++ b/Plugin/CachedAuthorizationService.cpp	Fri Sep 08 09:52:21 2023 +0200
@@ -17,6 +17,7 @@
  **/
 
 #include "CachedAuthorizationService.h"
+#include "AuthorizationWebService.h"
 
 #include <OrthancException.h>
 #include <Toolbox.h>
@@ -112,13 +113,49 @@
   }
 
   
-  bool CachedAuthorizationService::GetUserProfileInternal(unsigned int& validity,
+  bool CachedAuthorizationService::GetUserProfileInternal(unsigned int& validityNotUsed,
                                                           UserProfile& profile /* out */,
                                                           const Token* token,
                                                           const std::string& tokenValue)
   {
-    // no cache used when retrieving the full user profile
-    return decorated_->GetUserProfileInternal(validity, profile, token, tokenValue);
+    assert(decorated_.get() != NULL);
+
+    std::string key = ComputeKey("user-profile", token, tokenValue);
+    std::string serializedProfile;
+
+    if (cache_->Retrieve(serializedProfile, key))
+    {
+      // Return the previously cached profile
+      Json::Value jsonProfile;
+      
+      Orthanc::Toolbox::ReadJson(jsonProfile, serializedProfile);
+      
+      AuthorizationWebService::FromJson(profile, jsonProfile);
+
+      profile.tokenKey = token->GetKey();
+      profile.tokenType = token->GetType();
+      profile.tokenValue = tokenValue;
+
+      return true;
+    }        
+    else
+    {
+      unsigned int validity;
+
+      if (decorated_->GetUserProfileInternal(validity, profile, token, tokenValue))
+      {
+        Json::Value jsonProfile;
+
+        AuthorizationWebService::ToJson(jsonProfile, profile);
+        Orthanc::Toolbox::WriteFastJson(serializedProfile, jsonProfile);
+
+        cache_->Store(key, serializedProfile, validity);
+        
+        return true;
+      }
+    }
+
+    return false;
   }
 
   bool CachedAuthorizationService::HasUserPermissionInternal(unsigned int& validity,
--- a/Plugin/DefaultAuthorizationParser.cpp	Wed Sep 06 17:02:41 2023 +0200
+++ b/Plugin/DefaultAuthorizationParser.cpp	Fri Sep 08 09:52:21 2023 +0200
@@ -31,7 +31,8 @@
     instancesPattern_("^/web-viewer/instances/[a-z0-9]+-([a-f0-9-]+)_[0-9]+$"),
     osimisViewerSeries_("^/osimis-viewer/series/([a-f0-9-]+)(|/.*)$"),
     osimisViewerImages_("^/osimis-viewer/(images|custom-command)/([a-f0-9-]+)(|/.*)$"),
-    osimisViewerStudies_("^/osimis-viewer/studies/([a-f0-9-]+)(|/.*)$")
+    osimisViewerStudies_("^/osimis-viewer/studies/([a-f0-9-]+)(|/.*)$"),
+    listOfResourcesPattern_("^/(patients|studies|series|instances)(|/)$")
   {
     std::string tmp = dicomWebRoot;
     while (!tmp.empty() &&
@@ -53,6 +54,17 @@
       "^" + tmp + "/(studies|series|instances)(|/)$");
   }
 
+  bool DefaultAuthorizationParser::IsListOfResources(const std::string& uri)
+  {
+    if (boost::regex_match(uri, listOfResourcesPattern_))
+    {
+      return true;
+    }
+
+    return false;
+  }
+
+
 
   bool DefaultAuthorizationParser::Parse(AccessedResources& target,
                                          const std::string& uri,
--- a/Plugin/DefaultAuthorizationParser.h	Wed Sep 06 17:02:41 2023 +0200
+++ b/Plugin/DefaultAuthorizationParser.h	Fri Sep 08 09:52:21 2023 +0200
@@ -41,6 +41,8 @@
     boost::regex osimisViewerImages_;
     boost::regex osimisViewerStudies_;
 
+    boost::regex listOfResourcesPattern_;
+
   public:
     DefaultAuthorizationParser(ICacheFactory& factory,
                                const std::string& dicomWebRoot);
@@ -48,5 +50,7 @@
     virtual bool Parse(AccessedResources& target,
                        const std::string& uri,
                        const std::map<std::string, std::string>& getArguments);
+
+    virtual bool IsListOfResources(const std::string& uri);
   };
 }
--- a/Plugin/DefaultConfiguration.json	Wed Sep 06 17:02:41 2023 +0200
+++ b/Plugin/DefaultConfiguration.json	Fri Sep 08 09:52:21 2023 +0200
@@ -58,6 +58,7 @@
 
             // elemental browsing in OE2
             ["post", "^/tools/find$", "all|view"],
+            ["get" , "^/(patients|studies|series|instances)(|/)", "all|view"],
             ["get" , "^/(patients|studies|series|instances)/([a-f0-9-]+)$", "all|view"],
             ["get" , "^/(patients|studies|series|instances)/([a-f0-9-]+)/(studies|study|series|instances)$", "all|view"],
             ["get" , "^/instances/([a-f0-9-]+)/(tags|header)$", "all|view"],
--- a/Plugin/IAuthorizationParser.h	Wed Sep 06 17:02:41 2023 +0200
+++ b/Plugin/IAuthorizationParser.h	Fri Sep 08 09:52:21 2023 +0200
@@ -42,5 +42,7 @@
     virtual bool Parse(AccessedResources& target,
                        const std::string& uri,
                        const std::map<std::string, std::string>& getArguments) = 0;
+
+    virtual bool IsListOfResources(const std::string& uri) = 0;
   };
 }
--- a/Plugin/Plugin.cpp	Wed Sep 06 17:02:41 2023 +0200
+++ b/Plugin/Plugin.cpp	Fri Sep 08 09:52:21 2023 +0200
@@ -84,13 +84,17 @@
 }
 
 
-static bool CheckAuthorizedLabelsForResource(const std::string& uri,
+static bool CheckAuthorizedLabelsForResource(bool& granted,
+                                             const std::string& uri,
                                              const OrthancPlugins::AssociativeArray& getArguments,
                                              const OrthancPlugins::IAuthorizationService::UserProfile& profile)
 {
+  granted = false;
+
   if (HasAccessToAllLabels(profile))
   {
-    return true;
+    granted = true;
+    return true; // we could check labels
   }
 
   if (authorizationParser_.get() != NULL &&
@@ -101,7 +105,13 @@
 
     if (!authorizationParser_->Parse(accesses, uri, getArguments.GetMap()))
     {
-      return false;  // Unable to parse this URI
+      return false;  // Unable to parse this URI, we could not check labels
+    }
+
+    if (authorizationParser_->IsListOfResources(uri))
+    {
+      granted = false;
+      return true; // if a user does not have access to all labels, he can not have access to a list of resources
     }
 
     // Loop over all the accessed resources to ensure access is
@@ -122,21 +132,20 @@
         if (authorizedResourceLabels.size() == 0)
         {
           LOG(INFO) << msg << " -> not granted, no authorized labels";
-          return false;
+          return true; // we could check labels
         }
         else
         {
+          granted = true;
           LOG(INFO) << msg << " -> granted, at least one authorized labels";
-          return true;
+          return true; // we could check labels
         }
       }
     }
-
-    // Access is granted to all the resources that are 'unchecked'
-    return true;
   }
 
-  return false;  // TODO or true ???
+  // This method only checks if a resource is accessible thanks to its labels.  If we could not check it, we always return false !!
+  return false; // we could not check labels
 }
 
 static int32_t FilterHttpRequests(OrthancPluginHttpMethod method,
@@ -208,6 +217,8 @@
 
     // Based on the tokens, check if the user has access based on its permissions and the mapping between urls and permissions
     ////////////////////////////////////////////////////////////////
+    bool hasUserRequiredPermissions = false;
+    bool hasAuthorizedLabelsForResource = false;
 
     if (permissionParser_.get() != NULL &&
       authorizationService_.get() != NULL) 
@@ -220,25 +231,18 @@
         {
           std::string msg = std::string("Testing whether anonymous user has any of the required permissions '") + JoinStrings(requiredPermissions) + "'";
           
-          // TODO: how to handle anonymous user ?
-          
-          // LOG(INFO) << msg;
-          // if (authorizationService_->HasAnonymousUserPermission(validity, requiredPermissions))
-          // {
-          //     // TODO: check labels permissions
-          //   LOG(INFO) << msg << " -> granted";
-
-          //   if (CheckAuthorizedLabelsForResource(uri, getArguments, profile))
-          //   {
-          //     return 1;
-          //   }
-          // }
-          // else
-          // {
-          //   LOG(INFO) << msg << " -> not granted";
-          // }
-          LOG(INFO) << msg << " -> not granted, TODO ????";
-          return 0;
+          LOG(INFO) << msg; 
+          if (authorizationService_->HasAnonymousUserPermission(validity, requiredPermissions))
+          {
+            LOG(INFO) << msg << " -> granted";
+            hasUserRequiredPermissions = true;
+          }
+          else
+          {
+            LOG(INFO) << msg << " -> not granted";
+            hasUserRequiredPermissions = false;
+            // continue in order to check if there is a resource token that could grant access to the resource
+          }
         }
         else
         {
@@ -246,8 +250,7 @@
           {
             std::string msg = std::string("Testing whether user has the required permissions '") + JoinStrings(requiredPermissions) + "' based on the HTTP header '" + authTokens[i].GetToken().GetKey() + "' required to match '" + matchedPattern + "'";
 
-            LOG(INFO) << msg;
-            
+            // LOG(INFO) << msg;
             OrthancPlugins::IAuthorizationService::UserProfile profile;
             unsigned int validityNotUsed;
             authorizationService_->GetUserProfile(validityNotUsed, profile, authTokens[i].GetToken(), authTokens[i].GetValue());
@@ -255,25 +258,41 @@
             if (authorizationService_->HasUserPermission(validity, requiredPermissions, profile))
             {
               LOG(INFO) << msg << " -> granted";
+              hasUserRequiredPermissions = true;
 
               // check labels permissions
-              if (CheckAuthorizedLabelsForResource(uri, getArguments, profile))
+              std::string msg = std::string("Testing whether user has the authorized_labels to access '") + uri + "' based on the HTTP header '" + authTokens[i].GetToken().GetKey() + "'";
+              if (CheckAuthorizedLabelsForResource(hasAuthorizedLabelsForResource, uri, getArguments, profile))
               {
-                return 1;
+                if (hasAuthorizedLabelsForResource)
+                {
+                  LOG(INFO) << msg << " -> granted";
+                }
+                else
+                {
+                  LOG(INFO) << msg << " -> not granted";
+                  return 0; // the labels for this resource prevents access -> stop checking now !
+                }
               }
-              // not granted, but continue and check if a resource tokens grant access
             }
             else
             {
-              LOG(INFO) << msg << " -> not granted";  // but continue and check if a resource tokens grant access
+              LOG(INFO) << msg << " -> not granted";
+              hasUserRequiredPermissions = false;
             }
           }
         }
       }
     }
 
+    // no need to check for resource token if the user has access and if the labels checking has not prevented access
+    if (hasUserRequiredPermissions)
+    {
+      return 1;
+    }
 
-    // 
+    // If we get till here, it means that we have a resource token -> check that the resource is accessible
+    ////////////////////////////////////////////////////////////////
 
     if (authorizationParser_.get() != NULL &&
         authorizationService_.get() != NULL)
@@ -325,12 +344,10 @@
           else
           {
             LOG(INFO) << msg << " -> granted";
+            return 1;
           }
         }
       }
-
-      // Access is granted to all the resources
-      return 1;
     }
       
     // By default, forbid access to all the resources
--- a/Plugin/Token.cpp	Wed Sep 06 17:02:41 2023 +0200
+++ b/Plugin/Token.cpp	Fri Sep 08 09:52:21 2023 +0200
@@ -27,20 +27,12 @@
     type_(type),
     key_(key)
   {
-    if (key.empty())
-    {
-      throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange);
-    }
   }
 
   Token::Token(const Token& other) :
     type_(other.GetType()),
     key_(other.GetKey())
   {
-    if (key_.empty())
-    {
-      throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange);
-    }
   }
 
 }