# HG changeset patch # User Alain Mazy # Date 1693493475 -7200 # Node ID 572955904411c4c68b5338ecb75840282cd2a601 # Parent 2b1a95c7d26347a04606fcf72db9b32087d65d97 added tools/labels + removed forbidden_labels diff -r 2b1a95c7d263 -r 572955904411 Plugin/AuthorizationWebService.cpp --- 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; diff -r 2b1a95c7d263 -r 572955904411 Plugin/IAuthorizationService.h --- 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 permissions; std::set authorizedLabels; - std::set forbiddenLabels; }; virtual ~IAuthorizationService() diff -r 2b1a95c7d263 -r 572955904411 Plugin/Plugin.cpp --- 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 newLabelsToFind; - for (std::set::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 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 allLabels; + Orthanc::SerializationToolbox::ReadSetOfStrings(allLabels, jsonLabels); + + if (!HasAccessToAllLabels(profile)) + { + std::set 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::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("/tools/find", true); + OrthancPlugins::RegisterRestCallback("/tools/labels", true); if (authorizationParser_.get() != NULL || permissionParser_.get() != NULL) diff -r 2b1a95c7d263 -r 572955904411 UnitTestsSources/UnitTestsMain.cpp --- 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()); +// } +// } }