fix(auth): billing permissions migration + night-auditor refund gate - #362
Merged
Merged
Conversation
…ns() #340 migrated eighteen controllers off the legacy @roles() decorator and deliberately left six behind: accounting, cashier, folio, house-account, notifications and payment. Those six gated every route -- read and write alike -- with one identical realm-role list, so no existing permission key matched it without either widening or narrowing who can record a payment or post a charge. This finishes the migration on the rule of preserving what the realm-role gate was granting in practice, rather than narrowing it silently: - Six controllers move onto @RequirePermissions(), reusing existing keys where they exist (folios.manage, houseaccounts.manage, cashier.access, communications.manage) and adding one new key, accounting.manage, for the deposit / A-R / accounting-code routes, which had none. - ROLE_DEFAULT_PERMISSIONS gains the keys four system roles were already being granted by the old list: front_desk, night_auditor, accounting and reservations. In particular `reservations` was in the allow-list of every billing controller, so its doc comment ("reservations without cashier / folio posting") described a split the code never enforced. - Migration 0023_billing_write_access_grants.sql backfills those grants for properties that already have role_permissions rows, using the same every-property-with-RBAC cross-join as 0015. A property with no RBAC rows gets nothing, by design. - payment-legacy-seam.spec.ts asserted PaymentController's legacy @roles() metadata; it now asserts the permission key, updated in the same commit that changes the controller. - billing-role-controllers-migration.spec.ts covers all six, asserting both that no @roles() metadata is left behind and that the expected key is set. One side effect worth disclosing rather than burying: communications.manage is already held by revenue_manager for an unrelated existing route in groups.controller.ts, so reusing that key on notifications.controller.ts also lets revenue_manager send guest SMS/WhatsApp/Telegram. Reusing the existing key seemed better than inventing a near-duplicate, but it is easy to split if you would rather. README.md and docs/test-stats.json updated for the 15 added test cases and one added file.
Builds on #361 (Charles / modernitconsultants): billing controllers onto @RequirePermissions, plus a payments.refund key so night_auditor can post overnight without void/refund/correct or deposit refund. Seed now reads ROLE_DEFAULT_PERMISSIONS from @telivityhaip/shared/permissions-catalog so demo RBAC cannot drift from the API catalog. Co-authored-by: ModernIT Consultants <charles@modernit.com.au>
2245 tests / 268 files after payments.refund gate + shared catalog move. Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
Collaborator
Author
|
@modernitconsultants thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes / completes #361 by Charles (@modernitconsultants / ModernIT Consultants) — full credit for the six-controller
@Roles→@RequirePermissionsmigration,accounting.manage, catalog widenings, migration0023, and migration specs.What Charles shipped (#361)
@RequirePermissionsaccounting.managekey +ROLE_DEFAULT_PERMISSIONS/0023_billing_write_access_grants.sqlbackfillcommunications.managealso letsrevenue_managersend guest notificationsWhat this PR adds on top
payments.refund— void / refund / correct onPaymentControllerand deposit refund onAccountingController. Granted tofront_desk,reservations,accounting(and admin/GM viaALL_PERMISSIONS). Not granted tonight_auditor(overnight posting stays onfolios.manage/accounting.manage).PaymentControlleronmainwas onlyadmin | GM | front_desk | reservations, not the six-role list. Specs and migration comments say that.@telivityhaip/shared/permissions-catalog;packages/databaseseed builds role grants fromROLE_DEFAULT_PERMISSIONS(Nest-free subpath export).Validation
CI=true)0023applies cleanly onhaip_testSide effects (unchanged from #361, still disclosed)
revenue_manager+ guest notifications via sharedcommunications.managereservationskeeps folio/cashier write access the old realm-role gates already allowed on those controllersPrefer merging this branch and closing #361 as superseded.