changeset 153:3683f3d083bd

fix tools/find to allow accessing /dicom-web/studies/../series/../instances/.. on studies that have at least one authorized_labels
author Alain Mazy <am@osimis.io>
date Mon, 18 Mar 2024 11:58:20 +0100
parents 9f686ee4b158
children ae1bd3d15f81
files NEWS Plugin/AuthorizationParserBase.h Plugin/IAuthorizationParser.h Plugin/Plugin.cpp
diffstat 4 files changed, 111 insertions(+), 37 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Fri Mar 15 09:08:21 2024 +0100
+++ b/NEWS	Mon Mar 18 11:58:20 2024 +0100
@@ -2,6 +2,15 @@
 ===============================
 
 * Added missing parsing of /dicom-web/studies/.../instances
+* Clarification: the "authorized_labels" field of the user profile
+  is actually a list of studies authorized labels !  
+* The tools/find has been updated to take this into account and will 
+  possibly refuse to perform tools/find at instance or series level if there
+  is no StudyInstanceUID in the query.
+  This fixes forbidden access to e.g. /dicom-web/studies/../series/../instances/..
+  on studies that have at least one authorized_labels.
+
+
 
 2024-02-16 - v 0.7.0
 ====================
--- a/Plugin/AuthorizationParserBase.h	Fri Mar 15 09:08:21 2024 +0100
+++ b/Plugin/AuthorizationParserBase.h	Mon Mar 18 11:58:20 2024 +0100
@@ -60,9 +60,6 @@
     void AddDicomPatient(AccessedResources& target,
                          const std::string& patientId);
 
-    void AddDicomStudy(AccessedResources& target,
-                       const std::string& studyDicomUid);
-    
     void AddDicomSeries(AccessedResources& target,
                         const std::string& studyDicomUid,
                         const std::string& seriesDicomUid);
@@ -73,6 +70,9 @@
                           const std::string& instanceDicomUid);
 
   public:
+    virtual void AddDicomStudy(AccessedResources& target,
+                               const std::string& studyDicomUid);
+
     explicit AuthorizationParserBase(ICacheFactory& factory);
 
     virtual void Invalidate(Orthanc::ResourceType level,
--- a/Plugin/IAuthorizationParser.h	Fri Mar 15 09:08:21 2024 +0100
+++ b/Plugin/IAuthorizationParser.h	Mon Mar 18 11:58:20 2024 +0100
@@ -49,5 +49,9 @@
     virtual bool IsListOfResources(const std::string& uri) const = 0;
 
     virtual void GetSingleResourcePatterns(std::vector<boost::regex>& patterns) const = 0;
+
+    virtual void AddDicomStudy(AccessedResources& target,
+                               const std::string& studyDicomUid) = 0;
+
   };
 }
--- a/Plugin/Plugin.cpp	Fri Mar 15 09:08:21 2024 +0100
+++ b/Plugin/Plugin.cpp	Mon Mar 18 11:58:20 2024 +0100
@@ -97,6 +97,52 @@
   return (profile.authorizedLabels.size() > 0);
 }
 
+static bool HasAuthorizedLabelsForResource(bool& granted,
+                                           const OrthancPlugins::IAuthorizationParser::AccessedResources& accesses,
+                                           const OrthancPlugins::IAuthorizationService::UserProfile& profile)
+{
+  granted = false;
+
+  if (HasAccessToAllLabels(profile))
+  {
+    granted = true;
+    return true; // we could check labels
+  }
+
+  // Loop over all the accessed resources to ensure access is
+  // granted to each of them
+  for (OrthancPlugins::IAuthorizationParser::AccessedResources::const_iterator
+      access = accesses.begin(); access != accesses.end(); ++access)
+  {
+    // Ignored the access levels that are unchecked
+    // (cf. "UncheckedLevels" option)
+    if (uncheckedLevels_.find(access->GetLevel()) == uncheckedLevels_.end())
+    {
+      std::string msg = std::string("Testing whether access to ") + OrthancPlugins::EnumerationToString(access->GetLevel()) + " \"" + access->GetOrthancId() + "\" is allowed wrt Labels for User '" + profile.name + "'";
+      const std::set<std::string>& resourceLabels = access->GetLabels();
+      std::set<std::string> authorizedResourceLabels;
+
+      Orthanc::Toolbox::GetIntersection(authorizedResourceLabels, resourceLabels, profile.authorizedLabels);
+
+      if (authorizedResourceLabels.size() == 0)
+      {
+        LOG(INFO) << msg << " -> not granted, no authorized labels";
+        granted = false;
+        return true; // we could check labels
+      }
+      else
+      {
+        LOG(INFO) << msg << " -> granted, at least one authorized labels";
+        granted = true;
+        return true; // we could check labels
+      }
+    }
+  }
+
+  // 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 bool CheckAuthorizedLabelsForResource(bool& granted,
                                              const std::string& uri,
@@ -125,37 +171,10 @@
     if (authorizationParser_->IsListOfResources(uri))
     {
       granted = false;  // if a user does not have access to all labels, he can not have access to a list of resources
-      return true; 
+      return true; // we could check labels
     }
 
-    // Loop over all the accessed resources to ensure access is
-    // granted to each of them
-    for (OrthancPlugins::IAuthorizationParser::AccessedResources::const_iterator
-        access = accesses.begin(); access != accesses.end(); ++access)
-    {
-      // Ignored the access levels that are unchecked
-      // (cf. "UncheckedLevels" option)
-      if (uncheckedLevels_.find(access->GetLevel()) == uncheckedLevels_.end())
-      {
-        std::string msg = std::string("Testing whether access to ") + OrthancPlugins::EnumerationToString(access->GetLevel()) + " \"" + access->GetOrthancId() + "\" is allowed wrt Labels for User '" + profile.name + "'";
-        const std::set<std::string>& resourceLabels = access->GetLabels();
-        std::set<std::string> authorizedResourceLabels;
-
-        Orthanc::Toolbox::GetIntersection(authorizedResourceLabels, resourceLabels, profile.authorizedLabels);
-
-        if (authorizedResourceLabels.size() == 0)
-        {
-          LOG(INFO) << msg << " -> not granted, no authorized labels";
-          return true; // we could check labels
-        }
-        else
-        {
-          granted = true;
-          LOG(INFO) << msg << " -> granted, at least one authorized labels";
-          return true; // we could check labels
-        }
-      }
-    }
+    return HasAuthorizedLabelsForResource(granted, accesses, profile);
   }
 
   // This method only checks if a resource is accessible thanks to its labels.  If we could not check it, we always return false !!
@@ -662,8 +681,8 @@
     {
       // The filtering to this route is performed by this plugin as it is done for any other route before we get here.
 
-      Json::Value body;
-      if (!OrthancPlugins::ReadJson(body, request->body, request->bodySize))
+      Json::Value query;
+      if (!OrthancPlugins::ReadJson(query, request->body, request->bodySize))
       {
         throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat, "A JSON payload was expected");
       }
@@ -672,7 +691,48 @@
       OrthancPlugins::IAuthorizationService::UserProfile profile;
       if (GetUserProfileInternal(profile, request) && HasAccessToSomeLabels(profile))
       {
-        AdjustToolsFindQueryLabels(body, profile);
+        Orthanc::ResourceType queryLevel = Orthanc::StringToResourceType(query["Level"].asString().c_str());
+
+        if (queryLevel == Orthanc::ResourceType_Study)
+        {
+          AdjustToolsFindQueryLabels(query, profile);
+        }
+        else if (queryLevel == Orthanc::ResourceType_Patient && !HasAccessToAllLabels(profile))
+        {
+          throw Orthanc::OrthancException(Orthanc::ErrorCode_ForbiddenAccess, "Auth plugin: unable to call tools/find at Patient level when the user does not have access to ALL labels.");
+        }
+        else if (queryLevel == Orthanc::ResourceType_Series || queryLevel == Orthanc::ResourceType_Instance)
+        {
+          std::string studyInstanceUID;
+
+          if (!HasAccessToAllLabels(profile) && !GetStudyInstanceUIDFromQuery(studyInstanceUID, query))
+          {
+            throw Orthanc::OrthancException(Orthanc::ErrorCode_ForbiddenAccess, "Auth plugin: unable to call tools/find at Series or Instance level when the user does not have access to ALL labels or when there is no StudyInstanceUID in the query.");
+          }
+          else
+          {
+            // since this is a series/instance find, make sure the user has access to the parent study
+            Json::Value studyOrhtancIds;
+            if (!OrthancPlugins::RestApiPost(studyOrhtancIds, "/tools/lookup", studyInstanceUID, false) || studyOrhtancIds.size() != 1)
+            {
+              throw Orthanc::OrthancException(Orthanc::ErrorCode_ForbiddenAccess, "Auth plugin: when using tools/find at Series or Instance level, unable to get the orthanc ID of StudyInstanceUID specified in the query.");
+            }
+
+            bool granted = false;
+            OrthancPlugins::IAuthorizationParser::AccessedResources accessedResources;
+            authorizationParser_->AddDicomStudy(accessedResources, studyInstanceUID);
+
+            if (!HasAuthorizedLabelsForResource(granted, accessedResources, profile))
+            {
+              throw Orthanc::OrthancException(Orthanc::ErrorCode_ForbiddenAccess, "Auth plugin: when using tools/find at Series or Instance level, unable to check resource access based on the authorized_labels.");
+            }
+
+            if (!granted)
+            {
+              throw Orthanc::OrthancException(Orthanc::ErrorCode_ForbiddenAccess, "Auth plugin: when using tools/find at Series or Instance level, the user shall have access to the parent study.");
+            }
+          }
+        }
       }
       else // anonymous user profile or resource token
       {
@@ -681,7 +741,7 @@
         // If anonymous user profile, it might be a resource token e.g accessing /dicom-web/studies/.../metadata 
         // -> extract the StudyInstanceUID from the query and send the token for validation to the auth-service
         // If there is no StudyInstanceUID, then, return a 403 because we don't know what resource it relates to
-        if (!GetStudyInstanceUIDFromQuery(studyInstanceUID, body))
+        if (!GetStudyInstanceUIDFromQuery(studyInstanceUID, query))
         {
           throw Orthanc::OrthancException(Orthanc::ErrorCode_ForbiddenAccess, "Auth plugin: unable to call tools/find when the user does not have access to any labels and if there is no StudyInstanceUID in the query.");
         }
@@ -705,7 +765,8 @@
       }
 
       Json::Value result;
-      if (OrthancPlugins::RestApiPost(result, "/tools/find", body, false))
+
+      if (OrthancPlugins::RestApiPost(result, "/tools/find", query, false))
       {
         OrthancPlugins::AnswerJson(result, output);
       }