The permission check that did two jobs

I wrote a bug into the client portal about a year ago and only found it because a requirement changed.

There was one check guarding administrative work. isStaff. It answered a single question, and two separate powers hung off it: seeing every client company in the system, and creating user accounts. That was fine. It was fine for a long time, because the only person with staff access was me, and I could do both of those things anyway.

Then somebody needed to see across all the client companies without being able to create logins. Reporting, not administration.

And there was no way to say that. The check couldn’t express it. Granting the visibility handed over account creation in the same movement, because they were the same yes. I couldn’t grant one without the other without going back through every call site that had ever asked isStaff and working out which of the two things it had actually meant.

What was wrong with it

isStaff is a job title. It describes who somebody is.

canCreateAccounts is a permission. It describes what they may do.

Only one of those survives a new requirement. Job titles accumulate powers by accident, because when you’re adding a feature and you need a guard, the flag that’s already there and already means “trusted person” is right in front of you. Nobody sits down and decides that read access should imply account creation. It just ends up that way, one reasonable-looking line at a time.

The tell is when you can’t answer “what does this flag protect?” with one sentence.

The fix

Two questions instead of one. May this person see every client? May this person create accounts?

Six roles fall out of that fairly naturally, across two tiers, and the case that started all this is now just a role that answers yes to the first and no to the second. No new mechanism. The mechanism was always capable of it. I’d written the check at the wrong level of abstraction and then built on top for a year.

Roughly an afternoon of work, most of it reading every call site to decide which question it had really been asking. A few of them wanted both. Two of them wanted neither and had been guarded by isStaff out of caution rather than need.

Why I’m writing it down

Because I don’t think I’d have caught it by reading the code. It looked correct. It was correct, for the set of roles that existed when I wrote it. The bug was latent, and it only became visible when reality asked for something the model couldn’t represent.

The general version, as far as I can tell: a permission named after a person is a permission you’ll have to split later. Name it after the action and you’re naming the thing that doesn’t change.

I’d still rather have found it in a design review than in a feature request.