Permissions Hadn't Changed, but QA Said 'I Can't Create' — Answering with a Table Uncovered an Unreachable Branch

#java #backend #debugging #codequality

The question

QA for our internal business system sent me this:

I can't create a project. Did it change so that only executives can create them now?

Short answer: no, the permission rules hadn't changed at all. But the question turned out to be valuable. While building a permission table to answer it, I cross-checked the code — and found a branch that could never execute. Dead code, alive in production. Here's how it happened.

Background: we had just changed how roles are stored

Shortly before this, we changed how job roles are modeled.

Before: one role per user
After: one role per affiliation (seat) — to support concurrent posts and secondments, a user can hold multiple affiliations, each with its own role

The project-creation rules themselves were untouched:

Role	Can create projects in
Chairman / Secretary	Entire group
President / Executive	Own company
Department head / Section chief / Team leader	Own org and below
Team member	Nowhere

The "did the system change?" question almost certainly came because of that recent refactor. When a system is being actively changed, every unrelated symptom starts looking like a side effect of "that change."

Trying to answer with a table, I read the code

I could have just replied "nothing changed," but for permission questions, a table is the most reliable answer. To build the table above, I re-read the actual check:

Scope scopeOf(long userSeq, Role role) {
    if (role == CHAIRMAN || role == SECRETARY) return companyWide();
    if (role == PRESIDENT || role == EXECUTIVE) return ownCompany();
    if (role == TEAM_MEMBER) return denied();  // ← early return

    // Per-affiliation branch (added in the refactor):
    // even if the primary affiliation is "team member",
    // a section chief seat elsewhere can create under that org
    for (Affiliation a : affiliationsOf(userSeq)) {
        if (a.role().isLeaderOrAbove()) {
            return orgScope(a.org());
        }
    }
    return denied();
}

See it? The for loop was added during the per-affiliation refactor, precisely to handle "primary affiliation is team member, but section chief somewhere else." But above it, the pre-refactor early return for TEAM_MEMBER was never removed.

Since the role argument carries the primary affiliation's role, any user whose primary role is team member hits return denied() on line 3 — and the loop below it never runs. The code we wrote for concurrent-post users had never executed, not even once.

The compiler can't warn you about this. Syntactically, both branches are reachable. Code written after a return statement triggers an unreachable-code error, but a branch that's unreachable because of how the conditions combine can only be found by a human reading the code — or by a test.

The fix: drop TEAM_MEMBER from the early return

The fix itself was small: stop short-circuiting on "team member," and read the affiliations before judging.

Scope scopeOf(long userSeq, Role role) {
    if (role == CHAIRMAN || role == SECRETARY) return companyWide();
    if (role == PRESIDENT || role == EXECUTIVE) return ownCompany();

    // If any seat is leader-or-above: that org and below. Otherwise: denied.
    for (Affiliation a : affiliationsOf(userSeq)) {
        if (a.role().isLeaderOrAbove()) {
            return orgScope(a.org());
        }
    }
    return denied();
}

I verified with a user whose primary affiliation is a team member but who holds a section-chief seat at another company:

the creation-scope API returns allowed: true with the section-chief org
a project-creation POST under that section returns 201
creation under the primary org still returns 403

One more thing: deployment timing. If the server restarts while QA is in the middle of testing exactly this permission area, they get confused by unrelated errors. I scheduled the release for 14:00, right after lunch break, so the restart landed between test sessions.

Answering QA

The answer to the original question was simple: "The permission rules haven't changed — here's who can create where," with the table attached. And for the cases where creation still fails, I asked three things:

which account, and that account's affiliations and roles
whether the create button is missing or clicking it errors out (and if so, the exact message)
which organization was selected as the creation target

A bare "it doesn't work" is unreproducible for permission bugs, so having these triage questions templated saves a lot of round trips.

Takeaways

When you add a branch below an early return, check that the branch is actually reachable. The compiler won't catch condition-level unreachability; only a human read or a test will. If we'd had a concurrent-post user in the test suite from day one, we'd have caught this at refactor time.
Answer permission questions with a table. Building the table forces you to re-read the implementation, and mismatches between the rules and the code — like this dead branch — surface along the way.
Schedule permission-related deploys outside QA's testing hours. A single restart at the wrong moment costs you trust.

A naive-sounding "did the system change?" ended up unearthing a latent bug. I'm grateful for the question.

DEV Community