Fix 400 on group settings PUT when ancestor locks Duo availability

What does this MR do and why?

Fixes a 400 error on PUT /api/v4/groups/:id (and on the equivalent group settings form save) that occurred whenever a Duo availability attribute was locked either by an ancestor group or by an instance-level application setting, and a descendant group's settings were saved — even when the user only changed an unrelated Duo setting.

The error surfaced as:

{"message":{"namespace_settings.lock_duo_foundational_flows_enabled":["cannot be changed because it is locked by an ancestor"]}}

Root cause

The _availability= compound setter on NamespaceSetting writes both the _enabled and lock_*_enabled columns. The lock_<attr>_changeable? validator in CascadingNamespaceSettingAttribute then rejects any change to the lock column when the attribute is locked above the current group, without checking whether the write is a no-op.

Fix

Add an early-return guard to each of the five compound _availability= setters in ee/app/models/ee/namespace_setting.rb, so the setter is a no-op when the attribute is already locked above this group:

def duo_foundational_flows_availability=(value)
  return if duo_foundational_flows_enabled_locked?(include_self: false)

  self.duo_foundational_flows_enabled = value
  self.lock_duo_foundational_flows_enabled = !value
end

Two deliberate choices:

  • *_enabled_locked? rather than *_enabled_locked_by_ancestor?cascading_attribute_locked? ORs locked_by_ancestor? and locked_by_application_setting?, so the fix covers instance-wide locks on Self-Managed as well as group-hierarchy locks. An earlier iteration of this MR guarded only ancestor locks and left the instance-lock case broken.
  • include_self: false — a group that holds the lock itself must remain able to change or release it.

The guard lives next to the code that performs the write, so it applies to every caller (REST API, group settings form, GraphQL, internal callers) rather than to one service. An earlier iteration stripped params in EE::NamespaceSettings::AssignAttributesService; that approach was replaced per reviewer feedback and the service is now back to its state on master.

Covers all five attributes that share the compound-setter pattern: duo_foundational_flows, duo_remote_flows, duo_custom_agents, duo_custom_flows, duo_external_agents.

Scope and known follow-ups

  • The guard covers the _availability= compound setter. Direct <attr>_enabled params need no guard: the framework's define_attr_writer already short-circuits a write whose value matches the cascaded ancestor value, so no-ops never mark the record dirty. A genuine conflicting _enabled write still returns 400, which is the correct outcome for an ancestor-locked attribute.
  • A direct lock_<attr>_enabled param that is a no-op against an existing ancestor or instance lock still returns 400. This is pre-existing behaviour in define_lock_attr_writer / lock_<attr>_changeable? and affects every cascading setting, not just Duo. Left out of scope for this minimal fix.
  • validate_settings_param_for_admin guards are absent on master for duo_custom_agents, duo_custom_flows, duo_external_agents, and for every _availability key. Pre-existing, not introduced here, and not a functional access-control gap because both entry points already require :admin_group upfront. Suitable for a defence-in-depth follow-up.

References

Screenshots or screen recordings

N/A — backend-only fix, no UI changes.

Scenario Before After
Ancestor group locks duo_foundational_flows, child group PUT /api/v4/groups/:id 400, unrelated settings in the same request are not saved 200, locked param is a no-op, unrelated settings are saved
Instance application setting locks duo_foundational_flows, group PUT /api/v4/groups/:id 400 200, locked param is a no-op, unrelated settings are saved

How to set up and validate locally

Ancestor lock

  1. Create a parent group and a child group.
  2. As an owner of the parent group, set duo_foundational_flows_availability=false (this locks the attribute for all descendants).
  3. As an owner of the child group, PUT /api/v4/groups/:child_id with { duo_foundational_flows_availability: false, duo_features_enabled: true }.
  4. Before: 400 with the lock error, duo_features_enabled not saved. After: 200, duo_features_enabled saved, locked param silently dropped.

Instance lock

  1. In the Admin area, lock Duo foundational flows instance-wide (lock_duo_foundational_flows_enabled=true).
  2. As a group owner, PUT /api/v4/groups/:id with the same params as above.
  3. Before: 400. After: 200.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Duncan

Merge request reports

Loading
Loading