diff --git a/CHANGELOG.md b/CHANGELOG.md index 389287b..0f6ead7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ policy. Persist the required grantor authority and recheck it at acceptance, together with active, fully registered users and recipient email. Suspended members cannot reactivate themselves by accepting an older invitation. +- Invitations enroll new members rather than adding permissions to existing + members. Membership removal revokes pending invitations for that recipient + in the same transaction, preventing an older offer from restoring access. - Add SQLite schema 10 for invitation role sets and stored owner authority. Legacy single-role data remains readable after explicit migration; older schema-9 applications are not approved writers of the migrated database. diff --git a/authsqlite/organizations.go b/authsqlite/organizations.go index 68d4e3b..c6cee8b 100644 --- a/authsqlite/organizations.go +++ b/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. diff --git a/authsqlite/role_sets_test.go b/authsqlite/role_sets_test.go index f804287..d65d37e 100644 --- a/authsqlite/role_sets_test.go +++ b/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) + } + }) + } +} diff --git a/docs/ORGANIZATIONS.md b/docs/ORGANIZATIONS.md index 619813a..bea6c3b 100644 --- a/docs/ORGANIZATIONS.md +++ b/docs/ORGANIZATIONS.md @@ -14,7 +14,10 @@ authenticated user's normalized email matches and applies the membership, roles, teams, consumption marker, and audit event in one transaction. The recipient and issuing member must remain active, fully registered users of an active organization; a suspended recipient cannot use an invitation as implicit -reactivation. Duplicate or concurrent acceptance consumes the token only once. +reactivation. Existing members use the membership editor, not another invitation, +to change roles or teams. Duplicate or concurrent acceptance consumes the token +only once. Removal revokes older pending invitations for that recipient in the +same transaction; a new, intentional invitation is needed to rejoin later. When `OwnerRole` is configured, invitations granting that role require a current direct owner at creation and acceptance, and an owner for revocation. Set `OwnerManagedInvitations: true` to apply that rule to every invitation, including