Started reworking skaldctl. Give room status along with room name.

distro-packages/Arch-Linux/skald-git/PKGBUILD

@@ -2,7 +2,7 @@
 # shellcheck shell=bash disable=SC2034,SC2154
 
 pkgname=skald-git
-pkgver=0.0.0.r1538.g8e265ed
+pkgver=0.0.0.r1539.g4f35f4b
 pkgrel=1
 pkgdesc='Audio-only hall-based conferencing server'
 arch=('x86_64' 'aarch64')

fork-inventory.md

@@ -65,9 +65,9 @@ Current runtime defaults and config paths:
 - `data/ice-servers.json`
 - `data/var/tokens.jsonl`
 - `halls/hallname.json`
-- `skaldctl.json`
 
-The old `groups/` and `galenectl.json` paths are not current runtime defaults.
+The old `groups/`, `galenectl.json`, and `skaldctl.json` paths are not current
+runtime defaults.
 Existing Galene installations should be treated as incompatible unless a later
 task explicitly adds a one-shot manual migration tool or documented manual
 conversion procedure.

hall/description.go

@@ -18,6 +18,13 @@ import (
 var ErrTagMismatch = errors.New("tag mismatch")
 var ErrDescriptionsNotWritable = &NotAuthorisedError{}
 var ErrUnknownPermission = errors.New("unknown permission")
+var localAdministration bool
+
+// SetLocalAdministration allows a local administration tool to update hall
+// files without enabling writes through the server's HTTP API.
+func SetLocalAdministration(enabled bool) {
+	localAdministration = enabled
+}
 
 type Permissions struct {
 	// non-empty for a named permissions set
@@ -338,6 +345,9 @@ func makeETag(fileSize int64, modTime time.Time) string {
 // DeleteDescription deletes a description (and therefore persistently
 // deletes a hall) but only if it matches a given ETag.
 func DeleteDescription(name, etag string) error {
+	if !validHallName(name) {
+		return UserError("illegal hall name")
+	}
 	halls.mu.Lock()
 	defer halls.mu.Unlock()
 
@@ -357,7 +367,22 @@ func UpdateDescription(name, etag string, desc *Description) error {
 	if desc.Users != nil || desc.WildcardUser != nil || desc.AuthKeys != nil {
 		return errors.New("description is not sanitised")
 	}
+	return updateDescription(name, etag, desc, true)
+}
 
+// WriteDescription writes a full hall description from a local administration
+// tool. It is not available to the server's HTTP API.
+func WriteDescription(name, etag string, desc *Description) error {
+	if !localAdministration {
+		return ErrDescriptionsNotWritable
+	}
+	return updateDescription(name, etag, desc, false)
+}
+
+func updateDescription(name, etag string, desc *Description, preserveSecrets bool) error {
+	if !validHallName(name) {
+		return UserError("illegal hall name")
+	}
 	halls.mu.Lock()
 	defer halls.mu.Unlock()
 
@@ -381,7 +406,7 @@ func UpdateDescription(name, etag string, desc *Description) error {
 	}
 
 	newdesc := *desc
-	if old != nil {
+	if old != nil && preserveSecrets {
 		newdesc.Users = old.Users
 		newdesc.WildcardUser = old.WildcardUser
 		newdesc.AuthKeys = old.AuthKeys
@@ -395,7 +420,7 @@ func rewriteDescriptionFile(filename string, desc *Description) error {
 	if err != nil {
 		return err
 	}
-	if !conf.WritableHalls {
+	if !conf.WritableHalls && !localAdministration {
 		return ErrDescriptionsNotWritable
 	}
 
@@ -657,9 +682,15 @@ func GetUserTag(hall, username string, wildcard bool) (string, error) {
 }
 
 func DeleteUser(hall, username string, wildcard bool, etag string) error {
+	if !validHallName(hall) {
+		return UserError("illegal hall name")
+	}
 	if wildcard && username != "" {
 		return errors.New("wildcard with username")
 	}
+	if !wildcard && !validUsername(username) {
+		return UserError("illegal username")
+	}
 
 	halls.mu.Lock()
 	defer halls.mu.Unlock()
@@ -698,9 +729,15 @@ func DeleteUser(hall, username string, wildcard bool, etag string) error {
 }
 
 func UpdateUser(hall, username string, wildcard bool, etag string, user *UserDescription) error {
+	if !validHallName(hall) {
+		return UserError("illegal hall name")
+	}
 	if wildcard && username != "" {
 		return errors.New("wildcard with username")
 	}
+	if !wildcard && !validUsername(username) {
+		return UserError("illegal username")
+	}
 	if user.Password.Type != "" || user.Password.Key != nil {
 		return errors.New("user description is not sanitised")
 	}
@@ -750,9 +787,15 @@ func UpdateUser(hall, username string, wildcard bool, etag string, user *UserDes
 }
 
 func SetUserPassword(hall, username string, wildcard bool, pw Password) error {
+	if !validHallName(hall) {
+		return UserError("illegal hall name")
+	}
 	if wildcard && username != "" {
 		return errors.New("wildcard with username")
 	}
+	if !wildcard && !validUsername(username) {
+		return UserError("illegal username")
+	}
 
 	halls.mu.Lock()
 	defer halls.mu.Unlock()

hall/description_test.go

@@ -263,6 +263,44 @@ func TestWritableHalls(t *testing.T) {
 	testUser(t, "", true)
 }
 
+func TestLocalAdministrationWritesFullDescription(t *testing.T) {
+	err := setupTest(t.TempDir(), t.TempDir(), false)
+	if err != nil {
+		t.Fatalf("setupTest: %v", err)
+	}
+	SetLocalAdministration(true)
+	defer SetLocalAdministration(false)
+
+	permissions, err := NewPermissions("op")
+	if err != nil {
+		t.Fatalf("NewPermissions: %v", err)
+	}
+	pw := "secret"
+	err = WriteDescription("test", "", &Description{
+		Autolock: true,
+		Users: map[string]UserDescription{
+			"Username": {
+				Password: Password{
+					Type: "plain",
+					Key:  &pw,
+				},
+				Permissions: permissions,
+			},
+		},
+	})
+	if err != nil {
+		t.Fatalf("WriteDescription: %v", err)
+	}
+
+	desc, err := GetDescription("test")
+	if err != nil {
+		t.Fatalf("GetDescription: %v", err)
+	}
+	if !desc.Autolock || desc.Users["Username"].Permissions.String() != "op" {
+		t.Fatalf("description = %#v", desc)
+	}
+}
+
 func testUser(t *testing.T, username string, wildcard bool) {
 	_, _, err := GetSanitisedUser("test", username, wildcard)
 	if !errors.Is(err, os.ErrNotExist) {

hall/hall.go

@@ -405,6 +405,11 @@ func validHallName(name string) bool {
 	return s == "/"+name
 }
 
+// ValidHallName reports whether name is safe to use as a hall name.
+func ValidHallName(name string) bool {
+	return validHallName(name)
+}
+
 func add(name string, desc *Description) (*Hall, []Client, error) {
 	if !validHallName(name) {
 		return nil, nil, UserError("illegal hall name")
@@ -1034,6 +1039,11 @@ func validUsername(username string) bool {
 	return username != "" && validHallName(username)
 }
 
+// ValidUsername reports whether username is safe to use as a login name.
+func ValidUsername(username string) bool {
+	return validUsername(username)
+}
+
 // called locked
 func (g *Hall) getPermission(creds ClientCredentials) (string, []string, error) {
 	desc := g.description
@@ -1097,6 +1107,7 @@ type Status struct {
 	AuthServer        string `json:"authServer,omitempty"`
 	AuthPortal        string `json:"authPortal,omitempty"`
 	Locked            bool   `json:"locked,omitempty"`
+	Recording         bool   `json:"recording,omitempty"`
 	ClientCount       *int   `json:"clientCount,omitempty"`
 	CanChangePassword bool   `json:"canChangePassword,omitempty"`
 }

rtpconn/webclient.go

@@ -82,6 +82,15 @@ func broadcastHallInfo(g *hall.Hall, message string) {
 	}
 }
 
+func hallRecording(g *hall.Hall) bool {
+	for _, c := range g.GetClients(nil) {
+		if _, ok := c.(*diskwriter.Client); ok {
+			return true
+		}
+	}
+	return false
+}
+
 func canEditChalkboard(perms []string) bool {
 	return member("op", perms) ||
 		member("admin", perms) ||
@@ -1193,6 +1202,7 @@ func handleAction(c *webClient, a any) error {
 			g = hall.Get(a.hall)
 			if g != nil {
 				s := g.Status(true, nil)
+				s.Recording = hallRecording(g)
 				status = &s
 				data = g.Data()
 			}
@@ -1272,6 +1282,7 @@ func handleAction(c *webClient, a any) error {
 		}
 		perms := append([]string(nil), c.permissions...)
 		status := g.Status(true, nil)
+		status.Recording = hallRecording(g)
 		username := c.username
 		c.write(clientMessage{
 			Type:             "joined",

rtpconn/webclient_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"testing"
 
+	"git.stormux.org/storm/skald/diskwriter"
 	"git.stormux.org/storm/skald/hall"
 	"git.stormux.org/storm/skald/token"
 	"git.stormux.org/storm/skald/unbounded"
@@ -56,6 +57,40 @@ func TestParseStatefulToken(t *testing.T) {
 	}
 }
 
+func TestHallRecording(t *testing.T) {
+	oldDirectory := hall.Directory
+	hall.Directory = t.TempDir()
+	defer func() {
+		hall.Directory = oldDirectory
+	}()
+	if err := os.WriteFile(
+		filepath.Join(hall.Directory, "recording-status-test.json"),
+		[]byte("{}"), 0o600,
+	); err != nil {
+		t.Fatalf("Write hall description: %v", err)
+	}
+
+	g, err := hall.Add("recording-status-test", nil)
+	if err != nil {
+		t.Fatalf("Add hall: %v", err)
+	}
+	defer hall.Delete(g.Name())
+
+	if hallRecording(g) {
+		t.Fatal("empty hall reported active recording")
+	}
+
+	disk := diskwriter.New(g)
+	if _, err := hall.AddClient(g.Name(), disk, hall.ClientCredentials{}); err != nil {
+		t.Fatalf("Add recording client: %v", err)
+	}
+	defer hall.DelClient(disk)
+
+	if !hallRecording(g) {
+		t.Fatal("hall did not report active recording")
+	}
+}
+
 func testWebClient(id string) *webClient {
 	return &webClient{
 		id:         id,

skald-install.md

@@ -103,10 +103,9 @@ WantedBy=multi-user.target
 
 ### Set up skaldctl
 
-There are two ways to administer a Skald instance: by manually editing
+The `skaldctl` utility administers a Skald instance by editing its JSON files
-JSON files on the server, or by using the `skaldctl` utility.
+locally.  Run it on the server as a user that may write the Skald `data/` and
-The `skaldctl` utility is recommended, since it avoids issues with
+`halls/` directories.  It does not connect to Skald over HTTPS.
-concurrent modifications and is less error-prone than the alternative.
 
 Build the `skaldctl` utility, and copy it somewhere on your path:
 
@@ -116,35 +115,36 @@ go build -ldflags='-s -w'
 sudo cp skaldctl /usr/local/bin/
 ```
 
-Now create an administrator password, and set up skaldctl:
+Create the initial server configuration file:
 
 ```sh
-skaldctl -admin-username admin initial-setup
+skaldctl initial-setup
 ```
 
-This command creates two files: `skaldctl.json` and `config.json`.  The
+If Skald uses directories other than `./data` and `./halls`, pass them before
-former is already at the right place, the latter must be copied to the
+the command name.  For example, packages that use `/var/lib/skald` can be
-server's `data/` directory:
+administered with:
 
 ```sh
-rsync config.json skald@skald.example.org:data/
+sudo -u skald skaldctl -data /var/lib/skald/data -halls /var/lib/skald/halls initial-setup
 ```
 
 ### Hall setup
 
-Create a hall:
+For a guided setup, create a hall interactively:
+
+```sh
+skaldctl create-hall -interactive
+```
+
+This asks for the hall name, common hall settings, operator accounts, and an
+optional shared participant password.  To create a minimal hall without
+prompts, use:
 
 ```sh
 skaldctl create-hall -hall city-watch
 ```
 
-If you didn't install a TLS certificate above, you will need to run
-`skaldctl` with the flag `-insecure`:
-
-```sh
-skaldctl -insecure create-hall -hall city-watch
-```
-
 Create an "op", a user with hall moderation privileges:
 
 ```sh

skald.md

@@ -162,9 +162,9 @@ fields are as follows:
    same syntax as user definitions in halls (see below), except that the
    only meaningful permission is `"admin"`;
 
- - `writableHalls`: if true, then the API used by `skaldctl` can be used
+ - `writableHalls`: if true, then the HTTP administrative API can be used
-   to modify hall definitions; if unset or false, then only read-only
+   to modify hall definitions; if unset or false, then the HTTP API is
-   access is allowed;
+   read-only.  This does not affect local `skaldctl` commands;
 
  - `recordingRetention`: if set, completed recording files older than this
    duration are automatically deleted from the recordings directory.  The
@@ -175,8 +175,8 @@ fields are as follows:
  - `allowOrigin` is an array containing the list of HTTP origins that
    are allowed to access the server;
 
- - `allowAdminOrigin` is like `allowOrigin`, but applies to the
+ - `allowAdminOrigin` is like `allowOrigin`, but applies to the HTTP
-   administrative API (the one used by `skaldctl`);
+   administrative API;
 
  - `proxyURL`: if running behind a reverse proxy, this specifies the root
    URL that will be visible outside the proxy;
@@ -191,6 +191,9 @@ fields are as follows:
 Halls are described by JSON files in the `halls/` directory.  These
 files are normally administered using the `skaldctl` utility, but may
 also be edited manually (there is no need to restart the server).
+`skaldctl` edits these files directly and must be run on the server as a
+user that may write the hall directory.  By default it uses `./data` and
+`./halls`; use the global `-data` and `-halls` options for other paths.
 
 ### Managing halls using `skaldctl`
 
@@ -202,6 +205,13 @@ A hall is created using `skaldctl create-hall`:
 skaldctl create-hall -hall city-watch
 ```
 
+For a guided setup that asks for the hall name, common settings, operator
+accounts, and an optional shared participant password, use:
+
+```sh
+skaldctl create-hall -interactive
+```
+
 There are a number of options to customise the behaviour of the hall, see
 `skaldctl create-hall -help` for a full list.  For example, in order to
 create a hall that allows unrestricted creation of tokens, say:
@@ -291,8 +301,8 @@ It is sometimes necessary to create a large number of identical halls.
 For example, the author has been using Skald to supervise computer
 science practicals, where up to 40 students are working in pairs.
 
-While it is possible to automate the creation of halls by accessing
+While it is possible to automate the creation of halls by scripting calls to
-Skald's API, by scripting calls to skaldctl, or by generating files
+`skaldctl`, by accessing Skald's HTTP administrative API, or by generating files
 directly under `halls/`, Skald provides a facility called *automatic
 subhalls* that can be used to generate halls on demand.
 
@@ -300,7 +310,7 @@ Automatic subhalls are enabled by setting the `"auto-subhalls"`
 field in the hall description:
 
 ```sh
-skaldctl create-hall unseen-university -auto-subhalls
+skaldctl create-hall -hall unseen-university -auto-subhalls
 ```
 
 Whenever a user attempts to access a subhall of `unseen-university`, for

skaldctl/skaldctl_test.go

@@ -1,7 +1,10 @@
 package main
 
 import (
+	"bufio"
+	"bytes"
 	"encoding/json"
+	"strings"
 	"testing"
 
 	"git.stormux.org/storm/skald/hall"
@@ -40,6 +43,58 @@ func TestMakePassword(t *testing.T) {
 	}
 }
 
+func TestInteractiveHall(t *testing.T) {
+	input := strings.NewReader(strings.Join([]string{
+		"city-watch",
+		"City Watch",
+		"An audio hall",
+		"yes",
+		"",
+		"",
+		"no",
+		"no",
+		"vetinari",
+		"",
+		"yes",
+		"",
+	}, "\n"))
+	passwords := []string{"secret", "secret", "guest", "guest"}
+	readPassword := func(string) (string, error) {
+		password := passwords[0]
+		passwords = passwords[1:]
+		return password, nil
+	}
+	var output bytes.Buffer
+	name, desc, err := interactiveHall(
+		bufio.NewReader(input), &output, readPassword, "",
+	)
+	if err != nil {
+		t.Fatalf("interactiveHall: %v", err)
+	}
+	if name != "city-watch" {
+		t.Fatalf("name = %q", name)
+	}
+	if desc.DisplayName != "City Watch" || desc.Description != "An audio hall" {
+		t.Fatalf("description = %#v", desc)
+	}
+	if !desc.Public || !desc.AllowRecording || !desc.Autolock {
+		t.Fatalf("common settings = %#v", desc)
+	}
+	user := desc.Users["vetinari"]
+	if ok, err := user.Password.Match("secret"); !ok || err != nil {
+		t.Fatalf("operator password match = %v, %v", ok, err)
+	}
+	if user.Permissions.String() != "op" {
+		t.Fatalf("operator permissions = %v", user.Permissions)
+	}
+	if desc.WildcardUser == nil {
+		t.Fatal("wildcard user is nil")
+	}
+	if ok, err := desc.WildcardUser.Password.Match("guest"); !ok || err != nil {
+		t.Fatalf("wildcard password match = %v, %v", ok, err)
+	}
+}
+
 func TestFormatPermissions(t *testing.T) {
 	tests := []struct{ j, v, p string }{
 		{`"op"`, "op", "[cmopt]"},

static/skald.js

@@ -43,6 +43,13 @@ let serverConnection;
  */
 let hallStatus = {};
 
+/**
+ * Whether this hall is currently being recorded.
+ *
+ * @type {boolean}
+ */
+let hallRecording = false;
+
 /**
  * True if we need to request a password.
  *
@@ -2023,7 +2030,13 @@ function capitalise(s) {
  * @param {string} title
  */
 function setTitle(title) {
-    let displayTitle = title ? `Hall: ${title}` : 'Skald';
+    let displayTitle = 'Skald';
+    if(title) {
+        let status = hallStatus.locked ? 'locked' : 'unlocked';
+        if(hallRecording)
+            status += ', recording';
+        displayTitle = `Hall: ${title} (${status})`;
+    }
     document.title = displayTitle;
     document.getElementById('title').textContent = displayTitle;
 }
@@ -2091,6 +2104,7 @@ async function gotJoined(kind, hall, perms, status, data, error, message) {
         document.location.href = message;
         return;
     case 'leave':
+        hallRecording = false;
         closeSafariStream();
         this.close();
         setButtonsVisibility();
@@ -2110,9 +2124,12 @@ async function gotJoined(kind, hall, perms, status, data, error, message) {
             token = null;
         }
         // don't discard endPoint and friends
+        hallStatus.locked = false;
+        hallStatus.recording = false;
         for(let key in status)
             hallStatus[key] = status[key];
-        setTitle((status && status.displayName) || capitalise(hall));
+        hallRecording = !!hallStatus.recording;
+        setTitle(hallStatus.displayName || capitalise(hall));
         setConnected(true);
         displayUsername();
         updateChalkboardAccess();
@@ -2392,9 +2409,15 @@ function gotUserMessage(id, dest, username, time, privileged, kind, error, messa
         }
         let text = '' + message;
         if(text === 'Recording started') {
+            hallRecording = true;
+            setTitle(hallStatus.displayName || capitalise(hall));
             announceUrgent(text);
             playNotificationSound('recording-started');
         } else {
+            if(text === 'Recording stopped') {
+                hallRecording = false;
+                setTitle(hallStatus.displayName || capitalise(hall));
+            }
             announceChat(text);
             if(text === 'Recording stopped')
                 playNotificationSound('recording-stopped');

todo.txt

@@ -154,3 +154,19 @@ X Verify completed files with ffprobe or equivalent in tests where available.
 - Verify sounds play after login in Firefox.
 - Verify sounds never replace screen-reader live-region announcements.
 - Tune sound levels and shapes with live users.
+
+# ============================================================================
+# PHASE 8: LOCAL SKALDCTL ADMINISTRATION
+# ============================================================================
+
+X Remove skaldctl's remote HTTPS administration path and operate directly on
+  the server's local data, hall, and token files.
+X Add `create-hall -interactive` for guided hall settings, operator accounts,
+  and an optional shared participant password.
+X Document that skaldctl runs on the Skald server with filesystem access.
+
+# ============================================================================
+# PHASE 9: HALL HEADING STATUS
+# ============================================================================
+
+X Show lock and recording state in the browser's hall heading.

webserver/api.go

@@ -180,7 +180,7 @@ func apiHallHandler(w http.ResponseWriter, r *http.Request, pth string) {
 	first, kind, rest := splitPath(pth)
 	g := ""
 	if first != "" {
-		g = first[1:]
+		g = strings.TrimSuffix(first[1:], "/")
 	}
 	if g == "" && kind == "" {
 		if apiCORS(w, r, "HEAD, GET") {

webserver/static_accessibility_test.go

@@ -60,3 +60,15 @@ func TestStaticLiveRegionAnnouncementsStayWired(t *testing.T) {
 	requireFunctionContains(t, js, "setUserStatus", "announceUrgent(`${username}: hand raised`)")
 	requireFunctionContains(t, js, "setUserStatus", "announceUrgent(`${username}: hand lowered`)")
 }
+
+func TestHallHeadingIncludesCurrentStatus(t *testing.T) {
+	js := readStaticFile(t, "skald.js")
+
+	requireFunctionContains(t, js, "setTitle", "hallStatus.locked ? 'locked' : 'unlocked'")
+	requireFunctionContains(t, js, "setTitle", "status += ', recording'")
+	requireFunctionContains(t, js, "gotJoined", "hallStatus.locked = false")
+	requireFunctionContains(t, js, "gotJoined", "hallStatus.recording = false")
+	requireFunctionContains(t, js, "gotJoined", "hallRecording = !!hallStatus.recording")
+	requireFunctionContains(t, js, "gotUserMessage", "hallRecording = true")
+	requireFunctionContains(t, js, "gotUserMessage", "hallRecording = false")
+}