Mercurial > hg > orthanc-authorization

changeset 112:572955904411

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
added tools/labels + removed forbidden_labels
author Alain Mazy <am@osimis.io>
date Thu, 31 Aug 2023 16:51:15 +0200
parents 2b1a95c7d263
children 43154740ea2e
files Plugin/AuthorizationWebService.cpp Plugin/IAuthorizationService.h Plugin/Plugin.cpp UnitTestsSources/UnitTestsMain.cpp
diffstat 4 files changed, 178 insertions(+), 131 deletions(-) [+]
line wrap: on
line diff
--- a/Plugin/AuthorizationWebService.cpp	Wed Aug 30 18:10:09 2023 +0200
+++ b/Plugin/AuthorizationWebService.cpp	Thu Aug 31 16:51:15 2023 +0200
@@ -32,7 +32,6 @@
   static const char* VALIDITY = "validity";
   static const char* PERMISSIONS = "permissions";
   static const char* AUTHORIZED_LABELS = "authorized-labels";
-  static const char* FORBIDDEN_LABELS = "forbidden-labels";
   static const char* USER_NAME = "name";
 
 
@@ -377,11 +376,9 @@
           !jsonProfile.isMember(PERMISSIONS) ||
           !jsonProfile.isMember(VALIDITY) ||
           !jsonProfile.isMember(AUTHORIZED_LABELS) ||
-          !jsonProfile.isMember(FORBIDDEN_LABELS) ||
           !jsonProfile.isMember(USER_NAME) ||
           jsonProfile[PERMISSIONS].type() != Json::arrayValue ||
           jsonProfile[AUTHORIZED_LABELS].type() != Json::arrayValue ||
-          jsonProfile[FORBIDDEN_LABELS].type() != Json::arrayValue ||
           jsonProfile[VALIDITY].type() != Json::intValue ||
           jsonProfile[USER_NAME].type() != Json::stringValue)
       {
@@ -401,21 +398,10 @@
       {
         profile.authorizedLabels.insert(jsonProfile[AUTHORIZED_LABELS][i].asString());
       }
-      for (Json::ArrayIndex i = 0; i < jsonProfile[FORBIDDEN_LABELS].size(); ++i)
-      {
-        profile.forbiddenLabels.insert(jsonProfile[FORBIDDEN_LABELS][i].asString());
-      }
 
-      if (profile.authorizedLabels.size() > 0 && profile.forbiddenLabels.size() > 0)
+      if (profile.authorizedLabels.size() == 0)
       {
-        throw Orthanc::OrthancException(Orthanc::ErrorCode_NetworkProtocol,
-                                        "Syntax error in the result of the Auth Web service, the UserProfile can not contain both authorized and forbidden labels");
-      }
-
-      if (profile.authorizedLabels.size() == 0 && profile.forbiddenLabels.size() == 0)
-      {
-        LOG(WARNING) << "The UserProfile does not contain any authorized or forbidden labels, assuming the user has access to all data (equivalent to \"authorized_labels\": [\"*\"]) !";
-        profile.authorizedLabels.insert("*");
+        LOG(WARNING) << "The UserProfile does not contain any authorized labels, you should add, e.g, \"authorized_labels\": [\"*\"] to grant him access to all labels !";
       }
 
       return true;
--- a/Plugin/IAuthorizationService.h	Wed Aug 30 18:10:09 2023 +0200
+++ b/Plugin/IAuthorizationService.h	Thu Aug 31 16:51:15 2023 +0200
@@ -58,7 +58,6 @@
       std::string name;
       std::set<std::string> permissions;
       std::set<std::string> authorizedLabels;
-      std::set<std::string> forbiddenLabels;
     };
 
     virtual ~IAuthorizationService()
--- a/Plugin/Plugin.cpp	Wed Aug 30 18:10:09 2023 +0200
+++ b/Plugin/Plugin.cpp	Thu Aug 31 16:51:15 2023 +0200
@@ -388,6 +388,15 @@
   return false;
 }
 
+bool HasAccessToAllLabels(const OrthancPlugins::IAuthorizationService::UserProfile& profile)
+{
+  return (profile.authorizedLabels.find("*") != profile.authorizedLabels.end());
+}
+
+bool HasAccessToSomeLabels(const OrthancPlugins::IAuthorizationService::UserProfile& profile)
+{
+  return (profile.authorizedLabels.size() > 0);
+}
 
 void AdjustToolsFindQueryLabels(Json::Value& query, const OrthancPlugins::IAuthorizationService::UserProfile& profile)
 {
@@ -401,13 +410,17 @@
   }
   else if (query.isMember("Labels") || query.isMember("LabelsConstraint"))
   {
-    throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat, "Auth plugin: unable to transform tools/find query, both 'Labels' and 'LabelsConstraint' must be defined together if one of them is defined.");
+    throw Orthanc::OrthancException(Orthanc::ErrorCode_Unauthorized, "Auth plugin: unable to transform tools/find query, both 'Labels' and 'LabelsConstraint' must be defined together if one of them is defined.");
   }
 
-  if (profile.authorizedLabels.size() > 0 || profile.forbiddenLabels.size() > 0)
+  if (!HasAccessToSomeLabels(profile))
+  {
+    throw Orthanc::OrthancException(Orthanc::ErrorCode_Unauthorized, "Auth plugin: unable to call tools/find when the user does not have access to any labels.");
+  }
+  else if (profile.authorizedLabels.size() > 0)
   {
     // if the user has access to all labels: no need to transform the tools/find body, we keep it as is
-    if (profile.authorizedLabels.find("*") == profile.authorizedLabels.end())
+    if (!HasAccessToAllLabels(profile))
     { // the user does not have access to all labels -> transform the tools/find body
 
       if (labelsToFind.size() == 0)
@@ -417,14 +430,6 @@
           Orthanc::SerializationToolbox::WriteSetOfStrings(query, profile.authorizedLabels, "Labels");
           query["LabelsConstraint"] = "Any";
         }
-        else if (profile.forbiddenLabels.size() > 0)
-        {
-          if (labelsToFind.size() == 0)
-          { // in this case, we can add a None constraint
-            Orthanc::SerializationToolbox::WriteSetOfStrings(query, profile.forbiddenLabels, "Labels");
-            query["LabelsConstraint"] = "None";
-          }
-        }
       }
       else if (labelsConstraint == "All")
       {
@@ -432,57 +437,39 @@
         {
           if (!Orthanc::Toolbox::IsSetInSet(labelsToFind, profile.authorizedLabels))
           {
-            throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat, "Auth plugin: unable to transform tools/find query with 'All' labels constraint when the user does not have access to all listed labels.");
+            throw Orthanc::OrthancException(Orthanc::ErrorCode_Unauthorized, "Auth plugin: unable to transform tools/find query with 'All' labels constraint when the user does not have access to all listed labels.");
           }
         }
-        else if (profile.forbiddenLabels.size() > 0)
-        {
-          throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat, "Auth plugin: unable to transform tools/find query with 'All' labels constraint when the user has forbidden labels.");                
-        }
       }
       else if (labelsConstraint == "Any")
       {
         if (profile.authorizedLabels.size() > 0)
         {
           std::set<std::string> newLabelsToFind;
-          for (std::set<std::string>::const_iterator itLabel = labelsToFind.begin(); itLabel != labelsToFind.end(); ++itLabel)
-          {
-            if (profile.authorizedLabels.find(*itLabel) != profile.authorizedLabels.end())
-            {
-              newLabelsToFind.insert(*itLabel);
-            }
-          }
+          Orthanc::Toolbox::GetIntersection(newLabelsToFind, labelsToFind, profile.authorizedLabels);
 
           if (newLabelsToFind.size() == 0)
           {
-            throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat, "Auth plugin: unable to transform tools/find query with 'All' labels constraint when none of the labels to find is authorized for the user.");                
+            throw Orthanc::OrthancException(Orthanc::ErrorCode_Unauthorized, "Auth plugin: unable to transform tools/find query with 'All' labels constraint when none of the labels to find is authorized for the user.");                
           }
 
           query.removeMember("Labels");
           Orthanc::SerializationToolbox::WriteSetOfStrings(query, newLabelsToFind, "Labels");
         }
-        else if (profile.forbiddenLabels.size() > 0)
-        {
-          throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat, "Auth plugin: unable to transform tools/find query with 'Any' labels constraint when the user has forbidden labels.");                
-        }
       }
       else if (labelsConstraint == "None")
       {
         if (profile.authorizedLabels.size() > 0)
         {
-          throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat, "Auth plugin: unable to transform tools/find query with 'None' labels constraint when the user only has authorized_labels.");        
-        }
-        else if (profile.forbiddenLabels.size() > 0)
-        {
-          std::set<std::string> newLabelsToFind = labelsToFind;
-          Orthanc::Toolbox::AppendSets(newLabelsToFind, profile.forbiddenLabels);
-
-          query.removeMember("Labels");
-          Orthanc::SerializationToolbox::WriteSetOfStrings(query, newLabelsToFind, "Labels");
+          throw Orthanc::OrthancException(Orthanc::ErrorCode_Unauthorized, "Auth plugin: unable to transform tools/find query with 'None' labels constraint when the user only has authorized_labels.");        
         }
       }
     }
   }
+  else
+  {
+    // TODO what shall we do if the user has no authorized_labels ???
+  }
 }
 
 void ToolsFind(OrthancPluginRestOutput* output,
@@ -522,6 +509,55 @@
     {
       OrthancPluginSendHttpStatusCode(context, output, 403); // TODO: check
     }
+  }
+}
+
+void ToolsLabels(OrthancPluginRestOutput* output,
+                 const char* /*url*/,
+                 const OrthancPluginHttpRequest* request)
+{
+  OrthancPluginContext* context = OrthancPlugins::GetGlobalContext();
+
+  if (request->method != OrthancPluginHttpMethod_Get)
+  {
+    OrthancPluginSendMethodNotAllowed(context, output, "GET");
+  }
+  else
+  {
+    // The filtering to this route is performed by this plugin as it is done for any other route before we get here.
+
+    // If the logged in user has restrictions on the labels he can access, modify the tools/labels response before answering
+    OrthancPlugins::IAuthorizationService::UserProfile profile;
+    if (GetUserProfileInternal(profile, request))
+    {
+      if (!HasAccessToSomeLabels(profile))
+      {
+        Json::Value emptyLabels;
+        OrthancPlugins::AnswerJson(emptyLabels, output);
+        return;
+      }
+
+      Json::Value jsonLabels;
+      if (OrthancPlugins::RestApiGet(jsonLabels, "/tools/labels", false))
+      {
+        std::set<std::string> allLabels;
+        Orthanc::SerializationToolbox::ReadSetOfStrings(allLabels, jsonLabels);
+
+        if (!HasAccessToAllLabels(profile))
+        {
+          std::set<std::string> authorizedLabels;
+
+          Orthanc::Toolbox::GetIntersection(authorizedLabels, allLabels, profile.authorizedLabels);
+          Orthanc::SerializationToolbox::WriteSetOfStrings(jsonLabels, authorizedLabels);
+        }
+        OrthancPlugins::AnswerJson(jsonLabels, output);
+      }
+
+    }
+    else
+    {
+      OrthancPluginSendHttpStatusCode(context, output, 403); // TODO: check
+    }
 
 
   }
@@ -710,10 +746,6 @@
       {
         jsonProfile["authorized-labels"].append(*it);
       }
-      for (std::set<std::string>::const_iterator it = profile.forbiddenLabels.begin(); it != profile.forbiddenLabels.end(); ++it)
-      {
-        jsonProfile["forbidden-labels"].append(*it);
-      }
 
       OrthancPlugins::AnswerJson(jsonProfile, output);
     }
@@ -1064,6 +1096,7 @@
         }
 
         OrthancPlugins::RegisterRestCallback<ToolsFind>("/tools/find", true);
+        OrthancPlugins::RegisterRestCallback<ToolsLabels>("/tools/labels", true);
 
 
         if (authorizationParser_.get() != NULL || permissionParser_.get() != NULL)
--- a/UnitTestsSources/UnitTestsMain.cpp	Wed Aug 30 18:10:09 2023 +0200
+++ b/UnitTestsSources/UnitTestsMain.cpp	Thu Aug 31 16:51:15 2023 +0200
@@ -314,13 +314,30 @@
   return false;
 }
 
+TEST(ToolsFindLabels, AdjustQueryForUserWithoutAuthorizedLabels)
+{
+  // user who has access no authorized labels
+  OrthancPlugins::IAuthorizationService::UserProfile profile;
+
+  { // any call to tools/find for such a user should fail since it does not have access to anything
+    Json::Value query;
+    query["Query"] = Json::objectValue;
+    query["Query"]["PatientID"] = "*";
+
+    ASSERT_THROW(AdjustToolsFindQueryLabels(query, profile), Orthanc::OrthancException);
+  }
+
+}
+
+
+
 TEST(ToolsFindLabels, AdjustQueryForUserWithoutRestrictions)
 {
   // user who has access to all labels
   OrthancPlugins::IAuthorizationService::UserProfile profile;
   profile.authorizedLabels.insert("*");
 
-  { // no labels before transformation -> no labels after 
+  { // no labels filtering before transformation -> no labels filtering after 
     Json::Value query;
     query["Query"] = Json::objectValue;
     query["Query"]["PatientID"] = "*";
@@ -542,91 +559,103 @@
   }
 }
 
-TEST(ToolsFindLabels, AdjustQueryForUserWithForbiddenLabelsRestrictions)
-{
-  // user who has forbidden access to "b" and "c"
-  OrthancPlugins::IAuthorizationService::UserProfile profile;
-  profile.forbiddenLabels.insert("b");
-  profile.forbiddenLabels.insert("c");
+// TEST(ToolsFindLabels, AdjustQueryForUserWithForbiddenLabelsRestrictions)
+// {
+//   // user who has forbidden access to "b" and "c"
+//   OrthancPlugins::IAuthorizationService::UserProfile profile;
+//   profile.forbiddenLabels.insert("b");
+//   profile.forbiddenLabels.insert("c");
 
-  { // no labels before transformation -> "b", "c" label after (with a 'None' constraint)
-    Json::Value query;
-    query["Query"] = Json::objectValue;
-    query["Query"]["PatientID"] = "*";
+//   { // no labels before transformation -> "b", "c" label after (with a 'None' constraint)
+//     Json::Value query;
+//     query["Query"] = Json::objectValue;
+//     query["Query"]["PatientID"] = "*";
 
-    AdjustToolsFindQueryLabels(query, profile);
+//     AdjustToolsFindQueryLabels(query, profile);
 
-    ASSERT_EQ(2u, query["Labels"].size());
-    ASSERT_TRUE(IsInJsonArray("b", query["Labels"]));
-    ASSERT_TRUE(IsInJsonArray("c", query["Labels"]));
-    ASSERT_EQ("None", query["LabelsConstraint"].asString());
-  }
+//     ASSERT_EQ(2u, query["Labels"].size());
+//     ASSERT_TRUE(IsInJsonArray("b", query["Labels"]));
+//     ASSERT_TRUE(IsInJsonArray("c", query["Labels"]));
+//     ASSERT_EQ("None", query["LabelsConstraint"].asString());
+//   }
 
-  { // missing LabelsConstraint -> throw
-    Json::Value query;
-    query["Query"] = Json::objectValue;
-    query["Query"]["PatientID"] = "*";
-    query["Labels"] = Json::arrayValue;
-    query["Labels"].append("a");
+//   { // missing LabelsConstraint -> throw
+//     Json::Value query;
+//     query["Query"] = Json::objectValue;
+//     query["Query"]["PatientID"] = "*";
+//     query["Labels"] = Json::arrayValue;
+//     query["Labels"].append("a");
 
-    ASSERT_THROW(AdjustToolsFindQueryLabels(query, profile), Orthanc::OrthancException);
-  }
+//     ASSERT_THROW(AdjustToolsFindQueryLabels(query, profile), Orthanc::OrthancException);
+//   }
 
-  { // 'All' label constraint can not be modified for user with forbidden labels
-    Json::Value query;
-    query["Query"] = Json::objectValue;
-    query["Query"]["PatientID"] = "*";
-    query["Labels"] = Json::arrayValue;
-    query["Labels"].append("b");
-    query["Labels"].append("c");
-    query["LabelsConstraint"] = "All";
+//   { // 'All' label constraint can not be modified for user with forbidden labels
+//     Json::Value query;
+//     query["Query"] = Json::objectValue;
+//     query["Query"]["PatientID"] = "*";
+//     query["Labels"] = Json::arrayValue;
+//     query["Labels"].append("b");
+//     query["Labels"].append("c");
+//     query["LabelsConstraint"] = "All";
 
-    ASSERT_THROW(AdjustToolsFindQueryLabels(query, profile), Orthanc::OrthancException);
-  }
+//     ASSERT_THROW(AdjustToolsFindQueryLabels(query, profile), Orthanc::OrthancException);
+//   }
 
-  { // 'Any' label constraint can not be modified for user with forbidden labels
-    Json::Value query;
-    query["Query"] = Json::objectValue;
-    query["Query"]["PatientID"] = "*";
-    query["Labels"] = Json::arrayValue;
-    query["Labels"].append("b");
-    query["Labels"].append("c");
-    query["LabelsConstraint"] = "Any";
+//   { // 'Any' label constraint can not be modified for user with forbidden labels
+//     Json::Value query;
+//     query["Query"] = Json::objectValue;
+//     query["Query"]["PatientID"] = "*";
+//     query["Labels"] = Json::arrayValue;
+//     query["Labels"].append("b");
+//     query["Labels"].append("c");
+//     query["LabelsConstraint"] = "Any";
+
+//     ASSERT_THROW(AdjustToolsFindQueryLabels(query, profile), Orthanc::OrthancException);
+//   }
 
-    ASSERT_THROW(AdjustToolsFindQueryLabels(query, profile), Orthanc::OrthancException);
-  }
+//   { // 'Any' label constraint can not be modified for user with forbidden labels
+//     Json::Value query;
+//     query["Query"] = Json::objectValue;
+//     query["Query"]["PatientID"] = "*";
+//     query["Labels"] = Json::arrayValue;
+//     query["Labels"].append("a");
+//     query["LabelsConstraint"] = "Any";
 
-  { // 'None' label constraint are modified to always contain at least all forbidden_labels of the user
-    Json::Value query;
-    query["Query"] = Json::objectValue;
-    query["Query"]["PatientID"] = "*";
-    query["Labels"] = Json::arrayValue;
-    query["Labels"].append("b");
-    query["LabelsConstraint"] = "None";
+//     ASSERT_THROW(AdjustToolsFindQueryLabels(query, profile), Orthanc::OrthancException);
+//   }
+
 
-    AdjustToolsFindQueryLabels(query, profile);
-    ASSERT_EQ(2u, query["Labels"].size());
-    ASSERT_TRUE(IsInJsonArray("b", query["Labels"]));
-    ASSERT_TRUE(IsInJsonArray("c", query["Labels"]));
-    ASSERT_EQ("None", query["LabelsConstraint"].asString());
-  }
+//   { // 'None' label constraint are modified to always contain at least all forbidden_labels of the user
+//     Json::Value query;
+//     query["Query"] = Json::objectValue;
+//     query["Query"]["PatientID"] = "*";
+//     query["Labels"] = Json::arrayValue;
+//     query["Labels"].append("b");
+//     query["LabelsConstraint"] = "None";
+
+//     AdjustToolsFindQueryLabels(query, profile);
+//     ASSERT_EQ(2u, query["Labels"].size());
+//     ASSERT_TRUE(IsInJsonArray("b", query["Labels"]));
+//     ASSERT_TRUE(IsInJsonArray("c", query["Labels"]));
+//     ASSERT_EQ("None", query["LabelsConstraint"].asString());
+//   }
 
-  { // 'None' label constraint are modified to always contain at least all forbidden_labels of the user
-    Json::Value query;
-    query["Query"] = Json::objectValue;
-    query["Query"]["PatientID"] = "*";
-    query["Labels"] = Json::arrayValue;
-    query["Labels"].append("d");
-    query["LabelsConstraint"] = "None";
+//   { // 'None' label constraint are modified to always contain at least all forbidden_labels of the user
+//     Json::Value query;
+//     query["Query"] = Json::objectValue;
+//     query["Query"]["PatientID"] = "*";
+//     query["Labels"] = Json::arrayValue;
+//     query["Labels"].append("d");
+//     query["LabelsConstraint"] = "None";
 
-    AdjustToolsFindQueryLabels(query, profile);
-    ASSERT_EQ(3u, query["Labels"].size());
-    ASSERT_TRUE(IsInJsonArray("b", query["Labels"]));
-    ASSERT_TRUE(IsInJsonArray("c", query["Labels"]));
-    ASSERT_TRUE(IsInJsonArray("d", query["Labels"]));
-    ASSERT_EQ("None", query["LabelsConstraint"].asString());
-  }
-}
+//     AdjustToolsFindQueryLabels(query, profile);
+//     ASSERT_EQ(3u, query["Labels"].size());
+//     ASSERT_TRUE(IsInJsonArray("b", query["Labels"]));
+//     ASSERT_TRUE(IsInJsonArray("c", query["Labels"]));
+//     ASSERT_TRUE(IsInJsonArray("d", query["Labels"]));
+//     ASSERT_EQ("None", query["LabelsConstraint"].asString());
+//   }
+// }
 
 }