From 59827bf641bcf3a94a7ad05769bb5d288bfd8990 Mon Sep 17 00:00:00 2001 From: Cole Speelman Date: Thu, 3 Sep 2026 23:22:19 -0400 Subject: [PATCH] Add optimistic membership lifecycle --- CHANGELOG.md | 16 ++++ README.md | 6 +- authsqlite/organizations.go | 140 +++++++++++++++++++++++++++- authsqlite/store_test.go | 113 +++++++++++++++++++++- docs/DOGFOOD.md | 5 + docs/GETTING_STARTED.md | 2 +- docs/MODULES.md | 2 +- docs/ORGANIZATIONS.md | 9 ++ organizations/organizations.go | 82 ++++++++++++++-- organizations/organizations_test.go | 13 +++ 10 files changed, 368 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fab8cd4..f6a8f08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ # Changelog +## v0.1.0-preview.17 — 2026-09-04 + +- Add optimistic organization-membership suspension, reactivation, and + removal for fresh-authentication administration flows. The exact displayed + membership state is rechecked after acquiring the SQLite write lock, so a + concurrent or stale ceremony fails without changing access or writing an + audit event. +- Keep membership lifecycle consequences transactional: suspension removes + team membership, removal also revokes direct bindings, reactivation does not + silently restore former teams, and every successful change appends its + organization-visible audit before commit. +- Strengthen last-owner protection to require another active direct owner + whose platform account is also active. Existing storage adapters retain the + legacy interface; security-sensitive applications fail closed unless their + repository implements the optimistic lifecycle extension. + ## v0.1.0-preview.16 — 2026-09-03 - Add bounded organization-member and direct user-role listings for diff --git a/README.md b/README.md index fe2c77d..d2ab48e 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ router, handlers, HTML, authorization decisions, cache behavior, and deployment. Adopt one boundary at a time; Go compiles and links only the packages you import. -> **Public preview:** `v0.1.0-preview.16`. APIs may change before a stable +> **Public preview:** `v0.1.0-preview.17`. APIs may change before a stable > release. Linux is the maintained release platform. ## Why Web Foundations? @@ -57,14 +57,14 @@ owns—and, just as importantly, what remains application policy. Pin the preview in an application module: ```bash -go get gamertan.com/web@v0.1.0-preview.16 +go get gamertan.com/web@v0.1.0-preview.17 go mod verify ``` An application may name the first package it intends to adopt: ```bash -go get gamertan.com/web/requestmeta@v0.1.0-preview.16 +go get gamertan.com/web/requestmeta@v0.1.0-preview.17 ``` The version belongs to the `gamertan.com/web` module. See the diff --git a/authsqlite/organizations.go b/authsqlite/organizations.go index 84e5d86..a12d233 100644 --- a/authsqlite/organizations.go +++ b/authsqlite/organizations.go @@ -453,6 +453,48 @@ func (store *Store) SetMembershipStatus(ctx context.Context, organizationID, use return tx.Commit() } +func (store *Store) ChangeMembershipStatus(ctx context.Context, input organizations.MembershipStatusChange, ownerRole string, audit organizations.AuditEvent) error { + if !validMembershipStatusChange(input, ownerRole, audit) { + return organizations.ErrMembershipNotFound + } + tx, err := store.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + if err = lockActiveMembershipActor(ctx, tx, input.OrganizationID, input.ActorUserID); err != nil { + return err + } + current, err := membershipStatus(ctx, tx, input.OrganizationID, input.UserID) + if err != nil { + return err + } + if current != input.ExpectedStatus { + return organizations.ErrRevisionConflict + } + if input.Status == "suspended" { + if err = protectLastOwner(ctx, tx, input.OrganizationID, input.UserID, ownerRole); err != nil { + return err + } + } + result, err := tx.ExecContext(ctx, `UPDATE gwf_organization_memberships SET status=? WHERE organization_id=? AND user_id=? AND status=?`, input.Status, input.OrganizationID, input.UserID, input.ExpectedStatus) + if err != nil { + return err + } + if changed, _ := result.RowsAffected(); changed != 1 { + return organizations.ErrRevisionConflict + } + if input.Status == "suspended" { + if _, err = tx.ExecContext(ctx, `DELETE FROM gwf_team_members WHERE user_id=? AND team_id IN (SELECT id FROM gwf_teams WHERE organization_id=?)`, input.UserID, input.OrganizationID); err != nil { + return err + } + } + if err = appendOrganizationAudit(ctx, tx, audit); err != nil { + return err + } + return tx.Commit() +} + func (store *Store) RemoveMembership(ctx context.Context, organizationID, userID, ownerRole string, audit organizations.AuditEvent) error { if !opaqueID(organizationID) || !opaqueID(userID) || !safeName(ownerRole) || !validOrganizationAudit(audit, organizationID) { return organizations.ErrMembershipNotFound @@ -484,6 +526,77 @@ func (store *Store) RemoveMembership(ctx context.Context, organizationID, userID return tx.Commit() } +func (store *Store) RemoveMembershipIfCurrent(ctx context.Context, input organizations.MembershipRemoval, ownerRole string, audit organizations.AuditEvent) error { + if !validMembershipRemoval(input, ownerRole, audit) { + return organizations.ErrMembershipNotFound + } + tx, err := store.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + if err = lockActiveMembershipActor(ctx, tx, input.OrganizationID, input.ActorUserID); err != nil { + return err + } + current, err := membershipStatus(ctx, tx, input.OrganizationID, input.UserID) + if err != nil { + return err + } + if current != input.ExpectedStatus { + return organizations.ErrRevisionConflict + } + if err = protectLastOwner(ctx, tx, input.OrganizationID, input.UserID, ownerRole); err != nil { + return err + } + if _, err = tx.ExecContext(ctx, `DELETE FROM gwf_team_members WHERE user_id=? AND team_id IN (SELECT id FROM gwf_teams WHERE organization_id=?)`, input.UserID, input.OrganizationID); err != nil { + return err + } + if _, err = tx.ExecContext(ctx, `UPDATE gwf_access_bindings SET revoked_by_user_id=?,revoked_at=? WHERE organization_id=? AND subject_kind='user' AND subject_id=? AND revoked_at IS NULL`, audit.ActorUserID, audit.CreatedAt.Unix(), input.OrganizationID, input.UserID); err != nil { + return err + } + result, err := tx.ExecContext(ctx, `DELETE FROM gwf_organization_memberships WHERE organization_id=? AND user_id=? AND status=?`, input.OrganizationID, input.UserID, input.ExpectedStatus) + if err != nil { + return err + } + if changed, _ := result.RowsAffected(); changed != 1 { + return organizations.ErrRevisionConflict + } + if err = appendOrganizationAudit(ctx, tx, audit); err != nil { + return err + } + return tx.Commit() +} + +func lockActiveMembershipActor(ctx context.Context, tx *sql.Tx, organizationID, actorUserID string) error { + // Acquire the SQLite write lock before reading the optimistic state. This + // makes a competing lifecycle transaction observe the committed winner. + result, err := tx.ExecContext(ctx, `UPDATE gwf_organization_memberships SET status=status + WHERE organization_id=? AND user_id=? AND status='active' + AND EXISTS (SELECT 1 FROM gwf_organizations o WHERE o.id=? AND o.status='active') + AND EXISTS (SELECT 1 FROM gwf_users u WHERE u.id=? AND u.status='active')`, organizationID, actorUserID, organizationID, actorUserID) + if err != nil { + return err + } + if changed, _ := result.RowsAffected(); changed != 1 { + return organizations.ErrMembershipNotFound + } + 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 { + if errors.Is(err, sql.ErrNoRows) { + return "", organizations.ErrMembershipNotFound + } + return "", err + } + if status != "active" && status != "suspended" { + return "", errors.New("authsqlite: stored membership status is invalid") + } + return status, nil +} + func protectLastOwner(ctx context.Context, tx *sql.Tx, organizationID, userID, ownerRole string) error { var targetIsOwner int if err := tx.QueryRowContext(ctx, `SELECT COUNT(*) FROM gwf_access_bindings WHERE organization_id=? AND subject_kind='user' AND subject_id=? AND role_name=? AND project_id IS NULL AND environment_id IS NULL AND service_id IS NULL AND revoked_at IS NULL`, organizationID, userID, ownerRole).Scan(&targetIsOwner); err != nil { @@ -492,16 +605,37 @@ func protectLastOwner(ctx context.Context, tx *sql.Tx, organizationID, userID, o if targetIsOwner == 0 { return nil } - var activeOwners int - if err := tx.QueryRowContext(ctx, `SELECT COUNT(DISTINCT b.subject_id) FROM gwf_access_bindings b JOIN gwf_organization_memberships m ON m.organization_id=b.organization_id AND m.user_id=b.subject_id AND m.status='active' WHERE b.organization_id=? AND b.subject_kind='user' AND b.role_name=? AND b.project_id IS NULL AND b.environment_id IS NULL AND b.service_id IS NULL AND b.revoked_at IS NULL`, organizationID, ownerRole).Scan(&activeOwners); err != nil { + var otherActiveOwners int + if err := tx.QueryRowContext(ctx, `SELECT COUNT(DISTINCT b.subject_id) + FROM gwf_access_bindings b + JOIN gwf_organization_memberships m ON m.organization_id=b.organization_id AND m.user_id=b.subject_id AND m.status='active' + JOIN gwf_users u ON u.id=m.user_id AND u.status='active' + WHERE b.organization_id=? AND b.subject_kind='user' AND b.subject_id<>? AND b.role_name=? + AND b.project_id IS NULL AND b.environment_id IS NULL AND b.service_id IS NULL + AND b.revoked_at IS NULL`, organizationID, userID, ownerRole).Scan(&otherActiveOwners); err != nil { return err } - if activeOwners <= 1 { + if otherActiveOwners == 0 { return organizations.ErrLastOwner } return nil } +func validMembershipStatusChange(input organizations.MembershipStatusChange, ownerRole string, audit organizations.AuditEvent) bool { + return opaqueID(input.OrganizationID) && opaqueID(input.UserID) && opaqueID(input.ActorUserID) && safeName(ownerRole) && + (input.ExpectedStatus == "active" || input.ExpectedStatus == "suspended") && + (input.Status == "active" || input.Status == "suspended") && input.ExpectedStatus != input.Status && + validOrganizationAudit(audit, input.OrganizationID) && audit.ActorUserID == input.ActorUserID && + audit.Action == "membership."+input.Status && audit.ResourceType == "membership" && audit.ResourceID == input.UserID && audit.RequestID == input.RequestID +} + +func validMembershipRemoval(input organizations.MembershipRemoval, ownerRole string, audit organizations.AuditEvent) bool { + return opaqueID(input.OrganizationID) && opaqueID(input.UserID) && opaqueID(input.ActorUserID) && safeName(ownerRole) && + (input.ExpectedStatus == "active" || input.ExpectedStatus == "suspended") && + validOrganizationAudit(audit, input.OrganizationID) && audit.ActorUserID == input.ActorUserID && + audit.Action == "membership.remove" && audit.ResourceType == "membership" && audit.ResourceID == input.UserID && audit.RequestID == input.RequestID +} + func (store *Store) Invitations(ctx context.Context, organizationID string, limit int) ([]organizations.Invitation, error) { if !opaqueID(organizationID) || limit < 1 || limit > 1000 { return nil, errors.New("authsqlite: invalid invitation query") diff --git a/authsqlite/store_test.go b/authsqlite/store_test.go index 828ebd7..c545965 100644 --- a/authsqlite/store_test.go +++ b/authsqlite/store_test.go @@ -438,7 +438,7 @@ func TestInvitationAccessLifecycleAndLastOwnerProtection(t *testing.T) { if _, err = accessService.Grant(t.Context(), access.Grant{SubjectKind: access.User, SubjectID: owner.ID, Role: "organization.owner", Scope: access.Scope{OrganizationID: organization.ID}, GrantedBy: owner.ID}); err != nil { t.Fatal(err) } - if err = organizationService.SetMembershipStatus(t.Context(), organization.ID, owner.ID, "suspended", owner.ID, "request-last-owner"); !errors.Is(err, organizations.ErrLastOwner) { + if err = organizationService.ChangeMembershipStatus(t.Context(), organizations.MembershipStatusChange{OrganizationID: organization.ID, UserID: owner.ID, ExpectedStatus: "active", Status: "suspended", ActorUserID: owner.ID, RequestID: "request-last-owner"}); !errors.Is(err, organizations.ErrLastOwner) { t.Fatalf("last-owner suspension err=%v", err) } team, err := organizationService.CreateTeam(t.Context(), organizations.CreateTeam{OrganizationID: organization.ID, Slug: "operators", Name: "Operators", ActorUserID: owner.ID}) @@ -463,10 +463,10 @@ func TestInvitationAccessLifecycleAndLastOwnerProtection(t *testing.T) { if err != nil || len(teams) != 1 || teams[0].ID != team.ID { t.Fatalf("member teams=%+v err=%v", teams, err) } - if err = organizationService.SetMembershipStatus(t.Context(), organization.ID, owner.ID, "suspended", owner.ID, "request-suspend-owner"); err != nil { + if err = organizationService.ChangeMembershipStatus(t.Context(), organizations.MembershipStatusChange{OrganizationID: organization.ID, UserID: owner.ID, ExpectedStatus: "active", Status: "suspended", ActorUserID: owner.ID, RequestID: "request-suspend-owner"}); err != nil { t.Fatal(err) } - if err = organizationService.RemoveMembership(t.Context(), organization.ID, member.ID, member.ID, "request-last-member"); !errors.Is(err, organizations.ErrLastOwner) { + if err = organizationService.RemoveMembershipIfCurrent(t.Context(), organizations.MembershipRemoval{OrganizationID: organization.ID, UserID: member.ID, ExpectedStatus: "active", ActorUserID: member.ID, RequestID: "request-last-member"}); !errors.Is(err, organizations.ErrLastOwner) { t.Fatalf("sole active owner removal err=%v", err) } if _, err = organizationService.SetOrganizationStatus(t.Context(), organizations.SetOrganizationStatus{ID: organization.ID, Status: "archived", ActorUserID: member.ID, ExpectedRevision: organization.Revision, RequestID: "request-archive"}); err != nil { @@ -635,6 +635,113 @@ func TestOrganizationRoleAdministrationIsAtomicAndProtectsOwners(t *testing.T) { assertCount(t, store, `SELECT COUNT(*) FROM gwf_access_bindings WHERE id=?`, replacement.ID, 0) } +func TestOptimisticMembershipLifecycleIsSerializedAndAtomic(t *testing.T) { + store, err := Open(filepath.Join(t.TempDir(), "accounts.db")) + if err != nil { + t.Fatal(err) + } + defer store.Close() + now := time.Date(2026, 9, 4, 9, 0, 0, 0, time.UTC) + authService, err := auth.New(store, auth.Options{Now: func() time.Time { return now }}) + if err != nil { + t.Fatal(err) + } + owner, err := authService.CreateUser(t.Context(), auth.CreateUser{Username: "lifecycle.owner", Email: "lifecycle-owner@example.test", DisplayName: "Lifecycle Owner", Password: "correct horse battery staple"}) + if err != nil { + t.Fatal(err) + } + member, err := authService.CreateUser(t.Context(), auth.CreateUser{Username: "lifecycle.member", Email: "lifecycle-member@example.test", DisplayName: "Lifecycle Member", Password: "correct horse battery staple"}) + if err != nil { + t.Fatal(err) + } + organizationService, err := organizations.New(store, organizations.Options{Now: func() time.Time { return now }, OwnerRole: "owner"}) + if err != nil { + t.Fatal(err) + } + organization, err := organizationService.CreateOrganization(t.Context(), organizations.CreateOrganization{Slug: "optimistic-lifecycle", Name: "Optimistic Lifecycle", OwnerUserID: owner.ID}) + if err != nil { + t.Fatal(err) + } + policy := access.Policy{Roles: map[string]string{"owner": "Owner", "viewer": "Viewer"}, Permissions: map[string]string{"telemetry.read": "Read"}, Grants: map[string][]string{"owner": {"telemetry.read"}, "viewer": {"telemetry.read"}}} + accessService, err := access.New(store, policy, access.Options{Now: func() time.Time { return now }, OwnerRole: "owner"}) + if err != nil { + t.Fatal(err) + } + if err = accessService.Seed(t.Context()); err != nil { + t.Fatal(err) + } + if _, err = accessService.Grant(t.Context(), access.Grant{SubjectKind: access.User, SubjectID: owner.ID, Role: "owner", Scope: access.Scope{OrganizationID: organization.ID}, GrantedBy: owner.ID}); err != nil { + t.Fatal(err) + } + team, err := organizationService.CreateTeam(t.Context(), organizations.CreateTeam{OrganizationID: organization.ID, Slug: "operators", Name: "Operators", ActorUserID: owner.ID}) + if err != nil { + t.Fatal(err) + } + raw, _, err := organizationService.InviteWithAccess(t.Context(), organizations.InviteWithAccess{OrganizationID: organization.ID, Email: member.Email, InvitedByUserID: owner.ID, DirectRole: "viewer", TeamIDs: []string{team.ID}, Lifetime: 24 * time.Hour}) + if err != nil { + t.Fatal(err) + } + if err = organizationService.AcceptInvitation(t.Context(), raw, member.ID); err != nil { + t.Fatal(err) + } + + start := make(chan struct{}) + results := make(chan error, 2) + for _, requestID := range []string{"request-suspend-one", "request-suspend-two"} { + requestID := requestID + go func() { + <-start + results <- organizationService.ChangeMembershipStatus(t.Context(), organizations.MembershipStatusChange{OrganizationID: organization.ID, UserID: member.ID, ExpectedStatus: "active", Status: "suspended", ActorUserID: owner.ID, RequestID: requestID}) + }() + } + close(start) + var successful, conflicted int + for range 2 { + switch lifecycleErr := <-results; { + case lifecycleErr == nil: + successful++ + case errors.Is(lifecycleErr, organizations.ErrRevisionConflict): + conflicted++ + default: + t.Fatalf("concurrent membership suspension err=%v", lifecycleErr) + } + } + if successful != 1 || conflicted != 1 { + t.Fatalf("concurrent membership suspension success=%d conflict=%d", successful, conflicted) + } + assertCount(t, store, `SELECT COUNT(*) FROM gwf_access_audit_events WHERE action='membership.suspended' AND resource_id=?`, member.ID, 1) + assertCount(t, store, `SELECT COUNT(*) FROM gwf_team_members WHERE user_id=?`, member.ID, 0) + decision, err := accessService.Authorize(t.Context(), member.ID, access.Scope{OrganizationID: organization.ID}, "telemetry.read") + if err != nil || decision.Allowed { + t.Fatalf("suspended member decision=%+v err=%v", decision, err) + } + if err = organizationService.RemoveMembershipIfCurrent(t.Context(), organizations.MembershipRemoval{OrganizationID: organization.ID, UserID: member.ID, ExpectedStatus: "active", ActorUserID: owner.ID, RequestID: "request-stale-remove"}); !errors.Is(err, organizations.ErrRevisionConflict) { + t.Fatalf("stale membership removal err=%v", err) + } + assertCount(t, store, `SELECT COUNT(*) FROM gwf_access_audit_events WHERE request_id=?`, "request-stale-remove", 0) + assertCount(t, store, `SELECT COUNT(*) FROM gwf_organization_memberships WHERE user_id=?`, member.ID, 1) + + if err = organizationService.ChangeMembershipStatus(t.Context(), organizations.MembershipStatusChange{OrganizationID: organization.ID, UserID: member.ID, ExpectedStatus: "suspended", Status: "active", ActorUserID: owner.ID, RequestID: "request-reactivate"}); err != nil { + t.Fatal(err) + } + assertCount(t, store, `SELECT COUNT(*) FROM gwf_team_members WHERE user_id=?`, member.ID, 0) + if err = organizationService.ChangeMembershipStatus(t.Context(), organizations.MembershipStatusChange{OrganizationID: organization.ID, UserID: member.ID, ExpectedStatus: "suspended", Status: "active", ActorUserID: owner.ID, RequestID: "request-stale-reactivate"}); !errors.Is(err, organizations.ErrRevisionConflict) { + t.Fatalf("stale membership reactivation err=%v", err) + } + assertCount(t, store, `SELECT COUNT(*) FROM gwf_access_audit_events WHERE request_id=?`, "request-stale-reactivate", 0) + + if err = organizationService.RemoveMembershipIfCurrent(t.Context(), organizations.MembershipRemoval{OrganizationID: organization.ID, UserID: member.ID, ExpectedStatus: "active", ActorUserID: owner.ID, RequestID: "request-remove-member"}); err != nil { + t.Fatal(err) + } + assertCount(t, store, `SELECT COUNT(*) FROM gwf_organization_memberships WHERE user_id=?`, member.ID, 0) + assertCount(t, store, `SELECT COUNT(*) FROM gwf_access_bindings WHERE subject_id=? AND revoked_at IS NOT NULL`, member.ID, 1) + assertCount(t, store, `SELECT COUNT(*) FROM gwf_access_audit_events WHERE request_id=?`, "request-remove-member", 1) + decision, err = accessService.Authorize(t.Context(), member.ID, access.Scope{OrganizationID: organization.ID}, "telemetry.read") + if err != nil || decision.Allowed { + t.Fatalf("removed member decision=%+v err=%v", decision, err) + } +} + func membershipPresent(values []organizations.Membership, userID, status string) bool { for _, value := range values { if value.UserID == userID && value.Status == status { diff --git a/docs/DOGFOOD.md b/docs/DOGFOOD.md index 1ed2a22..d03ce53 100644 --- a/docs/DOGFOOD.md +++ b/docs/DOGFOOD.md @@ -60,3 +60,8 @@ application concern belongs in the shared module. owner and appends its audit before commit. The application still owns route authorization, role presentation, CSRF, and the exact fresh-passkey operation binding. +- Extending that page to membership suspension, reactivation, and removal + exposed the same time-of-check gap in the older lifecycle methods. The new + optimistic extension serializes on the active administrator membership, + rechecks the exact state bound into the passkey assertion, applies team and + direct-binding consequences, and writes the audit in one transaction. diff --git a/docs/GETTING_STARTED.md b/docs/GETTING_STARTED.md index fb6a6fd..40cd413 100644 --- a/docs/GETTING_STARTED.md +++ b/docs/GETTING_STARTED.md @@ -26,7 +26,7 @@ The packages are ordinary Go imports. Pin the current preview and verify its module checksum: ```bash -go get gamertan.com/web/requestmeta@v0.1.0-preview.16 +go get gamertan.com/web/requestmeta@v0.1.0-preview.17 go mod verify ``` diff --git a/docs/MODULES.md b/docs/MODULES.md index 13c5413..0e5c710 100644 --- a/docs/MODULES.md +++ b/docs/MODULES.md @@ -18,7 +18,7 @@ import "gamertan.com/web/requestmeta" and request the containing module at an exact version: ```bash -go get gamertan.com/web/requestmeta@v0.1.0-preview.16 +go get gamertan.com/web/requestmeta@v0.1.0-preview.17 ``` Only imported packages are compiled and linked. The packages nevertheless diff --git a/docs/ORGANIZATIONS.md b/docs/ORGANIZATIONS.md index c4f7008..4dc6763 100644 --- a/docs/ORGANIZATIONS.md +++ b/docs/ORGANIZATIONS.md @@ -22,6 +22,15 @@ team membership can be removed independently. Configure `OwnerRole` when constructing the service before exposing membership-removal operations. The SQLite adapter then refuses to suspend or remove the final active direct owner. +Fresh-authentication administration pages should use +`ChangeMembershipStatus` and `RemoveMembershipIfCurrent`, passing the exact +displayed state as `ExpectedStatus`. The SQLite adapter acquires its write lock +before checking that state, verifies the actor is still an active member of an +active organization, and commits the lifecycle effects and audit together. +Suspension removes team memberships; reactivation does not infer or restore +them. Removal also revokes current direct bindings. A repository without the +optimistic extension fails closed instead of falling back to a stale mutation. + For a reviewed access-administration page, use `organizations.Members` to list bounded active and suspended memberships, and `access.OrganizationUserBindings` to list only current direct, diff --git a/organizations/organizations.go b/organizations/organizations.go index c9d7f64..91cf96d 100644 --- a/organizations/organizations.go +++ b/organizations/organizations.go @@ -19,15 +19,16 @@ import ( ) var ( - ErrInvitationNotFound = errors.New("organizations: invitation not found") - ErrMembershipNotFound = errors.New("organizations: membership not found") - ErrOrganizationNotFound = errors.New("organizations: organization not found") - ErrTeamNotFound = errors.New("organizations: team not found") - ErrRevisionConflict = errors.New("organizations: revision conflict") - ErrPersonalOrganization = errors.New("organizations: personal organization lifecycle is fixed") - ErrLastOwner = errors.New("organizations: the last active direct owner must be preserved") - slugPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{1,62}$`) - idPattern = regexp.MustCompile(`^[A-Za-z0-9_-]{8,128}$`) + ErrInvitationNotFound = errors.New("organizations: invitation not found") + ErrMembershipNotFound = errors.New("organizations: membership not found") + ErrMembershipLifecycleUnsupported = errors.New("organizations: optimistic membership lifecycle is unsupported") + ErrOrganizationNotFound = errors.New("organizations: organization not found") + ErrTeamNotFound = errors.New("organizations: team not found") + ErrRevisionConflict = errors.New("organizations: revision conflict") + ErrPersonalOrganization = errors.New("organizations: personal organization lifecycle is fixed") + ErrLastOwner = errors.New("organizations: the last active direct owner must be preserved") + slugPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{1,62}$`) + idPattern = regexp.MustCompile(`^[A-Za-z0-9_-]{8,128}$`) ) type Organization struct { @@ -112,6 +113,16 @@ type Repository interface { TeamsForUser(context.Context, string, string) ([]Team, error) } +// OptimisticMembershipRepository is implemented by repositories that can +// bind a membership lifecycle mutation to the exact state authorized by the +// caller. Services deliberately do not fall back to the older lifecycle +// methods: a stale fresh-authentication ceremony must fail instead of acting +// on a membership that changed while the ceremony was in progress. +type OptimisticMembershipRepository interface { + ChangeMembershipStatus(context.Context, MembershipStatusChange, string, AuditEvent) error + RemoveMembershipIfCurrent(context.Context, MembershipRemoval, string, AuditEvent) error +} + type Options struct { Random io.Reader Now func() time.Time @@ -464,6 +475,34 @@ func (service *Service) SetMembershipStatus(ctx context.Context, organizationID, return service.repository.SetMembershipStatus(ctx, organizationID, userID, status, service.ownerRole, audit) } +// MembershipStatusChange describes an exact active-to-suspended or +// suspended-to-active transition. ExpectedStatus is part of the authorized +// operation and is checked again inside the repository transaction. +type MembershipStatusChange struct { + OrganizationID, UserID, ExpectedStatus, Status, ActorUserID, RequestID string +} + +func (service *Service) ChangeMembershipStatus(ctx context.Context, input MembershipStatusChange) error { + if !idPattern.MatchString(input.OrganizationID) || !idPattern.MatchString(input.UserID) || !idPattern.MatchString(input.ActorUserID) || + (input.ExpectedStatus != "active" && input.ExpectedStatus != "suspended") || + (input.Status != "active" && input.Status != "suspended") || input.Status == input.ExpectedStatus || + !boundedOptional(input.RequestID, 128) { + return errors.New("organizations: invalid membership status change") + } + if service.ownerRole == "" { + return errors.New("organizations: owner role is required for membership lifecycle changes") + } + repository, ok := service.repository.(OptimisticMembershipRepository) + if !ok { + return ErrMembershipLifecycleUnsupported + } + audit, err := service.auditWithRequest(input.ActorUserID, input.OrganizationID, "membership."+input.Status, "membership", input.UserID, input.RequestID, "Organization membership set to "+input.Status) + if err != nil { + return err + } + return repository.ChangeMembershipStatus(ctx, input, service.ownerRole, audit) +} + func (service *Service) RemoveMembership(ctx context.Context, organizationID, userID, actorUserID, requestID string) error { if !idPattern.MatchString(organizationID) || !idPattern.MatchString(userID) || !idPattern.MatchString(actorUserID) || !boundedOptional(requestID, 128) { return errors.New("organizations: invalid membership removal") @@ -478,6 +517,31 @@ func (service *Service) RemoveMembership(ctx context.Context, organizationID, us return service.repository.RemoveMembership(ctx, organizationID, userID, service.ownerRole, audit) } +// MembershipRemoval binds removal to the exact membership state observed by +// the caller before fresh authentication began. +type MembershipRemoval struct { + OrganizationID, UserID, ExpectedStatus, ActorUserID, RequestID string +} + +func (service *Service) RemoveMembershipIfCurrent(ctx context.Context, input MembershipRemoval) error { + if !idPattern.MatchString(input.OrganizationID) || !idPattern.MatchString(input.UserID) || !idPattern.MatchString(input.ActorUserID) || + (input.ExpectedStatus != "active" && input.ExpectedStatus != "suspended") || !boundedOptional(input.RequestID, 128) { + return errors.New("organizations: invalid membership removal") + } + if service.ownerRole == "" { + return errors.New("organizations: owner role is required for membership lifecycle changes") + } + repository, ok := service.repository.(OptimisticMembershipRepository) + if !ok { + return ErrMembershipLifecycleUnsupported + } + audit, err := service.auditWithRequest(input.ActorUserID, input.OrganizationID, "membership.remove", "membership", input.UserID, input.RequestID, "Organization membership removed") + if err != nil { + return err + } + return repository.RemoveMembershipIfCurrent(ctx, input, service.ownerRole, audit) +} + func (service *Service) Invitations(ctx context.Context, organizationID string, limit int) ([]Invitation, error) { if !idPattern.MatchString(organizationID) || limit < 1 || limit > 1000 { return nil, errors.New("organizations: invalid invitation query") diff --git a/organizations/organizations_test.go b/organizations/organizations_test.go index b78faf3..d24ac64 100644 --- a/organizations/organizations_test.go +++ b/organizations/organizations_test.go @@ -47,6 +47,19 @@ func TestInvitationFailsClosed(t *testing.T) { } } +func TestOptimisticMembershipLifecycleFailsClosedWithoutRepositorySupport(t *testing.T) { + service, err := New(&repositoryStub{}, Options{OwnerRole: "organization.owner"}) + if err != nil { + t.Fatal(err) + } + if err = service.ChangeMembershipStatus(t.Context(), MembershipStatusChange{OrganizationID: "organization-1234", UserID: "user-12345678", ExpectedStatus: "active", Status: "suspended", ActorUserID: "user-87654321", RequestID: "request-suspend"}); !errors.Is(err, ErrMembershipLifecycleUnsupported) { + t.Fatalf("status change err=%v", err) + } + if err = service.RemoveMembershipIfCurrent(t.Context(), MembershipRemoval{OrganizationID: "organization-1234", UserID: "user-12345678", ExpectedStatus: "active", ActorUserID: "user-87654321", RequestID: "request-remove"}); !errors.Is(err, ErrMembershipLifecycleUnsupported) { + t.Fatalf("removal err=%v", err) + } +} + type repositoryStub struct { organization Organization invitation Invitation