Restore administrator roles that migrate_v016.py drops

Chased down why a migrated instance had no working administrator. The account authenticated fine and was refused every management call, and the cause is that migrate_v016.py assigns every migrated account the User role regardless of what it held before: an account that was `roles: ["admin"]` in v0.15 comes out the far side as `roles: {"@type": "User"}`. Ordinary users were never affected - User is what they had and what they get - and their credentials, mail and mailboxes survive untouched. It is specifically administrators who lose their privileges, which is a bad thing to discover after cutting over. The v0.16 shape came from the server's own schema document rather than the published reference: GET /api/schema defines x:UserRoles as a multi-variant type with variants User, Admin and Custom. Account is itself multi-variant, so an upsert needs its own "@type" too - without it the server rejects the operation outright ("upsert entry is missing `@type`"). applyplan.AccountRoleOperations restores roles from the principals dump, emitting operations only for accounts whose role actually changes. Rewriting every account would be a much larger blast radius for no benefit. Where v0.15 listed several roles, admin wins - under-privileging an administrator locks them out, which is the failure being fixed - and the collapse is reported rather than done silently, as are roles with no known v0.16 equivalent. Verified end to end on the smoke VM: rehearse against the real 0.15.5 put the role operation in the supplement, applying that supplement to a migrated 0.16.14 whose admin was broken restored management access (accounts=3), and alice and bob logged in over IMAPS with unchanged credentials, read their mail, and accepted new SMTP delivery. Also recorded: x:Account.domainId returns an internal id on v0.16, not a domain name, so the post-migration directory comparison would read every domain as missing. Resolving that needs an x:Domain/get call not yet confirmed against the binary.
2026-08-23 21:42:40 -07:00
parent 3e155fa42c
commit 28c0fa57cb
5 changed files with 294 additions and 9 deletions
@@ -0,0 +1,113 @@
+// SPDX-FileCopyrightText: 2026 LINUXexpert-org
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package applyplan
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+
+	"github.com/LINUXexpert-org/stalwart-migrator/internal/backup"
+)
+
+// AccountRoleOperations restores the account roles a v0.16 migration drops.
+//
+// migrate_v016.py assigns every migrated account the User role regardless
+// of what it had before. Verified on a real migration: an account holding
+// the admin role in v0.15 came out the other side with
+// `roles: {"@type": "User"}`, authenticated fine, and was refused every
+// management call with "forbidden". The instance had no working
+// administrator, which is a bad thing to discover after cutting over.
+//
+// Ordinary users are unaffected - User is what they had and what they get -
+// so this deliberately emits operations only for accounts whose role
+// actually changes. A plan that rewrote every account would be a much
+// larger blast radius for no benefit.
+//
+// The v0.16 shape comes from the server's own schema document
+// (GET /api/schema), where x:UserRoles is a multi-variant type with
+// variants User, Admin and Custom, and from confirming the upsert against
+// a live 0.16.14: Account is itself multi-variant, so the entry needs its
+// own "@type" as well.
+func AccountRoleOperations(principals []backup.Principal) (ops []Operation, covered []string, warnings []string, err error) {
+	type change struct {
+		name    string
+		variant string
+	}
+	var changes []change
+
+	for _, p := range principals {
+		if !strings.EqualFold(p.Type, "individual") {
+			continue // domains and groups don't carry these roles
+		}
+		variant, note := roleVariant(p.Roles)
+		if note != "" {
+			warnings = append(warnings, fmt.Sprintf("account %q: %s", p.Name, note))
+		}
+		if variant == "" || variant == "User" {
+			// User is the migration's own default; re-asserting it would
+			// touch every account to no effect.
+			continue
+		}
+		changes = append(changes, change{name: p.Name, variant: variant})
+	}
+	sort.Slice(changes, func(i, j int) bool { return changes[i].name < changes[j].name })
+
+	for _, c := range changes {
+		ops = append(ops, Operation{
+			Type:    "upsert",
+			Object:  "Account",
+			MatchOn: []string{"name"},
+			Value: map[string]map[string]any{
+				"role-" + c.name: {
+					// Account is a multi-variant object: without its own
+					// @type the upsert is rejected outright.
+					"@type": "User",
+					"name":  c.name,
+					"roles": map[string]any{"@type": c.variant},
+				},
+			},
+		})
+		covered = append(covered, "principal:"+c.name+":roles")
+	}
+	return ops, covered, warnings, nil
+}
+
+// roleVariant maps a v0.15 role list onto v0.16's x:UserRoles variant.
+//
+// v0.15 carries a list of role names; v0.16 carries one variant. Where an
+// account had several, admin wins - under-privileging an administrator
+// locks them out, which is the failure this function exists to prevent -
+// and the collapse is reported rather than done silently.
+func roleVariant(roles []string) (variant, note string) {
+	if len(roles) == 0 {
+		return "", ""
+	}
+	hasAdmin, hasUser := false, false
+	var unknown []string
+	for _, r := range roles {
+		switch strings.ToLower(strings.TrimSpace(r)) {
+		case "admin", "administrator":
+			hasAdmin = true
+		case "user":
+			hasUser = true
+		default:
+			unknown = append(unknown, r)
+		}
+	}
+	if len(unknown) > 0 {
+		note = fmt.Sprintf("role(s) %s have no known v0.16 equivalent and are not restored - recreate them by hand",
+			strings.Join(unknown, ", "))
+	}
+	switch {
+	case hasAdmin && (hasUser || len(unknown) > 0):
+		return "Admin", note + " (collapsed several roles to Admin)"
+	case hasAdmin:
+		return "Admin", note
+	case hasUser:
+		return "User", note
+	default:
+		return "", note
+	}
+}
@@ -0,0 +1,113 @@
+// SPDX-FileCopyrightText: 2026 LINUXexpert-org
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package applyplan
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/LINUXexpert-org/stalwart-migrator/internal/backup"
+)
+
+// The exact principal shape a real 0.15.5 reports.
+var migratedPrincipals = []backup.Principal{
+	{ID: 4, Type: "individual", Name: "alice", Emails: []string{"[email protected]"}, Roles: []string{"user"}},
+	{ID: 5, Type: "individual", Name: "bob", Emails: []string{"[email protected]"}, Roles: []string{"user"}},
+	{ID: 6, Type: "individual", Name: "sysadmin", Emails: []string{"[email protected]"}, Roles: []string{"admin"}},
+	{ID: 1, Type: "domain", Name: "smoke.test"},
+}
+
+// The failure this exists to prevent: after a real migration the admin
+// account authenticated fine and was refused every management call, because
+// migrate_v016.py had given it the User role like everyone else.
+func TestAccountRolesRestoresTheAdministrator(t *testing.T) {
+	ops, covered, warnings, err := AccountRoleOperations(migratedPrincipals)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(warnings) != 0 {
+		t.Errorf("unexpected warnings: %v", warnings)
+	}
+	if len(ops) != 1 {
+		t.Fatalf("generated %d op(s), want 1 - only the admin's role actually changes", len(ops))
+	}
+	v := ops[0].Value["role-sysadmin"]
+	if v == nil {
+		t.Fatalf("no operation for sysadmin: %+v", ops[0].Value)
+	}
+	// Account is multi-variant; without its own @type the upsert is
+	// rejected outright by the server.
+	if v["@type"] != "User" {
+		t.Errorf("@type = %v, want User (the Account variant)", v["@type"])
+	}
+	roles, ok := v["roles"].(map[string]any)
+	if !ok || roles["@type"] != "Admin" {
+		t.Errorf("roles = %v, want {\"@type\": \"Admin\"}", v["roles"])
+	}
+	if len(covered) != 1 {
+		t.Errorf("covered = %v, want one entry", covered)
+	}
+}
+
+// Ordinary users already get User from the migration. Rewriting every
+// account would be a far larger blast radius for no benefit.
+func TestAccountRolesLeavesOrdinaryUsersAlone(t *testing.T) {
+	ops, _, _, err := AccountRoleOperations([]backup.Principal{
+		{Type: "individual", Name: "alice", Roles: []string{"user"}},
+		{Type: "individual", Name: "bob", Roles: []string{}},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(ops) != 0 {
+		t.Errorf("generated %d op(s) for plain users, want 0", len(ops))
+	}
+}
+
+func TestAccountRolesSkipsNonIndividuals(t *testing.T) {
+	ops, _, _, err := AccountRoleOperations([]backup.Principal{
+		{Type: "domain", Name: "smoke.test", Roles: []string{"admin"}},
+		{Type: "group", Name: "staff", Roles: []string{"admin"}},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(ops) != 0 {
+		t.Errorf("generated %d op(s) for non-individual principals, want 0", len(ops))
+	}
+}
+
+// v0.15 carries a list, v0.16 one variant. Under-privileging an
+// administrator locks them out, so admin wins - and the collapse is
+// reported, not silent.
+func TestAccountRolesCollapsesMultipleRolesToAdminAndSaysSo(t *testing.T) {
+	ops, _, warnings, err := AccountRoleOperations([]backup.Principal{
+		{Type: "individual", Name: "boss", Roles: []string{"user", "admin"}},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(ops) != 1 {
+		t.Fatalf("generated %d op(s), want 1", len(ops))
+	}
+	roles := ops[0].Value["role-boss"]["roles"].(map[string]any)
+	if roles["@type"] != "Admin" {
+		t.Errorf("roles = %v, want Admin to win", roles)
+	}
+	if len(warnings) != 1 || !strings.Contains(warnings[0], "collapsed") {
+		t.Errorf("warnings = %v, want the collapse reported", warnings)
+	}
+}
+
+func TestAccountRolesReportsRolesItCannotMap(t *testing.T) {
+	_, _, warnings, err := AccountRoleOperations([]backup.Principal{
+		{Type: "individual", Name: "auditor", Roles: []string{"compliance-reviewer"}},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(warnings) != 1 || !strings.Contains(warnings[0], "compliance-reviewer") {
+		t.Errorf("warnings = %v, want the unmappable role named", warnings)
+	}
+}
@@ -312,3 +312,28 @@ func ReadUnmigratedKeys(reportPath string, settings map[string]string) (map[stri
 	}
 	return keys, nil
 }
+
+// Principal is the subset of a v0.15 principals dump this tool needs. The
+// shape is what the REST management API returns and what
+// migrate_v016.py's dump step writes out verbatim.
+type Principal struct {
+	ID     int      `json:"id"`
+	Type   string   `json:"type"` // "individual", "group", "domain", ...
+	Name   string   `json:"name"`
+	Emails []string `json:"emails"`
+	Roles  []string `json:"roles"`
+}
+
+// ReadPrincipalsDump loads the principals dump written alongside the
+// settings dump.
+func ReadPrincipalsDump(path string) ([]Principal, error) {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return nil, fmt.Errorf("backup: read principals dump %s: %w", path, err)
+	}
+	var principals []Principal
+	if err := json.Unmarshal(data, &principals); err != nil {
+		return nil, fmt.Errorf("backup: parse principals dump %s: %w", path, err)
+	}
+	return principals, nil
+}