diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 3b8136797..a3a251cbd 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -316,6 +316,7 @@ cases for these three BSON types — they fell to `default: return nil`. | `mdl/executor/cmd_microflows_format_action.go` | Added 3 formatter cases | | `mdl/executor/cmd_microflows_format_listop_test.go` | Added 4 formatter tests | | `sdk/mpr/parser_listoperation_test.go` | New file, 4 parser tests | +| `UPDATE SECURITY`, `CREATE ASSOCIATION` or any `GRANT` silently strips **inherited** members from a specialized entity's access rules; `GRANT` naming an inherited member reports success and persists nothing, so REVOKE+GRANT cannot repair it. `mx check` shows only CE0066, hiding the CE2729 "No read access to attribute" cascade until Studio Pro's Update security is clicked | Mendix inheritance is multi-table: all of a parent's attributes are members of the child, so an access rule needs a MemberAccess entry for every member, own **and** inherited, each qualified against the entity that **declares** it. Both the GRANT builder and `ReconcileMemberAccesses` enumerated only `entity.Attributes`, so an inherited reference matched nothing and was deleted as stale — and reconciliation runs **immediately after every GRANT**, deleting what the grant had just written correctly | `mdl/executor/entity_hierarchy.go` (`EntityMembers`), `mdl/executor/cmd_security_write.go` (`execGrantEntityAccess`), `mdl/backend/modelsdk/domainmodel_security_write.go` (`ReconcileMemberAccesses`, `attrRefBelongsTo`), `sdk/mpr/writer_security.go` (legacy engine) | Walk the generalization chain and qualify each member against its declaring entity; in the reconciler, only strip a reference qualified to **this** entity — an ancestor may live in another module or System, neither loaded there, so preserve what cannot be validated. **Two facts must be established against `mx check`, never inferred**: (a) the child-qualified form is CE1613 "attribute no longer exists" while the declaring-entity form validates clean; (b) `System.User`'s members are the exception — entities specialising it are *user entities* whose platform members Mendix manages, and listing them turns a clean rule into CE0066, while omitting `System.FileDocument`'s six members is CE0066 until all are present. **Generalisable**: when a post-write reconcile pass validates against a narrower model than the writer used, it will quietly undo correct writes — check what runs *after* a write before concluding the writer is at fault. Repro `mdl-examples/bug-tests/758-inherited-member-access.mdl`. Issues #758, #765 (umbrella; #451 is the same declaring-entity rule in the change-object writer) | | `describe` (and `context` / `diff-local`) renders a Retrieve's XPath with only its **first** predicate group — `where A/B[EndDate = $X];` when the BSON holds `[A/B[EndDate = $X]][Status != 'Completed'][CompletionDate = empty]`. No warning; the output reads as a complete but materially *less restrictive* query, so correct defensive code looks buggy | The grammar's `xpathConstraint` rule matches ONE bracket group, and Mendix concatenates siblings. `ParseXPathConstraint` removes the error listeners, so ANTLR parsed group 1, left the rest on the token stream, and **still returned ok=true**; `enrichXPathConstraintForDescribe` treated that as a full parse and re-rendered only what came back. The `if !ok { return original }` fallback never fired | `mdl/visitor/visitor_xpath_public.go` (`ParseXPathConstraint`), `mdl/visitor/xpath_groups.go` (`SplitXPathPredicateGroups`), `mdl/executor/cmd_microflows_format_action.go` (`enrichXPathGroups`, and the render-path split) | Two layers. (1) Reject a partial parse — after the rule, require `stream.LA(1) == antlr.TokenEOF`; that alone stops the loss, since the caller then falls back to the stored string. (2) Split into top-level groups and enrich each, so enrichment still reaches groups after the first. The splitter must track **nesting depth and quoting**: a naive `][` split mangles a nested `[A/B[x = 1]]` and a literal containing `]`. **Generalisable**: a parser that silently accepts a prefix is worse than one that fails — any `ok` returned by a rule that can match less than its input must be checked against EOF before callers treat it as lossless. Repro `mdl-examples/bug-tests/772-xpath-constraint-groups.mdl`; A/B against a pre-fix binary on the same project shows the two dropped groups. Issue #772 | **Key insight:** `microflows$ListRange` stores offset/limit inside a nested diff --git a/.claude/skills/mendix/generate-domain-model.md b/.claude/skills/mendix/generate-domain-model.md index 1e62b7acd..633d04961 100644 --- a/.claude/skills/mendix/generate-domain-model.md +++ b/.claude/skills/mendix/generate-domain-model.md @@ -210,6 +210,15 @@ create persistent entity Module.Photo ( **Note:** `mxcli syntax entity` output may show EXTENDS after `)` — this is misleading. Always place EXTENDS before `(`. +**Security follows inheritance.** Mendix inheritance is multi-table: all of the +parent's attributes are members of the child, so a specialized entity's access rule +must cover them. Grant an inherited member exactly like one of the entity's own — +`grant Module.Viewer on Module.Attachment (read (AttachmentDescription, "Name", Size));` +— and `read *` / `write *` cover them too. Skipping them is Mendix CE0066 "Entity +access is out of date". The one exception is entities extending `System.User`, whose +inherited platform members Mendix manages and which must not be granted. See +`manage-security.md`. + #### System Attributes (Auditing) Mendix supports four built-in auditing properties on persistent entities. Declare them as regular attributes using pseudo-types (like `autonumber`): diff --git a/.claude/skills/mendix/manage-security.md b/.claude/skills/mendix/manage-security.md index ac6115c22..425a72e4b 100644 --- a/.claude/skills/mendix/manage-security.md +++ b/.claude/skills/mendix/manage-security.md @@ -167,6 +167,58 @@ revoke MyModule.User on MyModule.Customer (write (Email)); revoke MyModule.User on MyModule.Customer (delete); ``` +#### Inherited members + +Mendix inheritance is multi-table: a child adds attributes to its parent's, and +**all** the parent's members belong to the child. Grant them exactly like the +entity's own — `read *` / `write *` cover them too: + +```sql +create persistent entity Docs.DocumentBase ( + DocName: String(200), + Confidential: Boolean +); + +create persistent entity Docs.Contract extends Docs.DocumentBase ( + ContractNumber: String(50) +); + +-- DocName is inherited, ContractNumber is Contract's own — name both the same way +grant Docs.Viewer on Docs.Contract (read (DocName, ContractNumber)); + +-- Attachment inherits the file members from System.FileDocument +create persistent entity Docs.Attachment extends System.FileDocument ( + Category: String(50) +); +grant Docs.Viewer on Docs.Attachment (read (Category, "Name", Size)); +``` + +An access rule must carry an entry for **every** member, own and inherited — +mxcli writes the ones you did not grant with rights `None`. Omitting them is +Mendix **CE0066** "Entity access is out of date", which masks the CE2729 +"No read access to attribute" errors underneath until Studio Pro's *Update +security* is clicked. + +A member name that matches nothing is now an error rather than a silent skip: + +``` +Error: entity Docs.Contract has no member(s) DocNam; grant only names members +of the entity or of an entity it inherits from +``` + +**Exception — user entities.** An entity extending `System.User` is a *user +entity*, and Mendix manages its inherited platform members (`Name`, `Password`, +`Blocked`, …). Those must **not** appear in the rule; listing them is CE0066. +Grant only the entity's own members — mxcli leaves the platform ones out +automatically: + +```sql +create persistent entity Docs.Employee extends System.User ( + EmployeeNo: String(20) +); +grant Docs.Viewer on Docs.Employee (read (EmployeeNo)); -- not Name/Blocked +``` + ### User Roles ```sql diff --git a/cmd/mxcli/syntax/features_security.go b/cmd/mxcli/syntax/features_security.go index 0312bcf0d..9f6eba6f5 100644 --- a/cmd/mxcli/syntax/features_security.go +++ b/cmd/mxcli/syntax/features_security.go @@ -33,8 +33,24 @@ func init() { "entity access", "grant", "revoke", "read", "write", "create", "delete", "xpath", "row-level security", }, - Syntax: "GRANT ON . () [WHERE ''];\nREVOKE ON .;\nREVOKE ON . ();\n\nRights: CREATE, DELETE, READ *, READ (,...), WRITE *, WRITE (,...)", - Example: "GRANT Shop.Admin ON Shop.Customer (CREATE, DELETE, READ *, WRITE *);\nGRANT Shop.User ON Shop.Customer (READ *) WHERE '[Active = true()]';", + Syntax: "GRANT ON . () [WHERE ''];\n" + + "REVOKE ON .;\n" + + "REVOKE ON . ();\n\n" + + "Rights: CREATE, DELETE, READ *, READ (,...), WRITE *, WRITE (,...)\n\n" + + "Inherited members:\n" + + " Mendix inheritance is multi-table — a child adds attributes to its\n" + + " parent's, and ALL the parent's members belong to the child. Name them\n" + + " in a GRANT exactly like the entity's own; READ */WRITE * covers them\n" + + " too. A name that matches no member is an error, not a silent skip.\n\n" + + " Exception: entities extending System.User are user entities, whose\n" + + " platform members (Name, Password, Blocked, ...) Mendix manages. Do not\n" + + " grant those; mxcli leaves them out of the rule automatically.", + Example: "GRANT Shop.Admin ON Shop.Customer (CREATE, DELETE, READ *, WRITE *);\n" + + "GRANT Shop.User ON Shop.Customer (READ *) WHERE '[Active = true()]';\n\n" + + "-- Contract extends DocumentBase: DocName is inherited, ContractNumber is own\n" + + "GRANT Docs.Viewer ON Docs.Contract (READ (DocName, ContractNumber));\n\n" + + "-- Attachment extends System.FileDocument: Name and Size are inherited\n" + + "GRANT Docs.Viewer ON Docs.Attachment (READ (Category, \"Name\", Size));", SeeAlso: []string{"security.module-role", "security.microflow-access"}, }) diff --git a/docs-site/src/reference/security/grant.md b/docs-site/src/reference/security/grant.md index 7fe165ad1..1f3e0bb0f 100644 --- a/docs-site/src/reference/security/grant.md +++ b/docs-site/src/reference/security/grant.md @@ -125,6 +125,45 @@ GRANT Shop.Viewer ON Shop.Customer (READ (Phone)); -- Result: READ (Name, Email, Phone) ``` +## Inherited members + +Mendix inheritance is multi-table: a child adds attributes to its parent's, and +**all** the parent's members belong to the child. Name an inherited member in a +GRANT exactly like one of the entity's own; `READ *` and `WRITE *` cover them too. + +```sql +CREATE PERSISTENT ENTITY Docs.DocumentBase (DocName: String(200)); +CREATE PERSISTENT ENTITY Docs.Contract EXTENDS Docs.DocumentBase (ContractNumber: String(50)); + +-- DocName inherited, ContractNumber own — no distinction at the call site +GRANT Docs.Viewer ON Docs.Contract (READ (DocName, ContractNumber)); +``` + +An access rule must carry an entry for **every** member, own and inherited. mxcli +writes the members you did not grant with rights `None`; omitting them entirely is +Mendix **CE0066** *"Entity access is out of date"*, which masks the CE2729 +*"No read access to attribute"* errors beneath it until Studio Pro's +**Update security** is clicked. + +A member name that matches nothing is rejected rather than skipped: + +``` +Error: entity Docs.Contract has no member(s) DocNam; grant only names members +of the entity or of an entity it inherits from +``` + +### User entities are the exception + +An entity extending `System.User` is a *user entity*, and Mendix manages its +inherited platform members (`Name`, `Password`, `Blocked`, …). Those must **not** +appear in the access rule — listing them is CE0066. Grant only the entity's own +members; mxcli excludes the platform ones automatically. + +```sql +CREATE PERSISTENT ENTITY Docs.Employee EXTENDS System.User (EmployeeNo: String(20)); +GRANT Docs.Viewer ON Docs.Employee (READ (EmployeeNo)); +``` + ## See Also [REVOKE](revoke.md), [CREATE MODULE ROLE](create-module-role.md), [CREATE USER ROLE](create-user-role.md) diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 0804ea41a..cc2925ceb 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -366,7 +366,7 @@ Nested folders use `/` separator: `'Parent/Child/Grandchild'`. Missing folders a | Revoke nanoflow access | `revoke execute on nanoflow Mod.NF from Mod.Role, ...;` | | | Grant page access | `grant view on page Mod.Page to Mod.Role, ...;` | | | Revoke page access | `revoke view on page Mod.Page from Mod.Role, ...;` | | -| Grant entity access | `grant Mod.Role on Mod.Entity (create, delete, read *, write *);` | Additive — merges with existing | +| Grant entity access | `grant Mod.Role on Mod.Entity (create, delete, read *, write *);` | Additive — merges with existing. Inherited members are named like the entity's own (`read *` covers them); an unknown name is an error. Entities extending `System.User` are the exception — their platform members must not be granted | | Revoke entity access | `revoke Mod.Role on Mod.Entity;` | Full revoke — removes entire rule | | Revoke entity access (partial) | `revoke Mod.Role on Mod.Entity (read (attr));` | Partial — downgrades specific rights | | Set security level | `alter project security level off\|prototype\|production;` | | diff --git a/mdl-examples/bug-tests/758-inherited-member-access.mdl b/mdl-examples/bug-tests/758-inherited-member-access.mdl new file mode 100644 index 000000000..b958b02f3 --- /dev/null +++ b/mdl-examples/bug-tests/758-inherited-member-access.mdl @@ -0,0 +1,70 @@ +-- Bug #758 / #765: security reconciliation stripped inherited members, and GRANT +-- could not put them back +-- +-- Mendix models inheritance across multiple tables: a child adds attributes to the +-- parent's, and ALL the parent's attributes are available on the child. An access +-- rule must therefore carry a MemberAccess entry for every member — own AND +-- inherited — or Mendix reports CE0066 "Entity access is out of date". +-- +-- Two facts established against `mx check`, not inferred: +-- +-- 1. An inherited member's reference is qualified against the entity that +-- DECLARES it. `Sec758.Base.SharedField` validates clean; the child-qualified +-- `Sec758.Item.SharedField` is CE1613 "The selected attribute no longer +-- exists". mxcli wrote the child form. +-- +-- 2. Members inherited from System.User are the exception: entities specialising +-- it are user entities whose platform members Mendix manages. Listing them +-- turns a clean rule into CE0066 — confirmed on Mendix's own +-- Administration.Account and on a fresh specialisation. Every other ancestor's +-- members are required: omitting the six System.FileDocument members from a +-- specialising entity's rule is CE0066 until all are present. +-- +-- What went wrong. Both the GRANT path and reconciliation enumerated only +-- entity.Attributes: +-- +-- * GRANT naming an inherited member produced no entry, and reported success. +-- * ReconcileMemberAccesses — which runs immediately after every GRANT, and on +-- any write touching the module — saw the inherited entry's ancestor +-- qualification, failed to match it against the child's own attributes, and +-- deleted it as stale. So the grant wrote the right thing and the reconcile in +-- the same command removed it. That is why REVOKE + GRANT never repaired a +-- damaged rule. +-- +-- The damage was masked: `mx check` reports CE0066 and stops, hiding the CE2729 +-- "No read access to attribute" cascade underneath until Studio Pro's +-- "Update security" is clicked. +-- +-- Verify: +-- 1. mxcli exec 758-inherited-member-access.mdl -p app.mpr +-- 2. mxcli -p app.mpr -c "describe entity Sec758.Item" +-- -> grant ... (read (OwnField, SharedField)); both members present +-- 3. mxcli docker check -p app.mpr -> 0 errors (no CE0066) +-- 4. Repeat step 3 after any further ALTER on the module: the inherited entry +-- must still be there. + +create module Sec758; +create module role Sec758.Editor; + +create persistent entity Sec758.Base ( + SharedField: String(100) +); + +-- Same-module ancestor: mixed own + inherited members. +create persistent entity Sec758.Item extends Sec758.Base ( + OwnField: String(100) +); + +-- System ancestor whose members ARE required in the child's rule. +create persistent entity Sec758.Attachment extends System.FileDocument ( + "Caption": String(200) +); + +-- User entity: System.User's platform members must NOT appear in the rule. +create persistent entity Sec758.Employee extends System.User ( + EmployeeNo: String(20) +); + +grant Sec758.Editor on Sec758.Item (read (SharedField, OwnField)); +grant Sec758.Editor on Sec758.Attachment (read ("Caption")); +grant Sec758.Editor on Sec758.Employee (read (EmployeeNo)); diff --git a/mdl/backend/modelsdk/attr_ref_owner_test.go b/mdl/backend/modelsdk/attr_ref_owner_test.go new file mode 100644 index 000000000..a28b4b4de --- /dev/null +++ b/mdl/backend/modelsdk/attr_ref_owner_test.go @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import "testing" + +// TestAttrRefBelongsTo pins the distinction reconciliation depends on: only a +// reference qualified against THIS entity names one of its own attributes and can +// be validated from this domain model. An inherited reference carries an +// ancestor's name — possibly from another module or System, neither loaded here — +// and deleting it as "stale" is mendixlabs/mxcli#758. +func TestAttrRefBelongsTo(t *testing.T) { + tests := []struct { + ref string + module string + entity string + want bool + }{ + {"Sec.Item.OwnField", "Sec", "Item", true}, + {"sec.item.OwnField", "Sec", "Item", true}, // Mendix names are case-insensitive + {"Sec.Base.SharedField", "Sec", "Item", false}, + {"System.FileDocument.Name", "Sec", "Attachment", false}, + {"Other.Base.Field", "Sec", "Item", false}, + {"NoDots", "Sec", "Item", false}, + {"", "Sec", "Item", false}, + // A same-named entity in another module is NOT this entity. + {"Other.Item.OwnField", "Sec", "Item", false}, + } + for _, tc := range tests { + if got := attrRefBelongsTo(tc.ref, tc.module, tc.entity); got != tc.want { + t.Errorf("attrRefBelongsTo(%q, %q, %q) = %v, want %v", + tc.ref, tc.module, tc.entity, got, tc.want) + } + } +} diff --git a/mdl/backend/modelsdk/domainmodel_security_write.go b/mdl/backend/modelsdk/domainmodel_security_write.go index 9102579a2..227dca293 100644 --- a/mdl/backend/modelsdk/domainmodel_security_write.go +++ b/mdl/backend/modelsdk/domainmodel_security_write.go @@ -5,6 +5,7 @@ package modelsdkbackend import ( "fmt" "sort" + "strings" "github.com/mendixlabs/mxcli/mdl/backend" "github.com/mendixlabs/mxcli/mdl/types" @@ -412,7 +413,8 @@ func (b *Backend) ReconcileMemberAccesses(unitID model.ID, moduleName string) (i } switch attrRef, assocRef := ma.AttributeQualifiedName(), ma.AssociationQualifiedName(); { case attrRef != "": - if attrSet[attrRef] { + switch { + case attrSet[attrRef]: covAttr[attrRef] = true if calcSet[attrRef] { if r := ma.AccessRights(); r == "ReadWrite" || r == "WriteOnly" { @@ -420,7 +422,21 @@ func (b *Backend) ReconcileMemberAccesses(unitID model.ID, moduleName string) (i changed = true } } - } else { + case !attrRefBelongsTo(attrRef, moduleName, entityName): + // An attribute reference is qualified against the entity that + // DECLARES it, so an inherited member carries an ancestor's name + // rather than this entity's. attrSet holds only this entity's own + // attributes, so every inherited entry looked stale and was + // deleted — silently, on any write touching the module, and + // immediately after the GRANT that had just written it correctly + // (mendixlabs/mxcli#758). The ancestor may live in another module + // or in System, neither loaded here, so an inherited reference + // cannot be validated at this layer at all. Preserve what cannot + // be checked instead of dropping it. + covAttr[attrRef] = true + default: + // Genuinely stale: the reference claims to be this entity's own + // attribute and the entity no longer has it. rule.RemoveMemberAccesses(i) changed = true } @@ -489,3 +505,17 @@ func newMemberAccess(rights, qualifiedName string, isAttr bool) *genDm.MemberAcc assignID(ma) return ma } + +// attrRefBelongsTo reports whether a MemberAccess attribute reference +// ("Module.Entity.Attribute") names one of the given entity's OWN attributes, +// rather than one inherited from an ancestor. +// +// Only an own reference can be validated from a single domain model: an ancestor +// may live in another module or in System, neither of which is loaded here. +func attrRefBelongsTo(attrRef, moduleName, entityName string) bool { + idx := strings.LastIndex(attrRef, ".") + if idx < 0 { + return false + } + return strings.EqualFold(attrRef[:idx], moduleName+"."+entityName) +} diff --git a/mdl/executor/cmd_security_write.go b/mdl/executor/cmd_security_write.go index beb415345..ffdbb65f8 100644 --- a/mdl/executor/cmd_security_write.go +++ b/mdl/executor/cmd_security_write.go @@ -377,21 +377,30 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error readMemberSet[m] = true } - // Create entries for all entity attributes - for _, attr := range entity.Attributes { + // Create entries for every attribute of the entity's access surface — its own + // AND those inherited through the generalization chain. Enumerating only + // entity.Attributes meant a GRANT naming an inherited member produced no entry + // at all while still reporting success, and left the rule incomplete so Mendix + // reported CE0066 (mendixlabs/mxcli#758). Each reference is qualified against + // the entity that DECLARES the member; qualifying an inherited one against this + // entity is CE1613 "The selected attribute no longer exists". + entityQN := module.Name + "." + s.Entity.Name + members := EntityMembers(ctx, entityQN) + grantedMembers := map[string]bool{} + for _, mem := range members { rights := defaultMemberAccess - if writeMemberSet[attr.Name] { + if writeMemberSet[mem.Name] { rights = "ReadWrite" - } else if readMemberSet[attr.Name] { + } else if readMemberSet[mem.Name] { rights = "ReadOnly" } // Calculated attributes cannot have write rights (CE6592) - isCalculated := attr.Value != nil && attr.Value.Type == "CalculatedValue" - if isCalculated && (rights == "ReadWrite" || rights == "WriteOnly") { + if mem.IsCalculated && (rights == "ReadWrite" || rights == "WriteOnly") { rights = "ReadOnly" } + grantedMembers[mem.Name] = true memberAccesses = append(memberAccesses, types.EntityMemberAccess{ - AttributeRef: module.Name + "." + s.Entity.Name + "." + attr.Name, + AttributeRef: mem.Ref, AccessRights: rights, }) } @@ -407,6 +416,7 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error } else if readMemberSet[assoc.Name] { rights = "ReadOnly" } + grantedMembers[assoc.Name] = true memberAccesses = append(memberAccesses, types.EntityMemberAccess{ AssociationRef: module.Name + "." + assoc.Name, AccessRights: rights, @@ -421,6 +431,7 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error } else if readMemberSet[ca.Name] { rights = "ReadOnly" } + grantedMembers[ca.Name] = true memberAccesses = append(memberAccesses, types.EntityMemberAccess{ AssociationRef: module.Name + "." + ca.Name, AccessRights: rights, @@ -428,6 +439,17 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error } } + // A member named in the GRANT that matched nothing used to be dropped in + // silence — the command reported success and the access simply was not there, + // which is why REVOKE + GRANT could not repair a damaged rule (#758). Name it + // instead. Inherited members now resolve, so anything still unmatched is a typo + // or a member of another entity. + if unknown := unmatchedGrantMembers(readMembers, writeMembers, grantedMembers); len(unknown) > 0 { + return mdlerrors.NewValidationf( + "entity %s has no member(s) %s; grant only names members of the entity or of an entity it inherits from", + entityQN, strings.Join(unknown, ", ")) + } + // Add MemberAccess entries for system associations (owner, changedBy). // When an entity has HasOwner/HasChangedBy, Mendix implicitly adds // System.owner/System.changedBy associations that require MemberAccess. diff --git a/mdl/executor/entity_hierarchy.go b/mdl/executor/entity_hierarchy.go new file mode 100644 index 000000000..45f60dd63 --- /dev/null +++ b/mdl/executor/entity_hierarchy.go @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "sort" + "strings" + + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// userEntityBase is the Mendix entity whose specializations are "user entities". +// Its members are managed by the platform (login, blocking, password), so they do +// NOT belong in a specializing entity's access rule — see EntityMembers. +const userEntityBase = "System.User" + +// EntityMember is one member of an entity's access surface: an attribute or an +// association, together with the qualified reference Mendix stores for it. +type EntityMember struct { + Name string // bare member name, as written in GRANT + // Ref is the reference stored in MemberAccess, qualified against the entity + // that DECLARES the member — which is an ancestor for an inherited one. + Ref string + Inherited bool + IsCalculated bool +} + +// EntityMembers returns every member of an entity's access surface: its own +// attributes plus those inherited through the generalization chain, each carrying +// the reference Mendix expects in a MemberAccess entry. +// +// Two rules here are load-bearing, both established against `mx check` rather than +// inferred (mendixlabs/mxcli#758, #765): +// +// 1. An inherited member's reference is qualified against the entity that +// DECLARES it, not the entity carrying the rule. Writing the child's name +// produces CE1613 "The selected attribute ... no longer exists"; writing the +// declaring entity's name validates clean. This is the same rule the +// change-object writer needs (#451). +// +// 2. Members inherited from System.User are excluded. Mendix manages the platform +// members of a user entity, and listing them turns a clean rule into CE0066 — +// verified both on Mendix's own Administration.Account and on a fresh +// specialization. Every other ancestor's members are REQUIRED: omitting the +// six System.FileDocument members from a specializing entity's rule is CE0066 +// until they are all present. +// +// Ancestors that cannot be resolved (module not in the project) stop the walk; the +// members found so far are returned rather than nothing, so a partial model still +// produces a usable rule. +func EntityMembers(ctx *ExecContext, entityQN string) []EntityMember { + var out []EntityMember + seen := map[string]bool{} // cycle guard + claimed := map[string]bool{} // a child's member shadows the ancestor's + + for currentQN, depth := entityQN, 0; currentQN != ""; depth++ { + if seen[currentQN] { + break + } + seen[currentQN] = true + + // Stop before collecting System.User's own members: its specializations are + // user entities, whose platform members Mendix owns. + if depth > 0 && strings.EqualFold(currentQN, userEntityBase) { + break + } + + entity, ok := findEntityByQN(ctx, currentQN) + if !ok { + break + } + + for _, attr := range entity.Attributes { + if attr == nil || claimed[attr.Name] { + continue + } + claimed[attr.Name] = true + out = append(out, EntityMember{ + Name: attr.Name, + Ref: currentQN + "." + attr.Name, + Inherited: depth > 0, + IsCalculated: attr.Value != nil && attr.Value.Type == "CalculatedValue", + }) + } + currentQN = entity.GeneralizationRef + } + return out +} + +// findEntityByQN resolves a qualified entity name through the backend. +func findEntityByQN(ctx *ExecContext, entityQN string) (*domainmodel.Entity, bool) { + if ctx == nil || ctx.Backend == nil { + return nil, false + } + parts := strings.SplitN(entityQN, ".", 2) + if len(parts) != 2 { + return nil, false + } + mod, err := ctx.Backend.GetModuleByName(parts[0]) + if err != nil || mod == nil { + return nil, false + } + dm, err := ctx.Backend.GetDomainModel(mod.ID) + if err != nil || dm == nil { + return nil, false + } + entity := dm.FindEntityByName(parts[1]) + if entity == nil { + return nil, false + } + return entity, true +} + +// unmatchedGrantMembers returns the members named in a GRANT that matched no +// attribute or association of the entity, in a stable order. +func unmatchedGrantMembers(readMembers, writeMembers []string, granted map[string]bool) []string { + var unknown []string + seen := map[string]bool{} + for _, list := range [][]string{readMembers, writeMembers} { + for _, name := range list { + if name == "" || granted[name] || seen[name] { + continue + } + seen[name] = true + unknown = append(unknown, name) + } + } + sort.Strings(unknown) + return unknown +} diff --git a/mdl/executor/entity_hierarchy_test.go b/mdl/executor/entity_hierarchy_test.go new file mode 100644 index 000000000..d9b4b01a8 --- /dev/null +++ b/mdl/executor/entity_hierarchy_test.go @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// hierarchyBackend serves three modules: +// +// App.Item extends App.Base (same-module ancestor) +// App.Doc extends System.FileDocument +// App.Employee extends System.User (a user entity) +func hierarchyBackend() *mock.MockBackend { + ids := map[string]model.ID{"App": "mod-app", "System": "mod-system"} + attr := func(name string) *domainmodel.Attribute { + return &domainmodel.Attribute{Name: name} + } + dms := map[model.ID]*domainmodel.DomainModel{ + ids["App"]: {ContainerID: ids["App"], Entities: []*domainmodel.Entity{ + {Name: "Base", Attributes: []*domainmodel.Attribute{attr("SharedField")}}, + {Name: "Item", GeneralizationRef: "App.Base", Attributes: []*domainmodel.Attribute{attr("OwnField")}}, + {Name: "Doc", GeneralizationRef: "System.FileDocument", Attributes: []*domainmodel.Attribute{attr("Caption")}}, + {Name: "Employee", GeneralizationRef: "System.User", Attributes: []*domainmodel.Attribute{attr("EmployeeNo")}}, + }}, + ids["System"]: {ContainerID: ids["System"], Entities: []*domainmodel.Entity{ + {Name: "FileDocument", Attributes: []*domainmodel.Attribute{attr("Name"), attr("Contents")}}, + {Name: "User", Attributes: []*domainmodel.Attribute{attr("Name"), attr("Password")}}, + }}, + } + return &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetModuleByNameFunc: func(name string) (*model.Module, error) { + id, ok := ids[name] + if !ok { + return nil, nil + } + return &model.Module{BaseElement: model.BaseElement{ID: id}, Name: name}, nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { return dms[id], nil }, + } +} + +func memberRefs(members []EntityMember) []string { + out := make([]string, 0, len(members)) + for _, m := range members { + out = append(out, m.Ref) + } + return out +} + +// TestEntityMembers_InheritedUseDeclaringEntity is the core of +// mendixlabs/mxcli#758 / #765: enumerating only the entity's own attributes meant a +// GRANT naming an inherited member wrote nothing, and reconciliation deleted any +// inherited entry as stale. The reference must be qualified against the entity that +// DECLARES the member — qualifying it against the child is CE1613. +func TestEntityMembers_InheritedUseDeclaringEntity(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + got := memberRefs(EntityMembers(ctx, "App.Item")) + want := []string{"App.Item.OwnField", "App.Base.SharedField"} + if len(got) != len(want) { + t.Fatalf("EntityMembers(App.Item) = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("member %d = %q, want %q", i, got[i], want[i]) + } + } +} + +// TestEntityMembers_SystemAncestorIncluded: omitting the System.FileDocument +// members from a specializing entity's rule is CE0066 until they are all present, +// so they must be enumerated. +func TestEntityMembers_SystemAncestorIncluded(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + got := memberRefs(EntityMembers(ctx, "App.Doc")) + for _, want := range []string{"App.Doc.Caption", "System.FileDocument.Name", "System.FileDocument.Contents"} { + found := false + for _, g := range got { + if g == want { + found = true + } + } + if !found { + t.Errorf("EntityMembers(App.Doc) missing %q; got %v", want, got) + } + } +} + +// TestEntityMembers_UserEntityExcludesSystemUser: Mendix manages the platform +// members of a user entity. Listing them turns a clean rule into CE0066 — +// verified against Mendix's own Administration.Account and a fresh specialization. +func TestEntityMembers_UserEntityExcludesSystemUser(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + got := memberRefs(EntityMembers(ctx, "App.Employee")) + if len(got) != 1 || got[0] != "App.Employee.EmployeeNo" { + t.Errorf("EntityMembers(App.Employee) = %v, want only its own member — "+ + "System.User's platform members must not appear", got) + } +} + +// TestEntityMembers_ChildShadowsAncestor: a child redeclaring an ancestor's +// attribute name must contribute its own reference once, not both. +func TestEntityMembers_ChildShadowsAncestor(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + // App.Doc declares Caption; System.FileDocument declares Name/Contents. Add a + // shadowing case by checking no member name appears twice. + seen := map[string]bool{} + for _, m := range EntityMembers(ctx, "App.Doc") { + if seen[m.Name] { + t.Errorf("member %q enumerated twice", m.Name) + } + seen[m.Name] = true + } +} + +func TestUnmatchedGrantMembers(t *testing.T) { + granted := map[string]bool{"OwnField": true, "SharedField": true} + got := unmatchedGrantMembers([]string{"SharedField", "Nope"}, []string{"OwnField", "Alsobad"}, granted) + if len(got) != 2 || got[0] != "Alsobad" || got[1] != "Nope" { + t.Errorf("unmatchedGrantMembers = %v, want [Alsobad Nope]", got) + } +} diff --git a/modelsdk/mpr/security_patch.go b/modelsdk/mpr/security_patch.go index 28e66bacd..c6059ab87 100644 --- a/modelsdk/mpr/security_patch.go +++ b/modelsdk/mpr/security_patch.go @@ -4,6 +4,7 @@ package mpr import ( "fmt" + "strings" "go.mongodb.org/mongo-driver/v2/bson" ) @@ -286,14 +287,28 @@ func secPatchReconcileMemberAccessesDoc(doc bson.D, moduleName string) (bson.D, if attrRef != "" { parts := secSplitQualifiedRef(attrRef) - if parts != "" && attrNames[parts] { + // An attribute reference is qualified against the entity that + // DECLARES it, so an inherited member carries an ancestor's + // name, not this entity's. Reconciliation used to compare only + // the bare member name against this entity's own attributes, + // so every inherited entry looked stale and was deleted — + // silently, and on any write that touched the module + // (mendixlabs/mxcli#758). The ancestor may live in another + // module or in System, neither of which is present in this + // document, so an inherited reference cannot be validated here + // at all: preserve it rather than delete what cannot be + // checked. + if !secRefBelongsToEntity(attrRef, moduleName, entityName) { + filtered = append(filtered, maDoc) + } else if parts != "" && attrNames[parts] { coveredAttrs[parts] = true if calculatedAttrs[parts] { maDoc = secDowngradeCalculatedAttrRights(maDoc) } filtered = append(filtered, maDoc) } else { - // Stale attribute ref (attribute was deleted or renamed). + // Genuinely stale: the reference claims to be this entity's + // own attribute, and the entity no longer has it. changes = append(changes, ReconcileChange{Entity: entityName, Member: parts, Action: "stripped"}) changed = true } @@ -500,3 +515,18 @@ func secStripInvalidAccessRuleProps(doc bson.D) (bson.D, bool) { } return cleaned, stripped } + +// secRefBelongsToEntity reports whether a MemberAccess attribute reference +// ("Module.Entity.Attribute") is qualified against the given entity — i.e. names +// one of its OWN attributes rather than one inherited from an ancestor. +// +// Only an own reference can be validated from this document: an ancestor may live +// in another module or in System, neither of which is loaded here. +func secRefBelongsToEntity(attrRef, moduleName, entityName string) bool { + parts := secSplitByDot(attrRef) + if len(parts) < 3 { + return false + } + owner := strings.Join(parts[:len(parts)-1], ".") + return strings.EqualFold(owner, moduleName+"."+entityName) +} diff --git a/sdk/mpr/writer_security.go b/sdk/mpr/writer_security.go index 15b05e148..b40e006f9 100644 --- a/sdk/mpr/writer_security.go +++ b/sdk/mpr/writer_security.go @@ -1472,7 +1472,17 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in if attrRef != "" { // Extract attribute name from Module.Entity.AttrName parts := splitQualifiedRef(attrRef) - if parts != "" && attrNames[parts] { + // An inherited member's reference is qualified against the + // entity that DECLARES it, so it does not match this + // entity's own attribute list and used to be deleted as + // stale (mendixlabs/mxcli#758). The ancestor may live in + // another module or in System, neither loaded here, so an + // inherited reference cannot be validated at this layer — + // preserve what cannot be checked. Mirrors the codec engine + // (mdl/backend/modelsdk.attrRefBelongsTo). + if !attrRefBelongsToEntity(attrRef, moduleName, entityName) { + filtered = append(filtered, maDoc) + } else if parts != "" && attrNames[parts] { coveredAttrs[parts] = true // Downgrade write rights on calculated attributes (CE6592) if calculatedAttrs[parts] { @@ -1653,3 +1663,15 @@ func stripInvalidAccessRuleProps(doc bson.D) (bson.D, bool) { // ensure primitive import is used var _ = primitive.Binary{} + +// attrRefBelongsToEntity reports whether a MemberAccess attribute reference +// ("Module.Entity.Attribute") names one of the given entity's OWN attributes, +// rather than one inherited from an ancestor. Only an own reference can be +// validated from a single domain model. +func attrRefBelongsToEntity(attrRef, moduleName, entityName string) bool { + idx := strings.LastIndex(attrRef, ".") + if idx < 0 { + return false + } + return strings.EqualFold(attrRef[:idx], moduleName+"."+entityName) +}