Refactor Permission structure in Decision API (#887)

Change Permission from pair to a struct and
add missing resource field.

Resolves: OLPSUP-10514

Signed-off-by: Serhii Lozynskyi <ext-serhii.lozynskyi@here.com>

Author: meravingen123

olp-cpp-sdk-authentication/include/olp/authentication/AuthorizeResult.h

@@ -32,6 +32,60 @@ namespace authentication {
  */
 enum class DecisionType { kAllow, kDeny };
 
+/**
+ * @brief Represents the permission with the action, policy decision, and
+ * associated resource.
+ */
+class AUTHENTICATION_API Permission {
+ public:
+  /**
+   * @brief Sets the action associated with the resource.
+   *
+   * @param The action to associate with.
+   */
+  void SetAction(std::string action) { action_ = std::move(action); }
+
+  /**
+   * @brief Gets the action that is associated with the resource.
+   *
+   * @return A string that represents the action.
+   */
+  const std::string& GetAction() const { return action_; }
+
+  /**
+   * @brief Sets the resource with which the action and decision are associated.
+   *
+   * @param The resource to associate with the decision and action.
+   */
+  void SetResource(std::string resource) { resource_ = std::move(resource); }
+
+  /**
+   * @brief Gets the resource with which the action and decision are associated.
+   *
+   * @return The resource name.
+   */
+  const std::string& GetResource() const { return resource_; }
+
+  /**
+   * @brief Sets the decision associated with the resource.
+   *
+   * @param The decision to associate with the resource.
+   */
+  void SetDecision(DecisionType decision) { decision_ = decision; }
+
+  /**
+   * @brief Gets the decision associated with the resource.
+   *
+   * @return The decision for the associated resource.
+   */
+  DecisionType GetDecision() const { return decision_; }
+
+ private:
+  std::string action_;
+  std::string resource_;
+  DecisionType decision_;
+};
+
 /**
  * @brief Represents each action-resource pair response with an individual
  * policy decision for that action: DENY or ALLOW.
@@ -41,11 +95,6 @@ enum class DecisionType { kAllow, kDeny };
  */
 class AUTHENTICATION_API ActionResult {
  public:
-  /**
-   * @brief Represents the permission pair with the action and policy decision.
-   */
-  using Permissions = std::pair<std::string, DecisionType>;
-
   /**
    * @brief Gets the overall policy decision.
    *
@@ -77,20 +126,20 @@ class AUTHENTICATION_API ActionResult {
    *
    * @return The list of permissions.
    */
-  const std::vector<Permissions>& GetPermissions() const { return permissions_; }
+  const std::vector<Permission>& GetPermissions() const { return permissions_; }
 
   /**
    * @brief Sets the list of permissions.
    *
    * @param permissions The vector of the action-decision pair.
    */
-  void SetPermissions(std::vector<Permissions> permissions) {
+  void SetPermissions(std::vector<Permission> permissions) {
     permissions_ = std::move(permissions);
   }
 
  private:
   DecisionType decision_{DecisionType::kDeny};
-  std::vector<Permissions> permissions_;
+  std::vector<Permission> permissions_;
 };
 
 /**

olp-cpp-sdk-authentication/src/AuthenticationClientUtils.cpp

@@ -193,7 +193,7 @@ IntrospectAppResult GetIntrospectAppResult(const rapidjson::Document& doc) {
   return result;
 }
 
-DecisionType GetPermission(const std::string& str) {
+DecisionType GetDecision(const std::string& str) {
   return (str.compare("allow") == 0) ? DecisionType::kAllow
                                      : DecisionType::kDeny;
 }
@@ -205,22 +205,26 @@ std::vector<ActionResult> GetDiagnostics(rapidjson::Document& doc) {
     ActionResult action;
     if (element.HasMember(Constants::DECISION)) {
       action.SetDecision(
-          GetPermission(element[Constants::DECISION].GetString()));
+          GetDecision(element[Constants::DECISION].GetString()));
       // get permissions if avialible
       if (element.HasMember(Constants::PERMISSIONS) &&
           element[Constants::PERMISSIONS].IsArray()) {
-        std::vector<ActionResult::Permissions> permissions;
+        std::vector<Permission> permissions;
         const auto& permissions_array =
             element[Constants::PERMISSIONS].GetArray();
         for (auto& permission_element : permissions_array) {
-          ActionResult::Permissions permission;
+          Permission permission;
           if (permission_element.HasMember(Constants::ACTION)) {
-            permission.first =
-                permission_element[Constants::ACTION].GetString();
+            permission.SetAction(
+                permission_element[Constants::ACTION].GetString());
           }
           if (permission_element.HasMember(Constants::DECISION)) {
-            permission.second = GetPermission(
-                permission_element[Constants::DECISION].GetString());
+            permission.SetDecision(GetDecision(
+                permission_element[Constants::DECISION].GetString()));
+          }
+          if (permission_element.HasMember(Constants::RESOURCE)) {
+            permission.SetResource(
+                permission_element[Constants::RESOURCE].GetString());
           }
           permissions.push_back(std::move(permission));
         }
@@ -247,7 +251,7 @@ AuthorizeResult GetAuthorizeResult(rapidjson::Document& doc) {
   }
 
   if (doc.HasMember(Constants::DECISION)) {
-    result.SetDecision(GetPermission(doc[Constants::DECISION].GetString()));
+    result.SetDecision(GetDecision(doc[Constants::DECISION].GetString()));
   }
 
   // get diagnostics if available

olp-cpp-sdk-authentication/src/AuthenticationClientUtils.h

@@ -95,7 +95,7 @@ IntrospectAppResult GetIntrospectAppResult(const rapidjson::Document& doc);
  * @param str string representation of decision.
  * @return result DecisionType.
  */
-DecisionType GetPermission(const std::string& str);
+DecisionType GetDecision(const std::string& str);
 
 /*
  * @brief Parse json document to vector of ActionResults.

olp-cpp-sdk-authentication/src/Constants.cpp

@@ -58,6 +58,7 @@ const char* Constants::IDENTITY = "identity";
 const char* Constants::USER_ID = "userId";
 const char* Constants::DECISION = "decision";
 const char* Constants::ACTION = "action";
+const char* Constants::RESOURCE = "resource";
 const char* Constants::PERMISSIONS = "permissions";
 const char* Constants::DIAGNOSTICS = "diagnostics";
 

olp-cpp-sdk-authentication/src/Constants.h

@@ -64,6 +64,7 @@ class Constants {
   static const char* USER_ID;
   static const char* DECISION;
   static const char* ACTION;
+  static const char* RESOURCE;
   static const char* PERMISSIONS;
   static const char* DIAGNOSTICS;
 };

olp-cpp-sdk-authentication/tests/DecisionApiClientTest.cpp

@@ -22,18 +22,21 @@
 #include <olp/authentication/AuthorizeResult.h>
 
 namespace {
-using namespace olp::authentication;
+namespace auth = olp::authentication;
 
 TEST(DecisionApiClientTest, AuthorizeRequestTest) {
-  EXPECT_EQ(AuthorizeRequest().WithServiceId("ServiceId").GetServiceId(),
+  EXPECT_EQ(auth::AuthorizeRequest().WithServiceId("ServiceId").GetServiceId(),
             "ServiceId");
-  EXPECT_EQ(
-      AuthorizeRequest().WithContractId("ContractId").GetContractId().get(),
-      "ContractId");
-  auto request = AuthorizeRequest().WithAction("action1").WithAction(
+  EXPECT_EQ(auth::AuthorizeRequest()
+                .WithContractId("ContractId")
+                .GetContractId()
+                .get(),
+            "ContractId");
+  auto request = auth::AuthorizeRequest().WithAction("action1").WithAction(
       "action2", std::string("hrn::test"));
-  EXPECT_EQ(AuthorizeRequest().GetDiagnostics(), false);
-  EXPECT_EQ(AuthorizeRequest().WithDiagnostics(true).GetDiagnostics(), true);
+  EXPECT_EQ(auth::AuthorizeRequest().GetDiagnostics(), false);
+  EXPECT_EQ(auth::AuthorizeRequest().WithDiagnostics(true).GetDiagnostics(),
+            true);
   EXPECT_EQ(request.GetActions().size(), 2);
   auto actions_it = request.GetActions().begin();
   EXPECT_EQ(actions_it->first, "action1");
@@ -42,29 +45,44 @@ TEST(DecisionApiClientTest, AuthorizeRequestTest) {
   EXPECT_EQ(actions_it->first, "action2");
   EXPECT_EQ(actions_it->second, "hrn::test");
   EXPECT_EQ(request.GetOperatorType(),
-            AuthorizeRequest::DecisionOperatorType::kAnd);
-  request.WithOperatorType(AuthorizeRequest::DecisionOperatorType::kOr);
+            auth::AuthorizeRequest::DecisionOperatorType::kAnd);
+  request.WithOperatorType(auth::AuthorizeRequest::DecisionOperatorType::kOr);
   EXPECT_EQ(request.GetOperatorType(),
-            AuthorizeRequest::DecisionOperatorType::kOr);
+            auth::AuthorizeRequest::DecisionOperatorType::kOr);
   request.WithServiceId("service");
   EXPECT_EQ(request.CreateKey(), "service");
   request.WithContractId("contract");
   EXPECT_EQ(request.CreateKey(), "service[contract]");
 }
 
 TEST(DecisionApiClientTest, AuthorizeResponceTest) {
-  EXPECT_EQ(AuthorizeResult().GetDecision(), DecisionType::kDeny);
-  EXPECT_EQ(ActionResult().GetDecision(), DecisionType::kDeny);
-  EXPECT_EQ(AuthorizeResult().GetClientId(), "");
-  ActionResult action;
-  action.SetDecision(DecisionType::kAllow);
-  action.SetPermissions({{"read", DecisionType::kAllow}});
-  AuthorizeResult decision;
+  EXPECT_EQ(auth::AuthorizeResult().GetDecision(), auth::DecisionType::kDeny);
+  EXPECT_EQ(auth::ActionResult().GetDecision(), auth::DecisionType::kDeny);
+  EXPECT_EQ(auth::AuthorizeResult().GetClientId(), "");
+  auth::ActionResult action;
+  action.SetDecision(auth::DecisionType::kAllow);
+  auth::Permission permission;
+  permission.SetAction("read");
+  permission.SetResource("hrn:test");
+  permission.SetDecision(auth::DecisionType::kAllow);
+  action.SetPermissions({permission});
+  auth::AuthorizeResult decision;
   decision.SetActionResults({action});
-  EXPECT_EQ(decision.GetActionResults().front().GetPermissions().front().first,
-            "read");
-  EXPECT_EQ(decision.GetActionResults().front().GetPermissions().front().second,
-            DecisionType::kAllow);
+  EXPECT_EQ(
+      decision.GetActionResults().front().GetPermissions().front().GetAction(),
+      "read");
+  EXPECT_EQ(decision.GetActionResults()
+                .front()
+                .GetPermissions()
+                .front()
+                .GetDecision(),
+            auth::DecisionType::kAllow);
+  EXPECT_EQ(decision.GetActionResults()
+                .front()
+                .GetPermissions()
+                .front()
+                .GetResource(),
+            "hrn:test");
 }
 
 }  // namespace