This commit is contained in:
@@ -234,6 +234,15 @@ func (store *Store) ReplaceOrganizationUserRole(ctx context.Context, expected []
|
||||
if len(currentRoles) == 1 && currentRoles[0] == replacement.Role {
|
||||
return access.ErrRoleUnchanged
|
||||
}
|
||||
if replacement.Role == ownerRole || slices.Contains(currentRoles, ownerRole) {
|
||||
actorIsOwner, ownerErr := hasDirectOwnerRole(ctx, tx, replacement.Scope.OrganizationID, replacement.GrantedBy, ownerRole)
|
||||
if ownerErr != nil {
|
||||
return ownerErr
|
||||
}
|
||||
if !actorIsOwner {
|
||||
return access.ErrOwnerAuthority
|
||||
}
|
||||
}
|
||||
if replacement.Role != ownerRole && slices.Contains(currentRoles, ownerRole) {
|
||||
var otherOwners int
|
||||
if err = tx.QueryRowContext(ctx, `SELECT COUNT(DISTINCT b.subject_id)
|
||||
|
||||
@@ -430,6 +430,12 @@ func (store *Store) SetMembershipStatus(ctx context.Context, organizationID, use
|
||||
return err
|
||||
}
|
||||
defer tx.Rollback()
|
||||
if err = lockActiveMembershipActor(ctx, tx, organizationID, audit.ActorUserID); err != nil {
|
||||
return err
|
||||
}
|
||||
if err = requireOwnerAuthorityForOwnerTarget(ctx, tx, organizationID, audit.ActorUserID, userID, ownerRole); err != nil {
|
||||
return err
|
||||
}
|
||||
if status != "active" {
|
||||
if err = protectLastOwner(ctx, tx, organizationID, userID, ownerRole); err != nil {
|
||||
return err
|
||||
@@ -472,6 +478,9 @@ func (store *Store) ChangeMembershipStatus(ctx context.Context, input organizati
|
||||
if current != input.ExpectedStatus {
|
||||
return organizations.ErrRevisionConflict
|
||||
}
|
||||
if err = requireOwnerAuthorityForOwnerTarget(ctx, tx, input.OrganizationID, input.ActorUserID, input.UserID, ownerRole); err != nil {
|
||||
return err
|
||||
}
|
||||
if input.Status == "suspended" {
|
||||
if err = protectLastOwner(ctx, tx, input.OrganizationID, input.UserID, ownerRole); err != nil {
|
||||
return err
|
||||
@@ -504,6 +513,12 @@ func (store *Store) RemoveMembership(ctx context.Context, organizationID, userID
|
||||
return err
|
||||
}
|
||||
defer tx.Rollback()
|
||||
if err = lockActiveMembershipActor(ctx, tx, organizationID, audit.ActorUserID); err != nil {
|
||||
return err
|
||||
}
|
||||
if err = requireOwnerAuthorityForOwnerTarget(ctx, tx, organizationID, audit.ActorUserID, userID, ownerRole); err != nil {
|
||||
return err
|
||||
}
|
||||
if err = protectLastOwner(ctx, tx, organizationID, userID, ownerRole); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -545,6 +560,9 @@ func (store *Store) RemoveMembershipIfCurrent(ctx context.Context, input organiz
|
||||
if current != input.ExpectedStatus {
|
||||
return organizations.ErrRevisionConflict
|
||||
}
|
||||
if err = requireOwnerAuthorityForOwnerTarget(ctx, tx, input.OrganizationID, input.ActorUserID, input.UserID, ownerRole); err != nil {
|
||||
return err
|
||||
}
|
||||
if err = protectLastOwner(ctx, tx, input.OrganizationID, input.UserID, ownerRole); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -597,6 +615,32 @@ func membershipStatus(ctx context.Context, tx *sql.Tx, organizationID, userID st
|
||||
return status, nil
|
||||
}
|
||||
|
||||
func requireOwnerAuthorityForOwnerTarget(ctx context.Context, tx *sql.Tx, organizationID, actorUserID, targetUserID, ownerRole string) error {
|
||||
targetIsOwner, err := hasDirectOwnerRole(ctx, tx, organizationID, targetUserID, ownerRole)
|
||||
if err != nil || !targetIsOwner {
|
||||
return err
|
||||
}
|
||||
actorIsOwner, err := hasDirectOwnerRole(ctx, tx, organizationID, actorUserID, ownerRole)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !actorIsOwner {
|
||||
return organizations.ErrOwnerAuthority
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func hasDirectOwnerRole(ctx context.Context, tx *sql.Tx, organizationID, userID, ownerRole string) (bool, error) {
|
||||
var count 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(&count); err != nil {
|
||||
return false, err
|
||||
}
|
||||
return count > 0, 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 {
|
||||
|
||||
@@ -548,6 +548,24 @@ func TestOrganizationRoleAdministrationIsAtomicAndProtectsOwners(t *testing.T) {
|
||||
if err != nil || len(direct) != 2 {
|
||||
t.Fatalf("direct=%+v err=%v", direct, err)
|
||||
}
|
||||
if _, err = accessService.ReplaceOrganizationUserRole(t.Context(), access.OrganizationUserRoleChange{OrganizationID: organization.ID, UserID: member.ID, Role: "owner", ActorUserID: member.ID, RequestID: "request-self-promote", ExpectedBindingIDs: []string{memberBinding.ID}}); !errors.Is(err, access.ErrOwnerAuthority) {
|
||||
t.Fatalf("non-owner self-promotion err=%v", err)
|
||||
}
|
||||
if _, err = accessService.ReplaceOrganizationUserRole(t.Context(), access.OrganizationUserRoleChange{OrganizationID: organization.ID, UserID: owner.ID, Role: "viewer", ActorUserID: member.ID, RequestID: "request-demote-owner", ExpectedBindingIDs: []string{ownerBinding.ID}}); !errors.Is(err, access.ErrOwnerAuthority) {
|
||||
t.Fatalf("non-owner owner-demotion err=%v", err)
|
||||
}
|
||||
if err = organizationService.ChangeMembershipStatus(t.Context(), organizations.MembershipStatusChange{OrganizationID: organization.ID, UserID: owner.ID, ExpectedStatus: "active", Status: "suspended", ActorUserID: member.ID, RequestID: "request-suspend-owner-without-authority"}); !errors.Is(err, organizations.ErrOwnerAuthority) {
|
||||
t.Fatalf("non-owner owner-suspension err=%v", err)
|
||||
}
|
||||
if err = organizationService.RemoveMembershipIfCurrent(t.Context(), organizations.MembershipRemoval{OrganizationID: organization.ID, UserID: owner.ID, ExpectedStatus: "active", ActorUserID: member.ID, RequestID: "request-remove-owner-without-authority"}); !errors.Is(err, organizations.ErrOwnerAuthority) {
|
||||
t.Fatalf("non-owner owner-removal err=%v", err)
|
||||
}
|
||||
if err = organizationService.SetMembershipStatus(t.Context(), organization.ID, owner.ID, "suspended", member.ID, "request-legacy-suspend-owner-without-authority"); !errors.Is(err, organizations.ErrOwnerAuthority) {
|
||||
t.Fatalf("legacy non-owner owner-suspension err=%v", err)
|
||||
}
|
||||
if err = organizationService.RemoveMembership(t.Context(), organization.ID, owner.ID, member.ID, "request-legacy-remove-owner-without-authority"); !errors.Is(err, organizations.ErrOwnerAuthority) {
|
||||
t.Fatalf("legacy non-owner owner-removal err=%v", err)
|
||||
}
|
||||
|
||||
type replacementResult struct {
|
||||
binding access.Binding
|
||||
|
||||
Reference in New Issue
Block a user