Guard customer-owned profile and membership transactions

Signed-off-by: Cole Speelman <crspeelman@gmail.com>
2026-09-05 00:33:24 -04:00
parent f142ac23a9
commit d476179148
12 changed files with 369 additions and 10 deletions
@@ -408,6 +408,17 @@ func (store *Store) OrganizationByID(ctx context.Context, organizationID string)
 }

 func (store *Store) UpdateOrganization(ctx context.Context, value organizations.Organization, expectedRevision int64, audit organizations.AuditEvent) error {
+	return store.updateOrganization(ctx, value, expectedRevision, "", audit)
+}
+
+func (store *Store) UpdateOwnedOrganization(ctx context.Context, value organizations.Organization, expectedRevision int64, ownerRole string, audit organizations.AuditEvent) error {
+	if !safeName(ownerRole) || value.Personal || value.Status != "active" || audit.Action != "organization.update" || audit.ResourceType != "organization" || audit.ResourceID != value.ID {
+		return errors.New("authsqlite: invalid owner-managed organization update")
+	}
+	return store.updateOrganization(ctx, value, expectedRevision, ownerRole, audit)
+}
+
+func (store *Store) updateOrganization(ctx context.Context, value organizations.Organization, expectedRevision int64, ownerRole string, audit organizations.AuditEvent) error {
 	if !validOrganization(value) || expectedRevision < 1 || value.Revision != expectedRevision+1 || !validOrganizationAudit(audit, value.ID) {
 		return errors.New("authsqlite: invalid organization update")
 	}
@@ -416,6 +427,11 @@ func (store *Store) UpdateOrganization(ctx context.Context, value organizations.
 		return err
 	}
 	defer tx.Rollback()
+	if ownerRole != "" {
+		if err = lockOrganizationOwner(ctx, tx, value.ID, audit.ActorUserID, ownerRole); err != nil {
+			return err
+		}
+	}
 	result, err := tx.ExecContext(ctx, `UPDATE gwf_organizations SET slug=?,name=?,status=?,revision=?,updated_at=? WHERE id=? AND revision=?`, value.Slug, value.Name, value.Status, value.Revision, value.UpdatedAt.Unix(), value.ID, expectedRevision)
 	if err != nil {
 		return err
@@ -535,6 +551,14 @@ func (store *Store) SetMembershipStatus(ctx context.Context, organizationID, use
 }

 func (store *Store) ChangeMembershipStatus(ctx context.Context, input organizations.MembershipStatusChange, ownerRole string, audit organizations.AuditEvent) error {
+	return store.changeMembershipStatus(ctx, input, ownerRole, audit, false)
+}
+
+func (store *Store) ChangeOwnedMembershipStatus(ctx context.Context, input organizations.MembershipStatusChange, ownerRole string, audit organizations.AuditEvent) error {
+	return store.changeMembershipStatus(ctx, input, ownerRole, audit, true)
+}
+
+func (store *Store) changeMembershipStatus(ctx context.Context, input organizations.MembershipStatusChange, ownerRole string, audit organizations.AuditEvent, requireOwner bool) error {
 	if !validMembershipStatusChange(input, ownerRole, audit) {
 		return organizations.ErrMembershipNotFound
 	}
@@ -543,7 +567,12 @@ func (store *Store) ChangeMembershipStatus(ctx context.Context, input organizati
 		return err
 	}
 	defer tx.Rollback()
-	if err = lockActiveMembershipActor(ctx, tx, input.OrganizationID, input.ActorUserID); err != nil {
+	if requireOwner {
+		err = lockOrganizationOwner(ctx, tx, input.OrganizationID, input.ActorUserID, ownerRole)
+	} else {
+		err = lockActiveMembershipActor(ctx, tx, input.OrganizationID, input.ActorUserID)
+	}
+	if err != nil {
 		return err
 	}
 	current, err := membershipStatus(ctx, tx, input.OrganizationID, input.UserID)
@@ -620,6 +649,14 @@ func (store *Store) RemoveMembership(ctx context.Context, organizationID, userID
 }

 func (store *Store) RemoveMembershipIfCurrent(ctx context.Context, input organizations.MembershipRemoval, ownerRole string, audit organizations.AuditEvent) error {
+	return store.removeMembershipIfCurrent(ctx, input, ownerRole, audit, false)
+}
+
+func (store *Store) RemoveOwnedMembershipIfCurrent(ctx context.Context, input organizations.MembershipRemoval, ownerRole string, audit organizations.AuditEvent) error {
+	return store.removeMembershipIfCurrent(ctx, input, ownerRole, audit, true)
+}
+
+func (store *Store) removeMembershipIfCurrent(ctx context.Context, input organizations.MembershipRemoval, ownerRole string, audit organizations.AuditEvent, requireOwner bool) error {
 	if !validMembershipRemoval(input, ownerRole, audit) {
 		return organizations.ErrMembershipNotFound
 	}
@@ -628,7 +665,12 @@ func (store *Store) RemoveMembershipIfCurrent(ctx context.Context, input organiz
 		return err
 	}
 	defer tx.Rollback()
-	if err = lockActiveMembershipActor(ctx, tx, input.OrganizationID, input.ActorUserID); err != nil {
+	if requireOwner {
+		err = lockOrganizationOwner(ctx, tx, input.OrganizationID, input.ActorUserID, ownerRole)
+	} else {
+		err = lockActiveMembershipActor(ctx, tx, input.OrganizationID, input.ActorUserID)
+	}
+	if err != nil {
 		return err
 	}
 	current, err := membershipStatus(ctx, tx, input.OrganizationID, input.UserID)
@@ -691,6 +733,27 @@ func lockActiveMembershipActor(ctx context.Context, tx *sql.Tx, organizationID,
 	return nil
 }

+func lockOrganizationOwner(ctx context.Context, tx *sql.Tx, organizationID, actorUserID, ownerRole string) error {
+	if err := lockActiveMembershipActor(ctx, tx, organizationID, actorUserID); err != nil {
+		return err
+	}
+	var personal bool
+	if err := tx.QueryRowContext(ctx, `SELECT personal FROM gwf_organizations WHERE id=?`, organizationID).Scan(&personal); err != nil {
+		return err
+	}
+	if personal {
+		return organizations.ErrPersonalOrganization
+	}
+	owner, err := hasDirectOwnerRole(ctx, tx, organizationID, actorUserID, ownerRole)
+	if err != nil {
+		return err
+	}
+	if !owner {
+		return organizations.ErrOwnerAuthority
+	}
+	return nil
+}
+
 func membershipStatus(ctx context.Context, tx *sql.Tx, organizationID, userID string) (string, error) {
 	var status string
 	if err := tx.QueryRowContext(ctx, `SELECT status FROM gwf_organization_memberships WHERE organization_id=? AND user_id=?`, organizationID, userID).Scan(&status); err != nil {
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: MPL-2.0
+
+package authsqlite
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"gamertan.com/web/organizations"
+)
+
+type ownedOperation struct {
+	name, action string
+	apply        func(context.Context, roleSetFixture, string, string) error
+}
+
+func ownedOperations() []ownedOperation {
+	return []ownedOperation{
+		{"profile", "organization.update", func(ctx context.Context, f roleSetFixture, actor, _ string) error {
+			_, err := f.organizations.UpdateOwnedOrganization(ctx, organizations.UpdateOrganization{ID: f.org.ID, Slug: "updated-business", Name: "Updated business", ActorUserID: actor, ExpectedRevision: 1, RequestID: "request-profile"})
+			return err
+		}},
+		{"suspend", "membership.suspended", func(ctx context.Context, f roleSetFixture, actor, target string) error {
+			return f.organizations.ChangeOwnedMembershipStatus(ctx, organizations.MembershipStatusChange{OrganizationID: f.org.ID, UserID: target, ActorUserID: actor, ExpectedStatus: "active", Status: "suspended", RequestID: "request-status"})
+		}},
+		{"remove", "membership.remove", func(ctx context.Context, f roleSetFixture, actor, target string) error {
+			return f.organizations.RemoveOwnedMembershipIfCurrent(ctx, organizations.MembershipRemoval{OrganizationID: f.org.ID, UserID: target, ActorUserID: actor, ExpectedStatus: "active", RequestID: "request-remove"})
+		}},
+	}
+}
+
+func TestOwnedManagementRechecksActorInWriteTransaction(t *testing.T) {
+	for _, operation := range ownedOperations() {
+		for _, change := range []struct{ name, sql string }{
+			{"role revoked", `UPDATE gwf_access_bindings SET revoked_at=2100 WHERE subject_id='customer-12345'`},
+			{"role narrowed", `UPDATE gwf_access_bindings SET project_id=(SELECT id FROM gwf_projects LIMIT 1) WHERE subject_id='customer-12345'`},
+			{"actor suspended", `UPDATE gwf_organization_memberships SET status='suspended' WHERE user_id='customer-12345'`},
+			{"actor removed", `DELETE FROM gwf_organization_memberships WHERE user_id='customer-12345'`},
+			{"account disabled", `UPDATE gwf_users SET status='disabled' WHERE id='customer-12345'`},
+			{"registration incomplete", `UPDATE gwf_users SET registration_pending=1 WHERE id='customer-12345'`},
+			{"organization archived", `UPDATE gwf_organizations SET status='archived'`},
+			{"personal organization", `UPDATE gwf_organizations SET personal=1,personal_owner_user_id='customer-12345'`},
+		} {
+			t.Run(operation.name+"/"+change.name, func(t *testing.T) {
+				f := newRoleSetFixture(t)
+				f.addMember(t)
+				if change.name == "role narrowed" {
+					if _, err := f.organizations.CreateProject(t.Context(), organizations.CreateProject{OrganizationID: f.org.ID, Slug: "project", Name: "Project"}); err != nil {
+						t.Fatal(err)
+					}
+				}
+				// Model a change committed after the caller displayed/authorized the
+				// operation. The repository must not rely on that earlier decision.
+				if _, err := f.store.db.Exec(change.sql); err != nil {
+					t.Fatal(err)
+				}
+				if err := operation.apply(t.Context(), f, roleOwner, roleMember); err == nil {
+					t.Fatal("stale owner authority accepted")
+				}
+				assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_access_audit_events WHERE action=?`, operation.action, 0)
+				assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_organization_memberships WHERE user_id=? AND status='active'`, roleMember, 1)
+				assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_organizations WHERE id=? AND revision=1`, f.org.ID, 1)
+			})
+		}
+	}
+}
+
+func TestOwnedManagementDoesNotInheritDelegatedAdministratorSemantics(t *testing.T) {
+	for _, operation := range ownedOperations() {
+		t.Run(operation.name, func(t *testing.T) {
+			f := newRoleSetFixture(t)
+			f.addMember(t)
+			if err := operation.apply(t.Context(), f, roleMember, roleMember); !errors.Is(err, organizations.ErrOwnerAuthority) {
+				t.Fatalf("non-owner management: %v", err)
+			}
+			if err := operation.apply(t.Context(), f, roleOwner, roleMember); err != nil {
+				t.Fatal(err)
+			}
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_access_audit_events WHERE action=?`, operation.action, 1)
+			if err := operation.apply(t.Context(), f, roleOwner, roleMember); err == nil {
+				t.Fatal("replayed mutation accepted")
+			}
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_access_audit_events WHERE action=?`, operation.action, 1)
+		})
+	}
+	// Legacy callers still authorize non-owner administrative operations in
+	// their application policy; the new explicit methods do not alter that API.
+	f := newRoleSetFixture(t)
+	f.addMember(t)
+	err := f.organizations.ChangeMembershipStatus(t.Context(), organizations.MembershipStatusChange{OrganizationID: f.org.ID, UserID: roleMember, ActorUserID: roleMember, ExpectedStatus: "active", Status: "suspended"})
+	if err != nil {
+		t.Fatalf("delegated legacy operation changed: %v", err)
+	}
+}
+
+func TestOwnedMembershipPreservesLastOwnerAndRestoresSuspendedMember(t *testing.T) {
+	f := newRoleSetFixture(t)
+	f.addMember(t)
+	for _, operation := range ownedOperations()[1:] {
+		if err := operation.apply(t.Context(), f, roleOwner, roleOwner); !errors.Is(err, organizations.ErrLastOwner) {
+			t.Fatalf("%s last owner: %v", operation.name, err)
+		}
+	}
+	if err := ownedOperations()[1].apply(t.Context(), f, roleOwner, roleMember); err != nil {
+		t.Fatal(err)
+	}
+	input := organizations.MembershipStatusChange{OrganizationID: f.org.ID, UserID: roleMember, ActorUserID: roleOwner, ExpectedStatus: "suspended", Status: "active"}
+	if err := f.organizations.ChangeOwnedMembershipStatus(t.Context(), input); err != nil {
+		t.Fatal(err)
+	}
+	if err := f.organizations.ChangeOwnedMembershipStatus(t.Context(), input); !errors.Is(err, organizations.ErrRevisionConflict) {
+		t.Fatalf("stale reactivation: %v", err)
+	}
+	assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_organization_memberships WHERE user_id=? AND status='active'`, roleMember, 1)
+}
+
+func TestOwnedManagementRollsBackWithAuditFailure(t *testing.T) {
+	for _, operation := range ownedOperations() {
+		t.Run(operation.name, func(t *testing.T) {
+			f := newRoleSetFixture(t)
+			f.addMember(t)
+			_, pending := f.invite(t, "buyer")
+			team, err := f.organizations.CreateTeam(t.Context(), organizations.CreateTeam{OrganizationID: f.org.ID, Slug: "team", Name: "Team", ActorUserID: roleOwner})
+			if err != nil {
+				t.Fatal(err)
+			}
+			if err = f.organizations.AddTeamMember(t.Context(), team.ID, roleMember, roleOwner); err != nil {
+				t.Fatal(err)
+			}
+			if _, err = f.store.db.Exec(`CREATE TRIGGER reject_owned_audit BEFORE INSERT ON gwf_access_audit_events WHEN NEW.action='` + operation.action + `' BEGIN SELECT RAISE(ABORT,'injected audit failure'); END`); err != nil {
+				t.Fatal(err)
+			}
+			if err = operation.apply(t.Context(), f, roleOwner, roleMember); err == nil {
+				t.Fatal("audit failure accepted")
+			}
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_organization_memberships WHERE user_id=? AND status='active'`, roleMember, 1)
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_team_members WHERE user_id=?`, roleMember, 1)
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_access_bindings WHERE subject_id=? AND revoked_at IS NULL`, roleMember, 1)
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_organizations WHERE id=? AND revision=1`, f.org.ID, 1)
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_organization_invitations WHERE id=? AND revoked_at IS NULL`, pending.ID, 1)
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_access_audit_events WHERE action=?`, operation.action, 0)
+		})
+	}
+}
+
+func TestConcurrentOwnedManagementHasOneWinner(t *testing.T) {
+	for _, operation := range ownedOperations() {
+		t.Run(operation.name, func(t *testing.T) {
+			f := newRoleSetFixture(t)
+			f.addMember(t)
+			start, results := make(chan struct{}), make(chan error, 2)
+			for range 2 {
+				go func() { <-start; results <- operation.apply(t.Context(), f, roleOwner, roleMember) }()
+			}
+			close(start)
+			success, stale := 0, 0
+			for range 2 {
+				err := <-results
+				switch {
+				case err == nil:
+					success++
+				case errors.Is(err, organizations.ErrRevisionConflict), errors.Is(err, organizations.ErrMembershipNotFound):
+					stale++
+				default:
+					t.Fatalf("concurrent mutation: %v", err)
+				}
+			}
+			if success != 1 || stale != 1 {
+				t.Fatalf("success=%d stale=%d", success, stale)
+			}
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_access_audit_events WHERE action=?`, operation.action, 1)
+		})
+	}
+}