mirror of
https://github.com/mattermost/mattermost.git
synced 2026-09-21 05:54:10 +08:00
b9e8d5ce8213cef0295ece336989b16ef84ffbe3
22542
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jesse Hallam
|
b9e8d5ce82 | MM-68763: fix BuildAccessControlSubject call missing channelID argument (#36681) | ||
|
Ibrahim Serdar Acikgoz
|
e6c59693af | MM-68763: Discoverable Private Channels — Server feature complete (visibility, ABAC, queue API) (#36580) | ||
|
|
29fe2789a0 |
Exclude webhook posts from thread participation check (#36673)
* Exclude webhook posts from thread participation check When determining if the current user has replied to a thread for comment mention notifications, ignore posts made by webhooks even if they use the owning user's user_id. Co-authored-by: Sven Hüster <svelle@users.noreply.github.com> * Add missing isFromWebhook import in posts selector Co-authored-by: Sven Hüster <svelle@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Sven Hüster <svelle@users.noreply.github.com> |
||
|
Asaad MahmoodandCursor
|
eeb3c1ec04 |
MM-67237 - Open file preview modal when clicking draft attachment thumbnails. (#36590)
* Open file preview modal when clicking draft attachment thumbnails. Co-authored-by: Cursor <cursoragent@cursor.com> * Document draft thumbnail preview handler for CodeRabbit/doc checks. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix ESLint import/order in FilePreview connector and tests. Co-authored-by: Cursor <cursoragent@cursor.com> * Add tests for archived and deleted draft attachment preview guards. Co-authored-by: Cursor <cursoragent@cursor.com> * Enable draft attachment preview for all file types. Draft thumbnails were only clickable for images and SVGs; other attachments now open the standard file preview modal like post attachments do. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Ibrahim Serdar Acikgoz
|
ba1cec51a5 | [MM-68693] Resource level permission policies and new simulation (#36472) | ||
|
|
7739b349a0 |
[MM-68578] Add support packet DB performance diagnostics (#36324)
* Add support packet DB diagnostics for pool and pg_stat Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Fix support packet mock store for new DB diagnostics Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Add context timeouts to support packet pg diagnostics Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Move support packet DB diagnostics queries into sqlstore Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Fix support packet diagnostics lint and partial data handling Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Stabilize support packet pool idle assertion Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Relax live support packet DB counter assertions Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Fix deterministic pool diagnostics test wiring Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Mock support packet diagnostics in app test store Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Move SupportPacketDatabaseDiagnostics out of public model The struct is an internal store→platform transport (no yaml tags, never serialized directly) so it doesn't belong in server/public/model where it would form a public API contract for plugins. Move it into the store package as the natural return type of GetSupportPacketDatabaseDiagnostics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Describe SupportPacketDatabaseDiagnostics by content, not history Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * wording * Align store diagnostics with sqlstore conventions - Rename Store.GetSupportPacketDatabaseDiagnostics to Store.GetDiagnostics and rename the holding files to diagnostics{,_test}.go. - Drop the queryRowScanner / rowScanner / sqlQueryRowScanner test seam. The collectors now use the sqlxDBWrapper master handle and bind result rows into local structs via sqlx GetContext, matching how the rest of the sqlstore package talks to Postgres. - Replace the hand-rolled mock-based unit tests for the Postgres collector with an integration test driven through StoreTest, the pattern used by the other sqlstore tests (e.g. schema_dump_test.go). The pure pool-stats unit test (TestApplyDBPoolStats) is kept. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drop MasterDBStats/ReplicaDBStats from the Store interface After GetDiagnostics moved into the store layer, no caller outside the sqlstore package itself reads MasterDBStats/ReplicaDBStats through the Store interface — the diagnostics collector calls them on the concrete *SqlStore receiver. Remove them from the interface, the retry/timer layer wrappers, the storetest fake, and the generated mock; drop the now-redundant fixedDBStatsStore shim methods and mock setups in the support packet tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Rename SupportPacketDatabaseDiagnostics to DatabaseDiagnostics Now that the type lives in the store package and is returned by Store.GetDiagnostics, the SupportPacket prefix is just legacy framing — support packets are one consumer of the data, not its identity. Rename to store.DatabaseDiagnostics for consistency with the package and method name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Inline diagnostics SQL into the collector functions Each pg_stat query has a single caller, so a package-level constant just adds indirection between the function and the SQL it owns. Move the query strings to local consts inside the collectors that use them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix Connectios typo to Connections Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix sqlstore diagnostics build: call GetMaster().DB() method The recent rebase brought in a change that turned the SqlStore DB field into a method, so passing ss.GetMaster().DB to collectPostgresDatabaseDiagnostics (which expects *sqlx.DB) no longer compiles. Call the method instead. * Fix gofmt alignment in SupportPacketDiagnostics The post-merge struct had extra spaces on MasterConnections / ReplicaConnections that broke gofmt alignment. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
bc757c5c54 |
Fix content flagging update for unloaded posts (#36504)
* Fix content flagging update for unloaded posts Co-authored-by: mattermost-code <matty-code@mattermost.com> * Strengthen content flagging reducer tests Co-authored-by: mattermost-code <matty-code@mattermost.com> * Guard content flagging reducer updates Co-authored-by: mattermost-code <matty-code@mattermost.com> * Add non-array content flagging reducer test Co-authored-by: mattermost-code <matty-code@mattermost.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com> |
||
|
Ben Schumacher
|
2ab07701b5 |
[MM-68577] Add OAuth2/OpenID Connect provider status to support packet (#36451)
* [MM-68577] Add OAuth2/OpenID Connect provider status to support packet Probe configured GitLab, Google, Office365, and OpenID providers and report their connectivity status in the support packet diagnostics. For providers with a DiscoveryEndpoint, the probe verifies a valid OIDC discovery document (JSON with an "issuer" field) is returned; otherwise it probes the TokenEndpoint host, treating any HTTP response as reachable since token endpoints reject GETs. Disabled providers report status: disabled, enabled providers report ok or fail with the underlying error. No secrets are read or transmitted; only public endpoint URLs are probed. * [MM-68577] Drain response body in probeOAuthTokenEndpoint Closing resp.Body without first reading it leaves unread bytes on the wire, which prevents net/http from returning the underlying TCP connection to the idle pool for keep-alive reuse. Drain with io.Copy + io.LimitReader (1MB cap to bound a misbehaving server) and use defer for the close. * [MM-68577] Extract drainAndCloseBody helper for HTTP probe responses Three call sites in this file (probeOIDCDiscovery, probeOAuthTokenEndpoint, testPushProxyConnection) now share the same drain-then-close idiom needed to keep TCP connections eligible for keep-alive reuse. Replace the inline copies with a single drainAndCloseBody helper that bounds the discard at 1 MiB to limit exposure to a misbehaving server. Also fixes the same un-drained Close() bug in the pre-existing testPushProxyConnection while we're here. * Add comment explaining 1 MiB discovery response size cap Addresses review feedback asking for clarity on the 1<<20 limit. |
||
|
Miguel de la CruzandMiguel de la Cruz
|
6b20092e7a |
Fix MM-57406: prevent IPv6 hex segments from parsing as emoji (#36541)
* Fix MM-57406: prevent IPv6 hex segments from parsing as emoji Add word-boundary lookbehind and lookahead to EMOJI_PATTERN so a :name: token is only matched when neither side abuts an alphanumeric/underscore. Without this, hex runs in IPv6 addresses (e.g. :beef: in 2001:18:1:beef::/64) were tokenized and rendered as custom emoji. The new semantics align with the server parser at server/public/shared/markdown/emoji.go. Bump the webapp babel safari target from 16.2 to 16.4 since regex lookbehind is a syntax feature and is not lowered by @babel/preset-env. The actual supported minimum is well above 16.4, so the previous target was stale. Add regression tests for the IPv6 case and guardrail tests for back-to-back, paren-wrapped, and punctuation-terminated emoji. Update the existing asdf:goat:asdf:dash:asdf test to reflect the new (server-aligned) semantics. * Dedupe word-char class in EMOJI_PATTERN Use \w in the lookarounds and inside the name matcher ([\w+-]) so the word-char set is expressed once instead of being repeated as two slightly different literal character classes. Same set, same semantics; addresses review feedback on PR #36541. --------- Co-authored-by: Miguel de la Cruz <miguel@ctrlz.es> |
||
|
|
02bae8c3a1 |
Fix: Global Threads view shows only 1 quick reaction emoji instead of 3 (MM-68681) (#36512)
* Fix: Global Threads shows 1 quick reaction emoji instead of 3 (MM-68681) When posts are viewed in the Global Threads full-width view, the hover toolbar was incorrectly showing only 1 quick reaction emoji instead of 3. Root cause: In post/index.tsx, the isExpanded prop (which controls whether the toolbar shows 3 or 1 emojis) was derived only from state.views.rhs.isSidebarExpanded. When navigating to Global Threads, suppressRHS is dispatched (setting state.views.rhsSuppressed = true), but isSidebarExpanded remains false. Since posts in the thread viewer use RHS_ROOT/RHS_COMMENT locations (not CENTER), and the #sidebar-right element is suppressed (width = 0), the showMoreReactions check in post_options.tsx always fell through to showing only 1 emoji. Fix: Include state.views.rhsSuppressed in the isExpanded computation so that when the RHS is suppressed (i.e., we are in a full-width context like Global Threads or Drafts), the toolbar correctly renders 3 quick reaction emojis. Tests: Added post_options.test.tsx with 4 unit tests verifying: - CENTER location → 3 emojis (existing behavior) - RHS_ROOT + isExpanded=false → 1 emoji (narrow RHS, existing behavior) - RHS_ROOT + isExpanded=true → 3 emojis (expanded RHS or Global Threads) - RHS_COMMENT + isExpanded=true → 3 emojis Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Refactor: use getIsGlobalThreadsView selector instead of rhsSuppressed (MM-68681) Replace the broad state.views.rhsSuppressed check (which also fires for the Drafts view) with a precise getIsGlobalThreadsView() selector that reads the already-existing state.views.lhs.currentStaticPageId field. When global_threads.tsx mounts it dispatches selectLhsItem(LhsItemType.Page, LhsPage.Threads) which sets currentStaticPageId to LhsPage.Threads ('threads'). This is the existing, canonical signal that the Global Threads full-width view is active; the selector wraps it with a name that conveys intent directly. Changes: - selectors/lhs.ts: add getIsGlobalThreadsView() — returns true iff state.views.lhs.currentStaticPageId === LhsPage.Threads - selectors/lhs.test.ts: 3 new tests covering Threads, Drafts, and empty page - post/index.tsx: import getIsGlobalThreadsView and use it in isExpanded - post_options.test.tsx: update test description to match new mechanism Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Fix tests: use renderWithContext + real component tree, not jest.mock The previous post_options.test.tsx used jest.mock() to replace PostRecentReactions, DotMenu, and several other components. That pattern is not established in this codebase — post_component.test.tsx and post_reaction.test.tsx both exercise the real connected component tree via renderWithContext with a partial Redux state. Rewrite to match: - Drop all jest.mock() calls for child components. - Provide minimal Redux state (roles with ADD_REACTION + user with system_user role) so that ChannelPermissionGate lets the emoji buttons render — the same pattern used in post_reaction.test.tsx. - Use proper SystemEmoji shaped objects (with short_name) as recentEmojis so that getEmojiName() does not crash. - Assert on the real rendered emoji buttons (data-testid= 'post-menu__item_emoji') rather than a mocked prop capture. Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Remove selector comment; add e2e test for Global Threads quick reactions (MM-68681) - selectors/lhs.ts: remove JSDoc block from getIsGlobalThreadsView; the name is self-explanatory and the comment was narrating the code. - emoji_recently_used_spec.js: * Add group tag @collapsed_reply_threads (test now requires CRT config). * Add MM-T4261_3: verifies that hovering a post in the Global Threads full-width panel shows 3 quick reaction emojis, matching the center channel and unlike the narrow RHS sidebar which shows 1. * Add GLOBAL_THREADS case to validateQuickReactions helper (uses rhsPost id prefix, numReactions=3). Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Fix lint: correct import order in post_options.test.tsx and post/index.tsx - post_options.test.tsx: move @mattermost/types/emojis type import before mattermost-redux/constants; add missing blank line between import groups. - post/index.tsx: move selectors/lhs import before selectors/posts (alphabetical order within the selectors/* group). Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Fix types: use correct EmojiCategory value 'people-body' not 'people' Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Remove dead RHS_EXPANDED branch from validateQuickReactions helper No call site ever passes 'RHS_EXPANDED' to validateQuickReactions — the branch was unreachable. Keep only the 'GLOBAL_THREADS' case that the new MM-T4261_3 test actually exercises. Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> |
||
|
Eva Sarafianou
|
5cacd26776 |
Fix config-change-checker to use merge-base for per-file diffs (#36670)
Automatic Merge |
||
|
|
a7ef484fee |
[MM-68576] Add SAML connectivity status to support packet diagnostics (#36321)
* Add SAML connectivity status to support packet diagnostics Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Fix SAML diagnostics tests for config validation Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Add enterprise SAML diagnostics hook for support packet Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> * Cleanup * Fix SAML support packet tests to use enterprise mock interface Tests were expecting the platform layer to perform HTTP metadata URL checks directly, but that logic belongs in the enterprise SAML diagnostic implementation. Updated tests to install a mock enterprise interface (matching the existing pattern in the override test) instead of relying on bare HTTP calls that only work without the enterprise interface registered. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Simplify SamlDiagnosticInterface to return error instead of (string, string) The status return was always either StatusOk or StatusFail, which maps directly to nil/non-nil error. Removing the redundant status string makes the interface idiomatic Go and lets the call site derive status from error presence. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * lint fix --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Ben Schumacher <hanzei@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
981e5341ca |
Fix flaky TestUserHasJoinedChannel (#36660)
* Fix flaky TestUserHasJoinedChannel The UserHasJoinedChannel plugin hook is invoked from AddChannelMember via Srv().Go, so the hook post can appear after the join system post. Under CI load the 5s Eventually window was sometimes too short. Confirm the plugin is active before triggering the hook, poll posts in channel order like the sibling subtest, and extend the wait to 10s. Tests-only change. Verified with `go test -run '^TestUserHasJoinedChannel$' -race -count=50` locally. Co-authored-by: mattermost-code <matty-code@mattermost.com> * test: add plugin activation assertion messages * test: deduplicate plugin hook post assertion --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com> |
||
|
Jesse Hallam
|
77f9ecdfde | Upgrade Go to 1.26.3 and update deps in tool modules (#36658) | ||
|
|
c4b36dee16 |
Add user attribute validation banners (#36595)
* Add user attribute validation banners Add row-level and banner validation coverage for user attribute names so admins get clearer feedback before saving invalid attributes. Co-authored-by: Cursor <cursoragent@cursor.com> * UX Feedback --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Mattermost Build <build@mattermost.com> |
||
|
Jesse Hallam
|
2c925ccf88 | MM-68151: Update server dependencies (#36571) | ||
|
|
448a642835 |
Add inline action buttons for bot-posted markdown (#36219)
* Add inline action buttons for bot-posted markdown
Bots, webhooks, and plugins can now embed clickable action buttons
inside markdown (including table cells) using mmaction://actionId
links, with row-specific parameters forwarded to the integration on
click. This enables use cases like a per-row "Mx Plan" button in a
fleet-status table that opens a dialog scoped to the clicked row.
Design
- New post prop inline_actions maps actionId (alphanumeric) to a
PostActionIntegration {URL, Context}, capped at 50 entries.
- Markdown link with scheme mmaction:// emits a placeholder span that
messageHtmlToComponent converts to the InlineActionButton component.
- Click POSTs inline_context (parsed from the URL query string) to the
existing /posts/{id}/actions/{action_id} endpoint; the server merges
it into the integration request as context.inline_params while
preserving the post-level context.
- Only bot, webhook, and plugin posts render the button; non-integration
posts have inline_actions stripped on create, update, and ephemeral
broadcast. Hardened-mode also covers the new prop.
- Reuses the existing PostAction dialog pipeline: plugin handlers reply
with a trigger_id and call /actions/dialogs/open as before.
Security
- InlineContext capped at 50 entries / 128-char keys / 2 KB values.
- Integration Context cloned per click so per-click inline_params and
selected_option cannot leak into the cached post for other clickers.
- Plugin response updates cannot add inline_actions to a post that did
not already have them; invalid entries are dropped with a warn log.
- Label content and data attributes are escaped; labels are flattened
to plain text (tags stripped, entities decoded, then escaped).
- Malformed JSON request bodies now return 400 instead of falling
through with an empty inline_context.
Tests
- Model: validators, normalization, GetInlineAction, strip, fallback.
- App: create strip, update guard (4 subtests including
AllowInlineActionsUpdate bypass), ephemeral strip, inline_params
merge, context-map isolation, plugin-response guards, from_bot and
from_plugin retention across plugin updates.
- API: inline_context validation (size bounds + error id),
omitempty backward compat, malformed JSON 400.
- Webapp: renderer scheme handling, allow/deny flags, size caps,
HTML escape, tag strip, entity decode, attribute-injection defense;
component click dispatch, double-click race guard, unmount safety,
error-result recovery, aria state.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* lint fix
* i18n-extract
* Review fixes for inline action buttons
- renderer: preserve actionId case; reject opaque mmaction: URI
- app: require bot AND integration session to preserve inline_actions
- app: restore original inline_actions when plugin response is invalid
- i18n: rename key to ...app_error to match convention
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Tighten UpdatePost inline_actions guard; fix test seeds
- app: UpdatePost now requires AllowInlineActionsUpdate to modify
inline_actions. Integration session alone is insufficient — a
PAT-wielding user could otherwise inject inline_actions on any
post they could edit.
- tests: seed bot posts with inline_actions via an integration
session (intSeedCtx) so they survive the create-time strip.
- renderer: lint fix (blank line before comment block).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Reject malformed inline-action authorities at render time
- renderer: enforce ^[A-Za-z0-9]+$ on actionId, mirroring the server
regex. Authorities like mmaction://plan:443 or mmaction://user@plan
now fall through to plain text instead of rendering a dead button.
- post: clarify in the strip comment that webhooks and plugins bypass
CreatePostAsUser entirely (they call CreatePost / CreatePostMissingChannel
directly), so the strip block does not apply to them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Tighten inline-action renderer tests
- Replace oversized-params test with boundary pair (at-cap and over-cap)
to lock in the > vs >= behavior of the size-limit check.
- Add a "surrounding text survives" assertion for the tag-strip path so
a future swap from regex strip to a DOM sanitizer won't silently
drop legitimate content along with tags.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Inline action buttons via mmaction:// markdown links
Adds inline action buttons rendered from mmaction:// links in markdown,
with the click pipeline reusing the existing post-action infrastructure.
Aligned with the broader mm_blocks_actions framework (Daniel's PR).
* fix lint, DoS hardening, fix and rename test
* Address review feedback
* lint fix
* Reject percent-encoded path traversal in validateIntegrationURL (e.g. %2e%2e%2f) by parsing the URL and checking the decoded path.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Mattermost Build <build@mattermost.com>
|
||
|
Scott Bishel
|
a84941bec1 | Remove Legacy Interactive Dialog code (#35874) | ||
|
Ben Cooke
|
6189a3f54a | [MM-66489] Pull and populate certificate from metadata endpoint (#36557) | ||
|
Jesse Hallam
|
0790fc7281 | Upgrade Go to 1.26.3 (#36656) | ||
|
Jesse Hallam
|
c74e51f35e | chore(ci): upgrade Go to 1.26.3 in build container Dockerfiles (#36648) | ||
|
Jesse HallamandMattermost Build
|
51c6d5219f |
Fix config Sanitize fields missing from desanitize, causing FakeSetting to be persisted (#36619)
* Add TestDesanitizeRemovesAllFakeSettings to catch future omissions Walks every string field in the config after a Sanitize+desanitize round-trip and fails if any still holds FakeSetting. This catches the case where a field is added to Sanitize without a corresponding desanitize entry. * Fix ElasticsearchSettings.ClientKey being incorrectly masked as a secret ClientKey is a file path, not a secret value. Masking it caused the asterisk string to be persisted to the database on config writes, which broke TLS client auth on restart. * Fix desanitize missing entries for fields added in |
||
|
Jesse Hallam
|
41f3b22679 |
Fix flaky E2E tests (Cypress + Playwright) (#36637)
* Fix flaky email sort test by ignoring punctuation in localeCompare PostgreSQL's en_US.UTF-8 collation ignores hyphens at the primary sort level, but JS localeCompare() on a C-locale CI runner uses byte order, causing the expected and actual sort orders to diverge for emails containing hyphens. Passing ignorePunctuation:true aligns JS collation with Postgres behavior. * E2E/Cypress: re-enable CYPRESS_* env var overrides allowCypressEnv: false was introduced in the v15.13 upgrade (PR #36091) but broke the existing CYPRESS_adminUsername / CYPRESS_adminPassword override mechanism that local and CI runners depend on. * E2E/Cypress: fix MM-T1508 accessibility image test flakiness The test was failing because the admin user could have a stale compact display mode preference from a previous spec, causing post avatars to render with pointer-events: none and blocking the .status-wrapper click. Two fixes: - resetUserPreference() now resets message_display to 'clean' so compact mode doesn't leak across spec files - accessibility_image_spec before() now runs as a fresh user with default preferences rather than the shared admin account |
||
|
Maria A NunezandCursor
|
2db507464d |
Add auth token to flaky test webhook (#36636)
Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
|
345a0b76a6 |
Mm 68506 fe abac mask fe table editor cel and e2e (#36517)
* MM-68501 - implement GetMaskedVisualAST and wire API handler Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * add missing test and fix style issues * fix styles * implement coderabbit feedback * MM-68501 - PR review: split masking file, model-level access mode, reject contradictory config Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68501 - apply shared_only filter to non-option field values (binary masking) * MM-68501 - consolidate masking flag check and log corrupt text value during masking * MM-68503 - add CEL utilities, write-path validation, and merge helpers Combined set of helpers consumed by BE-5's save path: CEL construction / serialization - extractStringValues, buildCELFromConditions, conditionToCEL, celStringLiteral, celValueLiteral. Used to rebuild a CEL string from a VisualExpression, including for GetMaskedExpression on the read-side of policy GET / search responses. Merge-on-save helpers - getHiddenValues (per-condition, with pre-fetched fields map for N+1 avoidance) — finds which stored values are not visible to the caller. - mergeConditionValues — re-injects the hidden values into a submitted condition without duplicates. - Together, these let BE-5 preserve attribute values the caller cannot see while still letting them edit the visible parts of a policy. Write-path value-hold validation - validatePolicyExpressionValues, invalidValueError, validateConditionValues. - Generic "Invalid value." error on every rejection — no signal about whether the value exists or is merely not held (prevents enumeration). - Rejects the masked-token sentinel "--------" if submitted as a literal. These all live in access_control_masking.go alongside the masking primitives that BE-2 introduced. i18n entries added for the two new error IDs (app.pap.save_policy.invalid_value, app.pap.validate_expression_values.app_error). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68503 - handle the masked-token sentinel in validation and merge When the GET /policies endpoint returns a policy via MaskPolicyExpressions, the raw expression contains the masked-token sentinel "--------" in place of hidden values. If the frontend round-trips that expression unchanged back to the server (e.g., the admin only modified channel assignment, not the rules), the sentinel reaches the save path. The previous code in validateConditionValues rejected the sentinel as "Invalid value." This blocks the legitimate round-trip case. Fix: - validateConditionValues: treat the sentinel as a placeholder and skip it during visibility / source-only / unknown-mode checks. Other values are still validated normally. - mergeConditionValues: strip the sentinel from submitted values before appending hidden values, so it never propagates to the stored result. Both array and single-value forms (string == "--------") are handled. TestMaskedTokenRejection (which asserted the old rejection behavior) is replaced by TestMaskedTokenConstant which only verifies the sentinel string itself. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68504 - integrate save-path masking: 403 block on delete, merge-on-save, response masking Save path (CreateOrUpdateAccessControlPolicy): * validatePolicyExpressionValues runs on the submitted expression before merge so re-injected hidden values are never validated against the caller's holdings. * mergeStoredPolicyExpressions re-injects hidden values from the stored policy and blocks (HTTP 403) any attempt to remove a condition that contained values the caller cannot see — closes the row-deletion gap in classified environments. * mergeExpressionWithMaskedValues unwraps single-element arrays for scalar operators after restoring the stored operator (avoids "attr == [val]" invalid CEL when the frontend submits "attr in []" as the masked-row placeholder for an originally-scalar condition). * checkSelfInclusion is bypassed for system admins (they may legitimately write conditions for values they do not hold); masking and value-hold validation still apply to system admins. Delete path (DeleteAccessControlPolicy): * Same masked-values 403 block — a caller with masked values cannot delete the policy at all (UI Delete button is also disabled in FE-3). Response masking: * createAccessControlPolicy and setAccessControlPolicyActiveStatus run MaskPolicyExpressions on the response so even a save reply doesn't leak the values the caller does not hold. GetMaskedExpression, maskConditionValuesWithToken, replaceHiddenValuesWithToken, MaskPolicyExpressions live alongside the rest of the masking helpers in access_control_masking.go. team_access_control.go: corrects ValidateChannelEligibilityForAccessControl call site (drops the spurious receiver and rctx; it's a package-level helper that only takes channel). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68503 - address PR review: batch field fetches, propagate errors, fail-closed write path * MM-68503 - restore team-admin api4 tests accidentally dropped during BE-5 rebuild * MM-68503 - address review and CodeRabbit feedback on save-path masking * add tests for delete masking, self-inclusion, GET mask * add assertions to strengten tests * MM-68505 - add has_masked_values type and MaskedChip component Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * MM-68506 - add masking support to TableEditor and team settings modal TableEditor (table_editor.tsx, table_editor.scss): - hasMaskedValues plumbed through rows; lock operator/attribute selectors on masked rows. - Row remove (trash) button disabled on masked rows; disabled-state CSS so the icon doesn't show the destructive hover colour or a pointer cursor. - Test Rules button disabled when any row has masked values, with tooltip. - onMaskedStateChange callback to notify the parent for cross-component states (CEL editor read-only, Save disabled, banners). Value selectors (single_value_selector_menu.tsx, multi_value_selector_menu.tsx, selector_menus.scss, value_selector_menu.tsx): - Append MaskedChip after visible chips on multi-value rows. - Render MaskedChip as the sole value on single-value rows where the caller holds no visible value. Policy details (policy_details.tsx, .scss, .test.tsx): - Track hasMaskedRows state; receive from TableEditor via onMaskedStateChange. - Show masked-values warning banner above the editor when present. - Same banner on the Delete confirmation modal so admins understand why deletion is consequential. Team settings modal (team_policy_editor.tsx, .scss): - Same masked-values plumbing; delete button uses the disabled state when a policy has masked values, regardless of whether channels are assigned. - Pre-save check no longer treats "in []" as an incomplete rule — that placeholder comes from fully-masked rows that merge-on-save will fill in. i18n entries added for the new strings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68506 - fix hook order in SingleValueSelector when masked state changes The early return for `hasMaskedValues && !value` sat between useState and useCallback declarations, so when a parent re-render flipped the masked state (e.g. after deleting a sibling rule) React saw a different hook count and crashed with "Rendered fewer hooks than expected". Move the read-only short-circuit after all hook declarations so the hook order stays stable across renders. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68507 - CEL editor read-only when masked + system console wiring CEL editor (editor.tsx, editor.scss): - hasMaskedRows prop: when true, Monaco is set to read-only and a banner explains why ("This expression contains restricted values. Switch to Simple mode to edit the values you have access to, or delete the entire rule."). - Test Rules button disabled in CEL mode when hasMaskedRows is true. Policy details (policy_details.tsx, .scss): - hasMaskedRows state plumbed to CELEditor, TableEditor, and the Save / Delete buttons. - Save button disabled while masked rows are present (kept after the save-allowed-with-masked-values change in BE-5? — no, here we keep Save enabled so admins can add/modify rules; only row removal of masked rows is blocked). - Delete Policy button disabled when hasMaskedRows; a SectionNotice above the Delete card explains why ("This policy contains restricted values - Deletion not allowed"). - New save error messages: invalid_value and self_exclusion are surfaced from the server's generic responses. Policies list (policies.tsx): minor wiring change for the new state plumbing. Table editor (table_editor.tsx): cross-component coordination — emits onMaskedStateChange and respects the disabled-for-masked-row policy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68508 - E2E suite for attribute-value masking Covers the full read+write masking flow against a real server: - Masked chip rendering, operator/attribute lock, Test Rules disabled. - System admin subject to masking like any other caller (no role bypass). - Save with masked values: hidden values preserved by merge-on-save. - Trash button disabled on masked rows; server returns 403 on direct API attempt to remove a masked condition. - Delete Policy button disabled + server 403 when policy has masked values (both system console and team settings modal paths). - Self-inclusion failure only fires when the caller holds full visibility. - CEL editor read-only with banner when masked rows present. - Direct API validation: non-held values and the masked-token sentinel rejected with a generic "Invalid value." error. - Feature-flag-off path: no masking, all values visible. - Text-field shared_only masking (binary) with `in` and `==` operators. A pluggable DB-setup helper marks specific CPA fields as shared_only for the duration of a test (with per-test cleanup) since the API blocks setting access_mode=shared_only without a source_plugin_id. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68506 - fix lint, jest mock factory, and unreachable delete-modal test * MM-68506 - localize masked-condition-deleted save error * MM-68506 - fix masked-policy delete warning detection and localize masked_rule_deleted * fix linter issues * MM-68506 - surface delete error, lock value selector on masked rows, drop dead remove-modal * fix linter, add translations, adjust specs * import wittoltip from shared * fix linter and use the correct button variant * MM-68506 - drop dangling rationale comment in access_control_field_test * fix linter, translation and e2e tests * use pg ts types and dependencies for e2e types mocks * adjust switch mode persistance restriction * fix team settings style buttons * fail-closed guard for advanced expressions in merge-on-save, plus helper unit tests, and FF/test-helper cleanups * MM-68505 - add has_masked_values type and MaskedChip component Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * MM-68506 - add masking support to TableEditor and team settings modal TableEditor (table_editor.tsx, table_editor.scss): - hasMaskedValues plumbed through rows; lock operator/attribute selectors on masked rows. - Row remove (trash) button disabled on masked rows; disabled-state CSS so the icon doesn't show the destructive hover colour or a pointer cursor. - Test Rules button disabled when any row has masked values, with tooltip. - onMaskedStateChange callback to notify the parent for cross-component states (CEL editor read-only, Save disabled, banners). Value selectors (single_value_selector_menu.tsx, multi_value_selector_menu.tsx, selector_menus.scss, value_selector_menu.tsx): - Append MaskedChip after visible chips on multi-value rows. - Render MaskedChip as the sole value on single-value rows where the caller holds no visible value. Policy details (policy_details.tsx, .scss, .test.tsx): - Track hasMaskedRows state; receive from TableEditor via onMaskedStateChange. - Show masked-values warning banner above the editor when present. - Same banner on the Delete confirmation modal so admins understand why deletion is consequential. Team settings modal (team_policy_editor.tsx, .scss): - Same masked-values plumbing; delete button uses the disabled state when a policy has masked values, regardless of whether channels are assigned. - Pre-save check no longer treats "in []" as an incomplete rule — that placeholder comes from fully-masked rows that merge-on-save will fill in. i18n entries added for the new strings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68506 - fix hook order in SingleValueSelector when masked state changes The early return for `hasMaskedValues && !value` sat between useState and useCallback declarations, so when a parent re-render flipped the masked state (e.g. after deleting a sibling rule) React saw a different hook count and crashed with "Rendered fewer hooks than expected". Move the read-only short-circuit after all hook declarations so the hook order stays stable across renders. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68507 - CEL editor read-only when masked + system console wiring CEL editor (editor.tsx, editor.scss): - hasMaskedRows prop: when true, Monaco is set to read-only and a banner explains why ("This expression contains restricted values. Switch to Simple mode to edit the values you have access to, or delete the entire rule."). - Test Rules button disabled in CEL mode when hasMaskedRows is true. Policy details (policy_details.tsx, .scss): - hasMaskedRows state plumbed to CELEditor, TableEditor, and the Save / Delete buttons. - Save button disabled while masked rows are present (kept after the save-allowed-with-masked-values change in BE-5? — no, here we keep Save enabled so admins can add/modify rules; only row removal of masked rows is blocked). - Delete Policy button disabled when hasMaskedRows; a SectionNotice above the Delete card explains why ("This policy contains restricted values - Deletion not allowed"). - New save error messages: invalid_value and self_exclusion are surfaced from the server's generic responses. Policies list (policies.tsx): minor wiring change for the new state plumbing. Table editor (table_editor.tsx): cross-component coordination — emits onMaskedStateChange and respects the disabled-for-masked-row policy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68508 - E2E suite for attribute-value masking Covers the full read+write masking flow against a real server: - Masked chip rendering, operator/attribute lock, Test Rules disabled. - System admin subject to masking like any other caller (no role bypass). - Save with masked values: hidden values preserved by merge-on-save. - Trash button disabled on masked rows; server returns 403 on direct API attempt to remove a masked condition. - Delete Policy button disabled + server 403 when policy has masked values (both system console and team settings modal paths). - Self-inclusion failure only fires when the caller holds full visibility. - CEL editor read-only with banner when masked rows present. - Direct API validation: non-held values and the masked-token sentinel rejected with a generic "Invalid value." error. - Feature-flag-off path: no masking, all values visible. - Text-field shared_only masking (binary) with `in` and `==` operators. A pluggable DB-setup helper marks specific CPA fields as shared_only for the duration of a test (with per-test cleanup) since the API blocks setting access_mode=shared_only without a source_plugin_id. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68506 - fix lint, jest mock factory, and unreachable delete-modal test * MM-68506 - localize masked-condition-deleted save error * MM-68506 - fix masked-policy delete warning detection and localize masked_rule_deleted * fix linter issues * MM-68506 - surface delete error, lock value selector on masked rows, drop dead remove-modal * fix linter, add translations, adjust specs * import wittoltip from shared * fix linter and use the correct button variant * MM-68506 - drop dangling rationale comment in access_control_field_test * fix linter, translation and e2e tests * use pg ts types and dependencies for e2e types mocks * adjust switch mode persistance restriction * fix team settings style buttons * fail-closed guard for advanced expressions in merge-on-save, plus helper unit tests, and FF/test-helper cleanups * Refactor access control methods to use GetPropertyGroup for CPA group ID retrieval * fix styles * disable delete on masked policies in list view and remove dead modal warnings * fix unit tests * preserve hasAnyOf operator display for fully-masked multiselect conditions * address PR feedback: lock Actions on masked save, filter source/shared_only from /attributes, add unit tests and e2e tests * fix e2e tests * comment out e2e to isolate issue * completely remove the files to pass linter --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Mattermost Build <build@mattermost.com> |
||
|
Nick MisasiandCursor
|
1ffa4d8994 |
Add Docker Hub login to Cloud Agent start hook. (#36632)
Authenticate DinD pulls at runtime using Cursor dashboard secrets so agents avoid anonymous Docker Hub rate limits. Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Doug Lauder
|
5566604e03 |
MM-68838: Ping a restored plugin remote immediately on re-register (#36592)
* MM-68838: ping restored plugin remote immediately on re-register RegisterPluginForSharedChannels' restore branch updated the row but did not call PingNow, leaving the restored remote offline until the next pingLoop tick (up to PingFreq, default 1 minute). The new-connection branch already calls PingNow; the restore branch now mirrors it so sync attempts immediately after a plugin restart no longer fail with "offline remote cluster". * MM-68838: gob-encode error returns in apiRPCServer.ReceiveSharedChannelAttachmentSyncMsg The apiRPCServer wrapper for ReceiveSharedChannelAttachmentSyncMsg assigned the hook's error return directly to the gob-encoded response struct. When the framework's App.ReceiveSharedChannelAttachmentSyncMsg returned an error wrapped with %w (*fmt.wrapError, an unexported type), gob refused to encode it and the RPC server broke the connection with "type not registered for interface: fmt.wrapError". Every subsequent plugin/server RPC call then returned the zero-value response struct, causing plugins that dereferenced the nil returns to crash. Apply the existing encodableError() helper so the returned error becomes a gob-safe ErrorString, matching every other apiRPCServer method in this file. |
||
|
Maria A NunezandCursor Agent
|
5cd26002d3 |
Hide Download Apps link when running in Desktop app (#36614)
* Hide Download Apps UI when running in Desktop app Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * Fix ESLint import order for Desktop app visibility changes Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> |
||
|
Asaad MahmoodandCursor Agent
|
7bb6fb347b |
Fix AI toolbar separator visibility (#36356)
Co-authored-by: Cursor Agent <cursoragent@cursor.com> |
||
|
Daniel Espino García
|
92f6870a2b |
Add "last used" field for incoming webhooks (#36416)
* Add "last used" field for incoming webhooks * Address feedback * Rename migrations * Fix web lint |
||
|
|
0675d0ea0b |
Automations for config.json, API, audit log event, and Go release notes (#36075)
* Create config-change-checker.yml
* Create check_config_changes_ci.py
* Update config-change-checker.yml
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Update config-change-checker.yml
* Update check_config_changes_ci.py
* Update config-change-checker.yml
* Update config.go
* Fix check_api to detect multi-line and multi-method endpoints
The previous implementation matched the .Handle(...).Methods(...) regex
line-by-line against diff lines. This silently missed two real and
common patterns in api4/:
1. Multi-line .Handle(...) declarations — e.g. group.go has 18 of
them, where the path lives on one line and the wrapper/handler on
the next. The regex never matched, so PRs adding such endpoints
produced empty release-note entries.
2. Multi-method declarations like
.Methods(http.MethodGet, http.MethodHead) (4 instances in file.go)
— the old regex required a closing paren immediately after the
first method.
The fix:
- Add a file_at(ref, path) helper that snapshots a file at a git ref
via 'git show', so checkers can compare full file states instead of
pattern-matching diff text.
- Add _scan_endpoints() that whitespace-collapses the file before
matching, letting the regex span what were originally multiple
lines.
- Loosen _HANDLE_RE to capture the methods list as a substring and
extract individual HTTP verbs with a known-method allowlist, so
multi-method declarations produce one entry per verb.
- Switch check_api to set-diff (after - before) / (before - after)
on the parsed endpoint sets. This also cleanly handles routes
that move within a file (no fragile add/remove dedup needed).
- Anchor the new/deleted file detection to '^new file mode \d+' to
avoid false positives from stray text in source files.
Made-with: Cursor
* Track enclosing struct in check_config to avoid dedup collisions
The previous check_config keyed its add/remove dedup on the bare field
name. The dedup intent was to ignore fields that were merely reordered
within config.go (which appear in the diff as both '-Foo' and '+Foo').
But because the key was just the field name, an unrelated rename in one
struct could silently cancel out a real new field with the same name in
a different struct. For example, in a single PR:
- EnableFoo *bool // removed from ServiceSettings
+ EnableFooV2 *bool
- EnableBar *bool // removed from EmailSettings
+ EnableFoo *bool // newly added — but wrongly cancelled below
The dedup would see 'EnableFoo' in both lists and drop both entries,
hiding the brand-new EmailSettings.EnableFoo from the release-note
output.
The fix tracks each field's enclosing struct using a brace-depth stack
that walks the file at BASE_SHA and HEAD_SHA. Fields are keyed as
(struct_name, field_name) tuples, so identically-named fields in
different structs are distinct, and the dedup only collapses true
reorderings. As a side benefit the rendered output is now
'StructName.FieldName' which is much more useful to reviewers.
Switching to file-at-revision scanning + set diff also removes the
custom dedup logic entirely — set arithmetic handles "moved within
file" naturally.
Made-with: Cursor
* Switch remaining checkers to file-at-revision style; drop lines_by_sign
check_audit_events and check_go_version still parsed +/- diff lines
directly, with the same brittle dedup-and-cancel logic that was used in
the previous check_config. After the previous two commits the rest of
the file uses the file_at(ref, path) helper to compare full file
states between BASE_SHA and HEAD_SHA, which:
- removes the entire moved-within-file dedup dance (set arithmetic
handles it for free),
- aligns all four checkers on a single, easy-to-reason-about pattern,
- is robust to whitespace-only or reordering edits in the watched
files.
For Dockerfile.buildenv the helper also avoids a subtle case where the
old code only inspected +/- lines: an edit to an unrelated RUN line
that didn't touch the FROM line could in theory leave both old_ver and
new_ver as None even though the version was effectively unchanged.
Reading the file at each revision compares the actual current and
previous FROM line directly.
The lines_by_sign helper now has no callers, so remove it.
Made-with: Cursor
* Update config.go
* Update config.go
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Tighten check_config_changes_ci.py: regex coverage + idempotency
- Restore tolerant `_HANDLE_RE` so 2-arg wrappers (e.g. `api.APISessionRequired(handler, handlerParamFileAPI)`)
are not silently dropped from the api4 endpoint scan; broaden the `.Methods(...)`
capture so string-literal variants (`Methods("GET")`) work too. Filtering moves
back to the `_HTTP_METHODS` allowlist in `_parse_methods` to keep stray
identifiers from being treated as HTTP verbs.
- Make `strip_old_note` also remove auto-generated lines that landed outside
the ```release-note fence (the inject_note fallback paths) so reruns no
longer accumulate duplicates when a PR has no fence.
- Skip the GitHub PATCH when the PR description is already up to date, so
every commit no longer triggers an unconditional write.
- Wire up `check_go_version`'s `additions` path in `_format_lines` and
`_AUTO_LINE_RE` so a freshly-added Dockerfile.buildenv emits a note.
- Remove the now-dead `CheckResult.to_markdown` method (replaced by
`_format_lines`).
Made-with: Cursor
* Restore ExperimentalSettings.EnableWatermark
The field was removed in
|
||
|
sabril
|
9d318dc4cd |
refactor: speed up E2E test workflows and eliminate npm cache-restore failures (#36599)
Workers no longer run `npm ci` — `node_modules` and framework binaries are restored from actions/cache populated once by a new `prep-deps` job. This closes the intermittent EEXIST/ENOENT failure inside npm's own cacache writer that occasionally fails `npm ci` on a runner. Removing `npm ci` from workers also cuts ~5 min of duplicated install work per worker. dispatch-begin now runs as its own job after prep-deps so it fires once the per-worker test-server setup is the only remaining work before dispatch-run. |
||
|
sabril
|
1d1580cb3c | chore: update reusable workflows to specific commit sha (#36600) | ||
|
|
23b4d8275b |
MM-68197 Show classification banners in web and desktop apps (#36490)
* Add Classification Markings admin console page Adds a new admin console page under Site Configuration for managing classification markings. This allows system administrators to define classification levels (e.g., UNCLASSIFIED, SECRET, TOP SECRET) with associated colors and rank ordering, which will be used for system-wide and per-channel classification banners. The page includes: - Enable/disable toggle backed by the property field system (field existence = enabled) - Country preset dropdown (US DoD, NATO, UK GSCP, Canada, Australia PSPF) that auto-fills standard classification levels - Editable classification levels table with drag-and-drop reorder, inline text editing, color picker, and delete - Auto-switch to "Custom" preset when levels are manually modified - Confirmation dialog when switching presets would overwrite custom data Also adds: - ClassificationMarkings feature flag (default off) - Generic property field client methods (get/create/patch/delete) for the /api/v4/properties/ endpoints - Enterprise license + feature flag gating on the admin page Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix classification markings: add validation, error handling, and system object type - Add "system" as a valid property field object type so the classification markings API calls succeed - Surface load errors instead of silently swallowing them (only suppress 404 for unconfigured state) - Validate before save: require at least one level, non-empty names, and no duplicates - Default to custom preset with empty levels on first open - Add section strings to searchableStrings for admin console search Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Move classification field to CPA group targeting users Store the classification markings property field in the custom_profile_attributes group with object_type 'user' instead of the attributes group with object_type 'system'. Clear target_id for PSAv2 system target compliance and mark the field as admin-managed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Stabilize preset option IDs and add danger warning on preset switch Hardcode deterministic IDs for all preset classification levels so switching away and back preserves option IDs, preventing orphaned property values. Compare only level data (not preset label) for change detection so cosmetic preset switches don't trigger false save states. Show a danger modal with red confirm button when changing presets on an existing field, warning about system-wide impact on classified resources. The warning appears once per session then allows frictionless switching. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove system object type from property fields Not needed yet — will be added when system/channel banners are implemented. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix ESLint errors in classification markings admin page Fix import ordering and remove unused generateId import. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address CodeRabbit review feedback for classification markings - Register property field API endpoints when ClassificationMarkings flag is enabled (not just IntegratedBoards) to prevent 404s - Preserve preset option IDs when creating a new classification field instead of blanking them with empty strings - Add sysconsole read/write permission constants for classification markings across server and webapp, and wire up resource-level permission checks in the admin definition Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add rank attribute to classification marking options Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add classification markings permissions migration and read-only support Add a permissions migration to grant classification markings sysconsole permissions to existing roles on upgrade. Wire up the disabled prop so read-only users can view but not edit classification settings. Register the permission in the Delegated Granular Administration UI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Paginate loadField to find classification field beyond first page Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix lint errors and warnings in classification markings Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove classification markings sysconsole permissions; gate on sysadmin instead Classification markings admin page no longer uses feature-specific read/write permissions. Visibility is gated on license + feature flag, editing is gated on system admin role. This avoids coupling feature-specific permissions to the generic property service. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Set sysadmin-level permissions on classification markings field creation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use stable IDs instead of array indices for classification level operations Switch updateLevel/deleteLevel to identify levels by ID rather than index, sort levels by rank on load, and extract i18n strings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Refactor classification markings into extracted helper functions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add tests for classification markings admin console feature Add unit and component tests covering: - Pure function tests for detectPreset, optionsToLevels, levelsToOptions, processClassificationField, and fetchClassificationField pagination logic - React component tests for rendering states, validation, and user interactions - Client4 property field method tests for URL construction and HTTP verbs - Server routing test verifying routes register with ClassificationMarkings flag - Feature flag default and serialization test Export pure functions from classification_markings.tsx to enable direct testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix lint errors in classification markings tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix test compilation error * Fix color input auto-filling after 3 hex characters in classification markings Buffer ColorInput onChange in a LevelColorCell wrapper so the table doesn't re-render mid-typing, preventing the input from losing its focus-guarded local state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fixing style issues with color picker z-index * Added fix to prevent immediate dismissal when clicking inside color picker * Adding E2E test suite for configuration * Removing duplicates * Fixing unrelated linter error * Fixing test linting issues * Updating tests to skip appropriately * Matching configuration to UX specs * Fixing style lint * Added informational banner for presentational nature of markings * Enabling the markings flag on playwright server * Added missing feature flag to e2e test environment in ci * Reverting changes to color_input - Not needed as we're using a custom component * Added and polished global banner configuration * Refactoring webapp for readability - Separating components - Adding unit tests - Isolating helper methods into utilities * Fixing linter errors * linter fix * Manually fixing linter issues * Separating global classification component * Added persistence of classification marking configuration * Changing LevelID with LevelName * Making changes for PR reviews * Changing property object of classification field to template * syncing i18n file * Removing inaccurate note from comments * PR fixes for UX review * Cleaning up unused value * Added GlobalClassificationBanner component - Made sure it syncs on change by using normal configuration values on it - Works with "top" and "top_and_bottom" - Renders on both root and admin_console * Adding E2E test cases for global classification * Linter fixes, i18n extract * PR Fixes * Linter fix * Matching default messages * Fixing type errors * Fixing pipeline and runtime errors * Fixing announcementbar rendering on top of global classifications * Increasing banner & font sizes * Fixing font size to 12px instead of 16px - I read it wrong * Replacing config values with property * Test linter fixes * Fixing type errors and go format error * Making changes needed to align with specs - Ensuring system_classification is a separate linked property that differs from the template - Saving the global classification banner values as a propertyvalue * Added missing arguments in e2e tests * Added missing conditions for useEffect - Also fixing E2E error in pipeline * Fixing issues with V1 and V2 group mismatch * Fixes for linter errors and coderabbit review * Addressing more issues found by coderabbit * Fixing issues found by coderabbit * Migrating to use system properties * Ran all linters and prettier - Resolving coding style drift that happened from not running prettier on the webapp (even though CI doesn't check for this) * Undoing the prettier changes in webapp * Cleaning up unwanted autoformatted changes * Reverting prettier changes to clean diff * Fixing E2E test * Import fixes in test * Applying changes for PR feedback * Fixing issues with failing e2e tests * Changing key of selection from name to id * Replacing field setup in E2E tests to use levelId instead of levelName * Added classification setup per channel on channel creation * WIP: Adding classification banner integrated with channel banners - Using a hook to resolve which values should be evaluated when displaying the banner * Fixing style of dropdown input for classifications * Fixing visual issues with dropdown inputs * Adding E2E Tests and linter fixes * General fixes and improvements * Applying linter fixes * Resolving lingering linter issues * Updated snapshot and extracted i18n * Adding test cleanup to prevent failures due to duplicates * Addressing nitpick comment for test mapping of values * Applying more fixes to E2E tests * Improving test coverage and e2e test cleanup * Resolving type issues * Refactoring classification constant names an documentation * Ensuring propertyvalue only stores single id, storing banner text in banner_info * Fixing issues with linter alongside style issues on header * Updating test assertion to account for fallback * Fixing issues found during testing - Removing custom selection from being an option and turned it into a state - Ensuring only system administrators can set channel classification levels * Fixing z-index issue with color input popover * Setting classification level to lowest available value when switching it on * Updating unit tests to match new spec for preselection --------- Co-authored-by: David Krauser <david@krauser.org> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: David Krauser <david@kruser.org> Co-authored-by: Mattermost Build <build@mattermost.com> |
||
|
Carlos Garcia
|
548183d748 |
Mm 68282 admin ephemeral mode (#36194)
* adds feature flag to enable mattermost ephemeral mode * add ephemeral mode config settings to system console When feature flag is set to true a new section for Mobile Ephemeral Mode settings shows under the Mobile Security section in case a valid Enterprise Advanced License is active. * adds Mobile Ephemeral Mode settings playwright tests * improve descriptions for settings * improves error messages and hints * move validation to common helper and add new tests * reverts package-lock.json changes * proper struct alignment * proper message sorting in json file * use generic doc url for MEM section while docs are not ready * Proper formatting for playwright tests * fixes test |
||
|
|
9bd77d3fc4 |
MM-68702: Reject demoting bot accounts to guest (#36487)
* MM-68702: Reject demoting bot accounts to guest Deny DemoteUserToGuest when the target is a bot so User Managers cannot degrade bot capabilities via guest conversion without bot administration permissions. Adds API error string and tests. Co-authored-by: Julien Tant <JulienTant@users.noreply.github.com> * Fix TestDemoteUserToGuest bot subtest: enable bot creation in config Default test config disables bot accounts; enable ServiceSettings EnableBotAccountCreation for the subtest and restore afterward. Co-authored-by: Julien Tant <JulienTant@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Julien Tant <JulienTant@users.noreply.github.com> |
||
|
|
d4471bece1 |
Mm 68503 be abac mask save path masking (#36513)
* MM-68501 - implement GetMaskedVisualAST and wire API handler Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * add missing test and fix style issues * fix styles * implement coderabbit feedback * MM-68501 - PR review: split masking file, model-level access mode, reject contradictory config Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68501 - apply shared_only filter to non-option field values (binary masking) * MM-68501 - consolidate masking flag check and log corrupt text value during masking * MM-68503 - add CEL utilities, write-path validation, and merge helpers Combined set of helpers consumed by BE-5's save path: CEL construction / serialization - extractStringValues, buildCELFromConditions, conditionToCEL, celStringLiteral, celValueLiteral. Used to rebuild a CEL string from a VisualExpression, including for GetMaskedExpression on the read-side of policy GET / search responses. Merge-on-save helpers - getHiddenValues (per-condition, with pre-fetched fields map for N+1 avoidance) — finds which stored values are not visible to the caller. - mergeConditionValues — re-injects the hidden values into a submitted condition without duplicates. - Together, these let BE-5 preserve attribute values the caller cannot see while still letting them edit the visible parts of a policy. Write-path value-hold validation - validatePolicyExpressionValues, invalidValueError, validateConditionValues. - Generic "Invalid value." error on every rejection — no signal about whether the value exists or is merely not held (prevents enumeration). - Rejects the masked-token sentinel "--------" if submitted as a literal. These all live in access_control_masking.go alongside the masking primitives that BE-2 introduced. i18n entries added for the two new error IDs (app.pap.save_policy.invalid_value, app.pap.validate_expression_values.app_error). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68503 - handle the masked-token sentinel in validation and merge When the GET /policies endpoint returns a policy via MaskPolicyExpressions, the raw expression contains the masked-token sentinel "--------" in place of hidden values. If the frontend round-trips that expression unchanged back to the server (e.g., the admin only modified channel assignment, not the rules), the sentinel reaches the save path. The previous code in validateConditionValues rejected the sentinel as "Invalid value." This blocks the legitimate round-trip case. Fix: - validateConditionValues: treat the sentinel as a placeholder and skip it during visibility / source-only / unknown-mode checks. Other values are still validated normally. - mergeConditionValues: strip the sentinel from submitted values before appending hidden values, so it never propagates to the stored result. Both array and single-value forms (string == "--------") are handled. TestMaskedTokenRejection (which asserted the old rejection behavior) is replaced by TestMaskedTokenConstant which only verifies the sentinel string itself. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68504 - integrate save-path masking: 403 block on delete, merge-on-save, response masking Save path (CreateOrUpdateAccessControlPolicy): * validatePolicyExpressionValues runs on the submitted expression before merge so re-injected hidden values are never validated against the caller's holdings. * mergeStoredPolicyExpressions re-injects hidden values from the stored policy and blocks (HTTP 403) any attempt to remove a condition that contained values the caller cannot see — closes the row-deletion gap in classified environments. * mergeExpressionWithMaskedValues unwraps single-element arrays for scalar operators after restoring the stored operator (avoids "attr == [val]" invalid CEL when the frontend submits "attr in []" as the masked-row placeholder for an originally-scalar condition). * checkSelfInclusion is bypassed for system admins (they may legitimately write conditions for values they do not hold); masking and value-hold validation still apply to system admins. Delete path (DeleteAccessControlPolicy): * Same masked-values 403 block — a caller with masked values cannot delete the policy at all (UI Delete button is also disabled in FE-3). Response masking: * createAccessControlPolicy and setAccessControlPolicyActiveStatus run MaskPolicyExpressions on the response so even a save reply doesn't leak the values the caller does not hold. GetMaskedExpression, maskConditionValuesWithToken, replaceHiddenValuesWithToken, MaskPolicyExpressions live alongside the rest of the masking helpers in access_control_masking.go. team_access_control.go: corrects ValidateChannelEligibilityForAccessControl call site (drops the spurious receiver and rctx; it's a package-level helper that only takes channel). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * MM-68503 - address PR review: batch field fetches, propagate errors, fail-closed write path * MM-68503 - restore team-admin api4 tests accidentally dropped during BE-5 rebuild * MM-68503 - address review and CodeRabbit feedback on save-path masking * add tests for delete masking, self-inclusion, GET mask * add assertions to strengten tests * fail-closed guard for advanced expressions in merge-on-save, plus helper unit tests, and FF/test-helper cleanups * Refactor access control methods to use GetPropertyGroup for CPA group ID retrieval --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Mattermost Build <build@mattermost.com> |
||
|
|
f0360a838a |
Data spillage report generation UI (#36340)
* Added base fr report generation * WIP * implemented UI flow * implemented UI flow * restructured the modal code into sub components * Refactoring and cleanup * lint fixes, added new tests * i18n fix * test fix * Updated test * CI * Several improvements * WIP * Added tests * Addressed some security enhancements * Created zip writer entery later * Improved a test to check for file content * Improved error handling * Made a geneeric function * Updated classes * accepting comment in report API * Added more tests * Integrated new API param * Removed an unnecessary check * Made a geneeric function * Made a geneeric function * Made the comment body not required and updated API docs * Updated report generation API call in download report button * Included decision in report and removed confirmation when keeping message * Updated test * Add explicit wait for removeWithoutReportButton visibility in test Prevent race condition by waiting for the button to be visible after UI transitions to skip-confirm step before clicking it. Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * PR Feedback * explicitelly added return statement * Included actor details in report * Updated tests --------- Co-authored-by: maria.nunez <maria.nunez@mattermost.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Mattermost Build <build@mattermost.com> |
||
|
unified-ci-app[bot]
|
479103d868 |
chore: Update NOTICE.txt file with updated dependencies (#36609)
Automatic Merge |
||
|
|
bab9009825 |
MM-68592: Add leave confirmation modal for policy-added public channels (#36439)
* MM-68592: Add leave confirmation modal for policy-added public channels When a user attempts to leave a public channel they were auto-added to via a membership policy (channel.policy_enforced), show a confirmation modal informing them that the leave is permanent and offering a 'Mute instead' option as a lighter alternative. The flow follows the existing pattern used for private channel leave confirmation. The modal is opened from: - Channel header menu Leave action - Sidebar channel menu Leave action - /leave slash command The Mute instead button is hidden when the channel is already muted. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68592: address CodeRabbit review - Make handleMuteInstead async and only close the modal when the mute action resolves successfully, leaving it open on error so the user can retry or choose to leave instead. - Move autoFocus from the destructive 'Leave channel' button to the non-destructive secondary action ('Mute instead' or 'Cancel') so pressing Enter does not default-confirm a permanent leave. - Cover the failure path with a new unit test that asserts the modal remains open when muteChannel returns an error. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> |
||
|
|
f067fcde92 |
MM-66339 Hide empty content-flagging "With comment" section in reviewer DM (#36552)
* Add Cursor Cloud Agent Docker environment Co-authored-by: Cursor <cursoragent@cursor.com> * Fix Cloud Agent enterprise and Docker access Co-authored-by: Cursor <cursoragent@cursor.com> * Fix Cloud Agent Go path setup Co-authored-by: Cursor <cursoragent@cursor.com> * MM-66339 Stop double-JSON-stringifying content flagging comments The flagPost, removeFlaggedPost, and keepFlaggedPost Client4 helpers were calling JSON.stringify on the comment value before placing it in the JSON request body. When the reporter or reviewer left the optional comment blank, JSON.stringify('') returned the literal two-character string '""', which the server then stored as the comment and embedded in the reviewer DM as 'With comment:\n\n> ""'. Send comment as the plain string instead so an empty comment stays empty and the 'With comment' section is omitted entirely. Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> --------- Co-authored-by: Nick Misasi <nick.misasi@mattermost.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
|
9d06155540 |
Update bot checks (#36503)
* Fix bot permission checks in revokeSession, revokeAllSessionsForUser, and updatePassword MM-68701: Align permission checks with the bot-aware pattern used by updateUser, patchUser, deleteUser, and (via MM-68686) updateUserActive. Three handlers were missing the IsBot branch: - revokeSession / revokeAllSessionsForUser: both gated access through SessionHasPermissionToUser, which only requires EditOtherUsers (an ancillary permission granted to User Managers). Switching to SessionHasPermissionToUserOrBot routes bot targets through SessionHasPermissionToManageBot first and falls back to the user path only when the target is not a bot. - updatePassword: the permission flag canUpdatePassword was set by checking PermissionSysconsoleWriteUserManagementUsers (or PermissionManageSystem for system admins) with no IsBot branch. Adding an else-if user.IsBot guard routes bot targets through SessionHasPermissionToManageBot, consistent with every other handler in the file that touches bot accounts. Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Improve TestRevokeSessionBotPermissions: revoke a real bot session Seed a session directly via th.App.CreateSession instead of passing a fake ID and expecting a 400. The test now validates the full happy path: the session row is created, the privileged user revokes it, and the call returns 200 OK. Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Strengthen forbidden sub-test: revoke a real bot session with no perms Seed a real session for the bot before the unprivileged revoke call. The test now proves the permission gate blocks access even when the target session ID genuinely exists in the database. Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Address review feedback: add post-conditions to bot session revoke tests - TestRevokeSessionBotPermissions: after RevokeSession succeeds, assert GetSessionById returns an error to confirm the row is gone. - TestRevokeAllSessionsForUserBotPermissions: seed a real session before RevokeAllSessions so the call is not a no-op, then assert GetSessions returns an empty list afterwards. Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> Co-authored-by: Mattermost Build <build@mattermost.com> |
||
|
|
669eb104c6 |
Fix webhook list ordering instability when paginating (MM-65732) (#36470)
* Fix webhook list ordering instability when paginating (MM-65732) The webhook list view reorders entries when navigating between pages. The first page initially shows webhooks in insertion order (from the server), but after loading additional pages the display settles into alphabetical order. Going back to page 1 then shows different items than were originally visible. Root causes: 1. Server: GetIncomingByTeamByUser, GetIncomingListByUser, GetOutgoingByTeamByUser, and GetOutgoingListByUser had no ORDER BY clause, so the database could return rows in any order. 2. Client (incoming webhooks): incomingWebhookCompare only resolved the channel-name fallback for the 'a' argument, not 'b', making the comparator asymmetric and producing an unstable sort. 3. Client: both installed_incoming_webhooks and installed_outgoing_webhooks called Array.prototype.sort() directly on the props array, mutating it. Fix: - Add ORDER BY DisplayName, Id to the four listing SQL queries so API pages always come back in alphabetical order. With a stable server order, the client sort over merged pages produces the same slice for each page number regardless of how many pages have been loaded. - Symmetrise incomingWebhookCompare by applying the same channel-name and 'Private Webhook' fallback to the 'b' argument. - Sort a copy ([...hooks].sort()) in both webhook list components so the original prop arrays are never mutated. Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Fix lint: remove space before JSX closing tag in webhook test Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Fold ordering tests into existing webhook store test functions Instead of four separate top-level test registrations (GetIncomingListByUserOrdering, etc.), each ordering assertion is now a t.Run sub-test inside its corresponding existing function: testWebhookStoreGetIncomingListByUser └─ "GetIncomingListByUser, ordered alphabetically by display name" TestWebhookStoreGetIncomingByTeamByUser └─ "GetIncomingByTeamByUser, ordered alphabetically by display name" testWebhookStoreGetOutgoingListByUser └─ "GetOutgoingListByUser, ordered alphabetically by display name" testWebhookStoreGetOutgoingByTeamByUser └─ "GetOutgoingByTeamByUser, ordered alphabetically by display name" Each sub-test creates fresh hooks (Charlie, Alpha, Bravo in insertion order) scoped to its own IDs so they do not interfere with the outer test's fixtures. Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Fix govet shadow and gofmt issues in webhook store tests - Rename the outer err variable to errSave in three functions (testWebhookStoreGetIncomingListByUser, TestWebhookStoreGetIncomingByTeamByUser, testWebhookStoreGetOutgoingByTeamByUser) so that hooks, err := declarations in sub-test closures no longer shadow it. - Change hookC, err = to hookC, err := in each ordering sub-test to declare a local err instead of capturing the outer one. - Remove a trailing blank line at the end of the file (gofmt). Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Remove jest.mock from installed_incoming_webhooks test The mock for delete_integration_link was copied from the outgoing webhooks list test but is not needed here: the real component renders fine within renderWithContext (as shown by installed_incoming_webhook.test.tsx which tests the individual item without any mocks). Since the ordering tests do not interact with delete functionality, drop the mock and align the action stub style to mockReturnValue(Promise.resolve()). Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> * Add ORDER BY to GetOutgoingByChannelByUser; add ordering sub-test GetOutgoingByChannelByUser was the last paginated webhook listing function without an ORDER BY clause. Add OrderBy("DisplayName", "Id") consistent with all other listing functions. Add the corresponding ordering sub-test inside testWebhookStoreGetOutgoingByChannelByUser, following the same errSave pattern established for the other functions to avoid govet shadow warnings. Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com> |
||
|
Alejandro García MontoroandMattermost Build
|
238867e247 |
MM-68732: Remove global mutex for login attempts in favour of database serialization (#36515)
* Add atomic login-attempt counter primitives to UserStore
Two new store methods back the upcoming switch from a global
per-node mutex to per-user atomic slot claiming:
TryIncrementFailedPasswordAttempts(userID, maxAttempts) (bool, error)
UPDATE Users SET FailedAttempts = FailedAttempts + 1
WHERE Id = ? AND FailedAttempts < maxAttempts
Returns true when a slot was claimed (rows affected == 1) and
false when the cap was already reached. The conditional UPDATE
serialises concurrent attempts on the same user via the row
lock, so the cap is enforced without any application-level
locking and without serialising attempts across users.
DecrementFailedPasswordAttempts(userID) error
UPDATE Users SET FailedAttempts = FailedAttempts - 1
WHERE Id = ? AND FailedAttempts > 0
Releases a slot previously claimed by TryIncrement when the
in-flight authentication turns out not to be a credential
failure. The conditional UPDATE means concurrent decrements
cannot underflow.
Storetest covers both primitives: claim-below-cap, reject-at-cap,
reject-above-cap, no-op for unknown user, and a 50-goroutine
concurrent test with a start barrier asserting exactly
maxAttempts slots are ever claimed and that decrement clamps at
zero under contention.
The testify mock is regenerated here so the storetest package
that returns *mocks.UserStore as a store.UserStore still satisfies
the interface; the wrapper layers are regenerated in the next
commit.
------
AI assisted commit
* Regenerate store layers for the new primitives
Pick up TryIncrementFailedPasswordAttempts and
DecrementFailedPasswordAttempts in every generated wrapper:
- retrylayer: retry on repeatable errors using the standard
three-attempt loop.
- timerlayer: record store-method duration metrics under
UserStore.TryIncrementFailedPasswordAttempts and
UserStore.DecrementFailedPasswordAttempts.
- localcachelayer: invalidate the profile cache only after the
underlying conditional UPDATE actually changes a row; an
at-cap no-op return on TryIncrement no longer produces
unnecessary cluster invalidation traffic.
------
AI assisted commit
* Drop login-attempt mutex; use per-user slot claiming
Replace the global per-node mutex that serialised every login
attempt with the database-side atomic slot machine added on the
Users row. Each of the three authentication entry points now
pre-claims a slot via TryIncrementFailedPasswordAttempts before
running the expensive password / LDAP / MFA check, and releases
the slot when the failure path is not a real credential mismatch:
- CheckPasswordAndAllCriteria (email/password): refunds the
slot on backend errors during the password check (malformed
stored hash, hasher misc failure, password-migration write
failure) so a transient infra issue cannot ratchet
FailedAttempts to a lockout for a user with valid credentials;
refunds on the MFA pre-flight probe (empty mfaToken on an
MFA-enabled user) so the probe is not counted as a real
attempt.
- DoubleCheckPassword: same backend-error refund predicate.
- checkLdapUserPasswordAndAllCriteria: pre-claims only for
existing users (first-time LDAP users have no local row to
claim against); refunds non-credential DoLogin errors (server
unreachable, transient) so an LDAP outage cannot lock out
everyone; refunds the MFA pre-flight probe; for first-time
users, explicitly bumps the counter via UpdateFailedPasswordAttempts
on a real bad-password or bad-MFA attempt, matching the
pre-refactor counting behaviour.
If the refund itself fails the underlying authentication error is
preserved and returned to the caller (the failure is logged); a
leaked slot is annoying, but masking the real failure with a
generic store 500 would be a clear observability regression.
Cluster-wide behaviour also changes: the previous design honoured
MaximumLoginAttempts per node, so an n-node cluster effectively
permitted n * MaximumLoginAttempts attempts. The cap is now
enforced globally.
------
AI assisted commit
* Cover app-layer behaviors of the new login slot machine
The store-layer tests already exercise TryIncrement and Decrement
under concurrency and at the cap boundary. The new behavioural
contracts at the app layer were not covered, so a regression that
flipped a refund predicate, a probe condition, or a first-time
LDAP path would have slipped through type checking and existing
unit tests.
Add tests around the three callers of the new path:
- CheckPasswordAndAllCriteria: an MFA pre-flight probe (empty
token) does not consume a slot; a real attempt with a wrong
non-empty token does; a backend error during the password
check (malformed stored hash) refunds the slot; the happy
path also asserts FailedAttempts resets to zero.
- DoubleCheckPassword: gets its first test coverage, covering
the happy path, rate-limit rejection once max attempts is
reached, and the backend-error refund path.
- checkLdapUserPasswordAndAllCriteria: covers paths the table
loop did not exercise, first-time LDAP user with a bad
password (uses GetUserByAuth to reach the freshly created
row), first-time LDAP user with a wrong MFA token, existing
LDAP user with a non-credential DoLogin error (slot
refunded), and the existing LDAP user MFA pre-flight probe
(slot refunded).
------
AI assisted commit
* Address coderabbit review
------
AI assisted commit
* Fix race in first-time LDAP failed-attempt counter
For first-time LDAP users we have no local row to pre-claim, so
the bad-password and bad-MFA branches fell back to an absolute
UpdateFailedPasswordAttempts(id, ldapUser.FailedAttempts+1) based
on a snapshot from GetUserByAuth. Concurrent first-attempt
requests for the same user could all read FailedAttempts == 0 and
all write 1, losing increments. As a secondary issue the absolute
set did not enforce MaximumLoginAttempts, so the counter could
also drift past the cap.
Switch both branches to TryIncrementFailedPasswordAttempts, the
atomic conditional UPDATE already used on every other path. The
row lock serialises concurrent increments and the predicate caps
at MaximumLoginAttempts.
A new concurrent storetest-style subtest runs
3 * maxFailedLoginAttempts goroutines through the first-time
bad-password path against the same fresh LDAP row and asserts
FailedAttempts lands at exactly maxFailedLoginAttempts. Against
the previous absolute-set implementation the test fails (observed
FailedAttempts = 4 with maxFailedLoginAttempts = 3, either a lost
increment or a cap overshoot).
The first-time bad-password branch also switches from a wrapped
500 return on store error to log-and-continue, matching the rest
of the file's refund/probe error handling: the underlying LDAP
authentication failure is the more useful error for the caller.
------
AI assisted commit
* Address review comments
------
AI assisted commit
---------
Co-authored-by: Mattermost Build <build@mattermost.com>
|
||
|
sabril
|
8eb97fa6c3 |
refactor: remove redundant status update jobs from E2E test workflows (#36579)
* refactor: remove redundant status update jobs from E2E test workflows * refactor: rename context-name to commit-status-context in E2E test workflows |
||
|
Ben Cooke
|
02023f0328 | [MM-68463] New endpoint to GET user by auth_data (#36352) | ||
|
|
deafd88fd5 |
MM-68762: Discoverable Private Channels — Server data layer (#36539)
* MM-68762: Add Postgres migrations for discoverable private channels Three online-safe migrations introduce the schema that supports the Discoverable Private Channels feature (PRs 2-5 of MM-68430 will land behind it): - 000175 adds Channels.Discoverable BOOLEAN NOT NULL DEFAULT FALSE. Metadata-only on Postgres >= 11; no table rewrite. - 000176 creates a partial index on (TeamId) WHERE Discoverable AND Type='P' AND DeleteAt=0 using CREATE INDEX CONCURRENTLY (-- morph:nontransactional) so the build never blocks writes on the populated Channels table. - 000177 creates the ChannelJoinRequests table with three indexes, the important one being the partial unique index on (ChannelId, UserId) WHERE Status = 'pending'. That keeps the full audit history intact while still enforcing at-most-one active pending request per (channel, user). Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add FeatureFlagDiscoverableChannels (default false) Gates the per-channel Discoverable toggle and the channel-join-request flow. Default-OFF so all PRs in the MM-68430 series can land on master without exposing partial UX. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add Discoverable + ChannelJoinRequest models - Channel gains a Discoverable bool, ChannelPatch a *bool, both serialized as 'discoverable'. Patch() applies it, Auditable() logs it, and IsValid() rejects Discoverable=true on any non-private channel so a misconfigured patch can never produce a public discoverable channel. - New ChannelJoinRequest type captures the per-row state of a non-member's request: pending -> approved | denied | withdrawn. Rows are append-only with reviewer and timestamps so the table is also the audit trail. IsValid() enforces: * recognized status, * Message and DenialReason rune limits, * DenialReason only on denied rows (no orphan reasons), * reviewer + reviewed_at present for any terminal review (approved / denied) but not for self-service withdrawal. - Two new WebSocket event constants -- channel_join_request_created and channel_join_request_updated -- that later PRs broadcast on the admin queue and the requester's My Pending Requests panel. Unit tests cover Patch(), the new IsValid() rule on Discoverable, the PreSave/PreUpdate timestamp behavior on ChannelJoinRequest, and every IsValid branch including the reviewer-required-on-review invariant. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add discoverable-channel permissions Two new channel-scoped permissions, each independently rebindable from the System Console: - manage_private_channel_discoverability gates the per-channel toggle so admins can restrict who can flip discoverability without also handing out manage_private_channel_properties. - manage_channel_join_requests gates the queue list / approve / deny / count endpoints (added in PR 2). Both are added to the channel_admin role bootstrap so new deployments get them by default, and a new permissions migration (add_discoverable_channel_permissions) grants them to channel_admin, team_admin and system_admin scheme roles on existing deployments. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add ChannelJoinRequestStore and wire Discoverable into channel store - channelSliceColumns / channelToSlice / updateChannelT now include the new Discoverable column so Save() and Update() round-trip the field. Existing select paths inherit the column automatically because every read goes through channelSliceColumns. - New ChannelJoinRequestStore interface and SQL implementation: Save / Get / GetPendingForChannelAndUser / GetForChannel / GetForUser / Update / CountPending. Save translates the idx_channeljoinrequests_pending_unique partial unique index violation into store.ErrConflict so the app layer (PR 2) can return 409 without re-parsing pq errors. - Storetest suite at storetest/channel_join_request_store.go is invoked from sqlstore via the existing StoreTest harness; covers insert / partial-unique conflict / re-insert after withdrawal / NotFound / status filtering / pagination with TotalCount / Update / CountPending. - Mocks and retrylayer / timerlayer are regenerated via make store-mocks and go generate ./channels/store -- no hand-written generator output. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add TS types for Discoverable channels + join requests webapp/platform/types: - Channel.discoverable?: boolean alongside existing policy_enforced / policy_is_active so the web client sees the same wire shape the server emits. - ChannelJoinRequest, ChannelJoinRequestStatus, ChannelJoinRequestList, GetChannelJoinRequestsOptions for the API contract surfaced in PR 2. webapp/platform/client: - WebSocketEvents enum gains ChannelJoinRequestCreated and ChannelJoinRequestUpdated so PR 3 can hang WS handlers off them without redeclaring constants. These are model-only updates with no UI consumer yet; PR 3 introduces the toggle, request flow, and admin queue surfaces. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Split ChannelJoinRequests indexes into concurrent migrations The mattermost-govet concurrentIndex lint check enforces CREATE INDEX CONCURRENTLY on every CREATE INDEX statement, even on an empty freshly-created table where it would be a no-op. The original 000177 file inlined three CREATE INDEX statements; that failed check-style. Mirror the convention used by 000166_create_views + 000167_create_views_channel_id_delete_at_index: keep the CREATE TABLE in its own (transactional) file, and move each index into a separate nontransactional file that runs CREATE INDEX CONCURRENTLY. Verified locally against Postgres 15 that all four new migrations apply in order and the storetest suite (partial unique constraint + paged list + count) still passes. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Wire new permission migration into test fixtures Two CI test surfaces missed when the channel_admin role and the permission-migration list gained the new manage_private_channel_discoverability and manage_channel_join_requests entries: - testlib/store.go: the shared mocked SystemStore used by SetupWithStoreMock / SetupEnterpriseWithStoreMock needs an explicit GetByName expectation for every migration key (because the mock panics on unexpected calls). Add the new MigrationKeyAddDiscoverableChannelPermissions key so TestCreateOrUpdateAccessControlPolicy, the elasticsearch aggregation_job_test, and every other mock-store test stop panicking on server bootstrap. - cmd/mmctl/commands/permissions_test.go: TestResetPermissionsCmd hard-codes the channel_admin default permission list and expects PatchRole to be called with exactly that slice. Extend the expected slice with the two new permission ids so the mmctl reset path stays in sync with the role bootstrap. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Register new idx_channels_discoverable_team in TestGetSchemaDefinition The schema-dump test asserts an exact index count and definition map for the channels table. Migration 000176 added idx_channels_discoverable_team — a partial btree on (teamid) gated by discoverable=true AND type='P' AND deleteat=0. Bump the expected count from 12 to 13 and add the index's CREATE INDEX definition as produced by pg_indexes (note: type is cast to channel_type, the existing domain). Verified locally against Postgres 15. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Fix golangci-lint findings in ChannelJoinRequest store Two golangci-lint findings on the freshly-added files: - sqlstore/channel_join_request_store.go:133 (modernize): collapse the 'if page < 0 { page = 0 }' clamp into max(opts.Page, 0). - storetest/channel_join_request_store.go:243 (govet shadow): the inner Save loop redeclared err with :=, shadowing the outer err captured from the first CountPending call. Switch to plain assignment so the same err is reused. Verified locally with golangci-lint v2.11.4 across public/..., channels/app/..., channels/store/..., channels/testlib/... and cmd/mmctl/commands/... — 0 issues. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Sync channel_admin bootstrap with TestDoAdvancedPermissionsMigration app_test.go pins the exact list of permissions the channel_admin role is expected to hold after DoAdvancedPermissionsMigration completes. The role bootstrap in role.go grew two entries (manage_private_channel_discoverability and manage_channel_join_requests), so the test's expected slice needs the same two entries appended in the same order, otherwise assert.Equal fails on slice ordering. This is the same class of fix as the mmctl/permissions_test.go change in a previous commit -- two parallel test fixtures encode the channel_admin defaults and have to be updated in lockstep with the bootstrap. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Add English translations for new model error keys 12 keys were emitted by the new Discoverable + ChannelJoinRequest validation paths but had no en.json entry, which trips i18n-check on CI. Add the missing entries with one-line English copy that mirrors adjacent model errors (Invalid <field>., Create at must be a valid time., etc.). The new entries are: - model.channel.is_valid.discoverable.app_error - model.channel_join_request.is_valid.channel_id.app_error - model.channel_join_request.is_valid.create_at.app_error - model.channel_join_request.is_valid.denial_reason.app_error - model.channel_join_request.is_valid.denial_reason_status.app_error - model.channel_join_request.is_valid.id.app_error - model.channel_join_request.is_valid.message.app_error - model.channel_join_request.is_valid.reviewed_by.app_error - model.channel_join_request.is_valid.reviewer.app_error - model.channel_join_request.is_valid.status.app_error - model.channel_join_request.is_valid.update_at.app_error - model.channel_join_request.is_valid.user_id.app_error Generated through 'make i18n-extract'; verified clean with 'make i18n-check'. Per the workspace rule, only en.json was modified -- no other locale files. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Address CodeRabbit review: stable pagination + redact denial reason from audit log Two production-code findings from CodeRabbit on the freshly-added ChannelJoinRequest server code: - sqlstore/channel_join_request_store.go (GetForChannel / GetForUser): OrderBy("CreateAt DESC") alone is unstable when two rows share a millisecond (NewId is monotonic-ish but CreateAt is millisecond resolution), so offset paging could duplicate or skip rows between pages. Add Id DESC as a deterministic tie-breaker on both list queries. - model/channel_join_request.Auditable: the denial reason is admin-typed free text and could carry sensitive content. Mirror the existing has_message pattern by emitting has_denial_reason as a boolean presence flag instead of the raw value. Reviewer id, review timestamp, and status are still logged, so the audit trail keeps every piece needed for compliance review. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Tighten model tests per CodeRabbit review Two test-only findings from CodeRabbit: - TestChannelJoinRequestPreUpdateAdvancesUpdateAt previously asserted GreaterOrEqual(r.UpdateAt, originalCreate). Because validRequest initialises UpdateAt to GetMillis() (same call site as CreateAt), a no-op PreUpdate would still pass that check. Seed r.UpdateAt = 1 before calling PreUpdate() and assert Greater(r.UpdateAt, int64(1)) so any regression that drops the GetMillis assignment fails the test. - TestChannelIsValidDiscoverable did not cover ChannelTypeGroup. Add the case alongside ChannelTypeOpen and ChannelTypeDirect so the contract that 'only ChannelTypePrivate accepts Discoverable=true' is fully pinned across all four channel types. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> * MM-68762: Mock ChannelJoinRequest accessor in retrylayer test retrylayer_test.go's genStore() helper mocks every Store() accessor because retrylayer.New() wraps the entire surface. The new ChannelJoinRequest() method I added on Store was missing from the mock, so TestRetry/on_regular_error_should_not_retry panicked with 'Unexpected Method Call ChannelJoinRequest()' on Postgres shard 0. Add the mock alongside the other accessors. No production code change. Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com> |
||
|
Jesse HallamandMattermost Build
|
3f3d8408b2 |
Return descriptive errors from Role.IsValid and Role.IsValidWithoutId (#36582)
* Return descriptive errors from Role.IsValid and Role.IsValidWithoutId Previously both methods returned bool, leaving callers with no context about which validation check failed. Now both return error with a message identifying the specific constraint that was violated. * Add tests for Role.IsValid and Role.IsValidWithoutId * Log migration key on doPermissionsMigration failure --------- Co-authored-by: Mattermost Build <build@mattermost.com> |
||
|
|
6aae94f20b |
Add Display Name to User Properties in Webapp (#36363)
* Phase 1: CPA display_name + CEL-safe name validation (server) - Add typed DisplayName field to CPAAttrs + display_name attr key constant. - Add ValidateCPAFieldName helper enforcing CEL IDENTIFIER + reserved-word blacklist. - Wire validation into App.CreateCPAField (always) and App.PatchCPAField (lenient grandfather: skip when Name unchanged). - Trim + 255-rune cap DisplayName in CPAField.SanitizeAndValidate. - Developer-facing godoc note documenting rule, sources of truth, and Option C scoping. - Asserting test for documented Option C plugin-API bypass (closed by PR #36173). Spec: planner/projects/property-display-name/ideas/001-cpa-display-name/spec.md Plan: .planning/phase-1/PLAN.md Made-with: Cursor * Phase 1 (review): address Reza's Major + Minor findings - Rename misleading subtest "empty DisplayName is omitted from attrs" to "empty DisplayName round-trips as empty string" (Major #1). - Add TestCPAAttrs_JSONOmitEmpty pinning the omitempty wire-format contract that PR #36173's typed-attrs strategy relies on (Major #1). - Extend TestValidateCPAFieldName: case-sensitivity (IN/In ok), single-character names (a/_/A ok), missing "as" reserved word (Minor #2). Add whitespace-only DisplayName case (Minor #2). - Document PropertyFieldNameMaxRunes reuse in SanitizeAndValidate to prevent drift (Minor #3). - Replace broken PLAN-server.md reference in bypass-test docstring with in-tree CPAAttrs godoc reference (Minor #4). - Document omitempty semantics on CPAAttrs.DisplayName field to prevent the same misreading caught in review (Minor #5). - Document grouping intent above CPAFieldNameReservedWords (Minor #8). Review: .planning/phase-1/REVIEW.md Made-with: Cursor * Phase 2: in-app backfill migration for CPA display_name - Add cpaDisplayNameBackfillKey + cpaDisplayNameBackfillVersion constants. - Implement (*Server).doSetupCPADisplayNameBackfill: idempotent, cursor-paged scan over CPA group fields; backfill attrs.display_name = name when empty. - Register in m1 migration slice in doAppMigrations (mlog.Fatal on error, matching existing convention). - Three migration tests: NoExistingFields, BackfillsMissing, Idempotent. System-key idempotency + per-field DisplayName-empty check together provide HA-safe behavior on rolling deploys (last-write-wins on the System key; data-level idempotency from the per-field check). Spec: planner/projects/property-display-name/ideas/001-cpa-display-name/spec.md Plan: .planning/phase-2/PLAN.md Made-with: Cursor * Phase 2 (review): document race + harden idempotency test - Document SearchPropertyFields→UpdatePropertyFields rolling-deploy race: stale snapshot can revert concurrent admin CPA rename. Pre- existing systemic shape (no UpdateAt optimistic-lock); narrow window; bounded blast radius (admin re-rename, ABAC ID-keyed). Accepted limitation per spec Out of Scope (Major #1, Option C). - Tighten TestCPADisplayNameBackfill_Idempotent: snapshot UpdateAt before second run; assert no DB write on the System key or the field row (Major #2). - Extract clearCPABackfillMarker helper with explanatory godoc to centralize the 3x-repeated test precondition (Minor #1). - Comment fieldA seed as the "key-present-as-empty-string" idempotency boundary case (Minor #6). - Add godoc to doSetupCPADisplayNameBackfill (Minor #10). Review: .planning/phase-2/REVIEW.md Made-with: Cursor * Linting * Removing unnecessary comments * Clean up tests * Linting * Fix tests * Updated API doc * Phase 3: webapp helper + render-site migration for CPA display_name - Add display_name?: string to UserPropertyField.attrs type. - New getUserPropertyFieldLabel(field) helper: returns attrs.display_name?.trim() || name. Defensive against missing attrs. - Migrate ~10 user-facing CPA-name render sites to the helper: profile popover, user settings general (4 usages incl. line 1673 missed by high-level plan), admin user detail, admin CPA list (2 usages), and ABAC editor's selected-attribute UI (3 usages incl. the button label found in planning-stage research). - CEL paths (table_editor, attribute_selector_menu user.attributes expression construction, ABAC search filters) keep using `name` per spec — display_name is label-only. - Phase 4 boundary marker: TODOs in admin table + delete modal for follow-up admin-edit UX + client-side validator. Spec: planner/projects/property-display-name/ideas/001-cpa-display-name/spec.md Plan: .planning/phase-3/PLAN.md * Phase 3 (review): add Unicode test + correct helper docblock scope Address Reza's Phase 3 review: - Major #1: add missing test case for non-ASCII display_name (Latin-extended + CJK), pinning the trim/passthrough contract. - Nitpick #3: correct the helper's JSDoc to reflect that the delete modal is intentionally not migrated until Phase 4. No production behavior change. No new dependencies. Made-with: Cursor * Phase 4: admin CPA edit UX + client-side identifier validation Made-with: Cursor * docs: append Phase 4 implementation summary Made-with: Cursor * Phase 4: admin CPA edit UX + client-side identifier validation Complete the Phase 4 takeover from the existing dirty worktree and record the verified Stage 2 scope for admin CPA display-name editing, client-side identifier validation, and the required grandfather regression follow-ups. Document the targeted Jest, typecheck, and lint-equivalent validation results in the Phase 4 plan without widening the implementation scope or rewriting the prior in-scope work. Made-with: Cursor * docs: finalize Phase 4 implementation summary Made-with: Cursor * docs: correct Phase 4 summary commit reference Made-with: Cursor * Phase 4 (review): fix empty-name warning precedence Required-name validation now short-circuits before uniqueness checks so empty identifiers keep the correct warning. Add duplicate collision regression coverage for the dot-menu flow and add a stable validation-error testid for Phase 5 automation. Made-with: Cursor * Test updates * Fix merge issue * Fix tests * PR Feedback * Move migration to PropertyService * Updates to UX * Comment cleanup * Add webapp tests for CPA display_name and fix CEL-affected specs Update E2E seeds to use CEL-safe identifiers with display_name, add ABAC selector spec, and extend Jest coverage for label-rendering sites, auto-fill guard rails, and required-warning suppression. Co-authored-by: Cursor <cursoragent@cursor.com> * Remove .planning/phase-4/PLAN.md This planning artifact was committed inadvertently and should not be part of the codebase. Co-authored-by: Cursor <cursoragent@cursor.com> * Address CodeRabbit review comments - Fix e2e test to use display_name in label assertions - Make getIncrementedCELName case-insensitive to prevent collisions - Update tooltip to mention reserved CEL words - Replace hasSpaces check with full CEL identifier validation - Use CPA_FIELD_NAME_MAX_RUNES for consistent maxLength - Fix race condition by removing global cleanupAllFields - Enable IntegratedBoards flag for legacy field seeding - Replace fixed sleeps with state-based waits in tests Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * Fix linting errors in getIncrementedCELName - Use camelCase for destructured delete_at parameter - Place dots on same line for method chaining Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * Remove unused imports in user_attributes_display_name.spec.ts - Remove unused deleteCustomProfileAttributes import - Remove unused getFieldsMap function - Remove unused FieldsMap type Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * Fix webapp test failure - remove htmlFor assertion The htmlFor attribute assertion was failing in the test environment, likely due to a testing library issue. The important functionality (displaying display_name in labels) is still properly tested. Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * Fix post-merge CI failures: i18n drift and Playwright Prettier - Re-extract webapp en.json so the identifier tooltip string matches user_properties_table.tsx (source of truth was already shortened in Phase 4; en.json was not regenerated). - Apply Prettier formatting to three CPA display_name Playwright specs (whitespace and import/expression collapsing only). No test logic changes. Co-authored-by: Cursor <cursoragent@cursor.com> * Address CodeRabbit feedback: use stable locators, add reserved words to tooltip, remove regex from hasText Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * Fix i18n drift: align defaultMessage with en.json for identifier tooltip Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * Comment cleanup * Slugify CPA duplicate names to snake_case slugifyForCEL now lowercases and inserts underscores at camel/PascalCase boundaries (e.g. MyField -> my_field, XMLParser -> xml_parser) so duplicated CPA fields get conventional snake_case names instead of preserving the source casing. Co-authored-by: Cursor <cursoragent@cursor.com> * UX improvements: CEL identifier tooltip, validation, and attribute picker dual-name display - Add info tooltip to the Attribute column header explaining CEL identifier rules - Add client-side CEL identifier validation (pattern + reserved words) with a descriptive error message - Show both display name and unique identifier in the policy attribute picker - Filter attribute picker search by both display name and unique name - Add display_name to UserPropertyField attrs TypeScript type - Expand "CEL" to "Common Expression Language (CEL)" in the attribute-spaces tooltip Co-authored-by: Cursor <cursoragent@cursor.com> * Linting * PR Feedback * Restore name limit * Fix tests * Revert stray comment block above TestCPADisplayNameBackfill_BackfillsProtectedSourceOnlyField Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * Revert extended fieldA comment in TestCPADisplayNameBackfill_BackfillsMissing Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com> * Fix E2E tests * Fix test --------- Co-authored-by: Mattermost Build <build@mattermost.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Maria A NunezandCursor
|
d75155b39d |
Add flaky test webhook notification (#36573)
* Add flaky test webhook notification Co-authored-by: Cursor <cursoragent@cursor.com> * Bound flaky test webhook request time Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> |