Upgrade plugin to support Mattermost 11.8#3
Conversation
Mattermost 11.8 moved user attribute fields out of the deprecated custom_profile_attributes property group and into the access_control group, with a new data shape and stricter validation. This commit updates the plugin to work against the new system. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request migrates the Mattermost user attribute sync plugin from the deprecated Custom Profile Attributes property group to the access_control property group, enabling attributes to be addressable in ABAC policy rules via ChangesUser attribute sync migration from CPA to access_control
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
AGENTS.md (1)
15-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language hint to the architecture fenced block.
The fenced block is missing a language identifier, which triggers markdownlint MD040.
Suggested fix
-``` +```text Plugin Activation (Once) ├─> Create/Update User Attribute Fields (schema) └─> Start Background Job (cluster-aware) Background Job (Configurable interval, default 60min) ├─> Fetch Changed Values From Provider └─> Bulk Upsert Values via PropertyService</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@AGENTS.mdaround lines 15 - 23, The markdown fenced block in AGENTS.md lacks
a language identifier (triggering markdownlint MD040); update the opening
triple-backtick for the architecture block to include a language hint (e.g.,
replacewithtext) so the block is explicitly marked as plain text and
the linter warning is resolved.</details> </blockquote></details> <details> <summary>README.md (1)</summary><blockquote> `17-25`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Specify a language for the architecture fenced block.** This block is missing a fence language and triggers markdownlint MD040. <details> <summary>Suggested fix</summary> ```diff -``` +```text Plugin Activation (Once) ├─> Create/Update User Attribute Fields └─> Start Background Job Background Job (On timed interval) ├─> Fetch Changed Values From External Source └─> Bulk Upsert Values ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@README.mdaround lines 17 - 25, The fenced code block showing the
architecture diagram is missing a language specifier, causing markdownlint
MD040; update the triple-backtick fence that encloses the "Plugin Activation
(Once) ..." block to include a language (e.g., "text") so it becomes ```text to
satisfy the linter and preserve formatting; locate the fenced block in README.md
and edit the opening fence only.</details> </blockquote></details> </blockquote></details>🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Inline comments: In `@server/sync/field_sync.go`: - Around line 108-116: The update block that modifies existingField (inside updateField) is missing an update for the access mode; add a line to set existingField.Attrs[model.PropertyAttrsAccessMode] from the incoming field definition (e.g. existingField.Attrs[model.PropertyAttrsAccessMode] = def.Attrs[model.PropertyAttrsAccessMode] or, if the definition exposes AccessMode directly, existingField.Attrs[model.PropertyAttrsAccessMode] = def.AccessMode) so that access mode changes applied in createField are also applied when updating existing fields. --- Outside diff comments: In `@AGENTS.md`: - Around line 15-23: The markdown fenced block in AGENTS.md lacks a language identifier (triggering markdownlint MD040); update the opening triple-backtick for the architecture block to include a language hint (e.g., replace ``` with ```text) so the block is explicitly marked as plain text and the linter warning is resolved. In `@README.md`: - Around line 17-25: The fenced code block showing the architecture diagram is missing a language specifier, causing markdownlint MD040; update the triple-backtick fence that encloses the "Plugin Activation (Once) ..." block to include a language (e.g., "text") so it becomes ```text to satisfy the linter and preserve formatting; locate the fenced block in README.md and edit the opening fence only.🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID:
1b5a5dd8-87dc-45a2-b8a8-98aea1ffccd1⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum📒 Files selected for processing (10)
AGENTS.mdREADME.mdgo.modplugin.jsonserver/job.goserver/plugin.goserver/sync/field_sync.goserver/sync/field_sync_test.goserver/sync/provider.goserver/sync/value_sync.go
| existingField.Type = def.Type | ||
| existingField.Attrs[model.CustomProfileAttributesPropertyAttrsVisibility] = model.CustomProfileAttributesVisibilityAlways | ||
| existingField.Attrs[model.PropertyFieldAttrVisibility] = model.PropertyFieldVisibilityAlways | ||
| existingField.Attrs[model.PropertyFieldAttrDisplayName] = def.DisplayName | ||
| existingField.Attrs[model.PropertyAttrsProtected] = true | ||
| // See createField for why all three permission levels are set to sysadmin. | ||
| sysadmin := model.PermissionLevelSysadmin | ||
| existingField.PermissionField = &sysadmin | ||
| existingField.PermissionValues = &sysadmin | ||
| existingField.PermissionOptions = &sysadmin |
There was a problem hiding this comment.
Missing AccessMode update for existing fields.
createField sets model.PropertyAttrsAccessMode (line 215), but updateField does not. If a field definition's AccessMode changes between plugin versions, existing fields will retain the old value.
🔧 Proposed fix
existingField.Attrs[model.PropertyFieldAttrVisibility] = model.PropertyFieldVisibilityAlways
existingField.Attrs[model.PropertyFieldAttrDisplayName] = def.DisplayName
existingField.Attrs[model.PropertyAttrsProtected] = true
+ existingField.Attrs[model.PropertyAttrsAccessMode] = def.AccessMode
// See createField for why all three permission levels are set to sysadmin.
sysadmin := model.PermissionLevelSysadmin📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| existingField.Type = def.Type | |
| existingField.Attrs[model.CustomProfileAttributesPropertyAttrsVisibility] = model.CustomProfileAttributesVisibilityAlways | |
| existingField.Attrs[model.PropertyFieldAttrVisibility] = model.PropertyFieldVisibilityAlways | |
| existingField.Attrs[model.PropertyFieldAttrDisplayName] = def.DisplayName | |
| existingField.Attrs[model.PropertyAttrsProtected] = true | |
| // See createField for why all three permission levels are set to sysadmin. | |
| sysadmin := model.PermissionLevelSysadmin | |
| existingField.PermissionField = &sysadmin | |
| existingField.PermissionValues = &sysadmin | |
| existingField.PermissionOptions = &sysadmin | |
| existingField.Type = def.Type | |
| existingField.Attrs[model.PropertyFieldAttrVisibility] = model.PropertyFieldVisibilityAlways | |
| existingField.Attrs[model.PropertyFieldAttrDisplayName] = def.DisplayName | |
| existingField.Attrs[model.PropertyAttrsProtected] = true | |
| existingField.Attrs[model.PropertyAttrsAccessMode] = def.AccessMode | |
| // See createField for why all three permission levels are set to sysadmin. | |
| sysadmin := model.PermissionLevelSysadmin | |
| existingField.PermissionField = &sysadmin | |
| existingField.PermissionValues = &sysadmin | |
| existingField.PermissionOptions = &sysadmin |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/sync/field_sync.go` around lines 108 - 116, The update block that
modifies existingField (inside updateField) is missing an update for the access
mode; add a line to set existingField.Attrs[model.PropertyAttrsAccessMode] from
the incoming field definition (e.g.
existingField.Attrs[model.PropertyAttrsAccessMode] =
def.Attrs[model.PropertyAttrsAccessMode] or, if the definition exposes
AccessMode directly, existingField.Attrs[model.PropertyAttrsAccessMode] =
def.AccessMode) so that access mode changes applied in createField are also
applied when updating existing fields.
1 participant
Summary
Mattermost 11.8 reworked how user attribute fields are stored. The property group they live in was renamed from
custom_profile_attributestoaccess_control, the data model changed shape, and the server now enforces validation rules that didn't exist before. The plugin failed to activate against 11.8 because of these changes:This PR brings the plugin up to date. It's intended as a reference for other plugin authors who store user attributes and need to make the same upgrade.
What needed to change
Bumped the
server/publicdependency tov0.4.1, which is the first release containing the new system. Mattermost server's owngo.modpins Go 1.26.3, so the plugin's minimum Go version moved up to match. The minimum Mattermost server version went from 11.5.0 to 11.8.0.Looked up the property group by its new name. The old name
custom_profile_attributesis rejected outright now. The new name is exposed asmodel.AccessControlPropertyGroupName.Set
ObjectTypeandTargetTypeon every field. The new system requires fields to declare what kind of object they describe (user) and at what scope they apply (system, meaning the field is defined once globally and applies to every user). The old system didn't require either. We also explicitly setObjectType: userrather than relying on a default, both because the system rejects fields without it and because this plugin only ever creates user fields.Renamed field identifiers to snake_case and added a separate display name. The server now requires field names to be valid identifiers (letters, digits, underscores, no spaces) because those names are referenced from ABAC policy expressions. Display names — the human-readable labels you see in the UI — used to share the same field but now live in a separate
display_nameattribute. So"Job Title"becameName: "job_title"withDisplayName: "Job Title". OurfieldDefinitionstruct was already close to this shape; the rename made it match.Pinned all three permission levels to sysadmin. New fields now have three permission settings (
PermissionField,PermissionValues,PermissionOptions) that say who, role-wise, is allowed to change different aspects of the field. For protected fields like ours, two of these don't really do anything — the server auto-pins them. But the default forPermissionValuesis "members can edit their own value," and the server rejects that combined withshared_onlyaccess (the two are contradictory — if anyone could write any value, they could fake having values in common with anyone). We overridePermissionValuestosysadminto clear that validation, and set the other two explicitly for clarity. See the comment increateFieldfor the full explanation.Switched to the canonical attribute constants. The old
model.CustomProfileAttributesPropertyAttrs*constants are now aliases for the canonicalmodel.PropertyFieldAttr*constants. Swapped to the canonical names since we're no longer thinking of these as a CPA-only concern.🤖 Generated with Claude Code