From 2eb050b063c51bba906cd0d1f7ab5561842bebda Mon Sep 17 00:00:00 2001 From: Brian Wang Date: Fri, 7 Aug 2026 22:24:16 +0800 Subject: [PATCH 01/10] feat(auth): add LogicalModel scope for Method ACL and FieldRule - Add LogicalModelName (and Method LogicalMethods) as a fifth exclusive scope so one rule covers per-app isomorphic inject models across host apps. - Wire CheckMethodAccess, FieldRule eval, and PermissionState ACL aggregation; register short names from platform inject bases; seed bootstrap grants and admin scope UI. Co-authored-by: Cursor --- modules/auth/data/bootstrap.json | 70 +++++++--- .../service/models/_logical_model_registry.ts | 95 +++++++++++++ .../service/models/_rule_scope_helpers.ts | 62 ++++++--- .../service/models/_user_field_rule_eval.ts | 41 +++++- .../service/models/_user_lifecycle_auth.ts | 7 +- .../service/models/_user_method_access.ts | 33 ++++- .../models/_user_permission_state_acl.ts | 42 +++++- .../auth/service/models/role_field_rule.ts | 57 +++++++- .../auth/service/models/role_method_access.ts | 128 ++++++++++++++++-- modules/auth/service/models/user.ts | 2 +- modules/auth/service/tests/field_rule.test.ts | 40 ++++++ .../tests/logical_model_registry.test.ts | 41 ++++++ .../method_access_eval_observability.test.ts | 23 ++++ .../tests/permission_state_acl_source.test.ts | 43 ++++++ .../service/tests/rule_scope_helpers.test.ts | 66 ++++++--- .../auth/web/views/RoleFieldRuleFormView.vue | 9 +- .../auth/web/views/RoleFieldRuleListView.vue | 1 + .../web/views/RoleMethodAccessFormView.vue | 9 +- .../web/views/RoleMethodAccessListView.vue | 1 + .../views/access_rules_field_binding.test.ts | 11 ++ .../orm/model/app_setting_base_model.ts | 4 + .../orm/model/field_default_base_model.ts | 4 + .../orm/model/logical_model_registry.test.ts | 49 +++++++ .../orm/model/logical_model_registry.ts | 52 +++++++ .../orm/model/translation_term_base_model.ts | 4 + 25 files changed, 800 insertions(+), 94 deletions(-) create mode 100644 modules/auth/service/models/_logical_model_registry.ts create mode 100644 modules/auth/service/tests/logical_model_registry.test.ts create mode 100644 modules/core/service/orm/model/logical_model_registry.test.ts create mode 100644 modules/core/service/orm/model/logical_model_registry.ts diff --git a/modules/auth/data/bootstrap.json b/modules/auth/data/bootstrap.json index 8b1d854f..d598fd49 100644 --- a/modules/auth/data/bootstrap.json +++ b/modules/auth/data/bootstrap.json @@ -43,7 +43,7 @@ } }, { - "name": "rma_terminology_editor_tt_search", + "name": "rma_terminology_editor_tt_logical", "model": "RoleMethodAccess", "values": { "RoleId": { @@ -51,57 +51,87 @@ }, "MetaApplicationId": null, "MetaModelId": null, - "MetaServiceId": { - "serviceRef": "auth.TranslationTerm/Search" - }, + "MetaServiceId": null, + "LogicalModelName": "TranslationTerm", + "LogicalMethods": ["Search", "Browse", "Update", "Count"], "Mode": "allow" } }, { - "name": "rma_terminology_editor_tt_browse", - "model": "RoleMethodAccess", + "name": "rfr_terminology_editor_tt_logical", + "model": "RoleFieldRule", "values": { "RoleId": { "ref": "auth.role_terminology_editor" }, "MetaApplicationId": null, "MetaModelId": null, - "MetaServiceId": { - "serviceRef": "auth.TranslationTerm/Browse" - }, - "Mode": "allow" + "MetaFieldId": null, + "LogicalModelName": "TranslationTerm", + "PermRead": "allow", + "PermWrite": "allow" } }, { - "name": "rma_terminology_editor_tt_update", + "name": "rma_base_user_field_default_logical", "model": "RoleMethodAccess", "values": { "RoleId": { - "ref": "auth.role_terminology_editor" + "ref": "auth.role_base_user" }, "MetaApplicationId": null, "MetaModelId": null, - "MetaServiceId": { - "serviceRef": "auth.TranslationTerm/Update" - }, + "MetaServiceId": null, + "LogicalModelName": "FieldDefault", + "LogicalMethods": ["Get", "Set"], "Mode": "allow" } }, { - "name": "rma_terminology_editor_tt_count", - "model": "RoleMethodAccess", + "name": "rfr_base_user_field_default_logical", + "model": "RoleFieldRule", "values": { "RoleId": { - "ref": "auth.role_terminology_editor" + "ref": "auth.role_base_user" }, "MetaApplicationId": null, "MetaModelId": null, - "MetaServiceId": { - "serviceRef": "auth.TranslationTerm/Count" + "MetaFieldId": null, + "LogicalModelName": "FieldDefault", + "PermRead": "allow", + "PermWrite": "allow" + } + }, + { + "name": "rma_sys_admin_app_setting_logical", + "model": "RoleMethodAccess", + "values": { + "RoleId": { + "ref": "auth.role_sys_admin" }, + "MetaApplicationId": null, + "MetaModelId": null, + "MetaServiceId": null, + "LogicalModelName": "AppSetting", + "LogicalMethods": ["Get", "Set"], "Mode": "allow" } }, + { + "name": "rfr_sys_admin_app_setting_logical", + "model": "RoleFieldRule", + "values": { + "RoleId": { + "ref": "auth.role_sys_admin" + }, + "MetaApplicationId": null, + "MetaModelId": null, + "MetaFieldId": null, + "LogicalModelName": "AppSetting", + "PermRead": "allow", + "PermWrite": "allow" + } + }, { "name": "user_admin", "model": "User", diff --git a/modules/auth/service/models/_logical_model_registry.ts b/modules/auth/service/models/_logical_model_registry.ts new file mode 100644 index 00000000..8c1f87f0 --- /dev/null +++ b/modules/auth/service/models/_logical_model_registry.ts @@ -0,0 +1,95 @@ +// SPDX-FileCopyrightText: 2026-present Brian Wang +// SPDX-License-Identifier: Apache-2.0 + +/** + * Auth-side helpers for LogicalModel ACL scope. + * + * Short-name eligibility comes from core platform-inject bases via + * {@link registerLogicalModelName} (not an auth constant list). + * See `.dev/docs/auth/logical_model_acl_field_rule_design.md` §5. + */ + +// Ensure platform store bases have self-registered before write validation. +import '@/core/service/orm/model/app_setting_base_model'; +import '@/core/service/orm/model/field_default_base_model'; +import '@/core/service/orm/model/translation_term_base_model'; +import { + isRegisteredLogicalModelName, + listLogicalModelNames, + listLogicalModelSelection, +} from '@/core/service/orm/model/logical_model_registry'; + +export { isRegisteredLogicalModelName, listLogicalModelNames, listLogicalModelSelection }; + +/** + * Normalize a logical model short name for write validation. + * Empty → null. Non-empty unregistered → throws. + */ +export function normalizeLogicalModelName(raw: unknown): string | null { + if (raw == null) return null; + const n = String(raw).trim(); + if (!n) return null; + if (!isRegisteredLogicalModelName(n)) { + throw new Error(`invalid LogicalModelName: ${n} is not a registered logical model`); + } + return n; +} + +/** + * Canonicalize one RPC short name: trim, drop empties, PascalCase first letter. + * Comparison at eval time is case-insensitive (toLowerCase). + */ +export function canonicalizeLogicalMethodName(raw: unknown): string { + const t = String(raw ?? '').trim(); + if (!t) return ''; + return t.charAt(0).toUpperCase() + t.slice(1); +} + +/** + * Normalize LogicalMethods JSON payload to string[] | null. + * null / [] / missing → null (meaning "all methods" at eval time). + */ +export function normalizeLogicalMethods(raw: unknown): string[] | null { + if (raw == null) return null; + let arr: unknown[] | null = null; + if (Array.isArray(raw)) { + arr = raw; + } else if (typeof raw === 'string') { + const s = raw.trim(); + if (!s) return null; + try { + const parsed = JSON.parse(s); + if (Array.isArray(parsed)) arr = parsed; + else throw new Error('not an array'); + } catch { + throw new Error('invalid LogicalMethods: must be a JSON string array'); + } + } else { + throw new Error('invalid LogicalMethods: must be a string array'); + } + + const out: string[] = []; + const seen = new Set(); + for (const item of arr) { + const name = canonicalizeLogicalMethodName(item); + if (!name) continue; + const key = name.toLowerCase(); + if (seen.has(key)) continue; + seen.add(key); + out.push(name); + } + return out.length === 0 ? null : out; +} + +/** + * Whether a LogicalMethods whitelist (null/empty = all) covers `methodName`. + */ +export function logicalMethodsAllow(methods: unknown, methodName: string): boolean { + const want = String(methodName || '') + .trim() + .toLowerCase(); + if (!want) return false; + const list = normalizeLogicalMethods(methods); + if (list == null) return true; + return list.some(m => m.toLowerCase() === want); +} diff --git a/modules/auth/service/models/_rule_scope_helpers.ts b/modules/auth/service/models/_rule_scope_helpers.ts index 58624235..04768a29 100644 --- a/modules/auth/service/models/_rule_scope_helpers.ts +++ b/modules/auth/service/models/_rule_scope_helpers.ts @@ -2,11 +2,13 @@ // SPDX-License-Identifier: Apache-2.0 import { normalizeRefId } from '@/core/service/utils/normalization'; +import { normalizeLogicalModelName } from './_logical_model_registry'; /** * Scope profile for the four Role* rule models. * * Extracted in PR-E-1; wired into method/record/field/ui models in PR-E-2. + * LogicalModel scope added for method/field (logical_model_acl_field_rule_design.md). */ export type RuleScopeProfile = 'method' | 'record' | 'field' | 'ui'; @@ -15,7 +17,8 @@ export type RuleScopeProfile = 'method' | 'record' | 'field' | 'ui'; */ export type AssertExclusiveScopeMode = 'create' | 'update'; -type ScopeFieldKey = 'MetaServiceId' | 'MetaModelId' | 'MetaApplicationId' | 'MetaFieldId' | 'MetaUiResourceId'; +type MetaScopeFieldKey = 'MetaServiceId' | 'MetaModelId' | 'MetaApplicationId' | 'MetaFieldId' | 'MetaUiResourceId'; +type ScopeFieldKey = MetaScopeFieldKey | 'LogicalModelName'; type ProfileSpec = { modelName: string; @@ -23,21 +26,31 @@ type ProfileSpec = { shapesLabel: string; /** When true, create always validates/normalizes scope even if no scope keys are present (empty → global). */ alwaysValidateOnCreate: boolean; + /** When true, LogicalModelName participates and is registry-validated. */ + supportsLogicalModel: boolean; isValidShape: (ids: Record) => boolean; }; const PROFILE_SPECS: Record = { method: { modelName: 'RoleMethodAccess', - fields: ['MetaServiceId', 'MetaModelId', 'MetaApplicationId'], - shapesLabel: 'service/model/application/global', + fields: ['MetaServiceId', 'MetaModelId', 'MetaApplicationId', 'LogicalModelName'], + shapesLabel: 'service/model/application/logical_model/global', alwaysValidateOnCreate: false, + supportsLogicalModel: true, isValidShape: ids => { - const isService = ids.MetaServiceId != null && ids.MetaModelId == null && ids.MetaApplicationId == null; - const isModel = ids.MetaServiceId == null && ids.MetaModelId != null && ids.MetaApplicationId == null; - const isApplication = ids.MetaServiceId == null && ids.MetaModelId == null && ids.MetaApplicationId != null; - const isGlobal = ids.MetaServiceId == null && ids.MetaModelId == null && ids.MetaApplicationId == null; - return isService || isModel || isApplication || isGlobal; + const logical = ids.LogicalModelName; + const isService = + ids.MetaServiceId != null && ids.MetaModelId == null && ids.MetaApplicationId == null && logical == null; + const isModel = + ids.MetaServiceId == null && ids.MetaModelId != null && ids.MetaApplicationId == null && logical == null; + const isApplication = + ids.MetaServiceId == null && ids.MetaModelId == null && ids.MetaApplicationId != null && logical == null; + const isLogical = + logical != null && ids.MetaServiceId == null && ids.MetaModelId == null && ids.MetaApplicationId == null; + const isGlobal = + ids.MetaServiceId == null && ids.MetaModelId == null && ids.MetaApplicationId == null && logical == null; + return isService || isModel || isApplication || isLogical || isGlobal; }, }, record: { @@ -45,6 +58,7 @@ const PROFILE_SPECS: Record = { fields: ['MetaModelId', 'MetaApplicationId'], shapesLabel: 'model/application/global', alwaysValidateOnCreate: false, + supportsLogicalModel: false, isValidShape: ids => { const isModel = ids.MetaModelId != null && ids.MetaApplicationId == null; const isApplication = ids.MetaModelId == null && ids.MetaApplicationId != null; @@ -54,15 +68,23 @@ const PROFILE_SPECS: Record = { }, field: { modelName: 'RoleFieldRule', - fields: ['MetaFieldId', 'MetaModelId', 'MetaApplicationId'], - shapesLabel: 'field/model/application/global', + fields: ['MetaFieldId', 'MetaModelId', 'MetaApplicationId', 'LogicalModelName'], + shapesLabel: 'field/model/application/logical_model/global', alwaysValidateOnCreate: true, + supportsLogicalModel: true, isValidShape: ids => { - const isField = ids.MetaFieldId != null && ids.MetaModelId != null && ids.MetaApplicationId == null; - const isModel = ids.MetaFieldId == null && ids.MetaModelId != null && ids.MetaApplicationId == null; - const isApplication = ids.MetaFieldId == null && ids.MetaModelId == null && ids.MetaApplicationId != null; - const isGlobal = ids.MetaFieldId == null && ids.MetaModelId == null && ids.MetaApplicationId == null; - return isField || isModel || isApplication || isGlobal; + const logical = ids.LogicalModelName; + const isField = + ids.MetaFieldId != null && ids.MetaModelId != null && ids.MetaApplicationId == null && logical == null; + const isModel = + ids.MetaFieldId == null && ids.MetaModelId != null && ids.MetaApplicationId == null && logical == null; + const isApplication = + ids.MetaFieldId == null && ids.MetaModelId == null && ids.MetaApplicationId != null && logical == null; + const isLogical = + logical != null && ids.MetaFieldId == null && ids.MetaModelId == null && ids.MetaApplicationId == null; + const isGlobal = + ids.MetaFieldId == null && ids.MetaModelId == null && ids.MetaApplicationId == null && logical == null; + return isField || isModel || isApplication || isLogical || isGlobal; }, }, ui: { @@ -70,6 +92,7 @@ const PROFILE_SPECS: Record = { fields: ['MetaUiResourceId', 'MetaApplicationId'], shapesLabel: 'resource/application/global', alwaysValidateOnCreate: false, + supportsLogicalModel: false, isValidShape: ids => { const isResource = ids.MetaUiResourceId != null && ids.MetaApplicationId == null; const isApplication = ids.MetaUiResourceId == null && ids.MetaApplicationId != null; @@ -94,7 +117,8 @@ function touchesAnyScopeField(values: Record, fields: ScopeFieldKey * Assert and normalize mutually exclusive scope refs on a rule row payload. * * Error message strings are byte-stable with the pre-E-1 `_validateScopeShape` implementations - * so existing tests and call sites can migrate without golden churn. + * so existing tests and call sites can migrate without golden churn — except method/field + * shapesLabel which now includes `logical_model`. * * Mutates `values` in place when scope columns are validated/normalized. */ @@ -118,7 +142,11 @@ export function assertExclusiveScope(values: Record, mode: AssertEx const ids = {} as Record; for (const f of spec.fields) { - ids[f] = normalizeRefId((values as any)[f]); + if (f === 'LogicalModelName') { + ids[f] = normalizeLogicalModelName((values as any)[f]); + } else { + ids[f] = normalizeRefId((values as any)[f]); + } } if (!spec.isValidShape(ids)) { diff --git a/modules/auth/service/models/_user_field_rule_eval.ts b/modules/auth/service/models/_user_field_rule_eval.ts index b83a1e57..0505ab24 100644 --- a/modules/auth/service/models/_user_field_rule_eval.ts +++ b/modules/auth/service/models/_user_field_rule_eval.ts @@ -153,6 +153,7 @@ export async function evaluateFieldRules(input: FieldRuleEvalInput): Promise(); const modelRules: any[] = []; const appRules: any[] = []; + const logicalRules: any[] = []; const globalRules: any[] = []; + const modelNameWant = String(input.modelName || '').trim(); for (const r of rules || []) { const rid = String((r as any)?.Id ?? '').trim(); const irApp = normalizeRefId(pickField(r, ['MetaApplicationId', 'meta_application_id', 'irApplicationId'])); const irModel = normalizeRefId(pickField(r, ['MetaModelId', 'meta_model_id', 'irModelId'])); const irField = normalizeRefId(pickField(r, ['MetaFieldId', 'meta_field_id', 'irFieldId'])); + const logicalName = String(pickField(r, ['LogicalModelName', 'logical_model_name']) ?? '').trim() || null; const permRead = normalizeFieldPerm(pickField(r, ['PermRead', 'perm_read', 'permRead'])); const permWrite = normalizeFieldPerm(pickField(r, ['PermWrite', 'perm_write', 'permWrite'])); - const rule: Record = { irApp, irModel, irField, permRead, permWrite }; + const rule: Record = { irApp, irModel, irField, logicalName, permRead, permWrite }; if (rid) rule.__rid = rid; - const isField = irField != null && irModel != null && irApp == null; - const isModel = irField == null && irModel != null && irApp == null; - const isApp = irField == null && irModel == null && irApp != null; - const isGlobal = irField == null && irModel == null && irApp == null; + const isField = irField != null && irModel != null && irApp == null && logicalName == null; + const isModel = irField == null && irModel != null && irApp == null && logicalName == null; + const isApp = irField == null && irModel == null && irApp != null && logicalName == null; + const isLogical = irField == null && irModel == null && irApp == null && logicalName != null; + const isGlobal = irField == null && irModel == null && irApp == null && logicalName == null; if (isField) { if (!fieldIdSet.has(irField)) continue; @@ -230,6 +246,9 @@ export async function evaluateFieldRules(input: FieldRuleEvalInput): Promise MetaModel > Application > LogicalModel > Global + const buckets: any[][] = [ + fieldRulesByFieldName.get(fieldName) || [], + modelRules, + appRules, + logicalRules, + globalRules, + ]; for (const b of buckets) { const d = decideInScope(b, dim); if (d) return d; @@ -274,6 +300,7 @@ export async function evaluateFieldRules(input: FieldRuleEvalInput): Promise String((r as any)?.__rid ?? '').trim()) diff --git a/modules/auth/service/models/_user_lifecycle_auth.ts b/modules/auth/service/models/_user_lifecycle_auth.ts index b8c02e12..c49a0bc8 100644 --- a/modules/auth/service/models/_user_lifecycle_auth.ts +++ b/modules/auth/service/models/_user_lifecycle_auth.ts @@ -13,6 +13,7 @@ import Session from './session'; import Token from './token'; import UserRole from './user_role'; import { hashPassword, verifyPassword, withPermissionGraphBypass } from './_user_authz_shared'; +import { withRepositoryAuthzRuleBypass } from '@/core/service/orm/repository/authz'; import { buildScopePreferences } from './_user_lifecycle_scope'; const CompanyService = createServiceByModel('base.Company'); @@ -51,7 +52,11 @@ async function isPersistBrowserTimezoneEnabled(isPersistEnabled?: () => Promise< if (isPersistEnabled) { return await isPersistEnabled(); } - const flag = await pool('auth', 'AppSetting').Get(PERSIST_BROWSER_TIMEZONE_KEY, '1'); + // System/internal read: no terminal-user Logical grant for AppSetting on base.user. + // Narrow RR+FR bypass (Method ACL does not apply to in-process pool calls). + const flag = await withRepositoryAuthzRuleBypass(() => + pool('auth', 'AppSetting').Get(PERSIST_BROWSER_TIMEZONE_KEY, '1') + ); return flag === '1'; } diff --git a/modules/auth/service/models/_user_method_access.ts b/modules/auth/service/models/_user_method_access.ts index b0114a3d..9800bf2f 100644 --- a/modules/auth/service/models/_user_method_access.ts +++ b/modules/auth/service/models/_user_method_access.ts @@ -10,6 +10,7 @@ import { buildUiGrantCacheKey } from './_request_cache_invalidation'; import RoleMethodAccess from './role_method_access'; import RoleUiResource from './role_ui_resource'; import { normalizeScopeRefId, normalizeUiResourceId, parseJsonStringArray, requireMatchesMethod, sortStrings } from './_user_authz_shared'; +import { logicalMethodsAllow } from './_logical_model_registry'; import { resolveEffectiveApplicationId, resolveEffectiveModelId } from './_resolve_effective_model'; const MetaService = createServiceByModel('meta.MetaService'); @@ -86,6 +87,7 @@ export async function resolveMethodAccessMeta( ['MetaServiceId', '=', irServiceId], ['MetaModelId', 'is', null], ['MetaApplicationId', 'is', null], + ['LogicalModelName', 'is', null], ], }, { @@ -93,6 +95,7 @@ export async function resolveMethodAccessMeta( ['MetaServiceId', 'is', null], ['MetaModelId', '=', modelId], ['MetaApplicationId', 'is', null], + ['LogicalModelName', 'is', null], ], }, { @@ -100,16 +103,27 @@ export async function resolveMethodAccessMeta( ['MetaServiceId', 'is', null], ['MetaModelId', 'is', null], ['MetaApplicationId', 'is', null], + ['LogicalModelName', '=', modelName], + ], + }, + { + And: [ + ['MetaServiceId', 'is', null], + ['MetaModelId', 'is', null], + ['MetaApplicationId', 'is', null], + ['LogicalModelName', 'is', null], ], }, ]; if (irApplicationId) { + // Insert Application between MetaModel and LogicalModel (index 2). scopeOr.splice(2, 0, { And: [ ['MetaServiceId', 'is', null], ['MetaModelId', 'is', null], ['MetaApplicationId', '=', irApplicationId], + ['LogicalModelName', 'is', null], ], }); } @@ -127,19 +141,32 @@ export async function resolveMethodAccessMeta( /** * Evaluate explicit RoleMethodAccess rules with deny-wins semantics. + * + * LogicalModel rows may include LogicalMethods; callers pass methodLower so + * method-restricted logical rules only apply when the method matches. */ export async function evaluateRoleMethodAccess( roleIds: string[], - scopeOr: any[] + scopeOr: any[], + methodLower?: string ): Promise<{ denied: boolean; allowed: boolean; hitRuleIds: string[]; reason: string }> { const accessesRaw = await RoleMethodAccess.Search( { And: [['RoleId', 'in', roleIds], { Or: scopeOr } as any], } as any, - { fields: ['Id', 'Mode', 'Source'], limit: 5000 } + { fields: ['Id', 'Mode', 'Source', 'LogicalModelName', 'LogicalMethods'], limit: 5000 } ); // UI-Option-A: Source=ui rows are not manual ACL (runtime ui-derived path owns UI→Method). - const accesses = (accessesRaw || []).filter(a => String((a as any).Source || 'manual').toLowerCase() !== 'ui'); + const methodKey = String(methodLower || '') + .trim() + .toLowerCase(); + const accesses = (accessesRaw || []).filter(a => { + if (String((a as any).Source || 'manual').toLowerCase() === 'ui') return false; + const logicalName = String((a as any).LogicalModelName || '').trim(); + if (!logicalName) return true; + if (!methodKey) return false; + return logicalMethodsAllow((a as any).LogicalMethods, methodKey); + }); let allowed = false; const allowHitRuleIds: string[] = []; diff --git a/modules/auth/service/models/_user_permission_state_acl.ts b/modules/auth/service/models/_user_permission_state_acl.ts index 902c1ed6..b97a24cc 100644 --- a/modules/auth/service/models/_user_permission_state_acl.ts +++ b/modules/auth/service/models/_user_permission_state_acl.ts @@ -7,6 +7,7 @@ import type MetaModelModel from '@/meta/service/models/model'; import type MetaServiceModel from '@/meta/service/models/service'; import RoleMethodAccess from './role_method_access'; import { maybeId } from './_user_authz_shared'; +import { normalizeLogicalMethods } from './_logical_model_registry'; const MetaService = createServiceByModel('meta.MetaService'); const MetaModel = createServiceByModel('meta.MetaModel'); @@ -45,7 +46,7 @@ export async function buildAclAggregation( roleScopesById: Record ): Promise { const accessesRaw = await RoleMethodAccess.Search(['RoleId', 'in', roleIds] as any, { - fields: ['RoleId', 'MetaServiceId', 'MetaModelId', 'MetaApplicationId', 'Mode', 'Source'], + fields: ['RoleId', 'MetaServiceId', 'MetaModelId', 'MetaApplicationId', 'LogicalModelName', 'LogicalMethods', 'Mode', 'Source'], limit: 50000, }); // UI-Option-A: ignore legacy Source=ui rows in PermissionState ACL aggregation. @@ -182,11 +183,40 @@ export async function buildAclAggregation( const sid = String((a as any).MetaServiceId || '').trim(); const mid = String((a as any).MetaModelId || '').trim(); const aid = String((a as any).MetaApplicationId || '').trim(); + const logicalName = String((a as any).LogicalModelName || '').trim(); const mode = String((a as any).Mode || '').toLowerCase(); if (!roleId || (mode !== 'allow' && mode !== 'deny')) continue; - // global scope - if (!sid && !mid && !aid) { + // LogicalModel scope: all host apps whose @Model short name matches. + if (!sid && !mid && !aid && logicalName) { + const models = (await getAllModels()).filter(m => m.name === logicalName); + if (models.length === 0) continue; + let methods: string[] | null; + try { + methods = normalizeLogicalMethods((a as any).LogicalMethods); + } catch { + continue; + } + scope(roleId)(companyKey => { + for (const m of models) { + const serviceFullName = `${m.app}.${m.name}`; + const agg = ensureAgg(companyKey, serviceFullName); + if (methods == null) { + if (mode === 'allow') agg.allowAll = true; + else agg.denyAll = true; + } else { + for (const methodName of methods) { + if (mode === 'allow') agg.allow.add(methodName); + else agg.deny.add(methodName); + } + } + } + }); + continue; + } + + // global scope (Meta* and LogicalModelName all empty) + if (!sid && !mid && !aid && !logicalName) { const models = await getAllModels(); scope(roleId)(companyKey => { if (mode === 'allow') companyGlobalAllow.add(companyKey); @@ -202,7 +232,7 @@ export async function buildAclAggregation( } // application scope - if (!sid && !mid && aid) { + if (!sid && !mid && aid && !logicalName) { const appName = appNameById.get(aid); if (!appName) continue; const models = getModelsForApp(appName); @@ -218,7 +248,7 @@ export async function buildAclAggregation( } // model scope - if (!sid && mid && !aid) { + if (!sid && mid && !aid && !logicalName) { const mdl = modelById.get(mid); if (!mdl) continue; const serviceFullName = `${mdl.app}.${mdl.name}`; @@ -231,7 +261,7 @@ export async function buildAclAggregation( } // service(method) scope - if (sid && !mid && !aid) { + if (sid && !mid && !aid && !logicalName) { const svc = serviceById.get(sid); if (!svc) continue; const mdl = modelById.get(svc.modelId); diff --git a/modules/auth/service/models/role_field_rule.ts b/modules/auth/service/models/role_field_rule.ts index 51aba80f..5e75e5fa 100644 --- a/modules/auth/service/models/role_field_rule.ts +++ b/modules/auth/service/models/role_field_rule.ts @@ -6,6 +6,7 @@ import { Onchange } from '@/core/service/api/onchange'; import type { Insertable, Updateable } from '@/core/service/api/input'; import type { FieldSelection } from '@/core/service/api/selection'; import type { QueryCondition } from '@/core/service/api/query'; +import { listLogicalModelSelection } from './_logical_model_registry'; import { _lt } from '../i18n'; import Role from './role'; import type MetaApplication from '@/meta/service/models/application'; @@ -16,7 +17,7 @@ import { assertExclusiveScope } from './_rule_scope_helpers'; /** * RoleFieldRule stores field-level read and write overrides for a role at - * global, application, model, or field scope. + * global, application, model, field, or logical-model scope. */ @Model('RoleFieldRule') export default class RoleFieldRule extends BaseModel { @@ -73,10 +74,11 @@ export default class RoleFieldRule extends BaseModel { checkConstraint: `( ( (deleted_at IS NOT NULL) - OR (meta_field_id IS NOT NULL AND meta_model_id IS NOT NULL AND meta_application_id IS NULL) - OR (meta_field_id IS NULL AND meta_model_id IS NOT NULL AND meta_application_id IS NULL) - OR (meta_field_id IS NULL AND meta_model_id IS NULL AND meta_application_id IS NOT NULL) - OR (meta_field_id IS NULL AND meta_model_id IS NULL AND meta_application_id IS NULL) + OR (meta_field_id IS NOT NULL AND meta_model_id IS NOT NULL AND meta_application_id IS NULL AND logical_model_name IS NULL) + OR (meta_field_id IS NULL AND meta_model_id IS NOT NULL AND meta_application_id IS NULL AND logical_model_name IS NULL) + OR (meta_field_id IS NULL AND meta_model_id IS NULL AND meta_application_id IS NOT NULL AND logical_model_name IS NULL) + OR (meta_field_id IS NULL AND meta_model_id IS NULL AND meta_application_id IS NULL AND logical_model_name IS NOT NULL) + OR (meta_field_id IS NULL AND meta_model_id IS NULL AND meta_application_id IS NULL AND logical_model_name IS NULL) ) AND (perm_read IS NOT NULL OR perm_write IS NOT NULL) )`, @@ -87,6 +89,25 @@ export default class RoleFieldRule extends BaseModel { }) MetaFieldId?: string; + /** + * Logical model short name (@Model name without application prefix). + * Mutually exclusive with MetaFieldId / MetaModelId / MetaApplicationId. + * Options come from core platform-inject self-registration. + */ + @Field({ + type: 'selection', + selection: () => listLogicalModelSelection(), + notNull: false, + size: 128, + index: true, + string: _lt('Logical Model', { scope: 'auth.model.RoleFieldRule.fields' }), + help: _lt( + 'Registered short name covering all installed host applications that inject the same model. Applies to all business fields on that logical model. Mutually exclusive with Application / Model / Field.', + { scope: 'auth.model.RoleFieldRule.fields' } + ), + }) + LogicalModelName?: string | null; + /** * Read permission override for the selected scope. */ @@ -258,6 +279,9 @@ export default class RoleFieldRule extends BaseModel { @Onchange('MetaModelId') OnchangeMetaModelId() { this.MetaFieldId = null as any; + if (String(this.MetaModelId || '').trim() && String(this.LogicalModelName || '').trim()) { + this.LogicalModelName = null as any; + } const modelId = this.MetaModelId; @@ -273,4 +297,27 @@ export default class RoleFieldRule extends BaseModel { }; } } + + /** + * Selecting a logical model clears Meta* scopes (exclusive shapes). + */ + @Onchange('LogicalModelName') + OnchangeLogicalModelName() { + if (!String(this.LogicalModelName || '').trim()) return; + this.MetaFieldId = null as any; + this.MetaModelId = null as any; + this.MetaApplicationId = null as any; + } + + /** + * Selecting application or field scope clears logical-model scope. + */ + @Onchange('MetaApplicationId', 'MetaFieldId') + OnchangeMetaScopeClearsLogical() { + const hasMeta = + Boolean(String(this.MetaApplicationId || '').trim()) || Boolean(String(this.MetaFieldId || '').trim()); + if (hasMeta && String(this.LogicalModelName || '').trim()) { + this.LogicalModelName = null as any; + } + } } diff --git a/modules/auth/service/models/role_method_access.ts b/modules/auth/service/models/role_method_access.ts index 8f70ea15..f62487f6 100644 --- a/modules/auth/service/models/role_method_access.ts +++ b/modules/auth/service/models/role_method_access.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { BaseModel, Model, Field } from '@/core/service'; +import { Onchange } from '@/core/service/api/onchange'; import type { Insertable, Updateable } from '@/core/service/api/input'; import type { FieldSelection } from '@/core/service/api/selection'; import type { QueryCondition } from '@/core/service/api/query'; @@ -11,11 +12,12 @@ import type MetaApplication from '@/meta/service/models/application'; import type MetaModel from '@/meta/service/models/model'; import type MetaService from '@/meta/service/models/service'; import { mutateThenInvalidateAllAuthzCaches } from './_authz_mutation_helpers'; +import { listLogicalModelSelection, normalizeLogicalMethods } from './_logical_model_registry'; import { assertExclusiveScope } from './_rule_scope_helpers'; /** * RoleMethodAccess stores role-level RPC allow and deny overrides at global, - * application, model, or service scope. + * application, model, service, or logical-model scope. */ @Model('RoleMethodAccess') export default class RoleMethodAccess extends BaseModel { @@ -65,10 +67,11 @@ export default class RoleMethodAccess extends BaseModel { index: true, checkConstraint: `( (deleted_at IS NOT NULL) - OR (meta_service_id IS NOT NULL AND meta_model_id IS NULL AND meta_application_id IS NULL) - OR (meta_service_id IS NULL AND meta_model_id IS NOT NULL AND meta_application_id IS NULL) - OR (meta_service_id IS NULL AND meta_model_id IS NULL AND meta_application_id IS NOT NULL) - OR (meta_service_id IS NULL AND meta_model_id IS NULL AND meta_application_id IS NULL) + OR (meta_service_id IS NOT NULL AND meta_model_id IS NULL AND meta_application_id IS NULL AND logical_model_name IS NULL) + OR (meta_service_id IS NULL AND meta_model_id IS NOT NULL AND meta_application_id IS NULL AND logical_model_name IS NULL) + OR (meta_service_id IS NULL AND meta_model_id IS NULL AND meta_application_id IS NOT NULL AND logical_model_name IS NULL) + OR (meta_service_id IS NULL AND meta_model_id IS NULL AND meta_application_id IS NULL AND logical_model_name IS NOT NULL) + OR (meta_service_id IS NULL AND meta_model_id IS NULL AND meta_application_id IS NULL AND logical_model_name IS NULL) )`, string: _lt('Service', { scope: 'auth.model.RoleMethodAccess.fields' }), help: _lt('Leave all scopes empty for a global rule; matching scopes are OR-ed and any deny wins.', { @@ -77,6 +80,40 @@ export default class RoleMethodAccess extends BaseModel { }) MetaServiceId: string | null; + /** + * Logical model short name (@Model name without application prefix). + * Mutually exclusive with MetaServiceId / MetaModelId / MetaApplicationId. + * Options come from core platform-inject self-registration. + */ + @Field({ + type: 'selection', + selection: () => listLogicalModelSelection(), + notNull: false, + size: 128, + index: true, + string: _lt('Logical Model', { scope: 'auth.model.RoleMethodAccess.fields' }), + help: _lt( + 'Registered short name covering all installed host applications that inject the same model (e.g. TranslationTerm). Mutually exclusive with Application / Model / Service.', + { scope: 'auth.model.RoleMethodAccess.fields' } + ), + }) + LogicalModelName: string | null; + + /** + * Optional RPC short-name whitelist for LogicalModel scope. + * Null or empty = all methods on that logical model. + */ + @Field({ + type: 'jsonobject', + notNull: false, + string: _lt('Logical Methods', { scope: 'auth.model.RoleMethodAccess.fields' }), + help: _lt( + 'Optional method names for logical-model scope (JSON string array). Leave empty to allow or deny all methods on that logical model.', + { scope: 'auth.model.RoleMethodAccess.fields' } + ), + }) + LogicalMethods: string[] | null; + /** * Whether the matched scope is allowed or denied. */ @@ -124,6 +161,46 @@ export default class RoleMethodAccess extends BaseModel { (values as any).Source = 'manual'; } + /** + * Normalize LogicalMethods; clear them unless scope is LogicalModel. + * Call after assertExclusiveScope so LogicalModelName is already normalized. + */ + private static _normalizeLogicalMethodsPayload(values: Record, mode: 'create' | 'update'): void { + if (!values) return; + const touchesMethods = Object.prototype.hasOwnProperty.call(values, 'LogicalMethods'); + const touchesLogicalName = Object.prototype.hasOwnProperty.call(values, 'LogicalModelName'); + + // Partial update that does not touch scope/methods: leave LogicalMethods alone. + if (mode === 'update' && !touchesMethods && !touchesLogicalName) return; + + if (touchesMethods) { + (values as any).LogicalMethods = normalizeLogicalMethods((values as any).LogicalMethods); + } else if (mode === 'create') { + (values as any).LogicalMethods = normalizeLogicalMethods((values as any).LogicalMethods); + } + + // When LogicalModelName is present in the payload (create always after assert, or update touching scope), + // clear methods unless this row is logical scope. + if (mode === 'create' || touchesLogicalName) { + const name = String((values as any).LogicalModelName || '').trim(); + if (!name) { + if ((values as any).LogicalMethods != null) { + throw new Error('invalid RoleMethodAccess: LogicalMethods requires LogicalModel scope'); + } + (values as any).LogicalMethods = null; + } + } else if (mode === 'update' && touchesMethods && (values as any).LogicalMethods != null) { + // Methods-only update without LogicalModelName: cannot prove logical scope. + throw new Error('invalid RoleMethodAccess: LogicalMethods requires LogicalModel scope'); + } + } + + private static _prepareValues(values: Record, mode: 'create' | 'update'): void { + assertExclusiveScope(values, mode, 'method'); + RoleMethodAccess._normalizeLogicalMethodsPayload(values, mode); + RoleMethodAccess._coerceSourceManual(values, mode); + } + /** * Create one RoleMethodAccess row and invalidate request-scoped auth caches. */ @@ -132,8 +209,7 @@ export default class RoleMethodAccess extends BaseModel { value: Partial>, returnFields?: FieldSelection ): Promise { - assertExclusiveScope(value as any, 'create', 'method'); - RoleMethodAccess._coerceSourceManual(value as any, 'create'); + RoleMethodAccess._prepareValues(value as any, 'create'); return mutateThenInvalidateAllAuthzCaches(async () => { const out = await super.Create(value as any, returnFields as any); return out as unknown as T; @@ -150,8 +226,7 @@ export default class RoleMethodAccess extends BaseModel { ): Promise { const rows = values || []; for (const v of rows) { - assertExclusiveScope(v as any, 'create', 'method'); - RoleMethodAccess._coerceSourceManual(v as any, 'create'); + RoleMethodAccess._prepareValues(v as any, 'create'); } return mutateThenInvalidateAllAuthzCaches(async () => { const out = await super.CreateMany(rows as any, returnFields as any); @@ -169,8 +244,7 @@ export default class RoleMethodAccess extends BaseModel { returnFields?: FieldSelection, options?: any ): Promise[]> { - assertExclusiveScope(values as any, 'update', 'method'); - RoleMethodAccess._coerceSourceManual(values as any, 'update'); + RoleMethodAccess._prepareValues(values as any, 'update'); return mutateThenInvalidateAllAuthzCaches(async () => { const out = await super.Update(condition as any, values as any, returnFields as any, options as any); return out as unknown as Partial[]; @@ -187,8 +261,7 @@ export default class RoleMethodAccess extends BaseModel { returnFields?: FieldSelection, options?: any ): Promise> { - assertExclusiveScope(values as any, 'update', 'method'); - RoleMethodAccess._coerceSourceManual(values as any, 'update'); + RoleMethodAccess._prepareValues(values as any, 'update'); return mutateThenInvalidateAllAuthzCaches(async () => { const out = await super.UpdateById(id as any, values as any, returnFields as any, options as any); return out as unknown as Partial; @@ -212,4 +285,33 @@ export default class RoleMethodAccess extends BaseModel { static override async DeleteById(this: { new (...args: any[]): T } & typeof BaseModel, id: string, options?: any): Promise { return mutateThenInvalidateAllAuthzCaches(() => super.DeleteById(id as any, options as any)); } + + /** + * Selecting a logical model clears Meta* scopes (exclusive shapes). + */ + @Onchange('LogicalModelName') + OnchangeLogicalModelName() { + const name = String(this.LogicalModelName || '').trim(); + if (name) { + this.MetaServiceId = null as any; + this.MetaModelId = null as any; + this.MetaApplicationId = null as any; + } else if (this.LogicalMethods != null) { + this.LogicalMethods = null as any; + } + } + + /** + * Selecting a concrete Meta scope clears logical-model fields. + */ + @Onchange('MetaServiceId', 'MetaModelId', 'MetaApplicationId') + OnchangeMetaScopeClearsLogical() { + const hasMeta = + Boolean(String(this.MetaServiceId || '').trim()) || + Boolean(String(this.MetaModelId || '').trim()) || + Boolean(String(this.MetaApplicationId || '').trim()); + if (!hasMeta) return; + if (String(this.LogicalModelName || '').trim()) this.LogicalModelName = null as any; + if (this.LogicalMethods != null) this.LogicalMethods = null as any; + } } diff --git a/modules/auth/service/models/user.ts b/modules/auth/service/models/user.ts index 6cec791d..b1459537 100644 --- a/modules/auth/service/models/user.ts +++ b/modules/auth/service/models/user.ts @@ -793,7 +793,7 @@ export default class User extends BaseModel { const accessMeta = await resolveMethodAccessMeta(appName, modelName, methodName); if (!accessMeta) return deny('method_meta_not_found', []); - const accessResult = await evaluateRoleMethodAccess(roleIds, accessMeta.scopeOr); + const accessResult = await evaluateRoleMethodAccess(roleIds, accessMeta.scopeOr, accessMeta.methodLower); if (accessResult.denied) { const decision = deny(accessResult.reason, accessResult.hitRuleIds); diff --git a/modules/auth/service/tests/field_rule.test.ts b/modules/auth/service/tests/field_rule.test.ts index c82e796e..ef11b546 100644 --- a/modules/auth/service/tests/field_rule.test.ts +++ b/modules/auth/service/tests/field_rule.test.ts @@ -1612,6 +1612,46 @@ test('RoleFieldRule OnchangeMetaModelId clears MetaFieldId and blocks picker whe expect(result.value).toEqual({ MetaFieldId: null }); }); +test('RoleFieldRule OnchangeLogicalModelName clears Meta scopes', async () => { + const result = await RoleFieldRule.Onchange( + { + Id: 'onchange-rule-logical', + LogicalModelName: 'TranslationTerm', + MetaApplicationId: 'app-1', + MetaModelId: 'model-1', + MetaFieldId: 'field-1', + }, + ['LogicalModelName'] + ); + + expect(result.value).toEqual({ + MetaApplicationId: null, + MetaModelId: null, + MetaFieldId: null, + }); +}); + +test('RoleMethodAccess OnchangeLogicalModelName clears Meta scopes', async () => { + const RoleMethodAccess = (await import('@/auth/service/models/role_method_access')).default; + const result = await RoleMethodAccess.Onchange( + { + Id: 'onchange-ma-logical', + LogicalModelName: 'FieldDefault', + MetaApplicationId: 'app-1', + MetaModelId: 'model-1', + MetaServiceId: 'svc-1', + LogicalMethods: ['Get'], + }, + ['LogicalModelName'] + ); + + expect(result.value).toEqual({ + MetaApplicationId: null, + MetaModelId: null, + MetaServiceId: null, + }); +}); + test('RoleFieldRule coverage: CreateMany, perm validation branches, and Update paths', async () => { resetRequestContext(); diff --git a/modules/auth/service/tests/logical_model_registry.test.ts b/modules/auth/service/tests/logical_model_registry.test.ts new file mode 100644 index 00000000..5ae1c4c1 --- /dev/null +++ b/modules/auth/service/tests/logical_model_registry.test.ts @@ -0,0 +1,41 @@ +// SPDX-FileCopyrightText: 2026-present Brian Wang +// SPDX-License-Identifier: Apache-2.0 + +import { + canonicalizeLogicalMethodName, + isRegisteredLogicalModelName, + listLogicalModelNames, + logicalMethodsAllow, + normalizeLogicalMethods, + normalizeLogicalModelName, +} from '@/auth/service/models/_logical_model_registry'; + +test('logical model names come from core platform-inject self-registration', () => { + expect(listLogicalModelNames()).toEqual(['AppSetting', 'FieldDefault', 'TranslationTerm']); + expect(isRegisteredLogicalModelName('TranslationTerm')).toBe(true); + expect(isRegisteredLogicalModelName('Partner')).toBe(false); + expect(isRegisteredLogicalModelName('')).toBe(false); +}); + +test('normalizeLogicalModelName rejects unregistered names', () => { + expect(normalizeLogicalModelName(null)).toBe(null); + expect(normalizeLogicalModelName(' ')).toBe(null); + expect(normalizeLogicalModelName('FieldDefault')).toBe('FieldDefault'); + expect(() => normalizeLogicalModelName('Partner')).toThrow(/not a registered logical model/); +}); + +test('normalizeLogicalMethods canonicalizes PascalCase and dedupes case-insensitively', () => { + expect(normalizeLogicalMethods(null)).toBe(null); + expect(normalizeLogicalMethods([])).toBe(null); + expect(normalizeLogicalMethods(['search', 'Browse', 'SEARCH'])).toEqual(['Search', 'Browse']); + expect(canonicalizeLogicalMethodName('getEffective')).toBe('GetEffective'); + expect(normalizeLogicalMethods('["Update"]')).toEqual(['Update']); + expect(() => normalizeLogicalMethods('not-json')).toThrow(/must be a JSON string array/); +}); + +test('logicalMethodsAllow treats null/empty as all methods', () => { + expect(logicalMethodsAllow(null, 'Search')).toBe(true); + expect(logicalMethodsAllow([], 'Search')).toBe(true); + expect(logicalMethodsAllow(['Search', 'Browse'], 'update')).toBe(false); + expect(logicalMethodsAllow(['Search', 'Browse'], 'SEARCH')).toBe(true); +}); diff --git a/modules/auth/service/tests/method_access_eval_observability.test.ts b/modules/auth/service/tests/method_access_eval_observability.test.ts index c0f347c9..031acff9 100644 --- a/modules/auth/service/tests/method_access_eval_observability.test.ts +++ b/modules/auth/service/tests/method_access_eval_observability.test.ts @@ -85,6 +85,29 @@ test('evaluateRoleMethodAccess returns deny allow and empty diagnostics with hit hitRuleIds: [], reason: 'method_access_no_manual_rule', }); + + // LogicalModel + LogicalMethods: only matching methods participate. + (RoleMethodAccess as any).Search = async () => [ + { + Id: 'ma_logical_search', + Mode: 'allow', + Source: 'manual', + LogicalModelName: 'TranslationTerm', + LogicalMethods: ['Search', 'Browse'], + }, + ]; + expect(await evaluateRoleMethodAccess(['role_1'], [[['LogicalModelName', '=', 'TranslationTerm']] as any], 'search')).toEqual({ + denied: false, + allowed: true, + hitRuleIds: ['ma_logical_search'], + reason: 'method_access_allow', + }); + expect(await evaluateRoleMethodAccess(['role_1'], [[['LogicalModelName', '=', 'TranslationTerm']] as any], 'update')).toEqual({ + denied: false, + allowed: false, + hitRuleIds: [], + reason: 'method_access_no_manual_rule', + }); } finally { (RoleMethodAccess as any).Search = orig; } diff --git a/modules/auth/service/tests/permission_state_acl_source.test.ts b/modules/auth/service/tests/permission_state_acl_source.test.ts index 2e32e564..220f3793 100644 --- a/modules/auth/service/tests/permission_state_acl_source.test.ts +++ b/modules/auth/service/tests/permission_state_acl_source.test.ts @@ -239,3 +239,46 @@ test('buildAclAggregation dedupe tolerates null MetaModel Search and multi-app m (MetaApplication as any).Search = origApp; } }); + +test('buildAclAggregation treats LogicalModelName as logical scope not global', async () => { + const origAccess = (RoleMethodAccess as any).Search; + const origService = (MetaService as any).Search; + const origModel = (MetaModel as any).Search; + const origApp = (MetaApplication as any).Search; + + try { + (RoleMethodAccess as any).Search = async () => [ + { + RoleId: 'role_1', + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + LogicalMethods: ['Get', 'Set'], + Mode: 'allow', + Source: 'manual', + }, + ]; + (MetaService as any).Search = async () => []; + (MetaApplication as any).Search = async () => []; + (MetaModel as any).Search = async () => [ + { Application: 'auth', Name: 'FieldDefault', UpdatedAt: '2026-01-02' }, + { Application: 'base', Name: 'FieldDefault', UpdatedAt: '2026-01-01' }, + { Application: 'auth', Name: 'User', UpdatedAt: '2026-01-03' }, + ]; + + const agg = await buildAclAggregation(['role_1'], { role_1: { global: true, companies: [] } }); + expect(agg.companyGlobalAllow.has('*')).toBe(false); + const allows = agg.requiresAllowKeysByCompany.get('*') || new Set(); + expect(allows.has('rpc:/auth.FieldDefault/Get')).toBe(true); + expect(allows.has('rpc:/auth.FieldDefault/Set')).toBe(true); + expect(allows.has('rpc:/base.FieldDefault/Get')).toBe(true); + expect(allows.has('rpc:/auth.FieldDefault/*')).toBe(false); + expect(allows.has('rpc:/auth.User/*')).toBe(false); + } finally { + (RoleMethodAccess as any).Search = origAccess; + (MetaService as any).Search = origService; + (MetaModel as any).Search = origModel; + (MetaApplication as any).Search = origApp; + } +}); diff --git a/modules/auth/service/tests/rule_scope_helpers.test.ts b/modules/auth/service/tests/rule_scope_helpers.test.ts index bbf4b129..e4a8c0d5 100644 --- a/modules/auth/service/tests/rule_scope_helpers.test.ts +++ b/modules/auth/service/tests/rule_scope_helpers.test.ts @@ -3,26 +3,32 @@ import { assertExclusiveScope } from '@/auth/service/models/_rule_scope_helpers'; -test('rule scope helpers: method accepts service/model/application/global shapes', () => { - const service = { MetaServiceId: 'svc1', MetaModelId: null, MetaApplicationId: null }; +test('rule scope helpers: method accepts service/model/application/logical/global shapes', () => { + const service = { MetaServiceId: 'svc1', MetaModelId: null, MetaApplicationId: null, LogicalModelName: null }; assertExclusiveScope(service, 'create', 'method'); expect(service.MetaServiceId).toBe('svc1'); expect(service.MetaModelId).toBe(null); expect(service.MetaApplicationId).toBe(null); + expect(service.LogicalModelName).toBe(null); - const model = { MetaServiceId: null, MetaModelId: 'm1', MetaApplicationId: null }; + const model = { MetaServiceId: null, MetaModelId: 'm1', MetaApplicationId: null, LogicalModelName: null }; assertExclusiveScope(model, 'create', 'method'); expect(model.MetaModelId).toBe('m1'); - const app = { MetaServiceId: null, MetaModelId: null, MetaApplicationId: 'a1' }; + const app = { MetaServiceId: null, MetaModelId: null, MetaApplicationId: 'a1', LogicalModelName: null }; assertExclusiveScope(app, 'create', 'method'); expect(app.MetaApplicationId).toBe('a1'); - const global = { MetaServiceId: null, MetaModelId: null, MetaApplicationId: null }; + const logical = { MetaServiceId: null, MetaModelId: null, MetaApplicationId: null, LogicalModelName: 'TranslationTerm' }; + assertExclusiveScope(logical, 'create', 'method'); + expect(logical.LogicalModelName).toBe('TranslationTerm'); + + const global = { MetaServiceId: null, MetaModelId: null, MetaApplicationId: null, LogicalModelName: null }; assertExclusiveScope(global, 'create', 'method'); expect(global.MetaServiceId).toBe(null); expect(global.MetaModelId).toBe(null); expect(global.MetaApplicationId).toBe(null); + expect(global.LogicalModelName).toBe(null); }); test('rule scope helpers: record accepts model/application/global shapes', () => { @@ -41,24 +47,30 @@ test('rule scope helpers: record accepts model/application/global shapes', () => expect(global.MetaApplicationId).toBe(null); }); -test('rule scope helpers: field accepts field/model/application/global shapes', () => { - const field = { MetaFieldId: 'f1', MetaModelId: 'm1', MetaApplicationId: null }; +test('rule scope helpers: field accepts field/model/application/logical/global shapes', () => { + const field = { MetaFieldId: 'f1', MetaModelId: 'm1', MetaApplicationId: null, LogicalModelName: null }; assertExclusiveScope(field, 'create', 'field'); expect(field.MetaFieldId).toBe('f1'); expect(field.MetaModelId).toBe('m1'); expect(field.MetaApplicationId).toBe(null); + expect(field.LogicalModelName).toBe(null); - const model = { MetaFieldId: null, MetaModelId: 'm1', MetaApplicationId: null }; + const model = { MetaFieldId: null, MetaModelId: 'm1', MetaApplicationId: null, LogicalModelName: null }; assertExclusiveScope(model, 'create', 'field'); expect(model.MetaModelId).toBe('m1'); - const app = { MetaFieldId: null, MetaModelId: null, MetaApplicationId: 'a1' }; + const app = { MetaFieldId: null, MetaModelId: null, MetaApplicationId: 'a1', LogicalModelName: null }; assertExclusiveScope(app, 'create', 'field'); expect(app.MetaApplicationId).toBe('a1'); - const global = { MetaFieldId: null, MetaModelId: null, MetaApplicationId: null }; + const logical = { MetaFieldId: null, MetaModelId: null, MetaApplicationId: null, LogicalModelName: 'AppSetting' }; + assertExclusiveScope(logical, 'create', 'field'); + expect(logical.LogicalModelName).toBe('AppSetting'); + + const global = { MetaFieldId: null, MetaModelId: null, MetaApplicationId: null, LogicalModelName: null }; assertExclusiveScope(global, 'create', 'field'); expect(global.MetaFieldId).toBe(null); + expect(global.LogicalModelName).toBe(null); }); test('rule scope helpers: ui accepts resource/application/global shapes', () => { @@ -77,15 +89,22 @@ test('rule scope helpers: ui accepts resource/application/global shapes', () => }); test('rule scope helpers: mixed scope throws golden error messages', () => { - expect(() => assertExclusiveScope({ MetaServiceId: 's1', MetaModelId: 'm1', MetaApplicationId: null }, 'create', 'method')).toThrow( - 'invalid RoleMethodAccess scope: must be exactly one of service/model/application/global' - ); + expect(() => + assertExclusiveScope({ MetaServiceId: 's1', MetaModelId: 'm1', MetaApplicationId: null, LogicalModelName: null }, 'create', 'method') + ).toThrow('invalid RoleMethodAccess scope: must be exactly one of service/model/application/logical_model/global'); + expect(() => + assertExclusiveScope( + { MetaServiceId: null, MetaModelId: null, MetaApplicationId: null, LogicalModelName: 'Partner' }, + 'create', + 'method' + ) + ).toThrow(/not a registered logical model/); expect(() => assertExclusiveScope({ MetaModelId: 'm1', MetaApplicationId: 'a1' }, 'create', 'record')).toThrow( 'invalid RoleRecordRule scope: must be exactly one of model/application/global' ); - expect(() => assertExclusiveScope({ MetaFieldId: 'f1', MetaModelId: null, MetaApplicationId: null }, 'create', 'field')).toThrow( - 'invalid RoleFieldRule scope: must be exactly one of field/model/application/global' - ); + expect(() => + assertExclusiveScope({ MetaFieldId: 'f1', MetaModelId: null, MetaApplicationId: null, LogicalModelName: null }, 'create', 'field') + ).toThrow('invalid RoleFieldRule scope: must be exactly one of field/model/application/logical_model/global'); expect(() => assertExclusiveScope({ MetaUiResourceId: 'u1', MetaApplicationId: 'a1' }, 'create', 'ui')).toThrow( 'invalid RoleUiResource scope: must be exactly one of resource/application/global' ); @@ -93,13 +112,13 @@ test('rule scope helpers: mixed scope throws golden error messages', () => { test('rule scope helpers: update missing sibling fields throws golden messages', () => { expect(() => assertExclusiveScope({ MetaModelId: 'm1' }, 'update', 'method')).toThrow( - 'invalid RoleMethodAccess scope update: must provide MetaServiceId/MetaModelId/MetaApplicationId together' + 'invalid RoleMethodAccess scope update: must provide MetaServiceId/MetaModelId/MetaApplicationId/LogicalModelName together' ); expect(() => assertExclusiveScope({ MetaModelId: 'm1' }, 'update', 'record')).toThrow( 'invalid RoleRecordRule scope update: must provide MetaModelId/MetaApplicationId together' ); expect(() => assertExclusiveScope({ MetaFieldId: 'f1' }, 'update', 'field')).toThrow( - 'invalid RoleFieldRule scope update: must provide MetaFieldId/MetaModelId/MetaApplicationId together' + 'invalid RoleFieldRule scope update: must provide MetaFieldId/MetaModelId/MetaApplicationId/LogicalModelName together' ); expect(() => assertExclusiveScope({ MetaUiResourceId: 'u1' }, 'update', 'ui')).toThrow( 'invalid RoleUiResource scope update: must provide MetaUiResourceId/MetaApplicationId together' @@ -112,6 +131,7 @@ test('rule scope helpers: field empty create normalizes to global', () => { expect(values.MetaFieldId).toBe(null); expect(values.MetaModelId).toBe(null); expect(values.MetaApplicationId).toBe(null); + expect(values.LogicalModelName).toBe(null); }); test('rule scope helpers: method/record/ui empty create is a no-op', () => { @@ -141,11 +161,13 @@ test('rule scope helpers: normalizes object/string refs and blank strings', () = MetaServiceId: { id: 'svc-x' }, MetaModelId: null, MetaApplicationId: undefined, + LogicalModelName: null, }; assertExclusiveScope(method, 'create', 'method'); expect(method.MetaServiceId).toBe('svc-x'); expect(method.MetaModelId).toBe(null); expect(method.MetaApplicationId).toBe(null); + expect(method.LogicalModelName).toBe(null); }); test('rule scope helpers: update without scope keys is a no-op', () => { @@ -170,7 +192,13 @@ test('rule scope helpers: update with all sibling fields succeeds', () => { expect(record.MetaModelId).toBe('m1'); expect(record.MetaApplicationId).toBe(null); - const method: Record = { MetaServiceId: null, MetaModelId: null, MetaApplicationId: 'a1' }; + const method: Record = { + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: 'a1', + LogicalModelName: null, + }; assertExclusiveScope(method, 'update', 'method'); expect(method.MetaApplicationId).toBe('a1'); + expect(method.LogicalModelName).toBe(null); }); diff --git a/modules/auth/web/views/RoleFieldRuleFormView.vue b/modules/auth/web/views/RoleFieldRuleFormView.vue index 542aa269..961b184e 100644 --- a/modules/auth/web/views/RoleFieldRuleFormView.vue +++ b/modules/auth/web/views/RoleFieldRuleFormView.vue @@ -25,7 +25,11 @@ SPDX-License-Identifier: Apache-2.0 :closable="false" show-icon :title="_t('Cross-role field rule editor')" - :description="_t('Role is required. Leave Application/Model/Field empty for wider scopes (global / app / model).')" + :description=" + _t( + 'Role is required. Pick exactly one scope: Field (+ Model), Model, Application, Logical Model (all host apps / all business fields on that short name), or leave all empty for Global.' + ) + " /> @@ -46,6 +50,9 @@ SPDX-License-Identifier: Apache-2.0 + + + diff --git a/modules/auth/web/views/RoleFieldRuleListView.vue b/modules/auth/web/views/RoleFieldRuleListView.vue index 167eab5f..0dadcbd2 100644 --- a/modules/auth/web/views/RoleFieldRuleListView.vue +++ b/modules/auth/web/views/RoleFieldRuleListView.vue @@ -20,6 +20,7 @@ SPDX-License-Identifier: Apache-2.0 + diff --git a/modules/auth/web/views/RoleMethodAccessFormView.vue b/modules/auth/web/views/RoleMethodAccessFormView.vue index 5d3748fd..9740a983 100644 --- a/modules/auth/web/views/RoleMethodAccessFormView.vue +++ b/modules/auth/web/views/RoleMethodAccessFormView.vue @@ -27,7 +27,7 @@ SPDX-License-Identifier: Apache-2.0 :title="_t('Cross-role method access editor')" :description=" _t( - 'Role is required. New rows default to Mode=deny on the model; prefer allow for grants and deny as an explicit brake. Source is always manual under UI-Option-A (UI grants live in RoleUiResource; do not materialize Method).' + 'Role is required. Pick exactly one scope: Service, Model, Application, Logical Model (all host apps sharing that short name), or leave all empty for Global. New rows default to Mode=deny; prefer allow for grants. Source is always manual under UI-Option-A.' ) " /> @@ -50,6 +50,12 @@ SPDX-License-Identifier: Apache-2.0 + + + + + + @@ -88,6 +94,7 @@ import ODateTimeField from '@/web/web/components/field/ODatetimeField.vue'; import OSelectionField from '@/web/web/components/field/OSelectionField.vue'; import OManyToOneField from '@/web/web/components/field/OManyToOneField.vue'; import OManyToOneRefField from '@/web/web/components/field/OManyToOneRefField.vue'; +import OJsonobjectField from '@/web/web/components/field/OJsonobjectField.vue'; import type { ValueClickPayload as ManyToOneValueClickPayload } from '@/web/web/components/field/manyToOneTypes'; import RoleListView from '@/auth/web/views/RoleListView.vue'; import type { ViewMode } from '@/web/web/components/view/OViewScope.vue'; diff --git a/modules/auth/web/views/RoleMethodAccessListView.vue b/modules/auth/web/views/RoleMethodAccessListView.vue index c4a221ca..c3c106f3 100644 --- a/modules/auth/web/views/RoleMethodAccessListView.vue +++ b/modules/auth/web/views/RoleMethodAccessListView.vue @@ -20,6 +20,7 @@ SPDX-License-Identifier: Apache-2.0 + diff --git a/modules/auth/web/views/access_rules_field_binding.test.ts b/modules/auth/web/views/access_rules_field_binding.test.ts index 3b0d798a..8792144b 100644 --- a/modules/auth/web/views/access_rules_field_binding.test.ts +++ b/modules/auth/web/views/access_rules_field_binding.test.ts @@ -33,4 +33,15 @@ describe('Access Rules admin field binding (PR-C-5)', () => { expect(form).toContain('Select Role'); } }); + + it('exposes LogicalModel scope on Method Access and Field Rule forms (PR-LM-4)', () => { + const methodForm = viewSource('RoleMethodAccessFormView.vue'); + expect(methodForm).toContain('prop="LogicalModelName"'); + expect(methodForm).toContain('prop="LogicalMethods"'); + expect(methodForm).toContain('Logical Model (all host apps sharing that short name)'); + + const fieldForm = viewSource('RoleFieldRuleFormView.vue'); + expect(fieldForm).toContain('prop="LogicalModelName"'); + expect(fieldForm).toContain('Logical Model (all host apps / all business fields on that short name)'); + }); }); diff --git a/modules/core/service/orm/model/app_setting_base_model.ts b/modules/core/service/orm/model/app_setting_base_model.ts index 55038040..26884702 100644 --- a/modules/core/service/orm/model/app_setting_base_model.ts +++ b/modules/core/service/orm/model/app_setting_base_model.ts @@ -11,6 +11,7 @@ import { } from '../repository/authz'; import BaseModel from './model'; import type { InstantiableModelCtor } from './types'; +import { registerLogicalModelName } from './logical_model_registry'; /** Minimal surface for `pool('AppSetting')` typing. */ export type AppSettingModelCtor = { @@ -203,3 +204,6 @@ export function __invalidateAppSettingMemoForTest(application: string, key: stri export function __isUniqueConstraintErrorForTest(err: unknown): boolean { return isUniqueConstraintError(err); } + +// LogicalModel ACL eligibility (auth RoleMethodAccess / RoleFieldRule). +registerLogicalModelName('AppSetting'); diff --git a/modules/core/service/orm/model/field_default_base_model.ts b/modules/core/service/orm/model/field_default_base_model.ts index 40a4befa..ae49ea53 100644 --- a/modules/core/service/orm/model/field_default_base_model.ts +++ b/modules/core/service/orm/model/field_default_base_model.ts @@ -15,6 +15,7 @@ import { import BaseModel from './model'; import { resolveEffectiveFieldDefaults } from './field_default_resolve'; import type { InstantiableModelCtor } from './types'; +import { registerLogicalModelName } from './logical_model_registry'; /** Align Odoo: False→global; True→current; id→specific. */ export type FieldDefaultScopeDim = string | boolean | null | undefined; @@ -380,3 +381,6 @@ export function __resetFieldDefaultUniqueIndexTablesForTest(): void { export function __invalidateFieldDefaultMemoForTest(application: string, modelShort: string): void { invalidateFieldDefaultMemo(application, modelShort); } + +// LogicalModel ACL eligibility (auth RoleMethodAccess / RoleFieldRule). +registerLogicalModelName('FieldDefault'); diff --git a/modules/core/service/orm/model/logical_model_registry.test.ts b/modules/core/service/orm/model/logical_model_registry.test.ts new file mode 100644 index 00000000..33ea2143 --- /dev/null +++ b/modules/core/service/orm/model/logical_model_registry.test.ts @@ -0,0 +1,49 @@ +// SPDX-FileCopyrightText: 2026-present Brian Wang +// SPDX-License-Identifier: Apache-2.0 + +import { + __resetLogicalModelNamesForTest, + isRegisteredLogicalModelName, + listLogicalModelNames, + listLogicalModelSelection, + registerLogicalModelName, +} from './logical_model_registry'; + +// Side-effect: platform inject bases self-register on import. +import './app_setting_base_model'; +import './field_default_base_model'; +import './translation_term_base_model'; + +test('platform inject bases self-register logical model short names', () => { + expect(listLogicalModelNames()).toEqual(['AppSetting', 'FieldDefault', 'TranslationTerm']); + expect(isRegisteredLogicalModelName('TranslationTerm')).toBe(true); + expect(isRegisteredLogicalModelName('Partner')).toBe(false); + expect(isRegisteredLogicalModelName('')).toBe(false); +}); + +test('listLogicalModelSelection mirrors registered names for FieldsGet', () => { + expect(listLogicalModelSelection()).toEqual([ + { value: 'AppSetting', label: 'AppSetting' }, + { value: 'FieldDefault', label: 'FieldDefault' }, + { value: 'TranslationTerm', label: 'TranslationTerm' }, + ]); +}); + +test('registerLogicalModelName is idempotent and ignores blanks', () => { + registerLogicalModelName(' '); + registerLogicalModelName('AppSetting'); + expect(listLogicalModelNames()).toEqual(['AppSetting', 'FieldDefault', 'TranslationTerm']); +}); + +test('__resetLogicalModelNamesForTest clears registry for isolation', () => { + __resetLogicalModelNamesForTest(); + expect(listLogicalModelNames()).toEqual([]); + registerLogicalModelName('TmpLogical'); + expect(isRegisteredLogicalModelName('TmpLogical')).toBe(true); + // Restore platform names for sibling tests in the same process. + __resetLogicalModelNamesForTest(); + registerLogicalModelName('AppSetting'); + registerLogicalModelName('FieldDefault'); + registerLogicalModelName('TranslationTerm'); + expect(listLogicalModelNames()).toEqual(['AppSetting', 'FieldDefault', 'TranslationTerm']); +}); diff --git a/modules/core/service/orm/model/logical_model_registry.ts b/modules/core/service/orm/model/logical_model_registry.ts new file mode 100644 index 00000000..fd657117 --- /dev/null +++ b/modules/core/service/orm/model/logical_model_registry.ts @@ -0,0 +1,52 @@ +// SPDX-FileCopyrightText: 2026-present Brian Wang +// SPDX-License-Identifier: Apache-2.0 + +/** + * Process-local registry of @Model short names eligible for auth LogicalModel ACL scope. + * + * Platform inject bases (AppSetting / FieldDefault / TranslationTerm / …) call + * {@link registerLogicalModelName} at module load. Auth write validation reads this set; + * runtime Method/FieldRule matching still compares short names only (no Ensure flag). + * + * See `.dev/docs/auth/logical_model_acl_field_rule_design.md` §5. + */ + +const registered = new Set(); + +/** + * Register a logical model short name (idempotent). Empty names are ignored. + */ +export function registerLogicalModelName(name: string): void { + const n = String(name ?? '').trim(); + if (!n) return; + registered.add(n); +} + +/** + * Whether `name` was registered by a platform inject base (exact short-name match). + */ +export function isRegisteredLogicalModelName(name: string | null | undefined): boolean { + const n = String(name ?? '').trim(); + return n.length > 0 && registered.has(n); +} + +/** + * Sorted snapshot of registered logical model short names (for admin pickers / tests). + */ +export function listLogicalModelNames(): string[] { + return Array.from(registered).sort((a, b) => a.localeCompare(b)); +} + +/** + * FieldsGet / OSelectionField options for LogicalModelName. + */ +export function listLogicalModelSelection(): Array<{ value: string; label: string }> { + return listLogicalModelNames().map(name => ({ value: name, label: name })); +} + +/** + * Test-only: clear the registry (does not re-run base-model self-registration). + */ +export function __resetLogicalModelNamesForTest(): void { + registered.clear(); +} diff --git a/modules/core/service/orm/model/translation_term_base_model.ts b/modules/core/service/orm/model/translation_term_base_model.ts index 1ac6e644..96d416c9 100644 --- a/modules/core/service/orm/model/translation_term_base_model.ts +++ b/modules/core/service/orm/model/translation_term_base_model.ts @@ -7,6 +7,7 @@ import { raiseDomainError } from '@/core/service/error'; import { withRepositoryAuthzRuleBypass } from '../repository/authz'; import BaseModel from './model'; import type { InstantiableModelCtor } from './types'; +import { registerLogicalModelName } from './logical_model_registry'; import type { Insertable, Updateable, @@ -541,3 +542,6 @@ export default class TranslationTermBaseModel extends BaseModel { function hostApplication(ctor: any): string { return String(storeMeta(ctor as InstantiableModelCtor)?.application || '').trim(); } + +// LogicalModel ACL eligibility (auth RoleMethodAccess / RoleFieldRule). +registerLogicalModelName('TranslationTerm'); From 9e269e52622af21cd44ea9733c9456aaba559225 Mon Sep 17 00:00:00 2001 From: Brian Wang Date: Fri, 7 Aug 2026 22:47:08 +0800 Subject: [PATCH 02/10] fix(task): harden e2e smoke login against auth init race - Reuse auth's loginAsE2EAdmin helper so Login submit waits for nprogress and Login RPC, matching meta and avoiding stuck /web/login redirects. Co-authored-by: Cursor --- modules/task/e2e/smoke.spec.ts | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/modules/task/e2e/smoke.spec.ts b/modules/task/e2e/smoke.spec.ts index 6dac705a..02bcbb14 100644 --- a/modules/task/e2e/smoke.spec.ts +++ b/modules/task/e2e/smoke.spec.ts @@ -8,6 +8,7 @@ import { createClient, type Interceptor } from '@connectrpc/connect'; import { createGrpcWebTransport } from '@connectrpc/connect-web'; import { create } from '@bufbuild/protobuf'; import { ValueSchema, ListValueSchema, StructSchema, NullValue, type Value } from '@bufbuild/protobuf/wkt'; +import { loginAsE2EAdmin } from '../../auth/e2e/utils/login.ts'; /** * Runtime metadata injected by the task e2e harness. @@ -220,19 +221,6 @@ function makeTaskClients(baseURL: string, accessToken: string, services: { Sched }; } -/** - * Signs in as the e2e admin user through the browser UI. - */ -async function loginAsE2EAdmin(page: any, baseURL: string): Promise { - await page.goto(`${baseURL}/web/auth/users`, { waitUntil: 'domcontentloaded' }); - - await page.getByPlaceholder(/用户名|username/i).waitFor({ timeout: 10_000 }); - await page.getByPlaceholder(/用户名|username/i).fill('e2e-admin'); - await page.getByPlaceholder(/密码|password/i).fill('e2e-admin'); - await page.locator('button[type="submit"]').click(); - await expect(page).toHaveURL(/\/web\/auth\/users/, { timeout: 15_000 }); -} - test('task: create schedule and trigger job via gRPC-web', async ({ page }) => { test.setTimeout(120_000); From 7da554fd17efe65c00a308c24803b1d2d1b3d099 Mon Sep 17 00:00:00 2001 From: Brian Wang Date: Fri, 7 Aug 2026 22:56:01 +0800 Subject: [PATCH 03/10] fix(auth): address LogicalModel ACL review findings - Fail closed on malformed LogicalMethods (deny keeps/model-wide; allow skips) and reject non-string whitelist entries. - Clear stale LogicalMethods when LogicalModelName changes; allow methods-only updates; drop unused supportsLogicalModel. - Bypass RecordRule/FieldRule on FieldDefault store Get/Set/Unset paths so preset Logical Method grants work without O(apps) RR seeds. Co-authored-by: Cursor --- .../service/models/_logical_model_registry.ts | 4 ++ .../service/models/_rule_scope_helpers.ts | 6 --- .../service/models/_user_field_rule_eval.ts | 4 +- .../service/models/_user_method_access.ts | 7 ++- .../models/_user_permission_state_acl.ts | 4 +- .../auth/service/models/role_method_access.ts | 8 ++-- .../tests/logical_model_registry.test.ts | 1 + .../tests/permission_state_acl_source.test.ts | 18 +++++++ .../views/access_rules_field_binding.test.ts | 14 +++++- .../orm/model/field_default_base_model.ts | 47 +++++++++++-------- 10 files changed, 79 insertions(+), 34 deletions(-) diff --git a/modules/auth/service/models/_logical_model_registry.ts b/modules/auth/service/models/_logical_model_registry.ts index 8c1f87f0..8734db69 100644 --- a/modules/auth/service/models/_logical_model_registry.ts +++ b/modules/auth/service/models/_logical_model_registry.ts @@ -71,6 +71,9 @@ export function normalizeLogicalMethods(raw: unknown): string[] | null { const out: string[] = []; const seen = new Set(); for (const item of arr) { + if (typeof item !== 'string') { + throw new Error('invalid LogicalMethods: each entry must be a string'); + } const name = canonicalizeLogicalMethodName(item); if (!name) continue; const key = name.toLowerCase(); @@ -83,6 +86,7 @@ export function normalizeLogicalMethods(raw: unknown): string[] | null { /** * Whether a LogicalMethods whitelist (null/empty = all) covers `methodName`. + * Malformed payloads throw; callers that must not abort evaluation should catch. */ export function logicalMethodsAllow(methods: unknown, methodName: string): boolean { const want = String(methodName || '') diff --git a/modules/auth/service/models/_rule_scope_helpers.ts b/modules/auth/service/models/_rule_scope_helpers.ts index 04768a29..f700c210 100644 --- a/modules/auth/service/models/_rule_scope_helpers.ts +++ b/modules/auth/service/models/_rule_scope_helpers.ts @@ -26,8 +26,6 @@ type ProfileSpec = { shapesLabel: string; /** When true, create always validates/normalizes scope even if no scope keys are present (empty → global). */ alwaysValidateOnCreate: boolean; - /** When true, LogicalModelName participates and is registry-validated. */ - supportsLogicalModel: boolean; isValidShape: (ids: Record) => boolean; }; @@ -37,7 +35,6 @@ const PROFILE_SPECS: Record = { fields: ['MetaServiceId', 'MetaModelId', 'MetaApplicationId', 'LogicalModelName'], shapesLabel: 'service/model/application/logical_model/global', alwaysValidateOnCreate: false, - supportsLogicalModel: true, isValidShape: ids => { const logical = ids.LogicalModelName; const isService = @@ -58,7 +55,6 @@ const PROFILE_SPECS: Record = { fields: ['MetaModelId', 'MetaApplicationId'], shapesLabel: 'model/application/global', alwaysValidateOnCreate: false, - supportsLogicalModel: false, isValidShape: ids => { const isModel = ids.MetaModelId != null && ids.MetaApplicationId == null; const isApplication = ids.MetaModelId == null && ids.MetaApplicationId != null; @@ -71,7 +67,6 @@ const PROFILE_SPECS: Record = { fields: ['MetaFieldId', 'MetaModelId', 'MetaApplicationId', 'LogicalModelName'], shapesLabel: 'field/model/application/logical_model/global', alwaysValidateOnCreate: true, - supportsLogicalModel: true, isValidShape: ids => { const logical = ids.LogicalModelName; const isField = @@ -92,7 +87,6 @@ const PROFILE_SPECS: Record = { fields: ['MetaUiResourceId', 'MetaApplicationId'], shapesLabel: 'resource/application/global', alwaysValidateOnCreate: false, - supportsLogicalModel: false, isValidShape: ids => { const isResource = ids.MetaUiResourceId != null && ids.MetaApplicationId == null; const isApplication = ids.MetaUiResourceId == null && ids.MetaApplicationId != null; diff --git a/modules/auth/service/models/_user_field_rule_eval.ts b/modules/auth/service/models/_user_field_rule_eval.ts index 0505ab24..ed1fcad4 100644 --- a/modules/auth/service/models/_user_field_rule_eval.ts +++ b/modules/auth/service/models/_user_field_rule_eval.ts @@ -100,6 +100,7 @@ function denyAllNonSystemFields(fieldNames: string[], reason: string, hitRuleIds * More-specific scope wins; same-scope deny-wins; read-deny ⇒ write-deny. */ export async function evaluateFieldRules(input: FieldRuleEvalInput): Promise { + const modelNameWant = String(input.modelName || '').trim(); const [applicationId, modelId] = await Promise.all([ resolveApplicationId(input.appName), resolveModelId(input.appName, input.modelName), @@ -181,7 +182,7 @@ export async function evaluateFieldRules(input: FieldRuleEvalInput): Promise { for (const m of models) { diff --git a/modules/auth/service/models/role_method_access.ts b/modules/auth/service/models/role_method_access.ts index f62487f6..38de30f0 100644 --- a/modules/auth/service/models/role_method_access.ts +++ b/modules/auth/service/models/role_method_access.ts @@ -188,11 +188,13 @@ export default class RoleMethodAccess extends BaseModel { throw new Error('invalid RoleMethodAccess: LogicalMethods requires LogicalModel scope'); } (values as any).LogicalMethods = null; + } else if (mode === 'update' && !touchesMethods) { + // Logical name changed/re-set without a new whitelist → drop stale methods for the prior model. + (values as any).LogicalMethods = null; } - } else if (mode === 'update' && touchesMethods && (values as any).LogicalMethods != null) { - // Methods-only update without LogicalModelName: cannot prove logical scope. - throw new Error('invalid RoleMethodAccess: LogicalMethods requires LogicalModel scope'); } + // Methods-only update (no LogicalModelName in payload): normalize and persist. + // Eval ignores LogicalMethods unless the persisted row is Logical scope. } private static _prepareValues(values: Record, mode: 'create' | 'update'): void { diff --git a/modules/auth/service/tests/logical_model_registry.test.ts b/modules/auth/service/tests/logical_model_registry.test.ts index 5ae1c4c1..7fcf1eb3 100644 --- a/modules/auth/service/tests/logical_model_registry.test.ts +++ b/modules/auth/service/tests/logical_model_registry.test.ts @@ -31,6 +31,7 @@ test('normalizeLogicalMethods canonicalizes PascalCase and dedupes case-insensit expect(canonicalizeLogicalMethodName('getEffective')).toBe('GetEffective'); expect(normalizeLogicalMethods('["Update"]')).toEqual(['Update']); expect(() => normalizeLogicalMethods('not-json')).toThrow(/must be a JSON string array/); + expect(() => normalizeLogicalMethods([1 as any])).toThrow(/each entry must be a string/); }); test('logicalMethodsAllow treats null/empty as all methods', () => { diff --git a/modules/auth/service/tests/permission_state_acl_source.test.ts b/modules/auth/service/tests/permission_state_acl_source.test.ts index 220f3793..58107122 100644 --- a/modules/auth/service/tests/permission_state_acl_source.test.ts +++ b/modules/auth/service/tests/permission_state_acl_source.test.ts @@ -275,6 +275,24 @@ test('buildAclAggregation treats LogicalModelName as logical scope not global', expect(allows.has('rpc:/base.FieldDefault/Get')).toBe(true); expect(allows.has('rpc:/auth.FieldDefault/*')).toBe(false); expect(allows.has('rpc:/auth.User/*')).toBe(false); + + (RoleMethodAccess as any).Search = async () => [ + { + RoleId: 'role_1', + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + LogicalMethods: null, + Mode: 'allow', + Source: 'manual', + }, + ]; + const aggAll = await buildAclAggregation(['role_1'], { role_1: { global: true, companies: [] } }); + const allowsAll = aggAll.requiresAllowKeysByCompany.get('*') || new Set(); + expect(allowsAll.has('rpc:/auth.FieldDefault/*')).toBe(true); + expect(allowsAll.has('rpc:/base.FieldDefault/*')).toBe(true); + expect(aggAll.companyGlobalAllow.has('*')).toBe(false); } finally { (RoleMethodAccess as any).Search = origAccess; (MetaService as any).Search = origService; diff --git a/modules/auth/web/views/access_rules_field_binding.test.ts b/modules/auth/web/views/access_rules_field_binding.test.ts index 8792144b..475b040d 100644 --- a/modules/auth/web/views/access_rules_field_binding.test.ts +++ b/modules/auth/web/views/access_rules_field_binding.test.ts @@ -38,10 +38,20 @@ describe('Access Rules admin field binding (PR-C-5)', () => { const methodForm = viewSource('RoleMethodAccessFormView.vue'); expect(methodForm).toContain('prop="LogicalModelName"'); expect(methodForm).toContain('prop="LogicalMethods"'); - expect(methodForm).toContain('Logical Model (all host apps sharing that short name)'); + expect(methodForm).toContain(':allow-array="true"'); const fieldForm = viewSource('RoleFieldRuleFormView.vue'); expect(fieldForm).toContain('prop="LogicalModelName"'); - expect(fieldForm).toContain('Logical Model (all host apps / all business fields on that short name)'); + + expect(viewSource('RoleMethodAccessListView.vue')).toContain('prop="LogicalModelName"'); + expect(viewSource('RoleFieldRuleListView.vue')).toContain('prop="LogicalModelName"'); + + // Exclusive scope Onchange lives on the models (not the Vue templates). + const methodModel = readFileSync(resolve(__dirname, '../../service/models/role_method_access.ts'), 'utf8'); + expect(methodModel).toContain("Onchange('LogicalModelName')"); + expect(methodModel).toContain("Onchange('MetaServiceId', 'MetaModelId', 'MetaApplicationId')"); + const fieldModel = readFileSync(resolve(__dirname, '../../service/models/role_field_rule.ts'), 'utf8'); + expect(fieldModel).toContain("Onchange('LogicalModelName')"); + expect(fieldModel).toContain("Onchange('MetaApplicationId', 'MetaFieldId')"); }); }); diff --git a/modules/core/service/orm/model/field_default_base_model.ts b/modules/core/service/orm/model/field_default_base_model.ts index ae49ea53..21a18eda 100644 --- a/modules/core/service/orm/model/field_default_base_model.ts +++ b/modules/core/service/orm/model/field_default_base_model.ts @@ -209,11 +209,14 @@ async function findExactRow( userId: string | null, companyId: string | null ): Promise { - const rows = await (ctor as any).Search( - { - And: [['Model', '=', model], ['Field', '=', field], scopeCondition('UserId', userId), scopeCondition('CompanyId', companyId)], - } as any, - { fields: ['Id', 'Model', 'Field', 'UserId', 'CompanyId', 'Value'] as any, limit: 2 } as any + // Store lookup is not RecordRule-scoped (design §6.3); Method ACL gates Get/Set/Unset. + const rows = await withRepositoryAuthzRuleBypass(async () => + (ctor as any).Search( + { + And: [['Model', '=', model], ['Field', '=', field], scopeCondition('UserId', userId), scopeCondition('CompanyId', companyId)], + } as any, + { fields: ['Id', 'Model', 'Field', 'UserId', 'CompanyId', 'Value'] as any, limit: 2 } as any + ) ); return (rows && rows[0]) || undefined; } @@ -258,20 +261,23 @@ export default class FieldDefaultBaseModel extends BaseModel { await ensureScopeUniqueIndex(this); + // Method ACL gates Set; store rows are not RecordRule-scoped (design §6.3). try { - await (this as any).withSavepoint(async () => { - const existing = await findExactRow(this, modelShort, fieldName, userId, companyId); - if (existing?.Id) { - await (this as any).UpdateById(existing.Id, { Value: stored } as any); - return; - } - await (this as any).Create({ - Model: modelShort, - Field: fieldName, - UserId: userId, - CompanyId: companyId, - Value: stored, - } as any); + await withRepositoryAuthzRuleBypass(async () => { + await (this as any).withSavepoint(async () => { + const existing = await findExactRow(this, modelShort, fieldName, userId, companyId); + if (existing?.Id) { + await (this as any).UpdateById(existing.Id, { Value: stored } as any); + return; + } + await (this as any).Create({ + Model: modelShort, + Field: fieldName, + UserId: userId, + CompanyId: companyId, + Value: stored, + } as any); + }); }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); @@ -366,7 +372,10 @@ export default class FieldDefaultBaseModel extends BaseModel { const modelShort = String(model).trim(); const row = await findExactRow(this, modelShort, String(field).trim(), userId, companyId); if (row?.Id) { - await (this as any).DeleteById(row.Id); + // Method ACL gates Unset when exposed; store delete is not RecordRule-scoped (§6.3). + await withRepositoryAuthzRuleBypass(async () => { + await (this as any).DeleteById(row.Id); + }); invalidateFieldDefaultMemo(resolveFieldDefaultApplication(this, targetMeta), modelShort); } } From 87ab23fc42a307c98631c20765b8906a49ddd619 Mon Sep 17 00:00:00 2001 From: Brian Wang Date: Fri, 7 Aug 2026 23:37:47 +0800 Subject: [PATCH 04/10] test(auth): bring LogicalModel ACL patch coverage to 100% - Cover LogicalMethods normalize/Onchange/FieldsGet, malformed fail-closed paths, and Logical FieldRule eval. - Exercise remaining registry and method-access branch edges Codecov reported as partials. Co-authored-by: Cursor --- .../tests/logical_model_acl_coverage.test.ts | 374 ++++++++++++++++++ .../tests/logical_model_registry.test.ts | 19 + .../method_access_eval_observability.test.ts | 67 ++++ .../tests/permission_state_acl_source.test.ts | 76 ++++ .../orm/model/logical_model_registry.test.ts | 4 + 5 files changed, 540 insertions(+) create mode 100644 modules/auth/service/tests/logical_model_acl_coverage.test.ts diff --git a/modules/auth/service/tests/logical_model_acl_coverage.test.ts b/modules/auth/service/tests/logical_model_acl_coverage.test.ts new file mode 100644 index 00000000..97fa16b6 --- /dev/null +++ b/modules/auth/service/tests/logical_model_acl_coverage.test.ts @@ -0,0 +1,374 @@ +// SPDX-FileCopyrightText: 2026-present Brian Wang +// SPDX-License-Identifier: Apache-2.0 + +/** + * Patch-coverage gaps for LogicalModel Method/Field ACL (PR #259 Codecov). + */ + +import { withContext as withModelContext } from '@/core/service/api/context'; +import Role from '@/auth/service/models/role'; +import RoleMethodAccess from '@/auth/service/models/role_method_access'; +import RoleFieldRule from '@/auth/service/models/role_field_rule'; +import { evaluateFieldRules } from '@/auth/service/models/_user_field_rule_eval'; +import { createServiceByModel } from '@/core/service/rpc'; +import type MetaApplicationModel from '@/meta/service/models/application'; +import type MetaFieldModel from '@/meta/service/models/field'; +import type MetaModelModel from '@/meta/service/models/model'; +import type MetaServiceModel from '@/meta/service/models/service'; + +const MetaService = createServiceByModel('meta.MetaService'); +const MetaModel = createServiceByModel('meta.MetaModel'); +const MetaApplication = createServiceByModel('meta.MetaApplication'); +const MetaField = createServiceByModel('meta.MetaField'); + +const RR_CACHE_KEY = Symbol.for('choysum.recordrule.cache'); +const FR_CACHE_KEY = Symbol.for('choysum.fieldrule.cache'); + +function ensureRequestContext(): any { + const root: any = (globalThis as any).$choysum ?? {}; + if (!root.request) root.request = {}; + if (!root.request.context) root.request.context = {}; + const jsCtx = root.request.context; + if (!jsCtx.ctx) jsCtx.ctx = {}; + if (!jsCtx.req) jsCtx.req = {}; + if (!jsCtx.identity) jsCtx.identity = {}; + (globalThis as any).$choysum = root; + return jsCtx; +} + +function resetRequestContext(): void { + const jsCtx = ensureRequestContext(); + jsCtx.ctx = {}; + jsCtx.req = { depth: 0, fieldRuleMode: 'skip' }; + jsCtx.identity = {}; + delete (jsCtx as any)[Symbol.for('choysum.ctx.override')]; + delete (jsCtx as any)[Symbol.for('choysum.ctx.frozen')]; + delete (jsCtx as any)[RR_CACHE_KEY]; + delete (jsCtx as any)[FR_CACHE_KEY]; +} + +function uid(prefix: string): string { + const xid = (globalThis as any).$choysum?.xid?.New?.(); + const u = typeof xid === 'string' && xid.trim() ? xid.trim() : String(Date.now()); + return `${prefix}_${u}`; +} + +async function createRole(code: string): Promise<{ id: string }> { + const created = await Role.Create( + { Code: code, Name: code, IsActive: true, IsSystem: false } as any, + ['Id'] as any + ); + return { id: String((created as any)?.Id || '').trim() }; +} + +function setupAllowlistForFixtures(): void { + const jsCtx = ensureRequestContext(); + jsCtx.req = { + depth: 0, + fieldRuleMode: 'skip', + recordRuleMode: 'allowlist', + recordRuleAllow: [ + 'auth.Role:read', + 'auth.Role:write', + 'auth.Role:create', + 'auth.Role:delete', + 'Role:read', + 'Role:write', + 'Role:create', + 'Role:delete', + 'auth.RoleMethodAccess:read', + 'auth.RoleMethodAccess:write', + 'auth.RoleMethodAccess:create', + 'auth.RoleMethodAccess:delete', + 'RoleMethodAccess:read', + 'RoleMethodAccess:write', + 'RoleMethodAccess:create', + 'RoleMethodAccess:delete', + 'meta.MetaService:read', + 'meta.MetaModel:read', + 'MetaService:read', + 'MetaModel:read', + ], + }; +} + +async function resolveService(modelId: string, method: string): Promise<{ id: string }> { + const rows = await MetaService.Search( + { And: [['ModelId', '=', modelId]] } as any, + { fields: ['Id', 'Name'], limit: 5000 } as any + ); + const want = method.toLowerCase(); + const hit = (rows || []).find((r: any) => String(r?.Name || '').trim().toLowerCase() === want); + const id = String((hit as any)?.Id || '').trim(); + if (!id) throw new Error(`missing MetaService ${modelId}/${method}`); + return { id }; +} + +async function resolveModelId(app: string, name: string): Promise { + const rows = await MetaModel.Search( + { And: [['Application', '=', app], ['Name', '=', name]] } as any, + { fields: ['Id'], limit: 1 } as any + ); + const id = String((rows?.[0] as any)?.Id || '').trim(); + if (!id) throw new Error(`missing MetaModel ${app}.${name}`); + return id; +} + +test('RoleMethodAccess/RoleFieldRule FieldsGet exposes LogicalModelName selection', async () => { + resetRequestContext(); + const { withRepositoryAuthzRuleBypass } = await import('@/core/service/orm/repository/authz/authz_runtime'); + + // FieldsGet prunes deny-read fields; bypass so LogicalModelName stays visible without seeding FR. + const ma = await withRepositoryAuthzRuleBypass(() => + RoleMethodAccess.FieldsGet(['LogicalModelName'], ['type', 'selection', 'selectionKind']) + ); + expect(ma).toBeTruthy(); + expect(Object.keys(ma || {})).toContain('LogicalModelName'); + expect(ma.LogicalModelName?.type).toBe('selection'); + expect(ma.LogicalModelName?.selectionKind).toBe('dynamic'); + const maSel = ma.LogicalModelName?.selection || []; + expect(maSel.some((x: { value?: string }) => x.value === 'FieldDefault')).toBe(true); + + const fr = await withRepositoryAuthzRuleBypass(() => + RoleFieldRule.FieldsGet(['LogicalModelName'], ['type', 'selection', 'selectionKind']) + ); + expect(Object.keys(fr || {})).toContain('LogicalModelName'); + expect(fr.LogicalModelName?.type).toBe('selection'); + expect(fr.LogicalModelName?.selectionKind).toBe('dynamic'); + const frSel = fr.LogicalModelName?.selection || []; + expect(frSel.some((x: { value?: string }) => x.value === 'TranslationTerm')).toBe(true); +}); + +test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on name change', async () => { + resetRequestContext(); + setupAllowlistForFixtures(); + + await withModelContext({ activeCompanyId: uid('C'), enabledCompanyIds: [uid('C')] } as any, async () => { + const role = await createRole(`ROLE_LM_COV_${uid('R')}`); + const userModelId = await resolveModelId('auth', 'User'); + const browse = await resolveService(userModelId, 'browse'); + + // touchesMethods on create with logical scope. + const created = await RoleMethodAccess.Create( + { + RoleId: { Id: role.id } as any, + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + LogicalMethods: ['get', 'Set', 'GET'], + Mode: 'allow', + } as any, + ['Id', 'LogicalModelName', 'LogicalMethods'] as any + ); + const id = String((created as any)?.Id || '').trim(); + expect(id.length > 0).toBe(true); + expect(String((created as any)?.LogicalModelName || '')).toBe('FieldDefault'); + expect((created as any)?.LogicalMethods).toEqual(['Get', 'Set']); + + // Non-logical scope + LogicalMethods → throw. + let rejected = false; + try { + await RoleMethodAccess.Create({ + RoleId: { Id: role.id } as any, + MetaServiceId: browse.id, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: null, + LogicalMethods: ['Get'], + Mode: 'allow', + } as any); + } catch (e: any) { + rejected = true; + expect(String(e?.message || e).includes('LogicalMethods requires LogicalModel scope')).toBe(true); + } + expect(rejected).toBe(true); + + // Change LogicalModelName without LogicalMethods → clear stale whitelist. + await RoleMethodAccess.UpdateById( + id, + { + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'AppSetting', + } as any, + ['Id', 'LogicalModelName', 'LogicalMethods'] as any + ); + const after = await RoleMethodAccess.Search( + ['Id', '=', id] as any, + { fields: ['Id', 'LogicalModelName', 'LogicalMethods'], limit: 1 } as any + ); + expect(String((after[0] as any)?.LogicalModelName || '')).toBe('AppSetting'); + const clearedMethods = (after[0] as any)?.LogicalMethods; + expect(clearedMethods == null || (typeof clearedMethods === 'object' && !Array.isArray(clearedMethods) && Object.keys(clearedMethods).length === 0)).toBe(true); + + // Methods-only update on existing logical row. + await RoleMethodAccess.UpdateById(id, { LogicalMethods: ['Get'] } as any, ['Id', 'LogicalMethods'] as any); + const methodsOnly = await RoleMethodAccess.Search( + ['Id', '=', id] as any, + { fields: ['Id', 'LogicalMethods'], limit: 1 } as any + ); + expect((methodsOnly[0] as any)?.LogicalMethods).toEqual(['Get']); + + // Private helper no-ops on nullish values. + expect(() => (RoleMethodAccess as any)._normalizeLogicalMethodsPayload(null, 'update')).not.toThrow(); + expect(() => (RoleMethodAccess as any)._normalizeLogicalMethodsPayload(undefined, 'create')).not.toThrow(); + }); +}); + +test('RoleMethodAccess Onchange clears LogicalMethods when logical name emptied or Meta set', async () => { + const cleared = await RoleMethodAccess.Onchange( + { + Id: 'onchange-ma-clear-logical', + LogicalModelName: '', + LogicalMethods: ['Get'], + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + }, + ['LogicalModelName'] + ); + expect(cleared.value).toEqual({ LogicalMethods: null }); + + const metaClears = await RoleMethodAccess.Onchange( + { + Id: 'onchange-ma-meta-clears', + LogicalModelName: 'FieldDefault', + LogicalMethods: ['Get'], + MetaServiceId: 'svc-1', + MetaModelId: null, + MetaApplicationId: null, + }, + ['MetaServiceId'] + ); + expect(metaClears.value).toEqual({ + LogicalModelName: null, + LogicalMethods: null, + }); + + // Meta set with empty LogicalModelName: only LogicalMethods clears (|| '' branch). + const metaNoLogical = await RoleMethodAccess.Onchange( + { + Id: 'onchange-ma-meta-no-logical', + LogicalModelName: null, + LogicalMethods: ['Get'], + MetaServiceId: 'svc-1', + MetaModelId: null, + MetaApplicationId: null, + }, + ['MetaServiceId'] + ); + expect(metaNoLogical.value).toEqual({ LogicalMethods: null }); +}); + +test('RoleFieldRule Onchange clears Logical when MetaModel or App/Field set', async () => { + const modelClears = await RoleFieldRule.Onchange( + { + Id: 'onchange-fr-model-logical', + MetaModelId: 'model-123', + MetaFieldId: 'field-456', + LogicalModelName: 'TranslationTerm', + }, + ['MetaModelId'] + ); + expect(modelClears.value).toEqual({ MetaFieldId: null, LogicalModelName: null }); + expect(modelClears.condition).toEqual([{ field: 'MetaFieldId', condition: ['ModelId', '=', 'model-123'] }]); + + const emptyLogical = await RoleFieldRule.Onchange( + { + Id: 'onchange-fr-empty-logical', + LogicalModelName: '', + MetaApplicationId: 'app-1', + MetaModelId: 'model-1', + MetaFieldId: 'field-1', + }, + ['LogicalModelName'] + ); + expect(emptyLogical.value == null || Object.keys(emptyLogical.value || {}).length === 0).toBe(true); + + const appClears = await RoleFieldRule.Onchange( + { + Id: 'onchange-fr-app-clears', + LogicalModelName: 'AppSetting', + MetaApplicationId: 'app-1', + MetaFieldId: null, + }, + ['MetaApplicationId'] + ); + expect(appClears.value).toEqual({ LogicalModelName: null }); + + const appNoLogical = await RoleFieldRule.Onchange( + { + Id: 'onchange-fr-app-no-logical', + LogicalModelName: null, + MetaApplicationId: 'app-1', + MetaFieldId: null, + }, + ['MetaApplicationId'] + ); + expect(appNoLogical.value == null || Object.keys(appNoLogical.value || {}).length === 0).toBe(true); +}); + +test('evaluateFieldRules applies LogicalModel field rules and skips mismatched logical names', async () => { + resetRequestContext(); + const origModel = (MetaModel as any).Search; + const origApp = (MetaApplication as any).Search; + const origField = (MetaField as any).Search; + const origRules = (RoleFieldRule as any).Search; + + try { + (MetaModel as any).Search = async () => [{ Id: 'model-1', ModuleId: null, UpdatedAt: '2026-08-05T12:00:00.000Z' }]; + (MetaApplication as any).Search = async () => [{ Id: 'app-1' }]; + (MetaField as any).Search = async () => [ + { Id: 'f1', Name: 'Value' }, + { Id: 'f2', Name: 'Model' }, + ]; + (RoleFieldRule as any).Search = async () => [ + { + Id: 'rule-logical-ok', + MetaModelId: null, + MetaFieldId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + PermRead: 'allow', + PermWrite: 'allow', + }, + { + Id: 'rule-logical-mismatch', + MetaModelId: null, + MetaFieldId: null, + MetaApplicationId: null, + LogicalModelName: 'TranslationTerm', + PermRead: 'deny', + PermWrite: 'deny', + }, + ]; + + const out = await evaluateFieldRules({ + appName: 'auth', + modelName: 'FieldDefault', + modelFullName: 'auth.FieldDefault', + roleIds: ['r1'], + }); + expect(out.denyReadFields).toEqual([]); + expect(out.denyWriteFields).toEqual([]); + expect(out.hitRuleIds || []).toContain('rule-logical-ok'); + expect(out.hitRuleIds || []).not.toContain('rule-logical-mismatch'); + + // Falsy modelName hits `input.modelName || ''` on modelNameWant. + (RoleFieldRule as any).Search = async () => []; + const outEmptyName = await evaluateFieldRules({ + appName: 'auth', + modelName: null as any, + modelFullName: 'auth.', + roleIds: ['r1'], + }); + expect(Array.isArray(outEmptyName.denyReadFields)).toBe(true); + } finally { + (MetaModel as any).Search = origModel; + (MetaApplication as any).Search = origApp; + (MetaField as any).Search = origField; + (RoleFieldRule as any).Search = origRules; + } +}); diff --git a/modules/auth/service/tests/logical_model_registry.test.ts b/modules/auth/service/tests/logical_model_registry.test.ts index 7fcf1eb3..a99d83cf 100644 --- a/modules/auth/service/tests/logical_model_registry.test.ts +++ b/modules/auth/service/tests/logical_model_registry.test.ts @@ -29,8 +29,15 @@ test('normalizeLogicalMethods canonicalizes PascalCase and dedupes case-insensit expect(normalizeLogicalMethods([])).toBe(null); expect(normalizeLogicalMethods(['search', 'Browse', 'SEARCH'])).toEqual(['Search', 'Browse']); expect(canonicalizeLogicalMethodName('getEffective')).toBe('GetEffective'); + expect(canonicalizeLogicalMethodName(' ')).toBe(''); + expect(canonicalizeLogicalMethodName(null)).toBe(''); + expect(canonicalizeLogicalMethodName(undefined)).toBe(''); expect(normalizeLogicalMethods('["Update"]')).toEqual(['Update']); + expect(normalizeLogicalMethods(' ')).toBe(null); + expect(normalizeLogicalMethods(['Search', ' '])).toEqual(['Search']); expect(() => normalizeLogicalMethods('not-json')).toThrow(/must be a JSON string array/); + expect(() => normalizeLogicalMethods('{}')).toThrow(/must be a JSON string array/); + expect(() => normalizeLogicalMethods(42 as any)).toThrow(/must be a string array/); expect(() => normalizeLogicalMethods([1 as any])).toThrow(/each entry must be a string/); }); @@ -39,4 +46,16 @@ test('logicalMethodsAllow treats null/empty as all methods', () => { expect(logicalMethodsAllow([], 'Search')).toBe(true); expect(logicalMethodsAllow(['Search', 'Browse'], 'update')).toBe(false); expect(logicalMethodsAllow(['Search', 'Browse'], 'SEARCH')).toBe(true); + expect(logicalMethodsAllow(null, '')).toBe(false); + expect(logicalMethodsAllow(['Search'], ' ')).toBe(false); + expect(() => logicalMethodsAllow('{}', 'Search')).toThrow(/must be a JSON string array/); +}); + +test('listLogicalModelSelection is re-exported for admin FieldsGet', async () => { + const { listLogicalModelSelection } = await import('@/auth/service/models/_logical_model_registry'); + expect(listLogicalModelSelection()).toEqual([ + { value: 'AppSetting', label: 'AppSetting' }, + { value: 'FieldDefault', label: 'FieldDefault' }, + { value: 'TranslationTerm', label: 'TranslationTerm' }, + ]); }); diff --git a/modules/auth/service/tests/method_access_eval_observability.test.ts b/modules/auth/service/tests/method_access_eval_observability.test.ts index 031acff9..0843ac5f 100644 --- a/modules/auth/service/tests/method_access_eval_observability.test.ts +++ b/modules/auth/service/tests/method_access_eval_observability.test.ts @@ -108,6 +108,73 @@ test('evaluateRoleMethodAccess returns deny allow and empty diagnostics with hit hitRuleIds: [], reason: 'method_access_no_manual_rule', }); + + // Malformed LogicalMethods: deny kept (fail closed); allow dropped. + (RoleMethodAccess as any).Search = async () => [ + { + Id: 'ma_logical_bad_deny', + Mode: 'deny', + Source: 'manual', + LogicalModelName: 'TranslationTerm', + LogicalMethods: '{not-json', + }, + ]; + expect(await evaluateRoleMethodAccess(['role_1'], [[['LogicalModelName', '=', 'TranslationTerm']] as any], 'search')).toEqual({ + denied: true, + allowed: false, + hitRuleIds: ['ma_logical_bad_deny'], + reason: 'method_access_deny', + }); + + (RoleMethodAccess as any).Search = async () => [ + { + Id: 'ma_logical_bad_allow', + Mode: 'allow', + Source: 'manual', + LogicalModelName: 'TranslationTerm', + LogicalMethods: '{not-json', + }, + ]; + expect(await evaluateRoleMethodAccess(['role_1'], [[['LogicalModelName', '=', 'TranslationTerm']] as any], 'search')).toEqual({ + denied: false, + allowed: false, + hitRuleIds: [], + reason: 'method_access_no_manual_rule', + }); + + // Malformed + empty Mode: catch path uses Mode || '' (not deny → drop). + (RoleMethodAccess as any).Search = async () => [ + { + Id: 'ma_logical_bad_empty_mode', + Mode: null, + Source: 'manual', + LogicalModelName: 'TranslationTerm', + LogicalMethods: '{not-json', + }, + ]; + expect(await evaluateRoleMethodAccess(['role_1'], [[['LogicalModelName', '=', 'TranslationTerm']] as any], 'search')).toEqual({ + denied: false, + allowed: false, + hitRuleIds: [], + reason: 'method_access_no_manual_rule', + }); + + // Logical row without methodLower is filtered out. + (RoleMethodAccess as any).Search = async () => [ + { + Id: 'ma_logical_no_method', + Mode: 'allow', + Source: 'manual', + LogicalModelName: 'TranslationTerm', + LogicalMethods: null, + }, + ]; + expect(await evaluateRoleMethodAccess(['role_1'], [[['LogicalModelName', '=', 'TranslationTerm']] as any])).toEqual({ + denied: false, + allowed: false, + hitRuleIds: [], + reason: 'method_access_no_manual_rule', + }); } finally { (RoleMethodAccess as any).Search = orig; } diff --git a/modules/auth/service/tests/permission_state_acl_source.test.ts b/modules/auth/service/tests/permission_state_acl_source.test.ts index 58107122..a33853d7 100644 --- a/modules/auth/service/tests/permission_state_acl_source.test.ts +++ b/modules/auth/service/tests/permission_state_acl_source.test.ts @@ -293,6 +293,82 @@ test('buildAclAggregation treats LogicalModelName as logical scope not global', expect(allowsAll.has('rpc:/auth.FieldDefault/*')).toBe(true); expect(allowsAll.has('rpc:/base.FieldDefault/*')).toBe(true); expect(aggAll.companyGlobalAllow.has('*')).toBe(false); + + // Malformed LogicalMethods: deny → model-wide; allow → skipped. + (RoleMethodAccess as any).Search = async () => [ + { + RoleId: 'role_1', + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + LogicalMethods: '{bad', + Mode: 'deny', + Source: 'manual', + }, + { + RoleId: 'role_1', + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'TranslationTerm', + LogicalMethods: 123, + Mode: 'allow', + Source: 'manual', + }, + { + RoleId: 'role_1', + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'NoSuchLogical', + LogicalMethods: null, + Mode: 'allow', + Source: 'manual', + }, + ]; + (MetaModel as any).Search = async () => [ + { Application: 'auth', Name: 'FieldDefault', UpdatedAt: '2026-01-02' }, + { Application: 'auth', Name: 'TranslationTerm', UpdatedAt: '2026-01-01' }, + ]; + const aggMalformed = await buildAclAggregation(['role_1'], { role_1: { global: true, companies: [] } }); + const denies = aggMalformed.requiresDenyKeysByCompany?.get('*') || new Set(); + // Fail-closed deny treats malformed methods as model-wide deny. + expect([...denies].some((k: string) => String(k).includes('FieldDefault'))).toBe(true); + const allowsMalformed = aggMalformed.requiresAllowKeysByCompany.get('*') || new Set(); + expect([...allowsMalformed].some((k: string) => String(k).includes('TranslationTerm'))).toBe(false); + + // Logical denyAll + method-restricted deny. + (RoleMethodAccess as any).Search = async () => [ + { + RoleId: 'role_1', + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + LogicalMethods: null, + Mode: 'deny', + Source: 'manual', + }, + { + RoleId: 'role_1', + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'TranslationTerm', + LogicalMethods: ['Get'], + Mode: 'deny', + Source: 'manual', + }, + ]; + (MetaModel as any).Search = async () => [ + { Application: 'auth', Name: 'FieldDefault', UpdatedAt: '2026-01-02' }, + { Application: 'auth', Name: 'TranslationTerm', UpdatedAt: '2026-01-01' }, + ]; + const aggDeny = await buildAclAggregation(['role_1'], { role_1: { global: true, companies: [] } }); + const denyKeys = aggDeny.requiresDenyKeysByCompany?.get('*') || new Set(); + expect(denyKeys.has('rpc:/auth.FieldDefault/*') || [...denyKeys].some((k: string) => k.includes('FieldDefault'))).toBe(true); + expect(denyKeys.has('rpc:/auth.TranslationTerm/Get') || [...denyKeys].some((k: string) => k.includes('TranslationTerm/Get'))).toBe(true); } finally { (RoleMethodAccess as any).Search = origAccess; (MetaService as any).Search = origService; diff --git a/modules/core/service/orm/model/logical_model_registry.test.ts b/modules/core/service/orm/model/logical_model_registry.test.ts index 33ea2143..b9a913a5 100644 --- a/modules/core/service/orm/model/logical_model_registry.test.ts +++ b/modules/core/service/orm/model/logical_model_registry.test.ts @@ -31,8 +31,12 @@ test('listLogicalModelSelection mirrors registered names for FieldsGet', () => { test('registerLogicalModelName is idempotent and ignores blanks', () => { registerLogicalModelName(' '); + registerLogicalModelName(null as any); + registerLogicalModelName(undefined as any); registerLogicalModelName('AppSetting'); expect(listLogicalModelNames()).toEqual(['AppSetting', 'FieldDefault', 'TranslationTerm']); + expect(isRegisteredLogicalModelName(null)).toBe(false); + expect(isRegisteredLogicalModelName(undefined)).toBe(false); }); test('__resetLogicalModelNamesForTest clears registry for isolation', () => { From 53ed30972b548c5e3384fcb15f606dd8192bcafb Mon Sep 17 00:00:00 2001 From: Brian Wang Date: Sat, 8 Aug 2026 09:13:53 +0800 Subject: [PATCH 05/10] fix(auth): fail closed when renaming LogicalModel without methods - Require an explicit LogicalMethods payload when LogicalModelName is updated so clearing the whitelist cannot silently widen an allow. - Restore Logical Model help-text assertions and tighten empty-modelName FieldRule deny-default expectations. Co-authored-by: Cursor --- .../auth/service/models/role_method_access.ts | 4 +-- .../tests/logical_model_acl_coverage.test.ts | 29 +++++++++++++++---- .../views/access_rules_field_binding.test.ts | 2 ++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/modules/auth/service/models/role_method_access.ts b/modules/auth/service/models/role_method_access.ts index 38de30f0..60cb87ca 100644 --- a/modules/auth/service/models/role_method_access.ts +++ b/modules/auth/service/models/role_method_access.ts @@ -189,8 +189,8 @@ export default class RoleMethodAccess extends BaseModel { } (values as any).LogicalMethods = null; } else if (mode === 'update' && !touchesMethods) { - // Logical name changed/re-set without a new whitelist → drop stale methods for the prior model. - (values as any).LogicalMethods = null; + // Fail closed: clearing whitelist to null would mean "all methods" and can widen an allow. + throw new Error('invalid RoleMethodAccess: LogicalMethods must be provided when LogicalModelName is updated'); } } // Methods-only update (no LogicalModelName in payload): normalize and persist. diff --git a/modules/auth/service/tests/logical_model_acl_coverage.test.ts b/modules/auth/service/tests/logical_model_acl_coverage.test.ts index 97fa16b6..437c153b 100644 --- a/modules/auth/service/tests/logical_model_acl_coverage.test.ts +++ b/modules/auth/service/tests/logical_model_acl_coverage.test.ts @@ -184,7 +184,22 @@ test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on n } expect(rejected).toBe(true); - // Change LogicalModelName without LogicalMethods → clear stale whitelist. + // Change LogicalModelName without LogicalMethods → reject (null whitelist would widen allow). + let rejectedRename = false; + try { + await RoleMethodAccess.UpdateById(id, { + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'AppSetting', + } as any); + } catch (e: any) { + rejectedRename = true; + expect(String(e?.message || e).includes('LogicalMethods must be provided when LogicalModelName is updated')).toBe(true); + } + expect(rejectedRename).toBe(true); + + // Rename with explicit whitelist in the same payload. await RoleMethodAccess.UpdateById( id, { @@ -192,6 +207,7 @@ test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on n MetaModelId: null, MetaApplicationId: null, LogicalModelName: 'AppSetting', + LogicalMethods: ['Get'], } as any, ['Id', 'LogicalModelName', 'LogicalMethods'] as any ); @@ -200,16 +216,15 @@ test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on n { fields: ['Id', 'LogicalModelName', 'LogicalMethods'], limit: 1 } as any ); expect(String((after[0] as any)?.LogicalModelName || '')).toBe('AppSetting'); - const clearedMethods = (after[0] as any)?.LogicalMethods; - expect(clearedMethods == null || (typeof clearedMethods === 'object' && !Array.isArray(clearedMethods) && Object.keys(clearedMethods).length === 0)).toBe(true); + expect((after[0] as any)?.LogicalMethods).toEqual(['Get']); // Methods-only update on existing logical row. - await RoleMethodAccess.UpdateById(id, { LogicalMethods: ['Get'] } as any, ['Id', 'LogicalMethods'] as any); + await RoleMethodAccess.UpdateById(id, { LogicalMethods: ['Set'] } as any, ['Id', 'LogicalMethods'] as any); const methodsOnly = await RoleMethodAccess.Search( ['Id', '=', id] as any, { fields: ['Id', 'LogicalMethods'], limit: 1 } as any ); - expect((methodsOnly[0] as any)?.LogicalMethods).toEqual(['Get']); + expect((methodsOnly[0] as any)?.LogicalMethods).toEqual(['Set']); // Private helper no-ops on nullish values. expect(() => (RoleMethodAccess as any)._normalizeLogicalMethodsPayload(null, 'update')).not.toThrow(); @@ -364,7 +379,9 @@ test('evaluateFieldRules applies LogicalModel field rules and skips mismatched l modelFullName: 'auth.', roleIds: ['r1'], }); - expect(Array.isArray(outEmptyName.denyReadFields)).toBe(true); + // Empty logical match name + no rules → deny-default for stubbed business fields. + expect(outEmptyName.denyReadFields).toEqual(['Model', 'Value']); + expect(outEmptyName.denyWriteFields).toEqual(['Model', 'Value']); } finally { (MetaModel as any).Search = origModel; (MetaApplication as any).Search = origApp; diff --git a/modules/auth/web/views/access_rules_field_binding.test.ts b/modules/auth/web/views/access_rules_field_binding.test.ts index 475b040d..956434d2 100644 --- a/modules/auth/web/views/access_rules_field_binding.test.ts +++ b/modules/auth/web/views/access_rules_field_binding.test.ts @@ -39,9 +39,11 @@ describe('Access Rules admin field binding (PR-C-5)', () => { expect(methodForm).toContain('prop="LogicalModelName"'); expect(methodForm).toContain('prop="LogicalMethods"'); expect(methodForm).toContain(':allow-array="true"'); + expect(methodForm).toContain('Logical Model (all host apps sharing that short name)'); const fieldForm = viewSource('RoleFieldRuleFormView.vue'); expect(fieldForm).toContain('prop="LogicalModelName"'); + expect(fieldForm).toContain('Logical Model (all host apps / all business fields on that short name)'); expect(viewSource('RoleMethodAccessListView.vue')).toContain('prop="LogicalModelName"'); expect(viewSource('RoleFieldRuleListView.vue')).toContain('prop="LogicalModelName"'); From 5cdc04d8fa84035dc9ca8d13589b202f856fdb4d Mon Sep 17 00:00:00 2001 From: Brian Wang Date: Sat, 8 Aug 2026 09:29:23 +0800 Subject: [PATCH 06/10] fix(auth): allow same LogicalModelName reaffirm without methods - Gate the rename fail-closed throw on an actual LogicalModelName change versus the persisted value so Mode toggles that re-echo scope still succeed. - Assert rejected renames leave stored scope/methods unchanged and methods-only updates keep LogicalModelName. Co-authored-by: Cursor --- .../auth/service/models/role_method_access.ts | 65 +++++++++++++++++-- .../tests/logical_model_acl_coverage.test.ts | 30 ++++++++- 2 files changed, 87 insertions(+), 8 deletions(-) diff --git a/modules/auth/service/models/role_method_access.ts b/modules/auth/service/models/role_method_access.ts index 60cb87ca..fa8fade4 100644 --- a/modules/auth/service/models/role_method_access.ts +++ b/modules/auth/service/models/role_method_access.ts @@ -164,8 +164,15 @@ export default class RoleMethodAccess extends BaseModel { /** * Normalize LogicalMethods; clear them unless scope is LogicalModel. * Call after assertExclusiveScope so LogicalModelName is already normalized. + * + * @param previousLogicalModelName Persisted LogicalModelName for update rename checks. + * When omitted on an update that touches LogicalModelName without LogicalMethods, treat as a rename. */ - private static _normalizeLogicalMethodsPayload(values: Record, mode: 'create' | 'update'): void { + private static _normalizeLogicalMethodsPayload( + values: Record, + mode: 'create' | 'update', + previousLogicalModelName?: string | null + ): void { if (!values) return; const touchesMethods = Object.prototype.hasOwnProperty.call(values, 'LogicalMethods'); const touchesLogicalName = Object.prototype.hasOwnProperty.call(values, 'LogicalModelName'); @@ -189,20 +196,39 @@ export default class RoleMethodAccess extends BaseModel { } (values as any).LogicalMethods = null; } else if (mode === 'update' && !touchesMethods) { - // Fail closed: clearing whitelist to null would mean "all methods" and can widen an allow. - throw new Error('invalid RoleMethodAccess: LogicalMethods must be provided when LogicalModelName is updated'); + const previous = String(previousLogicalModelName ?? '').trim(); + // Re-echoing the same logical name (e.g. Mode toggle with full scope payload) is fine. + // A real rename without an explicit whitelist must fail closed — null would mean all methods. + if (previous !== name) { + throw new Error('invalid RoleMethodAccess: LogicalMethods must be provided when LogicalModelName is updated'); + } } } // Methods-only update (no LogicalModelName in payload): normalize and persist. // Eval ignores LogicalMethods unless the persisted row is Logical scope. } - private static _prepareValues(values: Record, mode: 'create' | 'update'): void { + private static _prepareValues( + values: Record, + mode: 'create' | 'update', + previousLogicalModelName?: string | null + ): void { assertExclusiveScope(values, mode, 'method'); - RoleMethodAccess._normalizeLogicalMethodsPayload(values, mode); + RoleMethodAccess._normalizeLogicalMethodsPayload(values, mode, previousLogicalModelName); RoleMethodAccess._coerceSourceManual(values, mode); } + /** + * Whether this update payload needs the persisted LogicalModelName for rename checks. + */ + private static _needsPreviousLogicalModelName(values: Record): boolean { + if (!values) return false; + const touchesMethods = Object.prototype.hasOwnProperty.call(values, 'LogicalMethods'); + const touchesLogicalName = Object.prototype.hasOwnProperty.call(values, 'LogicalModelName'); + if (!touchesLogicalName || touchesMethods) return false; + return Boolean(String((values as any).LogicalModelName || '').trim()); + } + /** * Create one RoleMethodAccess row and invalidate request-scoped auth caches. */ @@ -246,7 +272,24 @@ export default class RoleMethodAccess extends BaseModel { returnFields?: FieldSelection, options?: any ): Promise[]> { - RoleMethodAccess._prepareValues(values as any, 'update'); + let previousLogicalModelName: string | null | undefined; + if (RoleMethodAccess._needsPreviousLogicalModelName(values as any)) { + const existing = await (this as any).Search(condition as any, { + fields: ['LogicalModelName'], + limit: 5000, + } as any); + const names = new Set( + (existing || []).map((r: any) => String(r?.LogicalModelName || '').trim()).filter(Boolean) + ); + const next = String((values as any).LogicalModelName || '').trim(); + // Mixed persisted names under one condition cannot safely reaffirm without methods. + if (names.size > 1 || (names.size === 1 && !names.has(next)) || names.size === 0) { + previousLogicalModelName = null; + } else { + previousLogicalModelName = next; + } + } + RoleMethodAccess._prepareValues(values as any, 'update', previousLogicalModelName); return mutateThenInvalidateAllAuthzCaches(async () => { const out = await super.Update(condition as any, values as any, returnFields as any, options as any); return out as unknown as Partial[]; @@ -263,7 +306,15 @@ export default class RoleMethodAccess extends BaseModel { returnFields?: FieldSelection, options?: any ): Promise> { - RoleMethodAccess._prepareValues(values as any, 'update'); + let previousLogicalModelName: string | null | undefined; + if (RoleMethodAccess._needsPreviousLogicalModelName(values as any)) { + const existing = await (this as any).Search(['Id', '=', id] as any, { + fields: ['LogicalModelName'], + limit: 1, + } as any); + previousLogicalModelName = String((existing?.[0] as any)?.LogicalModelName || '').trim() || null; + } + RoleMethodAccess._prepareValues(values as any, 'update', previousLogicalModelName); return mutateThenInvalidateAllAuthzCaches(async () => { const out = await super.UpdateById(id as any, values as any, returnFields as any, options as any); return out as unknown as Partial; diff --git a/modules/auth/service/tests/logical_model_acl_coverage.test.ts b/modules/auth/service/tests/logical_model_acl_coverage.test.ts index 437c153b..a2959af6 100644 --- a/modules/auth/service/tests/logical_model_acl_coverage.test.ts +++ b/modules/auth/service/tests/logical_model_acl_coverage.test.ts @@ -198,6 +198,32 @@ test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on n expect(String(e?.message || e).includes('LogicalMethods must be provided when LogicalModelName is updated')).toBe(true); } expect(rejectedRename).toBe(true); + const afterReject = await RoleMethodAccess.Search( + ['Id', '=', id] as any, + { fields: ['Id', 'LogicalModelName', 'LogicalMethods'], limit: 1 } as any + ); + expect(String((afterReject[0] as any)?.LogicalModelName || '')).toBe('FieldDefault'); + expect((afterReject[0] as any)?.LogicalMethods).toEqual(['Get', 'Set']); + + // Re-echo same LogicalModelName without methods (e.g. Mode toggle) is allowed. + await RoleMethodAccess.UpdateById( + id, + { + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + Mode: 'deny', + } as any, + ['Id', 'LogicalModelName', 'LogicalMethods', 'Mode'] as any + ); + const afterEcho = await RoleMethodAccess.Search( + ['Id', '=', id] as any, + { fields: ['Id', 'LogicalModelName', 'LogicalMethods', 'Mode'], limit: 1 } as any + ); + expect(String((afterEcho[0] as any)?.LogicalModelName || '')).toBe('FieldDefault'); + expect((afterEcho[0] as any)?.LogicalMethods).toEqual(['Get', 'Set']); + expect(String((afterEcho[0] as any)?.Mode || '')).toBe('deny'); // Rename with explicit whitelist in the same payload. await RoleMethodAccess.UpdateById( @@ -208,6 +234,7 @@ test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on n MetaApplicationId: null, LogicalModelName: 'AppSetting', LogicalMethods: ['Get'], + Mode: 'allow', } as any, ['Id', 'LogicalModelName', 'LogicalMethods'] as any ); @@ -222,8 +249,9 @@ test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on n await RoleMethodAccess.UpdateById(id, { LogicalMethods: ['Set'] } as any, ['Id', 'LogicalMethods'] as any); const methodsOnly = await RoleMethodAccess.Search( ['Id', '=', id] as any, - { fields: ['Id', 'LogicalMethods'], limit: 1 } as any + { fields: ['Id', 'LogicalModelName', 'LogicalMethods'], limit: 1 } as any ); + expect(String((methodsOnly[0] as any)?.LogicalModelName || '')).toBe('AppSetting'); expect((methodsOnly[0] as any)?.LogicalMethods).toEqual(['Set']); // Private helper no-ops on nullish values. From 1e4a1451d041f18f84cd8cbf7c1ff9c00d0c9cb3 Mon Sep 17 00:00:00 2001 From: Brian Wang Date: Sat, 8 Aug 2026 13:29:31 +0800 Subject: [PATCH 07/10] fix(auth): harden bulk LogicalModelName reaffirm checks - Prove every matched row already has the target LogicalModelName via Count equality instead of a capped Search sample. - Fail closed when any matched row is non-logical or has a different name so null whitelist cannot widen allow. Co-authored-by: Cursor --- .../auth/service/models/role_method_access.ts | 23 ++--- .../tests/logical_model_acl_coverage.test.ts | 91 +++++++++++++++++++ 2 files changed, 103 insertions(+), 11 deletions(-) diff --git a/modules/auth/service/models/role_method_access.ts b/modules/auth/service/models/role_method_access.ts index fa8fade4..70e7cb7f 100644 --- a/modules/auth/service/models/role_method_access.ts +++ b/modules/auth/service/models/role_method_access.ts @@ -274,19 +274,20 @@ export default class RoleMethodAccess extends BaseModel { ): Promise[]> { let previousLogicalModelName: string | null | undefined; if (RoleMethodAccess._needsPreviousLogicalModelName(values as any)) { - const existing = await (this as any).Search(condition as any, { - fields: ['LogicalModelName'], - limit: 5000, - } as any); - const names = new Set( - (existing || []).map((r: any) => String(r?.LogicalModelName || '').trim()).filter(Boolean) - ); const next = String((values as any).LogicalModelName || '').trim(); - // Mixed persisted names under one condition cannot safely reaffirm without methods. - if (names.size > 1 || (names.size === 1 && !names.has(next)) || names.size === 0) { - previousLogicalModelName = null; + // Prove every matched row already has LogicalModelName === next (no sampling). + // Null/empty/other names fail Count equality → fail closed (null whitelist = all methods). + const matched = Number(await (this as any).Count(condition as any)) || 0; + if (matched > 0) { + const alreadyAtNext = + Number( + await (this as any).Count({ + And: [condition as any, ['LogicalModelName', '=', next] as any], + } as any) + ) || 0; + previousLogicalModelName = alreadyAtNext === matched ? next : null; } else { - previousLogicalModelName = next; + previousLogicalModelName = null; } } RoleMethodAccess._prepareValues(values as any, 'update', previousLogicalModelName); diff --git a/modules/auth/service/tests/logical_model_acl_coverage.test.ts b/modules/auth/service/tests/logical_model_acl_coverage.test.ts index a2959af6..343a0381 100644 --- a/modules/auth/service/tests/logical_model_acl_coverage.test.ts +++ b/modules/auth/service/tests/logical_model_acl_coverage.test.ts @@ -257,6 +257,97 @@ test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on n // Private helper no-ops on nullish values. expect(() => (RoleMethodAccess as any)._normalizeLogicalMethodsPayload(null, 'update')).not.toThrow(); expect(() => (RoleMethodAccess as any)._normalizeLogicalMethodsPayload(undefined, 'create')).not.toThrow(); + + // Bulk Update: mixed logical + non-logical rows must fail closed without LogicalMethods. + const logical2 = await RoleMethodAccess.Create( + { + RoleId: { Id: role.id } as any, + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + LogicalMethods: ['Get'], + Mode: 'allow', + } as any, + ['Id'] as any + ); + const logical2Id = String((logical2 as any)?.Id || '').trim(); + const serviceRow = await RoleMethodAccess.Create( + { + RoleId: { Id: role.id } as any, + MetaServiceId: browse.id, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: null, + Mode: 'allow', + } as any, + ['Id'] as any + ); + const serviceId = String((serviceRow as any)?.Id || '').trim(); + let rejectedMixed = false; + try { + await RoleMethodAccess.Update( + { Or: [['Id', '=', logical2Id], ['Id', '=', serviceId]] } as any, + { + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + Mode: 'deny', + } as any + ); + } catch (e: any) { + rejectedMixed = true; + expect(String(e?.message || e).includes('LogicalMethods must be provided when LogicalModelName is updated')).toBe(true); + } + expect(rejectedMixed).toBe(true); + const mixedAfter = await RoleMethodAccess.Search( + { Or: [['Id', '=', logical2Id], ['Id', '=', serviceId]] } as any, + { fields: ['Id', 'LogicalModelName', 'MetaServiceId', 'LogicalMethods'], limit: 10 } as any + ); + const byId = new Map((mixedAfter || []).map((r: any) => [String(r?.Id || ''), r])); + expect(String((byId.get(logical2Id) as any)?.LogicalModelName || '')).toBe('FieldDefault'); + expect((byId.get(logical2Id) as any)?.LogicalMethods).toEqual(['Get']); + expect(String((byId.get(serviceId) as any)?.LogicalModelName || '')).toBe(''); + expect(String((byId.get(serviceId) as any)?.MetaServiceId || '')).toBe(browse.id); + + // Bulk Update: same logical name reaffirm across rows is allowed. + const logical3 = await RoleMethodAccess.Create( + { + RoleId: { Id: role.id } as any, + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + LogicalMethods: ['Set'], + Mode: 'allow', + } as any, + ['Id'] as any + ); + const logical3Id = String((logical3 as any)?.Id || '').trim(); + await RoleMethodAccess.Update( + { Or: [['Id', '=', logical2Id], ['Id', '=', logical3Id]] } as any, + { + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + Mode: 'deny', + } as any, + ['Id', 'LogicalModelName', 'LogicalMethods', 'Mode'] as any + ); + const sameNameAfter = await RoleMethodAccess.Search( + { Or: [['Id', '=', logical2Id], ['Id', '=', logical3Id]] } as any, + { fields: ['Id', 'LogicalModelName', 'LogicalMethods', 'Mode'], limit: 10 } as any + ); + expect((sameNameAfter || []).length).toBe(2); + const sameById = new Map((sameNameAfter || []).map((r: any) => [String(r?.Id || ''), r])); + expect(String((sameById.get(logical2Id) as any)?.LogicalModelName || '')).toBe('FieldDefault'); + expect(String((sameById.get(logical3Id) as any)?.LogicalModelName || '')).toBe('FieldDefault'); + expect(String((sameById.get(logical2Id) as any)?.Mode || '')).toBe('deny'); + expect(String((sameById.get(logical3Id) as any)?.Mode || '')).toBe('deny'); + expect((sameById.get(logical2Id) as any)?.LogicalMethods).toEqual(['Get']); + expect((sameById.get(logical3Id) as any)?.LogicalMethods).toEqual(['Set']); }); }); From 8ec8177929a097e88138797da45072dcab71db34 Mon Sep 17 00:00:00 2001 From: Brian Wang Date: Sat, 8 Aug 2026 13:36:51 +0800 Subject: [PATCH 08/10] test(auth): assert Mode unchanged after rejected bulk logical update - Verify both mixed-scope rows keep Mode=allow when a no-methods LogicalModelName bulk update is rejected. Co-authored-by: Cursor --- modules/auth/service/tests/logical_model_acl_coverage.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/auth/service/tests/logical_model_acl_coverage.test.ts b/modules/auth/service/tests/logical_model_acl_coverage.test.ts index 343a0381..3ff8745f 100644 --- a/modules/auth/service/tests/logical_model_acl_coverage.test.ts +++ b/modules/auth/service/tests/logical_model_acl_coverage.test.ts @@ -303,13 +303,15 @@ test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on n expect(rejectedMixed).toBe(true); const mixedAfter = await RoleMethodAccess.Search( { Or: [['Id', '=', logical2Id], ['Id', '=', serviceId]] } as any, - { fields: ['Id', 'LogicalModelName', 'MetaServiceId', 'LogicalMethods'], limit: 10 } as any + { fields: ['Id', 'LogicalModelName', 'MetaServiceId', 'LogicalMethods', 'Mode'], limit: 10 } as any ); const byId = new Map((mixedAfter || []).map((r: any) => [String(r?.Id || ''), r])); expect(String((byId.get(logical2Id) as any)?.LogicalModelName || '')).toBe('FieldDefault'); expect((byId.get(logical2Id) as any)?.LogicalMethods).toEqual(['Get']); + expect(String((byId.get(logical2Id) as any)?.Mode || '')).toBe('allow'); expect(String((byId.get(serviceId) as any)?.LogicalModelName || '')).toBe(''); expect(String((byId.get(serviceId) as any)?.MetaServiceId || '')).toBe(browse.id); + expect(String((byId.get(serviceId) as any)?.Mode || '')).toBe('allow'); // Bulk Update: same logical name reaffirm across rows is allowed. const logical3 = await RoleMethodAccess.Create( From 27d3cfb89304ab7f5c33c984aa25f086dcc0e838 Mon Sep 17 00:00:00 2001 From: Brian Wang Date: Sat, 8 Aug 2026 13:51:03 +0800 Subject: [PATCH 09/10] fix(auth): align bulk LogicalModel reaffirm Count with Update options - Pass Update options into both Count proofs so withDeleted/onlyDeleted match the write set. - Narrow the update condition to LogicalModelName === next after a successful reaffirm proof so concurrent scope races are skipped instead of silently renamed. Co-authored-by: Cursor --- .../auth/service/models/role_method_access.ts | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/modules/auth/service/models/role_method_access.ts b/modules/auth/service/models/role_method_access.ts index 70e7cb7f..6f1ac49b 100644 --- a/modules/auth/service/models/role_method_access.ts +++ b/modules/auth/service/models/role_method_access.ts @@ -273,26 +273,39 @@ export default class RoleMethodAccess extends BaseModel { options?: any ): Promise[]> { let previousLogicalModelName: string | null | undefined; + let updateCondition: QueryCondition = condition; if (RoleMethodAccess._needsPreviousLogicalModelName(values as any)) { const next = String((values as any).LogicalModelName || '').trim(); // Prove every matched row already has LogicalModelName === next (no sampling). // Null/empty/other names fail Count equality → fail closed (null whitelist = all methods). - const matched = Number(await (this as any).Count(condition as any)) || 0; + // Pass the same options as super.Update so withDeleted/onlyDeleted stay aligned. + const matched = Number(await (this as any).Count(condition as any, options as any)) || 0; if (matched > 0) { const alreadyAtNext = Number( - await (this as any).Count({ - And: [condition as any, ['LogicalModelName', '=', next] as any], - } as any) + await (this as any).Count( + { + And: [condition as any, ['LogicalModelName', '=', next] as any], + } as any, + options as any + ) ) || 0; - previousLogicalModelName = alreadyAtNext === matched ? next : null; + if (alreadyAtNext === matched) { + previousLogicalModelName = next; + // Couple the write to the proof: rows that race away from `next` are skipped, not renamed. + updateCondition = { + And: [condition as any, ['LogicalModelName', '=', next] as any], + } as any; + } else { + previousLogicalModelName = null; + } } else { previousLogicalModelName = null; } } RoleMethodAccess._prepareValues(values as any, 'update', previousLogicalModelName); return mutateThenInvalidateAllAuthzCaches(async () => { - const out = await super.Update(condition as any, values as any, returnFields as any, options as any); + const out = await super.Update(updateCondition as any, values as any, returnFields as any, options as any); return out as unknown as Partial[]; }); } From 75936a8df9ba5b246f5ff6b4d9b4da4e487f4fc9 Mon Sep 17 00:00:00 2001 From: Brian Wang Date: Sat, 8 Aug 2026 14:28:13 +0800 Subject: [PATCH 10/10] test(auth): bring RoleMethodAccess LogicalModel patch coverage to 100% - Cover nullish _needsPreviousLogicalModelName, Count=0 bulk updates, non-logical bulk renames, and missing UpdateById rows. - Let Create omit Source so the Field default factory runs; still coerce Source=ui to manual when present. Co-authored-by: Cursor --- .../auth/service/models/role_method_access.ts | 10 +- .../tests/logical_model_acl_coverage.test.ts | 92 +++++++++++++++++++ 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/modules/auth/service/models/role_method_access.ts b/modules/auth/service/models/role_method_access.ts index 6f1ac49b..2c476e4d 100644 --- a/modules/auth/service/models/role_method_access.ts +++ b/modules/auth/service/models/role_method_access.ts @@ -154,10 +154,11 @@ export default class RoleMethodAccess extends BaseModel { /** * UI-Option-A: never persist Source=ui (runtime ui-derived ACL replaces materialization). */ - private static _coerceSourceManual(values: Record, mode: 'create' | 'update'): void { + private static _coerceSourceManual(values: Record, _mode: 'create' | 'update'): void { if (!values) return; - const touchesSource = Object.prototype.hasOwnProperty.call(values, 'Source'); - if (!touchesSource && mode !== 'create') return; + // Only coerce when Source is present in the payload (ui → manual). Create without Source + // keeps the Field default factory (`manual`) so defaults stay reachable. + if (!Object.prototype.hasOwnProperty.call(values, 'Source')) return; (values as any).Source = 'manual'; } @@ -275,7 +276,8 @@ export default class RoleMethodAccess extends BaseModel { let previousLogicalModelName: string | null | undefined; let updateCondition: QueryCondition = condition; if (RoleMethodAccess._needsPreviousLogicalModelName(values as any)) { - const next = String((values as any).LogicalModelName || '').trim(); + // Guard already proved LogicalModelName is a non-empty string after trim. + const next = String((values as any).LogicalModelName).trim(); // Prove every matched row already has LogicalModelName === next (no sampling). // Null/empty/other names fail Count equality → fail closed (null whitelist = all methods). // Pass the same options as super.Update so withDeleted/onlyDeleted stay aligned. diff --git a/modules/auth/service/tests/logical_model_acl_coverage.test.ts b/modules/auth/service/tests/logical_model_acl_coverage.test.ts index 3ff8745f..4ef0c4dc 100644 --- a/modules/auth/service/tests/logical_model_acl_coverage.test.ts +++ b/modules/auth/service/tests/logical_model_acl_coverage.test.ts @@ -257,6 +257,17 @@ test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on n // Private helper no-ops on nullish values. expect(() => (RoleMethodAccess as any)._normalizeLogicalMethodsPayload(null, 'update')).not.toThrow(); expect(() => (RoleMethodAccess as any)._normalizeLogicalMethodsPayload(undefined, 'create')).not.toThrow(); + expect((RoleMethodAccess as any)._needsPreviousLogicalModelName(null)).toBe(false); + expect((RoleMethodAccess as any)._needsPreviousLogicalModelName(undefined)).toBe(false); + expect((RoleMethodAccess as any)._needsPreviousLogicalModelName({ LogicalModelName: null })).toBe(false); + expect((RoleMethodAccess as any)._needsPreviousLogicalModelName({ LogicalModelName: ' ' })).toBe(false); + expect( + (RoleMethodAccess as any)._needsPreviousLogicalModelName({ + LogicalModelName: 'FieldDefault', + LogicalMethods: ['Get'], + }) + ).toBe(false); + expect((RoleMethodAccess as any)._needsPreviousLogicalModelName({ LogicalModelName: 'FieldDefault' })).toBe(true); // Bulk Update: mixed logical + non-logical rows must fail closed without LogicalMethods. const logical2 = await RoleMethodAccess.Create( @@ -350,6 +361,87 @@ test('RoleMethodAccess: LogicalMethods normalize, reject non-logical, clear on n expect(String((sameById.get(logical3Id) as any)?.Mode || '')).toBe('deny'); expect((sameById.get(logical2Id) as any)?.LogicalMethods).toEqual(['Get']); expect((sameById.get(logical3Id) as any)?.LogicalMethods).toEqual(['Set']); + + // Bulk Update: Count=0 (no matched rows) still fail-closes without LogicalMethods. + let rejectedEmpty = false; + try { + await RoleMethodAccess.Update( + ['Id', '=', `missing_${uid('RMA')}`] as any, + { + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + Mode: 'allow', + } as any + ); + } catch (e: any) { + rejectedEmpty = true; + expect(String(e?.message || e).includes('LogicalMethods must be provided when LogicalModelName is updated')).toBe(true); + } + expect(rejectedEmpty).toBe(true); + + // Bulk Update: matched>0 but none already at next (non-logical rows only). + const service2 = await RoleMethodAccess.Create( + { + RoleId: { Id: role.id } as any, + MetaServiceId: browse.id, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: null, + Mode: 'allow', + } as any, + ['Id'] as any + ); + const service2Id = String((service2 as any)?.Id || '').trim(); + let rejectedNonLogical = false; + try { + await RoleMethodAccess.Update( + { Or: [['Id', '=', serviceId], ['Id', '=', service2Id]] } as any, + { + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'AppSetting', + Mode: 'deny', + } as any + ); + } catch (e: any) { + rejectedNonLogical = true; + expect(String(e?.message || e).includes('LogicalMethods must be provided when LogicalModelName is updated')).toBe(true); + } + expect(rejectedNonLogical).toBe(true); + + // UpdateById on missing Id: empty Search → previous null → same fail-closed rename guard. + let rejectedMissingId = false; + try { + await RoleMethodAccess.UpdateById(`missing_${uid('RMA')}`, { + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'FieldDefault', + } as any); + } catch (e: any) { + rejectedMissingId = true; + expect(String(e?.message || e).includes('LogicalMethods must be provided when LogicalModelName is updated')).toBe(true); + } + expect(rejectedMissingId).toBe(true); + + // Omit Mode/Source so Field default factories run (deny / manual). + const defaultsRow = await RoleMethodAccess.Create( + { + RoleId: { Id: role.id } as any, + MetaServiceId: null, + MetaModelId: null, + MetaApplicationId: null, + LogicalModelName: 'TranslationTerm', + LogicalMethods: ['Get'], + } as any, + ['Id', 'Mode', 'Source', 'LogicalModelName'] as any + ); + expect(String((defaultsRow as any)?.Mode || '')).toBe('deny'); + expect(String((defaultsRow as any)?.Source || '')).toBe('manual'); + expect(String((defaultsRow as any)?.LogicalModelName || '')).toBe('TranslationTerm'); }); });