fix: r148 on the core: a module is withdrawn, never uninstalled; tombstones hold their principal

libraries/authority-core/go/authority/lifecycle.go

@@ -27,10 +27,20 @@ func (s *Service) ApplyModule(ctx context.Context, m domain.Module) ([]string, e
 		if err != nil && !errors.Is(err, domain.ErrNotFound) {
 			return err
 		}
+		withdrawn := false
 		if !exists {
 			// A module withdrawn before keeps its principal and its last
-			// generation: a redelivery never renames it or rewinds it.
+			// generation: a redelivery never renames it or rewinds it. Its
+			// principal stays its own: no other module claims it. A tombstone
+			// is never cleared; a module that must change principal is
+			// delivered under a new name.
+			if tomb, err := tx.TombstoneByPrincipal(m.Principal); err == nil && tomb.Name != m.Name {
+				return domain.Refuse("principal %q is withdrawn %s's for life; %s cannot claim it", m.Principal, tomb.Name, m.Name)
+			} else if err != nil && !errors.Is(err, domain.ErrNotFound) {
+				return err
+			}
 			tomb, err := tx.Tombstone(m.Name)
+			withdrawn = err == nil
 			switch {
 			case err == nil && tomb.Principal != m.Principal:
 				return domain.Refuse("%s is principal %q for life; a delivery cannot rename it %q", m.Name, tomb.Principal, m.Principal)
@@ -102,9 +112,12 @@ func (s *Service) ApplyModule(ctx context.Context, m domain.Module) ([]string, e
 				}
 			case err != nil:
 				return err
-			case inst.Status == domain.Revoked:
+			case inst.Status == domain.Revoked && withdrawn:
 				// A module withdrawn and delivered again is installed again,
-				// at a revision above every capability sealed before.
+				// at a revision above every capability sealed before. Only a
+				// withdrawal is undone this way: an installation an
+				// administrator uninstalled stays uninstalled (Uninstall
+				// refuses a module's anyway).
 				inst.Status, inst.Revision = domain.Active, inst.Revision+1
 				if err := tx.PutInstallation(inst); err != nil {
 					return err
@@ -216,6 +229,9 @@ func (s *Service) Install(ctx context.Context, caller *Person, org, target, agen
 		if other, err := tx.InstallationByPrincipal(principal); err == nil && other.ID != id {
 			return domain.Refuse("the principal %q of %s at %s is another installation's", principal, target, org)
 		}
+		if _, err := tx.TombstoneByPrincipal(principal); err == nil {
+			return domain.Refuse("the principal %q of %s at %s is a withdrawn module's", principal, target, org)
+		}
 		inst = domain.Installation{ID: id, Organization: org, Target: target, Status: domain.Pending, Revision: existing.Revision + 1,
 			AgentRole: agentRole, PrincipalID: principal, InstalledBy: installedBy}
 		if err := tx.DeleteRoleGrants(id); err != nil {
@@ -256,8 +272,14 @@ func (s *Service) Install(ctx context.Context, caller *Person, org, target, agen
 // absent one.
 func (s *Service) Uninstall(ctx context.Context, caller *Person, organization, installationID string) error {
 	return s.store.Write(ctx, func(tx Tx) error {
-		if caller != nil && callerOrganization(tx, *caller) != organization {
-			return decide.ForeignInstallation(installationID, organization)
+		// Anyone but a current administrator of the organization named learns
+		// nothing of its installations: the foreign answer comes before any
+		// installation is read.
+		if caller != nil {
+			p, err := tx.Person(caller.ID)
+			if err != nil || p.Organization != organization || !decide.IsAdministrator(p) {
+				return decide.ForeignInstallation(installationID, organization)
+			}
 		}
 		inst, err := tx.Installation(installationID)
 		if errors.Is(err, domain.ErrNotFound) || (err == nil && inst.Organization != organization) {
@@ -266,11 +288,12 @@ func (s *Service) Uninstall(ctx context.Context, caller *Person, organization, i
 		if err != nil {
 			return err
 		}
-		if caller != nil {
-			p, err := tx.Person(caller.ID)
-			if err != nil || p.Organization != organization || !decide.IsAdministrator(p) {
-				return domain.Refuse("installation administration requires an administrator or owner of organization %q", organization)
-			}
+		// A module is installed by composition: no person uninstalls it. The
+		// host's own setup (no caller) may retire it at one organization; it
+		// stays retired, since only a withdrawal and a redelivery reactivate
+		// a module's installations.
+		if m, err := tx.ModuleNamed(inst.Target); caller != nil && err == nil && m.Kind != domain.KindSolution {
+			return domain.Refuse("%s is a module, installed by composition: it is withdrawn, never uninstalled", inst.Target)
 		}
 		return s.revoke(tx, inst)
 	})

libraries/authority-core/go/authority/r148_test.go

@@ -0,0 +1,130 @@
+package authority_test
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/codefly-dev/authority/libraries/authority-core/go/authority"
+	"github.com/codefly-dev/authority/libraries/authority-core/go/domain"
+	basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0"
+)
+
+func storeModule(generation uint64) domain.Module {
+	return domain.Module{Name: "store", Principal: "store", Kind: domain.KindModule, Generation: generation, OwnerAware: true,
+		ResourceKinds: []*basev0.WorkScopeV1{sc(kind, "admin", "create", "read", "write")}}
+}
+
+// M1: a module's installation is never uninstalled by an administrator (it is
+// withdrawn); a routine delivery reactivates nothing an administrator cut.
+func TestAModuleInstallationIsNeverUninstalled(t *testing.T) {
+	h := newHarness(t)
+	alex := h.admin(acme)
+	refused(t, h.svc.Uninstall(h.ctx, &alex, acme, h.inst(acme, "store").ID), "never uninstalled")
+	if err := h.svc.Uninstall(h.ctx, &alex, acme, h.inst(acme, "a").ID); err != nil {
+		t.Fatal(err)
+	}
+	// A routine delivery of a solution leaves an uninstalled installation as it is.
+	a := solution("a", true)
+	a.Generation = 2
+	if _, err := h.svc.ApplyModule(h.ctx, a); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := h.svc.Seals().Seal(h.ctx, "maya", h.inst(acme, "a").ID); err == nil {
+		t.Fatal("a delivery reactivated an uninstalled installation")
+	}
+	// The host's own setup may retire a module at one organization; a
+	// routine delivery leaves it retired.
+	if err := h.svc.Uninstall(h.ctx, nil, acme, h.inst(acme, "store").ID); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := h.svc.ApplyModule(h.ctx, storeModule(2)); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := h.svc.Seals().Seal(h.ctx, "maya", h.inst(acme, "store").ID); err == nil {
+		t.Fatal("a delivery reactivated a module the host retired")
+	}
+}
+
+// m1: a withdrawn module's principal is its own for life: no other module
+// claims it, and no installation is given it.
+func TestATombstonedPrincipalIsNeverClaimed(t *testing.T) {
+	h := newHarness(t)
+	if err := h.svc.WithdrawModule(h.ctx, "store"); err != nil {
+		t.Fatal(err)
+	}
+	evil := storeModule(1)
+	evil.Name = "evil"
+	_, err := h.svc.ApplyModule(h.ctx, evil)
+	code(t, err, domain.Denied)
+	if _, err := h.svc.ApplyModule(h.ctx, storeModule(2)); err != nil {
+		t.Fatalf("store's own redelivery: %v", err)
+	}
+	if err := h.svc.WithdrawModule(h.ctx, "store"); err != nil {
+		t.Fatal(err)
+	}
+	cfg := h.cfg
+	cfg.PrincipalID = func(string, string) string { return "store" }
+	svc, err := authority.New(h.store, cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	admin := h.admin(globex)
+	_, err = svc.Install(h.ctx, &admin, globex, "b", "author", admin.ID, nil)
+	code(t, err, domain.Denied)
+}
+
+// m2: a caller the store no longer knows, and a current member who
+// administers nothing, cannot tell an installation from an absent one.
+func TestNoOneButAnAdministratorTellsPresentFromAbsent(t *testing.T) {
+	h := newHarness(t)
+	present, absent := h.inst(acme, "a").ID, authority.StableInstallationID(acme, "never-installed")
+	remi := h.person("remi")
+	maya := h.person("maya")
+	h.store.DeletePerson("maya")
+	for _, id := range []string{present, absent} {
+		want := "installation " + id + " belongs to another organization than \"acme\""
+		for name, err := range map[string]error{
+			"a member uninstalling": h.svc.Uninstall(h.ctx, &remi, acme, id),
+			"a deleted ex-member":   h.svc.Uninstall(h.ctx, &maya, acme, id),
+			"a deleted ex-member listing": func() error {
+				_, err := h.svc.ListRoleGrants(h.ctx, maya, acme, id)
+				return err
+			}(),
+		} {
+			if err == nil || err.Error() != want {
+				t.Fatalf("%s on %s: %v", name, id, err)
+			}
+		}
+	}
+}
+
+type outage struct{ authority.Store }
+
+type outageTx struct{ authority.Tx }
+
+func (outageTx) GrantEpoch(string, string) (uint64, error) {
+	return 0, errors.New("store unavailable")
+}
+
+func (o outage) Read(ctx context.Context, fn func(authority.Tx) error) error {
+	return o.Store.Read(ctx, func(tx authority.Tx) error { return fn(outageTx{tx}) })
+}
+
+// m3: a store failure during issuance is reported as itself, not as a
+// refusal.
+func TestAnIssueReportsAStoreOutage(t *testing.T) {
+	h := newHarness(t)
+	h.grant("a", "viewer", "maya")
+	svc, err := authority.New(outage{h.store}, h.cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = svc.IssueForPerson(h.ctx, h.person("maya"), authority.IssueRequest{Audience: "a"})
+	if err == nil {
+		t.Fatal("issued through an outage")
+	}
+	if _, refusal := domain.CodeOf(err); refusal {
+		t.Fatalf("an outage reads as a refusal: %v", err)
+	}
+}

libraries/authority-core/go/authority/roles.go

@@ -136,10 +136,12 @@ func (s *Service) PersonChanged(ctx context.Context, person string) error {
 }
 
 // callerOrganization is the organization the caller belongs to now, read from
-// the store; a caller the store no longer knows keeps the session's.
+// the store.
 func callerOrganization(tx Tx, caller Person) string {
 	if p, err := tx.Person(caller.ID); err == nil {
 		return p.Organization
 	}
-	return caller.Organization
+	// A caller the store no longer knows belongs to no organization: they get
+	// the foreign answer, whatever their session says.
+	return ""
 }

libraries/authority-core/go/authority/service.go

@@ -272,9 +272,12 @@ func (src sources) fenceIn(tx Tx, organization, owner, installationID string) (f
 		return fence{}, err
 	}
 	seal, err := src.sealIn(tx, owner, installationID)
-	if err != nil {
+	if errors.Is(err, workcontext.ErrNoSeal) {
 		return fence{}, domain.Refuse("%s may not use installation %s now", owner, installationID)
 	}
+	if err != nil {
+		return fence{}, err // the store's own failure, named as such
+	}
 	epoch, err := src.epochIn(tx, owner)
 	if err != nil {
 		return fence{}, err

libraries/authority-core/go/authority/sources_test.go

@@ -65,7 +65,8 @@ func TestAnUninstallIsTheOrganizationsAdministrators(t *testing.T) {
 	h := newHarness(t)
 	id := h.inst(acme, "a").ID
 	maya, zoe := h.person("maya"), h.person("zoe")
-	refused(t, h.svc.Uninstall(h.ctx, &maya, acme, id), "administrator")
+	// A member who administers nothing learns nothing either (r148 m2).
+	refused(t, h.svc.Uninstall(h.ctx, &maya, acme, id), "belongs to another organization")
 	foreign := h.svc.Uninstall(h.ctx, &zoe, globex, id)
 	absent := h.svc.Uninstall(h.ctx, &zoe, globex, "no-such-installation")
 	code(t, foreign, domain.Denied)

libraries/authority-core/go/authority/store.go

@@ -37,8 +37,10 @@ type Tx interface {
 	Modules() ([]domain.Module, error)
 	PutModule(domain.Module) error
 	DeleteModule(name string) error
-	// Tombstone answers what stays of a withdrawn module, by name.
+	// Tombstone answers what stays of a withdrawn module, by name, and
+	// TombstoneByPrincipal by its principal. A tombstone is never removed.
 	Tombstone(name string) (domain.Tombstone, error)
+	TombstoneByPrincipal(principal string) (domain.Tombstone, error)
 	PutTombstone(domain.Tombstone) error
 
 	Installation(id string) (domain.Installation, error)

libraries/authority-core/go/domain/domain.go

@@ -78,7 +78,9 @@ type MethodPolicy struct {
 }
 
 // Tombstone is what stays of a withdrawn module: its principal, which it
-// keeps for life, and the generation a redelivery must exceed.
+// keeps for life and no other module or installation may take, and the
+// generation a redelivery must exceed. A tombstone is never cleared: a module
+// that must change principal is delivered under a new name.
 type Tombstone struct {
 	Name, Principal string
 	Generation      uint64

libraries/authority-core/go/memstore/memstore.go

@@ -233,6 +233,15 @@ func (t tx) Tombstone(name string) (domain.Tombstone, error) {
 	return tomb, nil
 }
 
+func (t tx) TombstoneByPrincipal(principal string) (domain.Tombstone, error) {
+	for _, tomb := range t.s.tombstones {
+		if tomb.Principal == principal {
+			return tomb, nil
+		}
+	}
+	return domain.Tombstone{}, missing("tombstone of principal", principal)
+}
+
 func (t tx) PutTombstone(tomb domain.Tombstone) error {
 	t.mustWrite()
 	t.s.tombstones[tomb.Name] = tomb