# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1619625026 -7200
# Node ID 4beebbb3636e3d5a159f06088a011c3028312d4b
# Parent  1f90fe0fa13fb3041091766ad04c8b87bd805980
Fix regression in the handling of "DicomCheckModalityHost" configuration option

diff -r 1f90fe0fa13f -r 4beebbb3636e NEWS
--- a/NEWS	Wed Apr 28 16:40:36 2021 +0200
+++ b/NEWS	Wed Apr 28 17:50:26 2021 +0200
@@ -5,7 +5,9 @@
 * Fixed the lifetime of temporary files associated with jobs that create ZIP archive/media:
   - In synchronous mode, their number could grow up to "JobsHistorySize" in Orthanc <= 1.9.2
   - In asynchronous mode, the temporary files are removed as soon as their job gets canceled
-
+* Fix regression in the handling of "DicomCheckModalityHost" configuration option
+  introduced by changeset 4182 in Orthanc 1.7.4
+  
 
 Version 1.9.2 (2021-04-22)
 ==========================
diff -r 1f90fe0fa13f -r 4beebbb3636e OrthancServer/Sources/OrthancConfiguration.cpp
--- a/OrthancServer/Sources/OrthancConfiguration.cpp	Wed Apr 28 16:40:36 2021 +0200
+++ b/OrthancServer/Sources/OrthancConfiguration.cpp	Wed Apr 28 17:50:26 2021 +0200
@@ -754,7 +754,8 @@
     return false;
   }
 
-  bool OrthancConfiguration::LookupDicomModalitiesUsingAETitle(std::list<RemoteModalityParameters>& modalities,
+  
+  void OrthancConfiguration::LookupDicomModalitiesUsingAETitle(std::list<RemoteModalityParameters>& modalities,
                                                                const std::string& aet) const
   {
     modalities.clear();
@@ -766,8 +767,6 @@
         modalities.push_back(it->second);
       }
     }
-
-    return modalities.size() > 0;
   }
 
 
diff -r 1f90fe0fa13f -r 4beebbb3636e OrthancServer/Sources/OrthancConfiguration.h
--- a/OrthancServer/Sources/OrthancConfiguration.h	Wed Apr 28 16:40:36 2021 +0200
+++ b/OrthancServer/Sources/OrthancConfiguration.h	Wed Apr 28 17:50:26 2021 +0200
@@ -206,7 +206,7 @@
     bool IsSameAETitle(const std::string& aet1,
                        const std::string& aet2) const;
 
-    bool LookupDicomModalitiesUsingAETitle(std::list<RemoteModalityParameters>& modalities,
+    void LookupDicomModalitiesUsingAETitle(std::list<RemoteModalityParameters>& modalities,
                                            const std::string& aet) const;
 
     bool LookupDicomModalityUsingAETitle(RemoteModalityParameters& modality,
diff -r 1f90fe0fa13f -r 4beebbb3636e OrthancServer/Sources/main.cpp
--- a/OrthancServer/Sources/main.cpp	Wed Apr 28 16:40:36 2021 +0200
+++ b/OrthancServer/Sources/main.cpp	Wed Apr 28 17:50:26 2021 +0200
@@ -324,6 +324,17 @@
     }
   }
 
+  static void ReportDisallowedCommand(const std::string& remoteIp,
+                                      const std::string& remoteAet,
+                                      DicomRequestType type)
+  {
+    LOG(WARNING) << "Unable to check DICOM authorization for AET " << remoteAet
+                 << " on IP " << remoteIp << ": The DICOM command "
+                 << EnumerationToString(type) << " is not allowed for this modality "
+                 << "according to configuration option \"DicomModalities\"";
+  }
+  
+
   virtual bool IsAllowedRequest(const std::string& remoteIp,
                                 const std::string& remoteAet,
                                 const std::string& calledAet,
@@ -358,33 +369,68 @@
     }
     else
     {
-      OrthancConfiguration::ReaderLock lock;
+      bool checkIp;
+      std::list<RemoteModalityParameters> modalities;
 
-      std::list<RemoteModalityParameters> modalities;
-      if (lock.GetConfiguration().LookupDicomModalitiesUsingAETitle(modalities, remoteAet))
+      {
+        OrthancConfiguration::ReaderLock lock;
+        lock.GetConfiguration().LookupDicomModalitiesUsingAETitle(modalities, remoteAet);
+        checkIp = lock.GetConfiguration().GetBooleanParameter("DicomCheckModalityHost", false);
+      }
+      
+      if (modalities.empty())
       {
-        if (modalities.size() == 1) // don't check the IP if there's only one modality with this AET
-        {
-          return modalities.front().IsRequestAllowed(type);
-        }
-        else // if there are multiple modalities with the same AET, check the one matching this IP
+        LOG(WARNING) << "Unable to check DICOM authorization for AET " << remoteAet
+                     << " on IP " << remoteIp << ": This AET is not listed in "
+                     << "configuration option \"DicomModalities\"";
+        return false;
+      }
+      else if (modalities.size() == 1)
+      {
+        // DicomCheckModalityHost is true: check if the IP match the configured IP
+        if (checkIp &&
+            remoteIp != modalities.front().GetHost())
         {
-          for (std::list<RemoteModalityParameters>::const_iterator it = modalities.begin(); it != modalities.end(); ++it)
-          {
-            if (it->GetHost() == remoteIp)
-            {
-              return it->IsRequestAllowed(type);
-            }
-          }
-
           LOG(WARNING) << "Unable to check DICOM authorization for AET " << remoteAet
-                       << " on IP " << remoteIp << ", " << modalities.size()
-                       << " modalites found with this AET but none of them matching the IP";
+                       << " on IP " << remoteIp << ": Its IP address should be "
+                       << modalities.front().GetHost()
+                       << " according to configuration option \"DicomModalities\"";
+          return false;
         }
-        return false;
+        else if (modalities.front().IsRequestAllowed(type))
+        {
+          return true;
+        }
+        else
+        {
+          ReportDisallowedCommand(remoteIp, remoteAet, type);
+          return false;
+        }
       }
       else
       {
+        // If there are multiple modalities with the same AET, consider the one matching this IP
+        for (std::list<RemoteModalityParameters>::const_iterator
+               it = modalities.begin(); it != modalities.end(); ++it)
+        {
+          if (it->GetHost() == remoteIp)
+          {
+            if (it->IsRequestAllowed(type))
+            {
+              return true;
+            }
+            else
+            {
+              ReportDisallowedCommand(remoteIp, remoteAet, type);
+              return false;
+            }
+          }
+        }
+
+        LOG(WARNING) << "Unable to check DICOM authorization for AET " << remoteAet
+                     << " on IP " << remoteIp << ": " << modalities.size()
+                     << " modalites found with this AET in configuration option "
+                     << "\"DicomModalities\", but none of them matches the IP";
         return false;
       }
     }