Permission Bugs Are Silent: What an Adversarial Review Found Before Release

This is the engineering companion to our intranet platform case study. The platform is built to give employees of a regulated, two-entity, four-territory insurance group self-service access from home with no VPN, which means every meaningful action has to be authenticated, authorized, and audited, and the authorization layer carries most of the weight. This article covers the two bugs that layer had before release, how they were caught, and the design rules we would now start with anywhere.

First, the framing that everything else hangs on. A permission that doesn't work looks exactly like a permission that does. It doesn't throw. It doesn't log. The specification says the grant exists, the admin screen shows it assigned, the UI renders normally, and nothing anywhere tells you it isn't being enforced. You find out the other way: when something appears in front of the person who most shouldn't have seen it, or when an approval that should have required two people quietly took one.

The architecture: identity and authorization live apart

The identity provider answers exactly one question: who is this person. Roles, group memberships, permission bundles, and per-user overrides all live inside the application. The reason: a directory misconfiguration must never be able to grant business authority. Permissions kept in the directory put your approval limits in the hands of whoever administers the directory, and that is usually a different person, with different training and different change control.

The layers also double-check each other. The core engine independently validates the token on every request and ignores permission headers forwarded from the API gateway, even though the gateway is our own code. The second layer assumes the first might be wrong. The resolver itself evaluates in a fixed order (base role, group bundles, direct bundles, grant overrides, then deny overrides, then scope and legal-entity checks, then field-level filtering) with two boring choices at its heart. Expired records are stripped before evaluation begins, so an expired deny can never be outlived by a lingering grant. And a deny beats every grant, unconditionally, because the clever precedence rule is the one nobody can reason about at 4pm on a Friday.

Permission changes need their own controlled workflow

The resolver is only half the authorization system. The other half is how authority enters and leaves it. An access change moves through request, approval, application and, where necessary, revocation. It carries a mandatory reason and can carry an expiry. The requester and approver are different people. In other words, the system that grants approval rights is itself subject to approval.

Expiry is applied before precedence, not treated as another competing rule. An expired record is absent from the decision. This prevents a stale deny and a stale grant from participating in a precedence calculation nobody intended. Time-limited exceptional access uses the same machinery, so it expires through the normal resolver rather than depending on somebody to remember a calendar reminder.

Every access change writes actor, target, reason, scope, request identifier and before-and-after state into the append-only audit trail. There is no application update or delete route for those events. Restricting a delete permission is weaker: whatever can be denied can later be granted. Omitting the mutation path entirely turns immutability from a policy into a property of the application.

Bug one: the resolver that only worked at the widest scope

A late security review found that one domain's permission checks only ever resolved at the broadest scope tier. Every narrower grant the specification described, department-scoped and resource-scoped approval authority, was silently non-functional. Everything about it looked healthy. The permission keys existed. Administrators could assign them. The UI showed them assigned. And nothing enforced them.

No test caught it, and more tests would have changed nothing, because the tests were written against the same misunderstanding as the code. That is the defining property of this bug class. The error lives in the mental model, so everything derived from the model, tests included, agrees with the bug. It was caught by a review step whose entire job is to assume the implementation is wrong and go looking.

The fix introduced scoped-then-fallback resolution across the affected checks. Then the fix caused a new problem: resolving scope per row instead of once turned list endpoints into a per-row query storm. That regression was caught too, and closed with a per-request cache. The regression matters as much as the finding: it is the beat that shows the review ladder actually working.

Bug two: the empty set that approved itself

The second finding is the best teaching example we have ever shipped, because the offending line is idiomatic, reasonable, and passes every review filter you'd normally apply. An approval check used a universal quantifier over a collection, effectively "every configured second approver must match the acting user." But the collection could be empty, and all members of an empty set satisfy any condition. So where the check was meant to confirm the actor is the specifically named second approver, it silently returned true whenever no second approver was configured at all, meaning any holder of the second-approve permission could clear a pending reservation with no second-tier requirement.

It would have passed type-checking, code review, and any test written against the case its author was thinking about. What caught it was someone asking: what does this do when the list is empty? That question, what does this do in the degenerate case, is a repeatable, teachable review discipline, and it has caught something every time we have applied it seriously. We corrected both issues during the pre-release review; neither was present in production.

Authorization is evaluated again when the world changes

A request can be valid when submitted and invalid when approved. The room may have been booked in the meantime; the actor's scope may have expired; the resource may now be in a blackout period. Approval therefore reruns the relevant authorization and conflict checks instead of trusting the result captured at request time. The request records intent. It does not reserve yesterday's state of the world forever.

The booking model makes that concrete. A reservation is a claim on a set of resources, not just a room. If the room carries a linked projector, the set is checked atomically, and a conflict on the projector blocks the request even when the room appears free. Recurring requests materialise separate occurrences, each conflict-checked, with skipped dates reported. This avoids a compact recurrence rule hiding a collision six weeks from now.

In the helpdesk, concurrent transitions use a state precondition rather than only a generic version check. Both stop a stale write, but they explain it differently. One says the record changed; the other says another agent already moved this ticket to in-progress. The guard that protects state should also give the user enough context to recover without opening a second system or guessing what happened.

Negative requirements never show up in a demo

Beyond the two bugs, the platform's quiet theme is that the requirements that matter in internal software are negative ones, who must never see this, and negative requirements are invisible in a demo. Nobody watching a directory demo notices the field missing from the response. So the rules are enforced where omission is structural:

• Visibility is enforced in the query, not the template. A list endpoint returning a field the caller shouldn't see is a bulk export, and the UI simply not rendering the column hides the leak perfectly.
• Counts are disclosure channels. Protected documents are filtered out of category counts as well as listings. A category showing twelve and listing eleven announces that a restricted document exists, and roughly where.
• Conflict messages are disclosure channels. A room-booking conflict returns the resource and time window only, never the other booking's owner or purpose. The naive message tells an entire company who is meeting whom about what.
• Sensitive data lives in separate tables that a list serializer cannot reach even in error, because the table is simply absent from its queryset. And audit events for those fields record field names, never values, since an audit log of secrets is a second place to steal the secrets from.
• Exports omit sensitive columns entirely; a blanked column still reveals that the field exists. Exporting requires a permission distinct from viewing, and every export records exactly which columns left.

One more structural choice deserves a sentence. The audit trail has no application update or delete path at all; the code that would mutate it was never written. A permission that can be revoked is a permission that can be granted, so the only reliable way to guarantee an immutable trail is for the mutation path not to exist.

Test denials as contracts, then review the model

The API contract is the source of truth for the application boundary, and negative permission cases belong in its tests: a role that cannot see a restricted field, an expired override that stops working, a deny that defeats a grant, an entity boundary that trims the row set. These tests are valuable because they stop a later refactor from weakening a rule already understood.

They cannot prove the permission vocabulary itself is complete. That is why the review ladder has both tests and an adversarial pass over the cumulative implementation, followed by a live smoke test. Tests ask whether the code continues to express our model. Review asks whether the model omitted a scope, a state or a degenerate case. The two bugs in this article survived the first question and failed the second.

The takeaway

You cannot test your way out of authorization bugs, because the dangerous ones are born from the same misunderstanding your tests inherit. The defences that work are structural and procedural together: layers that independently verify, precedence rules boring enough to reason about, visibility enforced at the query, existence concealed as well as access blocked, and a scheduled adversarial review whose brief is to assume you got it wrong. On this project, that reviewer found an approval control that one idiomatic line had switched off. Finding it before release is the process working, and before release is the only time it counts.