Invalidate old invitations when organization membership changes

Signed-off-by: Cole Speelman <crspeelman@gmail.com>

authsqlite/organizations.go

@@ -240,7 +240,7 @@ func (store *Store) AcceptInvitationWithRoles(ctx context.Context, digest [32]by
 	}
 	var invitationID, organizationID, directRole, invitedBy, requiredOwnerRole string
 	var teamIDsJSON, rolesJSON []byte
-	err = tx.QueryRowContext(ctx, `SELECT i.id,i.organization_id,i.direct_role,i.team_ids_json,i.invited_by_user_id,i.direct_roles_json,i.required_owner_role FROM gwf_organization_invitations i JOIN gwf_users u ON u.id=? AND u.email_normalized=i.email_normalized AND u.status='active' AND u.registration_pending=0 JOIN gwf_organizations o ON o.id=i.organization_id AND o.status='active' WHERE i.token_hash=? AND i.used_at IS NULL AND i.revoked_at IS NULL AND i.expires_at>? AND NOT EXISTS (SELECT 1 FROM gwf_organization_memberships m WHERE m.organization_id=i.organization_id AND m.user_id=u.id AND m.status<>'active')`, userID, digest[:], acceptedAt.Unix()).Scan(&invitationID, &organizationID, &directRole, &teamIDsJSON, &invitedBy, &rolesJSON, &requiredOwnerRole)
+	err = tx.QueryRowContext(ctx, `SELECT i.id,i.organization_id,i.direct_role,i.team_ids_json,i.invited_by_user_id,i.direct_roles_json,i.required_owner_role FROM gwf_organization_invitations i JOIN gwf_users u ON u.id=? AND u.email_normalized=i.email_normalized AND u.status='active' AND u.registration_pending=0 JOIN gwf_organizations o ON o.id=i.organization_id AND o.status='active' WHERE i.token_hash=? AND i.used_at IS NULL AND i.revoked_at IS NULL AND i.expires_at>? AND NOT EXISTS (SELECT 1 FROM gwf_organization_memberships m WHERE m.organization_id=i.organization_id AND m.user_id=u.id)`, userID, digest[:], acceptedAt.Unix()).Scan(&invitationID, &organizationID, &directRole, &teamIDsJSON, &invitedBy, &rolesJSON, &requiredOwnerRole)
 	if errors.Is(err, sql.ErrNoRows) {
 		return organizations.ErrInvitationNotFound
 	}
@@ -272,7 +272,7 @@ func (store *Store) AcceptInvitationWithRoles(ctx context.Context, digest [32]by
 			return organizations.ErrOwnerAuthority
 		}
 	}
-	if _, err = tx.ExecContext(ctx, `INSERT INTO gwf_organization_memberships(organization_id,user_id,status,joined_at) VALUES(?,?,'active',?) ON CONFLICT(organization_id,user_id) DO UPDATE SET status='active'`, organizationID, userID, acceptedAt.Unix()); err != nil {
+	if _, err = tx.ExecContext(ctx, `INSERT INTO gwf_organization_memberships(organization_id,user_id,status,joined_at) VALUES(?,?,'active',?)`, organizationID, userID, acceptedAt.Unix()); err != nil {
 		return err
 	}
 	var teamIDs []string
@@ -597,6 +597,9 @@ func (store *Store) RemoveMembership(ctx context.Context, organizationID, userID
 	if err = protectLastOwner(ctx, tx, organizationID, userID, ownerRole); err != nil {
 		return err
 	}
+	if err = revokePendingMembershipInvitations(ctx, tx, organizationID, userID, audit.CreatedAt); 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=?)`, userID, organizationID); err != nil {
 		return err
 	}
@@ -641,6 +644,9 @@ func (store *Store) RemoveMembershipIfCurrent(ctx context.Context, input organiz
 	if err = protectLastOwner(ctx, tx, input.OrganizationID, input.UserID, ownerRole); err != nil {
 		return err
 	}
+	if err = revokePendingMembershipInvitations(ctx, tx, input.OrganizationID, input.UserID, audit.CreatedAt); 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
 	}
@@ -660,6 +666,15 @@ func (store *Store) RemoveMembershipIfCurrent(ctx context.Context, input organiz
 	return tx.Commit()
 }
 
+// Removing a member invalidates older enrollment offers too. A deliberate new
+// invitation may be issued later; an old link cannot undo this transaction.
+func revokePendingMembershipInvitations(ctx context.Context, tx *sql.Tx, organizationID, userID string, at time.Time) error {
+	_, err := tx.ExecContext(ctx, `UPDATE gwf_organization_invitations SET revoked_at=?
+		WHERE organization_id=? AND email_normalized=(SELECT email_normalized FROM gwf_users WHERE id=?)
+		AND used_at IS NULL AND revoked_at IS NULL`, at.Unix(), organizationID, userID)
+	return err
+}
+
 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.

authsqlite/role_sets_test.go

@@ -436,3 +436,52 @@ func TestRoleInvitationCreationIsAtomicAndRejectsUnknownRole(t *testing.T) {
 		})
 	}
 }
+
+func TestRoleInvitationCannotBypassMembershipChanges(t *testing.T) {
+	for _, optimistic := range []bool{false, true} {
+		t.Run(map[bool]string{false: "legacy removal", true: "optimistic removal"}[optimistic], func(t *testing.T) {
+			f := newRoleSetFixture(t)
+			oldToken, oldInvitation := f.invite(t, "buyer", "billing")
+			f.addMember(t)
+			if err := f.organizations.AcceptInvitation(t.Context(), oldToken, roleMember); !errors.Is(err, organizations.ErrInvitationNotFound) {
+				t.Fatalf("old invite elevated existing member=%v", err)
+			}
+			_, roles := f.bindings(t, roleMember)
+			if !slices.Equal(roles, []string{"member"}) {
+				t.Fatalf("roles changed=%v", roles)
+			}
+			remove := func() error {
+				if optimistic {
+					return f.organizations.RemoveMembershipIfCurrent(t.Context(), organizations.MembershipRemoval{OrganizationID: f.org.ID, UserID: roleMember, ActorUserID: roleOwner, ExpectedStatus: "active", RequestID: "request-remove"})
+				}
+				return f.organizations.RemoveMembership(t.Context(), f.org.ID, roleMember, roleOwner, "request-remove")
+			}
+			if _, err := f.store.db.Exec(`CREATE TRIGGER fail_removal BEFORE INSERT ON gwf_access_audit_events WHEN NEW.action='membership.remove' BEGIN SELECT RAISE(ABORT,'audit failure'); END`); err != nil {
+				t.Fatal(err)
+			}
+			if err := remove(); err == nil {
+				t.Fatal("unaudited removal succeeded")
+			}
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_organization_memberships WHERE user_id=?`, roleMember, 1)
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_organization_invitations WHERE id=? AND revoked_at IS NULL AND used_at IS NULL`, oldInvitation.ID, 1)
+			if _, err := f.store.db.Exec(`DROP TRIGGER fail_removal`); err != nil {
+				t.Fatal(err)
+			}
+			if err := remove(); err != nil {
+				t.Fatal(err)
+			}
+			assertCount(t, f.store, `SELECT COUNT(*) FROM gwf_organization_invitations WHERE id=? AND revoked_at IS NOT NULL AND used_at IS NULL`, oldInvitation.ID, 1)
+			if err := f.organizations.AcceptInvitation(t.Context(), oldToken, roleMember); !errors.Is(err, organizations.ErrInvitationNotFound) {
+				t.Fatalf("old invite restored removed member=%v", err)
+			}
+			newToken, _ := f.invite(t, "buyer")
+			if err := f.organizations.AcceptInvitation(t.Context(), newToken, roleMember); err != nil {
+				t.Fatalf("intentional fresh invitation=%v", err)
+			}
+			_, roles = f.bindings(t, roleMember)
+			if !slices.Equal(roles, []string{"buyer"}) {
+				t.Fatalf("fresh roles=%v", roles)
+			}
+		})
+	}
+}