Skip to content

feat(auth): tell staff why an elevated endpoint is refused - #4566

Merged
TaprootFreak merged 1 commit into
developfrom
feat/staff-kyc-error-code
Aug 1, 2026
Merged

feat(auth): tell staff why an elevated endpoint is refused#4566
TaprootFreak merged 1 commit into
developfrom
feat/staff-kyc-error-code

Conversation

@TaprootFreak

Copy link
Copy Markdown
Collaborator

Why

A staff member who holds the role but is not KYC-cleared received the generic
{"statusCode":403,"message":"Forbidden resource"} — indistinguishable from a removed role or a
missing endpoint. Neither the person nor any tooling in front of the API could tell that the fix is
to complete an identification, which makes the rollout of the staff KYC gate needlessly opaque.

What

RoleGuard now throws StaffKycRequiredException instead of returning a bare false:

{ "code": "STAFF_KYC_REQUIRED", "message": "Staff access requires a completed identification: KYC level 50 and a verified name on your account" }

Still HTTP 403. The code follows the existing TFA_REQUIRED pattern, so clients branch on a stable
identifier instead of matching prose.

A wrong role still yields the generic 403. The two situations need different actions — obtain a
role vs. complete an identification — and collapsing them into one answer is what caused the problem
in the first place.

The protected KYC file route (KycService.getFileByUid) checked role and clearance through a single
predicate and reported Requires admin or compliance role even when the role was fine. It now
reports each case separately.

JwtUserActiveGuard calls the guard programmatically, but only with UserRole.USER, which is never
elevated — so it still receives a boolean.

Tests

The guard suite now pins the thrown status, the code and the message, and asserts that a wrong role
still returns false without consulting the clearance. The new exception is added to the dedicated
100% coverage gate (test:staff-gate:cov) alongside the other files deciding elevated access.

Follow-up

The matching frontend change (showing the reason and offering to start KYC) is a separate PR in the
services repository.

A staff member who holds the role but is not KYC-cleared got the generic "Forbidden
resource", which is indistinguishable from a removed role. Neither the person nor any
tooling in front of the API could tell that the fix is to complete an identification.

RoleGuard now throws StaffKycRequiredException: HTTP 403 with a machine-readable
code STAFF_KYC_REQUIRED and a message naming the actual requirement. This follows the
existing TFA_REQUIRED pattern, so clients branch on the code instead of matching prose.
A wrong ROLE still yields the generic 403 - the two cases need different actions and
must stay distinguishable.

The protected KYC file route checked both conditions through one predicate and reported
"Requires admin or compliance role" even when the role was fine; it now reports each
case separately.

JwtUserActiveGuard calls the guard programmatically, but only with UserRole.USER, which
is never elevated - so it still receives a boolean.
@TaprootFreak

Copy link
Copy Markdown
Collaborator Author

CI is green on the current head (12/12), including the dedicated 100% coverage gate, which now also
covers the new exception.

Two independent reviews were run. The conformity review returned two minor points. Both concern
documentation and test strictness rather than access behaviour, so they are deliberately not part
of this PR and will be delivered as a follow-up:

  • KycController.getFile does not document the new structured 403 response, although the same
    controller already does this for the analogous 2FA case (@ApiForbiddenResponse).
  • The KYC service tests assert on the base ForbiddenException. Since the new exception extends it, a
    regression back to the previous generic answer would not be caught. The tests should pin the
    concrete exception and its code, and separately pin that a wrong role still yields the generic 403.

A second review pass (logic/correctness) is still in progress. Its findings will go into the same
follow-up — or be raised here immediately if any of them turn out to affect access behaviour rather
than presentation.

The matching frontend change is DFXswiss/services#1233. It is intentionally still a draft: a review
found that the new answer would not reach every staff screen there, which is being fixed before that
PR goes up for review. This PR does not depend on it — until the frontend lands, the improvement is
that clients and staff get a specific reason instead of a bare "Forbidden resource".

@TaprootFreak
TaprootFreak marked this pull request as ready for review August 1, 2026 09:27
@TaprootFreak
TaprootFreak merged commit dc50ea3 into develop Aug 1, 2026
12 checks passed
@TaprootFreak
TaprootFreak deleted the feat/staff-kyc-error-code branch August 1, 2026 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant