From 61b8d9ebe053921a7b7ecbb1f0a252431c76d5d0 Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Tue, 5 Apr 2016 17:52:55 +0200
Subject: nixos/tests: Add a test for the Taskserver service

A small test which checks whether tasks can be synced using the
Taskserver.

It doesn't test group functionality because I suspect that they're not
yet implemented upstream. I haven't done an in-depth check on that but I
couldn't find a method of linking groups to users yet so I guess this
will get in with one of the text releases of Taskwarrior/Taskserver.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 nixos/tests/taskserver.nix | 97 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 nixos/tests/taskserver.nix

(limited to 'nixos/tests')

diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
new file mode 100644
index 000000000000..927dcbd9b28b
--- /dev/null
+++ b/nixos/tests/taskserver.nix
@@ -0,0 +1,97 @@
+import ./make-test.nix {
+  name = "taskserver";
+
+  nodes = {
+    server = {
+      networking.firewall.enable = false;
+      services.taskserver.enable = true;
+      services.taskserver.server.host = "::";
+      services.taskserver.server.fqdn = "server";
+      services.taskserver.organisations = {
+        testOrganisation.users = [ "alice" "foo" ];
+        anotherOrganisation.users = [ "bob" ];
+      };
+    };
+
+    client1 = { pkgs, ... }: {
+      networking.firewall.enable = false;
+      environment.systemPackages = [ pkgs.taskwarrior ];
+      users.users.alice.isNormalUser = true;
+      users.users.bob.isNormalUser = true;
+      users.users.foo.isNormalUser = true;
+    };
+
+    client2 = { pkgs, ... }: {
+      networking.firewall.enable = false;
+      environment.systemPackages = [ pkgs.taskwarrior ];
+      users.users.alice.isNormalUser = true;
+      users.users.bob.isNormalUser = true;
+      users.users.foo.isNormalUser = true;
+    };
+  };
+
+  testScript = { nodes, ... }: let
+    cfg = nodes.server.config.services.taskserver;
+    portStr = toString cfg.server.port;
+  in ''
+    sub su ($$) {
+      my ($user, $cmd) = @_;
+      my $esc = $cmd =~ s/'/'\\${"'"}'/gr;
+      return "su - $user -c '$esc'";
+    }
+
+    sub setupClientsFor ($$) {
+      my ($org, $user) = @_;
+
+      for my $client ($client1, $client2) {
+        $client->nest("initialize client for user $user", sub {
+          $client->succeed(
+            su $user, "task rc.confirmation=no config confirmation no"
+          );
+
+          my $exportinfo = $server->succeed(
+            "nixos-taskdctl export-user $org $user"
+          );
+
+          $exportinfo =~ s/'/'\\'''/g;
+
+          $client->succeed(su $user, "eval '$exportinfo' >&2");
+          $client->succeed(su $user,
+            "task config taskd.server server:${portStr} >&2"
+          );
+
+          $client->succeed(su $user, "task sync init >&2");
+        });
+      }
+    }
+
+    startAll;
+
+    $server->waitForUnit("taskserver.service");
+
+    $server->succeed(
+      "nixos-taskdctl list-users testOrganisation | grep -qxF alice",
+      "nixos-taskdctl list-users testOrganisation | grep -qxF foo",
+      "nixos-taskdctl list-users anotherOrganisation | grep -qxF bob"
+    );
+
+    $server->waitForOpenPort(${portStr});
+
+    $client1->waitForUnit("multi-user.target");
+    $client2->waitForUnit("multi-user.target");
+
+    setupClientsFor "testOrganisation", "alice";
+    setupClientsFor "testOrganisation", "foo";
+    setupClientsFor "anotherOrganisation", "bob";
+
+    for ("alice", "bob", "foo") {
+      subtest "sync for $_", sub {
+        $client1->succeed(su $_, "task add foo >&2");
+        $client1->succeed(su $_, "task sync >&2");
+        $client2->fail(su $_, "task list >&2");
+        $client2->succeed(su $_, "task sync >&2");
+        $client2->succeed(su $_, "task list >&2");
+      };
+    }
+  '';
+}
-- 
cgit 1.4.1


From 2d8961705249a9a144e2f7d944e04fd938b4a2c9 Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Tue, 5 Apr 2016 18:40:15 +0200
Subject: nixos/taskserver: Rename nixos-taskdctl

Using nixos-taskserver is more verbose but less cryptic and I think it
fits the purpose better because it can't be confused to be a wrapper
around the taskdctl command from the upstream project as
nixos-taskserver shares no commonalities with it.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 nixos/modules/services/misc/taskserver/default.nix     | 6 +++---
 nixos/modules/services/misc/taskserver/helper-tool.nix | 7 ++++---
 nixos/tests/taskserver.nix                             | 8 ++++----
 3 files changed, 11 insertions(+), 10 deletions(-)

(limited to 'nixos/tests')

diff --git a/nixos/modules/services/misc/taskserver/default.nix b/nixos/modules/services/misc/taskserver/default.nix
index 4a33ddd9d246..793cc2aa36bb 100644
--- a/nixos/modules/services/misc/taskserver/default.nix
+++ b/nixos/modules/services/misc/taskserver/default.nix
@@ -81,12 +81,12 @@ let
 
   mkShellStr = val: "'${replaceStrings ["'"] ["'\\''"] val}'";
 
-  nixos-taskdctl = import ./helper-tool.nix {
+  nixos-taskserver = import ./helper-tool.nix {
     inherit pkgs lib mkShellStr taskd;
     config = cfg;
   };
 
-  ctlcmd = "${nixos-taskdctl}/bin/nixos-taskdctl --service-helper";
+  ctlcmd = "${nixos-taskserver}/bin/nixos-taskserver --service-helper";
 
 in {
 
@@ -303,7 +303,7 @@ in {
 
   config = mkIf cfg.enable {
 
-    environment.systemPackages = [ pkgs.taskserver nixos-taskdctl ];
+    environment.systemPackages = [ pkgs.taskserver nixos-taskserver ];
 
     users.users = optional (cfg.user == "taskd") {
       name = "taskd";
diff --git a/nixos/modules/services/misc/taskserver/helper-tool.nix b/nixos/modules/services/misc/taskserver/helper-tool.nix
index 90ac85948164..70660574d04c 100644
--- a/nixos/modules/services/misc/taskserver/helper-tool.nix
+++ b/nixos/modules/services/misc/taskserver/helper-tool.nix
@@ -1,6 +1,7 @@
 { config, pkgs, lib, mkShellStr, taskd }:
 
 let
+  commandName = "nixos-taskserver";
   mkShellName = lib.replaceStrings ["-"] ["_"];
 
   genClientKey = ''
@@ -51,7 +52,7 @@ let
     }
 
     usage_${mkShellName name}() {
-      echo "  nixos-taskdctl ${name} ${usagePosArgs}" >&2
+      echo "  ${commandName} ${name} ${usagePosArgs}" >&2
       ${lib.concatMapStringsSep "\n  " mkDesc description}
     }
   '';
@@ -265,7 +266,7 @@ let
       subcmd "${name}" ${cmdArgs};;
   '';
 
-in pkgs.writeScriptBin "nixos-taskdctl" ''
+in pkgs.writeScriptBin commandName ''
   #!${pkgs.stdenv.shell}
   export TASKDDATA=${mkShellStr config.dataDir}
 
@@ -311,7 +312,7 @@ in pkgs.writeScriptBin "nixos-taskdctl" ''
 
   case "$1" in
     ${lib.concatStrings (lib.mapAttrsToList mkCase subcommands)}
-    *) echo "Usage: nixos-taskdctl <subcommand> [<args>]" >&2
+    *) echo "Usage: ${commandName} <subcommand> [<args>]" >&2
        echo >&2
        echo "A tool to manage taskserver users on NixOS" >&2
        echo >&2
diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
index 927dcbd9b28b..61f2b06a7f74 100644
--- a/nixos/tests/taskserver.nix
+++ b/nixos/tests/taskserver.nix
@@ -50,7 +50,7 @@ import ./make-test.nix {
           );
 
           my $exportinfo = $server->succeed(
-            "nixos-taskdctl export-user $org $user"
+            "nixos-taskserver export-user $org $user"
           );
 
           $exportinfo =~ s/'/'\\'''/g;
@@ -70,9 +70,9 @@ import ./make-test.nix {
     $server->waitForUnit("taskserver.service");
 
     $server->succeed(
-      "nixos-taskdctl list-users testOrganisation | grep -qxF alice",
-      "nixos-taskdctl list-users testOrganisation | grep -qxF foo",
-      "nixos-taskdctl list-users anotherOrganisation | grep -qxF bob"
+      "nixos-taskserver list-users testOrganisation | grep -qxF alice",
+      "nixos-taskserver list-users testOrganisation | grep -qxF foo",
+      "nixos-taskserver list-users anotherOrganisation | grep -qxF bob"
     );
 
     $server->waitForOpenPort(${portStr});
-- 
cgit 1.4.1


From 636e0e552d7084c0a3b09bf7cf2ad42f560e3819 Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Mon, 11 Apr 2016 12:03:16 +0200
Subject: nixos/tests/taskserver: Test imperative users

As the nixos-taskserver command can also be used to imperatively manage
users, we need to test this as well.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 nixos/tests/taskserver.nix | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

(limited to 'nixos/tests')

diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
index 61f2b06a7f74..413c52a303ec 100644
--- a/nixos/tests/taskserver.nix
+++ b/nixos/tests/taskserver.nix
@@ -1,7 +1,7 @@
 import ./make-test.nix {
   name = "taskserver";
 
-  nodes = {
+  nodes = rec {
     server = {
       networking.firewall.enable = false;
       services.taskserver.enable = true;
@@ -19,15 +19,10 @@ import ./make-test.nix {
       users.users.alice.isNormalUser = true;
       users.users.bob.isNormalUser = true;
       users.users.foo.isNormalUser = true;
+      users.users.bar.isNormalUser = true;
     };
 
-    client2 = { pkgs, ... }: {
-      networking.firewall.enable = false;
-      environment.systemPackages = [ pkgs.taskwarrior ];
-      users.users.alice.isNormalUser = true;
-      users.users.bob.isNormalUser = true;
-      users.users.foo.isNormalUser = true;
-    };
+    client2 = client1;
   };
 
   testScript = { nodes, ... }: let
@@ -65,6 +60,17 @@ import ./make-test.nix {
       }
     }
 
+    sub testSync ($) {
+      my $user = $_[0];
+      subtest "sync for user $user", sub {
+        $client1->succeed(su $user, "task add foo >&2");
+        $client1->succeed(su $user, "task sync >&2");
+        $client2->fail(su $user, "task list >&2");
+        $client2->succeed(su $user, "task sync >&2");
+        $client2->succeed(su $user, "task list >&2");
+      };
+    }
+
     startAll;
 
     $server->waitForUnit("taskserver.service");
@@ -84,14 +90,16 @@ import ./make-test.nix {
     setupClientsFor "testOrganisation", "foo";
     setupClientsFor "anotherOrganisation", "bob";
 
-    for ("alice", "bob", "foo") {
-      subtest "sync for $_", sub {
-        $client1->succeed(su $_, "task add foo >&2");
-        $client1->succeed(su $_, "task sync >&2");
-        $client2->fail(su $_, "task list >&2");
-        $client2->succeed(su $_, "task sync >&2");
-        $client2->succeed(su $_, "task list >&2");
-      };
-    }
+    testSync $_ for ("alice", "bob", "foo");
+
+    $server->fail("nixos-taskserver add-user imperativeOrg bar");
+    $server->succeed(
+      "nixos-taskserver add-org imperativeOrg",
+      "nixos-taskserver add-user imperativeOrg bar"
+    );
+
+    setupClientsFor "imperativeOrg", "bar";
+
+    testSync "bar";
   '';
 }
-- 
cgit 1.4.1


From d6bd457d1f5514468a34c32e54076d0cf5a02122 Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Mon, 11 Apr 2016 12:26:34 +0200
Subject: nixos/taskserver: Rename server.{host,port}

Having an option called services.taskserver.server.host is quite
confusing because we already have "server" in the service name, so let's
first get rid of the listening options before we rename the rest of the
options in that .server attribute.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 nixos/modules/services/misc/taskserver/default.nix | 32 +++++++++++-----------
 nixos/tests/taskserver.nix                         |  4 +--
 2 files changed, 18 insertions(+), 18 deletions(-)

(limited to 'nixos/tests')

diff --git a/nixos/modules/services/misc/taskserver/default.nix b/nixos/modules/services/misc/taskserver/default.nix
index 86eabb9bcfc8..8f760a4579d4 100644
--- a/nixos/modules/services/misc/taskserver/default.nix
+++ b/nixos/modules/services/misc/taskserver/default.nix
@@ -42,7 +42,7 @@ let
     ${mkConfLine "client.deny" cfg.disallowedClientIDs}
 
     # server
-    server = ${cfg.server.host}:${toString cfg.server.port}
+    server = ${cfg.listenHost}:${toString cfg.listenPort}
     ${mkConfLine "server.crl" cfg.server.crl}
 
     # certificates
@@ -245,23 +245,23 @@ in {
         '';
       };
 
-      server = {
-        host = mkOption {
-          type = types.str;
-          default = "localhost";
-          description = ''
-            The address (IPv4, IPv6 or DNS) to listen on.
-          '';
-        };
+      listenHost = mkOption {
+        type = types.str;
+        default = "localhost";
+        description = ''
+          The address (IPv4, IPv6 or DNS) to listen on.
+        '';
+      };
 
-        port = mkOption {
-          type = types.int;
-          default = 53589;
-          description = ''
-            Port number of the Taskserver.
-          '';
-        };
+      listenPort = mkOption {
+        type = types.int;
+        default = 53589;
+        description = ''
+          Port number of the Taskserver.
+        '';
+      };
 
+      server = {
         fqdn = mkOption {
           type = types.str;
           default = "localhost";
diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
index 413c52a303ec..d588b178aae8 100644
--- a/nixos/tests/taskserver.nix
+++ b/nixos/tests/taskserver.nix
@@ -5,7 +5,7 @@ import ./make-test.nix {
     server = {
       networking.firewall.enable = false;
       services.taskserver.enable = true;
-      services.taskserver.server.host = "::";
+      services.taskserver.listenHost = "::";
       services.taskserver.server.fqdn = "server";
       services.taskserver.organisations = {
         testOrganisation.users = [ "alice" "foo" ];
@@ -27,7 +27,7 @@ import ./make-test.nix {
 
   testScript = { nodes, ... }: let
     cfg = nodes.server.config.services.taskserver;
-    portStr = toString cfg.server.port;
+    portStr = toString cfg.listenPort;
   in ''
     sub su ($$) {
       my ($user, $cmd) = @_;
-- 
cgit 1.4.1


From 6de94e7d2449eefccdb99100426759472e4b14a4 Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Mon, 11 Apr 2016 12:38:16 +0200
Subject: nixos/taskserver: Rename .server options to .pki

After moving out the PKI-unrelated options, let's name this a bit more
appropriate, so we can finally get rid of the taskserver.server thing.

This also moves taskserver.caCert to taskserver.pki.caCert, because that
clearly belongs to the PKI options.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 nixos/modules/services/misc/taskserver/default.nix | 37 +++++++++++-----------
 nixos/tests/taskserver.nix                         |  2 +-
 2 files changed, 19 insertions(+), 20 deletions(-)

(limited to 'nixos/tests')

diff --git a/nixos/modules/services/misc/taskserver/default.nix b/nixos/modules/services/misc/taskserver/default.nix
index 8f760a4579d4..063002167cf5 100644
--- a/nixos/modules/services/misc/taskserver/default.nix
+++ b/nixos/modules/services/misc/taskserver/default.nix
@@ -17,9 +17,7 @@ let
     result = "${key} = ${mkVal val}";
   in optionalString (val != null && val != []) result;
 
-  needToCreateCA = all isNull (with cfg; [
-    server.key server.cert server.crl caCert
-  ]);
+  needToCreateCA = all isNull (with cfg.pki; [ key cert crl caCert ]);
 
   configFile = pkgs.writeText "taskdrc" ''
     # systemd related
@@ -43,18 +41,18 @@ let
 
     # server
     server = ${cfg.listenHost}:${toString cfg.listenPort}
-    ${mkConfLine "server.crl" cfg.server.crl}
+    ${mkConfLine "server.crl" cfg.pki.crl}
 
     # certificates
-    ${mkConfLine "trust" cfg.server.trust}
+    ${mkConfLine "trust" cfg.pki.trust}
     ${if needToCreateCA then ''
       ca.cert = ${cfg.dataDir}/keys/ca.cert
       server.cert = ${cfg.dataDir}/keys/server.cert
       server.key = ${cfg.dataDir}/keys/server.key
     '' else ''
-      ca.cert = ${cfg.caCert}
-      server.cert = ${cfg.server.cert}
-      server.key = ${cfg.server.key}
+      ca.cert = ${cfg.pki.caCert}
+      server.cert = ${cfg.pki.cert}
+      server.key = ${cfg.pki.key}
     ''}
   '';
 
@@ -91,7 +89,7 @@ let
         certtool = "${pkgs.gnutls}/bin/certtool";
         inherit taskd;
         inherit (cfg) dataDir user group;
-        inherit (cfg.server) fqdn;
+        inherit (cfg.pki) fqdn;
       }}" > "$out/main.py"
       cat > "$out/setup.py" <<EOF
       from setuptools import setup
@@ -134,12 +132,6 @@ in {
         description = "Data directory for Taskserver.";
       };
 
-      caCert = mkOption {
-        type = types.nullOr types.path;
-        default = null;
-        description = "Fully qualified path to the CA certificate.";
-      };
-
       ciphers = mkOption {
         type = types.nullOr (types.separatedString ":");
         default = null;
@@ -261,12 +253,13 @@ in {
         '';
       };
 
-      server = {
+      pki = {
         fqdn = mkOption {
           type = types.str;
           default = "localhost";
           description = ''
-            The fully qualified domain name of this server.
+            The fully qualified domain name of this server, which is used as the
+            common name in the certificates.
           '';
         };
 
@@ -276,6 +269,12 @@ in {
           description = "Fully qualified path to the server certificate";
         };
 
+        caCert = mkOption {
+          type = types.nullOr types.path;
+          default = null;
+          description = "Fully qualified path to the CA certificate.";
+        };
+
         crl = mkOption {
           type = types.nullOr types.path;
           default = null;
@@ -346,7 +345,7 @@ in {
             --outfile "${cfg.dataDir}/keys/ca.key"
           ${pkgs.gnutls}/bin/certtool -s \
             --template "${pkgs.writeText "taskserver-ca.template" ''
-              cn = ${cfg.server.fqdn}
+              cn = ${cfg.pki.fqdn}
               cert_signing_key
               ca
             ''}" \
@@ -364,7 +363,7 @@ in {
 
           ${pkgs.gnutls}/bin/certtool -c \
             --template "${pkgs.writeText "taskserver-cert.template" ''
-              cn = ${cfg.server.fqdn}
+              cn = ${cfg.pki.fqdn}
               tls_www_server
               encryption_key
               signing_key
diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
index d588b178aae8..ad8347a3987e 100644
--- a/nixos/tests/taskserver.nix
+++ b/nixos/tests/taskserver.nix
@@ -6,7 +6,7 @@ import ./make-test.nix {
       networking.firewall.enable = false;
       services.taskserver.enable = true;
       services.taskserver.listenHost = "::";
-      services.taskserver.server.fqdn = "server";
+      services.taskserver.pki.fqdn = "server";
       services.taskserver.organisations = {
         testOrganisation.users = [ "alice" "foo" ];
         anotherOrganisation.users = [ "bob" ];
-- 
cgit 1.4.1


From 3affead91bd15345788554bdf1157393488a514e Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Mon, 11 Apr 2016 12:42:20 +0200
Subject: nixos/taskserver: Move .pki.fqdn to .fqdn

It's not necessarily related to the PKI options, because this is also
used for setting the server address on the Taskwarrior client.

So if someone doesn't have his/her own certificates from another CA, all
options that need to be adjusted are in .pki. And if someone doesn't
want to bother with getting certificates from another CA, (s)he just
doesn't set anything in .pki.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 nixos/modules/services/misc/taskserver/default.nix | 25 +++++++++++-----------
 nixos/tests/taskserver.nix                         |  2 +-
 2 files changed, 13 insertions(+), 14 deletions(-)

(limited to 'nixos/tests')

diff --git a/nixos/modules/services/misc/taskserver/default.nix b/nixos/modules/services/misc/taskserver/default.nix
index 063002167cf5..c5c3600c1a61 100644
--- a/nixos/modules/services/misc/taskserver/default.nix
+++ b/nixos/modules/services/misc/taskserver/default.nix
@@ -88,8 +88,7 @@ let
         src = ./helper-tool.py;
         certtool = "${pkgs.gnutls}/bin/certtool";
         inherit taskd;
-        inherit (cfg) dataDir user group;
-        inherit (cfg.pki) fqdn;
+        inherit (cfg) dataDir user group fqdn;
       }}" > "$out/main.py"
       cat > "$out/setup.py" <<EOF
       from setuptools import setup
@@ -253,16 +252,16 @@ in {
         '';
       };
 
-      pki = {
-        fqdn = mkOption {
-          type = types.str;
-          default = "localhost";
-          description = ''
-            The fully qualified domain name of this server, which is used as the
-            common name in the certificates.
-          '';
-        };
+      fqdn = mkOption {
+        type = types.str;
+        default = "localhost";
+        description = ''
+          The fully qualified domain name of this server, which is also used
+          as the common name in the certificates.
+        '';
+      };
 
+      pki = {
         cert = mkOption {
           type = types.nullOr types.path;
           default = null;
@@ -345,7 +344,7 @@ in {
             --outfile "${cfg.dataDir}/keys/ca.key"
           ${pkgs.gnutls}/bin/certtool -s \
             --template "${pkgs.writeText "taskserver-ca.template" ''
-              cn = ${cfg.pki.fqdn}
+              cn = ${cfg.fqdn}
               cert_signing_key
               ca
             ''}" \
@@ -363,7 +362,7 @@ in {
 
           ${pkgs.gnutls}/bin/certtool -c \
             --template "${pkgs.writeText "taskserver-cert.template" ''
-              cn = ${cfg.pki.fqdn}
+              cn = ${cfg.fqdn}
               tls_www_server
               encryption_key
               signing_key
diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
index ad8347a3987e..1a9c8dfaca25 100644
--- a/nixos/tests/taskserver.nix
+++ b/nixos/tests/taskserver.nix
@@ -6,7 +6,7 @@ import ./make-test.nix {
       networking.firewall.enable = false;
       services.taskserver.enable = true;
       services.taskserver.listenHost = "::";
-      services.taskserver.pki.fqdn = "server";
+      services.taskserver.fqdn = "server";
       services.taskserver.organisations = {
         testOrganisation.users = [ "alice" "foo" ];
         anotherOrganisation.users = [ "bob" ];
-- 
cgit 1.4.1


From 7889fcfa41c718b52e2161e74de38a8479cd50fb Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Tue, 12 Apr 2016 01:08:34 +0200
Subject: nixos/taskserver/helper: Implement deletion

Now we finally can delete organisations, groups and users along with
certificate revocation. The new subtests now make sure that the client
certificate is also revoked (both when removing the whole organisation
and just a single user).

If we use the imperative way to add and delete users, we have to restart
the Taskserver in order for the CRL to be effective.

However, by using the declarative configuration we now get this for
free, because removing a user will also restart the service and thus its
client certificate will end up in the CRL.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 .../services/misc/taskserver/helper-tool.py        | 132 ++++++++++++++++++---
 nixos/tests/taskserver.nix                         |  61 ++++++++--
 2 files changed, 168 insertions(+), 25 deletions(-)

(limited to 'nixos/tests')

diff --git a/nixos/modules/services/misc/taskserver/helper-tool.py b/nixos/modules/services/misc/taskserver/helper-tool.py
index c255081f5657..cd712332e039 100644
--- a/nixos/modules/services/misc/taskserver/helper-tool.py
+++ b/nixos/modules/services/misc/taskserver/helper-tool.py
@@ -7,6 +7,7 @@ import string
 import subprocess
 import sys
 
+from contextlib import contextmanager
 from shutil import rmtree
 from tempfile import NamedTemporaryFile
 
@@ -86,6 +87,19 @@ def fetch_username(org, key):
     return None
 
 
+@contextmanager
+def create_template(contents):
+    """
+    Generate a temporary file with the specified contents as a list of strings
+    and yield its path as the context.
+    """
+    template = NamedTemporaryFile(mode="w", prefix="certtool-template")
+    template.writelines(map(lambda l: l + "\n", contents))
+    template.flush()
+    yield template.name
+    template.close()
+
+
 def generate_key(org, user):
     basedir = os.path.join(TASKD_DATA_DIR, "keys", org, user)
     if os.path.exists(basedir):
@@ -100,30 +114,57 @@ def generate_key(org, user):
         os.makedirs(basedir, mode=0700)
 
         cmd = [CERTTOOL_COMMAND, "-p", "--bits", "2048", "--outfile", privkey]
-        subprocess.call(cmd, preexec_fn=lambda: os.umask(0077))
+        subprocess.check_call(cmd, preexec_fn=lambda: os.umask(0077))
 
-        template = NamedTemporaryFile(mode="w", prefix="certtool-template")
-        template.writelines(map(lambda l: l + "\n", [
+        template_data = [
             "organization = {0}".format(org),
             "cn = {}".format(FQDN),
             "tls_www_client",
             "encryption_key",
             "signing_key"
-        ]))
-        template.flush()
+        ]
 
-        cmd = [CERTTOOL_COMMAND, "-c",
-               "--load-privkey", privkey,
-               "--load-ca-privkey", cakey,
-               "--load-ca-certificate", cacert,
-               "--template", template.name,
-               "--outfile", pubcert]
-        subprocess.call(cmd, preexec_fn=lambda: os.umask(0077))
+        with create_template(template_data) as template:
+            cmd = [CERTTOOL_COMMAND, "-c",
+                   "--load-privkey", privkey,
+                   "--load-ca-privkey", cakey,
+                   "--load-ca-certificate", cacert,
+                   "--template", template,
+                   "--outfile", pubcert]
+            subprocess.check_call(cmd, preexec_fn=lambda: os.umask(0077))
     except:
         rmtree(basedir)
         raise
 
 
+def revoke_key(org, user):
+    cakey = os.path.join(TASKD_DATA_DIR, "keys", "ca.key")
+    cacert = os.path.join(TASKD_DATA_DIR, "keys", "ca.cert")
+    crl = os.path.join(TASKD_DATA_DIR, "keys", "server.crl")
+
+    basedir = os.path.join(TASKD_DATA_DIR, "keys", org, user)
+    if not os.path.exists(basedir):
+        raise OSError("Keyfile directory for {} doesn't exist.".format(user))
+
+    pubcert = os.path.join(basedir, "public.cert")
+
+    with create_template(["expiration_days = 3650"]) as template:
+        oldcrl = NamedTemporaryFile(mode="wb", prefix="old-crl")
+        oldcrl.write(open(crl, "rb").read())
+        oldcrl.flush()
+        cmd = [CERTTOOL_COMMAND,
+               "--generate-crl",
+               "--load-crl", oldcrl.name,
+               "--load-ca-privkey", cakey,
+               "--load-ca-certificate", cacert,
+               "--load-certificate", pubcert,
+               "--template", template,
+               "--outfile", crl]
+        subprocess.check_call(cmd, preexec_fn=lambda: os.umask(0077))
+        oldcrl.close()
+    rmtree(basedir)
+
+
 def is_key_line(line, match):
     return line.startswith("---") and line.lstrip("- ").startswith(match)
 
@@ -215,8 +256,13 @@ class Organisation(object):
         """
         Delete a user and revoke its keys.
         """
-        sys.stderr.write("Delete user {}.".format(name))
-        # TODO: deletion!
+        if name in self.users.keys():
+            # Work around https://bug.tasktools.org/browse/TD-40:
+            user = self.get_user(name)
+            rmtree(mkpath(self.name, "users", user.key))
+
+            revoke_key(self.name, name)
+            del self._lazy_users[name]
 
     def add_group(self, name):
         """
@@ -235,8 +281,9 @@ class Organisation(object):
         """
         Delete a group.
         """
-        sys.stderr.write("Delete group {}.".format(name))
-        # TODO: deletion!
+        if name in self.users.keys():
+            taskd_cmd("remove", "group", self.name, name)
+            del self._lazy_groups[name]
 
     def get_user(self, name):
         return self.users.get(name)
@@ -281,8 +328,14 @@ class Manager(object):
         Delete and revoke keys of an organisation with all its users and
         groups.
         """
-        sys.stderr.write("Delete org {}.".format(name))
-        # TODO: deletion!
+        org = self.get_org(name)
+        if org is not None:
+            for user in org.users.keys():
+                org.del_user(user)
+            for group in org.groups.keys():
+                org.del_group(group)
+            taskd_cmd("remove", "org", name)
+            del self._lazy_orgs[name]
 
     def get_org(self, name):
         return self.orgs.get(name)
@@ -383,6 +436,22 @@ def add_org(name):
     taskd_cmd("add", "org", name)
 
 
+@cli.command("del-org")
+@click.argument("name")
+def del_org(name):
+    """
+    Delete the organisation with the specified name.
+
+    All of the users and groups will be deleted as well and client certificates
+    will be revoked.
+    """
+    Manager().del_org(name)
+    msg = ("Organisation {} deleted. Be sure to restart the Taskserver"
+           " using 'systemctl restart taskserver.service' in order for"
+           " the certificate revocation to apply.")
+    click.echo(msg.format(name), err=True)
+
+
 @cli.command("add-user")
 @click.argument("organisation", type=ORGANISATION)
 @click.argument("user")
@@ -400,6 +469,22 @@ def add_user(organisation, user):
         sys.exit(msg.format(user, organisation))
 
 
+@cli.command("del-user")
+@click.argument("organisation", type=ORGANISATION)
+@click.argument("user")
+def del_user(organisation, user):
+    """
+    Delete a user from the given organisation.
+
+    This will also revoke the client certificate of the given user.
+    """
+    organisation.del_user(user)
+    msg = ("User {} deleted. Be sure to restart the Taskserver using"
+           " 'systemctl restart taskserver.service' in order for the"
+           " certificate revocation to apply.")
+    click.echo(msg.format(user), err=True)
+
+
 @cli.command("add-group")
 @click.argument("organisation", type=ORGANISATION)
 @click.argument("group")
@@ -413,6 +498,17 @@ def add_group(organisation, group):
         sys.exit(msg.format(group, organisation))
 
 
+@cli.command("del-group")
+@click.argument("organisation", type=ORGANISATION)
+@click.argument("group")
+def del_group(organisation, group):
+    """
+    Delete a group from the given organisation.
+    """
+    organisation.del_group(group)
+    click("Group {} deleted.".format(group), err=True)
+
+
 def add_or_delete(old, new, add_fun, del_fun):
     """
     Given an 'old' and 'new' list, figure out the intersections and invoke
diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
index 1a9c8dfaca25..574af0aa8803 100644
--- a/nixos/tests/taskserver.nix
+++ b/nixos/tests/taskserver.nix
@@ -15,7 +15,7 @@ import ./make-test.nix {
 
     client1 = { pkgs, ... }: {
       networking.firewall.enable = false;
-      environment.systemPackages = [ pkgs.taskwarrior ];
+      environment.systemPackages = [ pkgs.taskwarrior pkgs.gnutls ];
       users.users.alice.isNormalUser = true;
       users.users.bob.isNormalUser = true;
       users.users.foo.isNormalUser = true;
@@ -60,6 +60,22 @@ import ./make-test.nix {
       }
     }
 
+    sub restartServer {
+      $server->succeed("systemctl restart taskserver.service");
+      $server->waitForOpenPort(${portStr});
+    }
+
+    sub readdImperativeUser {
+      $server->nest("(re-)add imperative user bar", sub {
+        $server->execute("nixos-taskserver del-org imperativeOrg");
+        $server->succeed(
+          "nixos-taskserver add-org imperativeOrg",
+          "nixos-taskserver add-user imperativeOrg bar"
+        );
+        setupClientsFor "imperativeOrg", "bar";
+      });
+    }
+
     sub testSync ($) {
       my $user = $_[0];
       subtest "sync for user $user", sub {
@@ -71,6 +87,16 @@ import ./make-test.nix {
       };
     }
 
+    sub checkClientCert ($) {
+      my $user = $_[0];
+      my $cmd = "gnutls-cli".
+        " --x509cafile=/home/$user/.task/keys/ca.cert".
+        " --x509keyfile=/home/$user/.task/keys/private.key".
+        " --x509certfile=/home/$user/.task/keys/public.cert".
+        " --port=${portStr} server < /dev/null";
+      return su $user, $cmd;
+    }
+
     startAll;
 
     $server->waitForUnit("taskserver.service");
@@ -93,13 +119,34 @@ import ./make-test.nix {
     testSync $_ for ("alice", "bob", "foo");
 
     $server->fail("nixos-taskserver add-user imperativeOrg bar");
-    $server->succeed(
-      "nixos-taskserver add-org imperativeOrg",
-      "nixos-taskserver add-user imperativeOrg bar"
-    );
-
-    setupClientsFor "imperativeOrg", "bar";
+    readdImperativeUser;
 
     testSync "bar";
+
+    subtest "checking certificate revocation of user bar", sub {
+      $client1->succeed(checkClientCert "bar");
+
+      $server->succeed("nixos-taskserver del-user imperativeOrg bar");
+      restartServer;
+
+      $client1->fail(checkClientCert "bar");
+
+      $client1->succeed(su "bar", "task add destroy everything >&2");
+      $client1->fail(su "bar", "task sync >&2");
+    };
+
+    readdImperativeUser;
+
+    subtest "checking certificate revocation of org imperativeOrg", sub {
+      $client1->succeed(checkClientCert "bar");
+
+      $server->succeed("nixos-taskserver del-org imperativeOrg");
+      restartServer;
+
+      $client1->fail(checkClientCert "bar");
+
+      $client1->succeed(su "bar", "task add destroy even more >&2");
+      $client1->fail(su "bar", "task sync >&2");
+    };
   '';
 }
-- 
cgit 1.4.1


From cfb6ce2abed2c96d0f5af268e2d22322f47831ed Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Tue, 12 Apr 2016 01:49:47 +0200
Subject: nixos/tests/taskserver: Make tests less noisy

We were putting the whole output of "nixos-taskserver export-user" from
the server to the respective client and on every such operation the
whole output was shown again in the test log.

Now we're *only* showing these details whenever a user import fails on
the client.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 nixos/tests/taskserver.nix | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

(limited to 'nixos/tests')

diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
index 574af0aa8803..5d2e030a8f6d 100644
--- a/nixos/tests/taskserver.nix
+++ b/nixos/tests/taskserver.nix
@@ -50,7 +50,15 @@ import ./make-test.nix {
 
           $exportinfo =~ s/'/'\\'''/g;
 
-          $client->succeed(su $user, "eval '$exportinfo' >&2");
+          $client->nest("importing taskwarrior configuration", sub {
+            my $cmd = su $user, "eval '$exportinfo' >&2";
+            my ($status, $out) = $client->execute_($cmd);
+            if ($status != 0) {
+              $client->log("output: $out");
+              die "command `$cmd' did not succeed (exit code $status)\n";
+            }
+          });
+
           $client->succeed(su $user,
             "task config taskd.server server:${portStr} >&2"
           );
-- 
cgit 1.4.1


From a41b109bc10e66824af5e1f150cb741f9f9399c2 Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Tue, 12 Apr 2016 03:42:13 +0200
Subject: nixos/taskserver: Don't change imperative users

Whenever the nixos-taskserver tool was invoked manually for creating an
organisation/group/user we now add an empty file called .imperative to
the data directory.

During the preStart of the Taskserver service, we use process-json which
in turn now checks whether those .imperative files exist and if so, it
doesn't do anything with it.

This should now ensure that whenever there is a manually created user,
it doesn't get killed off by the declarative configuration in case it
shouldn't exist within that configuration.

In addition, we also add a small subtest to check whether this is
happening or not and fail if the imperatively created user got deleted
by process-json.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 .../services/misc/taskserver/helper-tool.py        | 69 +++++++++++++++++++---
 nixos/tests/taskserver.nix                         | 10 +++-
 2 files changed, 70 insertions(+), 9 deletions(-)

(limited to 'nixos/tests')

diff --git a/nixos/modules/services/misc/taskserver/helper-tool.py b/nixos/modules/services/misc/taskserver/helper-tool.py
index 30dcfe0a7a25..512aaa4ab9f8 100644
--- a/nixos/modules/services/misc/taskserver/helper-tool.py
+++ b/nixos/modules/services/misc/taskserver/helper-tool.py
@@ -96,6 +96,28 @@ def mkpath(*args):
     return os.path.join(TASKD_DATA_DIR, "orgs", *args)
 
 
+def mark_imperative(*path):
+    """
+    Mark the specified path as being imperatively managed by creating an empty
+    file called ".imperative", so that it doesn't interfere with the
+    declarative configuration.
+    """
+    open(os.path.join(mkpath(*path), ".imperative"), 'a').close()
+
+
+def is_imperative(*path):
+    """
+    Check whether the given path is marked as imperative, see mark_imperative()
+    for more information.
+    """
+    full_path = []
+    for component in path:
+        full_path.append(component)
+        if os.path.exists(os.path.join(mkpath(*full_path), ".imperative")):
+            return True
+    return False
+
+
 def fetch_username(org, key):
     for line in open(mkpath(org, "users", key, "config"), "r"):
         match = RE_CONFIGUSER.match(line)
@@ -247,8 +269,9 @@ class Group(object):
 
 
 class Organisation(object):
-    def __init__(self, name):
+    def __init__(self, name, ignore_imperative):
         self.name = name
+        self.ignore_imperative = ignore_imperative
 
     def add_user(self, name):
         """
@@ -256,6 +279,8 @@ class Organisation(object):
 
         Returns a 'User' object or None if the user already exists.
         """
+        if self.ignore_imperative and is_imperative(self.name):
+            return None
         if name not in self.users.keys():
             output = taskd_cmd("add", "user", self.name, name,
                                capture_stdout=True)
@@ -265,7 +290,7 @@ class Organisation(object):
                 raise TaskdError(msg.format(name))
 
             generate_key(self.name, name)
-            newuser = User(self.name, name, key)
+            newuser = User(self.name, name, key.group(1))
             self._lazy_users[name] = newuser
             return newuser
         return None
@@ -275,8 +300,12 @@ class Organisation(object):
         Delete a user and revoke its keys.
         """
         if name in self.users.keys():
-            # Work around https://bug.tasktools.org/browse/TD-40:
             user = self.get_user(name)
+            if self.ignore_imperative and \
+               is_imperative(self.name, "users", user.key):
+                return
+
+            # Work around https://bug.tasktools.org/browse/TD-40:
             rmtree(mkpath(self.name, "users", user.key))
 
             revoke_key(self.name, name)
@@ -288,6 +317,8 @@ class Organisation(object):
 
         Returns a 'Group' object or None if the group already exists.
         """
+        if self.ignore_imperative and is_imperative(self.name):
+            return None
         if name not in self.groups.keys():
             taskd_cmd("add", "group", self.name, name)
             newgroup = Group(self.name, name)
@@ -300,6 +331,9 @@ class Organisation(object):
         Delete a group.
         """
         if name in self.users.keys():
+            if self.ignore_imperative and \
+               is_imperative(self.name, "groups", name):
+                return
             taskd_cmd("remove", "group", self.name, name)
             del self._lazy_groups[name]
 
@@ -327,6 +361,16 @@ class Organisation(object):
 
 
 class Manager(object):
+    def __init__(self, ignore_imperative=False):
+        """
+        Instantiates an organisations manager.
+
+        If ignore_imperative is True, all actions that modify data are checked
+        whether they're created imperatively and if so, they will result in no
+        operation.
+        """
+        self.ignore_imperative = ignore_imperative
+
     def add_org(self, name):
         """
         Create a new organisation.
@@ -336,7 +380,7 @@ class Manager(object):
         """
         if name not in self.orgs.keys():
             taskd_cmd("add", "org", name)
-            neworg = Organisation(name)
+            neworg = Organisation(name, self.ignore_imperative)
             self._lazy_orgs[name] = neworg
             return neworg
         return None
@@ -348,6 +392,8 @@ class Manager(object):
         """
         org = self.get_org(name)
         if org is not None:
+            if self.ignore_imperative and is_imperative(name):
+                return
             for user in org.users.keys():
                 org.del_user(user)
             for group in org.groups.keys():
@@ -362,7 +408,7 @@ class Manager(object):
     def orgs(self):
         result = {}
         for org in os.listdir(mkpath()):
-            result[org] = Organisation(org)
+            result[org] = Organisation(org, self.ignore_imperative)
         return result
 
 
@@ -452,6 +498,7 @@ def add_org(name):
         sys.exit(msg.format(name))
 
     taskd_cmd("add", "org", name)
+    mark_imperative(name)
 
 
 @cli.command("del-org")
@@ -485,6 +532,8 @@ def add_user(organisation, user):
     if userobj is None:
         msg = "User {} already exists in organisation {}."
         sys.exit(msg.format(user, organisation))
+    else:
+        mark_imperative(organisation.name, "users", userobj.key)
 
 
 @cli.command("del-user")
@@ -510,10 +559,12 @@ def add_group(organisation, group):
     """
     Create a group for the given organisation.
     """
-    userobj = organisation.add_group(group)
-    if userobj is None:
+    groupobj = organisation.add_group(group)
+    if groupobj is None:
         msg = "Group {} already exists in organisation {}."
         sys.exit(msg.format(group, organisation))
+    else:
+        mark_imperative(organisation.name, "groups", groupobj.name)
 
 
 @cli.command("del-group")
@@ -562,10 +613,12 @@ def process_json(json_file):
     """
     data = json.load(json_file)
 
-    mgr = Manager()
+    mgr = Manager(ignore_imperative=True)
     add_or_delete(mgr.orgs.keys(), data.keys(), mgr.add_org, mgr.del_org)
 
     for org in mgr.orgs.values():
+        if is_imperative(org.name):
+            continue
         add_or_delete(org.users.keys(), data[org.name]['users'],
                       org.add_user, org.del_user)
         add_or_delete(org.groups.keys(), data[org.name]['groups'],
diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
index 5d2e030a8f6d..79a7703f037e 100644
--- a/nixos/tests/taskserver.nix
+++ b/nixos/tests/taskserver.nix
@@ -41,7 +41,8 @@ import ./make-test.nix {
       for my $client ($client1, $client2) {
         $client->nest("initialize client for user $user", sub {
           $client->succeed(
-            su $user, "task rc.confirmation=no config confirmation no"
+            (su $user, "rm -rf /home/$user/.task"),
+            (su $user, "task rc.confirmation=no config confirmation no")
           );
 
           my $exportinfo = $server->succeed(
@@ -156,5 +157,12 @@ import ./make-test.nix {
       $client1->succeed(su "bar", "task add destroy even more >&2");
       $client1->fail(su "bar", "task sync >&2");
     };
+
+    readdImperativeUser;
+
+    subtest "check whether declarative config overrides user bar", sub {
+      restartServer;
+      testSync "bar";
+    };
   '';
 }
-- 
cgit 1.4.1


From ce0954020c71007b7a9ec2822949d31f18aea170 Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Tue, 12 Apr 2016 05:13:04 +0200
Subject: nixos/taskserver: Set allowedTCPPorts accordingly

As suggested by @matthiasbeyer:

"We might add a short note that this port has to be opened in the
firewall, or is this done by the service automatically?"

This commit now adds the listenPort to
networking.firewall.allowedTCPPorts as soon as the listenHost is not
"localhost".

In addition to that, this is now also documented in the listenHost
option declaration and I have removed disabling of the firewall from the
VM test.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 nixos/modules/services/misc/taskserver/default.nix | 8 ++++++++
 nixos/tests/taskserver.nix                         | 2 --
 2 files changed, 8 insertions(+), 2 deletions(-)

(limited to 'nixos/tests')

diff --git a/nixos/modules/services/misc/taskserver/default.nix b/nixos/modules/services/misc/taskserver/default.nix
index 520a9c2ee1e5..8054dbe9f662 100644
--- a/nixos/modules/services/misc/taskserver/default.nix
+++ b/nixos/modules/services/misc/taskserver/default.nix
@@ -324,8 +324,13 @@ in {
       listenHost = mkOption {
         type = types.str;
         default = "localhost";
+        example = "::";
         description = ''
           The address (IPv4, IPv6 or DNS) to listen on.
+
+          If the value is something else than <literal>localhost</literal> the
+          port defined by <option>listenPort</option> is automatically added to
+          <option>networking.firewall.allowedTCPPorts</option>.
         '';
       };
 
@@ -519,6 +524,9 @@ in {
         '';
       };
     })
+    (mkIf (cfg.listenHost != "localhost") {
+      networking.firewall.allowedTCPPorts = [ cfg.listenPort ];
+    })
     { meta.doc = ./taskserver.xml; }
   ];
 }
diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
index 79a7703f037e..0521f97431b3 100644
--- a/nixos/tests/taskserver.nix
+++ b/nixos/tests/taskserver.nix
@@ -3,7 +3,6 @@ import ./make-test.nix {
 
   nodes = rec {
     server = {
-      networking.firewall.enable = false;
       services.taskserver.enable = true;
       services.taskserver.listenHost = "::";
       services.taskserver.fqdn = "server";
@@ -14,7 +13,6 @@ import ./make-test.nix {
     };
 
     client1 = { pkgs, ... }: {
-      networking.firewall.enable = false;
       environment.systemPackages = [ pkgs.taskwarrior pkgs.gnutls ];
       users.users.alice.isNormalUser = true;
       users.users.bob.isNormalUser = true;
-- 
cgit 1.4.1


From e2383b84f88e0e7d35f6a3a846b54c69e3bee6ee Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@redmoonstudios.org>
Date: Tue, 12 Apr 2016 05:38:37 +0200
Subject: nixos/taskserver/helper: Improve CLI subcommands

Try to match the subcommands to act more like the subcommands from the
taskd binary and also add a subcommand to list groups.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
---
 .../services/misc/taskserver/helper-tool.py        | 55 ++++++++++++++++++----
 nixos/tests/taskserver.nix                         | 20 ++++----
 2 files changed, 55 insertions(+), 20 deletions(-)

(limited to 'nixos/tests')

diff --git a/nixos/modules/services/misc/taskserver/helper-tool.py b/nixos/modules/services/misc/taskserver/helper-tool.py
index e2c340fbd2a0..f5d3c2ecbd37 100644
--- a/nixos/modules/services/misc/taskserver/helper-tool.py
+++ b/nixos/modules/services/misc/taskserver/helper-tool.py
@@ -441,7 +441,31 @@ def cli(ctx):
             ctx.fail(msg.format(path))
 
 
-@cli.command("list-users")
+@cli.group("org")
+def org_cli():
+    """
+    Manage organisations
+    """
+    pass
+
+
+@cli.group("user")
+def user_cli():
+    """
+    Manage users
+    """
+    pass
+
+
+@cli.group("group")
+def group_cli():
+    """
+    Manage groups
+    """
+    pass
+
+
+@user_cli.command("list")
 @click.argument("organisation", type=ORGANISATION)
 def list_users(organisation):
     """
@@ -452,7 +476,18 @@ def list_users(organisation):
         sys.stdout.write(user.name + "\n")
 
 
-@cli.command("list-orgs")
+@group_cli.command("list")
+@click.argument("organisation", type=ORGANISATION)
+def list_groups(organisation):
+    """
+    List all users belonging to the specified organisation.
+    """
+    label("The following users exists for {}:".format(organisation.name))
+    for group in organisation.groups.values():
+        sys.stdout.write(group.name + "\n")
+
+
+@org_cli.command("list")
 def list_orgs():
     """
     List available organisations
@@ -462,7 +497,7 @@ def list_orgs():
         sys.stdout.write(org.name + "\n")
 
 
-@cli.command("get-uuid")
+@user_cli.command("getkey")
 @click.argument("organisation", type=ORGANISATION)
 @click.argument("user")
 def get_uuid(organisation, user):
@@ -478,7 +513,7 @@ def get_uuid(organisation, user):
     sys.stdout.write(user.key + "\n")
 
 
-@cli.command("export-user")
+@user_cli.command("export")
 @click.argument("organisation", type=ORGANISATION)
 @click.argument("user")
 def export_user(organisation, user):
@@ -496,7 +531,7 @@ def export_user(organisation, user):
     sys.stdout.write(userobj.export())
 
 
-@cli.command("add-org")
+@org_cli.command("add")
 @click.argument("name")
 def add_org(name):
     """
@@ -510,7 +545,7 @@ def add_org(name):
     mark_imperative(name)
 
 
-@cli.command("del-org")
+@org_cli.command("remove")
 @click.argument("name")
 def del_org(name):
     """
@@ -526,7 +561,7 @@ def del_org(name):
     click.echo(msg.format(name), err=True)
 
 
-@cli.command("add-user")
+@user_cli.command("add")
 @click.argument("organisation", type=ORGANISATION)
 @click.argument("user")
 def add_user(organisation, user):
@@ -545,7 +580,7 @@ def add_user(organisation, user):
         mark_imperative(organisation.name, "users", userobj.key)
 
 
-@cli.command("del-user")
+@user_cli.command("remove")
 @click.argument("organisation", type=ORGANISATION)
 @click.argument("user")
 def del_user(organisation, user):
@@ -561,7 +596,7 @@ def del_user(organisation, user):
     click.echo(msg.format(user), err=True)
 
 
-@cli.command("add-group")
+@group_cli.command("add")
 @click.argument("organisation", type=ORGANISATION)
 @click.argument("group")
 def add_group(organisation, group):
@@ -576,7 +611,7 @@ def add_group(organisation, group):
         mark_imperative(organisation.name, "groups", groupobj.name)
 
 
-@cli.command("del-group")
+@group_cli.command("remove")
 @click.argument("organisation", type=ORGANISATION)
 @click.argument("group")
 def del_group(organisation, group):
diff --git a/nixos/tests/taskserver.nix b/nixos/tests/taskserver.nix
index 0521f97431b3..d770b20a7757 100644
--- a/nixos/tests/taskserver.nix
+++ b/nixos/tests/taskserver.nix
@@ -44,7 +44,7 @@ import ./make-test.nix {
           );
 
           my $exportinfo = $server->succeed(
-            "nixos-taskserver export-user $org $user"
+            "nixos-taskserver user export $org $user"
           );
 
           $exportinfo =~ s/'/'\\'''/g;
@@ -74,10 +74,10 @@ import ./make-test.nix {
 
     sub readdImperativeUser {
       $server->nest("(re-)add imperative user bar", sub {
-        $server->execute("nixos-taskserver del-org imperativeOrg");
+        $server->execute("nixos-taskserver org remove imperativeOrg");
         $server->succeed(
-          "nixos-taskserver add-org imperativeOrg",
-          "nixos-taskserver add-user imperativeOrg bar"
+          "nixos-taskserver org add imperativeOrg",
+          "nixos-taskserver user add imperativeOrg bar"
         );
         setupClientsFor "imperativeOrg", "bar";
       });
@@ -109,9 +109,9 @@ import ./make-test.nix {
     $server->waitForUnit("taskserver.service");
 
     $server->succeed(
-      "nixos-taskserver list-users testOrganisation | grep -qxF alice",
-      "nixos-taskserver list-users testOrganisation | grep -qxF foo",
-      "nixos-taskserver list-users anotherOrganisation | grep -qxF bob"
+      "nixos-taskserver user list testOrganisation | grep -qxF alice",
+      "nixos-taskserver user list testOrganisation | grep -qxF foo",
+      "nixos-taskserver user list anotherOrganisation | grep -qxF bob"
     );
 
     $server->waitForOpenPort(${portStr});
@@ -125,7 +125,7 @@ import ./make-test.nix {
 
     testSync $_ for ("alice", "bob", "foo");
 
-    $server->fail("nixos-taskserver add-user imperativeOrg bar");
+    $server->fail("nixos-taskserver user add imperativeOrg bar");
     readdImperativeUser;
 
     testSync "bar";
@@ -133,7 +133,7 @@ import ./make-test.nix {
     subtest "checking certificate revocation of user bar", sub {
       $client1->succeed(checkClientCert "bar");
 
-      $server->succeed("nixos-taskserver del-user imperativeOrg bar");
+      $server->succeed("nixos-taskserver user remove imperativeOrg bar");
       restartServer;
 
       $client1->fail(checkClientCert "bar");
@@ -147,7 +147,7 @@ import ./make-test.nix {
     subtest "checking certificate revocation of org imperativeOrg", sub {
       $client1->succeed(checkClientCert "bar");
 
-      $server->succeed("nixos-taskserver del-org imperativeOrg");
+      $server->succeed("nixos-taskserver org remove imperativeOrg");
       restartServer;
 
       $client1->fail(checkClientCert "bar");
-- 
cgit 1.4.1