Skip to content

Commit ccad1d0

Browse files
refactor(policy): decentralize subject resolution to modules (#3382)
* refactor(policy): decentralize subject resolution to modules Replace hardcoded resolveSubject() and deriveSubjectType() in lib/middlewares/policy.js with a registry pattern. Each module policy file now exports a *SubjectRegistration() function that registers its own document-level and path-level CASL subjects during discoverPolicies(). Adds lib/helpers/authorize.js as a simple route-level middleware helper. Closes #3381 * docs(skills): enforce decentralized policy pattern in skills * docs(skills): enforce module autonomy and decentralization in skills * fix(policy): address CodeRabbit review — idempotency, path precision, docs * fix(policy): complete JSDoc returns, add deprecation window, clarify feature skill boundaries * fix(skills): allow justified lib/services/ changes alongside lib/helpers/ in feature boundaries * fix(policy): use response helper in authorize, complete JSDoc returns on all policy registrations
1 parent 0dee70a commit ccad1d0

14 files changed

Lines changed: 270 additions & 66 deletions

File tree

.claude/skills/create-module/SKILL.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,19 @@ Example: `config.billing.plans` used in both `billing.subscription.model.mongoos
6464

6565
> Reference: `modules/users/models/users.schema.js` uses `z.enum(config.whitelists.users.roles)`.
6666
67-
### 5. Apply renames carefully
67+
### 5. Module autonomy
68+
69+
- Everything the module needs lives inside `modules/{module}/` — policies, config, routes, tests, doc
70+
- Module registers its own capabilities (subjects, abilities, config) via exports — auto-discovered by the core
71+
- **NEVER modify shared files** (`lib/middlewares/`, `lib/services/`, `config/`) to add module-specific logic
72+
- If the module needs authorization: create `policies/{module}.policy.js` with ability builder + subject registration exports
73+
74+
### 6. Apply renames carefully
6875

6976
- Case-sensitive, whole-word matches where possible
7077
- Show plan before applying if many files affected
7178
- Don't rename unrelated code (e.g., "tasks" in comments about other features)
7279

73-
### 6. Verify & report
80+
### 7. Verify & report
7481

7582
Run `/verify`, then report: module path, renamed tokens, lint/test results, next steps (customize schema/services — routes auto-discovered via `modules/*/routes/*.js`).

.claude/skills/feature/SKILL.md

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,13 @@ description: Implement a new feature or modify existing functionality. Use when
1212
- Which module? Default to **ONE** unless justified.
1313
- **If the module doesn't exist** → run `/create-module` to scaffold it first, then continue.
1414

15-
### 2. Analyze flows & edge cases
15+
### 2. Module boundaries
16+
17+
- All new code isolated inside the target module (`modules/{module}/`)
18+
- No modifications to shared core files (`lib/middlewares/`, `config/`) — except `config/templates/` for new email templates; additions to `lib/helpers/` or `lib/services/` require explicit justification
19+
- Module registers its own capabilities via exports (policies, subjects) and file discovery (config auto-discovered by filepath pattern) — auto-discovered by the core
20+
21+
### 3. Analyze flows & edge cases
1622

1723
For each user-facing flow this feature creates or modifies, identify:
1824

@@ -22,13 +28,13 @@ For each user-facing flow this feature creates or modifies, identify:
2228
- **Retry edge** — can the user retry after failure/rejection? (check unique indexes)
2329
- **Multi-user impact** — who else is affected? Do they need notification?
2430

25-
### 3. Check boilerplate resilience
31+
### 4. Check boilerplate resilience
2632

2733
- Works WITHOUT mailer configured? (graceful skip, no crash)
2834
- Works WITHOUT organizations enabled?
2935
- No hard dependency on external services for core flow?
3036

31-
### 4. Present plan & ask questions
37+
### 5. Present plan & ask questions
3238

3339
**STOP and present to the user:**
3440
- Flows identified (happy + error + edge cases)
@@ -39,7 +45,7 @@ For each user-facing flow this feature creates or modifies, identify:
3945

4046
## Phase 1 — Implementation
4147

42-
### 5. Apply layer rules
48+
### 6. Apply layer rules
4349

4450
Strict order — never skip or reverse:
4551

@@ -51,14 +57,14 @@ Routes → Controllers → Services → Repositories → Models
5157
- **Services**: Business logic, call repositories, throw `AppError`
5258
- **Repositories**: Database only — sole layer importing mongoose
5359

54-
### 6. Apply modularity rules
60+
### 7. Apply modularity rules
5561

5662
- Isolate inside module boundary
5763
- No cross-module imports unless justified (shared code → `lib/helpers/`)
5864
- **No cross-module Repository/Model imports** — a service must never import another module's repositories or models; use the target module's Service instead
5965
- Follow `/naming` conventions
6066

61-
### 7. Handle notifications
67+
### 8. Handle notifications
6268

6369
If an action affects another user:
6470
- Use `lib/helpers/mailer/` abstraction (never nodemailer directly)
@@ -68,7 +74,7 @@ If an action affects another user:
6874

6975
## Phase 2 — Definition of Done
7076

71-
### 8. Self-review checklist
77+
### 9. Self-review checklist
7278

7379
**Edge cases:**
7480
- [ ] "Last one" handled (last owner can't leave/be demoted)
@@ -83,6 +89,11 @@ If an action affects another user:
8389
- [ ] New enum values added to ALL schema definitions (Mongoose model `enum`, Zod `z.enum`, tests)
8490
- [ ] Grep existing enum values to find all locations before committing
8591

92+
**Module autonomy:**
93+
- [ ] No shared-file changes unless explicitly required; any `lib/helpers/` or `lib/services/` additions include explicit justification, and `config/templates/` is used for new email templates
94+
- [ ] Module self-registers capabilities (subjects, abilities) via policy exports
95+
- [ ] New routes/middleware defined inside the module boundary
96+
8697
**Modularity:**
8798
- [ ] Isolated in ONE module (or justified)
8899
- [ ] No cross-module Repository/Model imports (use target module's Service)
@@ -95,10 +106,10 @@ If an action affects another user:
95106
**Error documentation:**
96107
- [ ] If a non-obvious bug was fixed, add a single-line entry to `ERRORS.md` (root of repo) using format: `[YYYY-MM-DD] <scope>: <wrong> -> <right>` (see existing examples in `ERRORS.md`)
97108

98-
### 8b. Elegance check
109+
### 9b. Elegance check
99110

100111
For non-trivial changes: pause and ask yourself "is there a simpler or more elegant approach?" If the current implementation feels hacky, refactor before proceeding.
101112

102-
### 9. Run `/verify`
113+
### 10. Run `/verify`
103114

104-
### 10. Run `/pull-request`
115+
### 11. Run `/pull-request`

.claude/skills/verify/SKILL.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ description: Run quality loop (audit + lint + tests) to verify code quality, cor
1717
- Silent error swallowing (catch + console.log)
1818
- Config in wrong location, missing or orphaned files
1919
- Cross-module import violations
20+
- Module-specific logic added to shared files (`lib/middlewares/`, `lib/services/`, `config/`)
21+
- Module not self-registering its capabilities (subjects, abilities, config)
22+
- Cross-module dependencies that should use the target module's service instead
2023
- Inconsistent API response formats
2124
- Missing null/undefined checks on async returns
2225
- Fix all issues found before proceeding

MIGRATIONS.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,29 @@ Breaking changes and upgrade notes for downstream projects.
44

55
---
66

7+
## Decentralized Policy Subject Resolution (2026-04-03)
8+
9+
Subject resolution in `lib/middlewares/policy.js` is now registry-based instead of hardcoded. Each module's policy file exports a `*SubjectRegistration()` function that registers its own document-level and path-level subjects during `discoverPolicies()`.
10+
11+
### What changed
12+
13+
- `resolveSubject()` iterates `documentSubjectRegistry` instead of hardcoded if/else chain
14+
- `deriveSubjectType()` iterates `pathSubjectRegistry` instead of hardcoded if/else chain
15+
- New exports: `registerDocumentSubject`, `registerPathSubject`
16+
- New helper: `lib/helpers/authorize.js` — simple middleware for route-level CASL checks
17+
- Each module policy file now exports a `*SubjectRegistration({ registerDocumentSubject, registerPathSubject })` function
18+
19+
### Action for downstream
20+
21+
1. Run `/update-stack` to pull the change
22+
2. If you have custom modules with policy files, add a `*SubjectRegistration()` export following the pattern in any existing module (e.g. `modules/tasks/policies/tasks.policy.js`)
23+
3. `policy.isAllowed` continues to work unchanged — no route file modifications needed
24+
4. Optional: use `authorize(action, subject)` from `lib/helpers/authorize.js` for simple route guards
25+
26+
> **Deprecation notice**: `policy.isAllowed` is supported for this release cycle only. New routes should use `authorize(action, subject)` from `lib/helpers/authorize.js`. Custom modules using `policy.isAllowed` should migrate to `authorize()` before the next major version. The legacy middleware will be removed once all built-in module routes have been migrated.
27+
28+
---
29+
730
## Audit GDPR Flags (2026-03-26)
831

932
New config flags to control IP and User-Agent capture in audit logs for GDPR compliance.

lib/helpers/authorize.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* Module dependencies
3+
*/
4+
import responses from './responses.js';
5+
6+
/**
7+
* Simple middleware helper for route-level CASL authorization.
8+
* Designed to run after policy.isAllowed (which attaches req.ability).
9+
* Usage: router.get('/api/users/me', authorize('read', 'UserAccount'), controller.me);
10+
*
11+
* @param {string} action - CASL action (read, create, update, delete)
12+
* @param {string} subject - CASL subject type string
13+
* @returns {Function} Express middleware
14+
*/
15+
const authorize = (action, subject) => (req, res, next) => {
16+
if (req.ability && req.ability.can(action, subject)) return next();
17+
return responses.error(res, 403, 'Unauthorized', 'User is not authorized')();
18+
};
19+
20+
export default authorize;

lib/middlewares/policy.js

Lines changed: 71 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,48 @@ const methodToAction = {
1717
*/
1818
const abilitiesRegistry = [];
1919

20+
/**
21+
* Registry of document-level subject resolvers.
22+
* Each entry maps a req property to a CASL subject type string.
23+
* Modules register themselves via registerDocumentSubject() during startup.
24+
* @type {Array<{reqProperty: string, subjectType: string}>}
25+
*/
26+
const documentSubjectRegistry = [];
27+
28+
/**
29+
* Registry of path-level subject resolvers.
30+
* Each entry maps a route path (exact or prefix) to a CASL subject type string.
31+
* Modules register themselves via registerPathSubject() during startup.
32+
* Entries are matched in registration order; first match wins.
33+
* @type {Array<{routeMatch: string|Function, subjectType: string}>}
34+
*/
35+
const pathSubjectRegistry = [];
36+
37+
/**
38+
* Register a document-level subject resolver.
39+
* When a request property (e.g. 'task') is present on req, the corresponding
40+
* subject type (e.g. 'Task') is used for CASL document-level checks.
41+
* @param {string} reqProperty - Property name on the Express req object
42+
* @param {string} subjectType - CASL subject type string
43+
* @param {Function} [guard] - Optional guard function(req) => boolean; when provided,
44+
* the entry only matches if guard returns true (allows excluding certain routes)
45+
* @returns {void} - Modifies the documentSubjectRegistry in place
46+
*/
47+
const registerDocumentSubject = (reqProperty, subjectType, guard) => {
48+
documentSubjectRegistry.push({ reqProperty, subjectType, guard });
49+
};
50+
51+
/**
52+
* Register a path-level subject resolver.
53+
* Used for collection-level CASL checks when no document is loaded.
54+
* @param {string|Function} routeMatch - Exact route path string, or a function(routePath) => boolean
55+
* @param {string} subjectType - CASL subject type string
56+
* @returns {void} - Modifies the pathSubjectRegistry in place
57+
*/
58+
const registerPathSubject = (routeMatch, subjectType) => {
59+
pathSubjectRegistry.push({ routeMatch, subjectType });
60+
};
61+
2062
// Lazy CASL loader — dynamic import avoids Jest's experimental VM module linker
2163
let _casl = null;
2264

@@ -34,6 +76,7 @@ const loadCasl = async () => {
3476
* @param {Object} entry - Object with optional `abilities` and `guestAbilities` functions
3577
* @param {Function} [entry.abilities] - Called with (user, membership, { can, cannot }) for authenticated users
3678
* @param {Function} [entry.guestAbilities] - Called with ({ can, cannot }) for guests
79+
* @returns {void}
3780
*/
3881
const registerAbilities = (entry) => {
3982
abilitiesRegistry.push(entry);
@@ -114,20 +157,16 @@ const normalizeForCasl = (doc) => {
114157

115158
/**
116159
* Resolve the CASL subject type string from a loaded document on the request.
117-
* Maps known req properties to their CASL subject type name.
160+
* Iterates the documentSubjectRegistry populated by module policy files.
118161
* Documents are normalized so populated references become bare ObjectIds.
119162
* @param {Object} req - Express request object
120163
* @returns {{ subjectType: string, document: Object }|null} subject info or null
121164
*/
122165
const resolveSubject = (req) => {
123-
if (req.task) return { subjectType: 'Task', document: normalizeForCasl(req.task) };
124-
if (req.upload) return { subjectType: 'Upload', document: normalizeForCasl(req.upload) };
125-
// For user admin routes, the loaded user is stored in req.model by userByID param middleware
126-
if (req.model) return { subjectType: 'UserAdmin', document: normalizeForCasl(req.model) };
127-
if (req.membershipDoc) return { subjectType: 'Membership', document: normalizeForCasl(req.membershipDoc) };
128-
// Billing routes use req.organization for context but authorize via route-derived subjects
129-
if (req.organization && !req.route?.path?.startsWith('/api/billing')) {
130-
return { subjectType: 'Organization', document: normalizeForCasl(req.organization) };
166+
for (const { reqProperty, subjectType, guard } of documentSubjectRegistry) {
167+
if (!req[reqProperty]) continue;
168+
if (guard && !guard(req)) continue;
169+
return { subjectType, document: normalizeForCasl(req[reqProperty]) };
131170
}
132171
return null;
133172
};
@@ -149,6 +188,8 @@ const isAllowed = async (req, res, next) => {
149188
if (!action) return responses.error(res, 405, 'Method Not Allowed', `HTTP method ${req.method} is not supported`)();
150189

151190
const ability = await defineAbilityFor(req.user, req.membership || null);
191+
// Attach ability to req so downstream middleware (e.g. authorize()) can reuse it
192+
req.ability = ability;
152193
const subjectInfo = resolveSubject(req);
153194

154195
if (subjectInfo) {
@@ -164,54 +205,22 @@ const isAllowed = async (req, res, next) => {
164205
return responses.error(res, 403, 'Unauthorized', 'User is not authorized')();
165206
};
166207

167-
/**
168-
* Mapping of exact route paths to CASL subject types for account-related routes.
169-
* Self-service routes use 'UserAccount', admin routes use 'UserAdmin',
170-
* and the base '/api/users' uses 'UserSelf' to split update/delete (user).
171-
* @type {Object<string, string>}
172-
*/
173-
const USER_ROUTE_SUBJECTS = {
174-
'/api/users/me': 'UserAccount',
175-
'/api/users/terms': 'UserAccount',
176-
'/api/users/password': 'UserAccount',
177-
'/api/users/avatar': 'UserAccount',
178-
'/api/users/data': 'UserAccount',
179-
'/api/users/data/mail': 'UserAccount',
180-
'/api/users/stats': 'UserStats',
181-
'/api/users': 'UserSelf',
182-
};
183-
184208
/**
185209
* Derive a CASL subject type string from an Express route path.
186-
* Maps known route path prefixes to their corresponding subject type.
187-
* User routes are split into several subject types:
188-
* - 'UserAccount' for self-service (me, terms, password, avatar, data)
189-
* - 'UserSelf' for the base /api/users path (user update/delete own account + admin list)
190-
* - 'UserAdmin' for admin management (get/update/delete by userId, pagination)
210+
* Iterates the pathSubjectRegistry populated by module policy files.
211+
* Each entry's routeMatch can be an exact string, or a function(routePath) => boolean.
212+
* First match wins.
191213
* @param {string} routePath - Express route path (e.g. '/api/tasks' or '/api/users/me')
192214
* @returns {string|null} CASL subject type or null if not mappable
193215
*/
194216
const deriveSubjectType = (routePath) => {
195-
if (routePath === '/api/billing/checkout') return 'BillingCheckout';
196-
if (routePath === '/api/billing/portal') return 'BillingPortal';
197-
if (routePath === '/api/billing/subscription') return 'BillingSubscription';
198-
if (routePath === '/api/billing/usage') return 'BillingUsage';
199-
if (routePath === '/api/billing/plans') return 'BillingPlans';
200-
if (routePath.startsWith('/api/audit')) return 'AuditLog';
201-
if (routePath.startsWith('/api/tasks')) return 'Task';
202-
if (routePath.startsWith('/api/uploads')) return 'Upload';
203-
if (routePath.startsWith('/api/home')) return 'Home';
204-
if (routePath === '/api/admin/readiness') return 'Readiness';
205-
if (routePath.startsWith('/api/admin/organizations')) return 'Organization';
206-
if (routePath.includes('/requests')) return 'Membership';
207-
if (routePath.includes('/members')) return 'Membership';
208-
if (routePath.startsWith('/api/organizations')) return 'Organization';
209-
// Check exact account route matches first
210-
if (USER_ROUTE_SUBJECTS[routePath]) return USER_ROUTE_SUBJECTS[routePath];
211-
// Account routes fallback
212-
if (routePath.startsWith('/api/users')) return 'UserAccount';
213-
// Admin user routes (/:userId, /page/:userPage)
214-
if (routePath.startsWith('/api/admin/users')) return 'UserAdmin';
217+
for (const { routeMatch, subjectType } of pathSubjectRegistry) {
218+
if (typeof routeMatch === 'function') {
219+
if (routeMatch(routePath)) return subjectType;
220+
} else if (routeMatch === routePath) {
221+
return subjectType;
222+
}
223+
}
215224
return null;
216225
};
217226

@@ -223,6 +232,11 @@ const deriveSubjectType = (routePath) => {
223232
* @returns {Promise<void>}
224233
*/
225234
const discoverPolicies = async (policyPaths) => {
235+
// Clear registries for idempotency (bootstrap may be called multiple times in tests)
236+
abilitiesRegistry.length = 0;
237+
documentSubjectRegistry.length = 0;
238+
pathSubjectRegistry.length = 0;
239+
226240
for (const policyPath of policyPaths) {
227241
const mod = await import(path.resolve(policyPath));
228242
const entry = {};
@@ -235,6 +249,10 @@ const discoverPolicies = async (policyPaths) => {
235249
} else if (normalizedKey.endsWith('abilities')) {
236250
entry.abilities = value;
237251
}
252+
// Call subject registration functions exported by module policy files
253+
if (normalizedKey.endsWith('subjectregistration')) {
254+
value({ registerDocumentSubject, registerPathSubject });
255+
}
238256
}
239257
// Also support old invokeRolesPolicies pattern during transition — skip if new-style exports found
240258
if (entry.abilities || entry.guestAbilities) {
@@ -248,6 +266,8 @@ const discoverPolicies = async (policyPaths) => {
248266

249267
export default {
250268
registerAbilities,
269+
registerDocumentSubject,
270+
registerPathSubject,
251271
defineAbilityFor,
252272
isAllowed,
253273
discoverPolicies,

modules/audit/policies/audit.policy.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,16 @@
22
* Audit ability definitions for CASL document-level authorization.
33
*/
44

5+
/**
6+
* Register audit-related subjects for path-level resolution.
7+
* @param {Object} registry - Subject registration helpers
8+
* @param {Function} registry.registerPathSubject - Register route path → subject type
9+
* @returns {void}
10+
*/
11+
export function auditSubjectRegistration({ registerPathSubject }) {
12+
registerPathSubject((p) => p.startsWith('/api/audit'), 'AuditLog');
13+
}
14+
515
/**
616
* Define audit-related abilities for an authenticated user.
717
* Only platform admins can read audit logs.

0 commit comments

Comments
 (0)