Add test cases for evaluation order

In fact, this test uncovered that we were evaluating the rules in the
wrong order. Fix this.

Signed-off-by: David Zeuthen <davidz@redhat.com>

7 files changed, 201 insertions, 11 deletions

diff --git a/src/polkitbackend/init.js b/src/polkitbackend/init.js
index cf6a65d..0e8bcfb 100644
--- a/src/polkitbackend/init.js
+++ b/src/polkitbackend/init.js
@@ -45,7 +45,7 @@ polkit._adminRuleFuncs = [];
 polkit.addAdminRule = function(callback) {this._adminRuleFuncs.push(callback);};
 polkit._runAdminRules = function(action, subject, details) {
     var ret = null;
-    for (var n = this._adminRuleFuncs.length - 1; n >= 0; n--) {
+    for (var n = 0; n < this._adminRuleFuncs.length; n++) {
         var func = this._adminRuleFuncs[n];
         var func_ret = func(action, subject, details);
         if (func_ret) {
@@ -60,7 +60,7 @@ polkit._ruleFuncs = [];
 polkit.addRule = function(callback) {this._ruleFuncs.push(callback);};
 polkit._runRules = function(action, subject, details) {
     var ret = null;
-    for (var n = this._ruleFuncs.length - 1; n >= 0; n--) {
+    for (var n = 0; n < this._ruleFuncs.length; n++) {
         var func = this._ruleFuncs[n];
         var func_ret = func(action, subject, details);
         if (func_ret) {
diff --git a/test/data/etc/polkit-1/rules.d/10-testing.rules b/test/data/etc/polkit-1/rules.d/10-testing.rules
index 1c3a562..4fce8c5 100644
--- a/test/data/etc/polkit-1/rules.d/10-testing.rules
+++ b/test/data/etc/polkit-1/rules.d/10-testing.rules
@@ -5,10 +5,6 @@
 /* NOTE: this is the /etc/polkit-1/rules.d version of 10-testing.rules */
 
 polkit.addAdminRule(function(action, subject, details) {
-    return ["unix-group:admin", "unix-user:root"];
-});
-
-polkit.addAdminRule(function(action, subject, details) {
     if (action == "net.company.action1") {
         return ["unix-group:admin"];
     }
@@ -20,14 +16,28 @@ polkit.addAdminRule(function(action, subject, details) {
     }
 });
 
+// Fallback
+polkit.addAdminRule(function(action, subject, details) {
+    return ["unix-group:admin", "unix-user:root"];
+});
+
 // -----
 
 polkit.addRule(function(action, subject, details) {
-    return "auth_admin";
+    if (action == "net.company.productA.action0") {
+        return "auth_admin";
+    }
 });
 
 polkit.addRule(function(action, subject, details) {
-    if (action == "org.freedesktop.policykit.exec") {
-        return "auth_admin";
+    if (action == "net.company.productA.action1") {
+        return "auth_self";
+    }
+});
+
+polkit.addRule(function(action, subject, details) {
+    if (action == "net.company.order0") {
+        details["test_detail"] = "a";
+        return "yes";
     }
 });
diff --git a/test/data/etc/polkit-1/rules.d/15-testing.rules b/test/data/etc/polkit-1/rules.d/15-testing.rules
new file mode 100644
index 0000000..9968aa7
--- /dev/null
+++ b/test/data/etc/polkit-1/rules.d/15-testing.rules
@@ -0,0 +1,24 @@
+/* -*- mode: js; js-indent-level: 4; indent-tabs-mode: nil -*- */
+
+/* see test/polkitbackend/test-polkitbackendjsauthority.c */
+
+polkit.addRule(function(action, subject, details) {
+    if (action == "net.company.order0") {
+        details["test_detail"] = "c";
+        return "yes";
+    }
+});
+
+polkit.addRule(function(action, subject, details) {
+    if (action == "net.company.order1") {
+        details["test_detail"] = "c";
+        return "yes";
+    }
+});
+
+polkit.addRule(function(action, subject, details) {
+    if (action == "net.company.order2") {
+        details["test_detail"] = "c";
+        return "yes";
+    }
+});
diff --git a/test/data/usr/share/polkit-1/rules.d/10-testing.rules b/test/data/usr/share/polkit-1/rules.d/10-testing.rules
index ab2fd97..48c4957 100644
--- a/test/data/usr/share/polkit-1/rules.d/10-testing.rules
+++ b/test/data/usr/share/polkit-1/rules.d/10-testing.rules
@@ -3,3 +3,17 @@
 /* see test/polkitbackend/test-polkitbackendjsauthority.c */
 
 /* NOTE: this is the /usr/share/polkit-1/rules.d version of 10-testing.rules */
+
+polkit.addRule(function(action, subject, details) {
+    if (action == "net.company.order0") {
+        details["test_detail"] = "c";
+        return "yes";
+    }
+});
+
+polkit.addRule(function(action, subject, details) {
+    if (action == "net.company.order1") {
+        details["test_detail"] = "b";
+        return "yes";
+    }
+});
diff --git a/test/data/usr/share/polkit-1/rules.d/20-testing.rules b/test/data/usr/share/polkit-1/rules.d/20-testing.rules
new file mode 100644
index 0000000..16dd039
--- /dev/null
+++ b/test/data/usr/share/polkit-1/rules.d/20-testing.rules
@@ -0,0 +1,25 @@
+/* -*- mode: js; js-indent-level: 4; indent-tabs-mode: nil -*- */
+
+/* see test/polkitbackend/test-polkitbackendjsauthority.c */
+
+polkit.addRule(function(action, subject, details) {
+    if (action == "net.company.order0") {
+        polkit.log("blabla");
+        details["test_detail"] = "d";
+        return "yes";
+    }
+});
+
+polkit.addRule(function(action, subject, details) {
+    if (action == "net.company.order1") {
+        details["test_detail"] = "d";
+        return "yes";
+    }
+});
+
+polkit.addRule(function(action, subject, details) {
+    if (action == "net.company.order2") {
+        details["test_detail"] = "d";
+        return "yes";
+    }
+});
diff --git a/test/polkitbackend/polkitbackendlocalauthoritytest.c b/test/polkitbackend/polkitbackendlocalauthoritytest.c
index 9fc7848..40e9619 100644
--- a/test/polkitbackend/polkitbackendlocalauthoritytest.c
+++ b/test/polkitbackend/polkitbackendlocalauthoritytest.c
@@ -253,7 +253,7 @@ main (int argc, char *argv[])
       POLKIT_BACKEND_TYPE_AUTHORITY);
 
   add_check_authorization_tests ();
-  g_test_add_func ("/PolkitBackendLocalAuthority/get_admin_identities", test_get_admin_identities);
+  g_test_add_func ("/PolkitBackendJsAuthority/get_admin_identities", test_get_admin_identities);
 
   return g_test_run ();
 };
diff --git a/test/polkitbackend/test-polkitbackendjsauthority.c b/test/polkitbackend/test-polkitbackendjsauthority.c
index 67f5d8b..8057707 100644
--- a/test/polkitbackend/test-polkitbackendjsauthority.c
+++ b/test/polkitbackend/test-polkitbackendjsauthority.c
@@ -102,7 +102,7 @@ test_get_admin_identities_for_action_id (const gchar         *action_id,
   g_clear_object (&subject);
   g_clear_object (&caller);
   g_clear_object (&authority);
-}
+ }
 
 static void
 test_get_admin_identities (void)
@@ -140,6 +140,122 @@ test_get_admin_identities (void)
     }
 }
 
+/* ---------------------------------------------------------------------------------------------------- */
+
+typedef struct RulesTestCase RulesTestCase;
+
+struct RulesTestCase
+{
+  const gchar *test_name;
+  const gchar *action_id;
+  PolkitImplicitAuthorization expected_result;
+  const gchar *expected_detail;
+};
+
+static const RulesTestCase rules_test_cases[] = {
+  /* Check basics */
+  {
+    "basic0",
+    "net.company.productA.action0",
+    POLKIT_IMPLICIT_AUTHORIZATION_ADMINISTRATOR_AUTHENTICATION_REQUIRED,
+    NULL
+  },
+  {
+    "basic1",
+    "net.company.productA.action1",
+    POLKIT_IMPLICIT_AUTHORIZATION_AUTHENTICATION_REQUIRED,
+    NULL
+  },
+
+  /* Ordering tests ... we have four rules files, check they are
+   * evaluated in order by checking the detail set by each rules
+   *
+   * -       etc/polkit-1/rules.d/10-testing.rules (file a)
+   * - usr/share/polkit-1/rules.d/10-testing.rules (file b)
+   * -       etc/polkit-1/rules.d/15-testing.rules (file c)
+   * - usr/share/polkit-1/rules.d/20-testing.rules (file d)
+   *
+   * file.
+   */
+  {
+    /* defined in file a, b, c, d - should pick file a */
+    "order0",
+    "net.company.order0",
+    POLKIT_IMPLICIT_AUTHORIZATION_AUTHORIZED,
+    "a"
+  },
+  {
+    /* defined in file b, c, d - should pick file b */
+    "order1",
+    "net.company.order1",
+    POLKIT_IMPLICIT_AUTHORIZATION_AUTHORIZED,
+    "b"
+  },
+  {
+    /* defined in file c, d - should pick file c */
+    "order2",
+    "net.company.order2",
+    POLKIT_IMPLICIT_AUTHORIZATION_AUTHORIZED,
+    "c"
+  },
+};
+
+/* ---------------------------------------------------------------------------------------------------- */
+
+static void
+rules_test_func (gconstpointer user_data)
+{
+  const RulesTestCase *tc = user_data;
+  PolkitBackendJsAuthority *authority = NULL;
+  PolkitSubject *caller = NULL;
+  PolkitSubject *subject = NULL;
+  PolkitIdentity *user_for_subject = NULL;
+  PolkitDetails *details = NULL;
+  GError *error = NULL;
+  PolkitImplicitAuthorization result;
+
+  authority = get_authority ();
+
+  caller = polkit_unix_process_new (getpid ());
+  subject = polkit_unix_process_new (getpid ());
+  user_for_subject = polkit_identity_from_string ("unix-user:root", &error);
+  g_assert_no_error (error);
+
+  details = polkit_details_new ();
+
+  result = polkit_backend_interactive_authority_check_authorization_sync (POLKIT_BACKEND_INTERACTIVE_AUTHORITY (authority),
+                                                                          caller,
+                                                                          subject,
+                                                                          user_for_subject,
+                                                                          TRUE,
+                                                                          TRUE,
+                                                                          tc->action_id,
+                                                                          details,
+                                                                          POLKIT_IMPLICIT_AUTHORIZATION_UNKNOWN);
+  g_assert_cmpint (result, ==, tc->expected_result);
+  g_assert_cmpstr (polkit_details_lookup (details, "test_detail"), ==, tc->expected_detail);
+
+  g_clear_object (&user_for_subject);
+  g_clear_object (&subject);
+  g_clear_object (&caller);
+  g_clear_object (&authority);
+}
+
+static void
+add_rules_tests (void)
+{
+  guint n;
+  for (n = 0; n < G_N_ELEMENTS (rules_test_cases); n++)
+    {
+      const RulesTestCase *tc = &rules_test_cases[n];
+      gchar *s;
+      s = g_strdup_printf ("/PolkitBackendJsAuthority/rules_%s", tc->test_name);
+      g_test_add_data_func (s, &rules_test_cases[n], rules_test_func);
+      g_free (s);
+    }
+}
+
+/* ---------------------------------------------------------------------------------------------------- */
 
 int
 main (int argc, char *argv[])
@@ -154,6 +270,7 @@ main (int argc, char *argv[])
   g_io_extension_point_set_required_type (ep, POLKIT_BACKEND_TYPE_AUTHORITY);
 
   g_test_add_func ("/PolkitBackendJsAuthority/get_admin_identities", test_get_admin_identities);
+  add_rules_tests ();
 
   return g_test_run ();
 };