Skip to content

fix: show domain rule errors for 403 responses - #260

Open
Guflly wants to merge 1 commit into
apache:mainfrom
Guflly:fix/domain-rule-errors
Open

fix: show domain rule errors for 403 responses#260
Guflly wants to merge 1 commit into
apache:mainfrom
Guflly:fix/domain-rule-errors

Conversation

@Guflly

@Guflly Guflly commented Aug 8, 2026

Copy link
Copy Markdown

What and why

Show Fineract's domain-rule explanation for 403 responses while keeping the generic permission message for authorization failures.

Closes #258

Verification

  • Focused interceptor suite: 13 passed
  • npm run lint
  • npm run build
  • npm run check:icons
  • npm run i18n:check

Screenshots

Not applicable.

Checklist

  • I did not hand-edit generated files under src/app/api/.
  • No new component or service code was added.
  • No user-facing strings were added.
  • I added regression coverage for the domain-rule response.
  • No UI workflow changed.

@Aman-Mittal

Copy link
Copy Markdown
Member

@Guflly please sign your commits

@Aman-Mittal

Copy link
Copy Markdown
Member

Thanks for picking this up — the diagnosis is right, and letting a domain-rule 403 through to the stacked-message branch is the correct shape for the fix. One thing to change before it lands, and it is only visible against a real platform.

The stacked branch prefers developerMessage

The branch this now routes 403s into already reads:

const msg = err['developerMessage'] || err['defaultUserMessage'] || 'Validation error';

developerMessage first. Reproduced against a running Fineract just now:

DELETE /codes/15/codevalues/24   →   403
  errors[0].developerMessage   : "The request caused a data integrity issue to be fired by the database."
  errors[0].defaultUserMessage : "This code value is in use"

So after this change that user stops seeing "You do not have permission to perform this action" and starts seeing "The request caused a data integrity issue to be fired by the database." — which is not an improvement, and is the same category of problem the comment left in place is warning about. The identical preference sits in the fallback a few lines below.

Flipping it to defaultUserMessage || developerMessage in both places gives "This code value is in use", which is the message the platform wrote for the user.

The new test does not catch this because its fixture has no developerMessage on the error entry. Real Fineract bodies always carry one — the group-closure example in the test looks like this in practice:

{
  "errors": [
    {
      "developerMessage": "A loan with money outstanding cannot be closed",
      "defaultUserMessage": "A loan with money outstanding cannot be closed",
      "userMessageGlobalisationCode": "error.msg.loan.close.loan.has.money.outstanding",
      "parameterName": "id"
    }
  ]
}

A fixture where the two strings differ would pin the behaviour that matters.

Two smaller notes

isDomainRuleViolation collapses to hasValidationErrors. Since it is code === '…' || hasValidationErrors, and the FORBIDDEN branch is skipped whenever either holds, the globalisation-code half never changes an outcome. Worth dropping it, or keeping it and dropping the || — as written it reads like it does more than it does.

There is also a gap between the two: when the code matches but errors[] is empty, this skips FORBIDDEN, skips the stacked branch, and lands on the top-level developerMessage"Request was understood but caused a domain rule violation."

A question rather than a claim: does the platform's genuine permission-denied 403 also carry an errors[] array? If it does, this change routes it into the stacked branch too, and the user would be shown the NOT_ALLOWED permission code that the original guard existed to hide. I could not construct that case here without a restricted user, so I have not asserted it either way — but it seems worth one test before merge, since it is the only path that could make things worse rather than better.

Happy to be wrong on any of this; the underlying change is one the app needs.

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.

Business-rule refusals are reported as "you do not have permission", because Fineract returns 403 for them

2 participants