Update a couple permissions things, no longer allow anonymous users.

hall/hall.go

@@ -560,6 +560,12 @@ func AddClient(hall string, c Client, creds ClientCredentials) (*Hall, error) {
 		c.SetUsername(username)
 		c.SetPermissions(perms)
 
+		for _, cc := range clients {
+			if !isSystemClient(cc) && cc.Username() == username {
+				return nil, UserError("username already in use")
+			}
+		}
+
 		if !member("op", perms) {
 			if g.locked != nil {
 				m := *g.locked
@@ -1005,7 +1011,7 @@ func (g *Hall) userExists(username string) bool {
 // usernames might lead to security vulnaribilities.
 // For now, we just do the minimal validation that avoids path traversal.
 func validUsername(username string) bool {
-	return username == "" || validHallName(username)
+	return username != "" && validHallName(username)
 }
 
 // called locked

hall/hall_test.go

@@ -238,6 +238,83 @@ func strPtr(s string) *string {
 	return &s
 }
 
+func TestEmptyUsernameIsRejected(t *testing.T) {
+	halls.halls = nil
+	oldDirectory := Directory
+	Directory = t.TempDir()
+	t.Cleanup(func() {
+		Directory = oldDirectory
+		halls.halls = nil
+	})
+
+	err := os.WriteFile(
+		filepath.Join(Directory, "empty-username-test.json"),
+		[]byte(`{
+			"wildcard-user": {"password": "secret", "permissions": "present"}
+		}`),
+		0600,
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = AddClient("empty-username-test", &testClient{id: "user1"}, ClientCredentials{
+		Username: strPtr(""),
+		Password: "secret",
+	})
+	if err == nil {
+		t.Fatal("empty username unexpectedly joined")
+	}
+}
+
+func TestDuplicateUsernameIsRejected(t *testing.T) {
+	halls.halls = nil
+	oldDirectory := Directory
+	Directory = t.TempDir()
+	t.Cleanup(func() {
+		Directory = oldDirectory
+		halls.halls = nil
+	})
+
+	err := os.WriteFile(
+		filepath.Join(Directory, "duplicate-username-test.json"),
+		[]byte(`{
+			"wildcard-user": {"password": "secret", "permissions": "present"}
+		}`),
+		0600,
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = AddClient("duplicate-username-test", &testClient{id: "user1"}, ClientCredentials{
+		Username: strPtr("bob1"),
+		Password: "secret",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = AddClient("duplicate-username-test", &testClient{id: "user2"}, ClientCredentials{
+		Username: strPtr("bob1"),
+		Password: "secret",
+	})
+	if err == nil {
+		t.Fatal("duplicate username unexpectedly joined")
+	}
+	if got, want := err.Error(), "username already in use"; got != want {
+		t.Fatalf("duplicate username error = %q, want %q", got, want)
+	}
+
+	_, err = AddClient("duplicate-username-test", &testClient{id: "user3"}, ClientCredentials{
+		Username: strPtr("bob2"),
+		Password: "secret",
+	})
+	if err != nil {
+		t.Fatalf("different username should join: %v", err)
+	}
+}
+
 func TestLockedHallJoinMessage(t *testing.T) {
 	halls.halls = nil
 	oldDirectory := Directory