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
endTwo deliberate choices:
*_enabled_locked?rather than*_enabled_locked_by_ancestor?—cascading_attribute_locked?ORslocked_by_ancestor?andlocked_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>_enabledparams need no guard: the framework'sdefine_attr_writeralready short-circuits a write whose value matches the cascaded ancestor value, so no-ops never mark the record dirty. A genuine conflicting_enabledwrite still returns 400, which is the correct outcome for an ancestor-locked attribute. - A direct
lock_<attr>_enabledparam that is a no-op against an existing ancestor or instance lock still returns 400. This is pre-existing behaviour indefine_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_adminguards are absent onmasterforduo_custom_agents,duo_custom_flows,duo_external_agents, and for every_availabilitykey. Pre-existing, not introduced here, and not a functional access-control gap because both entry points already require:admin_groupupfront. Suitable for a defence-in-depth follow-up.
References
- Issue: #599212 (closed)
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
- Create a parent group and a child group.
- As an owner of the parent group, set
duo_foundational_flows_availability=false(this locks the attribute for all descendants). - As an owner of the child group,
PUT /api/v4/groups/:child_idwith{ duo_foundational_flows_availability: false, duo_features_enabled: true }. - Before: 400 with the lock error,
duo_features_enablednot saved. After: 200,duo_features_enabledsaved, locked param silently dropped.
Instance lock
- In the Admin area, lock Duo foundational flows instance-wide (
lock_duo_foundational_flows_enabled=true). - As a group owner,
PUT /api/v4/groups/:idwith the same params as above. - 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.