chef / chef/chef-server

Potential race condition with ACL & group editing

Open
#26 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Status: To be prioritized Triage: Try Reproducing Type: Bug
Dominant language
Erlang
Stars
303
Forks
211
Avg merge
1d 8h
Merged PRs (30d)
5

Description

As I've been working on adding orgs+RBAC to goiardi, I noticed that the behavior oc-chef-pedant expects when you edit the ACL or groups is to provide a list of the actors and groups to be in the group, whereupon the existing actors and groups in the ACL or group is cleared out and the actors and groups in the request are added back in.

This leads to a situation where if two people are simultaneously editing a group or ACL, or if a user is being created at the same moment a group is being edited, one of the changes could be overwritten. This could lead to strange situations where a user that's been added to an org is not in the users group, or trying to remove access from a group or actor gets overwritten and they retain their access.

It may be better to explicitly add and remove users from ACLs and groups to prevent this. It might be a little more cumbersome for the tooling, but I think it would be safer all around.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

No files, tests, or entry points are named. Start by locating the ACL and group update APIs, then examine how concurrent edits and simultaneous user creation are handled. Done means explicit additions and removals no longer allow concurrent changes to overwrite one another.

Written by the indexing model from the issue text.

Assessment

Tech stack
erlang
Domain
authorization
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.