Skip to content

Enforce consistent deletion permissions and state rules across single and batch APIs #3326

Description

@zackcl

Blocked by #3323, which adds the batch deletion endpoints this issue covers. Also, the load tests might need to be updated after this change is applied.

Problem

Deletion permissions and state restrictions are enforced in the UI but not by the single and batch deletion APIs.

For example, an authenticated Reader can directly request deletion, and an authenticated caller can delete an Enabled feature flag even though the UI hides Delete.

Scope

  • Apply the same role permissions to single and batch deletion of experiments, feature flags, and segments.
  • Define and enforce consistent state/usage restrictions based on the agreed UI rules.
  • Verify that rejected deletions leave the target unchanged and that batch results clearly report the affected items.
  • Update the load-testing setup to use an authorized account and make test entities deletable before cleanup.

Coordinate the API behavior change with existing callers. Request-size limits and timeout policies are outside this issue.

Rules to enforce

Roles, from the frontend permission map in auth.service.ts:

Role Experiments Feature Flags Segments
ADMIN delete delete delete
CREATOR delete delete delete
USER_MANAGER no no delete
READER no no no

State and usage:

  • Experiments: deletable in Draft, Inactive, Completed (including experiments stored as Cancelled) and Archived. Not deletable in Running, Paused, Preview or Scheduled.
  • Feature flags: not deletable while Enabled.
  • Segments: not deletable while Used. Global-exclude segments are not offered for deletion.

Implementation notes

  • DELETE /segments/:segmentId does not currently receive @CurrentUser(), so role checks there need the parameter added.
  • DeletionRepository.findForDeletion() locks the row but selects only id. Evaluating state or usage rules needs more fields read under the same lock, not a separate unlocked read.
  • DeletionReasonCode only has operational failure codes today, so a policy refusal would surface as delete_failed. Add codes that keep refusals distinguishable.
  • Every @Authorized usage in the backend passes no roles today, so making authorizationChecker honor the roles argument would not change behavior for any existing route.
  • The batch deletion integration tests currently assert that every role and every experiment state can delete through both routes. Those assertions need to be inverted as part of this work.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions