From 152f2cecc1f8936f73eecb79cab61d4447c1bf46 Mon Sep 17 00:00:00 2001
From: Nick Muerdter <12112+GUI@users.noreply.github.com>
Date: Wed, 24 Jan 2024 19:23:52 -0700
Subject: [PATCH] Add validations to ensure dependent permissions make sense.

---
 .../web-app/models/admin_group.lua            | 16 +++++
 test/apis/v1/admin_groups/test_create.rb      | 66 +++++++++++++++++++
 2 files changed, 82 insertions(+)
 create mode 100644 test/apis/v1/admin_groups/test_create.rb

diff --git a/src/api-umbrella/web-app/models/admin_group.lua b/src/api-umbrella/web-app/models/admin_group.lua
index 7e0b18e80..c52c4a713 100644
--- a/src/api-umbrella/web-app/models/admin_group.lua
+++ b/src/api-umbrella/web-app/models/admin_group.lua
@@ -1,6 +1,8 @@
 local admin_group_policy = require "api-umbrella.web-app.policies.admin_group_policy"
 local cjson = require "cjson"
 local db = require "lapis.db"
+local invert_table = require "api-umbrella.utils.invert_table"
+local is_array = require "api-umbrella.utils.is_array"
 local is_empty = require "api-umbrella.utils.is_empty"
 local json_array_fields = require "api-umbrella.web-app.utils.json_array_fields"
 local json_null_default = require "api-umbrella.web-app.utils.json_null_default"
@@ -9,6 +11,7 @@ local t = require("api-umbrella.web-app.utils.gettext").gettext
 local time = require "api-umbrella.utils.time"
 local validation_ext = require "api-umbrella.web-app.utils.validation_ext"
 
+local db_null = db.NULL
 local json_null = cjson.null
 local validate_field = model_ext.validate_field
 local validate_uniqueness = model_ext.validate_uniqueness
@@ -214,6 +217,19 @@ AdminGroup = model_ext.new_class("admin_groups", {
       { validation_ext.non_null_table:minlen(1), t("can't be blank") },
     }, { error_field = "permissions" })
     validate_uniqueness(errors, data, "name", t("Name"), AdminGroup, { "name" })
+
+    if data["permission_ids"] ~= db_null and is_array(data["permission_ids"]) then
+      local permissions = invert_table(data["permission_ids"])
+
+      if permissions["user_manage"] and not permissions["user_view"] then
+        model_ext.add_error(errors, "permission_ids", t("Permissions"), t("user_view permission must be included if user_manage is enabled"))
+      end
+
+      if permissions["admin_manage"] and not permissions["admin_view"] then
+        model_ext.add_error(errors, "permission_ids", t("Permissions"), t("admin_view permission must be included if admin_manage is enabled"))
+      end
+    end
+
     return errors
   end,
 
diff --git a/test/apis/v1/admin_groups/test_create.rb b/test/apis/v1/admin_groups/test_create.rb
new file mode 100644
index 000000000..e5a46420c
--- /dev/null
+++ b/test/apis/v1/admin_groups/test_create.rb
@@ -0,0 +1,66 @@
+require_relative "../../../test_helper"
+
+class Test::Apis::V1::AdminGroups::TestCreate < Minitest::Test
+  include ApiUmbrellaTestHelpers::AdminAuth
+  include ApiUmbrellaTestHelpers::Setup
+  parallelize_me!
+
+  def setup
+    super
+    setup_server
+  end
+
+  def test_validates_user_manage_permision
+    attributes = FactoryBot.build(:admin_group, {
+      :permission_ids => ["user_manage"],
+    }).serializable_hash
+    response = Typhoeus.post("https://127.0.0.1:9081/api-umbrella/v1/admin_groups.json", http_options.deep_merge(admin_token).deep_merge({
+      :headers => { "Content-Type" => "application/json" },
+      :body => MultiJson.dump(:admin_group => attributes),
+    }))
+    assert_response_code(422, response)
+
+    data = MultiJson.load(response.body)
+    assert_equal({
+      "permission_ids" => [
+        "user_view permission must be included if user_manage is enabled",
+      ],
+    }, data.fetch("errors"))
+
+    attributes = FactoryBot.build(:admin_group, {
+      :permission_ids => ["user_manage", "user_view"],
+    }).serializable_hash
+    response = Typhoeus.post("https://127.0.0.1:9081/api-umbrella/v1/admin_groups.json", http_options.deep_merge(admin_token).deep_merge({
+      :headers => { "Content-Type" => "application/json" },
+      :body => MultiJson.dump(:admin_group => attributes),
+    }))
+    assert_response_code(201, response)
+  end
+
+  def test_validates_admin_manage_permision
+    attributes = FactoryBot.build(:admin_group, {
+      :permission_ids => ["admin_manage"],
+    }).serializable_hash
+    response = Typhoeus.post("https://127.0.0.1:9081/api-umbrella/v1/admin_groups.json", http_options.deep_merge(admin_token).deep_merge({
+      :headers => { "Content-Type" => "application/json" },
+      :body => MultiJson.dump(:admin_group => attributes),
+    }))
+    assert_response_code(422, response)
+
+    data = MultiJson.load(response.body)
+    assert_equal({
+      "permission_ids" => [
+        "admin_view permission must be included if admin_manage is enabled",
+      ],
+    }, data.fetch("errors"))
+
+    attributes = FactoryBot.build(:admin_group, {
+      :permission_ids => ["admin_manage", "admin_view"],
+    }).serializable_hash
+    response = Typhoeus.post("https://127.0.0.1:9081/api-umbrella/v1/admin_groups.json", http_options.deep_merge(admin_token).deep_merge({
+      :headers => { "Content-Type" => "application/json" },
+      :body => MultiJson.dump(:admin_group => attributes),
+    }))
+    assert_response_code(201, response)
+  end
+end